From b6a6e10bb55930c071160fbc27eadc5fda6e9ba8 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 6 Mar 2013 20:16:24 -0500 Subject: [PATCH] Fix max_attempts='' That's what studio defaults to, and recent changes made it break. Added a few tests to make sure it doesn't happen again. --- .../lib/capa/capa/tests/test_html_render.py | 42 ++++++++++++------- common/lib/xmodule/xmodule/capa_module.py | 4 +- .../xmodule/xmodule/tests/test_capa_module.py | 34 +++++++++------ 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index e4c54edca0..ca2a3c2e2c 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -11,6 +11,20 @@ from . import test_system class CapaHtmlRenderTest(unittest.TestCase): + def test_blank_problem(self): + """ + It's important that blank problems don't break, since that's + what you start with in studio. + """ + xml_str = " " + + # Create the problem + problem = LoncapaProblem(xml_str, '1', system=test_system) + + # Render the HTML + rendered_html = etree.XML(problem.get_html()) + # expect that we made it here without blowing up + def test_include_html(self): # Create a test file to include self._create_test_file('test_include.xml', @@ -25,7 +39,7 @@ class CapaHtmlRenderTest(unittest.TestCase): # Create the problem problem = LoncapaProblem(xml_str, '1', system=test_system) - + # Render the HTML rendered_html = etree.XML(problem.get_html()) @@ -45,7 +59,7 @@ class CapaHtmlRenderTest(unittest.TestCase): # Create the problem problem = LoncapaProblem(xml_str, '1', system=test_system) - + # Render the HTML rendered_html = etree.XML(problem.get_html()) @@ -64,7 +78,7 @@ class CapaHtmlRenderTest(unittest.TestCase): # Create the problem problem = LoncapaProblem(xml_str, '1', system=test_system) - + # Render the HTML rendered_html = etree.XML(problem.get_html()) @@ -99,11 +113,11 @@ class CapaHtmlRenderTest(unittest.TestCase): response_element = rendered_html.find("span") self.assertEqual(response_element.tag, "span") - # Expect that the response + # Expect that the response # that contains a
for the textline textline_element = response_element.find("div") self.assertEqual(textline_element.text, 'Input Template Render') - + # Expect a child
for the solution # with the rendered template solution_element = rendered_html.find("div") @@ -112,14 +126,14 @@ class CapaHtmlRenderTest(unittest.TestCase): # Expect that the template renderer was called with the correct # arguments, once for the textline input and once for # the solution - expected_textline_context = {'status': 'unsubmitted', - 'value': '', - 'preprocessor': None, - 'msg': '', - 'inline': False, - 'hidden': False, - 'do_math': False, - 'id': '1_2_1', + expected_textline_context = {'status': 'unsubmitted', + 'value': '', + 'preprocessor': None, + 'msg': '', + 'inline': False, + 'hidden': False, + 'do_math': False, + 'id': '1_2_1', 'size': None} expected_solution_context = {'id': '1_solution_1'} @@ -148,7 +162,7 @@ class CapaHtmlRenderTest(unittest.TestCase): # Create the problem and render the html problem = LoncapaProblem(xml_str, '1', system=test_system) - + # Grade the problem correctmap = problem.grade_answers({'1_2_1': 'test'}) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 7ab7b60239..b0d3950f06 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -135,8 +135,8 @@ class CapaModule(XModule): self.grace_period = None self.close_date = self.display_due_date - max_attempts = self.metadata.get('attempts', None) - if max_attempts is not None: + max_attempts = self.metadata.get('attempts') + if max_attempts is not None and max_attempts != '': self.max_attempts = int(max_attempts) else: self.max_attempts = None diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index a1e3d31d76..6330511fc5 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -44,7 +44,7 @@ class CapaFactory(object): @staticmethod def answer_key(): """ Return the key stored in the capa problem answer dict """ - return ("-".join(['i4x', 'edX', 'capa_test', 'problem', + return ("-".join(['i4x', 'edX', 'capa_test', 'problem', 'SampleProblem%d' % CapaFactory.num]) + "_2_1") @@ -144,6 +144,8 @@ class CapaModuleTest(unittest.TestCase): "Factory should be creating unique names for each problem") + + def test_correct(self): """ Check that the factory creates correct and incorrect problems properly. @@ -332,7 +334,7 @@ class CapaModuleTest(unittest.TestCase): '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 @@ -475,7 +477,7 @@ class CapaModuleTest(unittest.TestCase): 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) @@ -506,7 +508,7 @@ class CapaModuleTest(unittest.TestCase): def test_reset_problem(self): module = CapaFactory.create() - # Mock the module's capa problem + # Mock the module's capa problem # to simulate that the problem is done mock_problem = MagicMock(capa.capa_problem.LoncapaProblem) mock_problem.done = True @@ -668,7 +670,7 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(max_attempts=0) self.assertFalse(module.should_show_check_button()) - # If user submitted a problem but hasn't reset, + # 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") @@ -707,7 +709,7 @@ class CapaModuleTest(unittest.TestCase): module.lcp.done = True self.assertFalse(module.should_show_reset_button()) - # If the user hasn't submitted an answer yet, + # If the user hasn't submitted an answer yet, # then do NOT show the reset button module = CapaFactory.create() module.lcp.done = False @@ -770,7 +772,7 @@ class CapaModuleTest(unittest.TestCase): # If the user is out of attempts, do NOT show the save button attempts = random.randint(1,10) - module = CapaFactory.create(attempts=attempts, + module = CapaFactory.create(attempts=attempts, max_attempts=attempts, force_save_button="true") module.lcp.done = True @@ -784,6 +786,12 @@ class CapaModuleTest(unittest.TestCase): module.lcp.done = True self.assertTrue(module.should_show_save_button()) + def test_no_max_attempts(self): + module = CapaFactory.create(max_attempts='') + html = module.get_problem_html() + # assert that we got here without exploding + + def test_get_problem_html(self): module = CapaFactory.create() @@ -797,7 +805,7 @@ 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 + # Mock the system rendering function module.system.render_template = Mock(return_value="
Test Template HTML
") # Patch the capa problem's HTML rendering @@ -809,7 +817,7 @@ class CapaModuleTest(unittest.TestCase): # 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
") @@ -831,7 +839,7 @@ class CapaModuleTest(unittest.TestCase): 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. @@ -845,10 +853,10 @@ 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 + # Stub out the test_system rendering function module.system.render_template = Mock(return_value="
Test Template HTML
") - # Turn off DEBUG + # Turn off DEBUG module.system.DEBUG = False # Try to render the module with DEBUG turned off @@ -860,4 +868,4 @@ class CapaModuleTest(unittest.TestCase): 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) + self.assertNotEqual(original_problem, module.lcp)