Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
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
I like this solution more. 👍 |
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. |
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.
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. |
@vstinner: I agree that it is a bad pattern. This is a temporary fix only. See my comment earlier in this PR:
|
ISTM we should rework parts of |
This needs a backport to 3.12 to resolve the test_import leak that only shows up when you run |
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. |
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. |
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.