-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
gh-137463: Update validate_abstract_methods
in test_collections.py
#137464
Conversation
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.
This comment was marked as resolved.
This comment was marked as resolved.
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.
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__') |
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.
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__', |
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.
__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__', |
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.
And __iter__
here is also not abstract.
Thanks @guilhermeleobas for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…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>
Sorry, @guilhermeleobas and @sobolevn, I could not cleanly backport this to
|
GH-137503 is a backport of this pull request to the 3.14 branch. |
@guilhermeleobas can you please do the manual backport to 3.13? :) |
Sure! Thanks for the review. |
GH-137516 is a backport of this pull request to the 3.13 branch. |
GH-137521 is a backport of this pull request to the 3.13 branch. |
…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)
…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)
The test for missing abstract methods in
validate_abstract_methods
incorrectly attempted to instantiate the generated classC
with an argument (C(name)
), which always raises aTypeError: 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.