GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||
GeezerVison is showing github.com — gh-102251: Disable non-rerunnable test in test_import by erlend-aasland · Pull Request #106013 · python/cpython Conversation
|
|
I like this solution more. 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes; my rationale is: Keep the diff down; keep the solution as simple as possible. We want to be able to easily remove the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Other parts LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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 to run such test in subprocesses. For example, test_audit runs subprocesses on functions registering audit hooks, since there is no function to unregister a audit hook by design. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@vstinner: I agree that it is a bad pattern. This is a temporary fix only. See my comment earlier in this PR:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
ISTM we should rework parts of |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This needs a backport to 3.12 to resolve the test_import leak that only shows up when you run |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…-106013) (cherry picked from commit 4849a80) Co-authored-by: Erlend E. Aasland <[email protected]>
|
GH-109540 is a backport of this pull request to the 3.12 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Anyway, thanks for removing this ugly workaround :-( |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Oh, I totally forgot about this. I still think Victor's proposal1 should be applied; this PR is a workaround, not a fix :) Footnotes
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.