From e41172d55df9f1a0cb142b6a59625eef59dfa519 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sun, 20 Jan 2013 11:50:51 -0500 Subject: [PATCH 1/5] Add start of test framework for capa --- .../xmodule/xmodule/tests/test_capa_module.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 common/lib/xmodule/xmodule/tests/test_capa_module.py diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py new file mode 100644 index 0000000000..148fd893ff --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -0,0 +1,60 @@ +import json +from mock import Mock +import unittest + +from xmodule.capa_module import CapaModule +from xmodule.modulestore import Location +from lxml import etree + +from . import test_system + +class CapaFactory(object): + """ + A helper class to create problem modules with various parameters for testing. + """ + + sample_problem_xml = """ + + +

What is pi, to two decimal placs?

+ + + + + +""" + + num = 0 + @staticmethod + def next_num(): + CapaFactory.num += 1 + return CapaFactory.num + + @staticmethod + def create(): + definition = {'data': CapaFactory.sample_problem_xml,} + location = Location(["i4x", "edX", "capa_test", "problem", + "SampleProblem{0}".format(CapaFactory.next_num())]) + metadata = {} + descriptor = Mock(weight="1") + instance_state = None + + module = CapaModule(test_system, location, + definition, descriptor, + instance_state, None, metadata=metadata) + + return module + + + +class CapaModuleTest(unittest.TestCase): + + def test_import(self): + module = CapaFactory.create() + self.assertEqual(module.get_score()['score'], 0) + + other_module = CapaFactory.create() + self.assertEqual(module.get_score()['score'], 0) + self.assertNotEqual(module.url_name, other_module.url_name, + "Factory should be creating unique names for each problem") + From 025b074b87b5fc60c712292d541449d0d470152b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sun, 20 Jan 2013 12:17:22 -0500 Subject: [PATCH 2/5] Add simple test for showanswer, fix test_system --- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../xmodule/xmodule/tests/test_capa_module.py | 60 ++++++++++++++++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index a07f1ddfaf..1f323834a9 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -26,7 +26,7 @@ test_system = ModuleSystem( # "render" to just the context... render_template=lambda template, context: str(context), replace_urls=Mock(), - user=Mock(), + user=Mock(is_staff=False), filestore=Mock(), debug=True, xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 10}, diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 148fd893ff..7537cb537c 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1,7 +1,9 @@ import json from mock import Mock +from pprint import pprint import unittest + from xmodule.capa_module import CapaModule from xmodule.modulestore import Location from lxml import etree @@ -31,13 +33,59 @@ class CapaFactory(object): return CapaFactory.num @staticmethod - def create(): + def create(graceperiod=None, + due=None, + max_attempts=None, + showanswer=None, + rerandomize=None, + force_save_button=None, + attempts=None, + problem_state=None, + ): + """ + All parameters are optional, and are added to the created problem if specified. + + Arguments: + graceperiod: + due: + max_attempts: + showanswer: + force_save_button: + rerandomize: all strings, as specified in the policy for the problem + + problem_state: a dict to to be serialized into the instance_state of the + module. + + attempts: also added to instance state. Should be a number. + """ definition = {'data': CapaFactory.sample_problem_xml,} location = Location(["i4x", "edX", "capa_test", "problem", "SampleProblem{0}".format(CapaFactory.next_num())]) metadata = {} + if graceperiod is not None: + metadata['graceperiod'] = graceperiod + if due is not None: + metadata['due'] = due + if max_attempts is not None: + metadata['attempts'] = max_attempts + if showanswer is not None: + metadata['showanswer'] = showanswer + if force_save_button is not None: + metadata['force_save_button'] = force_save_button + if rerandomize is not None: + metadata['rerandomize'] = rerandomize + + descriptor = Mock(weight="1") - instance_state = None + instance_state_dict = {} + if problem_state is not None: + instance_state_dict = problem_state + if attempts is not None: + instance_state_dict['attempts'] = attempts + if len(instance_state_dict) > 0: + instance_state = json.dumps(instance_state_dict) + else: + instance_state = None module = CapaModule(test_system, location, definition, descriptor, @@ -58,3 +106,11 @@ class CapaModuleTest(unittest.TestCase): self.assertNotEqual(module.url_name, other_module.url_name, "Factory should be creating unique names for each problem") + def test_showanswer(self): + """ + Make sure the show answer logic does the right thing. + """ + # default, no due date, showanswer 'closed' + problem = CapaFactory.create() + pprint(problem.__dict__) + self.assertFalse(problem.answer_available()) From ea091a6eb83b09fbc5bafbe4f0f5011b69c8db7b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sun, 20 Jan 2013 12:49:05 -0500 Subject: [PATCH 3/5] Add tests for showanswer --- .../xmodule/xmodule/tests/test_capa_module.py | 68 +++++++++++++++++-- 1 file changed, 62 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 7537cb537c..506c7faf9f 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1,9 +1,9 @@ +import datetime import json from mock import Mock from pprint import pprint import unittest - from xmodule.capa_module import CapaModule from xmodule.modulestore import Location from lxml import etree @@ -56,7 +56,7 @@ class CapaFactory(object): problem_state: a dict to to be serialized into the instance_state of the module. - attempts: also added to instance state. Should be a number. + attempts: also added to instance state. Will be converted to an int. """ definition = {'data': CapaFactory.sample_problem_xml,} location = Location(["i4x", "edX", "capa_test", "problem", @@ -81,7 +81,9 @@ class CapaFactory(object): if problem_state is not None: instance_state_dict = problem_state if attempts is not None: - instance_state_dict['attempts'] = attempts + # converting to int here because I keep putting "0" and "1" in the tests + # since everything else is a string. + instance_state_dict['attempts'] = int(attempts) if len(instance_state_dict) > 0: instance_state = json.dumps(instance_state_dict) else: @@ -97,6 +99,17 @@ class CapaFactory(object): class CapaModuleTest(unittest.TestCase): + + def setUp(self): + now = datetime.datetime.now() + day_delta = datetime.timedelta(days=1) + self.yesterday_str = str(now - day_delta) + self.today_str = str(now) + self.tomorrow_str = str(now + day_delta) + + # in the capa grace period format, not in time delta format + self.two_day_delta_str = "2 days" + def test_import(self): module = CapaFactory.create() self.assertEqual(module.get_score()['score'], 0) @@ -106,11 +119,54 @@ class CapaModuleTest(unittest.TestCase): self.assertNotEqual(module.url_name, other_module.url_name, "Factory should be creating unique names for each problem") - def test_showanswer(self): + def test_showanswer_default(self): """ Make sure the show answer logic does the right thing. """ - # default, no due date, showanswer 'closed' + # default, no due date, showanswer 'closed', so problem is open, and show_answer + # not visible. problem = CapaFactory.create() - pprint(problem.__dict__) self.assertFalse(problem.answer_available()) + + + def test_showanswer_attempted(self): + problem = CapaFactory.create(showanswer='attempted') + self.assertFalse(problem.answer_available()) + problem.attempts = 1 + self.assertTrue(problem.answer_available()) + + + def test_showanswer_closed(self): + + # can see after attempts used up + used_all_attempts = CapaFactory.create(showanswer='closed', + max_attempts="1", + attempts="1") + self.assertTrue(used_all_attempts.answer_available()) + + + # can see after due date + after_due_date = CapaFactory.create(showanswer='closed', + max_attempts="1", + attempts="0", + due=self.yesterday_str) + self.assertTrue(after_due_date.answer_available()) + + # can't see because attempts left + attempts_left_open = CapaFactory.create(showanswer='closed', + max_attempts="1", + attempts="0", + due=self.tomorrow_str) + self.assertFalse(attempts_left_open.answer_available()) + + # Can't see because grace period hasn't expired + still_in_grace = CapaFactory.create(showanswer='closed', + max_attempts="1", + attempts="0", + due=self.yesterday_str, + graceperiod=self.two_day_delta_str) + self.assertFalse(still_in_grace.answer_available()) + + + + From 6088a926cc0697094c1bd6ae095581895fcc4563 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sun, 20 Jan 2013 17:35:03 -0500 Subject: [PATCH 4/5] Add showanswer="past_due" and tests --- common/lib/xmodule/xmodule/capa_module.py | 35 ++++++++------ .../xmodule/xmodule/tests/test_capa_module.py | 47 ++++++++++++++++++- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index f33da6e3a4..6d258e61ed 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -389,38 +389,43 @@ class CapaModule(XModule): }) return json.dumps(d, cls=ComplexEncoder) + def is_past_due(self): + """ + Is it now past this problem's due date, including grace period? + """ + return (self.close_date is not None and + datetime.datetime.utcnow() > self.close_date) + def closed(self): ''' Is the student still allowed to submit answers? ''' if self.attempts == self.max_attempts: return True - if self.close_date is not None and datetime.datetime.utcnow() > self.close_date: + if self.is_past_due(): return True return False def answer_available(self): - ''' Is the user allowed to see an answer? + ''' + Is the user allowed to see an answer? ''' if self.show_answer == '': return False - - if self.show_answer == "never": + elif self.show_answer == "never": return False - - # Admins can see the answer, unless the problem explicitly prevents it - if self.system.user_is_staff: + elif self.system.user_is_staff: + # This i after the 'never' check because admins can see the answer + # unless the problem explicitly prevents it return True - - if self.show_answer == 'attempted': + elif self.show_answer == 'attempted': return self.attempts > 0 - - if self.show_answer == 'answered': + elif self.show_answer == 'answered': return self.lcp.done - - if self.show_answer == 'closed': + elif self.show_answer == 'closed': return self.closed() - - if self.show_answer == 'always': + elif self.show_answer == 'past_due': + return self.is_past_due() + elif self.show_answer == 'always': return True return False diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 506c7faf9f..e8f639e3c9 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -138,10 +138,11 @@ class CapaModuleTest(unittest.TestCase): def test_showanswer_closed(self): - # can see after attempts used up + # can see after attempts used up, even with due date in the future used_all_attempts = CapaFactory.create(showanswer='closed', max_attempts="1", - attempts="1") + attempts="1", + due=self.tomorrow_str) self.assertTrue(used_all_attempts.answer_available()) @@ -152,6 +153,7 @@ class CapaModuleTest(unittest.TestCase): due=self.yesterday_str) self.assertTrue(after_due_date.answer_available()) + # can't see because attempts left attempts_left_open = CapaFactory.create(showanswer='closed', max_attempts="1", @@ -169,4 +171,45 @@ class CapaModuleTest(unittest.TestCase): + def test_showanswer_past_due(self): + """ + With showanswer="past_due" should only show answer after the problem is closed + for everyone--e.g. after due date + grace period. + """ + + # can see after attempts used up, even with due date in the future + used_all_attempts = CapaFactory.create(showanswer='past_due', + max_attempts="1", + attempts="1", + due=self.tomorrow_str) + self.assertFalse(used_all_attempts.answer_available()) + + + # can see after due date + past_due_date = CapaFactory.create(showanswer='past_due', + max_attempts="1", + attempts="0", + due=self.yesterday_str) + self.assertTrue(past_due_date.answer_available()) + + + # can't see because attempts left + attempts_left_open = CapaFactory.create(showanswer='past_due', + max_attempts="1", + attempts="0", + due=self.tomorrow_str) + self.assertFalse(attempts_left_open.answer_available()) + + # Can't see because grace period hasn't expired, even though have no more + # attempts. + still_in_grace = CapaFactory.create(showanswer='past_due', + max_attempts="1", + attempts="1", + due=self.yesterday_str, + graceperiod=self.two_day_delta_str) + self.assertFalse(still_in_grace.answer_available()) + + + + From fe42d8e9a8a2b3a3ea0c38d0b6fec5fbc785eb3f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 4 Feb 2013 15:04:20 -0500 Subject: [PATCH 5/5] fix typo, add comment --- common/lib/xmodule/xmodule/capa_module.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 6d258e61ed..41e58d5c68 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -414,12 +414,14 @@ class CapaModule(XModule): elif self.show_answer == "never": return False elif self.system.user_is_staff: - # This i after the 'never' check because admins can see the answer + # This is after the 'never' check because admins can see the answer # unless the problem explicitly prevents it return True elif self.show_answer == 'attempted': return self.attempts > 0 elif self.show_answer == 'answered': + # NOTE: this is slightly different from 'attempted' -- resetting the problems + # makes lcp.done False, but leaves attempts unchanged. return self.lcp.done elif self.show_answer == 'closed': return self.closed() @@ -674,18 +676,18 @@ class CapaDescriptor(RawDescriptor): # TODO (vshnayder): do problems have any other metadata? Do they # actually use type and points? metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points') - + def get_context(self): _context = RawDescriptor.get_context(self) _context.update({'markdown': self.metadata.get('markdown', '')}) return _context - + @property def editable_metadata_fields(self): """Remove metadata from the editable fields since it has its own editor""" subset = super(CapaDescriptor,self).editable_metadata_fields if 'markdown' in subset: - subset.remove('markdown') + subset.remove('markdown') return subset