GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-130149: refactor tests for HMAC by picnixz · Pull Request #130150 · python/cpython

Conversation

@picnixz

@picnixz picnixz commented Feb 15, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • test the repr() of HMAC OpenSSL objects properly.
  • Issue: Refactor tests for HMAC #130149

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
picnixz added 12 commits February 13, 2025 14:26
@picnixz
add helper for _hashlib detection
d80b1b1
@picnixz
remove unused function ignore_warning
73b7756
@picnixz
cleanup whitespaces
fe7a300
@picnixz
clean imports and test guards
72f956f 
- Use `_hashlib` instead of `_hashopenssl` naming.
- Use `_hashlib.*` instead of top-level imports.
- Consistently use new hashlib helper guards.
@picnixz
Add mixins for interacting with HMAC
3e827a7
@picnixz
add mixin for RFC test cases
7f4fcc8
@picnixz
use RFC mixin classes
e47dd8b
@picnixz
refactor CompareDigestTestCase
acda122
@picnixz
refactor CopyTestCase
d2eb48a
@picnixz
refactor UpdateTestCase
105f6ce
@picnixz
refactor SanityTestCase
412f5c2
@picnixz
refactor ConstructorTestCase
c682945
@bedevere-app bedevere-app Bot mentioned this pull request Feb 15, 2025
Refactor tests for HMAC #130149
Closed
@picnixz
Merge branch 'main' into test/hmac/refactor-99108
7e62a58
@picnixz picnixz added the skip news label Feb 15, 2025
@picnixz
update comments
c04233d
@picnixz

picnixz commented Feb 15, 2025

Copy link
Copy Markdown
Member Author

This PR won't work against refleaks due to #130151.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
picnixz marked this pull request as ready for review February 15, 2025 12:29
@picnixz
picnixz requested review from gpshead and tiran as code owners February 15, 2025 12:29
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 15, 2025
@picnixz
extend coverage
fb7e577
@picnixz
picnixz marked this pull request as draft February 24, 2025 17:51
@bedevere-app bedevere-app Bot removed the awaiting core review label Feb 24, 2025
picnixz added 3 commits February 24, 2025 18:51
@picnixz
Merge branch 'main' into test/hmac/refactor-99108
168d018
@picnixz
add regression test for HMAC repr()
4177afa
@picnixz
change a bit the digestmod tests
644f577
@picnixz picnixz added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 1, 2025
@bedevere-bot

bedevere-bot commented Mar 1, 2025

Copy link
Copy Markdown

🤖 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 1, 2025
@picnixz
Merge branch 'main' into test/hmac/refactor-99108
e71f03d
@picnixz picnixz added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 2, 2025
@bedevere-bot

bedevere-bot commented Mar 2, 2025

Copy link
Copy Markdown

🤖 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 2, 2025
@picnixz
picnixz marked this pull request as ready for review March 2, 2025 12:00
@bedevere-app bedevere-app Bot added the awaiting core review label Mar 2, 2025
gpshead
gpshead approved these changes Mar 2, 2025
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Mar 2, 2025
picnixz added 7 commits March 3, 2025 09:37
@picnixz
remove mixin that is not yet needed
dbe3ce4
@picnixz
add comments
ca0822a
@picnixz
semantics: 'digest' -> 'hexdigest'
a0b3569
@picnixz
use __init_subclass__ instead of setUpClass
321a7a9
@picnixz
document and reorganize method order in DigestModTestCaseMixin
ca7851c
@picnixz
use separate factories for creating bad digestmod cases
a347ee5 
One factory is for test cases when the digestmod parameter is missing;
the other is for test cases when the digestmod parameter is unknown.
@picnixz
minor tweak on CompareDigestMixin
85364b4
@picnixz

picnixz commented Mar 3, 2025

Copy link
Copy Markdown
Member Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
do not allow 'hashfunc' or 'hashname' to be None for RFC tests
740fe81 
I previously allowed it to be None for future compatibility, but
I think it's better to construct a fake hash function instead, or
@picnixz
picnixz merged commit 8f11af4 into python:main Mar 3, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 3, 2025
@picnixz
picnixz deleted the test/hmac/refactor-99108 branch March 3, 2025 10:22
@picnixz

picnixz commented Mar 3, 2025

Copy link
Copy Markdown
Member Author

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.
encukou reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
@tiran tiran Awaiting requested review from tiran

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@picnixz @bedevere-bot @gpshead
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.