Skip to content

E
edx-platform
Commit c30665fb by Nick Parlante
Options

Merge pull request #3677 from edx/nick/fix-feedback-bug 

Fix targeted-feedback bug
parents 7d6e3319 f48e470f
Showing with 208 additions and 81 deletions
common/lib/capa/capa/capa_problem.py
...@@ -429,16 +429,16 @@ class LoncapaProblem(object): ...@@ -429,16 +429,16 @@ class LoncapaProblem(object):
def do_targeted_feedback(self, tree): def do_targeted_feedback(self, tree):
""" """
Implements the targeted-feedback=N in-place on <multiplechoiceresponse> -- Implements targeted-feedback in-place on <multiplechoiceresponse> --
choice-level explanations shown to a student after submission. choice-level explanations shown to a student after submission.
Does nothing if there is no targeted-feedback attribute. 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.
# Note that the modifications has been done, avoiding problems if called twice. if hasattr(self, 'has_targeted'):
if hasattr(self, 'has_targeted'): return
continue self.has_targeted = True # pylint: disable=W0201
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' show_explanation = mult_choice_response.get('targeted-feedback') == 'alwaysShowCorrectChoiceExplanation'
# Grab the first choicegroup (there should only be one within each <multiplechoiceresponse> tag) # Grab the first choicegroup (there should only be one within each <multiplechoiceresponse> tag)
......
common/lib/capa/capa/tests/__init__.py
...@@ -57,3 +57,15 @@ def test_capa_system(): ...@@ -57,3 +57,15 @@ def test_capa_system():
def new_loncapa_problem(xml, capa_system=None, seed=723): def new_loncapa_problem(xml, capa_system=None, seed=723):
"""Construct a `LoncapaProblem` suitable for unit tests.""" """Construct a `LoncapaProblem` suitable for unit tests."""
return LoncapaProblem(xml, id='1', seed=seed, capa_system=capa_system or test_capa_system()) 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')
common/lib/capa/capa/tests/test_files/targeted_feedback.xml 0 → 100644
<problem>
<p>What is the correct answer?</p>
<multiplechoiceresponse targeted-feedback="">
<choicegroup type="MultipleChoice">
<choice correct="false" explanation-id="feedback1">wrong-1</choice>
<choice correct="false" explanation-id="feedback2">wrong-2</choice>
<choice correct="true" explanation-id="feedbackC">correct-1</choice>
<choice correct="false" explanation-id="feedback3">wrong-3</choice>
</choicegroup>
</multiplechoiceresponse>
<targetedfeedbackset>
<targetedfeedback explanation-id="feedback1">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 1st WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback2">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 2nd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback3">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 3rd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedbackC">
<div class="detailed-targeted-feedback-correct">
<p>Targeted Feedback</p>
<p>Feedback on your correct solution...</p>
</div>
</targetedfeedback>
</targetedfeedbackset>
<solution explanation-id="feedbackC">
<div class="detailed-solution">
<p>Explanation</p>
<p>This is the solution explanation</p>
<p>Not much to explain here, sorry!</p>
</div>
</solution>
</problem>
\ No newline at end of file
common/lib/capa/capa/tests/test_files/targeted_feedback_multiple.xml 0 → 100644
<problem>
<p>Q1</p>
<multiplechoiceresponse targeted-feedback="">
<choicegroup type="MultipleChoice">
<choice correct="false" explanation-id="feedback1">wrong-1</choice>
<choice correct="false" explanation-id="feedback2">wrong-2</choice>
<choice correct="true" explanation-id="feedbackC">correct-1</choice>
<choice correct="false" explanation-id="feedback3">wrong-3</choice>
</choicegroup>
</multiplechoiceresponse>
<targetedfeedbackset>
<targetedfeedback explanation-id="feedback1">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 1st WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback3">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 3rd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedbackC">
<div class="detailed-targeted-feedback-correct">
<p>Targeted Feedback</p>
<p>Feedback on your correct solution...</p>
</div>
</targetedfeedback>
</targetedfeedbackset>
<solutionset>
<solution explanation-id="feedbackC">
<div class="detailed-solution">
<p>Explanation</p>
<p>This is the solution explanation</p>
<p>Not much to explain here, sorry!</p>
</div>
</solution>
</solutionset>
<hr/>
<p>Q2</p>
<multiplechoiceresponse targeted-feedback="">
<choicegroup type="MultipleChoice" answer-pool="3">
<choice correct="false" explanation-id="feedback1">wrong-1</choice>
<choice correct="false" explanation-id="feedback2">wrong-2</choice>
<choice correct="true" explanation-id="feedbackC">correct-1</choice>
<choice correct="false" explanation-id="feedback3">wrong-3</choice>
</choicegroup>
</multiplechoiceresponse>
<targetedfeedbackset>
<targetedfeedback explanation-id="feedback1">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 1st WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback3">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 3rd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedbackC">
<div class="detailed-targeted-feedback-correct">
<p>Targeted Feedback</p>
<p>Feedback on your correct solution...</p>
</div>
</targetedfeedback>
</targetedfeedbackset>
<solutionset>
<solution explanation-id="feedbackC">
<div class="detailed-solution">
<p>Explanation</p>
<p>This is the solution explanation</p>
<p>Not much to explain here, sorry!</p>
</div>
</solution>
</solutionset>
</problem>
\ No newline at end of file
common/lib/capa/capa/tests/test_responsetypes.py
...@@ -13,7 +13,7 @@ import textwrap ...@@ -13,7 +13,7 @@ import textwrap
import requests import requests
import mock import mock
from . import new_loncapa_problem, test_capa_system from . import new_loncapa_problem, test_capa_system, load_fixture
import calc import calc
from capa.responsetypes import LoncapaProblemError, \ from capa.responsetypes import LoncapaProblemError, \
...@@ -224,7 +224,7 @@ class SymbolicResponseTest(ResponseTest): ...@@ -224,7 +224,7 @@ class SymbolicResponseTest(ResponseTest):
for (input_str, input_mathml, server_fixture) in correct_inputs: for (input_str, input_mathml, server_fixture) in correct_inputs:
print "Testing input: {0}".format(input_str) print "Testing input: {0}".format(input_str)
server_resp = self._load_fixture(server_fixture) server_resp = load_fixture(server_fixture)
self._assert_symbolic_grade( self._assert_symbolic_grade(
problem, input_str, input_mathml, problem, input_str, input_mathml,
'correct', snuggletex_resp=server_resp 'correct', snuggletex_resp=server_resp
...@@ -253,8 +253,8 @@ class SymbolicResponseTest(ResponseTest): ...@@ -253,8 +253,8 @@ class SymbolicResponseTest(ResponseTest):
options=["matrix", "imaginary"] options=["matrix", "imaginary"]
) )
correct_snuggletex = self._load_fixture('snuggletex_correct.html') correct_snuggletex = load_fixture('snuggletex_correct.html')
dynamath_input = self._load_fixture('dynamath_input.txt') dynamath_input = load_fixture('dynamath_input.txt')
student_response = "cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]" student_response = "cos(theta)*[[1,0],[0,1]] + i*sin(theta)*[[0,1],[1,0]]"
self._assert_symbolic_grade( self._assert_symbolic_grade(
...@@ -269,7 +269,7 @@ class SymbolicResponseTest(ResponseTest): ...@@ -269,7 +269,7 @@ class SymbolicResponseTest(ResponseTest):
expect="[[cos(theta),i*sin(theta)],[i*sin(theta),cos(theta)]]", expect="[[cos(theta),i*sin(theta)],[i*sin(theta),cos(theta)]]",
options=["matrix", "imaginary"]) options=["matrix", "imaginary"])
wrong_snuggletex = self._load_fixture('snuggletex_wrong.html') wrong_snuggletex = load_fixture('snuggletex_wrong.html')
dynamath_input = textwrap.dedent(""" dynamath_input = textwrap.dedent("""
<math xmlns="http://www.w3.org/1998/Math/MathML"> <math xmlns="http://www.w3.org/1998/Math/MathML">
<mstyle displaystyle="true"><mn>2</mn></mstyle> <mstyle displaystyle="true"><mn>2</mn></mstyle>
...@@ -315,18 +315,6 @@ class SymbolicResponseTest(ResponseTest): ...@@ -315,18 +315,6 @@ class SymbolicResponseTest(ResponseTest):
correct_map.get_correctness('1_2_1'), expected_correctness 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): class OptionResponseTest(ResponseTest):
from capa.tests.response_xml_factory import OptionResponseXMLFactory from capa.tests.response_xml_factory import OptionResponseXMLFactory
......
common/lib/capa/capa/tests/test_targeted_feedback.py
...@@ -5,7 +5,7 @@ i.e. those with the <multiplechoiceresponse> element ...@@ -5,7 +5,7 @@ i.e. those with the <multiplechoiceresponse> element
import unittest import unittest
import textwrap import textwrap
from . import test_capa_system, new_loncapa_problem from . import test_capa_system, new_loncapa_problem, load_fixture
class CapaTargetedFeedbackTest(unittest.TestCase): class CapaTargetedFeedbackTest(unittest.TestCase):
...@@ -80,62 +80,8 @@ class CapaTargetedFeedbackTest(unittest.TestCase): ...@@ -80,62 +80,8 @@ class CapaTargetedFeedbackTest(unittest.TestCase):
self.assertRegexpMatches(without_new_lines, r"<div>.*'wrong-1'.*'wrong-2'.*'correct-1'.*'wrong-3'.*</div>") self.assertRegexpMatches(without_new_lines, r"<div>.*'wrong-1'.*'wrong-2'.*'correct-1'.*'wrong-3'.*</div>")
self.assertRegexpMatches(without_new_lines, r"feedback1|feedback2|feedback3|feedbackC") self.assertRegexpMatches(without_new_lines, r"feedback1|feedback2|feedback3|feedbackC")
# A targeted-feedback problem shared for a few tests
common_targeted_xml = textwrap.dedent("""
<problem>
<p>What is the correct answer?</p>
<multiplechoiceresponse targeted-feedback="">
<choicegroup type="MultipleChoice">
<choice correct="false" explanation-id="feedback1">wrong-1</choice>
<choice correct="false" explanation-id="feedback2">wrong-2</choice>
<choice correct="true" explanation-id="feedbackC">correct-1</choice>
<choice correct="false" explanation-id="feedback3">wrong-3</choice>
</choicegroup>
</multiplechoiceresponse>
<targetedfeedbackset>
<targetedfeedback explanation-id="feedback1">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 1st WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback2">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 2nd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedback3">
<div class="detailed-targeted-feedback">
<p>Targeted Feedback</p>
<p>This is the 3rd WRONG solution</p>
</div>
</targetedfeedback>
<targetedfeedback explanation-id="feedbackC">
<div class="detailed-targeted-feedback-correct">
<p>Targeted Feedback</p>
<p>Feedback on your correct solution...</p>
</div>
</targetedfeedback>
</targetedfeedbackset>
<solution explanation-id="feedbackC">
<div class="detailed-solution">
<p>Explanation</p>
<p>This is the solution explanation</p>
<p>Not much to explain here, sorry!</p>
</div>
</solution>
</problem>
""")
def test_targeted_feedback_not_finished(self): 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() the_html = problem.get_html()
without_new_lines = the_html.replace("\n", "") without_new_lines = the_html.replace("\n", "")
...@@ -144,7 +90,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase): ...@@ -144,7 +90,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase):
self.assertEquals(the_html, problem.get_html(), "Should be able to call get_html() twice") self.assertEquals(the_html, problem.get_html(), "Should be able to call get_html() twice")
def test_targeted_feedback_student_answer1(self): 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.done = True
problem.student_answers = {'1_2_1': 'choice_3'} problem.student_answers = {'1_2_1': 'choice_3'}
...@@ -158,7 +104,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase): ...@@ -158,7 +104,7 @@ class CapaTargetedFeedbackTest(unittest.TestCase):
self.assertEquals(the_html, the_html2) self.assertEquals(the_html, the_html2)
def test_targeted_feedback_student_answer2(self): 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.done = True
problem.student_answers = {'1_2_1': 'choice_0'} problem.student_answers = {'1_2_1': 'choice_0'}
...@@ -611,3 +557,41 @@ class CapaTargetedFeedbackTest(unittest.TestCase): ...@@ -611,3 +557,41 @@ class CapaTargetedFeedbackTest(unittest.TestCase):
self.assertNotRegexpMatches(without_new_lines, r"<targetedfeedback explanation-id=\"feedbackC\".*solution explanation") self.assertNotRegexpMatches(without_new_lines, r"<targetedfeedback explanation-id=\"feedbackC\".*solution explanation")
self.assertRegexpMatches(without_new_lines, r"<div>\{.*'1_solution_1'.*\}</div>") self.assertRegexpMatches(without_new_lines, r"<div>\{.*'1_solution_1'.*\}</div>")
self.assertNotRegexpMatches(without_new_lines, r"feedback1|feedback3|feedbackC") 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'<targetedfeedbackset.*?>\s*</targetedfeedbackset>.*' +
r'<targetedfeedbackset.*?>\s*</targetedfeedbackset>'
)
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'<targetedfeedbackset.*?>.*?explanation-id="feedback1".*?</targetedfeedbackset>.*' +
r'<targetedfeedbackset.*?>\s*</targetedfeedbackset>'
)
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'<targetedfeedbackset.*?>.*?explanation-id="feedback1".*?</targetedfeedbackset>.*' +
r'<targetedfeedbackset.*?>.*explanation-id="feedbackC".*?</targetedfeedbackset>'
)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment