FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Don't require `_testcapi` and `_testinternalcapi` for `test_monitoring.py` by ShaharNaveh · Pull Request #152311 · python/cpython · GitHub

/ cpython Public

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

Merged
StanFromIreland merged 6 commits into
python:mainfrom
ShaharNaveh:testcapi-monitoring
Jun 30, 2026
Merged

Don't require _testcapi and _testinternalcapi for test_monitoring.py#152311
StanFromIreland merged 6 commits into
python:mainfrom
ShaharNaveh:testcapi-monitoring

Conversation

Copy link
Copy Markdown
Contributor

same as #152171 and #152185

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

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Jun 26, 2026
edited
Loading

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)))

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.

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 marked this pull request as draft June 27, 2026 09:54

Copy link
Copy Markdown
Member

I see that it just crashes without any information:/

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

ShaharNaveh marked this pull request as ready for review June 29, 2026 09:16

ShaharNaveh commented Jun 29, 2026
edited
Loading

Copy link
Copy Markdown
Contributor Author

Seems like #152376 fixed it. tysm for looking into it!

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, thanks.

StanFromIreland added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 30, 2026
StanFromIreland merged commit 189ab83 into python:main Jun 30, 2026
58 checks passed

Copy link
Copy Markdown

Thanks @ShaharNaveh for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

bedevere-app Bot commented Jun 30, 2026

Copy link
Copy Markdown

GH-152660 is a backport of this pull request to the 3.15 branch.

bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 30, 2026

bedevere-app Bot commented Jun 30, 2026

Copy link
Copy Markdown

GH-152661 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 30, 2026
StanFromIreland added the needs backport to 3.13 bugs and security fixes label Jun 30, 2026

Copy link
Copy Markdown

Thanks @ShaharNaveh for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

Copy link
Copy Markdown

Sorry, @ShaharNaveh and @StanFromIreland, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 189ab8388762c8102b2d72ad3a1a921f7f2aef10 3.13

Copy link
Copy Markdown
Member

Skipping backport to 3.13 here.

StanFromIreland added a commit that referenced this pull request Jun 30, 2026
…oring.py` (GH-152311) (#152660)

(cherry picked from commit 189ab83)

Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <stan@python.org>
StanFromIreland added a commit that referenced this pull request Jun 30, 2026
…oring.py` (GH-152311) (#152661)

(cherry picked from commit 189ab83)

Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <stan@python.org>
maurycy added a commit to maurycy/cpython that referenced this pull request Jul 1, 2026
* main: (266 commits)
  pythongh-151626: Fix tests that fail when PYTHONPYCACHEPREFIX is set (pythonGH-151952)
  pythongh-152728: IDLE - move 3 toplevel fix_xyz functions to idlelb.util (python#152729)
  pythongh-152711: Add pythoninfo-build command to Platforms/Android (python#152713)
  pythongh-152715: Add pythoninfo-build command to Platforms/Apple (python#152716)
  pythongh-152433: Windows: enable mmapmodule for UWP (python#152473)
  pythongh-152433: Windows: use GetFileSizeEx instead of GetFileSize for memory mapped files (python#152383)
  pythonGH-81881: Raise `SpecialFileError` for sockets and devices in `shutil.copyfile` (python#142693)
  pythongh-152502: Detect the curses mouse interface and is_* methods portably (pythonGH-152705)
  pythongh-145857: Replace `DELETE_GLOBAL` with `PUSH_NULL; STORE_GLOBAL` (pythonGH-146314)
  pythongh-145854: Replace `DELETE_NAME` with `PUSH_NULL; STORE_NAME` (pythonGH-146006)
  pythongh-152680: Detect container/VM in test.pythoninfo (python#152668)
  pythongh-152682: Fix NULL dereference on OOM in `symtable_visit_type_param_bound_or_default` (python#152684)
  pythongh-151881: Skip tk_inactive negativity check on Windows (pythonGH-152683)
  pythongh-152546: Refactor `mappingproxy.__new__` to use `PyDictProxy_New` (python#152547)
  pythongh-151126: Fix a possible crash during the startup with no memory under `Py_STACKREF_DEBUG` (python#152478)
  pythongh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (python#152642)
  pythongh-151029: Fix `test_remote_exec_deleted_static_executable` on static installed builds (pythonGH-152653)
  pythongh-121249: Deprecate using F/D type codes in the struct module (python#152309)
  pythongh-152192: Fix JUMP_BACKWARD passing a truncated oparg to the jit tracer (pythonGH-152382)
  Don't require the `_test{internal}capi` modules in `test_monitoring.py` (python#152311)
  ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL