Skip to content

Don't require _testcapi and _testinternalcapi for test_monitoring.py#152311

Draft
ShaharNaveh wants to merge 5 commits into
python:mainfrom
ShaharNaveh:testcapi-monitoring
Draft

Don't require _testcapi and _testinternalcapi for test_monitoring.py#152311
ShaharNaveh wants to merge 5 commits into
python:mainfrom
ShaharNaveh:testcapi-monitoring

Conversation

@ShaharNaveh

Copy link
Copy Markdown
Contributor

same as #152171 and #152185

@bedevere-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Jun 26, 2026
@ShaharNaveh ShaharNaveh force-pushed the testcapi-monitoring branch 3 times, most recently from 261c9bd to 2f281f9 Compare June 26, 2026 16:59
@ShaharNaveh ShaharNaveh force-pushed the testcapi-monitoring branch from 2f281f9 to d043654 Compare June 26, 2026 17:08

@StanFromIreland StanFromIreland 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.

Do you know if there any more tests with the same issues (I'm not familiar with RustPython, so I don't know where to even try finding a list)? I'd rather get them all done in one go.

Comment thread Lib/test/test_monitoring.py Outdated


class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase):
_testcapi = import_helper.import_module("_testcapi")

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'll be executed at import time, so it will skip test_monitoring defeating the whole point of this PR. You can move it to a setUpClass.

@ShaharNaveh

ShaharNaveh commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Do you know if there any more tests with the same issues (I'm not familiar with RustPython, so I don't know where to even try finding a list)? I'd rather get them all done in one go.

We are copying the tests from CPython directly into Lib/test, this structure might look familiar:)


Anyway, I've gathered a list using a small script:

  • Lib/test/audit-tests.py
  • Lib/test/test_fileutils.py
  • Lib/test/test_optimizer.py
  • Lib/test/test_monitoring.py
  • Lib/test/test_type_cache.py
  • Lib/test/test_thread_local_bytecode.py
  • Lib/test/test_c_stack_unwind.py
  • Lib/test/test_free_threading/test_dict_watcher.py
  • Lib/test/test_gdb/test_jit.py

I've looked at each of those tests, all but test_monitoring.py requires either _testcapi or _testinternalcapi it seems. so this should be the last PR regarding this (hopefully).

Script used for findings
import ast
import pathlib

ROOT = pathlib.Path(__file__).parent
TEST_DIR = ROOT / "Lib/test"


class Visitor(ast.NodeVisitor):
    def __init__(self):
        self.found = False

    def visit_Call(self, node):
        func = node.func

        if not isinstance(func, ast.Attribute):
            return

        if func.attr != "import_module":
            return

        args = node.args
        if len(args) != 1:
            return

        arg = args[0]
        if not isinstance(arg, ast.Constant):
            return

        value = arg.value

        self.found = value in ("_testcapi", "_testinternalcapi")

    def visit_ClassDef(self, node):
        for bnode in node.body:
            if isinstance(bnode, ast.Assign):
                return self.generic_visit(bnode)

    def visit_FuncionDef(self, node):
        return

    def visit_AsyncFunctionDef(self, node):
        return


bad = set()
for child in TEST_DIR.glob("**/*.py"):
    if "test_capi" in child.parts:
        continue

    rchild = child.relative_to(ROOT)
    try:
        source = child.read_text(encoding="utf-8")
        mod = ast.parse(source)
    except:
        bad.add(rchild)
        continue

    visitor = Visitor()
    visitor.visit(mod)

    if visitor.found:
        print(rchild)


# print("\ncould not parse:\n" + "\n".join(map(str, bad)))

@StanFromIreland

Copy link
Copy Markdown
Member

Interestingly this uncovers UB in INSTRUMENTED_JUMP, I have a patch at #152376 to fix it and allow merging. I thought it would be accidental but after two re-runs it still hits it.

@ShaharNaveh

Copy link
Copy Markdown
Contributor Author

Interestingly this uncovers UB in INSTRUMENTED_JUMP, I have a patch at #152376 to fix it and allow merging. I thought it would be accidental but after two re-runs it still hits it.

I see that it just crashes without any information:/
Weird, I wonder if the test isn't isolated. maybe it somehow relies on importing _testcapi once at the top?

Anway, I've subscribed to #152376 so I'll update the PR once it's merged. puting on draft for now

@ShaharNaveh ShaharNaveh marked this pull request as draft June 27, 2026 09:54
@StanFromIreland

Copy link
Copy Markdown
Member

I see that it just crashes without any information:/

See the "Display logs" step, which has the UBSan output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants