From 336982e42b598b5306ccc9b43448cd9eb8487808 Mon Sep 17 00:00:00 2001 From: mirjamsk Date: Tue, 25 Aug 2015 12:06:44 +0000 Subject: [PATCH] Adresses TNL-3096 Add acceptance test for TNL-3096 Adress comments on pull request Fix test failing and img issue Modify tests --- common/lib/capa/capa/responsetypes.py | 9 +- ...tended_hints_multiple_choice_with_html.xml | 15 ++++ .../capa/tests/test_hint_functionality.py | 46 ++++++++-- common/lib/capa/capa/tests/test_util.py | 10 ++- common/lib/capa/capa/util.py | 14 +++ common/lib/xmodule/xmodule/capa_base.py | 4 +- .../acceptance/tests/lms/test_lms_problems.py | 85 +++++++++++++++++++ 7 files changed, 170 insertions(+), 13 deletions(-) create mode 100644 common/lib/capa/capa/tests/test_files/extended_hints_multiple_choice_with_html.xml diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index f1d79daea2..07c3cb1dda 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -43,7 +43,7 @@ from datetime import datetime from pytz import UTC from .util import ( compare_with_tolerance, contextualize_text, convert_files_to_filenames, - is_list_of_files, find_with_default, default_tolerance + is_list_of_files, find_with_default, default_tolerance, get_inner_html_from_xpath ) from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? @@ -312,9 +312,10 @@ class LoncapaResponse(object): # 1. Establish the hint_texts # This can lead to early-exit if the hint is blank. if not hint_log: - if hint_node is None or hint_node.text is None: # .text can be None, maybe just in testing + # .text can be None when node has immediate children nodes + if hint_node is None or (hint_node.text is None and len(hint_node.getchildren()) == 0): return '' - hint_text = hint_node.text.strip() + hint_text = get_inner_html_from_xpath(hint_node) if not hint_text: return '' hint_log = [{'text': hint_text}] @@ -1051,7 +1052,7 @@ class ChoiceResponse(LoncapaResponse): hint_nodes = choice.findall('./choicehint') for hint_node in hint_nodes: if hint_node.get('selected', '').lower() == selector: - text = hint_node.text.strip() + text = get_inner_html_from_xpath(hint_node) if hint_node.get('label') is not None: # tricky: label '' vs None is significant label = hint_node.get('label') label_count += 1 diff --git a/common/lib/capa/capa/tests/test_files/extended_hints_multiple_choice_with_html.xml b/common/lib/capa/capa/tests/test_files/extended_hints_multiple_choice_with_html.xml new file mode 100644 index 0000000000..246ba61da5 --- /dev/null +++ b/common/lib/capa/capa/tests/test_files/extended_hints_multiple_choice_with_html.xml @@ -0,0 +1,15 @@ + +

Select the fruit from the list

