Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-111495: Add tests for PyList C API#111562
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
rawwar commented Oct 31, 2023 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: kalyanr <kalyan.ben10@live.com>
…o kalyan/test-capi-list Signed-off-by: kalyanr <kalyan.ben10@live.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…i-list Signed-off-by: kalyanr <kalyan.ben10@live.com>
rawwar commented Nov 1, 2023
@serhiy-storchaka , Is adding Fails with But, when I include |
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I wrote a very similar PR yesterday! I have a few more tests.
serhiy-storchaka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Lib/test/test_capi/test_list.py Outdated
| lst= [1, 2, NULL] | ||
| self.assertEqual(getitem(lst, 0), 1) | ||
| self.assertRaises(IndexError, getitem, lst, -1) | ||
| self.assertRaises(IndexError, getitem, lst, 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For methods like slice, they accept multiple Py_ssize_t parameters. Should each of these parameters be tested with the values you mentioned?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
rawwar commented Nov 1, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@vstinner , What can we do now? I'm not sure If I should continue working on this PR? Although, I also added tests for |
Signed-off-by: kalyanr <kalyan.ben10@live.com>
Modules/_testcapi/list.c Outdated
| } | ||
| NULLABLE(obj); | ||
| NULLABLE(value); | ||
| RETURN_INT(PyList_SetSlice(obj, ilow, ihigh, Py_XNewRef(value))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Py_XNewRef() is wrong and causes a reference leak.
Modules/_testcapi/list.c Outdated
| } | ||
| NULLABLE(obj); | ||
| NULLABLE(value); | ||
| RETURN_INT(PyList_Append(obj, Py_XNewRef(value))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Py_XNewRef() is wrong and causes a reference leak.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
serhiy-storchaka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lib/test/test_capi/test_list.py Outdated
| self.assertRaises(TypeError, setitem, lst, 1.5, 10) | ||
| self.assertRaises(TypeError, setitem, 23, 'a', 5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only tests the wrapper, not the C API function.
vstinner commented Nov 8, 2023
I merged your PR @rawwar, thanks for adding tests for the PyList API! |
(cherry picked from commit a3903c8) Co-authored-by: Kalyan <kalyan.ben10@live.com> Signed-off-by: kalyanr <kalyan.ben10@live.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
GH-111861 is a backport of this pull request to the 3.12 branch. |
(cherry picked from commit a3903c8) Signed-off-by: kalyanr <kalyan.ben10@live.com> Co-authored-by: Kalyan <kalyan.ben10@live.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
Signed-off-by: kalyanr <kalyan.ben10@live.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
Signed-off-by: kalyanr <kalyan.ben10@live.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
Adding tests for PyList C API - Issue #111495