Merge pull request #1735 from MITx/fix/will/bug_258

Fix bug #258: multiple choice with choice names ending with 0 cause multiple choice answers marked incorrect

common/lib/capa/capa/templates/choicegroup.html

@@ -17,7 +17,7 @@
 
         % for choice_id, choice_description in choices:
         <label for="input_${id}_${choice_id}"
-            % if input_type == 'radio' and choice_id in value:
+            % if input_type == 'radio' and choice_id == value:
             <% 
                 if status == 'correct':
                     correctness = 'correct'
@@ -32,8 +32,10 @@
             % endif
             >  
             <input type="${input_type}" name="input_${id}${name_array_suffix}" id="input_${id}_${choice_id}" value="${choice_id}"
-            % if choice_id in value:
+            % if input_type == 'radio' and choice_id == value:
             checked="true" 
+            % elif input_type != 'radio' and choice_id in value:
+            checked="true"
             % endif
 
             /> ${choice_description} </label>

lms/djangoapps/courseware/features/problems.feature

@@ -8,6 +8,7 @@ Feature: Answer problems
         And I am viewing a "<ProblemType>" problem
         When I answer a "<ProblemType>" problem "correctly"
         Then My "<ProblemType>" answer is marked "correct"
+        And The "<ProblemType>" problem displays a "correct" answer
 
         Examples:
         | ProblemType       |
@@ -25,6 +26,7 @@ Feature: Answer problems
         And I am viewing a "<ProblemType>" problem
         When I answer a "<ProblemType>" problem "incorrectly"
         Then My "<ProblemType>" answer is marked "incorrect"
+        And The "<ProblemType>" problem displays a "incorrect" answer
 
         Examples:
         | ProblemType       |
@@ -41,6 +43,7 @@ Feature: Answer problems
         Given I am viewing a "<ProblemType>" problem
         When I check a problem
         Then My "<ProblemType>" answer is marked "incorrect"
+        And The "<ProblemType>" problem displays a "blank" answer
 
         Examples:
         | ProblemType       |
@@ -58,6 +61,7 @@ Feature: Answer problems
         And I answer a "<ProblemType>" problem "<Correctness>ly"
         When I reset the problem
         Then My "<ProblemType>" answer is marked "unanswered"
+        And The "<ProblemType>" problem displays a "blank" answer
 
         Examples:
         | ProblemType       | Correctness   |

lms/djangoapps/courseware/features/problems.py

+'''
+Steps for problem.feature lettuce tests
+'''
+
+
 from lettuce import world, step
 from lettuce.django import django_url
 import random
 import textwrap
-import time
-from common import i_am_registered_for_the_course, TEST_SECTION_NAME, section_location
+from common import i_am_registered_for_the_course, \
+                TEST_SECTION_NAME, section_location
 from capa.tests.response_xml_factory import OptionResponseXMLFactory, \
     ChoiceResponseXMLFactory, MultipleChoiceResponseXMLFactory, \
     StringResponseXMLFactory, NumericalResponseXMLFactory, \
