From ddd8d41906253307aba2817062db94fb7794e8da Mon Sep 17 00:00:00 2001 From: Peter Pinch Date: Mon, 7 May 2018 11:36:24 -0400 Subject: [PATCH 1/4] expose attempts to python custom grader --- .../lib/capa/capa/tests/test_responsetypes.py | 40 +++++++++++++++++++ common/lib/xmodule/xmodule/capa_base.py | 2 + .../xmodule/xmodule/tests/test_capa_module.py | 31 +++++++++----- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 676746a56f0b..b4779d6cf4c9 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -2057,6 +2057,46 @@ def check_func(expect, answer_given, options, dynamath): self.assertEqual(correctness, 'incorrect') self.assertEqual(msg, "Message text") + def test_function_code_with_attempt_number(self): + script = textwrap.dedent("""\ + def gradeit(expect, ans, **kwargs): + attempt = kwargs["attempt"] + message = "This is attempt number {}".format(str(attempt)) + return { + 'input_list': [ + { 'ok': True, 'msg': message}, + ] + } + """) + + problem = self.build_problem( + script=script, + cfn="gradeit", + expect="42", + cfn_extra_args="attempt" + ) + + # first attempt + input_dict = {'1_2_1': '42'} + problem.context['attempt'] = 1 + correct_map = problem.grade_answers(input_dict) + + correctness = correct_map.get_correctness('1_2_1') + msg = correct_map.get_msg('1_2_1') + + self.assertEqual(correctness, 'correct') + self.assertEqual(msg, "This is attempt number 1") + + # second attempt + problem.context['attempt'] = 2 + correct_map = problem.grade_answers(input_dict) + + correctness = correct_map.get_correctness('1_2_1') + msg = correct_map.get_msg('1_2_1') + + self.assertEqual(correctness, 'correct') + self.assertEqual(msg, "This is attempt number 2") + def test_multiple_inputs_return_one_status(self): # When given multiple inputs, the 'answer_given' argument # to the check_func() is a list of inputs diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 8afe4234721b..97abc3b4e914 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1216,6 +1216,7 @@ def submit_problem(self, data, override_time=False): } try: + self.lcp.context['attempt'] = self.attempts + 1 correct_map = self.lcp.grade_answers(answers) self.attempts = self.attempts + 1 self.lcp.done = True @@ -1678,6 +1679,7 @@ def update_correctness(self): Operates by creating a new correctness map based on the current state of the LCP, and updating the old correctness map of the LCP. """ + self.lcp.context['attempt'] = self.attempts new_correct_map = self.lcp.get_grade_from_current_answers(None) self.lcp.correct_map.update(new_correct_map) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 5bb1504a2678..15cb781cf7ed 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -650,6 +650,7 @@ def test_submit_problem_correct(self): # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 2) + self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_incorrect(self): @@ -668,6 +669,7 @@ def test_submit_problem_incorrect(self): # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_submit_problem_closed(self): module = CapaFactory.create(attempts=3) @@ -717,8 +719,9 @@ def test_submit_problem_resubmitted_no_randomize(self, rerandomize): self.assertEqual(result['success'], 'correct') - # Expect that number of attempts IS incremented + # Expect that number of attempts IS incremented, still same attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_submit_problem_queued(self): module = CapaFactory.create(attempts=1) @@ -852,8 +855,9 @@ def test_submit_problem_error(self): self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented + # Expect that the number of attempts is NOT incremented, but it is 2nd attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_error_with_codejail_exception(self): @@ -888,8 +892,9 @@ def test_submit_problem_error_with_codejail_exception(self): expected_msg = 'Couldn\'t execute jailed code' self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented + # Expect that the number of attempts is NOT incremented, but it is 2nd attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_other_errors(self): """ @@ -957,8 +962,9 @@ def test_submit_problem_error_nonascii(self): self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented + # Expect that the number of attempts is NOT incremented, but it is 2nd attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_error_with_staff_user(self): @@ -986,8 +992,9 @@ def test_submit_problem_error_with_staff_user(self): # We DO include traceback information for staff users self.assertIn('Traceback', result['success']) - # Expect that the number of attempts is NOT incremented + # Expect that the number of attempts is NOT incremented, but it is 2nd attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 2) @ddt.data( ("never", True, None, 'submitted'), @@ -1018,6 +1025,7 @@ def test_handle_ajax_show_correctness(self, show_correctness, is_correct, expect # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_reset_problem(self): module = CapaFactory.create(done=True) @@ -1093,6 +1101,7 @@ def test_rescore_problem_correct(self): # Expect that the number of attempts is not incremented self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_additional_correct(self): # make sure it also works when new correct answer has been added @@ -1107,8 +1116,9 @@ def test_rescore_problem_additional_correct(self): self.assertEqual(result['success'], 'incorrect') self.assertEqual(module.get_score(), (0, 1)) self.assertEqual(module.correct_map[answer_id]['correctness'], 'incorrect') - # Expect that the number of attempts is incremented + # Expect that the number of attempts is incremented, still same attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) # Simulate that after making an incorrect answer to the correct answer # the new calculated score is (1,1) @@ -1126,8 +1136,9 @@ def test_rescore_problem_additional_correct(self): # Expect that the problem is marked correct and user earned the score self.assertEqual(module.get_score(), (1, 1)) self.assertEqual(module.correct_map[answer_id]['correctness'], 'correct') - # Expect that the number of attempts is not incremented + # Expect that the number of attempts is not incremented, still same attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_incorrect(self): # make sure it also works when attempts have been reset, @@ -1143,8 +1154,9 @@ def test_rescore_problem_incorrect(self): # Expect that the problem is marked incorrect self.assertEqual(module.is_correct(), False) - # Expect that the number of attempts is not incremented + # Expect that the number of attempts is not incremented, still same attempt self.assertEqual(module.attempts, 0) + self.assertEqual(module.lcp.context['attempt'], 0) def test_rescore_problem_not_done(self): # Simulate that the problem is NOT done @@ -1174,8 +1186,9 @@ def _rescore_problem_error_helper(self, exception_class): with self.assertRaises(exception_class): module.rescore(only_if_higher=False) - # Expect that the number of attempts is NOT incremented + # Expect that the number of attempts is NOT incremented, still same attempt self.assertEqual(module.attempts, 1) + self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_student_input_error(self): self._rescore_problem_error_helper(StudentInputError) From 3763d2665118d23847f8e455bc8f6400cd96fccc Mon Sep 17 00:00:00 2001 From: Peter Pinch Date: Thu, 30 Aug 2018 15:57:42 -0400 Subject: [PATCH 2/4] clarify comments --- common/lib/xmodule/xmodule/capa_base.py | 4 +++- common/lib/xmodule/xmodule/tests/test_capa_module.py | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 97abc3b4e914..97e9a665fc72 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1216,7 +1216,9 @@ def submit_problem(self, data, override_time=False): } try: - self.lcp.context['attempt'] = self.attempts + 1 + # expose the attempt number to the python custom grader + # self.lcp.context['attempt'] is 1 based, but the self.attempts is 0 based + self.lcp.context['attempt'] = self.attempts + 1 correct_map = self.lcp.grade_answers(answers) self.attempts = self.attempts + 1 self.lcp.done = True diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 15cb781cf7ed..f5930f078b06 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1116,7 +1116,8 @@ def test_rescore_problem_additional_correct(self): self.assertEqual(result['success'], 'incorrect') self.assertEqual(module.get_score(), (0, 1)) self.assertEqual(module.correct_map[answer_id]['correctness'], 'incorrect') - # Expect that the number of attempts is incremented, still same attempt + + # Expect that the number of attempts is not incremented self.assertEqual(module.attempts, 1) self.assertEqual(module.lcp.context['attempt'], 1) From 28e64d95cc969ae1472b80025cbb3a8814960c87 Mon Sep 17 00:00:00 2001 From: Jolyon Bloomfield Date: Thu, 30 Aug 2018 16:21:23 -0400 Subject: [PATCH 3/4] Handle situation where attempts have been reset to 0 and problem is regraded; Update comments --- common/lib/xmodule/xmodule/capa_base.py | 8 +++-- .../xmodule/xmodule/tests/test_capa_module.py | 30 ++++++++++++------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 97e9a665fc72..e591946530ec 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1216,10 +1216,12 @@ def submit_problem(self, data, override_time=False): } try: - # expose the attempt number to the python custom grader - # self.lcp.context['attempt'] is 1 based, but the self.attempts is 0 based - self.lcp.context['attempt'] = self.attempts + 1 + # expose the attempt number to a potential python custom grader + # self.lcp.context['attempt'] refers to the attempt number (1-based) + self.lcp.context['attempt'] = self.attempts + 1 correct_map = self.lcp.grade_answers(answers) + # self.attempts refers to the number of attempts that did not + # raise an error (0-based) self.attempts = self.attempts + 1 self.lcp.done = True self.set_state_from_lcp() diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index f5930f078b06..986ce57531af 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -650,6 +650,7 @@ def test_submit_problem_correct(self): # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 2) + # and that this was considered attempt number 2 for grading purposes self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_incorrect(self): @@ -669,6 +670,7 @@ def test_submit_problem_incorrect(self): # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 1) + # and that this is considered the first attempt self.assertEqual(module.lcp.context['attempt'], 1) def test_submit_problem_closed(self): @@ -855,8 +857,9 @@ def test_submit_problem_error(self): self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented, but it is 2nd attempt + # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + # but that this was considered attempt number 2 for grading purposes self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_error_with_codejail_exception(self): @@ -892,8 +895,9 @@ def test_submit_problem_error_with_codejail_exception(self): expected_msg = 'Couldn\'t execute jailed code' self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented, but it is 2nd attempt + # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + # but that this was considered the second attempt for grading purposes self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_other_errors(self): @@ -962,8 +966,9 @@ def test_submit_problem_error_nonascii(self): self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented, but it is 2nd attempt + # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + # but that this was considered the second attempt for grading purposes self.assertEqual(module.lcp.context['attempt'], 2) def test_submit_problem_error_with_staff_user(self): @@ -992,8 +997,9 @@ def test_submit_problem_error_with_staff_user(self): # We DO include traceback information for staff users self.assertIn('Traceback', result['success']) - # Expect that the number of attempts is NOT incremented, but it is 2nd attempt + # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + # but that it was considered the second attempt for grading purposes self.assertEqual(module.lcp.context['attempt'], 2) @ddt.data( @@ -1101,6 +1107,7 @@ def test_rescore_problem_correct(self): # Expect that the number of attempts is not incremented self.assertEqual(module.attempts, 1) + # and that this was considered attempt number 1 for grading purposes self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_additional_correct(self): @@ -1116,8 +1123,8 @@ def test_rescore_problem_additional_correct(self): self.assertEqual(result['success'], 'incorrect') self.assertEqual(module.get_score(), (0, 1)) self.assertEqual(module.correct_map[answer_id]['correctness'], 'incorrect') - - # Expect that the number of attempts is not incremented + + # Expect that the number of attempts has incremented to 1 self.assertEqual(module.attempts, 1) self.assertEqual(module.lcp.context['attempt'], 1) @@ -1137,8 +1144,9 @@ def test_rescore_problem_additional_correct(self): # Expect that the problem is marked correct and user earned the score self.assertEqual(module.get_score(), (1, 1)) self.assertEqual(module.correct_map[answer_id]['correctness'], 'correct') - # Expect that the number of attempts is not incremented, still same attempt + # Expect that the number of attempts is not incremented self.assertEqual(module.attempts, 1) + # and hence that this was still considered the first attempt for grading purposes self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_incorrect(self): @@ -1155,9 +1163,10 @@ def test_rescore_problem_incorrect(self): # Expect that the problem is marked incorrect self.assertEqual(module.is_correct(), False) - # Expect that the number of attempts is not incremented, still same attempt + # Expect that the number of attempts is not incremented self.assertEqual(module.attempts, 0) - self.assertEqual(module.lcp.context['attempt'], 0) + # and that this is treated as the first attempt for grading purposes + self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_not_done(self): # Simulate that the problem is NOT done @@ -1187,8 +1196,9 @@ def _rescore_problem_error_helper(self, exception_class): with self.assertRaises(exception_class): module.rescore(only_if_higher=False) - # Expect that the number of attempts is NOT incremented, still same attempt + # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + # and that this was considered the first attempt for grading purposes self.assertEqual(module.lcp.context['attempt'], 1) def test_rescore_problem_student_input_error(self): From 9981b153b35608b5747ec4d3c792c37ea4e9c29c Mon Sep 17 00:00:00 2001 From: Jolyon Bloomfield Date: Thu, 30 Aug 2018 16:22:50 -0400 Subject: [PATCH 4/4] Remember to add changes before committing... --- common/lib/xmodule/xmodule/capa_base.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index e591946530ec..f23c2db81bc3 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1683,7 +1683,9 @@ def update_correctness(self): Operates by creating a new correctness map based on the current state of the LCP, and updating the old correctness map of the LCP. """ - self.lcp.context['attempt'] = self.attempts + # Make sure that the attempt number is always at least 1 for grading purposes, + # even if the number of attempts have been reset and this problem is regraded. + self.lcp.context['attempt'] = max(self.attempts, 1) new_correct_map = self.lcp.get_grade_from_current_answers(None) self.lcp.correct_map.update(new_correct_map)