diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index 26eb3a4d..c0a9f726 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -232,22 +232,33 @@ def answer_mapper(self, answer_status): """ Create a JSON-dumpable object with readable key names from a list of student answers. """ - return [ - { - 'number': self.get_question_number(answer[0]), - 'id': answer[0], - 'details': answer[1], - } for answer in self.student_results if answer[1]['status'] == answer_status - ] + answer_map = [] + for answer in self.student_results: + if answer[1]['status'] == answer_status: + try: + answer_map.append({ + 'number': self.get_question_number(answer[0]), + 'id': answer[0], + 'details': answer[1], + }) + except ValueError: + pass # The question has been deleted since the student answered it. + return answer_map @property def score(self): """Compute the student score taking into account the weight of each step.""" - weights = (float(self.runtime.get_block(step_id).weight) for step_id in self.steps) - total_child_weight = sum(weights) + steps = [self.runtime.get_block(step_id) for step_id in self.steps] + steps_map = {q.name: q for q in steps} + total_child_weight = sum(float(step.weight) for step in steps) if total_child_weight == 0: return Score(0, 0, [], [], []) - score = sum(r[1]['score'] * r[1]['weight'] for r in self.student_results) / total_child_weight + points_earned = 0 + for q_name, q_details in self.student_results: + question = steps_map.get(q_name) + if question: + points_earned += q_details['score'] * question.weight + score = points_earned / total_child_weight correct = self.answer_mapper(CORRECT) incorrect = self.answer_mapper(INCORRECT) partially_correct = self.answer_mapper(PARTIAL) diff --git a/problem_builder/tests/integration/base_test.py b/problem_builder/tests/integration/base_test.py index 2278caf7..70616426 100644 --- a/problem_builder/tests/integration/base_test.py +++ b/problem_builder/tests/integration/base_test.py @@ -60,24 +60,22 @@ def popup_check(self, mentoring, item_feedbacks, prefix='', do_submit=True): self.assertFalse(item_feedback_popup.is_displayed()) -class MentoringBaseTest(SeleniumBaseTest, PopupCheckMixin): - module_name = __name__ - default_css_selector = 'div.mentoring' - - -class MentoringBaseTemplateTest(SeleniumXBlockTest, PopupCheckMixin): +class ProblemBuilderBaseTest(SeleniumXBlockTest, PopupCheckMixin): """ - Base class for mentoring tests that use templated XML. - All new tests should inherit from this rather than MentoringBaseTest + The new base class for integration tests. + Scenarios can be loaded and edited on the fly. """ - module_name = __name__ default_css_selector = 'div.mentoring' - def load_scenario(self, xml_file, params=None): + def load_scenario(self, xml_file, params=None, load_immediately=True): + """ + Given the name of an XML file in the xml_templates folder, load it into the workbench. + """ params = params or {} scenario = loader.render_template("xml_templates/{}".format(xml_file), params) self.set_scenario_xml(scenario) - return self.go_to_view("student_view") + if load_immediately: + return self.go_to_view("student_view") def click_submit(self, mentoring): """ Click the submit button and wait for the response """ @@ -88,7 +86,12 @@ def click_submit(self, mentoring): self.wait_until_disabled(submit) -class MentoringAssessmentBaseTest(MentoringBaseTemplateTest): +class MentoringBaseTest(SeleniumBaseTest, PopupCheckMixin): + module_name = __name__ + default_css_selector = 'div.mentoring' + + +class MentoringAssessmentBaseTest(ProblemBuilderBaseTest): """ Base class for tests of assessment mode """ diff --git a/problem_builder/tests/integration/test_author_changes.py b/problem_builder/tests/integration/test_author_changes.py new file mode 100644 index 00000000..85601630 --- /dev/null +++ b/problem_builder/tests/integration/test_author_changes.py @@ -0,0 +1,77 @@ +""" +If an author makes changes to the block after students have started using it, will bad things +happen? +""" +from .base_test import ProblemBuilderBaseTest +import re + + +class AuthorChangesTest(ProblemBuilderBaseTest): + """ + Test various scenarios involving author changes made to a block already in use by students + """ + def setUp(self): + super(AuthorChangesTest, self).setUp() + self.load_scenario("author_changes.xml", load_immediately=False) + self.refresh_page() + + def refresh_page(self): + """ + [Re]load the page with our scenario + """ + self.pb_block_dom = self.go_to_view("student_view") + self.reload_pb_block() + + def reload_pb_block(self): + """ + [Re]load the Problem Builder block, potentially with updated field data + """ + vertical = self.load_root_xblock() + self.pb_block = vertical.runtime.get_block(vertical.children[0]) + + def submit_answers(self, q1_answer='yes', q2_answer='elegance', q3_answer="It's boring."): + """ Answer all three questions in the 'author_changes.xml' scenario correctly """ + self.pb_block_dom.find_element_by_css_selector('input[name=q1][value={}]'.format(q1_answer)).click() + self.pb_block_dom.find_element_by_css_selector('input[name=q2][value={}]'.format(q2_answer)).click() + self.pb_block_dom.find_element_by_css_selector('textarea').send_keys(q3_answer) + self.click_submit(self.pb_block_dom) + + def test_delete_question(self): + """ Test what the block behaves correctly when deleting a question """ + # First, submit an answer to each of the three questions, but get the second question wrong: + self.submit_answers(q2_answer='bugs') + self.reload_pb_block() + self.assertEqual(self.pb_block.score.percentage, 67) + + # Delete the second question: + self.pb_block.children = [self.pb_block.children[0], self.pb_block.children[2]] + self.pb_block.save() + self.reload_pb_block() + + # Now that the wrong question is deleted, the student should have a perfect score: + self.assertEqual(self.pb_block.score.percentage, 100) + # NOTE: This is questionable, since the block does not send a new 'grade' event to the + # LMS. So the LMS 'grade' (based on the event sent when the student actually submitted + # the answers) and the block's current 'score' may be different. + + def test_reweight_question(self): + """ Test what the block behaves correctly when changing the weight of a question """ + # First, submit an answer to each of the three questions, but get the first question wrong: + self.submit_answers(q1_answer='no') + self.reload_pb_block() + self.assertEqual(self.pb_block.score.percentage, 67) + + # Re-weight Q1 to '5': + q1 = self.pb_block.runtime.get_block(self.pb_block.children[0]) + q1.weight = 5 + q1.save() + self.reload_pb_block() + self.assertEqual(self.pb_block.score.percentage, 29) # 29% is 2 out of 7 (5+1+1) + + # Delete Q2 (the MRQ) + self.pb_block.children = [self.pb_block.children[0], self.pb_block.children[2]] + self.pb_block.save() + self.reload_pb_block() + + # Now, the student's score should be 1 out of 6 (only q3 is correct): + self.assertEqual(self.pb_block.score.percentage, 17) diff --git a/problem_builder/tests/integration/test_dashboard.py b/problem_builder/tests/integration/test_dashboard.py index 12093314..b4912a1c 100644 --- a/problem_builder/tests/integration/test_dashboard.py +++ b/problem_builder/tests/integration/test_dashboard.py @@ -18,7 +18,7 @@ # "AGPLv3". If not, see . # from mock import Mock, patch -from .base_test import MentoringBaseTemplateTest +from .base_test import ProblemBuilderBaseTest class MockSubmissionsAPI(object): @@ -50,7 +50,7 @@ def get_submissions(self, key, limit=1): return [] -class TestDashboardBlock(MentoringBaseTemplateTest): +class TestDashboardBlock(ProblemBuilderBaseTest): """ Test the Student View of a dashboard XBlock linked to some problem builder blocks """ diff --git a/problem_builder/tests/integration/test_messages.py b/problem_builder/tests/integration/test_messages.py index 6112bfb9..4bdfa37c 100644 --- a/problem_builder/tests/integration/test_messages.py +++ b/problem_builder/tests/integration/test_messages.py @@ -17,7 +17,7 @@ # along with this program in a file in the toplevel directory called # "AGPLv3". If not, see . # -from .base_test import MentoringBaseTemplateTest +from .base_test import ProblemBuilderBaseTest import ddt @@ -32,7 +32,7 @@ @ddt.ddt -class MessagesTest(MentoringBaseTemplateTest): +class MessagesTest(ProblemBuilderBaseTest): """ Test the various types of message that can be added to a problem. """ diff --git a/problem_builder/tests/integration/xml_templates/author_changes.xml b/problem_builder/tests/integration/xml_templates/author_changes.xml new file mode 100644 index 00000000..86b62409 --- /dev/null +++ b/problem_builder/tests/integration/xml_templates/author_changes.xml @@ -0,0 +1,17 @@ + + + + + Yes + No + + + + Its elegance + Its bugs + + + + + + diff --git a/requirements.txt b/requirements.txt index 51b38395..17829910 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ddt mock unicodecsv==0.9.4 --e git+https://github.com/edx/xblock-utils.git@b2a17fa3793e98e67bdb86273317c41b6297dcbb#egg=xblock-utils +-e git+https://github.com/edx/xblock-utils.git@a0e77eeb4eb971ac57243fe1056dd8db6806f514#egg=xblock-utils -e . diff --git a/test_requirements.txt b/test_requirements.txt index 7b7a84e8..2b0a2bac 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -1 +1 @@ --e git+https://github.com/edx/XBlock.git@496d3cb9aca1d9e0a18b0f5e73c7bede824e465f#egg=XBlock \ No newline at end of file +-e git+https://github.com/edx/XBlock.git@496d3cb9aca1d9e0a18b0f5e73c7bede824e465f#egg=XBlock