gh-110529: Guard _testcapi imports in tests#110530
Conversation
| import _testcapi | ||
| from test.support import import_helper | ||
|
|
||
| _testcapi = import_helper.import_module('_testcapi') |
There was a problem hiding this comment.
I think that such check should be done in Lib/test/test_capi/__init__.py. I don't think that it's worth to attempt to run any of sub-tests if _testcapi cannot be imported.
There was a problem hiding this comment.
I don't agree that it should only be done there, because people can run only a single test file. In case of ./python.exe Lib/test/test_capi/test_WHATEVER.py, this test must also be skipped correctly.
However, I think that we can also check this in test_capi/__init__.py just to speed up test runs without _testcapi
|
There is a test in |
|
cpython/Lib/test/test_capi/test_unicode.py Lines 6 to 26 in 0df772f But, not in all tests, here's the result without Only two tests are executed:
|
|
Running all Shows that only a single test is executed: So, I guess we can move it to Btw, without this patch tests fail with: |
|
With my latest commit and without |
|
This was unexpected: |
|
This happens because |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Please return test_from_format where it was. It is better to keep all C API tests in one place.
The rest LGTM. It is a small change and it is easier to go this way.
Currently, these modules are required for building 3.13-dev. See python/cpython/pull/110530
Currently, these modules are required for building 3.13-dev. See python/cpython/pull/110530
Currently, these modules are required for building 3.13-dev. See python/cpython/pull/110530
Currently, these modules are required for building 3.13-dev. See python/cpython/pull/110530
It's suspicious that this test is implemented with ctypes, and not _testcapi. I think that I wrote it long time ago, it should be reimplemented in _testcapi (the thin wrapper to the C API) :-) The problem is how to encode variadic arguments :-( ctypes make it easy for such quick test. Maybe ctypes is just fine. I don't think that it's worth it to still allow running test_capi if _testcapi is missing, just for this test.
One day, I wanted to make test_capi empty, move all tests to their test_xxx companion test, like move PyUnicode C API tests to test_str. But the ship has sailed, it's now too late :-) |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
If @vstinner is fine with skipping a test for PyUnicode_FromFormat() on platforms without _testcapi, then LGTM.
I propose to only check if _testcapi module is available in On CPython, the |
I tried to do that, but it didn't work: because there's |
Oh that's strange. It should be moved to test.support if it's used by other tests. |
Here's the offending test I am talking about: cpython/Lib/test/test_capi/test_misc.py Lines 1972 to 1977 in 8f07b6e So, when making all How about (either):
|
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Changes in test_dict and test_abstract LGTM. Changes in test_unicode are more controversial and not actually needed.
|
I don't get how it's an issue that cpython/Lib/test/test_capi/test_misc.py runs a subprocess which imports test_capi. The test should not be run if _testcapi extension is not available, since I propose to skip the whole test_capi package. No? |
Here's how I understand this:
|
I suppose that _testcapi lacks Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED or Py_MOD_PER_INTERPRETER_GIL_SUPPORTED. See own_gil and gil options of It seems like it's a real issue in the test, and this work just made it more obvious, no? |
|
|
|
See also #111067 which also has tests that depend on |
|
This PR is stale because it has been open for 30 days with no activity. |
|
@sobolevn: Are you still working on this old PR? If yes, you should update it, there are now conflicts. If not, I suggest closing it with its issue. |
test_dictandtest_abstractoftest_capiunconditionally import_testcapimodule #110529