Skip to content

Improve CAPI tests of set and frozenset #110525

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.

Sign up for GitHub

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
Closed
3 tasks done
sobolevn opened this issue Oct 8, 2023 · 0 comments
Closed
3 tasks done

Improve CAPI tests of set and frozenset #110525

sobolevn opened this issue Oct 8, 2023 · 0 comments
Assignees
@sobolevn
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) tests Tests in the Lib/test dir topic-C-API type-feature A feature request or enhancement

Comments

@sobolevn
Copy link
Member

sobolevn commented Oct 8, 2023 •
edited
Loading

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:

cpython/Objects/setobject.c

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

cpython/Lib/test/test_set.py

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 set and frozenset subclasses
  • 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.c and add Lib/test/test_capi/test_set.py
  • Move internal CAPI tests Modules/_testinternalcapi/set.c and make sure that they are executed in test_capi.py
  • Delete test_c_api from setobject.c

I plan to work on this and already have the PR for 1. :)

Linked PRs
@sobolevn sobolevn added type-feature A feature request or enhancement tests Tests in the Lib/test dir interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API labels Oct 8, 2023
@sobolevn sobolevn self-assigned this Oct 8, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 8, 2023
@sobolevn
pythongh-110525: Add CAPI tests for set and frozenset objects
ccd2255
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 8, 2023
@sobolevn
pythongh-110525: Add CAPI tests for set and frozenset objects
bf13ca0
@bedevere-app bedevere-app bot mentioned this issue Oct 8, 2023
Merged
serhiy-storchaka pushed a commit that referenced this issue Oct 9, 2023
@sobolevn
gh-110525: Add CAPI tests for set and frozenset objects (GH-110526)
c49edd7
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 9, 2023
@sobolevn
pythongh-110525: Cover PySet_Add corner case with frozenset objects
588b3e5
@bedevere-app bedevere-app bot mentioned this issue Oct 9, 2023
Merged
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 9, 2023
@sobolevn
[3.12] pythongh-110525: Add CAPI tests for set and frozenset objects (p…
90c2b08 
…ythonGH-110526).

(cherry picked from commit c49edd7)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-app bedevere-app bot mentioned this issue Oct 9, 2023
Merged
serhiy-storchaka pushed a commit that referenced this issue Oct 9, 2023
@sobolevn
[3.12] gh-110525: Add CAPI tests for set and frozenset objects (GH-11…
3ed5cb0 
…0526). (GH-110547)

(cherry picked from commit c49edd7)
serhiy-storchaka pushed a commit that referenced this issue Oct 9, 2023
@sobolevn
gh-110525: Cover PySet_Add corner case with frozenset objects (GH-110544
ea39c87 
)
@bedevere-app bedevere-app bot mentioned this issue Oct 9, 2023
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 9, 2023
@sobolevn @miss-islington
pythongh-110525: Cover PySet_Add corner case with frozenset objects (p…
de6f38f 
…ythonGH-110544)

(cherry picked from commit ea39c87)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
serhiy-storchaka pushed a commit that referenced this issue Oct 9, 2023
@miss-islington @sobolevn
[3.12] gh-110525: Cover PySet_Add corner case with frozenset objects (G…
b2419ef 
…H-110544) (GH-110554)

(cherry picked from commit ea39c87)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 10, 2023
@sobolevn
pythongh-110525: Add tests for internal set CAPI
f82ca46
@bedevere-app bedevere-app bot mentioned this issue Oct 10, 2023
Merged
serhiy-storchaka pushed a commit that referenced this issue Oct 10, 2023
@sobolevn
gh-110525: Add tests for internal set CAPI (GH-110630)
9cfb4e0
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 11, 2023
@sobolevn
pythongh-110525: Delete test_c_api method from set object
2221607
@bedevere-app bedevere-app bot mentioned this issue Oct 11, 2023
Merged
vstinner pushed a commit that referenced this issue Oct 13, 2023
@sobolevn
gh-110525: Delete test_c_api method from set object (#110688)
989a253
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@sobolevn @Glyphack
pythongh-110525: Add CAPI tests for set and frozenset objects (python…
2bce9fd 
…GH-110526)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@sobolevn @Glyphack
pythongh-110525: Cover PySet_Add corner case with frozenset objects (p…
d2893d4 
…ythonGH-110544)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@sobolevn @Glyphack
pythongh-110525: Add tests for internal set CAPI (pythonGH-110630)
570e655
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@sobolevn @Glyphack
pythongh-110525: Delete test_c_api method from set object (python…
6584a4c 
…#110688)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sobolevn sobolevn

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) tests Tests in the Lib/test dir topic-C-API type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@sobolevn