Updated behavior for split_answer_dict, get_score, and check_student_inputs (responsetypes.py)

common/lib/capa/capa/inputtypes.py

@@ -460,10 +460,10 @@ class JSInput(InputTypeBase):
     DO NOT USE! HAS NOT BEEN TESTED BEYOND 700X PROBLEMS, AND MAY CHANGE IN
     BACKWARDS-INCOMPATIBLE WAYS.
       Inputtype for general javascript inputs. Intended to be used with
-    customresponse.  
+    customresponse.
       Loads in a sandboxed iframe to help prevent css and js conflicts between
-    frame and top-level window. 
-    
+    frame and top-level window.
+
     iframe sandbox whitelist:
         - allow-scripts
         - allow-popups
@@ -474,9 +474,9 @@ class JSInput(InputTypeBase):
     window elements.
       Example:
 
-        <jsinput html_file="/static/test.html" 
-                 gradefn="grade" 
-                 height="500" 
+        <jsinput html_file="/static/test.html"
+                 gradefn="grade"
+                 height="500"
                  width="400"/>
 
      See the documentation in the /doc/public folder for more information.
@@ -500,7 +500,7 @@ class JSInput(InputTypeBase):
                 Attribute('width', "400"),       # iframe width
                 Attribute('height', "300")]      # iframe height
 
-        
+
 
     def _extra_context(self):
         context = {
@@ -510,11 +510,12 @@ class JSInput(InputTypeBase):
 
         return context
 
-        
+
 
 registry.register(JSInput)
 #-----------------------------------------------------------------------------
 
+
 class TextLine(InputTypeBase):
     """
     A text line input.  Can do math preview if "math"="1" is specified.
@@ -1373,8 +1374,6 @@ registry.register(AnnotationInput)
 class ChoiceTextGroup(InputTypeBase):
     """
     Groups of radiobutton/checkboxes with text inputs.
-    Allows for a "not enough information" option to be added
-    to problems with numerical answers.
 
     Examples:
     RadioButton problem

common/lib/capa/capa/responsetypes.py

@@ -2256,7 +2256,6 @@ class ChoiceTextResponse(LoncapaResponse):
         binary_choices, numtolerance_inputs = self._split_answers_dict(answer_dict)
         # Check the binary choices first.
         choices_correct = self._check_student_choices(binary_choices)
-        inputs_correct = True
         inputs_correct = self._check_student_inputs(numtolerance_inputs)
         # Only return correct if the student got both the binary
         # and numtolerance_inputs are correct
@@ -2376,7 +2375,6 @@ class ChoiceTextResponse(LoncapaResponse):
 
         Returns True if and only if all student inputs are correct.
         """
-
         inputs_correct = True
         for answer_name, answer_value in numtolerance_inputs.iteritems():
             # If `self.corrrect_inputs` does not contain an entry for

common/lib/capa/capa/tests/response_xml_factory.py

@@ -857,15 +857,15 @@ class ChoiceTextResponseXMLFactory(ResponseXMLFactory):
         choice_element.set("correct", correct)
         choice_element.text = text
         for inp in inputs:
-            # Add all of the inputs as children of this element
+            # Add all of the inputs as children of this choice
             choice_element.append(inp)
 
         return choice_element
 
     def _create_numtolerance_input_element(self, params):
         """
-        Creates a <numtolerance_input/> element with optionally
-        specified tolerance and answer.
+        Creates a <numtolerance_input/>  or <decoy_input/> element with
+        optionally specified tolerance and answer.
         """
         answer = params['answer'] if 'answer' in params else None
         # If there is not an answer specified, Then create a <decoy_input/>

common/lib/capa/capa/tests/test_responsetypes.py

@@ -1432,54 +1432,15 @@ class AnnotationResponseTest(ResponseTest):
 
 
 class ChoiceTextResponseTest(ResponseTest):
+    """
+    Class containing setup and tests for ChoiceText responsetype.
+    """
 
     from response_xml_factory import ChoiceTextResponseXMLFactory
     xml_factory_class = ChoiceTextResponseXMLFactory
 
-    one_choice_one_input = lambda itype, inst: inst._make_problem(
-        ("true", {"answer": "123", "tolerance": "1"}),
-        itype
-    )
-
-    one_choice_two_inputs = lambda itype, inst: inst._make_problem(
-        [("true", ({"answer": "123", "tolerance": "1"},
-         {"answer": "456", "tolerance": "10"}))
-         ],
-        itype
-    )
-
-    one_input_script = lambda itype, inst: inst._make_problem(
-        ("true", {"answer": "$computed_response", "tolerance": "1"}),
-        itype,
-        "computed_response = math.sqrt(4)"
-    )
-
-    one_choice_no_input = lambda itype, inst: inst._make_problem(
-        ("true", {}),
-        itype
-    )
-
-    two_choices_no_inputs = lambda itype, inst: inst._make_problem(
-        [("false", {}), ("true", {})],
-        itype
-    )
-
-    two_choices_one_input_1 = lambda itype, inst: inst._make_problem(
-        [("false", {}), ("true", {"answer": "123", "tolerance": "0"})],
-        itype
-    )
-
-    two_choices_one_input_2 = lambda itype, inst: inst._make_problem(
-        [("true", {}), ("false", {"answer": "123", "tolerance": "0"})],
-        itype
-    )
-
-    two_choices_two_inputs = lambda itype, inst: inst._make_problem(
-        [("true", {"answer": "123", "tolerance": "0"}),
-         ("false", {"answer": "999", "tolerance": "0"})],
-        itype
-    )
-
+    # `TEST_INPUTS` is a dictionary mapping from
+    # test_name to a representation of inputs for a test problem.
     TEST_INPUTS = {
         "1_choice_0_input_correct": [(True, [])],
         "1_choice_0_input_incorrect": [(False, [])],
@@ -1511,6 +1472,10 @@ class ChoiceTextResponseTest(ResponseTest):
         "2_choices_2_inputs_wrong_input": [(True, ["321"]), (False, [])]
     }
 
+    # `TEST_SCENARIOS` is a dictionary of the form
+    # {Test_Name" : (Test_Problem_name, correctness)}
+    # correctness represents whether the problem should be graded as
+    # correct or incorrect when the test is run.
     TEST_SCENARIOS = {
         "1_choice_0_input_correct": ("1_choice_0_input", "correct"),
         "1_choice_0_input_incorrect": ("1_choice_0_input", "incorrect"),
@@ -1542,15 +1507,53 @@ class ChoiceTextResponseTest(ResponseTest):
         "2_choices_2_inputs_wrong_input": ("2_choices_2_inputs", "incorrect")
     }
 
-    TEST_PROBLEMS = {
-        "1_choice_0_input": one_choice_no_input,
-        "1_choice_1_input": one_choice_one_input,
-        "1_input_script": one_input_script,
-        "1_choice_2_inputs": one_choice_two_inputs,
-        "2_choices_0_inputs": two_choices_no_inputs,
-        "2_choices_1_input_1": two_choices_one_input_1,
-        "2_choices_1_input_2": two_choices_one_input_2,
-        "2_choices_2_inputs": two_choices_two_inputs
+    # Dictionary that maps from problem_name to arguments for
+    # _make_problem, that will create the problem.
+    TEST_PROBLEM_ARGS = {
+        "1_choice_0_input": {"choices": ("true", {}), "script": ''},
+        "1_choice_1_input": {
+            "choices": ("true", {"answer": "123", "tolerance": "1"}),
+            "script": ''
+        },
+
+        "1_input_script": {
+            "choices": ("true", {"answer": "$computed_response", "tolerance": "1"}),
+            "script": "computed_response = math.sqrt(4)"
+        },
+
+        "1_choice_2_inputs": {
+            "choices": [
+                (
+                    "true", (
+                        {"answer": "123", "tolerance": "1"},
+                        {"answer": "456", "tolerance": "10"}
+                    )
+                )
+            ],
+            "script": ''
+        },
+        "2_choices_0_inputs": {
+            "choices": [("false", {}), ("true", {})],
+            "script": ''
+
+        },
+        "2_choices_1_input_1": {
+            "choices": [
+                ("false", {}), ("true", {"answer": "123", "tolerance": "0"})
+            ],
+            "script": ''
+        },
+        "2_choices_1_input_2": {
+            "choices": [("true", {}), ("false", {"answer": "123", "tolerance": "0"})],
+            "script": ''
+        },
+        "2_choices_2_inputs": {
+            "choices": [
+                ("true", {"answer": "123", "tolerance": "0"}),
+                ("false", {"answer": "999", "tolerance": "0"})
+            ],
+            "script": ''
+        }
     }
 
     def _make_problem(self, choices, in_type='radiotextgroup', script=''):
@@ -1598,26 +1601,69 @@ class ChoiceTextResponseTest(ResponseTest):
         return answer_dict
 
     def test_invalid_xml(self):
+        """
+        Test that build problem raises errors for invalid options
+        """
         with self.assertRaises(Exception):
             self.build_problem(type="invalidtextgroup")
 
     def test_valid_xml(self):
+        """
+        Test that `build_problem` builds valid xml
+        """
         self.build_problem()
         self.assertTrue(True)
 
+    def test_unchecked_input_not_validated(self):
+        """
+        Test that a student can have a non numeric answer in an unselected
+        choice without causing an error to be raised when the problem is
+        checked.
+        """
+
+        two_choice_two_input = self._make_problem(
+            [
+                ("true", {"answer": "123", "tolerance": "1"}),
+                ("false", {})
+            ],
+            "checkboxtextgroup"
+        )
+
+        self.assert_grade(
+            two_choice_two_input,
+            self._make_answer_dict([(True, ["1"]), (False, ["Platypus"])]),
+            "incorrect"
+        )
+
     def test_interpret_error(self):
-        one_choice_one_input = lambda itype: self._make_problem(
-            ("true", {"answer": "123", "tolerance": "1"}),
-            itype
+        """
+        Test that student answers that cannot be interpeted as numbers
+        cause the response type to raise an error.
+        """
+        two_choice_two_input = self._make_problem(
+            [
+                ("true", {"answer": "123", "tolerance": "1"}),
+                ("false", {})
+            ],
+            "checkboxtextgroup"
         )
 
         with self.assertRaisesRegexp(StudentInputError, "Could not interpret"):
+            # Test that error is raised for input in selected correct choice.
             self.assert_grade(
-                one_choice_one_input('radiotextgroup'),
+                two_choice_two_input,
                 self._make_answer_dict([(True, ["Platypus"])]),
                 "correct"
             )
 
+        with self.assertRaisesRegexp(StudentInputError, "Could not interpret"):
+            # Test that error is raised for input in selected incorrect choice.
+            self.assert_grade(
+                two_choice_two_input,
+                self._make_answer_dict([(True, ["1"]), (True, ["Platypus"])]),
+                "correct"
+            )
+
     def test_staff_answer_error(self):
         broken_problem = self._make_problem(
             [("true", {"answer": "Platypus", "tolerance": "0"}),
@@ -1638,14 +1684,26 @@ class ChoiceTextResponseTest(ResponseTest):
             )
 
     def test_radio_grades(self):
+        """
+        Test that confirms correct operation of grading when the inputtag is
+        radiotextgroup.
+        """
 
         for name, inputs in self.TEST_INPUTS.iteritems():
+            # Turn submission into the form expected when grading this problem.
             submission = self._make_answer_dict(inputs)
+            # Lookup the problem_name, and the whether this test problem
+            # and inputs should be graded as correct or incorrect.
             problem_name, correctness = self.TEST_SCENARIOS[name]
-            problem = self.TEST_PROBLEMS[problem_name]
-
+            # Load the args needed to build the problem for this test.
+            problem_args = self.TEST_PROBLEM_ARGS[problem_name]
+            test_choices = problem_args["choices"]
+            test_script = problem_args["script"]
+            # Build the actual problem for the test.
+            test_problem = self._make_problem(test_choices, 'radiotextgroup', test_script)
+            # Make sure the actual grade matches the expected grade.
             self.assert_grade(
-                problem('radiotextgroup', self),
+                test_problem,
                 submission,
                 correctness,
                 msg="{0} should be {1}".format(
@@ -1655,9 +1713,16 @@ class ChoiceTextResponseTest(ResponseTest):
             )
 
     def test_checkbox_grades(self):
+        """
+        Test that confirms correct operation of grading when the inputtag is
+        checkboxtextgroup.
+        """
+        # Dictionary from name of test_scenario to (problem_name, correctness)
+        # Correctness is used to test whether the problem was graded properly
         scenarios = {
             "2_choices_correct": ("checkbox_two_choices", "correct"),
             "2_choices_incorrect": ("checkbox_two_choices", "incorrect"),
+
             "2_choices_2_inputs_correct": (
                 "checkbox_2_choices_2_inputs",
                 "correct"
@@ -1673,6 +1738,7 @@ class ChoiceTextResponseTest(ResponseTest):
                 "incorrect"
             )
         }
+        # Dictionary scenario_name: test_inputs
         inputs = {
             "2_choices_correct": [(True, []), (True, [])],
             "2_choices_incorrect": [(True, []), (False, [])],
@@ -1685,9 +1751,11 @@ class ChoiceTextResponseTest(ResponseTest):
             ]
         }
 
+        # Two choice zero input problem with both choices being correct.
         checkbox_two_choices = self._make_problem(
             [("true", {}), ("true", {})], "checkboxtextgroup"
         )
+        # Two choice two input problem with both choices correct.
         checkbox_two_choices_two_inputs = self._make_problem(
             [("true", {"answer": "123", "tolerance": "0"}),
              ("true", {"answer": "456", "tolerance": "0"})
@@ -1695,17 +1763,20 @@ class ChoiceTextResponseTest(ResponseTest):
             "checkboxtextgroup"
         )
 
+        # Dictionary problem_name: problem
         problems = {
             "checkbox_two_choices": checkbox_two_choices,
             "checkbox_2_choices_2_inputs": checkbox_two_choices_two_inputs
         }
-        problems.update(self.TEST_PROBLEMS)
 
         for name, inputs in inputs.iteritems():
             submission = self._make_answer_dict(inputs)
+            # Load the test problem's name and desired correctness
             problem_name, correctness = scenarios[name]
+            # Load the problem
             problem = problems[problem_name]
 
+            # Make sure the actual grade matches the expected grade
             self.assert_grade(
                 problem,
                 submission,

common/lib/xmodule/xmodule/capa_module.py

@@ -820,10 +820,11 @@ class CapaModule(CapaFields, XModule):
                 elif is_dict_key:
                     try:
                         val = json.loads(data[key])
+                    # If the submission wasn't deserializable, raise an error.
                     except(KeyError, ValueError):
-                        # Send this information along to be reported by
-                        # The grading method
-                        val = {"error": "error"}
+                        raise ValueError(
+                            u"Invalid submission: {val} for {key}".format(val=data[key], key=key)
+                        )
                 else:
                     val = data[key]
 

lms/djangoapps/courseware/features/problems_setup.py

@@ -388,8 +388,9 @@ def assert_choicetext_values(problem_type, choices, expected_values):
     Asserts that only the given choices are checked, and given
     text fields have a desired value
     """
-
+    # Names of the radio buttons or checkboxes
     all_choices = ['choiceinput_0bc', 'choiceinput_1bc']
+    # Names of the numtolerance_inputs
     all_inputs = [
         "choiceinput_0_numtolerance_input_0",
         "choiceinput_1_numtolerance_input_0"