From 1beb4f38a10155f5abeaeb028771e1221e227f6c Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Tue, 2 Mar 2021 21:51:48 +0500 Subject: [PATCH 1/6] test: Remove the invalid pylint message from common/lib/pytest.ini This auto-added pylint message is preventing tests in common/lib from running. --- common/lib/pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/pytest.ini b/common/lib/pytest.ini index d166a11965..93c230eab2 100644 --- a/common/lib/pytest.ini +++ b/common/lib/pytest.ini @@ -1,6 +1,6 @@ [pytest] DJANGO_SETTINGS_MODULE = openedx.tests.settings -addopts = --nomigrations --reuse-db --durations=20 --json-report --json-report-omit keywords streams collectors log traceback tests --json-report-file=none # lint-amnesty, pylint: disable=syntax-error +addopts = --nomigrations --reuse-db --durations=20 --json-report --json-report-omit keywords streams collectors log traceback tests --json-report-file=none # Enable default handling for all warnings, including those that are ignored by default; # but hide rate-limit warnings (because we deliberately don't throttle test user logins) # and field_data deprecation warnings (because fixing them requires a major low-priority refactoring) From 6af0eb5c28d5003f1a2fe0a73629ef038b46901d Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Tue, 2 Mar 2021 22:00:47 +0500 Subject: [PATCH 2/6] test: Ignore running tests from /common/lib/pytest_cache. --- pavelib/utils/envs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index c295fd43d6..34b4968040 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -211,7 +211,7 @@ class Env: JS_REPORT_DIR = REPORT_DIR / 'javascript' # Directories used for common/lib/tests - IGNORED_TEST_DIRS = ('__pycache__', '.cache') + IGNORED_TEST_DIRS = ('__pycache__', '.cache', '.pytest_cache') LIB_TEST_DIRS = [] for item in (REPO_ROOT / "common/lib").listdir(): dir_name = (REPO_ROOT / 'common/lib' / item) From 7441702adecd95cfc0a26099ed96ef9245b4968c Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Wed, 3 Mar 2021 01:03:36 +0500 Subject: [PATCH 3/6] test: The exception on the pytest ExceptionInfo object can be accessed on the value attribute. --- common/lib/capa/capa/safe_exec/tests/test_safe_exec.py | 2 +- common/lib/capa/capa/tests/test_responsetypes.py | 2 +- common/lib/xmodule/xmodule/tests/test_course_module.py | 2 +- common/lib/xmodule/xmodule/tests/test_graders.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py b/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py index e8f6215995..68db41feec 100644 --- a/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py +++ b/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py @@ -77,7 +77,7 @@ class TestSafeExec(unittest.TestCase): # lint-amnesty, pylint: disable=missing- g = {} with pytest.raises(SafeExecException) as cm: safe_exec("1/0", g) - assert 'ZeroDivisionError' in text_type(cm.exception) + assert 'ZeroDivisionError' in text_type(cm.value) class TestSafeOrNot(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index ec7146d9d6..bf1f9dcf31 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -798,7 +798,7 @@ class StringResponseTest(ResponseTest): # pylint: disable=missing-class-docstri problem = self.build_problem(answer="a2", case_sensitive=False, regexp=True, additional_answers=['?\\d?']) with pytest.raises(Exception) as cm: self.assert_grade(problem, "a3", "correct") - exception_message = text_type(cm.exception) + exception_message = text_type(cm.value) assert 'nothing to repeat' in exception_message def test_hints(self): diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index 7d75c2d126..974ff7d07d 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -451,7 +451,7 @@ class ProctoringProviderTestCase(unittest.TestCase): with pytest.raises(ValueError) as context_manager: self.proctoring_provider.from_json(provider) - assert context_manager.exception.args[0] ==\ + assert context_manager.value.args[0] ==\ [f'The selected proctoring provider, {provider},' f' is not a valid provider. Please select from one of {allowed_proctoring_providers}.'] diff --git a/common/lib/xmodule/xmodule/tests/test_graders.py b/common/lib/xmodule/xmodule/tests/test_graders.py index 30995b5cf1..f8e88f8127 100644 --- a/common/lib/xmodule/xmodule/tests/test_graders.py +++ b/common/lib/xmodule/xmodule/tests/test_graders.py @@ -337,7 +337,7 @@ class GraderTest(unittest.TestCase): def test_grader_with_invalid_conf(self, invalid_conf, expected_error_message): with pytest.raises(ValueError) as error: graders.grader_from_conf([invalid_conf]) - assert expected_error_message in text_type(error.exception) + assert expected_error_message in text_type(error.value) @ddt.ddt From f35eed4e4c5c813fb2b3312575853d69b45e8515 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Wed, 3 Mar 2021 16:38:36 +0500 Subject: [PATCH 4/6] test: Fix test_capa_module.py::ProblemBlockTest::test_demand_hint_logging. --- .../xmodule/xmodule/tests/test_capa_module.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index f79d197e45..30ee3e0782 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1642,26 +1642,16 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss assert not result['should_enable_next_hint'] def test_demand_hint_logging(self): - def mock_location_text(self): # lint-amnesty, pylint: disable=unused-argument - """ - Mock implementation of __unicode__ or __str__ for the module's location. - """ - return u'i4x://edX/capa_test/problem/meh' - + """ + Test calling get_demand_hunt() results in an event being published. + """ module = CapaFactory.create(xml=self.demand_xml) - # Re-mock the module_id to a fixed string, so we can check the logging - module.location = Mock(module.location) - if six.PY2: - module.location.__unicode__ = mock_location_text - else: - module.location.__str__ = mock_location_text - with patch.object(module.runtime, 'publish') as mock_track_function: module.get_problem_html() module.get_demand_hint(0) mock_track_function.assert_called_with( module, 'edx.problem.hint.demandhint_displayed', - {'hint_index': 0, 'module_id': u'i4x://edX/capa_test/problem/meh', + {'hint_index': 0, 'module_id': str(module.location), 'hint_text': 'Demand 1', 'hint_len': 2} ) 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 5/6] 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: From 0e7664fbceb7a9c90db717561293b82b59307125 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Wed, 3 Mar 2021 18:09:01 +0500 Subject: [PATCH 6/6] test: Fix failing pytest asserts in GradesheetTest.test_weighted_grading. --- common/lib/xmodule/xmodule/tests/test_graders.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_graders.py b/common/lib/xmodule/xmodule/tests/test_graders.py index f8e88f8127..ddbe9b8873 100644 --- a/common/lib/xmodule/xmodule/tests/test_graders.py +++ b/common/lib/xmodule/xmodule/tests/test_graders.py @@ -43,14 +43,14 @@ class GradesheetTest(unittest.TestCase): agg_fields['first_attempted'] = now scores.append(ProblemScore(weighted_earned=3, weighted_possible=5, graded=True, **prob_fields)) all_total, graded_total = aggregate_scores(scores) - assert round(all_total - AggregatedScore(tw_earned=3, tw_possible=10, graded=False, **agg_fields), 7) >= 0 - assert round(graded_total - AggregatedScore(tw_earned=3, tw_possible=5, graded=True, **agg_fields), 7) >= 0 + assert all_total == AggregatedScore(tw_earned=3, tw_possible=10, graded=False, **agg_fields) + assert graded_total == AggregatedScore(tw_earned=3, tw_possible=5, graded=True, **agg_fields) # (0/5 non-graded) + (3/5 graded) + (2/5 graded) = 5/15 total, 5/10 graded scores.append(ProblemScore(weighted_earned=2, weighted_possible=5, graded=True, **prob_fields)) all_total, graded_total = aggregate_scores(scores) - assert round(all_total - AggregatedScore(tw_earned=5, tw_possible=15, graded=False, **agg_fields), 7) >= 0 - assert round(graded_total - AggregatedScore(tw_earned=5, tw_possible=10, graded=True, **agg_fields), 7) >= 0 + assert all_total == AggregatedScore(tw_earned=5, tw_possible=15, graded=False, **agg_fields) + assert graded_total == AggregatedScore(tw_earned=5, tw_possible=10, graded=True, **agg_fields) @ddt.ddt