]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-108927: Fix removing testing modules from sys.modules (GH-108952)
authorSerhiy Storchaka <storchaka@gmail.com>
Mon, 4 Dec 2023 15:43:27 +0000 (17:43 +0200)
committerGitHub <noreply@github.com>
Mon, 4 Dec 2023 15:43:27 +0000 (15:43 +0000)
It breaks import machinery if the test module has submodules used in
other tests.

Lib/test/libregrtest/main.py
Lib/test/libregrtest/single.py
Lib/test/regrtestdata/import_from_tests/test_regrtest_a.py [new file with mode: 0644]
Lib/test/regrtestdata/import_from_tests/test_regrtest_b/__init__.py [new file with mode: 0644]
Lib/test/regrtestdata/import_from_tests/test_regrtest_b/util.py [new file with mode: 0644]
Lib/test/regrtestdata/import_from_tests/test_regrtest_c.py [new file with mode: 0644]
Lib/test/test_regrtest.py
Misc/NEWS.d/next/Tests/2023-09-05-20-46-35.gh-issue-108927.TpwWav.rst [new file with mode: 0644]

index 16f6974ae324653f484c7be7137b7eaac97cbffa..7ca1b1cb65ae40e9f8fd7b69ca645b0e7379908f 100644 (file)
@@ -311,7 +311,7 @@ class Regrtest:
         else:
             tracer = None
 
-        save_modules = sys.modules.keys()
+        save_modules = set(sys.modules)
 
         jobs = runtests.get_jobs()
         if jobs is not None:
@@ -335,10 +335,18 @@ class Regrtest:
 
             result = self.run_test(test_name, runtests, tracer)
 
-            # Unload the newly imported modules (best effort finalization)
-            for module in sys.modules.keys():
-                if module not in save_modules and module.startswith("test."):
-                    support.unload(module)
+            # Unload the newly imported test modules (best effort finalization)
+            new_modules = [module for module in sys.modules
+                           if module not in save_modules and
+                                module.startswith(("test.", "test_"))]
+            for module in new_modules:
+                sys.modules.pop(module, None)
+                # Remove the attribute of the parent module.
+                parent, _, name = module.rpartition('.')
+                try:
+                    delattr(sys.modules[parent], name)
+                except (KeyError, AttributeError):
+                    pass
 
             if result.must_stop(self.fail_fast, self.fail_env_changed):
                 break
index eafeb5fe26f3f3a878b8be04087ccd69de5f58ad..235029d8620ff5b57c522a564430156d74b9bff7 100644 (file)
@@ -122,10 +122,6 @@ def _load_run_test(result: TestResult, runtests: RunTests) -> None:
     # Load the test module and run the tests.
     test_name = result.test_name
     module_name = abs_module_name(test_name, runtests.test_dir)
-
-    # Remove the module from sys.module to reload it if it was already imported
-    sys.modules.pop(module_name, None)
-
     test_mod = importlib.import_module(module_name)
 
     if hasattr(test_mod, "test_main"):
diff --git a/Lib/test/regrtestdata/import_from_tests/test_regrtest_a.py b/Lib/test/regrtestdata/import_from_tests/test_regrtest_a.py
new file mode 100644 (file)
index 0000000..9c3d0c7
--- /dev/null
@@ -0,0 +1,11 @@
+import sys
+import unittest
+import test_regrtest_b.util
+
+class Test(unittest.TestCase):
+    def test(self):
+        test_regrtest_b.util  # does not fail
+        self.assertIn('test_regrtest_a', sys.modules)
+        self.assertIs(sys.modules['test_regrtest_b'], test_regrtest_b)
+        self.assertIs(sys.modules['test_regrtest_b.util'], test_regrtest_b.util)
+        self.assertNotIn('test_regrtest_c', sys.modules)
diff --git a/Lib/test/regrtestdata/import_from_tests/test_regrtest_b/__init__.py b/Lib/test/regrtestdata/import_from_tests/test_regrtest_b/__init__.py
new file mode 100644 (file)
index 0000000..3dfba25
--- /dev/null
@@ -0,0 +1,9 @@
+import sys
+import unittest
+
+class Test(unittest.TestCase):
+    def test(self):
+        self.assertNotIn('test_regrtest_a', sys.modules)
+        self.assertIn('test_regrtest_b', sys.modules)
+        self.assertNotIn('test_regrtest_b.util', sys.modules)
+        self.assertNotIn('test_regrtest_c', sys.modules)
diff --git a/Lib/test/regrtestdata/import_from_tests/test_regrtest_b/util.py b/Lib/test/regrtestdata/import_from_tests/test_regrtest_b/util.py
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/Lib/test/regrtestdata/import_from_tests/test_regrtest_c.py b/Lib/test/regrtestdata/import_from_tests/test_regrtest_c.py
new file mode 100644 (file)
index 0000000..de80769
--- /dev/null
@@ -0,0 +1,11 @@
+import sys
+import unittest
+import test_regrtest_b.util
+
+class Test(unittest.TestCase):
+    def test(self):
+        test_regrtest_b.util  # does not fail
+        self.assertNotIn('test_regrtest_a', sys.modules)
+        self.assertIs(sys.modules['test_regrtest_b'], test_regrtest_b)
+        self.assertIs(sys.modules['test_regrtest_b.util'], test_regrtest_b.util)
+        self.assertIn('test_regrtest_c', sys.modules)
index d7b9f801092498447db6d846feaa9ccac6f2641d..e828941f6c779df514bdc4e70defe1c2e3ce8ded 100644 (file)
@@ -2031,6 +2031,25 @@ class ArgsTestCase(BaseTestCase):
         self.check_executed_tests(output, tests,
                                   stats=len(tests), parallel=True)
 
+    def test_unload_tests(self):
+        # Test that unloading test modules does not break tests
+        # that import from other tests.
+        # The test execution order matters for this test.
+        # Both test_regrtest_a and test_regrtest_c which are executed before
+        # and after test_regrtest_b import a submodule from the test_regrtest_b
+        # package and use it in testing. test_regrtest_b itself does not import
+        # that submodule.
+        # Previously test_regrtest_c failed because test_regrtest_b.util in
+        # sys.modules was left after test_regrtest_a (making the import
+        # statement no-op), but new test_regrtest_b without the util attribute
+        # was imported for test_regrtest_b.
+        testdir = os.path.join(os.path.dirname(__file__),
+                               'regrtestdata', 'import_from_tests')
+        tests = [f'test_regrtest_{name}' for name in ('a', 'b', 'c')]
+        args = ['-Wd', '-E', '-bb', '-m', 'test', '--testdir=%s' % testdir, *tests]
+        output = self.run_python(args)
+        self.check_executed_tests(output, tests, stats=3)
+
     def check_add_python_opts(self, option):
         # --fast-ci and --slow-ci add "-u -W default -bb -E" options to Python
         code = textwrap.dedent(r"""
diff --git a/Misc/NEWS.d/next/Tests/2023-09-05-20-46-35.gh-issue-108927.TpwWav.rst b/Misc/NEWS.d/next/Tests/2023-09-05-20-46-35.gh-issue-108927.TpwWav.rst
new file mode 100644 (file)
index 0000000..b1a7837
--- /dev/null
@@ -0,0 +1,4 @@
+Fixed order dependence in running tests in the same process
+when a test that has submodules (e.g. test_importlib) follows a test that
+imports its submodule (e.g. test_importlib.util) and precedes a test
+(e.g. test_unittest or test_compileall) that uses that submodule.