New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split up _testcapimodule.c #93649
Comments
Can hypothetical |
As far as I know they can, as long as |
The _testcapimodule.c file is getting too large to work with effectively. Vectorcall tests aren't the biggest issue -- it's just an area I want to work on next, so I started there. It does make it clear that MethodDescriptor2 is related to testing vectorcall, which wasn't clear before (the /* Test PEP 590 */ section had an ambiguous end). This PR lays out a general structure of how tests can be split up, with more splitting to come later if the structure is OK.
PR: #94549 |
I tried to group de 299 attributes of the _testcapi extension module. I'm not Python/getargs.c and Python/modsupport.c (parse and build arguments) (51):
datetime, pytime.c (33):
Types, metaclasses, type inheritance (wide category, not sure if it makes sense) (30):
Function calls and spawn threads to call functions (25):
Unicode, codecs (12):
Memory allocators (17):
Exceptions, "error handling", C "errno" variable and signals (13):
C types limit and size (20):
Float, PyLongObject and PyNumber C API (16):
Docstring (10):
Garbage collector (3):
Tracemalloc (3):
os module helpers (1):
Subinterpreters (1):
C API (57):
Misc (7):
|
It might be interesting to split the large _testcapi module into multiple modules. But I like the idea of starting by splitting the long C file into multiple C files. |
The `_testcapimodule.c` file is getting too large to work with effectively. This PR lays out a general structure of how tests can be split up, with more splitting to come later if the structure is OK. Vectorcall tests aren't the biggest issue -- it's just an area I want to work on next, so I'm starting here. An issue specific to vectorcall tests is that it wasn't clear that e.g. `MethodDescriptor2` is related to testing vectorcall: the `/* Test PEP 590 */` section had an ambiguous end. Separate file should make things like this much clearer. OTOH, for some pieces it might not be clear where they should be -- I left `meth_fastcall` with tests of the other calling conventions. IMO, even with the ambiguity it's still worth it to split the huge file up. I'm not sure about the buildsystem changes, hopefully CI will tell me what's wrong. @vstinner, @markshannon: Do you think this is a good idea? Automerge-Triggered-By: GH:encukou
You do not need to include |
- header files are located in $(srcdir) - dependencies must not list C files that are also in a makesetup Setup file
- header files are located in $(srcdir) - dependencies must not list C files that are also in a makesetup Setup file - generate SRCDIRS for OOT builds
Thank you for the fix! I'll |
We think that there was a problem in the buildbot hook, python/buildmaster-config#333 |
This removes the unused negative_dictoffset function: the type this function would create is available as _testcapi.HeapCTypeWithNegativeDict
How should cpython/Modules/_testcapimodule.c Lines 52 to 62 in c7e5bba
|
I agree. Maybe we can write down (somewhere, in the C/Python code of test_capi, in the devguide?) a list of tests which belong to test_capi file, and which tests should be done in more specific test files? Like a general guideline. |
Maybe my comment is just outdated. Ignore it if it's the case :-) I don't understand compiler warnings on the old PR #95793 (merged last August): These warnings should only occur when assert() statements are removed by the compiler. Otherwise, these two variables are used. And the purpose of this PR is to make sure that assertions are not removed. Does it mean that NDEBUG is undefined too late? In Sadly, Address Sanitizer logs of this PR are no longer available:
On a recent job, I see: |
* main: (8272 commits) Update Windows readme.txt to clarify Visual Studio required versions (pythonGH-99522) pythongh-99460 Emscripten trampolines on optimized METH_O and METH_NOARGS code paths (python#99461) pythongh-92647: [Enum] use final status to determine lookup or create (pythonGH-99500) pythongh-81057: Move Globals in Core Code to _PyRuntimeState (pythongh-99496) Post 3.12.0a2 pythongh-99300: Use Py_NewRef() in Python/Python-ast.c (python#99499) pythongh-93649: Split pytime and datetime tests from _testcapimodule.c (python#99494) pythongh-99370: fix test_zippath_from_non_installed_posix (pythonGH-99483) pythonGH-99205: remove `_static` field from `PyThreadState` and `PyInterpreterState` (pythonGH-99385) pythongh-81057: Move the Remaining Import State Globals to _PyRuntimeState (pythongh-99488) pythongh-87604: Avoid publishing list of active per-interpreter audit hooks via the gc module (pythonGH-99373) pythongh-93649: Split getargs tests from _testcapimodule.c (python#99346) pythongh-81057: Move Global Variables Holding Objects to _PyRuntimeState. (pythongh-99487) pythonGH-98219: reduce sleep time in `asyncio` subprocess test (python#99464) pythonGH-99388: add `loop_factory` parameter to `asyncio.run` (python#99462) pythongh-99300: Use Py_NewRef() in PC/ directory (python#99479) pythongh-99300: Use Py_NewRef() in Doc/ directory (python#99480) pythongh-99300: Use Py_NewRef() in Modules/ directory (python#99473) pythongh-99300: Use Py_NewRef() in Modules/ directory (python#99469) pythongh-99370: Calculate zip path from prefix when in a venv (pythonGH-99371) ...
Automerge-Triggered-By: GH:erlend-aasland
Is it time to close issue #78453 to continue the work here? cc @serhiy-storchaka |
I moved PyUnicode C API tests from |
There are around 152 "PyUnicode" C API functions. That's a lot knowing that Python 3.12 exports 939 public functions (and 354 private functions): stats on the C API.
Currently, test_codecs is around 3 500 lines and test_unicode around 2 700 lines. If we want to extend the coverage of the PyUnicode C API (what is being discussed here :-)), I agree that moving the Python parts of PyUnicode C API tests can be the newly added |
In PR #99613, I proposed to @serhiy-storchaka to separate "codecs" tests from "unicode" tests. For me, "codecs" includes PyCodec C API but also "encode" and "decode" functions of the PyUnicode C API. What do you think? |
* origin/main: (1306 commits) Correct CVE-2020-10735 documentation (python#100306) pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273) pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551) Docs: Use `PY_VERSION_HEX` for version comparison (python#100179) pythongh-97909: Fix markup for `PyMethodDef` members (python#100089) pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890) pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890) pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205) pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469) pythongh-99540: Constant hash for _PyNone_Type to aid reproducibility (pythonGH-99541) pythongh-100039: enhance __signature__ to work with str and callables (pythonGH-100168) pythongh-99830: asyncio: Document returns of remove_{reader,writer} (python#100302) "Compound statement" docs: Fix with-statement step indexing (python#100286) pythonGH-90043: Handle NaNs in COMPARE_OP_FLOAT_JUMP (pythonGH-100278) Improve stats presentation for calls. (pythonGH-100274) Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295) pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277) Document that zipfile's pwd parameter is a `bytes` object (python#100209) pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271) Fix typo in introduction.rst (python#100266) ...
* main: pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803) Docs: Don't upload CI artifacts (python#100330) pythongh-94912: Added marker for non-standard coroutine function detection (python#99247) Correct CVE-2020-10735 documentation (python#100306) pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273) pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551) Docs: Use `PY_VERSION_HEX` for version comparison (python#100179) pythongh-97909: Fix markup for `PyMethodDef` members (python#100089) pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890) pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890) pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205) pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
encukou commentedJun 9, 2022
•
edited by bedevere-bot
Modules/_testcapimodule.c
is a nearly-8000-line behemoth with no clear structure or organization. It is getting hard to maintain.It also doesn't work well with testing (a) feature macros that affect
Python.h
and (b) module initialization, so we have additional C-API testing modules:_testmultiphase
,_testimportmultiple
,_testinternalcapi
._testbuffer
is already split out, but there are many other aspects of the API that would use a similar dedicated test suite.We should split and combine these, ideally without polluting the namespace of top-level modules.
The text was updated successfully, but these errors were encountered: