From 6c9ad30ee1a9862fc94e78852e8332c523973bd1 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 20 Nov 2013 09:26:42 -0500
Subject: [PATCH 1/2] Update open-ended tests to load the module between
 'requests'

Under normal operation, XModules are reloaded on each request from a
student. CombinedOpenEnded modules have code that runs at initialization
that validates the students state. These changes makes that code run
during several long-form unit tests (testing CombinedOpenEnded across
multiple 'requests').

These tests are marked as expectedFailure because they now exhibit the
same failures as observed in [LMS-1493] (namely, the students state gets
reset, because CombinedOpenEnded interprets system.ajax_url raising an
error as meaning that the problem definition and the student answers are
in conflict)
---
 .../combined_open_ended_modulev1.py           |   1 -
 .../xmodule/tests/test_combined_open_ended.py | 190 ++++++++++--------
 .../xmodule/tests/test_peer_grading.py        |  34 +++-
 .../xmodule/tests/test_util_open_ended.py     |   6 +-
 4 files changed, 130 insertions(+), 101 deletions(-)

diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
index 4a1715c48d..72915eb7b3 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
@@ -93,7 +93,6 @@ class CombinedOpenEndedV1Module():
         Definition file should have one or many task blocks, a rubric block, and a prompt block.  See DEFAULT_DATA in combined_open_ended_module for a sample.
 
         """
-
         self.instance_state = instance_state
         self.display_name = instance_state.get('display_name', "Open Ended")
 
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 b72eeab66b..07e0a9183e 100644
--- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
+++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
@@ -726,76 +726,84 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
     assessment = [0, 1]
     hint = "blah"
 
-    def setUp(self):
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
-        self.test_system.xqueue['interface'] = Mock(
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        test_system.xqueue['interface'] = Mock(
             send_to_queue=Mock(side_effect=[1, "queued"])
         )
+
+        return test_system
+
+    def setUp(self):
         self.setup_modulestore(COURSE)
 
+    def _handle_ajax(self, dispatch, content):
+        # Load the module from persistence
+        module = self._module()
+
+        # Call handle_ajax on the module
+        result = module.handle_ajax(dispatch, content)
+
+        # Persist the state
+        module.save()
+
+        return result
+
+    def _module(self):
+        return self.get_module_from_location(self.problem_location, COURSE)
+
     def test_open_ended_load_and_save(self):
         """
         See if we can load the module and save an answer
         @return:
         """
-        # Load the module
-        module = self.get_module_from_location(self.problem_location, COURSE)
-
         # Try saving an answer
-        module.handle_ajax("save_answer", {"student_answer": self.answer})
-        # Save our modifications to the underlying KeyValueStore so they can be persisted
-        module.save()
-        task_one_json = json.loads(module.task_states[0])
-        self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer)
-
-        module = self.get_module_from_location(self.problem_location, COURSE)
-        task_one_json = json.loads(module.task_states[0])
+        self._handle_ajax("save_answer", {"student_answer": self.answer})
+
+        task_one_json = json.loads(self._module().task_states[0])
         self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer)
 
+    @unittest.expectedFailure
     def test_open_ended_flow_reset(self):
         """
         Test the flow of the module if we complete the self assessment step and then reset
         @return:
         """
         assessment = [0, 1]
-        module = self.get_module_from_location(self.problem_location, COURSE)
 
         # Simulate a student saving an answer
-        html = module.handle_ajax("get_html", {})
-        module.save()
-        module.handle_ajax("save_answer", {"student_answer": self.answer})
-        module.save()
-        html = module.handle_ajax("get_html", {})
-        module.save()
+        self._handle_ajax("get_html", {})
+        self._handle_ajax("save_answer", {"student_answer": self.answer})
+        self._handle_ajax("get_html", {})
 
         # Mock a student submitting an assessment
         assessment_dict = MultiDict({'assessment': sum(assessment)})
         assessment_dict.extend(('score_list[]', val) for val in assessment)
 
-        module.handle_ajax("save_assessment", assessment_dict)
-        module.save()
-        task_one_json = json.loads(module.task_states[0])
+        self._handle_ajax("save_assessment", assessment_dict)
+
+        task_one_json = json.loads(self._module().task_states[0])
         self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment)
-        rubric = module.handle_ajax("get_combined_rubric", {})
-        module.save()
+
+        self._handle_ajax("get_combined_rubric", {})
 
         # Move to the next step in the problem
-        module.handle_ajax("next_problem", {})
-        module.save()
-        self.assertEqual(module.current_task_number, 0)
+        self._handle_ajax("next_problem", {})
+        self.assertEqual(self._module().current_task_number, 0)
 
-        html = module.render('student_view').content
+        html = self._module().render('student_view').content
         self.assertIsInstance(html, basestring)
 
-        rubric = module.handle_ajax("get_combined_rubric", {})
-        module.save()
+        rubric = self._handle_ajax("get_combined_rubric", {})
         self.assertIsInstance(rubric, basestring)
-        self.assertEqual(module.state, "assessing")
-        module.handle_ajax("reset", {})
-        module.save()
-        self.assertEqual(module.current_task_number, 0)
 
+        self.assertEqual(self._module().state, "assessing")
+
+        self._handle_ajax("reset", {})
+        self.assertEqual(self._module().current_task_number, 0)
+
+    @unittest.expectedFailure
     def test_open_ended_flow_correct(self):
         """
         Test a two step problem where the student first goes through the self assessment step, and then the
