refactor crypto keys test #158
Conversation
Codecov Report
@@ Coverage Diff @@
## main #158 +/- ##
==========================================
+ Coverage 53.27% 53.32% +0.05%
==========================================
Files 32 32
Lines 3773 3773
==========================================
+ Hits 2010 2012 +2
+ Misses 1391 1389 -2
Partials 372 372
Continue to review full report at Codecov.
|
crypto/keys_test.go
Outdated
| assert.NoError(tt, err) | ||
| assert.NotEmpty(tt, pub) | ||
| assert.NotEmpty(tt, priv) | ||
| func validateKeyGeneration(tt *testing.T, pub crypto.PublicKey, priv crypto.PrivateKey, keyType KeyType, err error) { |
There was a problem hiding this comment.
I'm not sure I see the benefit in having this as a separate function.
Perhaps just embedding it in the above is sufficient?
There was a problem hiding this comment.
Yup embedding is a good call
| assert.EqualValues(tt, priv, reconstructedPriv) | ||
| }) | ||
| for _, keyType := range GetSupportedKeyTypes() { | ||
| t.Run(string(keyType), func(tt *testing.T) { |
There was a problem hiding this comment.
Table driven tests are a go convention.
What do you think about adjusting this from a loop to a table test format?
There was a problem hiding this comment.
It seems like for a table driven test it has known input and output. With this unit test it doesn't really have a known output, so the table would only have on dimension an input.
I could explicitly create an array in the test to match this basically or we can keep using GetSupportedKeyTypes
https://github.com/TBD54566975/ssi-sdk/blob/main/crypto/models.go#L45
Removing duplicate code and dynamically running all supported key types