Skip to content

Add tests for confirming struct and oneshot hashes are not allocating#91

Merged
mertakman merged 4 commits intomainfrom
hash-noalloc
Apr 29, 2025
Merged

Add tests for confirming struct and oneshot hashes are not allocating#91
mertakman merged 4 commits intomainfrom
hash-noalloc

Conversation

@mertakman
Copy link
Contributor

@mertakman mertakman commented Apr 28, 2025

Add tests for confirming struct and oneshot hashes are not allocating, also removed 1.22.x from test matrix and added 1.24.x.

@mertakman mertakman changed the title fix:add struct oneshot allocations test Add tests for confirming struct and oneshot hashes are not allocating Apr 28, 2025
@mertakman mertakman requested review from dagood and qmuntal April 28, 2025 14:43
Copy link
Member

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely better to have these than nothing, but I do have a question.

sink ^= cng.SHA1(msg)[0]
sink ^= cng.SHA256(msg)[0]
sink ^= cng.SHA384(msg)[0]
sink ^= cng.SHA512(msg)[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it help in some way to put all of these in the same test?

Or could these be separate tests (perhaps using t.Run) to make it easier to figure out what went wrong when something changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good recommendation. We have so many tests with this approach so I actually used the same code for most of the tests, I'll see if we can cut any lines or make it look simpler with that approach on the other repositories too.

@mertakman mertakman merged commit 9b8dc63 into main Apr 29, 2025
19 checks passed
@dagood dagood deleted the hash-noalloc branch April 29, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants