Skip to content

gh-110525: Add CAPI tests for set and frozenset objects #110526

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

Merged
serhiy-storchaka merged 4 commits into from
Oct 9, 2023
Merged

gh-110525: Add CAPI tests for set and frozenset objects #110526

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf13ca0
gh-110525: Add CAPI tests for `set` and `frozenset` objects
sobolevn Oct 8, 2023
314a7b3
Fix Win builds
sobolevn Oct 8, 2023
9feda28
Address review
sobolevn Oct 9, 2023
9b8f7a5
Address review
sobolevn Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address review
  • Loading branch information
@sobolevn
sobolevn committed Oct 9, 2023
commit 9feda284557142ab60d2564b5de0cba042a57025
144 changes: 91 additions & 53 deletions Lib/test/test_capi/test_set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,139 +5,163 @@
# Skip this test if the _testcapi module isn't available.
_testcapi = import_helper.import_module('_testcapi')

class set_child(set):
class set_subclass(set):
pass

class frozenset_child(frozenset):
class frozenset_subclass(frozenset):
pass


class TestSetCAPI(unittest.TestCase):
def assertImmutable(self, action, *args):
self.assertRaises(SystemError, action, frozenset(), *args)
Copy link
Copy Markdown
Member

@vstinner vstinner Oct 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemError? That's a surprising error. Usually, it's used when the C API is misused, like passing NULL or the wrong type. frozenset is a "wrong type" here?

Copy link
Copy Markdown
Member Author

@sobolevn sobolevn Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, immutable frozenset is a wrong type for mutation-based functions :)

self.assertRaises(SystemError, action, frozenset({1}), *args)
self.assertRaises(SystemError, action, frozenset_child(), *args)
self.assertRaises(SystemError, action, frozenset_child({1}), *args)
self.assertRaises(SystemError, action, frozenset_subclass(), *args)
self.assertRaises(SystemError, action, frozenset_subclass({1}), *args)

