-
Notifications
You must be signed in to change notification settings - Fork 15
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
all: increase test coverage #3
Conversation
openssl/aes_test.go
Outdated
// is only available since go1.17. | ||
// TODO: use math.MaxInt once go1.16 is no longer supported. | ||
const intSize = 32 << (^uint(0) >> 63) // 32 or 64 | ||
maxInt := 1<<(intSize-1) - 1 |
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.
Would bits.OnesCount(^uint(0))
be a clearer swap-in? (I imagine the extra dependency is ok here because it's test code.)
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.
Yes it does!
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.
I just realized this code is only using intSize
to make maxInt
... could use int((^uint(0)) >> 1)
?
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.
Even better :)
Co-authored-by: Davis Goodin <[email protected]>
This PR increases the test coverage from ~30% to ~75%.
The new tests are mostly about calling OpenSSL functions and checking that there are no errors, that nothing panics, and, when possible, that encrypt/decrypt and sign/verify functions can roundtrip correctly.
This tests will help a lot on ensuring that there are no regressions in our OpenSSL support matrix once the CI is in place.
It is out of the scope of this PR, and probably from this repo, to ensure the cryptographic correctness for the exposed functions. This validation is not trivial is is very tied to how this package is integrated with the Go toolchain.