Skip to content
New issue

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

Proper fix for 'mod' leaking across importlib tests #61

Conversation

jaraco
Copy link

@jaraco jaraco commented Mar 6, 2024

Addresses the reported issue:

 cpython ghpython-116307/safe-site-dir @ ./python.exe -m test test_importlib -v -m test_resolve -m test_module_resources
== CPython 3.13.0a3+ (heads/main:7fdd4235d7, Feb 6 2024, 12:06:21) [Clang 15.0.0 (clang-1500.1.0.2.5)]
== macOS-14.3.1-arm64-arm-64bit-Mach-O little-endian
== Python build: release
== cwd: /Users/jaraco/code/python/cpython/build/test_python_worker_67968æ
== CPU count: 12
== encodings: locale=UTF-8 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 1092583257
0:00:00 load avg: 1.19 Run 1 test sequentially
0:00:00 load avg: 1.19 [1/1] test_importlib
test_module_resources (test.test_importlib.resources.test_files.ModulesFilesTests.test_module_resources)
A module can have resources found adjacent to the module. ... ok
test_resolve (test.test_importlib.test_main.ImportTests.test_resolve) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.004s

OK

== Tests result: SUCCESS ==

1 test OK.

Total duration: 161 ms
Total tests: run=2 (filtered)
Total test files: run=1/1 (filtered)
Result: SUCCESS

…e that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.
@jaraco
Copy link
Author

jaraco commented Mar 6, 2024

We'll want to decide if this can be backported to older Pythons or not. If you'd rather I propose this change directly to CPython, then you can rebase the other PR on this after merged.

@jaraco
Copy link
Author

jaraco commented Mar 6, 2024

I've backported this change in python/importlib_resources@f3f4b0ab8b.

Copy link
Owner

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks; elegant!

@erlend-aasland
Copy link
Owner

I think it is fine to backport test.support stuff like this. We've done it before.

@erlend-aasland erlend-aasland merged commit ccd8bea into erlend-aasland:sqlite/testcapi Mar 6, 2024
2 checks passed
@jaraco
Copy link
Author

jaraco commented Mar 12, 2024

I plan to submit these changes as a separate PR to CPython, as the re-import of importlib.resources now depends on it. (python#116678)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants