Skip to content
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

Jump to bottom

Split up _testcapimodule.c #93649

Open
encukou opened this issue Jun 9, 2022 · 10 comments
Open

Split up _testcapimodule.c #93649

encukou opened this issue Jun 9, 2022 · 10 comments
Labels
expert-C-API tests

Comments

@encukou
Copy link
Member

encukou commented Jun 9, 2022

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.
@encukou encukou added tests expert-C-API labels Jun 9, 2022
@arhadthedev
Copy link
Contributor

arhadthedev commented Jun 9, 2022

ideally without polluting the namespace of top-level modules

Can hypothetical Lib/_testcapimodule/*.c export their objects as test.capi.*?

@encukou
Copy link
Member Author

encukou commented Jun 10, 2022

As far as I know they can, as long as test.capi itself is not an extension module. It would need some extra support in both make and Windows builds, though.

encukou added a commit to encukou/cpython that referenced this issue Jul 4, 2022
@encukou
pythongh-93649: Split vectorcall testing from _testcapimodule.c
9020fe3 
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.
@bedevere-bot bedevere-bot mentioned this issue Jul 4, 2022
Merged
@encukou
Copy link
Member Author

encukou commented Jul 4, 2022 •
edited

PR: #94549

@vstinner
Copy link
Member

vstinner commented Jul 5, 2022 •
edited by encukou

I tried to group de 299 attributes of the _testcapi extension module. I'm not
sure about each attribute, I only looked quickly at the name, sometimes at the
implementation. But it should give a coarse idea of which "groups" make sense
or not.

Python/getargs.c and Python/modsupport.c (parse and build arguments) (51):
  • parse_tuple_and_keywords
  • getargs_B
  • getargs_C
  • getargs_D
  • getargs_H
  • getargs_I
  • getargs_K
  • getargs_L
  • getargs_S
  • getargs_U
  • getargs_Y
  • getargs_Z
  • getargs_Z_hash
  • getargs_b
  • getargs_c
  • getargs_d
  • getargs_es
  • getargs_es_hash
  • getargs_et
  • getargs_et_hash
  • getargs_f
  • getargs_h
  • getargs_i
  • getargs_k
  • getargs_keyword_only
  • getargs_keywords
  • getargs_l
  • getargs_n
  • getargs_p
  • getargs_positional_only_and_keywords
  • getargs_s
  • getargs_s_hash
  • getargs_s_hash_int
  • getargs_s_star
  • getargs_tuple
  • getargs_u
  • getargs_u_hash
  • getargs_w_star
  • getargs_y
  • getargs_y_hash
  • getargs_y_star
  • getargs_z
  • getargs_z_hash
  • getargs_z_star
  • test_L_code
  • test_k_code
  • test_s_code
  • test_buildvalue_N
  • test_buildvalue_issue38913
  • test_empty_argparse
  • argparsing
datetime, pytime.c (33):
  • PyDateTime_DATE_GET
  • PyDateTime_DELTA_GET
  • PyDateTime_GET
  • PyDateTime_TIME_GET
  • PyTime_AsMicroseconds
  • PyTime_AsMilliseconds
  • PyTime_AsSecondsDouble
  • PyTime_AsTimespec
  • PyTime_AsTimespec_clamp
  • PyTime_AsTimeval
  • PyTime_AsTimeval_clamp
  • PyTime_FromSeconds
  • PyTime_FromSecondsObject
  • datetime_check_date
  • datetime_check_datetime
  • datetime_check_delta
  • datetime_check_time
  • datetime_check_tzinfo
  • get_date_fromdate
  • get_date_fromtimestamp
  • get_datetime_fromdateandtime
  • get_datetime_fromdateandtimeandfold
  • get_datetime_fromtimestamp
  • get_delta_fromdsu
  • get_time_fromtime
  • get_time_fromtimeandfold
  • get_timezone_utc_capi
  • get_timezones_offset_zero
  • make_timezones_capi
  • pytime_object_to_time_t
  • pytime_object_to_timespec
  • pytime_object_to_timeval
  • test_datetime_capi
Types, metaclasses, type inheritance (wide category, not sure if it makes sense) (30):
  • _test_structmembersType
  • awaitType
  • instancemethod
  • ipowType
  • create_type_from_repeated_slots
  • matmulType
  • NullTpDocType
  • Generic
  • GenericAlias
  • MyList
  • ContainerNoGC
  • HeapCTypeMetaclass
  • HeapCTypeMetaclassCustomNew
  • HeapCTypeSetattr
  • HeapCTypeSubclass
  • HeapCTypeSubclassWithFinalizer
  • HeapCTypeWithBuffer
  • HeapCTypeWithDict
  • HeapCTypeWithNegativeDict
  • HeapCTypeWithWeakref
  • HeapDocCType
  • HeapGcCType
  • test_from_spec_invalid_metatype_inheritance
  • pytype_fromspec_meta
  • test_from_spec_metatype_inheritance
  • test_get_statictype_slots
  • test_get_type_name
  • test_get_type_qualname
  • test_lazy_hash_inheritance
  • test_type_from_ephemeral_spec
Function calls and spawn threads to call functions (25):
  • _test_thread_state
  • call_in_temporary_c_thread
  • get_kwargs
  • get_args
  • create_cfunction
  • pyobject_vectorcall
  • pyvectorcall_call
  • pyobject_fastcall
  • pyobject_fastcalldict
  • meth_fastcall
  • meth_fastcall_keywords
  • meth_noargs
  • meth_o
  • meth_varargs
  • meth_varargs_keywords
  • MethClass
  • MethInstance
  • MethStatic
  • MethodDescriptor2
  • MethodDescriptorBase
  • MethodDescriptorDerived
  • MethodDescriptorNopGet
  • return_null_without_error
  • return_result_with_error
  • stack_pointer
Unicode, codecs (12):
  • codec_incrementaldecoder
  • codec_incrementalencoder
  • test_unicode_compare_with_ascii
  • test_widechar
  • test_string_from_format
  • unicode_asucs4
  • unicode_asutf8
  • unicode_asutf8andsize
  • unicode_aswidechar
  • unicode_aswidecharstring
  • unicode_copycharacters
  • unicode_findchar
Memory allocators (17):
  • WITH_PYMALLOC
  • pyobject_malloc_without_gil
  • pymem_api_misuse
  • pymem_buffer_overflow
  • pymem_getallocatorsname
  • pymem_malloc_without_gil
  • test_pymem_alloc0
  • test_pymem_setallocators
  • test_pymem_setrawallocators
  • test_pyobject_new
  • test_pyobject_setallocators
  • set_nomemory
  • check_pyobject_forbidden_bytes_is_freed
  • check_pyobject_freed_is_freed
  • check_pyobject_null_is_freed
  • check_pyobject_uninitialized_is_freed
  • remove_mem_hooks
Exceptions, "error handling", C "errno" variable and signals (13):
  • RecursingInfinitelyError
  • raise_memoryerror
  • make_exception_with_doc
  • write_unraisable_exc
  • traceback_print
  • set_exception
  • raise_exception
  • set_exc_info
  • exception_print
  • fatal_error
  • error
  • set_errno
  • raise_SIGINT_then_send_None
C types limit and size (20):
  • CHAR_MAX
  • CHAR_MIN
  • INT_MAX
  • INT_MIN
  • LLONG_MAX
  • LLONG_MIN
  • LONG_MAX
  • LONG_MIN
  • PY_SSIZE_T_MAX
  • PY_SSIZE_T_MIN
  • SHRT_MAX
  • SHRT_MIN
  • UCHAR_MAX
  • UINT_MAX
  • ULLONG_MAX
  • ULONG_MAX
  • USHRT_MAX
  • SIZEOF_TIME_T
  • test_config
  • test_sizeof_c_types
Float, PyLongObject and PyNumber C API (16):
  • test_long_and_overflow
  • test_long_api
  • test_long_as_size_t
  • test_long_as_unsigned_long_long_mask
  • test_long_long_and_overflow
  • test_long_numbits
  • test_longlong_api
  • pynumber_tobase
  • DBL_MAX
  • DBL_MIN
  • FLT_MAX
  • FLT_MIN
  • float_pack
  • float_unpack
  • test_string_to_double
  • test_long_as_double
Docstring (10):
  • docstring_empty
  • docstring_no_signature
  • docstring_with_invalid_signature
  • docstring_with_invalid_signature2
  • docstring_with_signature
  • docstring_with_signature_and_extra_newlines
  • docstring_with_signature_but_no_doc
  • docstring_with_signature_with_defaults
  • no_docstring
  • test_with_docstring
Garbage collector (3):
  • test_gc_control
  • without_gc
  • with_tp_del
Tracemalloc (3):
  • tracemalloc_get_traceback
  • tracemalloc_track
  • tracemalloc_untrack
os module helpers (1):
  • W_STOPCODE
Subinterpreters (1):
  • run_in_subinterp
C API (57):

  • PyObject and PyVarObject C API (9):

    • negative_refcount
    • test_decref_doesnt_leak
    • test_incref_decref_API
    • test_incref_doesnt_leak
    • test_refcount_funcs
    • test_refcount_macros
    • test_xdecref_doesnt_leak
    • test_xincref_doesnt_leak
    • test_set_type_size

  • PyObject_Bytes(), PyObject_Repr(), PyObject_Str() (3):

    • pyobject_bytes_from_null
    • pyobject_repr_from_null
    • pyobject_str_from_null

  • PyTypeObject C API (2):

    • negative_dictoffset
    • type_get_version

  • C API macros (3):

    • test_macros
    • test_py_is_funcs
    • test_py_is_macros

  • PyCodeObject C API (2):

    • test_code_api
    • code_newempty

  • PyFrameObject C API (5):

    • frame_getbuiltins
    • frame_getgenerator
    • frame_getglobals
    • frame_getlasti
    • frame_getlocals

  • PyDictObject C API (3):

    • dict_get_version
    • dict_getitem_knownhash
    • test_dict_iteration

  • PyThreadState C API (2):

    • test_tstate_capi
    • crash_no_current_thread

  • PyListObject C API (1):

    • test_list_api

  • PySequence and PyMapping C API, PyObject_GetItem() (6):

    • sequence_getitem
    • get_mapping_items
    • get_mapping_keys
    • get_mapping_values
    • getitem_with_error
    • bad_get

  • C API "feature" macros like HAVE_FORK (1):

    • get_feature_macros

  • HAMT C API (1):

    • hamt

  • PyBuffer and memoryview C API (5):

    • getbuffer_with_null_view
    • make_memoryview_from_NULL_pointer
    • test_from_contiguous
    • PyBuffer_SizeFromFormat
    • test_pep3118_obsolete_write_locks

  • PyCapsule C API (1):

    • test_capsule

  • PyThread_tss C API (1):

    • test_pythread_tss_key_state

  • PyStructSeq C API (2):

    • test_structseq_newtype_doesnt_leak
    • test_structseq_newtype_null_descr_doc

  • PyEval_SetTrace C API (1):

    • settrace_to_record

  • PyMarshal C API (6):

    • pymarshal_read_last_object_from_file
    • pymarshal_read_long_from_file
    • pymarshal_read_object_from_file
    • pymarshal_read_short_from_file
    • pymarshal_write_long_to_file
    • pymarshal_write_object_to_file

  • Py_AddPendingCall C API (1):

    • _pending_threadfunc

  • Misc C API (2):

    • Py_Version
    • Py_CompileString
Misc (7):
  • __doc__
  • __file__
  • __loader__
  • __name__
  • __package__
  • __spec__
  • the_number_three

@vstinner
Copy link
Member

vstinner commented Jul 5, 2022

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.

miss-islington pushed a commit that referenced this issue Jul 8, 2022
@encukou
gh-93649: Split vectorcall testing from _testcapimodule.c (GH-94549)
be862b4 
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
@tiran
Copy link
Member

tiran commented Jul 8, 2022

You do not need to include _testcapi/vectorcall.c in MODULE__TESTCAPI_DEPS. It is already listed as dependency for _testcapi in the Setup file.

@gpshead gpshead mentioned this issue Jul 8, 2022
Open
tiran added a commit to tiran/cpython that referenced this issue Jul 8, 2022
@tiran
pythongh-93649: Fix dependencies of _testcapi
35b1153 
- header files are located in $(srcdir)
- dependencies must not list C files that are also in a makesetup Setup
  file
@bedevere-bot bedevere-bot mentioned this issue Jul 8, 2022
Merged
@brandtbucher brandtbucher mentioned this issue Jul 9, 2022
Merged
@bedevere-bot bedevere-bot mentioned this issue Jul 9, 2022
Merged
tiran added a commit that referenced this issue Jul 9, 2022
@tiran
gh-93649: Fix dependencies of _testcapi (#94695)
18ef240 
- 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
@encukou
Copy link
Member Author

encukou commented Jul 11, 2022

Thank you for the fix! I'll 🔨 test-with-buildbots next time I touch the buildsystem.
(I wonder why this issue didn't get the broken buildbot notification, though...)

@tiran
Copy link
Member

tiran commented Jul 11, 2022

Thank you for the fix! I'll hammer test-with-buildbots next time I touch the buildsystem. (I wonder why this issue didn't get the broken buildbot notification, though...)

We think that there was a problem in the buildbot hook, python/buildmaster-config#333

encukou pushed a commit that referenced this issue Jul 13, 2022
@arhadthedev
gh-93649: Fix linkage of _PyTestCapi_Init_Vectorcall (#94702)
fc21a43
encukou added a commit to encukou/cpython that referenced this issue Jul 28, 2022
@encukou
pythongh-93649: Split heaptype tests from _testcapimodule.c
fb2101c
@bedevere-bot bedevere-bot mentioned this issue Jul 28, 2022
Merged
encukou added a commit that referenced this issue Aug 1, 2022
@encukou
gh-93649: Split heaptype tests from _testcapimodule.c (GH-95386)
664e96a 
This removes the unused negative_dictoffset function:
the type this function would create is available as
    _testcapi.HeapCTypeWithNegativeDict
@philg314 philg314 mentioned this issue Aug 1, 2022
Closed
@philg314
Copy link
Contributor

philg314 commented Aug 1, 2022 •
edited

How should raiseTestError be handled?

cpython/Modules/_testcapimodule.c

Lines 52 to 62 in c7e5bba

static PyObject *TestError; /* set to exception object in init */
/* Raise TestError with test_name + ": " + msg, and return NULL. */
static PyObject *
raiseTestError(const char* test_name, const char* msg)
{
PyErr_Format(TestError, "%s: %s", test_name, msg);
return NULL;
}

Edit: For the Unicode tests I copied it and renamed it to _testcapi.unicode_error.

philg314 added a commit to philg314/cpython that referenced this issue Aug 2, 2022
@philg314
pythongh-93649: Split unicode tests from _testcapimodule.c
63e9afd
@encukou
Copy link
Member Author

encukou commented Aug 2, 2022

I'd switch to the built-in AssertionError, unless it's specifically a test for custom exceptions.

encukou added a commit to encukou/cpython that referenced this issue Aug 8, 2022
@encukou
pythongh-93649: Undefine NDEBUG in Modules/_testcapi/* to enable asse…
9b1375d 
…rt()
@encukou encukou mentioned this issue Aug 8, 2022
Closed
@bedevere-bot bedevere-bot mentioned this issue Aug 8, 2022
Merged
encukou pushed a commit to encukou/cpython that referenced this issue Aug 9, 2022
@philg314 @encukou
pythongh-93649: Split unicode tests from _testcapimodule.c
0f6cd30
@bedevere-bot bedevere-bot mentioned this issue Aug 9, 2022
Merged
encukou added a commit that referenced this issue Aug 10, 2022
@encukou @philg314
gh-93649: Split unicode tests from _testcapimodule.c & add some more (G…
325ae93 
…H-95819)

- Move PyUnicode tests to a separate file
- Add some more tests for PyUnicode_FromFormat

Co-authored-by: philg314 <110174000+philg314@users.noreply.github.com>
encukou added a commit that referenced this issue Aug 10, 2022
@encukou
gh-93649: Undefine NDEBUG in Modules/_testcapi/* to enable assert() (G…
5a97a93 
…H-95793)
iritkatriel pushed a commit to iritkatriel/cpython that referenced this issue Aug 11, 2022
@encukou @philg314 @iritkatriel
pythongh-93649: Split unicode tests from _testcapimodule.c & add some…
10d8903 
… more (pythonGH-95819)

- Move PyUnicode tests to a separate file
- Add some more tests for PyUnicode_FromFormat

Co-authored-by: philg314 <110174000+philg314@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
expert-C-API tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@vstinner @encukou @tiran @arhadthedev @philg314