Skip to content

gh-131652: remove duplicated bits in Lib/test/clinic.test.c #131653

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

Merged
merged 1 commit into from
Apr 25, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Mar 24, 2025

@picnixz picnixz marked this pull request as draft March 24, 2025 11:34
@picnixz picnixz force-pushed the cleanup/deadcode/clinic.test-131652 branch from f34169f to 115a388 Compare March 24, 2025 11:35
@picnixz picnixz marked this pull request as ready for review March 24, 2025 11:35
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

@picnixz picnixz requested a review from erlend-aasland April 4, 2025 13:15
@picnixz picnixz self-assigned this Apr 19, 2025
{"test_vararg_and_posonly", _PyCFunction_CAST(test_vararg_and_posonly), METH_FASTCALL, test_vararg_and_posonly__doc__},

static PyObject *
test_vararg_and_posonly_impl(PyObject *module, PyObject *a, Py_ssize_t nargs,
Copy link
Member Author

Choose a reason for hiding this comment

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

@skirpichev You added this one in 8c22eba but it already existed above. Can you explain what you wanted to test here please?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, but both functions existed before.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but maybe you knew why they were duplicated ...

@@ -5045,39 +5007,6 @@ static int
Test___init___impl(TestObj *self, PyObject *args)
/*[clinic end generated code: output=f172425cec373cd6 input=4b8388c4e6baab6f]*/

PyDoc_STRVAR(Test___init____doc__,
Copy link
Member Author

Choose a reason for hiding this comment

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

@erlend-aasland You added this back in ec45c51 but I don't know why there is a duplicated entry, one generated by clinic and one that is not. Note that the one generated by clinic has been changed since then while the non-clinic one hasn't change (and I think the docstring is now outdated for the clinic one).

@@ -5120,37 +5049,6 @@ static PyObject *
Test_impl(PyTypeObject *type, PyObject *args)
/*[clinic end generated code: output=ee1e8892a67abd4a input=a8259521129cad20]*/

PyDoc_STRVAR(Test__doc__,
Copy link
Member Author

Choose a reason for hiding this comment

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

Ditto for this block @erlend-aasland

{"test_critical_section_object", (PyCFunction)test_critical_section_object, METH_O, test_critical_section_object__doc__},

static PyObject *
test_critical_section_object_impl(PyObject *module, PyObject *a);
Copy link
Member Author

Choose a reason for hiding this comment

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

@corona10 you added this block in e52cc80. Could you explain why it was added? Ditto for the duplicated test_critical_section_object2_impl

Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

This seems reasonable, and tests are passing.

A

@picnixz
Copy link
Member Author

picnixz commented Apr 25, 2025

I'll merge this one. I'm almost 100% sure that the duplicated tests were there because we just duplicated other parts. Considering the whole file never has implementations being defined, I think it's safe to assume that those were just oversight.

@picnixz picnixz merged commit 9888f17 into python:main Apr 25, 2025
47 checks passed
@picnixz picnixz deleted the cleanup/deadcode/clinic.test-131652 branch April 25, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants