diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index a21ce346b1..00d3badd5e 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -429,16 +429,16 @@ class LoncapaProblem(object): def do_targeted_feedback(self, tree): """ - Implements the targeted-feedback=N in-place on -- + Implements targeted-feedback in-place on -- choice-level explanations shown to a student after submission. Does nothing if there is no targeted-feedback attribute. """ - for mult_choice_response in tree.xpath('//multiplechoiceresponse[@targeted-feedback]'): - # Note that the modifications has been done, avoiding problems if called twice. - if hasattr(self, 'has_targeted'): - continue - self.has_targeted = True # pylint: disable=W0201 + # Note that the modifications has been done, avoiding problems if called twice. + if hasattr(self, 'has_targeted'): + return + self.has_targeted = True # pylint: disable=W0201 + for mult_choice_response in tree.xpath('//multiplechoiceresponse[@targeted-feedback]'): show_explanation = mult_choice_response.get('targeted-feedback') == 'alwaysShowCorrectChoiceExplanation' # Grab the first choicegroup (there should only be one within each tag) diff --git a/common/lib/capa/capa/tests/__init__.py b/common/lib/capa/capa/tests/__init__.py index cec2399222..bbcef25bb4 100644 --- a/common/lib/capa/capa/tests/__init__.py +++ b/common/lib/capa/capa/tests/__init__.py @@ -57,3 +57,15 @@ def test_capa_system(): def new_loncapa_problem(xml, capa_system=None, seed=723): """Construct a `LoncapaProblem` suitable for unit tests.""" return LoncapaProblem(xml, id='1', seed=seed, capa_system=capa_system or test_capa_system()) + + +def load_fixture(relpath): + """ + Return a `unicode` object representing the contents + of the fixture file at the given path within a test_files directory + in the same directory as the test file. + """ + abspath = os.path.join(os.path.dirname(__file__), 'test_files', relpath) + with open(abspath) as fixture_file: + contents = fixture_file.read() + return contents.decode('utf8') diff --git a/common/lib/capa/capa/tests/test_files/targeted_feedback.xml b/common/lib/capa/capa/tests/test_files/targeted_feedback.xml new file mode 100644 index 0000000000..e762f19c65 --- /dev/null +++ b/common/lib/capa/capa/tests/test_files/targeted_feedback.xml @@ -0,0 +1,50 @@ + +

What is the correct answer?

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 2nd WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + \ No newline at end of file diff --git a/common/lib/capa/capa/tests/test_files/targeted_feedback_multiple.xml b/common/lib/capa/capa/tests/test_files/targeted_feedback_multiple.xml new file mode 100644 index 0000000000..785df87c64 --- /dev/null +++ b/common/lib/capa/capa/tests/test_files/targeted_feedback_multiple.xml @@ -0,0 +1,91 @@ + +

Q1

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + +
+ +

Q2

+ + + wrong-1 + wrong-2 + correct-1 + wrong-3 + + + + + +
+

Targeted Feedback

+

This is the 1st WRONG solution

+
+ + + +
+

Targeted Feedback

+

This is the 3rd WRONG solution

+
+ + + +
+

Targeted Feedback

+

Feedback on your correct solution...

+
+ + + + + + +
+

Explanation

+

This is the solution explanation

+

Not much to explain here, sorry!

