Skip to content
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

add test vector for select credentials #74

Merged
merged 5 commits into from
Dec 13, 2023
Merged

Conversation

nitro-neal
Copy link
Contributor

@nitro-neal nitro-neal commented Dec 7, 2023

Will look something like this:

const selectCredTestVectorJson = get(url)
const pd = selectCredTestVectorJson.vectors[0].input.presentationDefinition
const creds = selectCredTestVectorJson.vectors[0].input.credentialJwts

const actualSelectedVcJwts = PresentationExchange.selectCredentials(creds, pd);

expect(actualSelectedVcJwts).to.deep.equal(selectCredTestVectorJson.vectors[0].output.selectedCredentials);

@mistermoe
Copy link
Contributor

@nitro-neal what's the rationale behind the renaming?

@nitro-neal
Copy link
Contributor Author

We decided on underscores everywhere to create the function naming easier across languages

We wanted to add Web5TestVectors to the class name for knowing which tests are SPEC vc normal unit test

Example:
class: Web5TestVectorsPresentationExchange

in Web5-SDK-Development:
folder => presentation_exchange
filename => select_credentials.json

in JS => Web5TestVectorsPresentationExchangeSpec.select_credentials
in KT => Web5TestVectorsPresentationExchangeTest.select_credentials
local folder => presentation_exchange
local filename => select_credentials.json

@mistermoe
Copy link
Contributor

gotcha. is this something we're trying out to see if it works or something we've decided to commit to? if the latter, can we document the rationale and instructions others should follow?

@nitro-neal
Copy link
Contributor Author

Yup was an initial decision, not 100% in love with it.
I think we can get a full flow working with it, and then discuss and make changes if necessary.

Once we all check off on the naming convention we will definitely add to documentation / readme

@amika-sq
Copy link
Collaborator

I just want to clarify a little bit about how we intend on using these test vectors:

I'm currently under the impression that we should be comparing the output defined within the test vectors as the source of truth. In the PR description, you have the following line doing the expectation:

expect(actualSelectedVcJwts).to.deep.equal(selectCredTestVectorJson.vectors[0].output.selectedCredentials);

Would this be comparing against the actual strings that are present in the test vector output? Or would this be comparing against the representation of the JWTs against their language's representation of them?

@mistermoe
Copy link
Contributor

mistermoe commented Dec 12, 2023

We decided on underscores everywhere to create the function naming easier across languages

can you provide a bit more context here? trying to understand how all of the PRs related to a test harness prototype spread across 3 repos intend to come together. it's hard to see right now.

would appreciate a write-up of what y'all have in mind! It's difficult to provide actual feedback for something we intend to land in main without:

  • understanding the full picture
  • knowing whether PRs to main imply y'all have figured something out and committed to it or are simply trying things out

based on the limited amount i understand right now, instinctively feel like expecting all languages to have the exact same function names is a bit rigid but again, not sure what you mean here by "function naming"

@nitro-neal
Copy link
Contributor Author

Yup happy to create a full fledged README / documentation for this.

Here is the tdlr of what it would be for you understanding.

Step 1.
make a new test vector folder in web5-sdk-development
presentation_exchange/select_credentials.json ✅

the class name for the unit test in Web5-JS and Web5-KT algorithm goes as follows

"Web5TestVectors" + ("presentation_exchange".removeUnderScore().CamelCase().CapitalizeFirst()) + "Test"

which equals the class name - Web5TestVectorsPresentationExchangeTest

we do this so that the golang test runner can easily find all the test it cares about by grepping the prefix - Web5TestVectors

Step 2.
the function name is the same as the json file name
select_credentials
so the file that will be generated is:

TEST-web5.sdk.credentials.Web5TestVectorsPresentationExchangeTest.xml

Step 3.
the golang test runner can now look at this xml, it looks it its own directory looking at the directory structure and the names of the jsons, it makes sure the xml exists, and then it looks at the result of the xml to see if it passed:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="web5.sdk.credentials.Web5TestVectorsPresentationExchangeTest" tests="1" skipped="0" failures="0" errors="0" timestamp="2023-12-12T18:01:26" hostname="nealr-macbookpro.local" time="0.009">
  <properties/>
  <testcase name="select_credentials()" classname="web5.sdk.credentials.Web5TestVectorsPresentationExchangeTest" time="0.009"/>
  <system-out><![CDATA[]]></system-out>
  <system-err><![CDATA[]]></system-err>
</testsuite>

image

@nitro-neal
Copy link
Contributor Author

image

@mistermoe
Copy link
Contributor

thanks for all of the info @nitro-neal!

question for you: what is the "go test runner". do we have something that's running the actual tests?

@nitro-neal
Copy link
Contributor Author

Web5-JS and Web5-KT run the tests locally and then use a local github action to upload the .xml files to their local repos as an artifact.

The Web5-SDK-Development has a github action that runs when web5-kt or web5-js merge to main

This Web5-SDK-Development github action will download and parse the xml artifact, and create the checkmark report

I guess by go runner I mean the Web5-SDK-Development code that downloads and parses children xml files

Copy link
Contributor

@phoebe-lew phoebe-lew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra details in comments - will be great to see the documented e2e and iterate on the process.

@nitro-neal nitro-neal merged commit 44b7d25 into main Dec 13, 2023
6 checks passed
@nitro-neal nitro-neal deleted the select-cred-test-vector branch December 13, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants