-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-130149: refactor tests for HMAC #130150
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
Conversation
- Use `_hashlib` instead of `_hashopenssl` naming. - Use `_hashlib.*` instead of top-level imports. - Consistently use new hashlib helper guards.
This PR won't work against refleaks due to #130151. |
🤖 New build scheduled with the buildbot fleet by @picnixz for commit 644f577 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130150%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
🤖 New build scheduled with the buildbot fleet by @picnixz for commit e71f03d 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130150%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
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.
While I'm not a fan of a giant maze of mixin classes from a code clarify point of view (nobody reading code can rationally follow a non-trivial class hierarchy MRO graph in their head)... this refactor should actually work and allow for easier testing of our too many possible implementation choices - and I don't have meaningfully better ideas for that that'd work within the stdlib test suite.
One factory is for test cases when the digestmod parameter is missing; the other is for test cases when the digestmod parameter is unknown.
To ease usage in the future (although you or I will be the one reading this one with IntelliSense from IDE), I've added comments |
I previously allowed it to be None for future compatibility, but I think it's better to construct a fake hash function instead, or
Arf, again I forgot about the build bots that don't have the built-in implementations. I'll do something for that so that we can cleanly skip the test. |
Since we plan to introduce a built-in implementation for HMAC based on HACL*, it becomes important for the HMAC tests to be flexible enough to avoid code duplication. In addition to the new layout based on mixin classes, we extend test coverage by also testing the `__repr__` of HMAC objects and the HMAC one-shot functions. We also fix the import to `_sha256` which, since pythongh-101924, resulted in some tests being skipped as the module is no more available (its content was moved to the `_sha2` module).
repr()
of HMAC OpenSSL objects properly.