From 082216395c57d3f6c89e97178c4e5fa1ffe44fa8 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 4 Mar 2013 12:38:11 -0500 Subject: [PATCH 01/13] Added unit test for conversion of GET parameters for capa module. Added error checking for possible error cases --- common/lib/xmodule/xmodule/capa_module.py | 35 ++++++++++--- .../xmodule/xmodule/tests/test_capa_module.py | 50 +++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ade5cc6ef6..05a12f1761 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -534,15 +534,34 @@ class CapaModule(XModule): # e.g. input_resistor_1 ==> resistor_1 _, _, name = key.partition('_') - # This allows for answers which require more than one value for - # the same form input (e.g. checkbox inputs). The convention is that - # if the name ends with '[]' (which looks like an array), then the - # answer will be an array. - if not name.endswith('[]'): - answers[name] = get[key] + # If key has no underscores, then partition + # will return (key, '', '') + # We detect this and raise an error + if name is '': + raise ValueError("%s must contain at least one underscore" % str(key)) + else: - name = name[:-2] - answers[name] = get.getlist(key) + # This allows for answers which require more than one value for + # the same form input (e.g. checkbox inputs). The convention is that + # if the name ends with '[]' (which looks like an array), then the + # answer will be an array. + is_list_key = name.endswith('[]') + name = name[:-2] if is_list_key else name + + if is_list_key: + if type(get[key]) is list: + val = get[key] + else: + val = [get[key]] + else: + val = get[key] + + # If the name already exists, then we don't want + # to override it. Raise an error instead + if name in answers: + raise ValueError("Key %s already exists in answers dict" % str(name)) + else: + answers[name] = val return answers diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index e84267c1e7..7b565ff2da 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -282,3 +282,53 @@ class CapaModuleTest(unittest.TestCase): due=self.yesterday_str, graceperiod=self.two_day_delta_str) self.assertTrue(still_in_grace.answer_available()) + + + def test_parse_get_params(self): + + # Valid GET param dict + valid_get_dict = {'input_1': 'test', + 'input_1_2': 'test', + 'input_1_2_3': 'test', + 'input_[]_3': 'test', + 'input_4': None, + 'input_5': [], + 'input_6': 5} + + result = CapaModule.make_dict_of_responses(valid_get_dict) + + # Expect that we get a dict with "input" stripped from key names + # and that we get the same values back + for key in result.keys(): + original_key = "input_" + key + self.assertTrue(original_key in valid_get_dict, + "Output dict should have key %s" % original_key) + self.assertEqual(valid_get_dict[original_key], result[key]) + + + # Valid GET param dict with list keys + valid_get_dict = {'input_2[]': ['test1', 'test2']} + result = CapaModule.make_dict_of_responses(valid_get_dict) + self.assertTrue('2' in result) + self.assertEqual(valid_get_dict['input_2[]'], result['2']) + + # If we use [] at the end of a key name, we should always + # get a list, even if there's just one value + valid_get_dict = {'input_1[]': 'test'} + result = CapaModule.make_dict_of_responses(valid_get_dict) + self.assertEqual(result['1'], ['test']) + + + # If we have no underscores in the name, then the key is invalid + invalid_get_dict = {'input': 'test'} + with self.assertRaises(ValueError): + result = CapaModule.make_dict_of_responses(invalid_get_dict) + + + # Two equivalent names (one list, one non-list) + # One of the values would overwrite the other, so detect this + # and raise an exception + invalid_get_dict = {'input_1[]': 'test 1', + 'input_1': 'test 2' } + with self.assertRaises(ValueError): + result = CapaModule.make_dict_of_responses(invalid_get_dict) From 1cc895078067e00f1b194528b0c660f1ffd43051 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 4 Mar 2013 12:45:13 -0500 Subject: [PATCH 02/13] Added comments for make_dict_of_responses() in capa module --- common/lib/xmodule/xmodule/capa_module.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 05a12f1761..d9ada0038e 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -528,6 +528,27 @@ class CapaModule(XModule): def make_dict_of_responses(get): '''Make dictionary of student responses (aka "answers") get is POST dictionary. + + The *get* dict has keys of the form 'x_y', which are mapped + to key 'y' in the returned dict. For example, + 'input_1_2_3' would be mapped to '1_2_3' in the returned dict. + + Some inputs always expect a list in the returned dict + (e.g. checkbox inputs). The convention is that + keys in the *get* dict that end with '[]' will always + have list values in the returned dict. + For example, if the *get* dict contains {'input_1[]': 'test' } + then the output dict would contain {'1': ['test'] } + (the value is a list). + + Raises an exception if: + + A key in the *get* dictionary does not contain >= 1 underscores + (e.g. "input" is invalid; "input_1" is valid) + + Two keys end up with the same name in the returned dict. + (e.g. 'input_1' and 'input_1[]', which both get mapped + to 'input_1' in the returned dict) ''' answers = dict() for key in get: From d14890685179460236fbdde5b040c677252de244 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 4 Mar 2013 16:06:14 -0500 Subject: [PATCH 03/13] Added unit tests for capa module check_problem() method --- .../xmodule/xmodule/tests/test_capa_module.py | 147 ++++++++++++++++-- 1 file changed, 137 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 7b565ff2da..c569309af9 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1,9 +1,11 @@ import datetime import json -from mock import Mock +from mock import Mock, patch from pprint import pprint import unittest +import xmodule +import capa from xmodule.capa_module import CapaModule from xmodule.modulestore import Location from lxml import etree @@ -33,6 +35,14 @@ class CapaFactory(object): CapaFactory.num += 1 return CapaFactory.num + @staticmethod + def input_key(): + """ Return the input key to use when passing GET parameters """ + return ("input_" + + "-".join(['i4x', 'edX', 'capa_test', 'problem', + 'SampleProblem%d' % CapaFactory.num]) + + "_2_1") + @staticmethod def create(graceperiod=None, due=None, @@ -59,11 +69,10 @@ class CapaFactory(object): module. attempts: also added to instance state. Will be converted to an int. - correct: if True, the problem will be initialized to be answered correctly. """ definition = {'data': CapaFactory.sample_problem_xml, } location = Location(["i4x", "edX", "capa_test", "problem", - "SampleProblem{0}".format(CapaFactory.next_num())]) + "SampleProblem%d" % CapaFactory.next_num()]) metadata = {} if graceperiod is not None: metadata['graceperiod'] = graceperiod @@ -89,13 +98,6 @@ class CapaFactory(object): # since everything else is a string. instance_state_dict['attempts'] = int(attempts) - if correct: - # TODO: make this actually set an answer of 3.14, and mark it correct - #instance_state_dict['student_answers'] = {} - #instance_state_dict['correct_map'] = {} - pass - - if len(instance_state_dict) > 0: instance_state = json.dumps(instance_state_dict) else: @@ -332,3 +334,128 @@ class CapaModuleTest(unittest.TestCase): 'input_1': 'test 2' } with self.assertRaises(ValueError): result = CapaModule.make_dict_of_responses(invalid_get_dict) + + def test_check_problem_correct(self): + + module = CapaFactory.create(attempts=1) + + # Simulate that all answers are marked correct, no matter + # what the input is, by patching CorrectMap.is_correct() + with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + mock_is_correct.return_value = True + + # Check the problem + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.check_problem(get_request_dict) + + # Expect that the problem is marked correct + self.assertEqual(result['success'], 'correct') + + # Expect that the number of attempts is incremented by 1 + self.assertEqual(module.attempts, 2) + + + def test_check_problem_incorrect(self): + + module = CapaFactory.create(attempts=0) + + # Simulate marking the input incorrect + with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + mock_is_correct.return_value = False + + # Check the problem + get_request_dict = { CapaFactory.input_key(): '0' } + result = module.check_problem(get_request_dict) + + # Expect that the problem is marked correct + self.assertEqual(result['success'], 'incorrect') + + # Expect that the number of attempts is incremented by 1 + self.assertEqual(module.attempts, 1) + + + def test_check_problem_closed(self): + module = CapaFactory.create(attempts=3) + + # Problem closed -- cannot submit + # Simulate that CapaModule.closed() always returns True + with patch('xmodule.capa_module.CapaModule.closed') as mock_closed: + mock_closed.return_value = True + with self.assertRaises(xmodule.exceptions.NotFoundError): + get_request_dict = { CapaFactory.input_key(): '3.14' } + module.check_problem(get_request_dict) + + # Expect that number of attempts NOT incremented + self.assertEqual(module.attempts, 3) + + + def test_check_problem_resubmitted_with_randomize(self): + # Randomize turned on + module = CapaFactory.create(rerandomize='always', attempts=0) + + # Simulate that the problem is completed + module.lcp.done = True + + # Expect that we cannot submit + with self.assertRaises(xmodule.exceptions.NotFoundError): + get_request_dict = { CapaFactory.input_key(): '3.14' } + module.check_problem(get_request_dict) + + # Expect that number of attempts NOT incremented + self.assertEqual(module.attempts, 0) + + + def test_check_problem_resubmitted_no_randomize(self): + # Randomize turned off + module = CapaFactory.create(rerandomize='never', attempts=0) + + # Simulate that the problem is completed + module.lcp.done = True + + # Expect that we can submit successfully + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.check_problem(get_request_dict) + + self.assertEqual(result['success'], 'correct') + + # Expect that number of attempts IS incremented + self.assertEqual(module.attempts, 1) + + + def test_check_problem_queued(self): + module = CapaFactory.create(attempts=1) + + # Simulate that the problem is queued + with patch('capa.capa_problem.LoncapaProblem.is_queued') \ + as mock_is_queued,\ + patch('capa.capa_problem.LoncapaProblem.get_recentmost_queuetime') \ + as mock_get_queuetime: + + mock_is_queued.return_value = True + mock_get_queuetime.return_value = datetime.datetime.now() + + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.check_problem(get_request_dict) + + # Expect an AJAX alert message in 'success' + self.assertTrue('You must wait' in result['success']) + + # Expect that the number of attempts is NOT incremented + self.assertEqual(module.attempts, 1) + + + def test_check_problem_student_input_error(self): + module = CapaFactory.create(attempts=1) + + # Simulate a student input exception + with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + mock_grade.side_effect = capa.responsetypes.StudentInputError('test error') + + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.check_problem(get_request_dict) + + # Expect an AJAX alert message in 'success' + self.assertTrue('test error' in result['success']) + + # Expect that the number of attempts is NOT incremented + self.assertEqual(module.attempts, 1) From 9743f6bea9ce4cdcf859d5f0c8a735e3a287080c Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 4 Mar 2013 16:16:38 -0500 Subject: [PATCH 04/13] Fixed indentation in capa module. Added test of HTML output for successful check to capa module unit tests --- common/lib/xmodule/xmodule/capa_module.py | 6 +++--- common/lib/xmodule/xmodule/tests/test_capa_module.py | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index d9ada0038e..cf1d786b72 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -649,11 +649,11 @@ class CapaModule(XModule): # 'success' will always be incorrect event_info['correct_map'] = correct_map.get_dict() event_info['success'] = success - event_info['attempts'] = self.attempts + event_info['attempts'] = self.attempts self.system.track_function('save_problem_check', event_info) - if hasattr(self.system, 'psychometrics_handler'): # update PsychometricsData using callback - self.system.psychometrics_handler(self.get_instance_state()) + if hasattr(self.system, 'psychometrics_handler'): # update PsychometricsData using callback + self.system.psychometrics_handler(self.get_instance_state()) # render problem into HTML html = self.get_problem_html(encapsulate=False) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index c569309af9..453365f73b 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -341,8 +341,11 @@ class CapaModuleTest(unittest.TestCase): # Simulate that all answers are marked correct, no matter # what the input is, by patching CorrectMap.is_correct() - with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + # Also simulate rendering the HTML + with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct,\ + patch('xmodule.capa_module.CapaModule.get_problem_html') as mock_html: mock_is_correct.return_value = True + mock_html.return_value = "Test HTML" # Check the problem get_request_dict = { CapaFactory.input_key(): '3.14' } @@ -351,6 +354,9 @@ class CapaModuleTest(unittest.TestCase): # Expect that the problem is marked correct self.assertEqual(result['success'], 'correct') + # Expect that we get the (mocked) HTML + self.assertEqual(result['contents'], 'Test HTML') + # Expect that the number of attempts is incremented by 1 self.assertEqual(module.attempts, 2) From 21af0493fdda90e389955a8c939290c5b2b175b5 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 11:21:28 -0500 Subject: [PATCH 05/13] Added unit tests for capa_module reset_problem() --- common/lib/xmodule/xmodule/capa_module.py | 10 +++- .../xmodule/xmodule/tests/test_capa_module.py | 59 ++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index cf1d786b72..22f09b4591 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -703,7 +703,12 @@ class CapaModule(XModule): ''' Changes problem state to unfinished -- removes student answers, and causes problem to rerender itself. - Returns problem html as { 'html' : html-string }. + Returns a dictionary of the form: + {'success': True/False, + 'html': Problem HTML string } + + If an error occurs, the dictionary will also have an + 'error' key containing an error message. ''' event_info = dict() event_info['old_state'] = self.lcp.get_state() @@ -734,7 +739,8 @@ class CapaModule(XModule): event_info['new_state'] = self.lcp.get_state() self.system.track_function('reset_problem', event_info) - return {'html': self.get_problem_html(encapsulate=False)} + return { 'success': True, + 'html': self.get_problem_html(encapsulate=False)} class CapaDescriptor(RawDescriptor): diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 453365f73b..afe4a73725 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1,6 +1,6 @@ import datetime import json -from mock import Mock, patch +from mock import Mock, MagicMock, patch from pprint import pprint import unittest @@ -465,3 +465,60 @@ class CapaModuleTest(unittest.TestCase): # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + + + def test_reset_problem(self): + module = CapaFactory.create() + + # Mock the module's capa problem + # to simulate that the problem is done + mock_problem = MagicMock(capa.capa_problem.LoncapaProblem) + mock_problem.done = True + module.lcp = mock_problem + + # Stub out HTML rendering + with patch('xmodule.capa_module.CapaModule.get_problem_html') as mock_html: + mock_html.return_value = "
Test HTML
" + + # Reset the problem + get_request_dict = {} + result = module.reset_problem(get_request_dict) + + # Expect that the request was successful + self.assertTrue('success' in result and result['success']) + + # Expect that the problem HTML is retrieved + self.assertTrue('html' in result) + self.assertEqual(result['html'], "
Test HTML
") + + # Expect that the problem was reset + mock_problem.do_reset.assert_called_once_with() + + + def test_reset_problem_closed(self): + module = CapaFactory.create() + + # Simulate that the problem is closed + with patch('xmodule.capa_module.CapaModule.closed') as mock_closed: + mock_closed.return_value = True + + # Try to reset the problem + get_request_dict = {} + result = module.reset_problem(get_request_dict) + + # Expect that the problem was NOT reset + self.assertTrue('success' in result and not result['success']) + + + def test_reset_problem_not_done(self): + module = CapaFactory.create() + + # Simulate that the problem is NOT done + module.lcp.done = False + + # Try to reset the problem + get_request_dict = {} + result = module.reset_problem(get_request_dict) + + # Expect that the problem was NOT reset + self.assertTrue('success' in result and not result['success']) From 49784009d538a686e8b47455b5e4c83d7b95ae9e Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 11:23:14 -0500 Subject: [PATCH 06/13] Fixed incorrect doc string --- common/lib/xmodule/xmodule/capa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 22f09b4591..8e2f8825de 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -590,7 +590,7 @@ class CapaModule(XModule): ''' Checks whether answers to a problem are correct, and returns a map of correct/incorrect answers: - {'success' : bool, + {'success' : 'correct' | 'incorrect' | AJAX alert msg string, 'contents' : html} ''' event_info = dict() From 6ec3b94d0c0df050257fffc9deab4783206777ef Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 11:47:41 -0500 Subject: [PATCH 07/13] Added test for capa module closed() method; modified closed() to handle edge cases --- common/lib/xmodule/xmodule/capa_module.py | 2 +- .../xmodule/xmodule/tests/test_capa_module.py | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 8e2f8825de..5e0e2e9ac4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -419,7 +419,7 @@ class CapaModule(XModule): def closed(self): ''' Is the student still allowed to submit answers? ''' - if self.attempts == self.max_attempts: + if self.max_attempts is not None and self.attempts >= self.max_attempts: return True if self.is_past_due(): return True diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index afe4a73725..803b91d139 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -286,6 +286,34 @@ class CapaModuleTest(unittest.TestCase): self.assertTrue(still_in_grace.answer_available()) + def test_closed(self): + + # Attempts < Max attempts --> NOT closed + module = CapaFactory.create(max_attempts="1", attempts="0") + self.assertFalse(module.closed()) + + # Attempts < Max attempts --> NOT closed + module = CapaFactory.create(max_attempts="2", attempts="1") + self.assertFalse(module.closed()) + + # Attempts = Max attempts --> closed + module = CapaFactory.create(max_attempts="1", attempts="1") + self.assertTrue(module.closed()) + + # Attempts > Max attempts --> closed + module = CapaFactory.create(max_attempts="1", attempts="2") + self.assertTrue(module.closed()) + + # Max attempts = 0 --> closed + module = CapaFactory.create(max_attempts="0", attempts="2") + self.assertTrue(module.closed()) + + # Past due --> closed + module = CapaFactory.create(max_attempts="1", attempts="0", + due=self.yesterday_str) + self.assertTrue(module.closed()) + + def test_parse_get_params(self): # Valid GET param dict @@ -522,3 +550,21 @@ class CapaModuleTest(unittest.TestCase): # Expect that the problem was NOT reset self.assertTrue('success' in result and not result['success']) + + + def test_save_problem(self): + module = CapaFactory.create() + + # Simulate + + + def test_save_problem_closed(self): + self.fail() + + + def test_save_problem_submitted_with_randomize(self): + self.fail() + + + def test_save_problem_submitted_no_randomize(self): + self.fail() From 42043d2ccc4d0600b1610d4e3a195d7b6ba7e145 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 12:01:04 -0500 Subject: [PATCH 08/13] Added tests for capa module save_problem() --- .../xmodule/xmodule/tests/test_capa_module.py | 62 +++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 803b91d139..2469431217 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -38,8 +38,12 @@ class CapaFactory(object): @staticmethod def input_key(): """ Return the input key to use when passing GET parameters """ - return ("input_" + - "-".join(['i4x', 'edX', 'capa_test', 'problem', + return ("input_" + CapaFactory.answer_key()) + + @staticmethod + def answer_key(): + """ Return the key stored in the capa problem answer dict """ + return ("-".join(['i4x', 'edX', 'capa_test', 'problem', 'SampleProblem%d' % CapaFactory.num]) + "_2_1") @@ -555,16 +559,62 @@ class CapaModuleTest(unittest.TestCase): def test_save_problem(self): module = CapaFactory.create() - # Simulate + # Simulate that the problem is not done (not attempted or reset) + module.lcp.done = False + + # Save the problem + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.save_problem(get_request_dict) + + # Expect that answers are saved to the problem + expected_answers = { CapaFactory.answer_key(): '3.14' } + self.assertEqual(module.lcp.student_answers, expected_answers) + + # Expect that the result is success + self.assertTrue('success' in result and result['success']) def test_save_problem_closed(self): - self.fail() + module = CapaFactory.create() + + # Simulate that the problem is NOT done (not attempted or reset) + module.lcp.done = False + + # Simulate that the problem is closed + with patch('xmodule.capa_module.CapaModule.closed') as mock_closed: + mock_closed.return_value = True + + # Try to save the problem + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.save_problem(get_request_dict) + + # Expect that the result is failure + self.assertTrue('success' in result and not result['success']) def test_save_problem_submitted_with_randomize(self): - self.fail() + module = CapaFactory.create(rerandomize='always') + + # Simulate that the problem is completed + module.lcp.done = True + + # Try to save + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.save_problem(get_request_dict) + + # Expect that we cannot save + self.assertTrue('success' in result and not result['success']) def test_save_problem_submitted_no_randomize(self): - self.fail() + module = CapaFactory.create(rerandomize='never') + + # Simulate that the problem is completed + module.lcp.done = True + + # Try to save + get_request_dict = { CapaFactory.input_key(): '3.14' } + result = module.save_problem(get_request_dict) + + # Expect that we succeed + self.assertTrue('success' in result and result['success']) From 608552429f5c5f456e3bd104cc43a6943c80061b Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 15:35:14 -0500 Subject: [PATCH 09/13] Refactored capa module HTML rendering, isolating logic for determining which buttons to show --- common/lib/xmodule/xmodule/capa_module.py | 130 ++++++++------ .../xmodule/xmodule/tests/test_capa_module.py | 162 ++++++++++++++++++ 2 files changed, 242 insertions(+), 50 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 5e0e2e9ac4..19bddd1a19 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -136,7 +136,7 @@ class CapaModule(XModule): self.close_date = self.display_due_date max_attempts = self.metadata.get('attempts', None) - if max_attempts: + if max_attempts is not None: self.max_attempts = int(max_attempts) else: self.max_attempts = None @@ -247,6 +247,78 @@ class CapaModule(XModule): 'progress': Progress.to_js_status_str(self.get_progress()) }) + def check_button_name(self): + """ + Determine the name for the "check" button. + Usually it is just "Check", but if this is the student's + final attempt, change the name to "Final Check" + """ + if self.max_attempts is not None: + final_check = (self.attempts >= self.max_attempts - 1) + else: + final_check = False + + return "Final Check" if final_check else "Check" + + def should_show_check_button(self): + """ + Return True/False to indicate whether to show the "Check" button. + """ + submitted_without_reset = (self.is_completed() and self.rerandomize == "always") + + # If the problem is closed (past due / too many attempts) + # then we do NOT show the "check" button + # Also, do not show the "check" button if we're waiting + # for the user to reset a randomized problem + if self.closed() or submitted_without_reset: + return False + else: + return True + + def should_show_reset_button(self): + """ + Return True/False to indicate whether to show the "Reset" button. + """ + survey_question = (self.max_attempts == 0) + + if self.rerandomize in ["always", "onreset"]: + + # If the problem is closed (and not a survey question with max_attempts==0), + # then do NOT show the reset button. + # If the problem hasn't been submitted yet, then do NOT show + # the reset button. + if (self.closed() and not survey_question) or not self.is_completed(): + return False + else: + return True + + # Only randomized problems need a "reset" button + else: + return False + + def should_show_save_button(self): + """ + Return True/False to indicate whether to show the "Save" button. + """ + + # If the user has forced the save button to display, + # then show it as long as the problem is not closed + # (past due / too many attempts) + if self.force_save_button == "true": + return not self.closed() + else: + survey_question = (self.max_attempts == 0) + needs_reset = self.is_completed() and self.rerandomize == "always" + + # If the problem is closed (and not a survey question with max_attempts==0), + # then do NOT show the reset button + # If we're waiting for the user to reset a randomized problem + # then do NOT show the reset button + if (self.closed() and not survey_question) or needs_reset: + return False + else: + return True + def get_problem_html(self, encapsulate=True): '''Return html for the problem. Adds check, reset, save buttons as necessary based on the problem config and state.''' @@ -313,57 +385,14 @@ class CapaModule(XModule): 'weight': self.descriptor.weight, } - # We using strings as truthy values, because the terminology of the - # check button is context-specific. - - # Put a "Check" button if unlimited attempts or still some left - if self.max_attempts is None or self.attempts < self.max_attempts - 1: - check_button = "Check" - else: - # Will be final check so let user know that - check_button = "Final Check" - - reset_button = True - save_button = True - - # If we're after deadline, or user has exhausted attempts, - # question is read-only. - if self.closed(): - check_button = False - reset_button = False - save_button = False - - # If attempts=0 then show just check and reset buttons; this is for survey questions using capa - if self.max_attempts==0: - check_button = False - reset_button = True - save_button = True - - # User submitted a problem, and hasn't reset. We don't want - # more submissions. - if self.lcp.done and self.rerandomize == "always": - check_button = False - save_button = False - - # Only show the reset button if pressing it will show different values - if self.rerandomize not in ["always", "onreset"]: - reset_button = False - - # User hasn't submitted an answer yet -- we don't want resets - if not self.lcp.done: - reset_button = False - - # We may not need a "save" button if infinite number of attempts and - # non-randomized. The problem author can force it. It's a bit weird for - # randomization to control this; should perhaps be cleaned up. - if (self.force_save_button == "false") and (self.max_attempts is None and self.rerandomize != "always"): - save_button = False - context = {'problem': content, 'id': self.id, - 'check_button': check_button, - 'reset_button': reset_button, - 'save_button': save_button, + + # Pass in the name of the check button or False + # if we do not need a check button + 'check_button': self.check_button_name() if self.should_show_check_button() else False, + 'reset_button': self.should_show_reset_button(), + 'save_button': self.should_show_save_button(), 'answer_available': self.answer_available(), 'ajax_url': self.system.ajax_url, 'attempts_used': self.attempts, @@ -731,6 +760,7 @@ class CapaModule(XModule): # reset random number generator seed (note the self.lcp.get_state() # in next line) self.lcp.seed = None + self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), self.lcp.get_state(), diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 2469431217..1ac9c2e644 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -3,6 +3,7 @@ import json from mock import Mock, MagicMock, patch from pprint import pprint import unittest +import random import xmodule import capa @@ -618,3 +619,164 @@ class CapaModuleTest(unittest.TestCase): # Expect that we succeed self.assertTrue('success' in result and result['success']) + + def test_check_button_name(self): + + # If last attempt, button name changes to "Final Check" + # Just in case, we also check what happens if we have + # more attempts than allowed. + attempts = random.randint(1, 10) + module = CapaFactory.create(attempts=attempts-1, max_attempts=attempts) + self.assertEqual(module.check_button_name(), "Final Check") + + module = CapaFactory.create(attempts=attempts, max_attempts=attempts) + self.assertEqual(module.check_button_name(), "Final Check") + + module = CapaFactory.create(attempts=attempts + 1, max_attempts=attempts) + self.assertEqual(module.check_button_name(), "Final Check") + + # Otherwise, button name is "Check" + module = CapaFactory.create(attempts=attempts-2, max_attempts=attempts) + self.assertEqual(module.check_button_name(), "Check") + + module = CapaFactory.create(attempts=attempts-3, max_attempts=attempts) + self.assertEqual(module.check_button_name(), "Check") + + # If no limit on attempts, then always show "Check" + module = CapaFactory.create(attempts=attempts-3) + self.assertEqual(module.check_button_name(), "Check") + + module = CapaFactory.create(attempts=0) + self.assertEqual(module.check_button_name(), "Check") + + def test_should_show_check_button(self): + + attempts = random.randint(1,10) + + # If we're after the deadline, do NOT show check button + module = CapaFactory.create(due=self.yesterday_str) + self.assertFalse(module.should_show_check_button()) + + # If user is out of attempts, do NOT show the check button + module = CapaFactory.create(attempts=attempts, max_attempts=attempts) + self.assertFalse(module.should_show_check_button()) + + # If survey question (max_attempts = 0), do NOT show the check button + module = CapaFactory.create(max_attempts=0) + self.assertFalse(module.should_show_check_button()) + + # If user submitted a problem but hasn't reset, + # do NOT show the check button + # Note: we can only reset when rerandomize="always" + module = CapaFactory.create(rerandomize="always") + module.lcp.done = True + self.assertFalse(module.should_show_check_button()) + + # Otherwise, DO show the check button + module = CapaFactory.create() + self.assertTrue(module.should_show_check_button()) + + # If the user has submitted the problem + # and we do NOT have a reset button, then we can show the check button + # Setting rerandomize to "never" ensures that the reset button + # is not shown + module = CapaFactory.create(rerandomize="never") + module.lcp.done = True + self.assertTrue(module.should_show_check_button()) + + + def test_should_show_reset_button(self): + + attempts = random.randint(1,10) + + # If we're after the deadline, do NOT show the reset button + module = CapaFactory.create(due=self.yesterday_str) + module.lcp.done = True + self.assertFalse(module.should_show_reset_button()) + + # If the user is out of attempts, do NOT show the reset button + module = CapaFactory.create(attempts=attempts, max_attempts=attempts) + module.lcp.done = True + self.assertFalse(module.should_show_reset_button()) + + # If we're NOT randomizing, then do NOT show the reset button + module = CapaFactory.create(rerandomize="never") + module.lcp.done = True + self.assertFalse(module.should_show_reset_button()) + + # If the user hasn't submitted an answer yet, + # then do NOT show the reset button + module = CapaFactory.create() + module.lcp.done = False + self.assertFalse(module.should_show_reset_button()) + + # Otherwise, DO show the reset button + module = CapaFactory.create() + module.lcp.done = True + self.assertTrue(module.should_show_reset_button()) + + # If survey question for capa (max_attempts = 0), + # DO show the reset button + module = CapaFactory.create(max_attempts=0) + module.lcp.done = True + self.assertTrue(module.should_show_reset_button()) + + + def test_should_show_save_button(self): + + attempts = random.randint(1,10) + + # If we're after the deadline, do NOT show the save button + module = CapaFactory.create(due=self.yesterday_str) + module.lcp.done = True + self.assertFalse(module.should_show_save_button()) + + # If the user is out of attempts, do NOT show the save button + module = CapaFactory.create(attempts=attempts, max_attempts=attempts) + module.lcp.done = True + self.assertFalse(module.should_show_save_button()) + + # If user submitted a problem but hasn't reset, do NOT show the save button + module = CapaFactory.create(rerandomize="always") + module.lcp.done = True + self.assertFalse(module.should_show_save_button()) + + # Otherwise, DO show the save button + module = CapaFactory.create() + module.lcp.done = False + self.assertTrue(module.should_show_save_button()) + + # If we're not randomizing, then we can re-save + module = CapaFactory.create(rerandomize="never") + module.lcp.done = True + self.assertTrue(module.should_show_save_button()) + + # If survey question for capa (max_attempts = 0), + # DO show the save button + module = CapaFactory.create(max_attempts=0) + module.lcp.done = False + self.assertTrue(module.should_show_save_button()) + + def test_should_show_save_button_force_save_button(self): + # If we're after the deadline, do NOT show the save button + # even though we're forcing a save + module = CapaFactory.create(due=self.yesterday_str, + force_save_button="true") + module.lcp.done = True + self.assertFalse(module.should_show_save_button()) + + # If the user is out of attempts, do NOT show the save button + attempts = random.randint(1,10) + module = CapaFactory.create(attempts=attempts, + max_attempts=attempts, + force_save_button="true") + module.lcp.done = True + self.assertFalse(module.should_show_save_button()) + + # Otherwise, if we force the save button, + # then show it even if we would ordinarily + # require a reset first + module = CapaFactory.create(force_save_button="true", + rerandomize="always") + module.lcp.done = True + self.assertTrue(module.should_show_save_button()) From 40d7e8addf0bbd3b12e14c853ad0fa0ae818fa25 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 15:43:47 -0500 Subject: [PATCH 10/13] Moved problem HTML error handling to its own function --- common/lib/xmodule/xmodule/capa_module.py | 122 ++++++++++++---------- 1 file changed, 69 insertions(+), 53 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 19bddd1a19..e6d26bf88e 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -319,66 +319,82 @@ class CapaModule(XModule): else: return True + def handle_problem_html_error(self, err): + """ + Change our problem to a dummy problem containing + a warning message to display to users. + + Returns the HTML to show to users + + *err* is the Exception encountered while rendering the problem HTML. + """ + log.exception(err) + + # TODO (vshnayder): another switch on DEBUG. + if self.system.DEBUG: + msg = ( + '[courseware.capa.capa_module] ' + 'Failed to generate HTML for problem %s' % + (self.location.url())) + msg += '

Error:

%s

' % str(err).replace('<', '<') + msg += '

%s

' % traceback.format_exc().replace('<', '<') + html = msg + + # We're in non-debug mode, and possibly even in production. We want + # to avoid bricking of problem as much as possible + else: + + # Presumably, student submission has corrupted LoncapaProblem HTML. + # First, pull down all student answers + student_answers = self.lcp.student_answers + answer_ids = student_answers.keys() + + # Some inputtypes, such as dynamath, have additional "hidden" state that + # is not exposed to the student. Keep those hidden + # TODO: Use regex, e.g. 'dynamath' is suffix at end of answer_id + hidden_state_keywords = ['dynamath'] + for answer_id in answer_ids: + for hidden_state_keyword in hidden_state_keywords: + if answer_id.find(hidden_state_keyword) >= 0: + student_answers.pop(answer_id) + + # Next, generate a fresh LoncapaProblem + self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), + state=None, # Tabula rasa + seed=self.seed, system=self.system) + + # Prepend a scary warning to the student + warning = '
'\ + '

Warning: The problem has been reset to its initial state!

'\ + 'The problem\'s state was corrupted by an invalid submission. ' \ + 'The submission consisted of:'\ + '
    ' + for student_answer in student_answers.values(): + if student_answer != '': + warning += '
  • ' + cgi.escape(student_answer) + '
    • ' + warning += '
'\ + 'If this error persists, please contact the course staff.'\ + '
' + + html = warning + try: + html += self.lcp.get_html() + except Exception, err: # Couldn't do it. Give up + log.exception(err) + raise + + return html + + def get_problem_html(self, encapsulate=True): '''Return html for the problem. Adds check, reset, save buttons as necessary based on the problem config and state.''' try: html = self.lcp.get_html() + except Exception, err: - log.exception(err) - - # TODO (vshnayder): another switch on DEBUG. - if self.system.DEBUG: - msg = ( - '[courseware.capa.capa_module] ' - 'Failed to generate HTML for problem %s' % - (self.location.url())) - msg += '

Error:

%s

' % str(err).replace('<', '<') - msg += '

%s

' % traceback.format_exc().replace('<', '<') - html = msg - else: - # We're in non-debug mode, and possibly even in production. We want - # to avoid bricking of problem as much as possible - - # Presumably, student submission has corrupted LoncapaProblem HTML. - # First, pull down all student answers - student_answers = self.lcp.student_answers - answer_ids = student_answers.keys() - - # Some inputtypes, such as dynamath, have additional "hidden" state that - # is not exposed to the student. Keep those hidden - # TODO: Use regex, e.g. 'dynamath' is suffix at end of answer_id - hidden_state_keywords = ['dynamath'] - for answer_id in answer_ids: - for hidden_state_keyword in hidden_state_keywords: - if answer_id.find(hidden_state_keyword) >= 0: - student_answers.pop(answer_id) - - # Next, generate a fresh LoncapaProblem - self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), - state=None, # Tabula rasa - seed=self.seed, system=self.system) - - # Prepend a scary warning to the student - warning = '
'\ - '

Warning: The problem has been reset to its initial state!

'\ - 'The problem\'s state was corrupted by an invalid submission. ' \ - 'The submission consisted of:'\ - '
    ' - for student_answer in student_answers.values(): - if student_answer != '': - warning += '
  • ' + cgi.escape(student_answer) + '
    • ' - warning += '
'\ - 'If this error persists, please contact the course staff.'\ - '
' - - html = warning - try: - html += self.lcp.get_html() - except Exception, err: # Couldn't do it. Give up - log.exception(err) - raise + return self.handle_problem_html_error(err) content = {'name': self.display_name, 'html': html, From 73281a43c97406715d7bf4b51d6cc5c452aa7bb2 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 17:02:12 -0500 Subject: [PATCH 11/13] Added tests for get_problem_html, including error conditions --- common/lib/xmodule/xmodule/capa_module.py | 18 +++- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../xmodule/xmodule/tests/test_capa_module.py | 92 +++++++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index e6d26bf88e..b42b7e4ccc 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -393,8 +393,19 @@ class CapaModule(XModule): try: html = self.lcp.get_html() + # If we cannot construct the problem HTML, + # then generate an error message instead. except Exception, err: - return self.handle_problem_html_error(err) + html = self.handle_problem_html_error(err) + + + # The convention is to pass the name of the check button + # if we want to show a check button, and False otherwise + # This works because non-empty strings evaluate to True + if self.should_show_check_button(): + check_button = self.check_button_name() + else: + check_button = False content = {'name': self.display_name, 'html': html, @@ -403,10 +414,7 @@ class CapaModule(XModule): context = {'problem': content, 'id': self.id, - - # Pass in the name of the check button or False - # if we do not need a check button - 'check_button': self.check_button_name() if self.should_show_check_button() else False, + 'check_button': check_button, 'reset_button': self.should_show_reset_button(), 'save_button': self.should_show_save_button(), 'answer_available': self.answer_available(), diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 9474717cb2..220f122e7a 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -34,7 +34,7 @@ test_system = ModuleSystem( get_module=Mock(), # "render" to just the context... render_template=lambda template, context: str(context), - replace_urls=Mock(), + replace_urls=lambda html: str(html), user=Mock(is_staff=False), filestore=Mock(), debug=True, diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 1ac9c2e644..a42143da82 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -780,3 +780,95 @@ class CapaModuleTest(unittest.TestCase): rerandomize="always") module.lcp.done = True self.assertTrue(module.should_show_save_button()) + + def test_get_problem_html(self): + module = CapaFactory.create() + + # We've tested the show/hide button logic in other tests, + # so here we hard-wire the values + show_check_button = bool(random.randint(0,1) % 2) + show_reset_button = bool(random.randint(0,1) % 2) + show_save_button = bool(random.randint(0,1) % 2) + + module.should_show_check_button = Mock(return_value=show_check_button) + module.should_show_reset_button = Mock(return_value=show_reset_button) + module.should_show_save_button = Mock(return_value=show_save_button) + + # Mock the system rendering function (reset when we're done) + old_render_func = test_system.render_template + test_system.render_template = Mock(return_value="
Test Template HTML
") + + def cleanup_func(): + test_system.render_template = old_render_func + + self.addCleanup(cleanup_func) + + # Patch the capa problem's HTML rendering + with patch('capa.capa_problem.LoncapaProblem.get_html') as mock_html: + mock_html.return_value = "
Test Problem HTML
" + + # Render the problem HTML + html = module.get_problem_html(encapsulate=False) + + # Also render the problem encapsulated in a
+ html_encapsulated = module.get_problem_html(encapsulate=True) + + # Expect that we get the rendered template back + self.assertEqual(html, "
Test Template HTML
") + + # Check the rendering context + render_args,_ = test_system.render_template.call_args + self.assertEqual(len(render_args), 2) + + template_name = render_args[0] + self.assertEqual(template_name, "problem.html") + + context = render_args[1] + self.assertEqual(context['problem']['html'], "
Test Problem HTML
") + self.assertEqual(bool(context['check_button']), show_check_button) + self.assertEqual(bool(context['reset_button']), show_reset_button) + self.assertEqual(bool(context['save_button']), show_save_button) + + # Assert that the encapsulated html contains the original html + self.assertTrue(html in html_encapsulated) + + + def test_get_problem_html_error(self): + """ + In production, when an error occurs with the problem HTML + rendering, a "dummy" problem is created with an error + message to display to the user. + """ + module = CapaFactory.create() + + # Save the original problem so we can compare it later + original_problem = module.lcp + + # Simulate throwing an exception when the capa problem + # is asked to render itself as HTML + module.lcp.get_html = Mock(side_effect=Exception("Test")) + + # Stub out the test_system rendering function temporarily + old_render_func = test_system.render_template + test_system.render_template = Mock(return_value="
Test Template HTML
") + + # Turn off DEBUG temporarily + old_debug = test_system.DEBUG + test_system.DEBUG = False + + def cleanup_func(): + test_system.render_template = old_render_func + test_system.DEBUG = old_debug + + self.addCleanup(cleanup_func) + + # Try to render the module with DEBUG turned off + html = module.get_problem_html() + + # Check the rendering context + render_args,_ = test_system.render_template.call_args + context = render_args[1] + self.assertTrue("error" in context['problem']['html']) + + # Expect that the module has created a new dummy problem with the error + self.assertNotEqual(original_problem, module.lcp) From 43d8574e920568f35f6847da9173a3ed4ec615d1 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 17:26:15 -0500 Subject: [PATCH 12/13] Refactored use of test_system in xmodule unit tests so that a new one is created for each test --- common/lib/xmodule/xmodule/tests/__init__.py | 44 ++++++++++++------- .../xmodule/xmodule/tests/test_capa_module.py | 32 ++++---------- .../xmodule/tests/test_combined_open_ended.py | 39 +++++++++------- .../xmodule/xmodule/tests/test_conditional.py | 9 ++-- .../xmodule/tests/test_self_assessment.py | 14 +++--- 5 files changed, 76 insertions(+), 62 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 220f122e7a..43c2bbe24d 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -28,21 +28,35 @@ open_ended_grading_interface = { 'grading_controller' : 'grading_controller' } -test_system = ModuleSystem( - ajax_url='courses/course_id/modx/a_location', - track_function=Mock(), - get_module=Mock(), - # "render" to just the context... - render_template=lambda template, context: str(context), - replace_urls=lambda html: str(html), - user=Mock(is_staff=False), - filestore=Mock(), - debug=True, - xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10}, - node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), - anonymous_student_id='student', - open_ended_grading_interface= open_ended_grading_interface -) + +def test_system(): + """ + Construct a test ModuleSystem instance. + + By default, the render_template() method simply returns + the context it is passed as a string. + You can override this behavior by monkey patching: + + system = test_system() + system.render_template = my_render_func + + where my_render_func is a function of the form + my_render_func(template, context) + """ + return ModuleSystem( + ajax_url='courses/course_id/modx/a_location', + track_function=Mock(), + get_module=Mock(), + render_template=lambda template, context: str(context), + replace_urls=lambda html: str(html), + user=Mock(is_staff=False), + filestore=Mock(), + debug=True, + xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10}, + node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), + anonymous_student_id='student', + open_ended_grading_interface= open_ended_grading_interface + ) class ModelsTest(unittest.TestCase): diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index a42143da82..0e64e740fd 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -108,7 +108,7 @@ class CapaFactory(object): else: instance_state = None - module = CapaModule(test_system, location, + module = CapaModule(test_system(), location, definition, descriptor, instance_state, None, metadata=metadata) @@ -794,14 +794,8 @@ class CapaModuleTest(unittest.TestCase): module.should_show_reset_button = Mock(return_value=show_reset_button) module.should_show_save_button = Mock(return_value=show_save_button) - # Mock the system rendering function (reset when we're done) - old_render_func = test_system.render_template - test_system.render_template = Mock(return_value="
Test Template HTML
") - - def cleanup_func(): - test_system.render_template = old_render_func - - self.addCleanup(cleanup_func) + # Mock the system rendering function + module.system.render_template = Mock(return_value="
Test Template HTML
") # Patch the capa problem's HTML rendering with patch('capa.capa_problem.LoncapaProblem.get_html') as mock_html: @@ -817,7 +811,7 @@ class CapaModuleTest(unittest.TestCase): self.assertEqual(html, "
Test Template HTML
") # Check the rendering context - render_args,_ = test_system.render_template.call_args + render_args,_ = module.system.render_template.call_args self.assertEqual(len(render_args), 2) template_name = render_args[0] @@ -848,25 +842,17 @@ class CapaModuleTest(unittest.TestCase): # is asked to render itself as HTML module.lcp.get_html = Mock(side_effect=Exception("Test")) - # Stub out the test_system rendering function temporarily - old_render_func = test_system.render_template - test_system.render_template = Mock(return_value="
Test Template HTML
") + # Stub out the test_system rendering function + module.system.render_template = Mock(return_value="
Test Template HTML
") - # Turn off DEBUG temporarily - old_debug = test_system.DEBUG - test_system.DEBUG = False - - def cleanup_func(): - test_system.render_template = old_render_func - test_system.DEBUG = old_debug - - self.addCleanup(cleanup_func) + # Turn off DEBUG + module.system.DEBUG = False # Try to render the module with DEBUG turned off html = module.get_problem_html() # Check the rendering context - render_args,_ = test_system.render_template.call_args + render_args,_ = module.system.render_template.call_args context = render_args[1] self.assertTrue("error" in context['problem']['html']) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 5f6496f823..a524ac2fd9 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -54,7 +54,8 @@ class OpenEndedChildTest(unittest.TestCase): descriptor = Mock() def setUp(self): - self.openendedchild = OpenEndedChild(test_system, self.location, + self.test_system = test_system() + self.openendedchild = OpenEndedChild(self.test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) @@ -69,7 +70,7 @@ class OpenEndedChildTest(unittest.TestCase): def test_latest_post_assessment_empty(self): - answer = self.openendedchild.latest_post_assessment(test_system) + answer = self.openendedchild.latest_post_assessment(self.test_system) self.assertEqual(answer, "") @@ -106,7 +107,7 @@ class OpenEndedChildTest(unittest.TestCase): post_assessment = "Post assessment" self.openendedchild.record_latest_post_assessment(post_assessment) self.assertEqual(post_assessment, - self.openendedchild.latest_post_assessment(test_system)) + self.openendedchild.latest_post_assessment(self.test_system)) def test_get_score(self): new_answer = "New Answer" @@ -125,7 +126,7 @@ class OpenEndedChildTest(unittest.TestCase): def test_reset(self): - self.openendedchild.reset(test_system) + self.openendedchild.reset(self.test_system) state = json.loads(self.openendedchild.get_instance_state()) self.assertEqual(state['state'], OpenEndedChild.INITIAL) @@ -182,11 +183,13 @@ class OpenEndedModuleTest(unittest.TestCase): descriptor = Mock() def setUp(self): - test_system.location = self.location + self.test_system = test_system() + + self.test_system.location = self.location self.mock_xqueue = MagicMock() self.mock_xqueue.send_to_queue.return_value = (None, "Message") - test_system.xqueue = {'interface': self.mock_xqueue, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 1} - self.openendedmodule = OpenEndedModule(test_system, self.location, + self.test_system.xqueue = {'interface': self.mock_xqueue, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 1} + self.openendedmodule = OpenEndedModule(self.test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) def test_message_post(self): @@ -195,7 +198,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'grader_id': '1', 'score': 3} qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) - student_info = {'anonymous_student_id': test_system.anonymous_student_id, + student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, 'submission_time': qtime} contents = { 'feedback': get['feedback'], @@ -205,7 +208,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'student_info': json.dumps(student_info) } - result = self.openendedmodule.message_post(get, test_system) + result = self.openendedmodule.message_post(get, self.test_system) self.assertTrue(result['success']) # make sure it's actually sending something we want to the queue self.mock_xqueue.send_to_queue.assert_called_with(body=json.dumps(contents), header=ANY) @@ -216,7 +219,7 @@ class OpenEndedModuleTest(unittest.TestCase): def test_send_to_grader(self): submission = "This is a student submission" qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) - student_info = {'anonymous_student_id': test_system.anonymous_student_id, + student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, 'submission_time': qtime} contents = self.openendedmodule.payload.copy() contents.update({ @@ -224,7 +227,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'student_response': submission, 'max_score': self.max_score }) - result = self.openendedmodule.send_to_grader(submission, test_system) + result = self.openendedmodule.send_to_grader(submission, self.test_system) self.assertTrue(result) self.mock_xqueue.send_to_queue.assert_called_with(body=json.dumps(contents), header=ANY) @@ -238,7 +241,7 @@ class OpenEndedModuleTest(unittest.TestCase): } get = {'queuekey': "abcd", 'xqueue_body': score_msg} - self.openendedmodule.update_score(get, test_system) + self.openendedmodule.update_score(get, self.test_system) def update_score_single(self): self.openendedmodule.new_history_entry("New Entry") @@ -261,11 +264,11 @@ class OpenEndedModuleTest(unittest.TestCase): } get = {'queuekey': "abcd", 'xqueue_body': json.dumps(score_msg)} - self.openendedmodule.update_score(get, test_system) + self.openendedmodule.update_score(get, self.test_system) def test_latest_post_assessment(self): self.update_score_single() - assessment = self.openendedmodule.latest_post_assessment(test_system) + assessment = self.openendedmodule.latest_post_assessment(self.test_system) self.assertFalse(assessment == '') # check for errors self.assertFalse('errors' in assessment) @@ -336,7 +339,13 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): descriptor = Mock() def setUp(self): - self.combinedoe = CombinedOpenEndedV1Module(test_system, self.location, self.definition, self.descriptor, static_data = self.static_data, metadata=self.metadata) + self.test_system = test_system() + self.combinedoe = CombinedOpenEndedV1Module(self.test_system, + self.location, + self.definition, + self.descriptor, + static_data = self.static_data, + metadata=self.metadata) def test_get_tag_name(self): name = self.combinedoe.get_tag_name("Tag") diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 361a6ea785..16bd222b9e 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -56,6 +56,9 @@ class ConditionalModuleTest(unittest.TestCase): '''Get a dummy system''' return DummySystem(load_error_modules) + def setUp(self): + self.test_system = test_system() + def get_course(self, name): """Get a test course by directory name. If there's more than one, error.""" print "Importing {0}".format(name) @@ -85,14 +88,14 @@ class ConditionalModuleTest(unittest.TestCase): location = descriptor.location instance_state = instance_states.get(location.category, None) print "inner_get_module, location=%s, inst_state=%s" % (location, instance_state) - return descriptor.xmodule_constructor(test_system)(instance_state, shared_state) + return descriptor.xmodule_constructor(self.test_system)(instance_state, shared_state) location = Location(["i4x", "edX", "cond_test", "conditional", "condone"]) def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None): return text - test_system.replace_urls = replace_urls - test_system.get_module = inner_get_module + self.test_system.replace_urls = replace_urls + self.test_system.get_module = inner_get_module module = inner_get_module(location) print "module: ", module diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index b9c3076b7c..362b73df67 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -53,13 +53,13 @@ class SelfAssessmentTest(unittest.TestCase): 'skip_basic_checks' : False, } - self.module = SelfAssessmentModule(test_system, self.location, + self.module = SelfAssessmentModule(test_system(), self.location, self.definition, self.descriptor, static_data, state, metadata=self.metadata) def test_get_html(self): - html = self.module.get_html(test_system) + html = self.module.get_html(self.module.system) self.assertTrue("This is sample prompt text" in html) def test_self_assessment_flow(self): @@ -82,10 +82,11 @@ class SelfAssessmentTest(unittest.TestCase): self.assertEqual(self.module.get_score()['score'], 0) - self.module.save_answer({'student_answer': "I am an answer"}, test_system) + self.module.save_answer({'student_answer': "I am an answer"}, + self.module.system) self.assertEqual(self.module.state, self.module.ASSESSING) - self.module.save_assessment(mock_query_dict, test_system) + self.module.save_assessment(mock_query_dict, self.module.system) self.assertEqual(self.module.state, self.module.DONE) @@ -94,7 +95,8 @@ class SelfAssessmentTest(unittest.TestCase): self.assertEqual(self.module.state, self.module.INITIAL) # if we now assess as right, skip the REQUEST_HINT state - self.module.save_answer({'student_answer': 'answer 4'}, test_system) + self.module.save_answer({'student_answer': 'answer 4'}, + self.module.system) responses['assessment'] = '1' - self.module.save_assessment(mock_query_dict, test_system) + self.module.save_assessment(mock_query_dict, self.module.system) self.assertEqual(self.module.state, self.module.DONE) From c7d80a91eeff6875ba48096fa4cb92b06a3ebe40 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 5 Mar 2013 17:30:38 -0500 Subject: [PATCH 13/13] Changed name of survey_question to is_survey_question for clarity --- common/lib/xmodule/xmodule/capa_module.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index b42b7e4ccc..9ae6583c50 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -279,7 +279,7 @@ class CapaModule(XModule): """ Return True/False to indicate whether to show the "Reset" button. """ - survey_question = (self.max_attempts == 0) + is_survey_question = (self.max_attempts == 0) if self.rerandomize in ["always", "onreset"]: @@ -287,7 +287,7 @@ class CapaModule(XModule): # then do NOT show the reset button. # If the problem hasn't been submitted yet, then do NOT show # the reset button. - if (self.closed() and not survey_question) or not self.is_completed(): + if (self.closed() and not is_survey_question) or not self.is_completed(): return False else: return True @@ -307,14 +307,14 @@ class CapaModule(XModule): if self.force_save_button == "true": return not self.closed() else: - survey_question = (self.max_attempts == 0) + is_survey_question = (self.max_attempts == 0) needs_reset = self.is_completed() and self.rerandomize == "always" # If the problem is closed (and not a survey question with max_attempts==0), # then do NOT show the reset button # If we're waiting for the user to reset a randomized problem # then do NOT show the reset button - if (self.closed() and not survey_question) or needs_reset: + if (self.closed() and not is_survey_question) or needs_reset: return False else: return True