+
+ + + \ No newline at end of file diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 770f6959ee..0057b90c3d 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -13,7 +13,7 @@ import textwrap import requests import mock -from . import new_loncapa_problem, test_capa_system +from . import new_loncapa_problem, test_capa_system, load_fixture import calc from capa.responsetypes import LoncapaProblemError, \ @@ -224,7 +224,7 @@ class SymbolicResponseTest(ResponseTest): for (input_str, input_mathml, server_fixture) in correct_inputs: print "Testing input: {0}".format(input_str) - server_resp = self._load_fixture(server_fixture) + server_resp = load_fixture(server_fixture) self._assert_symbolic_grade( problem, input_str, input_mathml, 'correct', snuggletex_resp=server_resp @@ -253,8 +253,8 @@ class SymbolicResponseTest(ResponseTest): options=["matrix", "imaginary"] ) - correct_snuggletex = self._load_fixture('snuggletex_correct.html') - dynamath_input = self._load_fixture('dynamath_input.txt') + correct_snuggletex = load_fixture('snuggletex_correct.html') + dynamath_input = load_fixture('dynamath_input.txt') student_response = "cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]" self._assert_symbolic_grade( @@ -269,7 +269,7 @@ class SymbolicResponseTest(ResponseTest): expect="[[cos(theta),i*sin(theta)],[i*sin(theta),cos(theta)]]", options=["matrix", "imaginary"]) - wrong_snuggletex = self._load_fixture('snuggletex_wrong.html') + wrong_snuggletex = load_fixture('snuggletex_wrong.html') dynamath_input = textwrap.dedent(""" 2 @@ -315,18 +315,6 @@ class SymbolicResponseTest(ResponseTest): correct_map.get_correctness('1_2_1'), expected_correctness ) - @staticmethod - def _load_fixture(relpath): - """ - Return a `unicode` object representing the contents - of the fixture file at `relpath` (relative to the test files dir) - """ - abspath = os.path.join(os.path.dirname(__file__), 'test_files', relpath) - with open(abspath) as fixture_file: - contents = fixture_file.read() - - return contents.decode('utf8') - class OptionResponseTest(ResponseTest): from capa.tests.response_xml_factory import OptionResponseXMLFactory diff --git a/common/lib/capa/capa/tests/test_targeted_feedback.py b/common/lib/capa/capa/tests/test_targeted_feedback.py index 6e0df87ff1..a9e34381a5 100644 --- a/common/lib/capa/capa/tests/test_targeted_feedback.py +++ b/common/lib/capa/capa/tests/test_targeted_feedback.py @@ -5,7 +5,7 @@ i.e. those with the element import unittest import textwrap -from . import test_capa_system, new_loncapa_problem +from . import test_capa_system, new_loncapa_problem, load_fixture class CapaTargetedFeedbackTest(unittest.TestCase): @@ -80,62 +80,8 @@ class CapaTargetedFeedbackTest(unittest.TestCase): self.assertRegexpMatches(without_new_lines, r"
.*'wrong-1'.*'wrong-2'.*'correct-1'.*'wrong-3'.*
") self.assertRegexpMatches(without_new_lines, r"feedback1|feedback2|feedback3|feedbackC") - # A targeted-feedback problem shared for a few tests - common_targeted_xml = textwrap.dedent(""" - -

What is the correct answer?

- - - wrong-1 - wrong-2 - correct-1 - wrong-3 - - - - - -
-

Targeted Feedback

-

This is the 1st WRONG solution

-
- - - -
-

Targeted Feedback

-

This is the 2nd WRONG solution

-
- - - -
-

Targeted Feedback

-

This is the 3rd WRONG solution

-
- - - -
-

Targeted Feedback

-

Feedback on your correct solution...

-
- - - - - -
-

Explanation

-

This is the solution explanation

-

Not much to explain here, sorry!

-
- - - """) - def test_targeted_feedback_not_finished(self): - problem = new_loncapa_problem(self.common_targeted_xml) + problem = new_loncapa_problem(load_fixture('targeted_feedback.xml')) the_html = problem.get_html() without_new_lines = the_html.replace("\n", "") @@ -144,7 +90,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase): self.assertEquals(the_html, problem.get_html(), "Should be able to call get_html() twice") def test_targeted_feedback_student_answer1(self): - problem = new_loncapa_problem(self.common_targeted_xml) + problem = new_loncapa_problem(load_fixture('targeted_feedback.xml')) problem.done = True problem.student_answers = {'1_2_1': 'choice_3'} @@ -158,7 +104,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase): self.assertEquals(the_html, the_html2) def test_targeted_feedback_student_answer2(self): - problem = new_loncapa_problem(self.common_targeted_xml) + problem = new_loncapa_problem(load_fixture('targeted_feedback.xml')) problem.done = True problem.student_answers = {'1_2_1': 'choice_0'} @@ -611,3 +557,41 @@ class CapaTargetedFeedbackTest(unittest.TestCase): self.assertNotRegexpMatches(without_new_lines, r"\{.*'1_solution_1'.*\}") self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback3|feedbackC") + + def test_targeted_feedback_multiple_not_answered(self): + # Not answered -> empty targeted feedback + problem = new_loncapa_problem(load_fixture('targeted_feedback_multiple.xml')) + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + # Q1 and Q2 have no feedback + self.assertRegexpMatches( + without_new_lines, + r'\s*.*' + + r'\s*' + ) + + def test_targeted_feedback_multiple_answer_1(self): + problem = new_loncapa_problem(load_fixture('targeted_feedback_multiple.xml')) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0'} # feedback1 + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + # Q1 has feedback1 and Q2 has nothing + self.assertRegexpMatches( + without_new_lines, + r'.*?explanation-id="feedback1".*?.*' + + r'\s*' + ) + + def test_targeted_feedback_multiple_answer_2(self): + problem = new_loncapa_problem(load_fixture('targeted_feedback_multiple.xml')) + problem.done = True + problem.student_answers = {'1_2_1': 'choice_0', '1_3_1': 'mask_1'} # Q1 wrong, Q2 correct + the_html = problem.get_html() + without_new_lines = the_html.replace("\n", "") + # Q1 has feedback1 and Q2 has feedbackC + self.assertRegexpMatches( + without_new_lines, + r'.*?explanation-id="feedback1".*?.*' + + r'.*explanation-id="feedbackC".*?' + )