Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-102251: Disable non-rerunnable test in test_import#106013
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-102251: Disable non-rerunnable test in test_import#106013
Uh oh!
There was an error while loading.Please reload this page.
Conversation
test_basic_multiple_interpreters_main_no_reset() does not support rerunning
sunmy2019 commentedJun 23, 2023
I like this solution more. 👍 |
erlend-aasland commentedJun 23, 2023
Yes; my rationale is: Keep the diff down; keep the solution as simple as possible. We want to be able to easily remove the |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
erlend-aasland commentedJun 23, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Comment on ref leak run: For all buildbots, only
Suggesting to land this PR as a start, then focus on the RHEL8/s390x issue. |
sunmy2019 left a comment
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.
Other parts LGTM
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
vstinner commentedJun 28, 2023
IMO @no_rerun() is bad pattern and should be avoided or removed. They are many tests with side effects which cannot be cancelled because there is no API for that. For example, in the past, there were tests registering codecs and error handlers. The usual fix is torun such test in subprocesses. For example, test_audit runs subprocesses on functions registering audit hooks, since there is no function to unregister a audit hookby design. |
erlend-aasland commentedJun 28, 2023
@vstinner: I agree that it is a bad pattern. This is a temporary fix only. See my comment earlier in this PR:
|
erlend-aasland commentedJun 28, 2023
ISTM we should rework parts of |
Yhg1s commentedSep 18, 2023
This needs a backport to 3.12 to resolve the test_import leak that only shows up when you run |
miss-islington commentedSep 18, 2023
Thanks@erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
GH-109540 is a backport of this pull request to the3.12 branch. |
vstinner commentedSep 18, 2023
If a test makes a change which cannot be undone, one option is to run it in a subprocess. Another option is to add an API to undo the change. For example, codecs.unregister() was added to fix tests adding codecs. See issue#108963 which is a similar issue. |
vstinner commentedSep 18, 2023
Anyway, thanks for removing this ugly workaround :-( |
Uh oh!
There was an error while loading.Please reload this page.
Alternative togh-104796; proposed as this PR has a much smaller diff and is less intrusive.