-
Notifications
You must be signed in to change notification settings - Fork 493
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
Use context instead of an atomic variable #3599
Use context instead of an atomic variable #3599
Conversation
@@ -240,7 +241,7 @@ func TestSigning(t *testing.T) { | |||
|
|||
err = signer.GetVerifier().Verify(start+5, hashable, sig) | |||
a.Error(err) | |||
a.Contains(err.Error(), ErrSignatureSchemeVerificationFailed) | |||
a.True(errors.Is(err, ErrSignatureSchemeVerificationFailed)) |
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.
nit: newer versions of testify have ErrorIs https://pkg.go.dev/github.com/stretchr/testify/require?utm_source=godoc#ErrorIs
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## feature/dilithium-scheme-integration #3599 +/- ##
=========================================================================
+ Coverage 29.04% 47.90% +18.85%
=========================================================================
Files 381 381
Lines 61878 61879 +1
=========================================================================
+ Hits 17975 29643 +11668
+ Misses 41451 28807 -12644
- Partials 2452 3429 +977
Continue to review full report at Codecov.
|
Use context instead of an atomic variable
Summary
Switch the usage of a atomic variable with a context.
Test Plan
The updated tests seems to be failing due to unrelated issues, and these are fixed now.