From 63574e12b8d87dd9519c18eb1250ff35e0b3f4ac Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Mon, 30 Dec 2019 09:10:57 -0500 Subject: [PATCH] Remove pytest version constraint (#22626) Fix the issue that was preventing us from upgrading pytest. pytest does some manipulation of test packages that prevents `pkg_resources` from loading resources from them, but used to contain a workaround for the problem. That workaround was [removed](https://github.com/pytest-dev/pytest/issues/5392) in 4.6.0 as a performance enhancement when pytest switched from `pkg_resources` to `importlib-metadata` for its own entrypoint handling. This tripped up one of our test modules which defined classes that loaded templates from inside a test package. Moving these resources to the parent package fixes the problem. More and more, `pkg_resources` is being abandoned in favor of `importlib-metadata` and `importlib_resources` as they have a simpler design with much better performance. However, `importlib_resources` doesn't support loading files from any directory which isn't itself a Python package (and doesn't allow direct use of paths including directories within the package). Jinja2 chose a [different approach](https://github.com/pallets/jinja/pull/1082) that we may want to emulate in our resource handling. Also fixed usage of a removed `pytest.raises()` parameter and a bug in our configuration of the `common/lib` tests that became a problem after the upgrade. --- .../xmodule/{tests => }/templates/test/announcement.yaml | 0 .../xmodule/{tests => }/templates/test/anon_user_id.yaml | 0 .../xmodule/{tests => }/templates/test/latex_html.yaml | 0 .../xmodule/{tests => }/templates/test/somefile.notyaml | 0 .../xmodule/{tests => }/templates/test/zooming_image.yaml | 0 .../lib/xmodule/xmodule/tests/test_resource_templates.py | 2 +- openedx/core/djangoapps/schedules/tests/test_signals.py | 4 ++-- openedx/core/djangoapps/util/tests/test_print_setting.py | 2 +- pavelib/prereqs.py | 1 + pavelib/utils/test/suites/pytest_suite.py | 6 ++++++ requirements/constraints.txt | 7 ------- requirements/edx/development.txt | 4 +--- requirements/edx/testing.in | 1 - requirements/edx/testing.txt | 6 ++---- 14 files changed, 14 insertions(+), 19 deletions(-) rename common/lib/xmodule/xmodule/{tests => }/templates/test/announcement.yaml (100%) rename common/lib/xmodule/xmodule/{tests => }/templates/test/anon_user_id.yaml (100%) rename common/lib/xmodule/xmodule/{tests => }/templates/test/latex_html.yaml (100%) rename common/lib/xmodule/xmodule/{tests => }/templates/test/somefile.notyaml (100%) rename common/lib/xmodule/xmodule/{tests => }/templates/test/zooming_image.yaml (100%) diff --git a/common/lib/xmodule/xmodule/tests/templates/test/announcement.yaml b/common/lib/xmodule/xmodule/templates/test/announcement.yaml similarity index 100% rename from common/lib/xmodule/xmodule/tests/templates/test/announcement.yaml rename to common/lib/xmodule/xmodule/templates/test/announcement.yaml diff --git a/common/lib/xmodule/xmodule/tests/templates/test/anon_user_id.yaml b/common/lib/xmodule/xmodule/templates/test/anon_user_id.yaml similarity index 100% rename from common/lib/xmodule/xmodule/tests/templates/test/anon_user_id.yaml rename to common/lib/xmodule/xmodule/templates/test/anon_user_id.yaml diff --git a/common/lib/xmodule/xmodule/tests/templates/test/latex_html.yaml b/common/lib/xmodule/xmodule/templates/test/latex_html.yaml similarity index 100% rename from common/lib/xmodule/xmodule/tests/templates/test/latex_html.yaml rename to common/lib/xmodule/xmodule/templates/test/latex_html.yaml diff --git a/common/lib/xmodule/xmodule/tests/templates/test/somefile.notyaml b/common/lib/xmodule/xmodule/templates/test/somefile.notyaml similarity index 100% rename from common/lib/xmodule/xmodule/tests/templates/test/somefile.notyaml rename to common/lib/xmodule/xmodule/templates/test/somefile.notyaml diff --git a/common/lib/xmodule/xmodule/tests/templates/test/zooming_image.yaml b/common/lib/xmodule/xmodule/templates/test/zooming_image.yaml similarity index 100% rename from common/lib/xmodule/xmodule/tests/templates/test/zooming_image.yaml rename to common/lib/xmodule/xmodule/templates/test/zooming_image.yaml diff --git a/common/lib/xmodule/xmodule/tests/test_resource_templates.py b/common/lib/xmodule/xmodule/tests/test_resource_templates.py index 5a819f651c..881fabf51f 100644 --- a/common/lib/xmodule/xmodule/tests/test_resource_templates.py +++ b/common/lib/xmodule/xmodule/tests/test_resource_templates.py @@ -39,7 +39,7 @@ class TestClass(ResourceTemplates): derive a class from it in order to fill in some data it's expecting to find in its mro. """ - template_packages = [__name__] + template_packages = ['xmodule'] @classmethod def get_template_dir(cls): diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index 6d94e84fea..bcbb5eab56 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -49,7 +49,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): course_id=course.id, mode=CourseMode.AUDIT, ) - with pytest.raises(Schedule.DoesNotExist, message="Expecting Schedule to not exist"): + with pytest.raises(Schedule.DoesNotExist): enrollment.schedule @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) @@ -92,7 +92,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): ScheduleConfigFactory.create(site=site, enabled=True, create_schedules=True) course = _create_course_run(self_paced=False) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) - with pytest.raises(Schedule.DoesNotExist, message="Expecting Schedule to not exist"): + with pytest.raises(Schedule.DoesNotExist): enrollment.schedule @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) diff --git a/openedx/core/djangoapps/util/tests/test_print_setting.py b/openedx/core/djangoapps/util/tests/test_print_setting.py index ebdaa5ec97..7ed62bff34 100644 --- a/openedx/core/djangoapps/util/tests/test_print_setting.py +++ b/openedx/core/djangoapps/util/tests/test_print_setting.py @@ -6,7 +6,7 @@ from django.core.management import CommandError, call_command def test_without_args(capsys): - with pytest.raises(CommandError, message='Error: too few arguments'): + with pytest.raises(CommandError, match='Error: the following arguments are required: setting'): call_command('print_setting') diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 0141b0159b..cc376f9f60 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -203,6 +203,7 @@ PACKAGES_TO_UNINSTALL = [ "i18n-tools", # Because now it's called edx-i18n-tools "python-saml", # Because python3-saml shares the same directory name "pdfminer", # Replaced by pdfminer.six, which shares the same directory name + "pytest-faulthandler", # Because it was bundled into pytest ] diff --git a/pavelib/utils/test/suites/pytest_suite.py b/pavelib/utils/test/suites/pytest_suite.py index 68f6aa37b3..fa3d86c73a 100644 --- a/pavelib/utils/test/suites/pytest_suite.py +++ b/pavelib/utils/test/suites/pytest_suite.py @@ -316,6 +316,12 @@ class LibTestSuite(PytestSuite): cmd.append(xdist_string) for rsync_dir in Env.rsync_dirs(): cmd.append(u'--rsyncdir {}'.format(rsync_dir)) + # "--rsyncdir" throws off the configuration root, set it explicitly + if 'common/lib' in self.test_id: + cmd.append('--rootdir=common/lib') + cmd.append('-c common/lib/pytest.ini') + elif 'pavelib/paver_tests' in self.test_id: + cmd.append('--rootdir=pavelib/paver_tests') else: if self.processes == -1: cmd.append('-n auto') diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 375ed47084..65658b45de 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -52,13 +52,6 @@ oauthlib==2.1.0 # python3-saml 1.6.0 breaks unittests in common/djangoapps/third_party_auth/tests/test_views.py::SAMLMetadataTest python3-saml==1.5.0 -# Unless we constrain pytest to <4.6.0, it may break all tests inside of -# xmodule.tests.test_resource_templates.ResourceTemplatesTests See TE-2391 for details. -pytest<4.6.0 - -# 2.0.0 is a dummy package, because faulthandler has been incorporated into pytest 5.0 -pytest-faulthandler<2.0.0 - # transifex-client 0.13.5 and 0.13.6 pin six and urllib3 to old versions needlessly # https://github.com/transifex/transifex-client/issues/252 transifex-client==0.13.4 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 1a55f32fd8..4a554229c6 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -28,7 +28,6 @@ apipkg==1.5 appdirs==1.4.3 argh==0.26.2 astroid==1.5.3 -atomicwrites==1.3.0 attrs==19.3.0 aws-xray-sdk==0.95 babel==1.3 @@ -249,11 +248,10 @@ pysrt==1.1.1 pytest-attrib==0.1.3 git+https://github.com/nedbat/pytest-cov.git@nedbat/cov5-combine#egg=pytest-cov==0.0 pytest-django==3.7.0 -pytest-faulthandler==1.6.0 pytest-forked==1.1.3 pytest-randomly==3.2.0 pytest-xdist==1.31.0 -pytest==4.5.0 +pytest==5.3.2 python-dateutil==2.4.0 python-levenshtein==0.12.0 python-memcached==1.59 diff --git a/requirements/edx/testing.in b/requirements/edx/testing.in index eaadfc0b3f..0d223a6f00 100644 --- a/requirements/edx/testing.in +++ b/requirements/edx/testing.in @@ -36,7 +36,6 @@ pytest # Testing framework pytest-attrib # Select tests based on attributes git+https://github.com/nedbat/pytest-cov.git@nedbat/cov5-combine#egg=pytest-cov==0.0 git+https://github.com/nedbat/coverage_pytest_plugin.git@29de030251471e200ff255eb9e549218cd60e872#egg=coverage_pytest_plugin==0.0 -pytest-faulthandler # Report on serious crashes pytest-django # Django support for pytest pytest-randomly # pytest plugin to randomly order tests pytest-xdist # Parallel execution of tests on multiple CPU cores or hosts diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index a7678c1c26..e2e6b731ec 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -27,7 +27,6 @@ apipkg==1.5 # via execnet appdirs==1.4.3 argh==0.26.2 astroid==1.5.3 # via pylint, pylint-celery -atomicwrites==1.3.0 # via pytest attrs==19.3.0 aws-xray-sdk==0.95 # via moto babel==1.3 @@ -200,7 +199,7 @@ numpy==1.18.0 git+https://github.com/joestump/python-oauth2.git@b94f69b1ad195513547924e380d9265133e995fa#egg=oauth2 oauthlib==2.1.0 git+https://github.com/edx/edx-ora2.git@2.5.3#egg=ora2==2.5.3 -packaging==19.2 # via tox +packaging==19.2 # via pytest, tox path.py==8.2.1 pathlib2==2.3.5 # via pytest pathtools==0.1.2 @@ -238,11 +237,10 @@ pysrt==1.1.1 pytest-attrib==0.1.3 git+https://github.com/nedbat/pytest-cov.git@nedbat/cov5-combine#egg=pytest-cov==0.0 pytest-django==3.7.0 -pytest-faulthandler==1.6.0 pytest-forked==1.1.3 # via pytest-xdist pytest-randomly==3.2.0 pytest-xdist==1.31.0 -pytest==4.5.0 +pytest==5.3.2 python-dateutil==2.4.0 python-levenshtein==0.12.0 python-memcached==1.59