| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -311,7 +311,7 @@ def run_tests_sequentially(self, runtests) -> None: | |
| else: | ||
| tracer = None | ||
|
|
||
| save_modules = sys.modules.keys() | ||
| save_modules = set(sys.modules) | ||
|
|
||
| jobs = runtests.get_jobs() | ||
| if jobs is not None: | ||
| Expand All | @@ -335,10 +335,18 @@ def run_tests_sequentially(self, runtests) -> None: | |
|
|
||
| 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('.') | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMaybe raise an error if the parent itself has a parent other than test.. Currently, it should not happen. But it may happen in the future, if tests are reorganized.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityWhat do you mean? The parent should be "test" or start with "test.", because the module starts with "test.". If the parent starts with "test." it will be removed as well. If it was already removed before children, the code ignores KeyError.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIf a test imports test.test_a.test_b.test_c, does this code unloads test.test_a, test.test_a.test_b and test.test_a.test_b.test_c?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityYes, if they were not imported before running tests.
Sorry, something went wrong.
All reactions
|
||
| try: | ||
| delattr(sys.modules[parent], name) | ||
| except (KeyError, AttributeError): | ||
| pass | ||
|
|
||
| if result.must_stop(self.fail_fast, self.fail_env_changed): | ||
| break | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -2031,6 +2031,25 @@ def test_dev_mode(self): | |
| 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')] | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityWould you mind to add a comment to explain the purpose of these test? By reading the code, it's unclear to me. Explain that the test execution order matters for this test. Does the test fails if a regression is introduced? Maybe add a test_regrtest_d test checking if test_regrtest_a, test_regrtest_b and test_regrtest_c are not loaded?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityDone.
Sorry, something went wrong.
All reactions
|
||
| 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""" | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI would prefer to move this logic inside libregrtest/single.py, in the run_single_test() function.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI did it, but it is only needed in single-process run. In multi-process run it only adds an overhead. So I moved it back.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.