From 28e64d95cc969ae1472b80025cbb3a8814960c87 Mon Sep 17 00:00:00 2001 From: Jolyon Bloomfield Date: Thu, 30 Aug 2018 16:21:23 -0400 Subject: [PATCH] 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 97e9a665fc..e591946530 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1216,10 +1216,12 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): } 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 f5930f078b..986ce57531 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): 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 @@ class CapaModuleTest(unittest.TestCase): 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 @@ class CapaModuleTest(unittest.TestCase): 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): # 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 @@ class CapaModuleTest(unittest.TestCase): 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):