Skip to content

gh-111495: Add tests for PyTuple C API #118757

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
vstinner merged 24 commits into from
Aug 26, 2024
Merged

gh-111495: Add tests for PyTuple C API #118757

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0e590c4
gh-111495: Add tests for PyTuple C API
rawwar Nov 1, 2023
ddf0af7
+1
skirpichev May 8, 2024
571da31
Update Lib/test/test_capi/test_tuple.py
skirpichev May 9, 2024
69d9ead
address review: more tests
skirpichev May 9, 2024
3260ef4
address review: simplify tuple_pack()
skirpichev May 9, 2024
b5b4664
address review: strict checks in tuple_set_item/setitem
skirpichev May 9, 2024
1d53b0f
address review: redo resize() tests
skirpichev May 10, 2024
3248144
Merge branch 'master' into capi-tuple-tests
skirpichev Jun 17, 2024
8451a4d
address review:
skirpichev Jun 17, 2024
b2d0eac
address review: check error for PyTuple_SetItem()
skirpichev Jun 18, 2024
a977efa
address review: fix reference leak and add tests for non-immortal obj…
skirpichev Jun 18, 2024
95d1544
Add missing unittest.main() calls
skirpichev Jun 18, 2024
14b3594
address review: add tests with NULL for setitem/set_item
skirpichev Jun 18, 2024
f9554bc
address review: fix refleak
skirpichev Jun 18, 2024
e79616f
address review: rename tuple_resize
skirpichev Jun 18, 2024
26fc5a2
address review: a helper to test if item is NULL
skirpichev Jun 18, 2024
5047d6d
address review: check no refleaks if tuple shrinks
skirpichev Jun 18, 2024
a2ea4c4
address review: test added elements on resize
skirpichev Jun 18, 2024
72aa1ed
address review: test new elements
skirpichev Jun 18, 2024
f10160c
Merge branch 'main' into capi-tuple-tests
serhiy-storchaka Aug 8, 2024
8a6d636
Merge branch 'main' into capi-tuple-tests
skirpichev Aug 9, 2024
266de84
Polish and fix some errors.
serhiy-storchaka Aug 9, 2024
190e88a
Update Modules/_testcapi/tuple.c
serhiy-storchaka Aug 9, 2024
d25eae8
Address Victor's comments.
serhiy-storchaka Aug 9, 2024
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: fix reference leak and add tests for non-immortal obj… 
…ects
  • Loading branch information
@skirpichev
skirpichev committed Jun 18, 2024
commit a977efa0e674fbfc26c38777fc7b12f4500ae407
35 changes: 19 additions & 16 deletions Lib/test/test_capi/test_tuple.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def test_tuple_pack(self):
self.assertEqual(pack(0), ())
self.assertEqual(pack(1, 1), (1,))
self.assertEqual(pack(2, 1, 2), (1, 2))
self.assertEqual(pack(2, [1], 2), ([1], 2))

self.assertRaises(SystemError, pack, PY_SSIZE_T_MIN)
self.assertRaises(SystemError, pack, -1)
Expand Down Expand Up @@ -105,8 +106,9 @@ def test_tuple_getitem(self):
self.assertEqual(getitem(tup, 0), 1)
self.assertEqual(getitem(tup, 2), 3)

tup = (1, 2, 3)
tup = (1, [2], 3)
self.assertEqual(getitem(tup, 0), 1)
self.assertEqual(getitem(tup, 1), [2])
self.assertEqual(getitem(tup, 2), 3)

self.assertRaises(IndexError, getitem, tup, PY_SSIZE_T_MIN)
Expand All @@ -126,8 +128,9 @@ def test_tuple_get_item(self):
self.assertEqual(get_item(tup, 0), 1)
self.assertEqual(get_item(tup, 2), 3)

tup = (1, 2, 3)
tup = (1, [2], 3)
self.assertEqual(get_item(tup, 0), 1)
self.assertEqual(get_item(tup, 1), [2])
self.assertEqual(get_item(tup, 2), 3)

def test_tuple_getslice(self):
Expand All @@ -149,22 +152,22 @@ def test_tuple_getslice(self):
self.assertEqual(getslice(tup, 3, 0), ())

