gh-102251: Disable non-rerunnable test in test_import

[erlend-aasland]

Alternative to gh-104796; proposed as this PR has a much smaller diff and is less intrusive.

• Issue: Fix Refleak in test_import #102251

[sunmy2019]

I like this solution more. 👍

[erlend-aasland]

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 no_rerun() hack when the original problem has been addressed. With this solution we can simply remove the decorator from the affected test function. Also, it limits the decorator to this file only, and adds a relatively big warning for the programmer.

[erlend-aasland]

Comment on ref leak run:

For all buildbots, only test_peg_generator is leaking, except:

• buildbot/s390x RHEL8 Refleaks PR: test_import leaked [1, 1, 4] memory blocks, sum=6 Link

Suggesting to land this PR as a start, then focus on the RHEL8/s390x issue.

[sunmy2019]

Other parts LGTM

[vstinner]

@no_rerun(reason="rerun not possible; module state is never cleared (see gh-102251)")

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.

[erlend-aasland]

@vstinner: I agree that it is a bad pattern. This is a temporary fix only. See my comment earlier in this PR:

We want to be able to easily remove the no_rerun() hack when the original problem has been addressed.

[erlend-aasland]

The usual fix is to run such test in subprocesses.

ISTM we should rework parts of test_import to follow this pattern.

[Yhg1s]

This needs a backport to 3.12 to resolve the test_import leak that only shows up when you run python -m test -R:: test_import instead of python -m test -R :: test_import (note the space between -R ::). Some of the buildbots run with -R::.

[miss-islington]

Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

[bedevere-app]

GH-109540 is a backport of this pull request to the 3.12 branch.

[vstinner]

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]

Anyway, thanks for removing this ugly workaround :-(

 def setUpClass(cls): if '-R' in sys.argv or '--huntrleaks' in sys.argv: # https://github.com/python/cpython/issues/102251 raise unittest.SkipTest('unresolved refleaks (see gh-102251)') 

[erlend-aasland]

Oh, I totally forgot about this. I still think Victor's proposal¹ should be applied; this PR is a workaround, not a fix :)

Footnotes

1. "The usual fix is to run such test in subprocesses." — V. Stinner. ↩