Skip to content

gh-137463: Update validate_abstract_methods in test_collections.py #137464

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

Conversation

guilhermeleobas
Copy link
Contributor

@guilhermeleobas guilhermeleobas commented Aug 6, 2025

The test for missing abstract methods in validate_abstract_methods incorrectly attempted to instantiate the generated class C with an argument (C(name)), which always raises a TypeError: C() takes no arguments. Although the test originally passes, it passes for the wrong reason.

This change makes the test correctly validate the enforcement of abstract methods in ABCs.

The test for missing abstract methods in `validate_abstract_methods` incorrectly attempted to instantiate the generated class `C` with an argument (`C(name)`), which always raises a `TypeError: C() takes no arguments`. Although the test originally passes, it passes for the wrong reason.

This change makes the test correctly validate the enforcement of abstract methods in ABCs.
@python-cla-bot
Copy link

python-cla-bot bot commented Aug 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app bot added awaiting review tests Tests in the Lib/test dir labels Aug 6, 2025
@bedevere-app

This comment was marked as resolved.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Great catch!

@@ -963,7 +963,7 @@ class AnextOnly:
async def __anext__(self):
raise StopAsyncIteration
self.assertNotIsInstance(AnextOnly(), AsyncIterator)
self.validate_abstract_methods(AsyncIterator, '__anext__', '__aiter__')
self.validate_abstract_methods(AsyncIterator, '__anext__')
Copy link
Member

Choose a reason for hiding this comment

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

Since AsyncIterator.__aiter__ is not abstract, it is correct to remove it here.

@@ -1859,7 +1858,7 @@ def test_MutableMapping(self):
for sample in [dict]:
self.assertIsInstance(sample(), MutableMapping)
self.assertIsSubclass(sample, MutableMapping)
self.validate_abstract_methods(MutableMapping, '__contains__', '__iter__', '__len__',
self.validate_abstract_methods(MutableMapping, '__iter__', '__len__',
Copy link
Member

Choose a reason for hiding this comment

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

__contains__ is also not abstract, except for Container.__contains__

@@ -1954,8 +1952,8 @@ def test_MutableSequence(self):
self.assertIsSubclass(sample, MutableSequence)
self.assertIsSubclass(array.array, MutableSequence)
self.assertNotIsSubclass(str, MutableSequence)
self.validate_abstract_methods(MutableSequence, '__contains__', '__iter__',
'__len__', '__getitem__', '__setitem__', '__delitem__', 'insert')
self.validate_abstract_methods(MutableSequence, '__len__', '__getitem__',
Copy link
Member

Choose a reason for hiding this comment

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

And __iter__ here is also not abstract.

@sobolevn sobolevn added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Aug 7, 2025
@sobolevn sobolevn merged commit 5be8723 into python:main Aug 7, 2025
54 checks passed
@miss-islington-app
Copy link

Thanks @guilhermeleobas for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 7, 2025
…ons.py` (pythonGH-137464)

Update `validate_abstract_methods` in `test_collections.py`

The test for missing abstract methods in `validate_abstract_methods` incorrectly attempted to instantiate the generated class `C` with an argument (`C(name)`), which always raises a `TypeError: C() takes no arguments`. Although the test originally passes, it passes for the wrong reason.

This change makes the test correctly validate the enforcement of abstract methods in ABCs.
(cherry picked from commit 5be8723)

Co-authored-by: Guilherme Leobas <guilhermeleobas@gmail.com>
@miss-islington-app
Copy link

Sorry, @guilhermeleobas and @sobolevn, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 5be872350d562e6c9987b09ff4b7bda80a2f9cd0 3.13

@bedevere-app
Copy link

bedevere-app bot commented Aug 7, 2025

GH-137503 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Aug 7, 2025
@sobolevn
Copy link
Member

sobolevn commented Aug 7, 2025

@guilhermeleobas can you please do the manual backport to 3.13? :)
This is your first PR (congrats 🎉), so, here's how to do it: https://pypi.org/project/cherry-picker/

@guilhermeleobas
Copy link
Contributor Author

Sure! Thanks for the review.

@bedevere-app
Copy link

bedevere-app bot commented Aug 7, 2025

GH-137516 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 7, 2025
@bedevere-app
Copy link

bedevere-app bot commented Aug 7, 2025

GH-137521 is a backport of this pull request to the 3.13 branch.

guilhermeleobas added a commit to guilhermeleobas/cpython that referenced this pull request Aug 7, 2025
…ons.py` (python#137464)

Update `validate_abstract_methods` in `test_collections.py`

The test for missing abstract methods in `validate_abstract_methods` incorrectly attempted to instantiate the generated class `C` with an argument (`C(name)`), which always raises a `TypeError: C() takes no arguments`. Although the test originally passes, it passes for the wrong reason.

This change makes the test correctly validate the enforcement of abstract methods in ABCs.

(cherry picked from commit 5be8723)
sobolevn pushed a commit that referenced this pull request Aug 7, 2025
…s.py (GH-137464) (#137521)

gh-137463: Update `validate_abstract_methods` in `test_collections.py` (#137464)

Update `validate_abstract_methods` in `test_collections.py`

The test for missing abstract methods in `validate_abstract_methods` incorrectly attempted to instantiate the generated class `C` with an argument (`C(name)`), which always raises a `TypeError: C() takes no arguments`. Although the test originally passes, it passes for the wrong reason.

This change makes the test correctly validate the enforcement of abstract methods in ABCs.

(cherry picked from commit 5be8723)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants