Pres submission test vectors #109
Conversation
nitro-neal
requested review from
decentralgabe,
mistermoe,
phoebe-lew,
frankhinek and
amika-sq
as code owners
| @@ -0,0 +1,150 @@ | |||
| { | |||
There was a problem hiding this comment.
i was under the impression that we want to have 1 JSON per test vector, so we can do something like
read 1 test vector file -> parse 1 JSON object -> testCase(JSON)
with this structure, how do you parse out and use for multiple test cases?
There was a problem hiding this comment.
for the web5-* we have formatting like this:
https://github.com/TBD54566975/web5-spec/blob/main/test-vectors/README.md
you take the array of test vectors, they will all attempt to parse in the object and run the feature
There was a problem hiding this comment.
will want to consolidate and have tbdex be the same, otherwise we will have 100s of json files
There was a problem hiding this comment.
Actually after discussion with Diane it makes sense to leave tbdex vectors the way they are
| @@ -0,0 +1,150 @@ | |||
| { | |||
| "description": "did:dht create", | |||
There was a problem hiding this comment.
Just wanted to double check that you wanted to include did:dht test vectors with this PR. Didn't see it mentioned in the PR title or description.
There was a problem hiding this comment.
yup i found that these were not included from the old repo so I threw them in
| }, | ||
| { | ||
| "description":"valid presentation definition 2", | ||
| "input":{ |
There was a problem hiding this comment.
Input is slightly different than the other test vectors for presentation_exchange. Worth adding to the README?
There was a problem hiding this comment.
how is the input different?
There was a problem hiding this comment.
The input for these vectors is presentationDefinition (and presentationSubmission in the validate_submisstion.json vector below), and there's no output. Current README specifies only the input/output for CreatePresentationFromCredentials, which has completely different input/output.
There was a problem hiding this comment.
Right those READMEs need to be updated,
it basically needs to comply with the parent readme - https://github.com/TBD54566975/web5-spec/blob/main/test-vectors/README.md
| @@ -0,0 +1,434 @@ | |||
| { | |||
| "description":"Validate definition", | |||
| "vectors":[ | |||
There was a problem hiding this comment.
Each of these vectors don't have an output, but they do specify that there should/shouldn't be errors. Does that mean the validation should pass? If so, should the output be true to make that clearer?
There was a problem hiding this comment.
right so these are testing a function called
PresentationExchange.validateDefinition()
so if errors is false, then this should be a valid definition. (and not throw)
if errors is true, then this should be an invalid definition json (and throw)
There was a problem hiding this comment.
@frankhinek I'd like to get your thoughts on this approach. I know that in the crypto test vectors you added (ex: crypto_edd25519/verify.json), we also have a verify vector, with the output of true/false accordingly.
I think in some languages, it makes sense to return a boolean value for validate/verify operations, in addition to producing an error when things go horribly wrong. Wondering if we can capture that somehow.
Or maybe I'm way off, and validate/verify are completely separate concepts to others. I've always thought of them as similar concepts in my head 😅
There was a problem hiding this comment.
Yea this has been the point of discussion lately,
Across the sdks we have some verify functions that return true / false, other that throw and exception or not, and others that give a Results object
I'm capturing requirements here: and then I hope we can come to consensus on a global "Results" object (or not) to be used across all these verify functions - decentralized-identity/web5-js#386
| @@ -0,0 +1,243 @@ | |||
| { | |||
| "description":"Validate submission", | |||
| "vectors":[ | |||
There was a problem hiding this comment.
These vectors also don't have output. What would an "errors": false represent for these vectors?
There was a problem hiding this comment.
output is optional, errors false means that this input does not throw an exception
There was a problem hiding this comment.
I don't think that my comments in https://github.com/TBD54566975/web5-spec/pull/109/files#r1467053239 are blocking at all, just wanted to continue the thread a little bit. Maybe there's something there, maybe not.
👍 from me for bringing it
No description provided.