Remove raw asserts in test_typing.py #104951
Conversation
JelleZijlstra
added
skip issue
skip news
needs backport to 3.11
JelleZijlstra
requested review from
gvanrossum,
Fidget-Spinner and
AlexWaygood
as code owners
Lib/test/test_typing.py
Outdated
| self.assertEqual(Point2Dor3D.__required_keys__, frozenset(['x', 'y'])) | ||
| self.assertEqual(Point2Dor3D.__optional_keys__, frozenset(['z'])) |
There was a problem hiding this comment.
The changes in this PR look good. But it looks like we have a lot of tests in this test class that do things like self.assertEqual(some_frozenset, frozenset(['x', 'y'])). That looks like it's checking that some_frozenset is a frozenset, but it's actually not, because frozensets and sets with the same members compare equal. So I wonder if we should be doing this:
| self.assertEqual(Point2Dor3D.__required_keys__, frozenset(['x', 'y'])) | |
| self.assertEqual(Point2Dor3D.__optional_keys__, frozenset(['z'])) | |
| self.assertEqual(Point2Dor3D.__required_keys__, {'x', 'y'}) | |
| self.assertIsInstance(Point2Dor3D.__required_keys__, frozenset) | |
| self.assertEqual(Point2Dor3D.__optional_keys__, {'z'}) | |
| self.assertIsInstance(Point2Dor3D.__optional_keys__, frozenset) |
But maybe we don't need to assert the type of __required_keys__ every time we want to check the contents of the set.
There was a problem hiding this comment.
Anyway, we don't need to change that in this PR; the changes here are definitely good.
There was a problem hiding this comment.
Might as well do that in this PR too.
There was a problem hiding this comment.
Pushed a few new asserts checking that it's really a frozenset
|
Thanks @JelleZijlstra for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
(cherry picked from commit 2cb4456) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
GH-104978 is a backport of this pull request to the 3.12 branch. |
|
GH-104979 is a backport of this pull request to the 3.11 branch. |
(cherry picked from commit 2cb4456) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
No description provided.