# slice
tup = (1, 2, 3)
self.assertEqual(getslice(tup, 1, 3), (2, 3))
tup = TupleSubclass((1, 2, 3))
self.assertEqual(getslice(tup, 1, 3), (2, 3))
tup = (1, [2], 3)
self.assertEqual(getslice(tup, 1, 3), ([2], 3))
tup = TupleSubclass((1, [2], 3))
self.assertEqual(getslice(tup, 1, 3), ([2], 3))

# whole
tup = (1, 2, 3)
tup = (1, [2], 3)
self.assertEqual(getslice(tup, 0, 3), tup)
self.assertEqual(getslice(tup, 0, 100), tup)
self.assertEqual(getslice(tup, -100, 100), tup)
tup = TupleSubclass((1, 2, 3))
tup = TupleSubclass((1, [2], 3))
self.assertEqual(getslice(tup, 0, 3), tup)
self.assertEqual(getslice(tup, 0, 100), tup)
self.assertEqual(getslice(tup, -100, 100), tup)

self.assertRaises(SystemError, getslice, [1, 2, 3], 0, 1)
self.assertRaises(SystemError, getslice, [1, [2], 3], 0, 1)
self.assertRaises(SystemError, getslice, 42, 0, 1)

# CRASHES getslice(NULL, 0, 0)
Comment thread
skirpichev marked this conversation as resolved.
Expand All @@ -173,9 +176,9 @@ def test_tuple_setitem(self):
# Test PyTuple_SetItem()
setitem = _testlimitedcapi.tuple_setitem

tup = (0, 0)
self.assertEqual(setitem(tup, 0, 1), (1, 0))
self.assertEqual(setitem(tup, 1, 1), (0, 1))
tup = (0, [0])
self.assertEqual(setitem(tup, 0, [1]), ([1], [0]))
self.assertEqual(setitem(tup, 1, [1]), (0, [1]))

self.assertRaises(IndexError, setitem, tup, PY_SSIZE_T_MIN, 1)
self.assertRaises(IndexError, setitem, tup, -1, 1)
Expand All @@ -190,9 +193,9 @@ def test_tuple_set_item(self):
# Test PyTuple_SET_ITEM()
set_item = _testcapi.tuple_set_item

tup = (0, 0)
self.assertEqual(set_item(tup, 0, 1), (1, 0))
self.assertEqual(set_item(tup, 1, 1), (0, 1))
tup = (0, [0])
self.assertEqual(set_item(tup, 0, [1]), ([1], [0]))
self.assertEqual(set_item(tup, 1, [1]), (0, [1]))

def test_tuple_resize(self):
# Test PyTuple_Resize()
Comment thread
skirpichev marked this conversation as resolved.
Outdated
Comment thread
skirpichev marked this conversation as resolved.
Outdated
Expand All @@ -204,7 +207,7 @@ def test_tuple_resize(self):
b = resize(a, 2)
self.assertEqual(len(b), 2)
Comment thread
serhiy-storchaka marked this conversation as resolved.

a = (1, 2, 3)
a = (1, [2], 3)
b = resize(a, 3)
self.assertEqual(b, a)
b = resize(a, 2)
Comment thread
skirpichev marked this conversation as resolved.
Expand Down
2 changes: 2 additions & 0 deletions Modules/_testcapi/tuple.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ tuple_set_item(PyObject *Py_UNUSED(module), PyObject *args)
NULLABLE(obj);
newtuple = obj;

@vstinner vstinner Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer to copy the tuple in both cases.

@serhiy-storchaka serhiy-storchaka Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How do you copy an instance of a tuple subclass? Or non-tuple? We need to test this API with tuple subclasses and non-tuples.

@vstinner vstinner Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't you have to INCREF obj here?

@serhiy-storchaka serhiy-storchaka Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not here, but before returning result. I fixed this in other way.

}
PyObject *val = PyTuple_GET_ITEM(newtuple, i);
PyTuple_SET_ITEM(newtuple, i, Py_XNewRef(value));
Comment thread
skirpichev marked this conversation as resolved.
Outdated
Py_DECREF(val);

@vstinner vstinner Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Py_DECREF(val);
Py_XDECREF(val);

@serhiy-storchaka serhiy-storchaka Aug 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not NULL.

return newtuple;
}

Expand Down