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

Limited API tests are now incorrectly skipped unconditionally #109045

Closed
andersk opened this issue Sep 7, 2023 · 5 comments · Fixed by #109046
Closed

Limited API tests are now incorrectly skipped unconditionally #109045

andersk opened this issue Sep 7, 2023 · 5 comments · Fixed by #109046

Comments

@andersk
Copy link
Contributor

andersk commented Sep 7, 2023

Commit 13a0007 (#108663, cc @vstinner) made all Python builds compatible with the Limited API, and removed the LIMITED_API_AVAILABLE flag. However, some tests are still checking for that flag, so they are now being incorrectly skipped.

#ifndef LIMITED_API_AVAILABLE
PyModule_AddObjectRef(m, "LIMITED_API_AVAILABLE", Py_False);
#else
PyModule_AddObjectRef(m, "LIMITED_API_AVAILABLE", Py_True);
if (_PyTestCapi_Init_VectorcallLimited(m) < 0) {
return NULL;
}
if (_PyTestCapi_Init_HeaptypeRelative(m) < 0) {
return NULL;
}
#endif

def requires_limited_api(test):
try:
import _testcapi
except ImportError:
return unittest.skip('needs _testcapi module')(test)
return unittest.skipUnless(
_testcapi.LIMITED_API_AVAILABLE, 'needs Limited API support')(test)

Linked PRs

andersk added a commit to andersk/cpython that referenced this issue Sep 7, 2023
Commit 13a0007 (python#108663) made all
Python builds compatible with the Limited API, and removed the
LIMITED_API_AVAILABLE flag.  However, some tests were still checking
for that flag, so they were now being incorrectly skipped.  Remove
these checks to let these tests run again.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
vstinner pushed a commit that referenced this issue Sep 7, 2023
…09046)

Commit 13a0007 (#108663) made all
Python builds compatible with the Limited API, and removed the
LIMITED_API_AVAILABLE flag.  However, some tests were still checking
for that flag, so they were now being incorrectly skipped.  Remove
these checks to let these tests run again.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@vstinner
Copy link
Member

vstinner commented Sep 7, 2023

I'm now curious. How did you notice that some tests were always skipped?

@vstinner
Copy link
Member

vstinner commented Sep 7, 2023

Before:

./python -m test test_capi 
(...)
Total tests: run=668 skipped=13
Total test files: run=1/1
Result: SUCCESS

After:

./python -m test test_capi 
(...)
Total tests: run=669 skipped=6
Total test files: run=1/1
Result: SUCCESS

I can see that there are less skipped tests :-) By the way, remaining skips:

$ ./python -m test test_capi -v|grep 'skipped '
test_Z (test.test_capi.test_getargs.String_TestCase.test_Z) ... skipped 'requires legacy Unicode C API'
test_Z_hash (test.test_capi.test_getargs.String_TestCase.test_Z_hash) ... skipped 'requires legacy Unicode C API'
test_u (test.test_capi.test_getargs.String_TestCase.test_u) ... skipped 'requires legacy Unicode C API'
test_u_hash (test.test_capi.test_getargs.String_TestCase.test_u_hash) ... skipped 'requires legacy Unicode C API'
test_trashcan_python_class1 (test.test_capi.test_misc.CAPITest.test_trashcan_python_class1) ... skipped "resource 'cpu' is not enabled"
test_trashcan_python_class2 (test.test_capi.test_misc.CAPITest.test_trashcan_python_class2) ... skipped "resource 'cpu' is not enabled"

@vstinner
Copy link
Member

vstinner commented Sep 7, 2023

I created issue #109050 for skipped 'requires legacy Unicode C API' tests.

@andersk
Copy link
Contributor Author

andersk commented Sep 7, 2023

I'm now curious. How did you notice that some tests were always skipped?

I actually found this from the source—during an unrelated investigation I noticed that changes to heaptype_relative.c and vectorcall_limited.c had no effect even when introducing deliberate compile errors.

(The unrelated investigation is into the feasibility of const-marking fields like tp_methods, tp_members, tp_getset, m_ml, m_methods, m_slots so that we can conveniently move a bunch of structs with function pointers to read-only memory for slightly improved security hardening.)

@vstinner
Copy link
Member

vstinner commented Sep 7, 2023

had no effect even when introducing deliberate compile errors

It's always good to test the tests by injecting errors on purpose and making sure that... they fail :-) Especially during development!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants