Consolidate test vectors, add nextest filter #288
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
KendallWeihe
requested review from
nitro-neal,
kirahsapong,
shamilovtim,
diehuxx,
decentralgabe,
frankhinek,
leordev and
ALRubinger
as code owners
KendallWeihe
commented
Aug 16, 2024
crates/web5/src/test_vectors.rs
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_did_jwk_resolve() { |
There was a problem hiding this comment.
also slight rename's in the two test names, but everything else is copy/pasta
|
if we move forward with this, then we need to rebase to retrieve the changes I just merged from #284 |
KendallWeihe
force-pushed
the
kendall/vcs
branch
from
0a425f3
to
f83e121
Compare
KendallWeihe
force-pushed
the
kendall/consolidate_test_vectors
branch
from
0808ad9
to
4c0df0e
Compare
Alright this is done actually, rebase and moved the latest did:dht test vector into the single module as well |
nitro-neal
reviewed
Aug 16, 2024
crates/web5/src/test_vectors.rs
Outdated
| } | ||
| } | ||
|
|
||
| mod presentation_definition { |
There was a problem hiding this comment.
Suggested change
| mod presentation_definition { | |
| mod presentation_exchange { |
needs
There was a problem hiding this comment.
nice catch, fixed
nitro-neal
approved these changes
Aug 16, 2024
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.
The original driving force for this was a failing CI. The CI was failing because our new
UnitTestSuitesolution isn't compatible with ournextestconfiguration (best guess isnextestis parallelizing things in a way wherein our solution doesn't work;cargo testworks fine so ourtestCI job is passing without issues). We could probably find a way to configure thenextestrunner/harness such that it is indeed compatible, or we could use an external state file/env-var. But, I think we should consolidate test vectors regardless (IMO we think of test vectors as a singular suite of tests so it makes sense for them to exist in a single module), and if we do, then we can use thenextestfilter feature to only execute the test vectors in ourrust-test-vectorsCI job.@nitro-neal lmk what you think (please note, this PR is a merge into #283)