gh-111495: Add tests for PyList C API #111562

rawwar · 2023-10-31T16:49:28Z

Adding tests for PyList C API - Issue #111495

Issue: Add more C API tests #111495

Signed-off-by: kalyanr <[email protected]>

…o kalyan/test-capi-list Signed-off-by: kalyanr <[email protected]>

Lib/test/test_capi/test_list.py

Modules/_testcapi/list.c

Lib/test/test_capi/test_list.py

…i-list Signed-off-by: kalyanr <[email protected]>

rawwar · 2023-11-01T07:46:42Z

@serhiy-storchaka , Is adding {NULL}, to test_methods[] necessary? yesterday, i was able to run make without any issues. Today, after taking the pull from main, its been failing. below are the details

static PyMethodDef test_methods[] = {
    {"list_check", list_check, METH_O},
    {"list_check_exact", list_check_exact, METH_O},
    {"list_new", list_new, METH_O},
    {"list_size", list_size, METH_O},
};

Fails with

(base) kalyan@rawwar:~/oss/python-projects/cpython$ make
gcc  -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall    -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include    -fPIC -c ./Modules/_testcapi/list.c -o Modules/_testcapi/list.o
gcc -shared      Modules/_testcapimodule.o Modules/_testcapi/vectorcall.o Modules/_testcapi/vectorcall_limited.o Modules/_testcapi/heaptype.o Modules/_testcapi/abstract.o Modules/_testcapi/bytearray.o Modules/_testcapi/bytes.o Modules/_testcapi/unicode.o Modules/_testcapi/dict.o Modules/_testcapi/set.o Modules/_testcapi/list.o Modules/_testcapi/tuple.o Modules/_testcapi/getargs.o Modules/_testcapi/datetime.o Modules/_testcapi/docstring.o Modules/_testcapi/mem.o Modules/_testcapi/watchers.o Modules/_testcapi/long.o Modules/_testcapi/float.o Modules/_testcapi/complex.o Modules/_testcapi/numbers.o Modules/_testcapi/structmember.o Modules/_testcapi/exceptions.o Modules/_testcapi/code.o Modules/_testcapi/buffer.o Modules/_testcapi/pyatomic.o Modules/_testcapi/pyos.o Modules/_testcapi/file.o Modules/_testcapi/codec.o Modules/_testcapi/immortal.o Modules/_testcapi/heaptype_relative.o Modules/_testcapi/gc.o Modules/_testcapi/sys.o   -o Modules/_testcapi.cpython-313-x86_64-linux-gnu.so
[ERROR] Importing extension '_testcapi' failed!
Traceback (most recent call last):
  File "/home/kalyan/oss/python-projects/cpython/./Tools/build/check_extension_modules.py", line 412, in check_module_import
    bootstrap_load(spec)
  File "<frozen importlib._bootstrap>", line 960, in _load
  File "<frozen importlib._bootstrap>", line 915, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 813, in module_from_spec
  File "<frozen importlib._bootstrap_external>", line 1302, in create_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
ValueError: module functions cannot set METH_CLASS or METH_STATIC
Traceback (most recent call last):
  File "/home/kalyan/oss/python-projects/cpython/./Tools/build/check_extension_modules.py", line 483, in <module>
    main()
  File "/home/kalyan/oss/python-projects/cpython/./Tools/build/check_extension_modules.py", line 474, in main
    checker.check()
  File "/home/kalyan/oss/python-projects/cpython/./Tools/build/check_extension_modules.py", line 171, in check
    self.check_module_import(modinfo)
  File "/home/kalyan/oss/python-projects/cpython/./Tools/build/check_extension_modules.py", line 412, in check_module_import
    bootstrap_load(spec)
  File "<frozen importlib._bootstrap>", line 960, in _load
  File "<frozen importlib._bootstrap>", line 915, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 813, in module_from_spec
  File "<frozen importlib._bootstrap_external>", line 1302, in create_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
ValueError: module functions cannot set METH_CLASS or METH_STATIC
make: *** [Makefile:1177: checksharedmods] Error 1

But, when I include {NULL} like below, I am able to run make without any issues:

static PyMethodDef test_methods[] = {
    {"list_check", list_check, METH_O},
    {"list_check_exact", list_check_exact, METH_O},
    {"list_new", list_new, METH_O},
    {"list_size", list_size, METH_O},
    {NULL},
};

Signed-off-by: kalyanr <[email protected]>

vstinner

Oh! I wrote a very similar PR yesterday! I have a few more tests.

serhiy-storchaka

See also #111582. It would be nice if you with @vstinner unite your works.

Lib/test/test_capi/test_list.py

serhiy-storchaka · 2023-11-01T09:54:54Z

Lib/test/test_capi/test_list.py

+        lst = [1, 2, NULL]
+        self.assertEqual(getitem(lst, 0), 1)
+        self.assertRaises(IndexError, getitem, lst, -1)
+        self.assertRaises(IndexError, getitem, lst, 10)


Add tests for index equal to the size of the list, PY_SSIZE_T_MIN, PY_SSIZE_T_MAX (imported from _testcapi).

All functions that have Py_ssize_t parameter should be tested with the following values: 0, size-1, -1, size, PY_SSIZE_T_MIN, PY_SSIZE_T_MAX.

For methods like slice, they accept multiple Py_ssize_t parameters. Should each of these parameters be tested with the values you mentioned?

Lib/test/test_capi/test_list.py

Modules/_testcapi/list.c

Signed-off-by: kalyanr <[email protected]>

Co-authored-by: Serhiy Storchaka <[email protected]>

rawwar · 2023-11-01T10:12:04Z

@vstinner , What can we do now? I'm not sure If I should continue working on this PR? Although, I also added tests for Py_LIMITED_API ones as well.
I am also planning on contributing tests for PyTuple. let me know if I can go ahead with that

Signed-off-by: kalyanr <[email protected]>

vstinner · 2023-11-08T14:51:55Z

Modules/_testcapi/list.c

+    }
+    NULLABLE(obj);
+    NULLABLE(value);
+    RETURN_INT(PyList_SetSlice(obj, ilow, ihigh, Py_XNewRef(value)));


Py_XNewRef() is wrong and causes a reference leak.

vstinner · 2023-11-08T14:52:09Z

Modules/_testcapi/list.c

+    }
+    NULLABLE(obj);
+    NULLABLE(value);
+    RETURN_INT(PyList_Append(obj, Py_XNewRef(value)));


Py_XNewRef() is wrong and causes a reference leak.

Modules/_testcapi/list.c

serhiy-storchaka

Thank you @rawwar.

I had too much minor suggestions, so I simply applied them myself.

Now it LGTM.

serhiy-storchaka · 2023-11-08T15:53:22Z

Lib/test/test_capi/test_list.py

+        self.assertRaises(TypeError, setitem, lst, 1.5, 10)
+        self.assertRaises(TypeError, setitem, 23, 'a', 5)


It only tests the wrapper, not the C API function.

vstinner · 2023-11-08T17:11:24Z

I merged your PR @rawwar, thanks for adding tests for the PyList API!

miss-islington-app · 2023-11-08T17:23:26Z

Thanks @rawwar for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

(cherry picked from commit a3903c8) Co-authored-by: Kalyan <[email protected]> Signed-off-by: kalyanr <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

bedevere-app · 2023-11-08T17:23:37Z

GH-111861 is a backport of this pull request to the 3.12 branch.

(cherry picked from commit a3903c8) Signed-off-by: kalyanr <[email protected]> Co-authored-by: Kalyan <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

Signed-off-by: kalyanr <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>

rawwar added 2 commits October 31, 2023 21:37

init test c file

d43d9b2

single test added

119cd0e

bedevere-app bot mentioned this pull request Oct 31, 2023

Add more C API tests #111495

Open

10 tasks

rawwar added 5 commits October 31, 2023 22:43

fix lint

ccf1bff

fix list check tests

8d17ca6

lint

bd9415d

add 2 tests

17d60d1

add test

322eab9

Eclips4 requested a review from serhiy-storchaka October 31, 2023 19:15

rawwar added 5 commits November 1, 2023 02:06

add getitem test

5c58022

test list_get_item

6b12f1f

add test for setitem

70e385f

add test for setitem

3166ba4

Signed-off-by: kalyanr <[email protected]>

Merge branch 'kalyan/test-capi-list' of github.com:rawwar/cpython int…

2a50fb8

…o kalyan/test-capi-list Signed-off-by: kalyanr <[email protected]>

serhiy-storchaka reviewed Oct 31, 2023

View reviewed changes

skirpichev mentioned this pull request Nov 1, 2023

gh-111495: add stub files for C API test modules #111586

Merged

Merge branch 'main' of github.com:rawwar/cpython into kalyan/test-cap…

1c30f3b

…i-list Signed-off-by: kalyanr <[email protected]>

serhiy-storchaka mentioned this pull request Nov 1, 2023

gh-111138: Test the public PyList C API #111582

Closed

rawwar added 2 commits November 1, 2023 14:28

add tests

392b3c6

Signed-off-by: kalyanr <[email protected]>

lint fix

eac8c67

Signed-off-by: kalyanr <[email protected]>

vstinner reviewed Nov 1, 2023

View reviewed changes

serhiy-storchaka reviewed Nov 1, 2023

View reviewed changes

rawwar and others added 5 commits November 1, 2023 15:36

add tests

0f40f6d

Signed-off-by: kalyanr <[email protected]>

Update Lib/test/test_capi/test_list.py

40e4022

Co-authored-by: Serhiy Storchaka <[email protected]>

Update Modules/_testcapi/list.c

d31fa57

Co-authored-by: Serhiy Storchaka <[email protected]>

Update Lib/test/test_capi/test_list.py

433351d

Co-authored-by: Serhiy Storchaka <[email protected]>

Update Lib/test/test_capi/test_list.py

34e915b

Co-authored-by: Serhiy Storchaka <[email protected]>

pr feedback fixes

0e340a1

Signed-off-by: kalyanr <[email protected]>

bedevere-app bot added the awaiting review label Nov 1, 2023

Update list.c

e486451

rawwar requested review from serhiy-storchaka and vstinner November 1, 2023 17:13

rawwar added 3 commits November 1, 2023 22:43

Merge branch 'main' into kalyan/test-capi-list

633951b

Merge branch 'main' into kalyan/test-capi-list

9afa50c

Merge branch 'main' into kalyan/test-capi-list

361b045

vstinner reviewed Nov 8, 2023

View reviewed changes

Modules/_testcapi/list.c Outdated Show resolved Hide resolved

Update Modules/_testcapi/list.c

ca07ac9

vstinner reviewed Nov 8, 2023

View reviewed changes

Modules/_testcapi/list.c Outdated Show resolved Hide resolved

vstinner and others added 3 commits November 8, 2023 15:57

Update Modules/_testcapi/list.c

dc039f1

Merge branch 'main' into kalyan/test-capi-list

5ecfa40

Minor fixes.

42a2a54

serhiy-storchaka approved these changes Nov 8, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Nov 8, 2023

vstinner added the skip news label Nov 8, 2023

vstinner merged commit a3903c8 into python:main Nov 8, 2023
31 checks passed

bedevere-app bot removed the awaiting merge label Nov 8, 2023

serhiy-storchaka added the needs backport to 3.12 bug and security fixes label Nov 8, 2023

bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Nov 8, 2023

vstinner mentioned this pull request Nov 8, 2023

C API: Consider adding a public PyList_Extend() function #111138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-111495: Add tests for PyList C API #111562

gh-111495: Add tests for PyList C API #111562

rawwar commented Oct 31, 2023 •

edited by bedevere-app bot

Loading

rawwar commented Nov 1, 2023

vstinner left a comment

serhiy-storchaka left a comment

serhiy-storchaka Nov 1, 2023

rawwar Nov 1, 2023

rawwar commented Nov 1, 2023 •

edited

Loading

vstinner Nov 8, 2023

vstinner Nov 8, 2023

serhiy-storchaka left a comment

serhiy-storchaka Nov 8, 2023

vstinner commented Nov 8, 2023

miss-islington-app bot commented Nov 8, 2023

bedevere-app bot commented Nov 8, 2023

		self.assertRaises(TypeError, setitem, lst, 1.5, 10)
		self.assertRaises(TypeError, setitem, 23, 'a', 5)

gh-111495: Add tests for PyList C API #111562

gh-111495: Add tests for PyList C API #111562

Conversation

rawwar commented Oct 31, 2023 • edited by bedevere-app bot Loading

rawwar commented Nov 1, 2023

vstinner left a comment

Choose a reason for hiding this comment

serhiy-storchaka left a comment

Choose a reason for hiding this comment

serhiy-storchaka Nov 1, 2023

Choose a reason for hiding this comment

rawwar Nov 1, 2023

Choose a reason for hiding this comment

rawwar commented Nov 1, 2023 • edited Loading

vstinner Nov 8, 2023

Choose a reason for hiding this comment

vstinner Nov 8, 2023

Choose a reason for hiding this comment

serhiy-storchaka left a comment

Choose a reason for hiding this comment

serhiy-storchaka Nov 8, 2023

Choose a reason for hiding this comment

vstinner commented Nov 8, 2023

miss-islington-app bot commented Nov 8, 2023

bedevere-app bot commented Nov 8, 2023

rawwar commented Oct 31, 2023 •

edited by bedevere-app bot

Loading

rawwar commented Nov 1, 2023 •

edited

Loading