-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
Closed
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)testsTests in the Lib/test dirTests in the Lib/test dirtopic-C-APItype-featureA feature request or enhancementA feature request or enhancement
Description
Feature or enhancement
Right now CAPI tests of set and frozenset are quite outdated. They are defined as .test_c_api method on set objects, when Py_DEBUG is set:
Lines 2371 to 2511 in 7e30821
| #ifdef Py_DEBUG | |
| /* Test code to be called with any three element set. | |
| Returns True and original set is restored. */ | |
| #define assertRaises(call_return_value, exception) \ | |
| do { \ | |
| assert(call_return_value); \ | |
| assert(PyErr_ExceptionMatches(exception)); \ | |
| PyErr_Clear(); \ | |
| } while(0) | |
| static PyObject * | |
| test_c_api(PySetObject *so, PyObject *Py_UNUSED(ignored)) | |
| { | |
| Py_ssize_t count; | |
| const char *s; | |
| Py_ssize_t i; | |
| PyObject *elem=NULL, *dup=NULL, *t, *f, *dup2, *x=NULL; | |
| PyObject *ob = (PyObject *)so; | |
| Py_hash_t hash; | |
| PyObject *str; | |
| /* Verify preconditions */ | |
| assert(PyAnySet_Check(ob)); | |
| assert(PyAnySet_CheckExact(ob)); | |
| assert(!PyFrozenSet_CheckExact(ob)); | |
| /* so.clear(); so |= set("abc"); */ | |
| str = PyUnicode_FromString("abc"); | |
| if (str == NULL) | |
| return NULL; | |
| set_clear_internal(so); | |
| if (set_update_internal(so, str)) { | |
| Py_DECREF(str); | |
| return NULL; | |
| } | |
| Py_DECREF(str); | |
| /* Exercise type/size checks */ | |
| assert(PySet_Size(ob) == 3); | |
| assert(PySet_GET_SIZE(ob) == 3); | |
| /* Raise TypeError for non-iterable constructor arguments */ | |
| assertRaises(PySet_New(Py_None) == NULL, PyExc_TypeError); | |
| assertRaises(PyFrozenSet_New(Py_None) == NULL, PyExc_TypeError); | |
| /* Raise TypeError for unhashable key */ | |
| dup = PySet_New(ob); | |
| assertRaises(PySet_Discard(ob, dup) == -1, PyExc_TypeError); | |
| assertRaises(PySet_Contains(ob, dup) == -1, PyExc_TypeError); | |
| assertRaises(PySet_Add(ob, dup) == -1, PyExc_TypeError); | |
| /* Exercise successful pop, contains, add, and discard */ | |
| elem = PySet_Pop(ob); | |
| assert(PySet_Contains(ob, elem) == 0); | |
| assert(PySet_GET_SIZE(ob) == 2); | |
| assert(PySet_Add(ob, elem) == 0); | |
| assert(PySet_Contains(ob, elem) == 1); | |
| assert(PySet_GET_SIZE(ob) == 3); | |
| assert(PySet_Discard(ob, elem) == 1); | |
| assert(PySet_GET_SIZE(ob) == 2); | |
| assert(PySet_Discard(ob, elem) == 0); | |
| assert(PySet_GET_SIZE(ob) == 2); | |
| /* Exercise clear */ | |
| dup2 = PySet_New(dup); | |
| assert(PySet_Clear(dup2) == 0); | |
| assert(PySet_Size(dup2) == 0); | |
| Py_DECREF(dup2); | |
| /* Raise SystemError on clear or update of frozen set */ | |
| f = PyFrozenSet_New(dup); | |
| assertRaises(PySet_Clear(f) == -1, PyExc_SystemError); | |
| assertRaises(_PySet_Update(f, dup) == -1, PyExc_SystemError); | |
| assert(PySet_Add(f, elem) == 0); | |
| Py_INCREF(f); | |
| assertRaises(PySet_Add(f, elem) == -1, PyExc_SystemError); | |
| Py_DECREF(f); | |
| Py_DECREF(f); | |
| /* Exercise direct iteration */ | |
| i = 0, count = 0; | |
| while (_PySet_NextEntry((PyObject *)dup, &i, &x, &hash)) { | |
| s = PyUnicode_AsUTF8(x); | |
| assert(s && (s[0] == 'a' || s[0] == 'b' || s[0] == 'c')); | |
| count++; | |
| } | |
| assert(count == 3); | |
| /* Exercise updates */ | |
| dup2 = PySet_New(NULL); | |
| assert(_PySet_Update(dup2, dup) == 0); | |
| assert(PySet_Size(dup2) == 3); | |
| assert(_PySet_Update(dup2, dup) == 0); | |
| assert(PySet_Size(dup2) == 3); | |
| Py_DECREF(dup2); | |
| /* Raise SystemError when self argument is not a set or frozenset. */ | |
| t = PyTuple_New(0); | |
| assertRaises(PySet_Size(t) == -1, PyExc_SystemError); | |
| assertRaises(PySet_Contains(t, elem) == -1, PyExc_SystemError); | |
| Py_DECREF(t); | |
| /* Raise SystemError when self argument is not a set. */ | |
| f = PyFrozenSet_New(dup); | |
| assert(PySet_Size(f) == 3); | |
| assert(PyFrozenSet_CheckExact(f)); | |
| assertRaises(PySet_Discard(f, elem) == -1, PyExc_SystemError); | |
| assertRaises(PySet_Pop(f) == NULL, PyExc_SystemError); | |
| Py_DECREF(f); | |
| /* Raise KeyError when popping from an empty set */ | |
| assert(PyNumber_InPlaceSubtract(ob, ob) == ob); | |
| Py_DECREF(ob); | |
| assert(PySet_GET_SIZE(ob) == 0); | |
| assertRaises(PySet_Pop(ob) == NULL, PyExc_KeyError); | |
| /* Restore the set from the copy using the PyNumber API */ | |
| assert(PyNumber_InPlaceOr(ob, dup) == ob); | |
| Py_DECREF(ob); | |
| /* Verify constructors accept NULL arguments */ | |
| f = PySet_New(NULL); | |
| assert(f != NULL); | |
| assert(PySet_GET_SIZE(f) == 0); | |
| Py_DECREF(f); | |
| f = PyFrozenSet_New(NULL); | |
| assert(f != NULL); | |
| assert(PyFrozenSet_CheckExact(f)); | |
| assert(PySet_GET_SIZE(f) == 0); | |
| Py_DECREF(f); | |
| Py_DECREF(elem); | |
| Py_DECREF(dup); | |
| Py_RETURN_TRUE; | |
| } | |
| #undef assertRaises | |
| #endif |
There are several things that I can see as problematic:
- It is stored together with the production code
- These tests are mixing CAPI (
PySet_Check), internal CAPI (_PySet_Update), and internal function calls (set_clear_internal) - These tests are hard to parametrize since they are called as
Lines 638 to 641 in 7e30821
| @unittest.skipUnless(hasattr(set, "test_c_api"), | |
| 'C API test only available in a debug build') | |
| def test_c_api(self): | |
| self.assertEqual(set().test_c_api(), True) |
- Multiple things are not tested: like
setandfrozensetsubclasses - Test failures are not quite informative, because using C
assert - This is a single huge test
- This test is only run under debug builds and not under "release" builds
I propose a plan to improve it:
- Move CAPI tests to
Modules/_testcapi/set.cand addLib/test/test_capi/test_set.py - Move internal CAPI tests
Modules/_testinternalcapi/set.cand make sure that they are executed intest_capi.py - Delete
test_c_apifromsetobject.c
I plan to work on this and already have the PR for 1. :)
Linked PRs
- gh-110525: Add CAPI tests for
setandfrozensetobjects #110526 - gh-110525: Cover
PySet_Addcorner case withfrozensetobjects #110544 - [3.12] gh-110525: Add CAPI tests for set and frozenset objects (GH-110526). #110547
- [3.12] gh-110525: Cover PySet_Add corner case with frozenset objects (GH-110544) #110554
- gh-110525: Add tests for internal
setCAPI #110630 - gh-110525: Delete
test_c_apimethod fromsetobject #110688
serhiy-storchaka
Metadata
Metadata
Assignees
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)testsTests in the Lib/test dirTests in the Lib/test dirtopic-C-APItype-featureA feature request or enhancementA feature request or enhancement