From 8a8dbee340c4e3c228ecf2f082cd4d43cf655fad Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Wed, 3 Mar 2021 17:57:21 +0500 Subject: [PATCH] test: Fix assertstore and modulestore tests failing because of missing equality functions. https://github.com/edx/edx-platform/pull/26530 updated the tests to use pytest assertions instead of unittest assertionss. However, some tests depended on custom equality functions being set up in the test classes. These tests have been updated to explicitly do the needed comparisons. --- .../modulestore/tests/test_assetstore.py | 27 ++++++++++--------- .../tests/test_mixed_modulestore.py | 14 +++------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 905705491f..24ff7f5569 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -106,8 +106,6 @@ class TestMongoAssetMetadataStorage(TestCase): def setUp(self): super(TestMongoAssetMetadataStorage, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments - self.addTypeEqualityFunc(datetime, self._compare_datetimes) - self.addTypeEqualityFunc(AssetMetadata, self._compare_metadata) self.differents = (('different', 'burn.jpg'),) self.vrmls = ( @@ -117,21 +115,21 @@ class TestMongoAssetMetadataStorage(TestCase): self.regular_assets = (('asset', 'zippy.png'),) self.alls = self.differents + self.vrmls + self.regular_assets - def _compare_metadata(self, mdata1, mdata2, msg=None): + def _assert_metadata_equal(self, mdata1, mdata2): """ So we can use the below date comparison """ - if type(mdata1) != type(mdata2): # lint-amnesty, pylint: disable=unidiomatic-typecheck - self.fail(self._formatMessage(msg, u"{} is not same type as {}".format(mdata1, mdata2))) for attr in mdata1.ATTRS_ALLOWED_TO_UPDATE: # lint-amnesty, pylint: disable=redefined-outer-name - assert getattr(mdata1, attr) == getattr(mdata2, attr), msg + if isinstance(getattr(mdata1, attr), datetime): + self._assert_datetimes_equal(getattr(mdata1, attr), getattr(mdata2, attr)) + else: + assert getattr(mdata1, attr) == getattr(mdata2, attr) - def _compare_datetimes(self, datetime1, datetime2, msg=None): + def _assert_datetimes_equal(self, datetime1, datetime2): """ Don't compare microseconds as mongo doesn't encode below milliseconds """ - if not timedelta(seconds=-1) < datetime1 - datetime2 < timedelta(seconds=1): - self.fail(self._formatMessage(msg, u"{} != {}".format(datetime1, datetime2))) + assert datetime1.replace(microsecond=0) == datetime2.replace(microsecond=0) def _make_asset_metadata(self, asset_loc): """ @@ -187,7 +185,7 @@ class TestMongoAssetMetadataStorage(TestCase): # Find the asset's metadata and confirm it's the same. found_asset_md = store.find_asset_metadata(new_asset_loc) assert found_asset_md is not None - assert new_asset_md == found_asset_md + self._assert_metadata_equal(new_asset_md, found_asset_md) assert len(store.get_all_asset_metadata(course.id, 'asset')) == 1 @ddt.data(*MODULESTORE_SETUPS) @@ -378,9 +376,14 @@ class TestMongoAssetMetadataStorage(TestCase): # Find the same course and check its changed attribute. updated_asset_md = store.find_asset_metadata(new_asset_loc) assert updated_asset_md is not None - assert getattr(updated_asset_md, attribute, None) is not None + + updated_attr_val = getattr(updated_asset_md, attribute, None) + assert updated_attr_val is not None # Make sure that the attribute is unchanged from its original value. - assert getattr(updated_asset_md, attribute, None) == original_attr_val + if isinstance(original_attr_val, datetime): + self._assert_datetimes_equal(updated_attr_val, original_attr_val) + else: + assert updated_attr_val == original_attr_val @ddt.data(*MODULESTORE_SETUPS) def test_set_unknown_attrs(self, storebuilder): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 7852995749..d03fdfa2f4 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -117,13 +117,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): 'xblock_mixins': modulestore_options['xblock_mixins'], } - def _compare_ignore_version(self, loc1, loc2, msg=None): - """ - AssertEqual replacement for CourseLocator - """ - if loc1.for_branch(None) != loc2.for_branch(None): - self.fail(self._formatMessage(msg, u"{} != {}".format(six.text_type(loc1), six.text_type(loc2)))) - def setUp(self): """ Set up the database for testing @@ -148,8 +141,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.addCleanup(self.connection.drop_database, self.DB) self.addCleanup(self.connection.close) - self.addTypeEqualityFunc(BlockUsageLocator, '_compare_ignore_version') - self.addTypeEqualityFunc(CourseLocator, '_compare_ignore_version') # define attrs which get set in initdb to quell pylint self.writable_chapter_location = self.store = self.fake_location = None self.course_locations = {} @@ -1142,7 +1133,8 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): Verifies the results of calling get_parent_locations matches expected_results. """ for child_location, parent_location, revision in expected_results: - assert parent_location == self.store.get_parent_location(child_location, revision=revision) + assert parent_location.for_branch(None) if parent_location else parent_location == \ + self.store.get_parent_location(child_location, revision=revision) def verify_item_parent(self, item_location, expected_parent_location, old_parent_location, is_reverted=False): """ @@ -1631,7 +1623,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # problem location revision with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id): parent = mongo_store.get_parent_location(self.problem_x1a_1) # lint-amnesty, pylint: disable=no-member - assert parent == self.vertical_x1a # lint-amnesty, pylint: disable=no-member + assert parent.for_branch(None) == self.vertical_x1a # lint-amnesty, pylint: disable=no-member # Draft: # Problem path: