GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-111495: Add tests for PyList C API by rawwar · Pull Request #111562 · python/cpython Conversation
|
|
@serhiy-storchaka , Is adding Fails with But, when I include |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Signed-off-by: kalyanr <[email protected]>
Signed-off-by: kalyanr <[email protected]>
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| 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.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Signed-off-by: kalyanr <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Signed-off-by: kalyanr <[email protected]>
| } | ||
| NULLABLE(obj); | ||
| NULLABLE(value); | ||
| RETURN_INT(PyList_SetSlice(obj, ilow, ihigh, Py_XNewRef(value))); |
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| } | ||
| NULLABLE(obj); | ||
| NULLABLE(value); | ||
| RETURN_INT(PyList_Append(obj, Py_XNewRef(value))); |
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| self.assertRaises(TypeError, setitem, lst, 1.5, 10) | ||
| self.assertRaises(TypeError, setitem, 23, 'a', 5) |
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I merged your PR @rawwar, thanks for adding tests for the PyList API! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
(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]>
|
GH-111861 is a backport of this pull request to the 3.12 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
(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]>
Signed-off-by: kalyanr <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.