+ + + Mushroom + Mushroom is a fungus, not a fruit. + + Potato + Potato is not a fruit. + Apple + Apple is a fruit. + + + + diff --git a/common/lib/capa/capa/tests/test_hint_functionality.py b/common/lib/capa/capa/tests/test_hint_functionality.py index 0f374ea9b7..69d103dc35 100644 --- a/common/lib/capa/capa/tests/test_hint_functionality.py +++ b/common/lib/capa/capa/tests/test_hint_functionality.py @@ -65,21 +65,21 @@ class TextInputHintsTest(HintTest): @data( {'problem_id': u'1_2_1', u'choice': u'GermanyΩ', - 'expected_string': u'
Incorrect:
I do not think so.Ω
'}, + 'expected_string': u'
Incorrect:
I do not think so.Ω
'}, {'problem_id': u'1_2_1', u'choice': u'franceΩ', - 'expected_string': u'
Correct:
Viva la France!Ω
'}, + 'expected_string': u'
Correct:
Viva la France!Ω
'}, {'problem_id': u'1_2_1', u'choice': u'FranceΩ', - 'expected_string': u'
Correct:
Viva la France!Ω
'}, + 'expected_string': u'
Correct:
Viva la France!Ω
'}, {'problem_id': u'1_2_1', u'choice': u'Mexico', 'expected_string': ''}, {'problem_id': u'1_2_1', u'choice': u'USAΩ', - 'expected_string': u'
Correct:
Less well known, but yes, there is a Paris, Texas.Ω
'}, + 'expected_string': u'
Correct:
Less well known, but yes, there is a Paris, Texas.Ω
'}, {'problem_id': u'1_2_1', u'choice': u'usaΩ', - 'expected_string': u'
Correct:
Less well known, but yes, there is a Paris, Texas.Ω
'}, + 'expected_string': u'
Correct:
Less well known, but yes, there is a Paris, Texas.Ω
'}, {'problem_id': u'1_2_1', u'choice': u'uSAxΩ', 'expected_string': u''}, {'problem_id': u'1_2_1', u'choice': u'NICKLANDΩ', - 'expected_string': u'
Incorrect:
The country name does not end in LANDΩ
'}, + 'expected_string': u'
Incorrect:
The country name does not end in LANDΩ
'}, {'problem_id': u'1_3_1', u'choice': u'Blue', 'expected_string': u'
Correct:
The red light is scattered by water molecules leaving only blue light.
'}, {'problem_id': u'1_3_1', u'choice': u'blue', @@ -450,6 +450,40 @@ class MultpleChoiceHintsTest(HintTest): self.assertEqual(hint, expected_string) +@ddt +class MultpleChoiceHintsWithHtmlTest(HintTest): + """ + This class consists of a suite of test cases to be run on the multiple choice problem represented by the XML below. + + """ + xml = load_fixture('extended_hints_multiple_choice_with_html.xml') + problem = new_loncapa_problem(xml) + + def test_tracking_log(self): + """Test that the tracking log comes out right.""" + self.problem.capa_module.reset_mock() + self.get_hint(u'1_2_1', u'choice_0') + self.problem.capa_module.runtime.track_function.assert_called_with( + 'edx.problem.hint.feedback_displayed', + {'module_id': 'i4x://Foo/bar/mock/abc', 'problem_part_id': '1_1', 'trigger_type': 'single', + 'student_answer': [u'choice_0'], 'correctness': False, 'question_type': 'multiplechoiceresponse', + 'hint_label': 'Incorrect', 'hints': [{'text': 'Mushroom is a fungus, not a fruit.'}]} + ) + + @data( + {'problem_id': u'1_2_1', 'choice': u'choice_0', + 'expected_string': '
Incorrect:
Mushroom is a fungus, not a fruit.
'}, + {'problem_id': u'1_2_1', 'choice': u'choice_1', + 'expected_string': '
Incorrect:
Potato is not a fruit.
'}, + {'problem_id': u'1_2_1', 'choice': u'choice_2', + 'expected_string': '
Correct:
Apple is a fruit.
'} + ) + @unpack + def test_multiplechoice_hints(self, problem_id, choice, expected_string): + hint = self.get_hint(problem_id, choice) + self.assertEqual(hint, expected_string) + + @ddt class DropdownHintsTest(HintTest): """ diff --git a/common/lib/capa/capa/tests/test_util.py b/common/lib/capa/capa/tests/test_util.py index 30709f35ac..5d34792499 100644 --- a/common/lib/capa/capa/tests/test_util.py +++ b/common/lib/capa/capa/tests/test_util.py @@ -2,9 +2,10 @@ Tests capa util """ import unittest +from lxml import etree from . import test_capa_system -from capa.util import compare_with_tolerance, sanitize_html +from capa.util import compare_with_tolerance, sanitize_html, get_inner_html_from_xpath class UtilTest(unittest.TestCase): @@ -118,3 +119,10 @@ class UtilTest(unittest.TestCase): queue_msg = "<{0}>Test message".format(not_allowed_tag) expected = "<script>Test message</script>" self.assertEqual(sanitize_html(queue_msg), expected) + + def test_get_inner_html_from_xpath(self): + """ + Test for getting inner html as string from xpath node. + """ + xpath_node = etree.XML('aabbcc') # pylint: disable=no-member + self.assertEqual(get_inner_html_from_xpath(xpath_node), 'aabbcc') diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index 970a40ef98..cebc83c83b 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -6,6 +6,8 @@ from decimal import Decimal from calc import evaluator from cmath import isinf, isnan +import re +from lxml import etree #----------------------------------------------------------------------------- # # Utility functions used in CAPA responsetypes @@ -181,3 +183,15 @@ def sanitize_html(html_code): attributes=attributes ) return output + + +def get_inner_html_from_xpath(xpath_node): + """ + Returns inner html as string from xpath node. + + """ + # returns string from xpath node + html = etree.tostring(xpath_node).strip() # pylint: disable=no-member + # strips outer tag from html string + inner_html = re.sub('(?ms)<%s[^>]*>(.*)' % (xpath_node.tag, xpath_node.tag), '\\1', html) + return inner_html.strip() diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index f863d436ee..8838f8910b 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -21,7 +21,7 @@ except ImportError: from capa.capa_problem import LoncapaProblem, LoncapaSystem from capa.responsetypes import StudentInputError, \ ResponseError, LoncapaProblemError -from capa.util import convert_files_to_filenames +from capa.util import convert_files_to_filenames, get_inner_html_from_xpath from .progress import Progress from xmodule.exceptions import NotFoundError from xblock.fields import Scope, String, Boolean, Dict, Integer, Float @@ -606,7 +606,7 @@ class CapaMixin(CapaFields): _ = self.runtime.service(self, "i18n").ugettext # pylint: disable=redefined-outer-name hint_element = demand_hints[hint_index] - hint_text = hint_element.text.strip() + hint_text = get_inner_html_from_xpath(hint_element) if len(demand_hints) == 1: prefix = _('Hint: ') else: diff --git a/common/test/acceptance/tests/lms/test_lms_problems.py b/common/test/acceptance/tests/lms/test_lms_problems.py index ba14a10208..c4a5a5d084 100644 --- a/common/test/acceptance/tests/lms/test_lms_problems.py +++ b/common/test/acceptance/tests/lms/test_lms_problems.py @@ -166,6 +166,91 @@ class ProblemExtendedHintTest(ProblemsTest, EventsTestMixin): actual_events) +class ProblemHintWithHtmlTest(ProblemsTest, EventsTestMixin): + """ + Tests that hints containing html get rendered properly + """ + + def get_problem(self): + """ + Problem with extended hint features. + """ + xml = dedent(""" + +

question text

+ + aa bb cc + aa bb cc + + + + aa bb cc + dd ee ff + + + """) + return XBlockFixtureDesc('problem', 'PROBLEM HTML HINT TEST', data=xml) + + def test_check_hint(self): + """ + Test clicking Check shows the extended hint in the problem message. + """ + self.courseware_page.visit() + problem_page = ProblemPage(self.browser) + self.assertEqual(problem_page.problem_text[0], u'question text') + problem_page.fill_answer('B') + problem_page.click_check() + self.assertEqual(problem_page.message_text, u'Incorrect: aa bb cc') + problem_page.fill_answer('C') + problem_page.click_check() + self.assertEqual(problem_page.message_text, u'Incorrect: aa bb cc') + # Check for corresponding tracking event + actual_events = self.wait_for_events( + event_filter={'event_type': 'edx.problem.hint.feedback_displayed'}, + number_of_matches=2 + ) + self.assert_events_match( + [{'event': {'hint_label': u'Incorrect', + 'trigger_type': 'single', + 'student_answer': [u'B'], + 'correctness': False, + 'question_type': 'stringresponse', + 'hints': [{'text': 'aa bb cc'}]}}, + {'event': {'hint_label': u'Incorrect', + 'trigger_type': 'single', + 'student_answer': [u'C'], + 'correctness': False, + 'question_type': 'stringresponse', + 'hints': [{'text': 'aa bb cc'}]}}], + actual_events) + + def test_demand_hint(self): + """ + Test clicking hint button shows the demand hint in its div. + """ + self.courseware_page.visit() + problem_page = ProblemPage(self.browser) + # The hint button rotates through multiple hints + problem_page.click_hint() + self.assertEqual(problem_page.hint_text, u'Hint (1 of 2): aa bb cc') + problem_page.click_hint() + self.assertEqual(problem_page.hint_text, u'Hint (2 of 2): dd ee ff') + problem_page.click_hint() + self.assertEqual(problem_page.hint_text, u'Hint (1 of 2): aa bb cc') + # Check corresponding tracking events + actual_events = self.wait_for_events( + event_filter={'event_type': 'edx.problem.hint.demandhint_displayed'}, + number_of_matches=3 + ) + self.assert_events_match( + [ + {'event': {u'hint_index': 0, u'hint_len': 2, u'hint_text': u'aa bb cc'}}, + {'event': {u'hint_index': 1, u'hint_len': 2, u'hint_text': u'dd ee ff'}}, + {'event': {u'hint_index': 0, u'hint_len': 2, u'hint_text': u'aa bb cc'}} + ], + actual_events) + + class ProblemWithMathjax(ProblemsTest): """ Tests the used in problem