@@ -26,7 +31,7 @@ PROBLEM_FACTORY_DICT = {
         'kwargs': {
             'question_text': 'The correct answer is Choice 3',
             'choices': [False, False, True, False],
-            'choice_names': ['choice_1', 'choice_2', 'choice_3', 'choice_4']}},
+            'choice_names': ['choice_0', 'choice_1', 'choice_2', 'choice_3']}},
 
     'checkbox': {
         'factory': ChoiceResponseXMLFactory(),
@@ -88,6 +93,9 @@ PROBLEM_FACTORY_DICT = {
 
 
 def add_problem_to_course(course, problem_type):
+    '''
+    Add a problem to the course we have created using factories.
+    '''
 
     assert(problem_type in PROBLEM_FACTORY_DICT)
 
@@ -98,11 +106,12 @@ def add_problem_to_course(course, problem_type):
     # Create a problem item using our generated XML
     # We set rerandomize=always in the metadata so that the "Reset" button
     # will appear.
-    problem_item = world.ItemFactory.create(parent_location=section_location(course),
-                                            template="i4x://edx/templates/problem/Blank_Common_Problem",
-                                            display_name=str(problem_type),
-                                            data=problem_xml,
-                                            metadata={'rerandomize': 'always'})
+    template_name = "i4x://edx/templates/problem/Blank_Common_Problem"
+    world.ItemFactory.create(parent_location=section_location(course),
+                            template=template_name,
+                            display_name=str(problem_type),
+                            data=problem_xml,
+                            metadata={'rerandomize': 'always'})
 
 
 @step(u'I am viewing a "([^"]*)" problem')
@@ -152,9 +161,9 @@ def answer_problem(step, problem_type, correctness):
 
     elif problem_type == "multiple choice":
         if correctness == 'correct':
-            inputfield('multiple choice', choice='choice_3').check()
-        else:
             inputfield('multiple choice', choice='choice_2').check()
+        else:
+            inputfield('multiple choice', choice='choice_1').check()
 
     elif problem_type == "checkbox":
         if correctness == 'correct':
@@ -164,11 +173,13 @@ def answer_problem(step, problem_type, correctness):
             inputfield('checkbox', choice='choice_3').check()
 
     elif problem_type == 'string':
-        textvalue = 'correct string' if correctness == 'correct' else 'incorrect'
+        textvalue = 'correct string' if correctness == 'correct' \
+                                    else 'incorrect'
         inputfield('string').fill(textvalue)
 
     elif problem_type == 'numerical':
-        textvalue = "pi + 1" if correctness == 'correct' else str(random.randint(-2, 2))
+        textvalue = "pi + 1" if correctness == 'correct' \
+                            else str(random.randint(-2, 2))
         inputfield('numerical').fill(textvalue)
 
     elif problem_type == 'formula':
@@ -203,6 +214,67 @@ def answer_problem(step, problem_type, correctness):
     check_problem(step)
 
 
+@step(u'The "([^"]*)" problem displays a "([^"]*)" answer')
+def assert_problem_has_answer(step, problem_type, answer_class):
+    '''
+    Assert that the problem is displaying a particular answer.
+    These correspond to the same correct/incorrect
+    answers we set in answer_problem()
+
+    We can also check that a problem has been left blank
+    by setting answer_class='blank'
+    '''
+    assert answer_class in ['correct', 'incorrect', 'blank']
+
+    if problem_type == "drop down":
+        if answer_class == 'blank':
+            assert world.browser.is_element_not_present_by_css('option[selected="true"]')
+        else:
+            actual = world.browser.find_by_css('option[selected="true"]').value
+            expected = 'Option 2' if answer_class == 'correct' else 'Option 3'
+            assert actual == expected
+
+    elif problem_type == "multiple choice":
+        if answer_class == 'correct':
+            assert_checked('multiple choice', ['choice_2'])
+        elif answer_class == 'incorrect':
+            assert_checked('multiple choice', ['choice_1'])
+        else:
+            assert_checked('multiple choice', [])
+
+    elif problem_type == "checkbox":
+        if answer_class == 'correct':
+            assert_checked('checkbox', ['choice_0', 'choice_2'])
+        elif answer_class == 'incorrect':
+            assert_checked('checkbox', ['choice_3'])
+        else:
+            assert_checked('checkbox', [])
+
+    elif problem_type == 'string':
+        if answer_class == 'blank':
+            expected = ''
+        else:
+            expected = 'correct string' if answer_class == 'correct' \
+                                        else 'incorrect'
+
+        assert_textfield('string', expected)
+
+    elif problem_type == 'formula':
+        if answer_class == 'blank':
+            expected = ''
+        else:
+            expected = "x^2+2*x+y" if answer_class == 'correct' else 'x^2'
+
+        assert_textfield('formula', expected)
+
+    else:
+        # The other response types use random data,
+        # which would be difficult to check
+        # We trade input value coverage in the other tests for
+        # input type coverage in this test.
+        pass
+
+
 @step(u'I check a problem')
 def check_problem(step):
     world.css_click("input.check")
@@ -227,7 +299,7 @@ CORRECTNESS_SELECTORS = {
                         'string': ['div.correct'],
                         'numerical': ['div.correct'],
                         'formula': ['div.correct'],
-                        'script': ['div.correct'], 
+                        'script': ['div.correct'],
                         'code': ['span.correct']},
 
         'incorrect': {'drop down': ['span.incorrect'],
@@ -247,12 +319,14 @@ CORRECTNESS_SELECTORS = {
                         'numerical': ['div.unanswered'],
                         'formula': ['div.unanswered'],
                         'script': ['div.unanswered'],
-                        'code': ['span.unanswered'] }}
+                        'code': ['span.unanswered']}}
 
 
 @step(u'My "([^"]*)" answer is marked "([^"]*)"')
 def assert_answer_mark(step, problem_type, correctness):
-    """ Assert that the expected answer mark is visible for a given problem type.
+    """
+    Assert that the expected answer mark is visible
+    for a given problem type.
 
     *problem_type* is a string identifying the type of problem (e.g. 'drop down')
     *correctness* is in ['correct', 'incorrect', 'unanswered']
@@ -274,6 +348,7 @@ def assert_answer_mark(step, problem_type, correctness):
     # Expect that we found the expected selector
     assert(has_expected)
 
+
 def inputfield(problem_type, choice=None, input_num=1):
     """ Return the <input> element for *problem_type*.
     For example, if problem_type is 'string', return
@@ -289,8 +364,32 @@ def inputfield(problem_type, choice=None, input_num=1):
         base = "_choice_" if problem_type == "multiple choice" else "_"
         sel = sel + base + str(choice)
 
+
     # If the input element doesn't exist, fail immediately
     assert(world.browser.is_element_present_by_css(sel, wait_time=4))
 
     # Retrieve the input element
     return world.browser.find_by_css(sel)
+
+
+def assert_checked(problem_type, choices):
+    '''
+    Assert that choice names given in *choices* are the only
+    ones checked.
+
+    Works for both radio and checkbox problems
+    '''
+
+    all_choices = ['choice_0', 'choice_1', 'choice_2', 'choice_3']
+    for this_choice in all_choices:
+        element = inputfield(problem_type, choice=this_choice)
+
+        if this_choice in choices:
+            assert element.checked
+        else:
+            assert not element.checked
+
+
+def assert_textfield(problem_type, expected_text, input_num=1):
+    element = inputfield(problem_type, input_num=input_num)
+    assert element.value == expected_text