def test_set_check(self):
check = _testcapi.set_check
self.assertTrue(check(set()))
self.assertTrue(check({1, 2}))
self.assertFalse(check(frozenset()))
self.assertTrue(check(set_child()))
self.assertFalse(check(frozenset_child()))
self.assertTrue(check(set_subclass()))
self.assertFalse(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_set_check_exact(self):
check = _testcapi.set_checkexact
self.assertTrue(check(set()))
self.assertTrue(check({1, 2}))
self.assertFalse(check(frozenset()))
self.assertFalse(check(set_child()))
self.assertFalse(check(frozenset_child()))
self.assertFalse(check(set_subclass()))
self.assertFalse(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_frozenset_check(self):
check = _testcapi.frozenset_check
self.assertFalse(check(set()))
self.assertTrue(check(frozenset()))
self.assertTrue(check(frozenset({1, 2})))
self.assertFalse(check(set_child()))
self.assertTrue(check(frozenset_child()))
self.assertFalse(check(set_subclass()))
self.assertTrue(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_frozenset_check_exact(self):
check = _testcapi.frozenset_checkexact
self.assertFalse(check(set()))
self.assertTrue(check(frozenset()))
self.assertTrue(check(frozenset({1, 2})))
self.assertFalse(check(set_child()))
self.assertFalse(check(frozenset_child()))
self.assertFalse(check(set_subclass()))
self.assertFalse(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_anyset_check(self):
check = _testcapi.anyset_check
self.assertTrue(check(set()))
self.assertTrue(check({1, 2}))
self.assertTrue(check(frozenset()))
self.assertTrue(check(frozenset({1, 2})))
self.assertTrue(check(set_child()))
self.assertTrue(check(frozenset_child()))
self.assertTrue(check(set_subclass()))
self.assertTrue(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_anyset_check_exact(self):
check = _testcapi.anyset_checkexact
self.assertTrue(check(set()))
self.assertTrue(check({1, 2}))
self.assertTrue(check(frozenset()))
self.assertTrue(check(frozenset({1, 2})))
self.assertFalse(check(set_child()))
self.assertFalse(check(frozenset_child()))
self.assertFalse(check(set_subclass()))
self.assertFalse(check(frozenset_subclass()))
self.assertFalse(check(object()))
# CRASHES: check(NULL)

def test_set_new(self):
new = _testcapi.set_new
self.assertEqual(new().__class__, set)
self.assertEqual(new(), set())
self.assertEqual(new((1, 1, 2)), {1, 2})
set_new = _testcapi.set_new
self.assertEqual(set_new().__class__, set)
self.assertEqual(set_new(), set())
self.assertEqual(set_new((1, 1, 2)), {1, 2})
with self.assertRaisesRegex(TypeError, 'object is not iterable'):
new(object())
set_new(object())
with self.assertRaisesRegex(TypeError, 'object is not iterable'):
set_new(None)
Comment thread
sobolevn marked this conversation as resolved.
Outdated
with self.assertRaisesRegex(TypeError, "unhashable type: 'dict'"):
new((1, {}))
set_new((1, {}))

def test_frozenset_new(self):
new = _testcapi.frozenset_new
self.assertEqual(new().__class__, frozenset)
self.assertEqual(new(), frozenset())
self.assertEqual(new((1, 1, 2)), frozenset({1, 2}))
frozenset_new = _testcapi.frozenset_new
self.assertEqual(frozenset_new().__class__, frozenset)
self.assertEqual(frozenset_new(), frozenset())
self.assertEqual(frozenset_new((1, 1, 2)), frozenset({1, 2}))
with self.assertRaisesRegex(TypeError, 'object is not iterable'):
frozenset_new(object())
with self.assertRaisesRegex(TypeError, 'object is not iterable'):
new(object())
frozenset_new(None)
with self.assertRaisesRegex(TypeError, "unhashable type: 'dict'"):
new((1, {}))
frozenset_new((1, {}))

def test_set_size(self):
l = _testcapi.set_size
self.assertEqual(l(set()), 0)
self.assertEqual(l(frozenset()), 0)
self.assertEqual(l({1, 1, 2}), 2)
self.assertEqual(l(frozenset({1, 1, 2})), 2)
self.assertEqual(l(set_child((1, 2, 3))), 3)
self.assertEqual(l(frozenset_child((1, 2, 3))), 3)
get_size = _testcapi.set_size
self.assertEqual(get_size(set()), 0)
self.assertEqual(get_size(frozenset()), 0)
self.assertEqual(get_size({1, 1, 2}), 2)
self.assertEqual(get_size(frozenset({1, 1, 2})), 2)
self.assertEqual(get_size(set_subclass((1, 2, 3))), 3)
self.assertEqual(get_size(frozenset_subclass((1, 2, 3))), 3)
with self.assertRaises(SystemError):
l([])
get_size([])
# CRASHES: get_size(NULL)

def test_set_get_size(self):
l = _testcapi.set_get_size
self.assertEqual(l(set()), 0)
self.assertEqual(l(frozenset()), 0)
self.assertEqual(l({1, 1, 2}), 2)
self.assertEqual(l(frozenset({1, 1, 2})), 2)
self.assertEqual(l(set_child((1, 2, 3))), 3)
self.assertEqual(l(frozenset_child((1, 2, 3))), 3)
# CRASHES: l([])
get_size = _testcapi.set_get_size
self.assertEqual(get_size(set()), 0)
self.assertEqual(get_size(frozenset()), 0)
self.assertEqual(get_size({1, 1, 2}), 2)
self.assertEqual(get_size(frozenset({1, 1, 2})), 2)
self.assertEqual(get_size(set_subclass((1, 2, 3))), 3)
self.assertEqual(get_size(frozenset_subclass((1, 2, 3))), 3)
# CRASHES: get_size(NULL)
# CRASHES: get_size(object())
Comment thread
sobolevn marked this conversation as resolved.

def test_set_contains(self):
c = _testcapi.set_contains
for cls in (set, frozenset, set_child, frozenset_child):
contains = _testcapi.set_contains
for cls in (set, frozenset, set_subclass, frozenset_subclass):
with self.subTest(cls=cls):
instance = cls((1, 2))
self.assertTrue(c(instance, 1))
self.assertFalse(c(instance, 'missing'))
self.assertTrue(contains(instance, 1))
self.assertFalse(contains(instance, 'missing'))
with self.assertRaisesRegex(TypeError, "unhashable type: 'list'"):
contains(instance, [])
# CRASHES: contains(instance, NULL)
# CRASHES: contains(NULL, object())
# CRASHES: contains(NULL, NULL)
Comment thread
sobolevn marked this conversation as resolved.
Outdated

def test_add(self):
add = _testcapi.set_add
for cls in (set, set_child):
for cls in (set, set_subclass):
with self.subTest(cls=cls):
instance = cls((1, 2))
self.assertEqual(add(instance, 1), 0)
self.assertEqual(instance, {1, 2})
self.assertEqual(add(instance, 3), 0)
self.assertEqual(instance, {1, 2, 3})
with self.assertRaisesRegex(TypeError, "unhashable type: 'list'"):
add(instance, [])
# CRASHES: add(NULL, object())
# CRASHES: add(instance, NULL)
# CRASHES: add(NULL, NULL)
Comment thread
sobolevn marked this conversation as resolved.
Outdated
with self.assertRaises(SystemError):
add(object(), 1)
self.assertImmutable(add, 1)

def test_discard(self):
discard = _testcapi.set_discard
for cls in (set, set_child):
for cls in (set, set_subclass):
with self.subTest(cls=cls):
instance = cls((1, 2))
self.assertEqual(discard(instance, 3), 0)
Expand All @@ -146,15 +170,21 @@ def test_discard(self):
self.assertEqual(instance, {2})
self.assertEqual(discard(instance, 2), 1)
self.assertEqual(instance, set())
# Discarding from empty set works
self.assertEqual(discard(instance, 2), 0)
self.assertEqual(instance, set())
with self.assertRaisesRegex(TypeError, "unhashable type: 'list'"):
discard(instance, [])
# CRASHES: discard(NULL, object())
# CRASHES: discard(instance, NULL)
# CRASHES: discard(NULL, NULL)
with self.assertRaises(SystemError):
discard(object(), 1)
self.assertImmutable(discard, 1)

def test_pop(self):
pop = _testcapi.set_pop
orig = (1, 2)
for cls in (set, set_child):
for cls in (set, set_subclass):
with self.subTest(cls=cls):
instance = cls(orig)
self.assertIn(pop(instance), orig)
Expand All @@ -163,13 +193,21 @@ def test_pop(self):
self.assertEqual(len(instance), 0)
with self.assertRaises(KeyError):
pop(instance)
# CRASHES: pop(NULL)
with self.assertRaises(SystemError):
pop(object())
self.assertImmutable(pop)

def test_clear(self):
clear = _testcapi.set_clear
for cls in (set, set_child):
for cls in (set, set_subclass):
with self.subTest(cls=cls):
instance = cls((1, 2))
self.assertEqual(clear(instance), 0)
self.assertEqual(instance, set())
self.assertEqual(clear(instance), 0)
self.assertEqual(instance, set())
# CRASHES: clear(NULL)
with self.assertRaises(SystemError):
clear(object())
self.assertImmutable(clear)
6 changes: 6 additions & 0 deletions Modules/_testcapi/set.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,42 @@
static PyObject *
set_check(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PySet_Check(obj));
}

static PyObject *
set_checkexact(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PySet_CheckExact(obj));
}

static PyObject *
frozenset_check(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PyFrozenSet_Check(obj));
}

static PyObject *
frozenset_checkexact(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PyFrozenSet_CheckExact(obj));
}

static PyObject *
anyset_check(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PyAnySet_Check(obj));
}

static PyObject *
anyset_checkexact(PyObject *self, PyObject *obj)
{
NULLABLE(obj);
RETURN_INT(PyAnySet_CheckExact(obj));
}

Expand Down