Skip to content

gh-152548: Convert test_audit to the test.support.isolation.runInSubprocess() decorator - #152565

Open
serhiy-storchaka wants to merge 1 commit into
python:mainfrom
serhiy-storchaka:test-audit-isolated
Open

gh-152548: Convert test_audit to the test.support.isolation.runInSubprocess() decorator#152565
serhiy-storchaka wants to merge 1 commit into
python:mainfrom
serhiy-storchaka:test-audit-isolated

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Jun 29, 2026

Copy link
Copy Markdown
Member

Each audit test needs a fresh interpreter because sys.addaudithook() cannot be undone. With @isolation.runInSubprocess() these become ordinary TestCase methods that use self.assert* directly in the subprocess, removing the separate audit-tests.py driver script and the run_test_in_subprocess()/do_test()/run_python() code that parsed events out of the subprocess's stdout.

test_excepthook stays a small subprocess test (now inline via script_helper): the sys.excepthook audit event fires only for an exception reaching the interpreter's top level, which a @runInSubprocess() body (run under unittest) never does.

@read-the-docs-community

read-the-docs-community Bot commented Jun 29, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33364354 | 📁 Comparing 8f064af against main (6d209cb)

  🔍 Preview build  

8 files changed · ± 8 modified

± Modified

@serhiy-storchaka
serhiy-storchaka requested a review from zooba June 29, 2026 09:23
@serhiy-storchaka

Copy link
Copy Markdown
Member Author

@zooba, please look at this. I think it reflects the intention of the test, but is more clear.

@zooba zooba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall, I love it. I trust your implementation (and CI to detect when it doesn't work yet), and it's just the extra imports during test cases that worry me. Maybe those are dealt with in a way I didn't spot.

Comment thread Lib/test/test_audit.py

@support.isolated()
def test_marshal(self):
import_helper.import_module("marshal")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are intentionally trying to skip the test if the module can't be imported, so it'd be nice to preserve that somehow (marshal is unlikely to be missing, of course, but I assume there are others here that are less unlikely)

Comment thread Lib/test/test_audit.py
Comment on lines 14 to +18
from test import support
from test.support import import_helper
from test.support import os_helper
from test.support import script_helper
from unittest.mock import ANY

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the bit that worries me most about the change. Previously, we weren't importing all these for each test case, and now we are? I don't know that it'll change anything, but most critically it may lead to modules being imported early when we intended to import them during the test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

os and unittest.mock were already imported in Lib/test/audit-tests.py, at the top level.

I moved the rest at the methods level. But the one big left -- test.support itself, which imports many other modules. Even if we handle it somehow, unittest will left. This requires thought.

sys.addaudithook() cannot be undone, so each test needs a fresh
interpreter.  With @isolation.runInSubprocess() each test becomes an
ordinary TestCase method using self.assert* directly in the subprocess,
replacing the separate audit-tests.py driver script and the parsing of
events out of its stdout.

Per-test imports stay inside the methods, and a local requires_module()
decorator checks optional modules in the parent process, so a missing
module skips the test without first spawning the subprocess.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@serhiy-storchaka serhiy-storchaka changed the title gh-152548: Convert test_audit to the test.support.isolated() decorator gh-152548: Convert test_audit to the test.support.isolation.runInSubprocess() decorator Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants