feat: add __eq__ for Key and KeySet
#38
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
=======================================
Coverage 99.84% 99.84%
=======================================
Files 44 44
Lines 2606 2613 +7
Branches 306 307 +1
=======================================
+ Hits 2602 2609 +7
Misses 2 2
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
|
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
| @@ -120,6 +120,10 @@ def __iter__(self) -> t.Iterator[Key]: | |||
| def __bool__(self) -> bool: | |||
| return bool(self.keys) | |||
|
|
|||
| def __eq__(self, other: t.Any) -> bool: | |||
| assert isinstance(other, KeySet) | |||
There was a problem hiding this comment.
This can raise an AssertionError not return a bool
I think it's usually preferred to avoid assert outside of tests because in certain circumstances it can be removed by optimization flags.
There was a problem hiding this comment.
Separately: isinstance() checks can sometimes conflict with ducktyping. This answer suggests returning NotImplemented to give the other object a chance to check its own notion of equality.
There was a problem hiding this comment.
The assert is used to fix mypy issues
There was a problem hiding this comment.
What happens if you do KeySet([]) == object()? Does it throw an AssertionError or return False? I don't see any tests covering comparison against non-KeySet objects.
| @@ -76,3 +76,18 @@ def test_key_set_iter(self): | |||
| key_set = KeySet.generate_key_set('RSA', 2048) | |||
| for k in key_set: | |||
| self.assertEqual(k.key_type, "RSA") | |||
|
|
|||
| def test_key_eq_with_same_keys(self): | |||
| key_set1 = KeySet.generate_key_set('RSA', 2048) | |||
There was a problem hiding this comment.
Nitpick but generating all these key/sets repeatedly in every test is inefficient in terms of wall/CPU time and energy.
Consider using setUpClass() or module-scoped Pytest fixtures to make the tests run faster / be kinder to your (+ everyone else's) computer and the planet.
Replacing: #37
@bjmc