@@ -803,42 +811,36 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
         @return:
         """
         assessment = [1, 1]
-        # Load the module
-        module = self.get_module_from_location(self.problem_location, COURSE)
 
         # Simulate a student saving an answer
-        module.handle_ajax("save_answer", {"student_answer": self.answer})
-        module.save()
-        status = module.handle_ajax("get_status", {})
-        module.save()
+        self._handle_ajax("save_answer", {"student_answer": self.answer})
+        status = self._handle_ajax("get_status", {})
         self.assertIsInstance(status, basestring)
 
         # Mock a student submitting an assessment
         assessment_dict = MultiDict({'assessment': sum(assessment)})
         assessment_dict.extend(('score_list[]', val) for val in assessment)
 
-        module.handle_ajax("save_assessment", assessment_dict)
-        module.save()
-        task_one_json = json.loads(module.task_states[0])
+        self._handle_ajax("save_assessment", assessment_dict)
+
+        task_one_json = json.loads(self._module().task_states[0])
         self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment)
 
         # Move to the next step in the problem
         try:
-            module.handle_ajax("next_problem", {})
-            module.save()
+            self._handle_ajax("next_problem", {})
         except GradingServiceError:
             # This error is okay.  We don't have a grading service to connect to!
             pass
-        self.assertEqual(module.current_task_number, 1)
+        self.assertEqual(self._module().current_task_number, 1)
         try:
-            module.render('student_view')
+            self._module().render('student_view')
         except GradingServiceError:
             # This error is okay.  We don't have a grading service to connect to!
             pass
 
         # Try to get the rubric from the module
-        module.handle_ajax("get_combined_rubric", {})
-        module.save()
+        self._handle_ajax("get_combined_rubric", {})
 
         # Make a fake reply from the queue
         queue_reply = {
@@ -856,29 +858,26 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
             })
         }
 
-        module.handle_ajax("check_for_score", {})
-        module.save()
+        self._handle_ajax("check_for_score", {})
 
         # Update the module with the fake queue reply
-        module.handle_ajax("score_update", queue_reply)
-        module.save()
+        self._handle_ajax("score_update", queue_reply)
+
+        module = self._module()
         self.assertFalse(module.ready_to_reset)
         self.assertEqual(module.current_task_number, 1)
 
         # Get html and other data client will request
         module.render('student_view')
 
-        module.handle_ajax("skip_post_assessment", {})
-        module.save()
+        self._handle_ajax("skip_post_assessment", {})
 
         # Get all results
-        module.handle_ajax("get_combined_rubric", {})
-        module.save()
+        self._handle_ajax("get_combined_rubric", {})
 
         # reset the problem
-        module.handle_ajax("reset", {})
-        module.save()
-        self.assertEqual(module.state, "initial")
+        self._handle_ajax("reset", {})
+        self.assertEqual(self._module().state, "initial")
 
 
 class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
@@ -890,14 +889,33 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
     assessment = [0, 1]
     hint = "blah"
 
-    def setUp(self):
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
-        self.test_system.xqueue['interface'] = Mock(
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        test_system.xqueue['interface'] = Mock(
             send_to_queue=Mock(side_effect=[1, "queued"])
         )
+        return test_system
+
+    def setUp(self):
         self.setup_modulestore(COURSE)
 
+    def _handle_ajax(self, dispatch, content):
+        # Load the module from persistence
+        module = self._module()
+
+        # Call handle_ajax on the module
+        result = module.handle_ajax(dispatch, content)
+
+        # Persist the state
+        module.save()
+
+        return result
+
+    def _module(self):
+        return self.get_module_from_location(self.problem_location, COURSE)
+
+    @unittest.expectedFailure
     def test_reset_fail(self):
         """
        Test the flow of the module if we complete the self assessment step and then reset
@@ -905,39 +923,32 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
        @return:
        """
         assessment = [0, 1]
-        module = self.get_module_from_location(self.problem_location, COURSE)
-        module.save()
 
         # Simulate a student saving an answer
-        module.handle_ajax("save_answer", {"student_answer": self.answer})
-        module.save()
+        self._handle_ajax("save_answer", {"student_answer": self.answer})
 
         # Mock a student submitting an assessment
         assessment_dict = MultiDict({'assessment': sum(assessment)})
         assessment_dict.extend(('score_list[]', val) for val in assessment)
 
-        module.handle_ajax("save_assessment", assessment_dict)
-        module.save()
-        task_one_json = json.loads(module.task_states[0])
+        self._handle_ajax("save_assessment", assessment_dict)
+        task_one_json = json.loads(self._module().task_states[0])
         self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment)
 
         # Move to the next step in the problem
-        module.handle_ajax("next_problem", {})
-        module.save()
-        self.assertEqual(module.current_task_number, 0)
+        self._handle_ajax("next_problem", {})
+        self.assertEqual(self._module().current_task_number, 0)
 
-        html = module.render('student_view').content
+        html = self._module().render('student_view').content
         self.assertIsInstance(html, basestring)
 
         # Module should now be done
-        rubric = module.handle_ajax("get_combined_rubric", {})
-        module.save()
+        rubric = self._handle_ajax("get_combined_rubric", {})
         self.assertIsInstance(rubric, basestring)
-        self.assertEqual(module.state, "done")
+        self.assertEqual(self._module().state, "done")
 
         # Try to reset, should fail because only 1 attempt is allowed
-        reset_data = json.loads(module.handle_ajax("reset", {}))
-        module.save()
+        reset_data = json.loads(self._handle_ajax("reset", {}))
         self.assertEqual(reset_data['success'], False)
 
 class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore):
@@ -951,13 +962,16 @@ class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore):
     answer_link = "http://www.edx.org"
     autolink_tag = "<a href="
 
-    def setUp(self):
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
-        self.test_system.s3_interface = test_util_open_ended.S3_INTERFACE
-        self.test_system.xqueue['interface'] = Mock(
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        test_system.s3_interface = test_util_open_ended.S3_INTERFACE
+        test_system.xqueue['interface'] = Mock(
             send_to_queue=Mock(side_effect=[1, "queued"])
         )
+        return test_system
+
+    def setUp(self):
         self.setup_modulestore(COURSE)
 
     def test_file_upload_fail(self):
diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
index c6d08b2026..e7f820d7f0 100644
--- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py
+++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
@@ -41,13 +41,16 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
     })
     save_dict.extend(('rubric_scores[]', val) for val in (0, 1))
 
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        return test_system
+
     def setUp(self):
         """
         Create a peer grading module from a test system
         @return:
         """
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
         self.setup_modulestore(COURSE)
         self.peer_grading = self.get_module_from_location(self.problem_location, COURSE)
         self.coe = self.get_module_from_location(self.coe_location, COURSE)
@@ -173,13 +176,16 @@ class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
         ["i4x", "edX", "open_ended", "peergrading", "PeerGradingScored"]
     )
 
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        return test_system
+
     def setUp(self):
         """
         Create a peer grading module from a test system
         @return:
         """
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
         self.setup_modulestore(COURSE)
 
     def test_metadata_load(self):
@@ -213,12 +219,15 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
     coe_location = Location(["i4x", "edX", "open_ended", "combinedopenended",
                              "SampleQuestion"])
 
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        return test_system
+
     def setUp(self):
         """
         Create a peer grading module from a test system.
         """
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
         self.setup_modulestore(COURSE)
 
     @property
@@ -270,14 +279,16 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
         else:
             pg_descriptor.get_required_module_descriptors = lambda: []
 
+        test_system = self.get_module_system(pg_descriptor)
+
         # Initialize the peer grading module.
         peer_grading = PeerGradingModule(
             pg_descriptor,
-            self.test_system,
+            test_system,
             self.field_data,
             self.scope_ids,
         )
-        self.test_system.xmodule_instance = peer_grading
+        test_system.xmodule_instance = peer_grading
 
         return peer_grading
 
@@ -384,13 +395,16 @@ class PeerGradingModuleTrackChangesTest(unittest.TestCase, DummyModulestore):
     mock_track_changes_problem = Mock(side_effect=[MockedTrackChangesProblem()])
     pgm_location = Location(["i4x", "edX", "open_ended", "peergrading", "PeerGradingSample"])
 
+    def get_module_system(self, descriptor):
+        test_system = get_test_system()
+        test_system.open_ended_grading_interface = None
+        return test_system
+
     def setUp(self):
         """
         Create a peer grading module from a test system
         @return:
         """
-        self.test_system = get_test_system()
-        self.test_system.open_ended_grading_interface = None
         self.setup_modulestore(COURSE)
         self.peer_grading = self.get_module_from_location(self.pgm_location, COURSE)
 
diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py
index 939152251a..bbb0653512 100644
--- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py
+++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py
@@ -78,7 +78,9 @@ class DummyModulestore(object):
     """
     A mixin that allows test classes to have convenience functions to get a module given a location
     """
-    get_test_system = get_test_system()
+
+    def get_module_system(self, descriptor):
+        raise NotImplementedError("Sub-tests must specify how to generate a module-system")
 
     def setup_modulestore(self, name):
         self.modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name])
@@ -93,7 +95,7 @@ class DummyModulestore(object):
         if not isinstance(location, Location):
             location = Location(location)
         descriptor = self.modulestore.get_instance(course.id, location, depth=None)
-        descriptor.xmodule_runtime = self.test_system
+        descriptor.xmodule_runtime = self.get_module_system(descriptor)
         return descriptor
 
 # Task state for a module with self assessment then instructor assessment.

From b4a1840344bbeeff095ef4a214c6d693831bb408 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 20 Nov 2013 09:36:25 -0500
Subject: [PATCH 2/2] Move xmodule_runtime.xmodule_instance registration
 earlier

This allows XModules (specifically CombinedOpenEnded) to use ajax_url
during their init functions (which would, before, have thrown an
exception).

[LMS-1493]
---
 common/lib/xmodule/xmodule/abtest_module.py                  | 2 +-
 common/lib/xmodule/xmodule/annotatable_module.py             | 2 +-
 common/lib/xmodule/xmodule/capa_module.py                    | 2 +-
 common/lib/xmodule/xmodule/combined_open_ended_module.py     | 2 +-
 common/lib/xmodule/xmodule/crowdsource_hinter.py             | 2 +-
 common/lib/xmodule/xmodule/foldit_module.py                  | 2 +-
 common/lib/xmodule/xmodule/randomize_module.py               | 2 +-
 common/lib/xmodule/xmodule/seq_module.py                     | 2 +-
 common/lib/xmodule/xmodule/tests/test_capa_module.py         | 1 -
 common/lib/xmodule/xmodule/tests/test_combined_open_ended.py | 5 +----
 common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py  | 1 -
 common/lib/xmodule/xmodule/tests/test_peer_grading.py        | 1 -
 common/lib/xmodule/xmodule/timelimit_module.py               | 3 ---
 common/lib/xmodule/xmodule/vertical_module.py                | 3 ---
 common/lib/xmodule/xmodule/x_module.py                       | 4 +++-
 15 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py
index c0a53d048f..06d4e0b2d2 100644
--- a/common/lib/xmodule/xmodule/abtest_module.py
+++ b/common/lib/xmodule/xmodule/abtest_module.py
@@ -45,7 +45,7 @@ class ABTestModule(ABTestFields, XModule):
     """
 
     def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
+        super(ABTestModule, self).__init__(*args, **kwargs)
 
         if self.group is None:
             self.group = group_from_value(
diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py
index ca85065577..fbc175b5b9 100644
--- a/common/lib/xmodule/xmodule/annotatable_module.py
+++ b/common/lib/xmodule/xmodule/annotatable_module.py
@@ -50,7 +50,7 @@ class AnnotatableModule(AnnotatableFields, XModule):
     icon_class = 'annotatable'
 
     def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
+        super(AnnotatableModule, self).__init__(*args, **kwargs)
 
         xmltree = etree.fromstring(self.data)
 
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 6d664acbf6..cf6c2e3dce 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -190,7 +190,7 @@ class CapaModule(CapaFields, XModule):
         """
         Accepts the same arguments as xmodule.x_module:XModule.__init__
         """
-        XModule.__init__(self, *args, **kwargs)
+        super(CapaModule, self).__init__(*args, **kwargs)
 
         due_date = self.due
 
diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py
index 6dc2ac045b..68a0d65617 100644
--- a/common/lib/xmodule/xmodule/combined_open_ended_module.py
+++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py
@@ -412,7 +412,7 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
         See DEFAULT_DATA for a sample.
 
         """
-        XModule.__init__(self, *args, **kwargs)
+        super(CombinedOpenEndedModule, self).__init__(*args, **kwargs)
 
         self.system.set('location', self.location)
 
diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py
index 62bfe5b586..5a1091f6fb 100644
--- a/common/lib/xmodule/xmodule/crowdsource_hinter.py
+++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py
@@ -75,7 +75,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
     js_module_name = "Hinter"
 
     def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
+        super(CrowdsourceHinterModule, self).__init__(*args, **kwargs)
         # We need to know whether we are working with a FormulaResponse problem.
         try:
             responder = self.get_display_items()[0].lcp.responders.values()[0]
diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py
index e1714ff96b..655ff1911a 100644
--- a/common/lib/xmodule/xmodule/foldit_module.py
+++ b/common/lib/xmodule/xmodule/foldit_module.py
@@ -39,7 +39,7 @@ class FolditModule(FolditFields, XModule):
             required_sublevel_half_credit="3"
             show_leaderboard="false"/>
         """
-        XModule.__init__(self, *args, **kwargs)
+        super(FolditModule, self).__init__(*args, **kwargs)
         self.due_time = self.due
 
     def is_complete(self):
diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py
index 00baf3f140..71d23012d1 100644
--- a/common/lib/xmodule/xmodule/randomize_module.py
+++ b/common/lib/xmodule/xmodule/randomize_module.py
@@ -39,7 +39,7 @@ class RandomizeModule(RandomizeFields, XModule):
         modules.
 """
     def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
+        super(RandomizeModule, self).__init__(*args, **kwargs)
 
         # NOTE: calling self.get_children() creates a circular reference--
         # it calls get_child_descriptors() internally, but that doesn't work until
diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py
index 291d7a1ea1..62e93cb90e 100644
--- a/common/lib/xmodule/xmodule/seq_module.py
+++ b/common/lib/xmodule/xmodule/seq_module.py
@@ -38,7 +38,7 @@ class SequenceModule(SequenceFields, XModule):
 
 
     def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
+        super(SequenceModule, self).__init__(*args, **kwargs)
 
         # if position is specified in system, then use that instead
         if getattr(self.system, 'position', None) is not None:
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index 889376ba42..73d2eb111f 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -133,7 +133,6 @@ class CapaFactory(object):
             DictFieldData(field_data),
             ScopeIds(None, None, location, location),
         )
-        system.xmodule_instance = module
 
         if correct:
             # TODO: probably better to actually set the internal state properly, but...
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 07e0a9183e..e1b2a4ebe2 100644
--- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
+++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py
@@ -664,7 +664,6 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
                                                static_data=self.static_data,
                                                metadata=self.metadata,
                                                instance_state=instance_state)
-        self.test_system.xmodule_instance = module
         return combinedoe
 
     def ai_state_reset(self, task_state, task_number=None):
@@ -717,6 +716,7 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
     def test_state_pe_single(self):
         self.ai_state_success(TEST_STATE_PE_SINGLE, iscore=0, tasks=[self.task_xml2])
 
+
 class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
     """
     Test the student flow in the combined open ended xmodule
@@ -764,7 +764,6 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
         task_one_json = json.loads(self._module().task_states[0])
         self.assertEqual(task_one_json['child_history'][0]['answer'], self.answer)
 
-    @unittest.expectedFailure
     def test_open_ended_flow_reset(self):
         """
         Test the flow of the module if we complete the self assessment step and then reset
@@ -803,7 +802,6 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
         self._handle_ajax("reset", {})
         self.assertEqual(self._module().current_task_number, 0)
 
-    @unittest.expectedFailure
     def test_open_ended_flow_correct(self):
         """
         Test a two step problem where the student first goes through the self assessment step, and then the
@@ -915,7 +913,6 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
     def _module(self):
         return self.get_module_from_location(self.problem_location, COURSE)
 
-    @unittest.expectedFailure
     def test_reset_fail(self):
         """
        Test the flow of the module if we complete the self assessment step and then reset
diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py
index d73c79035d..33fc264ff9 100644
--- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py
+++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py
@@ -143,7 +143,6 @@ class CHModuleFactory(object):
             return capa_module
         system.get_module = fake_get_module
         module = CrowdsourceHinterModule(descriptor, system, DictFieldData(field_data), Mock())
-        system.xmodule_instance = module
 
         return module
 
diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
index e7f820d7f0..66929c19d6 100644
--- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py
+++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
@@ -288,7 +288,6 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
             self.field_data,
             self.scope_ids,
         )
-        test_system.xmodule_instance = peer_grading
 
         return peer_grading
 
diff --git a/common/lib/xmodule/xmodule/timelimit_module.py b/common/lib/xmodule/xmodule/timelimit_module.py
index e1c4042dc0..73744b5e8b 100644
--- a/common/lib/xmodule/xmodule/timelimit_module.py
+++ b/common/lib/xmodule/xmodule/timelimit_module.py
@@ -31,9 +31,6 @@ class TimeLimitModule(TimeLimitFields, XModule):
     Wrapper module which imposes a time constraint for the completion of its child.
     '''
 
-    def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
-
     # For a timed activity, we are only interested here
     # in time-related accommodations, and these should be disjoint.
     # (For proctored exams, it is possible to have multiple accommodations
diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py
index 0879e3bba3..0053cb5ca1 100644
--- a/common/lib/xmodule/xmodule/vertical_module.py
+++ b/common/lib/xmodule/xmodule/vertical_module.py
@@ -16,9 +16,6 @@ class VerticalFields(object):
 class VerticalModule(VerticalFields, XModule):
     ''' Layout module for laying out submodules vertically.'''
 
-    def __init__(self, *args, **kwargs):
-        XModule.__init__(self, *args, **kwargs)
-
     def student_view(self, context):
         fragment = Fragment()
         contents = []
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 055033a3bb..e644185c8c 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -392,6 +392,7 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock):  # pylint: disable=abstract-me
         super(XModule, self).__init__(*args, **kwargs)
         self._loaded_children = None
         self.system = self.runtime
+        self.runtime.xmodule_instance = self
 
     def __unicode__(self):
         return u'<x_module(id={0})>'.format(self.id)
@@ -737,7 +738,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
         assert self.xmodule_runtime.error_descriptor_class is not None
         if self.xmodule_runtime.xmodule_instance is None:
             try:
-                self.xmodule_runtime.xmodule_instance = self.xmodule_runtime.construct_xblock_from_class(
+                self.xmodule_runtime.construct_xblock_from_class(
                     self.module_class,
                     descriptor=self,
                     scope_ids=self.scope_ids,
@@ -1041,6 +1042,7 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime):  # pylint: disable=abs
         """
         The url prefix to be used by XModules to call into handle_ajax
         """
+        assert self.xmodule_instance is not None
         return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?')