From 10fe9c01802a8b8d73a2169142fc66ef89b811ff Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 12 Jan 2015 11:00:51 -0800 Subject: [PATCH] pylint fixes --- .../xmodule/xmodule/library_content_module.py | 2 +- .../xmodule/tests/test_library_content.py | 37 +++++++++++-------- .../courseware/tests/test_module_render.py | 11 +++--- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index e3384efefc..c1d5dc9fb5 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -222,7 +222,7 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): return self._selected_set # pylint: disable=access-member-before-definition lib_tools = self.runtime.service(self, 'library_tools') - format_block_keys = lambda block_keys: lib_tools.create_block_analytics_summary(self.location.course_key, block_keys) + format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys) # Determine which of our children we will show: selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 7eb1c81a18..c3bb661337 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -356,7 +356,8 @@ class TestLibraryContentAnalytics(LibraryContentTest): # Now increase max_count so that one more child will be added: self.lc_block.max_count = 2 - del self.lc_block._xmodule._selected_set # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + del self.lc_block._xmodule._selected_set children = self.lc_block.get_child_descriptors() self.assertEqual(len(children), 2) child, new_child = children if children[0].location == child.location else reversed(children) @@ -394,24 +395,25 @@ class TestLibraryContentAnalytics(LibraryContentTest): event_data = self._assert_event_was_published("assigned") for block_list in (event_data["added"], event_data["result"]): - self.assertEqual(len(block_list), 1) # The main_vertical is the only root block added, and is the only result. + self.assertEqual(len(block_list), 1) # main_vertical is the only root block added, and is the only result. self.assertEqual(block_list[0]["usage_key"], unicode(course_usage_main_vertical)) # Check that "descendants" is a flat, unordered list of all of main_vertical's descendants: - descendants_expected = {} - for lib_key, course_usage_key in ( + descendants_expected = ( (inner_vertical.location, course_usage_inner_vertical), (html_block.location, course_usage_html), (problem_block.location, course_usage_problem), - ): - descendants_expected[unicode(course_usage_key)] = { + ) + descendant_data_expected = {} + for lib_key, course_usage_key in descendants_expected: + descendant_data_expected[unicode(course_usage_key)] = { "usage_key": unicode(course_usage_key), "original_usage_key": unicode(lib_key), "original_usage_version": unicode(self.store.get_block_original_usage(course_usage_key)[1]), } - self.assertEqual(len(block_list[0]["descendants"]), len(descendants_expected)) + self.assertEqual(len(block_list[0]["descendants"]), len(descendant_data_expected)) for descendant in block_list[0]["descendants"]: - self.assertEqual(descendant, descendants_expected.get(descendant["usage_key"])) + self.assertEqual(descendant, descendant_data_expected.get(descendant["usage_key"])) def test_removed_overlimit(self): """ @@ -419,10 +421,11 @@ class TestLibraryContentAnalytics(LibraryContentTest): We go from one blocks assigned to none because max_count has been decreased. """ # Decrease max_count to 1, causing the block to be overlimit: - self.lc_block.get_child_descriptors() # We must call an XModule method before we can change max_count - otherwise the change has no effect + self.lc_block.get_child_descriptors() # This line is needed in the test environment or the change has no effect self.publisher.reset_mock() # Clear the "assigned" event that was just published. self.lc_block.max_count = 0 - del self.lc_block._xmodule._selected_set # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + del self.lc_block._xmodule._selected_set # Check that the event says that one block was removed, leaving no blocks left: children = self.lc_block.get_child_descriptors() @@ -438,21 +441,24 @@ class TestLibraryContentAnalytics(LibraryContentTest): We go from two blocks assigned, to one because the others have been deleted from the library. """ # Start by assigning two blocks to the student: - self.lc_block.get_child_descriptors() # We must call an XModule method before we can change max_count - otherwise the change has no effect + self.lc_block.get_child_descriptors() # This line is needed in the test environment or the change has no effect self.lc_block.max_count = 2 - del self.lc_block._xmodule._selected_set # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + del self.lc_block._xmodule._selected_set initial_blocks_assigned = self.lc_block.get_child_descriptors() self.assertEqual(len(initial_blocks_assigned), 2) self.publisher.reset_mock() # Clear the "assigned" event that was just published. # Now make sure that one of the assigned blocks will have to be un-assigned. - # To cause an "invalid" event, we delete all blocks from the content library except for one of the two already assigned to the student: + # To cause an "invalid" event, we delete all blocks from the content library + # except for one of the two already assigned to the student: keep_block_key = initial_blocks_assigned[0].location keep_block_lib_usage_key, keep_block_lib_version = self.store.get_block_original_usage(keep_block_key) deleted_block_key = initial_blocks_assigned[1].location self.library.children = [keep_block_lib_usage_key] self.store.update_item(self.library, self.user_id) self.lc_block.refresh_children() - del self.lc_block._xmodule._selected_set # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access + del self.lc_block._xmodule._selected_set # Check that the event says that one block was removed, leaving one block left: children = self.lc_block.get_child_descriptors() @@ -460,7 +466,8 @@ class TestLibraryContentAnalytics(LibraryContentTest): event_data = self._assert_event_was_published("removed") self.assertEqual(event_data["removed"], [{ "usage_key": unicode(deleted_block_key), - "original_usage_key": None, # Note: original_usage_key info is sadly unavailable because the block has been deleted so that info can no longer be retrieved + "original_usage_key": None, # Note: original_usage_key info is sadly unavailable because the block has been + # deleted so that info can no longer be retrieved "original_usage_version": None, "descendants": [], }]) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index ca7c07f8a4..d6ce93563e 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -973,10 +973,10 @@ class TestModuleTrackingContext(ModuleStoreTestCase): def test_context_contains_display_name(self, mock_tracker): problem_display_name = u'Option Response Problem' - module_info = self.handle_callback_and_get_module_info_from_event(mock_tracker, problem_display_name) + module_info = self.handle_callback_and_get_module_info(mock_tracker, problem_display_name) self.assertEquals(problem_display_name, module_info['display_name']) - def handle_callback_and_get_module_info_from_event(self, mock_tracker, problem_display_name=None): + def handle_callback_and_get_module_info(self, mock_tracker, problem_display_name=None): """ Creates a fake module, invokes the callback and extracts the 'module' metadata from the emitted problem_check event. @@ -1006,7 +1006,7 @@ class TestModuleTrackingContext(ModuleStoreTestCase): return event['context']['module'] def test_missing_display_name(self, mock_tracker): - actual_display_name = self.handle_callback_and_get_module_info_from_event(mock_tracker)['display_name'] + actual_display_name = self.handle_callback_and_get_module_info(mock_tracker)['display_name'] self.assertTrue(actual_display_name.startswith('problem')) def test_library_source_information(self, mock_tracker): @@ -1017,8 +1017,9 @@ class TestModuleTrackingContext(ModuleStoreTestCase): """ original_usage_key = UsageKey.from_string(u'block-v1:A+B+C+type@problem+block@abcd1234') original_usage_version = ObjectId() - with patch('xmodule.modulestore.mixed.MixedModuleStore.get_block_original_usage', lambda _, key: (original_usage_key, original_usage_version)): - module_info = self.handle_callback_and_get_module_info_from_event(mock_tracker) + mock_get_original_usage = lambda _, key: (original_usage_key, original_usage_version) + with patch('xmodule.modulestore.mixed.MixedModuleStore.get_block_original_usage', mock_get_original_usage): + module_info = self.handle_callback_and_get_module_info(mock_tracker) self.assertIn('original_usage_key', module_info) self.assertEqual(module_info['original_usage_key'], unicode(original_usage_key)) self.assertIn('original_usage_version', module_info)