Convert key tests to tabular tests #157
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
qmuntal
requested review from
setrofim,
thomas-fossati,
yogeshbdeshpande and
shizhMSFT
Codecov Report
@@ Coverage Diff @@
## main #157 +/- ##
==========================================
+ Coverage 91.32% 91.35% +0.02%
==========================================
Files 12 12
Lines 1545 1550 +5
==========================================
+ Hits 1411 1416 +5
- Misses 97 98 +1
+ Partials 37 36 -1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
setrofim
approved these changes
Jul 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR contains converts all COSE_Key-related tests to tabular tests, making those test more similar to the COSE_Sign related tests. Also, IMO, tabular tests make it easier to understand what is being tested, as they don't depend on reusing and mutating inputs but on explicit and clearly definid inputs and outputs.
There is just one behavior change in this PR:
NewEC2KeyandNewOKPKeyreturn a nil key if the validation fails, together with the corresponding error. Returning nil objects when there is an error instead is more idiomatic and less error prone.This will also facilitate testing the API redesign that I'll submit as part of #151.