Fix part of oppia#6550: Write tests for extensions/ and utils (oppia#…

…7113) * Write tests for utils.py * write tests for extensions/ * fix lint * fix tests * fix changes * address comments
saketringsia · Jul 17, 2019 · 243e374 · 243e374
1 parent 6b9de55
commit 243e374
Show file tree

Hide file tree

Showing 13 changed files with 155 additions and 103 deletions.
diff --git a/core/domain/visualization_registry_test.py b/core/domain/visualization_registry_test.py
@@ -19,6 +19,7 @@
 import importlib
 import inspect
 import os
+import re
 
 from core.domain import visualization_registry
 from core.tests import test_utils
@@ -39,6 +40,34 @@ def test_get_visualization_class_with_invalid_id_raises_error(self):
             visualization_registry.Registry.get_visualization_class(
                 'invalid_visualization_id')
 
+    def test_visualization_class_with_invalid_option_names(self):
+        bar_chart = visualization_registry.Registry.get_visualization_class(
+            'BarChart')
+        bar_chart_instance = bar_chart('AnswerFrequencies', {}, True)
+
+        with self.assertRaisesRegexp(
+            Exception,
+            re.escape(
+                'For visualization BarChart, expected option names '
+                '[\'x_axis_label\', \'y_axis_label\']; received names []')):
+            bar_chart_instance.validate()
+
+    def test_visualization_class_with_invalid_addressed_info_is_supported(self):
+        bar_chart = visualization_registry.Registry.get_visualization_class(
+            'BarChart')
+        option_names = {
+            'x_axis_label': 'Answer',
+            'y_axis_label': 'Count'
+        }
+        bar_chart_instance = bar_chart(
+            'AnswerFrequencies', option_names, 'invalid_value')
+
+        with self.assertRaisesRegexp(
+            Exception,
+            'For visualization BarChart, expected a bool value for '
+            'addressed_info_is_supported; received invalid_value'):
+            bar_chart_instance.validate()
+
 
 class VisualizationsNameTests(test_utils.GenericTestBase):
 

diff --git a/core/tests/data/dummy_assets/assets/example1.yaml b/core/tests/data/dummy_assets/assets/example1.yaml
diff --git a/core/tests/data/dummy_assets/assets/example2.yaml b/core/tests/data/dummy_assets/assets/example2.yaml
diff --git a/extensions/answer_summarizers/models_test.py b/extensions/answer_summarizers/models_test.py
@@ -30,6 +30,15 @@ def test_requires_override_for_calculation(self):
             answer_models.BaseCalculation().calculate_from_state_answers_dict(
                 state_answers_dict={})
 
+    def test_equality_of_hashable_answers(self):
+        hashable_answer_1 = answer_models._HashableAnswer('answer_1')  # pylint: disable=protected-access
+        hashable_answer_2 = answer_models._HashableAnswer('answer_2')  # pylint: disable=protected-access
+        hashable_answer_3 = answer_models._HashableAnswer('answer_1')  # pylint: disable=protected-access
+
+        self.assertFalse(hashable_answer_1 == hashable_answer_2)
+        self.assertTrue(hashable_answer_1 == hashable_answer_3)
+        self.assertFalse(hashable_answer_1 == 1)
+
 
 class CalculationUnitTestBase(test_utils.GenericTestBase):
     """Utility methods for testing calculations."""
@@ -74,9 +83,6 @@ def _create_state_answers_dict(
 
     def _get_calculation_instance(self):
         """Requires the existance of the class constant: CALCULATION_ID."""
-        if not hasattr(self, 'CALCULATION_ID'):
-            raise NotImplementedError(
-                'Subclasses must provide a value for CALCULATION_ID.')
         return calculation_registry.Registry.get_calculation_by_id(
             self.CALCULATION_ID)
 

diff --git a/extensions/interactions/base.py b/extensions/interactions/base.py
@@ -211,18 +211,6 @@ def html_body(self):
             feconf.INTERACTIONS_DIR, self.id, '%s.html' % self.id))
         return html_templates
 
-    @property
-    def validator_html(self):
-        """The HTML code containing validators for the interaction's
-        customization_args and submission handler.
-        """
-        return (
-            '<script>%s</script>\n' %
-            utils.get_file_contents(os.path.join(
-                feconf.INTERACTIONS_DIR,
-                self.id,
-                '%sValidationService.js' % self.id)))
-
     def to_dict(self):
         """Gets a dict representing this interaction. Only default values are
         provided.

diff --git a/extensions/interactions/base_test.py b/extensions/interactions/base_test.py
@@ -24,7 +24,6 @@
 from core.domain import dependency_registry
 from core.domain import exp_fetchers
 from core.domain import exp_services
-from core.domain import html_validation_service
 from core.domain import interaction_registry
 from core.domain import obj_services
 from core.tests import test_utils
@@ -49,11 +48,6 @@
     ('show_generic_submit_button', bool)]
 
 
-def mock_get_filename_with_dimensions(filename, unused_exp_id):
-    return html_validation_service.regenerate_image_filename_using_dimensions(
-        filename, 490, 120)
-
-
 class InteractionAnswerUnitTests(test_utils.GenericTestBase):
     """Test the answer object and type properties of an interaction object."""
 
@@ -70,6 +64,23 @@ def test_rules_property(self):
             interaction.answer_type = 'FakeObjType'
             interaction.normalize_answer('15')
 
+    def test_get_rule_description_with_invalid_rule_name_raises_error(self):
+        interaction = interaction_registry.Registry.get_interaction_by_id(
+            'CodeRepl')
+        with self.assertRaisesRegexp(
+            Exception, 'Could not find rule with name invalid_rule_name'):
+            interaction.get_rule_description('invalid_rule_name')
+
+    def test_get_rule_param_type_with_invalid_rule_param_name_raises_error(
+            self):
+        interaction = interaction_registry.Registry.get_interaction_by_id(
+            'CodeRepl')
+        with self.assertRaisesRegexp(
+            Exception,
+            'Rule CodeEquals has no param called invalid_rule_param_name'):
+            interaction.get_rule_param_type(
+                'CodeEquals', 'invalid_rule_param_name')
+
 
 class InteractionUnitTests(test_utils.GenericTestBase):
     """Test that the default interactions are valid."""
@@ -578,12 +589,9 @@ class InteractionDemoExplorationUnitTests(test_utils.GenericTestBase):
     _DEMO_EXPLORATION_ID = '16'
 
     def test_interactions_demo_exploration(self):
-        with self.swap(
-            html_validation_service, 'get_filename_with_dimensions',
-            mock_get_filename_with_dimensions):
-            exp_services.load_demo(self._DEMO_EXPLORATION_ID)
-            exploration = exp_fetchers.get_exploration_by_id(
-                self._DEMO_EXPLORATION_ID)
+        exp_services.load_demo(self._DEMO_EXPLORATION_ID)
+        exploration = exp_fetchers.get_exploration_by_id(
+            self._DEMO_EXPLORATION_ID)
 
         all_interaction_ids = set(
             interaction_registry.Registry.get_all_interaction_ids())

diff --git a/extensions/objects/models/objects_test.py b/extensions/objects/models/objects_test.py
@@ -498,3 +498,26 @@ def test_default_values_for_objects_are_valid(self):
                         member.normalize(member.default_value),
                         type(member.default_value),
                         msg=type_error_message)
+
+
+class NormalizedRectangleTests(test_utils.GenericTestBase):
+
+    def test_normalize(self):
+        normalized_rectangle = objects.NormalizedRectangle2D()
+        self.assertEqual(normalized_rectangle.normalize(
+            [[0, 1], [1, 0]]), [[0.0, 0.0], [0.0, 0.0]])
+
+        with self.assertRaisesRegexp(
+            TypeError, 'Cannot convert to Normalized Rectangle '):
+            normalized_rectangle.normalize('')
+
+
+class CodeStringTests(test_utils.GenericTestBase):
+
+    def test_normalize(self):
+        code_string = objects.CodeString()
+        self.assertEqual(code_string.normalize(code_string.default_value), '')
+
+        with self.assertRaisesRegexp(
+            TypeError, 'Unexpected tab characters in code string: \t'):
+            code_string.normalize('\t')
diff --git a/extensions/rich_text_components/components_test.py b/extensions/rich_text_components/components_test.py
@@ -38,14 +38,7 @@ def check_validation(self, rte_component_class, valid_items, invalid_items):
             a TypeError when validated.
         """
         for item in valid_items:
-            try:
-                rte_component_class.validate(item)
-            except Exception as e:
-                self.fail(
-                    msg=(
-                        'Unexpected exception %s raised during '
-                        'validation of %s' % (
-                            str(e), str(item))))
+            rte_component_class.validate(item)
 
         for item in invalid_items:
             with self.assertRaises(Exception):

diff --git a/extensions/value_generators/models/generators_test.py b/extensions/value_generators/models/generators_test.py
@@ -28,6 +28,8 @@ def test_copier(self):
         self.assertEqual(generator.generate_value({}, **{'value': 'a'}), 'a')
         self.assertEqual(generator.generate_value(
             {}, **{'value': 'a', 'parse_with_jinja': False}), 'a')
+        self.assertEqual(generator.generate_value(
+            None, **{'value': 'a', 'parse_with_jinja': False}), 'a')
         self.assertEqual(generator.generate_value(
             {}, **{'value': '{{a}}', 'parse_with_jinja': False}), '{{a}}')
         self.assertEqual(generator.generate_value(

diff --git a/extensions/visualizations/models.py b/extensions/visualizations/models.py
@@ -68,7 +68,7 @@ def validate(self):
             raise utils.ValidationError(
                 'For visualization %s, expected a bool value for '
                 'addressed_info_is_supported; received %s' %
-                self.addressed_info_is_supported)
+                (self.id, self.addressed_info_is_supported))
 
 
 class BarChart(BaseVisualization):

diff --git a/schema_utils_test.py b/schema_utils_test.py
@@ -273,11 +273,6 @@ def validate_schema(schema):
         _validate_ui_config(
             schema[SCHEMA_KEY_TYPE], schema[SCHEMA_KEY_UI_CONFIG])
 
-    if SCHEMA_KEY_CHOICES in schema and SCHEMA_KEY_POST_NORMALIZERS in schema:
-        raise AssertionError(
-            'Schema cannot contain both a \'choices\' and a '
-            '\'post_normalizers\' key.')
-
     if SCHEMA_KEY_POST_NORMALIZERS in schema:
         assert isinstance(schema[SCHEMA_KEY_POST_NORMALIZERS], list)
         for post_normalizer in schema[SCHEMA_KEY_POST_NORMALIZERS]:

diff --git a/utils.py b/utils.py
@@ -14,7 +14,6 @@
 
 """Common utility functions."""
 
-import StringIO
 import base64
 import collections
 import datetime
@@ -29,7 +28,6 @@
 import unicodedata
 import urllib
 import urlparse
-import zipfile
 
 from constants import constants  # pylint: disable=relative-import
 import feconf  # pylint: disable=relative-import
@@ -116,21 +114,20 @@ def get_exploration_components_from_dir(dir_path):
         for filename in files:
             filepath = os.path.join(root, filename)
             if root == dir_path:
-                if filepath.endswith('.DS_Store'):
-                    # These files are added automatically by Mac OS Xsystems.
-                    # We ignore them.
-                    continue
-
-                if yaml_content is not None:
-                    raise Exception('More than one non-asset file specified '
-                                    'for %s' % dir_path)
-                elif not filepath.endswith('.yaml'):
-                    raise Exception('Found invalid non-asset file %s. There '
-                                    'should only be a single non-asset file, '
-                                    'and it should have a .yaml suffix.' %
-                                    filepath)
-                else:
-                    yaml_content = get_file_contents(filepath)
+                # These files are added automatically by Mac OS Xsystems.
+                # We ignore them.
+                if not filepath.endswith('.DS_Store'):
+                    if yaml_content is not None:
+                        raise Exception(
+                            'More than one non-asset file specified '
+                            'for %s' % dir_path)
+                    elif not filepath.endswith('.yaml'):
+                        raise Exception(
+                            'Found invalid non-asset file %s. There '
+                            'should only be a single non-asset file, '
+                            'and it should have a .yaml suffix.' % filepath)
+                    else:
+                        yaml_content = get_file_contents(filepath)
             else:
                 filepath_array = filepath.split('/')
                 # The additional offset is to remove the 'assets/' prefix.
@@ -144,50 +141,6 @@ def get_exploration_components_from_dir(dir_path):
     return yaml_content, assets_list
 
 
-def get_exploration_components_from_zip(zip_file_contents):
-    """Gets the (yaml, assets) from the contents of an exploration zip file.
-
-    Args:
-        zip_file_contents: a string of raw bytes representing the contents of
-            a zip file that comprises the exploration.
-
-    Returns:
-        a 2-tuple, the first element of which is a yaml string, and the second
-        element of which is a list of (filepath, content) 2-tuples. The filepath
-        does not include the assets/ prefix.
-
-    Raises:
-        Exception: if the following condition doesn't hold: "There is exactly
-            one file not in assets/, and this file has a .yaml suffix".
-    """
-    memfile = StringIO.StringIO()
-    memfile.write(zip_file_contents)
-
-    zf = zipfile.ZipFile(memfile, 'r')
-    yaml_content = None
-    assets_list = []
-    for filepath in zf.namelist():
-        if filepath.startswith('assets/'):
-            assets_list.append('/'.join(filepath.split('/')[1:]),
-                               zf.read(filepath))
-        else:
-            if yaml_content is not None:
-                raise Exception(
-                    'More than one non-asset file specified for zip file')
-            elif not filepath.endswith('.yaml'):
-                raise Exception('Found invalid non-asset file %s. There '
-                                'should only be a single file not in assets/, '
-                                'and it should have a .yaml suffix.' %
-                                filepath)
-            else:
-                yaml_content = zf.read(filepath)
-
-    if yaml_content is None:
-        raise Exception('No yaml file specified in zip file contents')
-
-    return yaml_content, assets_list
-
-
 def get_comma_sep_string_from_list(items):
     """Turns a list of items into a comma-separated string.
 
@@ -552,7 +505,7 @@ def require_valid_name(name, name_type, allow_empty=False):
         allow_empty: bool. If True, empty strings are allowed.
     """
     if not isinstance(name, basestring):
-        raise ValidationError('%s must be a string.' % name_type)
+        raise ValidationError('%s must be a string.' % name)
 
     if allow_empty and name == '':
         return