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

Update test text to match spec. #81

Merged
merged 34 commits into from
Jul 15, 2024
Merged

Update test text to match spec. #81

merged 34 commits into from
Jul 15, 2024

Conversation

BigBlueHat
Copy link
Member

This includes minimal test changes. Some assert error messages were added and a
couple test fixtures where removed since they are no longer needed).

Details below:

  • Update @context test to use ordered set language.
  • Use a single MUST statement for IDs.
  • Rename credential ID fixtures for easy findling.
  • Remove duplicate fixture/test post consolidation.
  • Update requirements text in Types section.
  • Remove skipped SHOULD statement.
  • Remove proof type tests.
  • Add failure messages to some rejections.
  • Add TODOs for Conformance section.
  • Extract Verifiable Presentations section.
  • Update existing VP test to use new text.
  • Add pending test for VP id field.
  • Add VP type pending test.
  • Add pending tests for Enveloped VCs.
  • Add VP holder claims pending tests.
  • Create sections for subsections of VP tests.
  • Move Data Schemas tests up from Advanced.
  • Add some assert messages to help debugging.
  • Begin updating Advanced section.
  • Remove id tests in Refreshing.
  • Unskip Refreshing tests.
  • Add links to Refreshing section.
  • Add links to termsOfUse tests.
  • Add link and note to Evidence test.
  • Remove skipped ZKP tests.
  • Remove remaining two skipped tests.
  • Add comments for possible future tests.
  • Add .editorconfig.

No tests were changed. Just MUST statements, links, and whitespace.
We are only testing MUSTs for the moment.
No longer included in the VCDMv2 text (most likely moved to
Data Integrity spec).
Update language to match spec.
Reword one test in the Refreshing subsection.
The `id` is no longer required.
They no longer have MUST grade statements in the spec.
These were also not in the spec.
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Replacing apparently accidentally-dropped periods.

tests/10-vcdm2.js Outdated Show resolved Hide resolved
tests/10-vcdm2.js Outdated Show resolved Hide resolved
tests/10-vcdm2.js Outdated Show resolved Hide resolved
BigBlueHat and others added 2 commits July 12, 2024 10:49
Thanks, @TallTed!

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@BigBlueHat BigBlueHat requested a review from TallTed July 12, 2024 14:52
Copy link
Collaborator

@aljones15 aljones15 left a comment

Choose a reason for hiding this comment

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

Approving as it adds the text, but not necessarily the test. good PR to move forward from and thanks for the work. Do what you want with that editorConfig file.

tests/10-vcdm2.js Outdated Show resolved Hide resolved
tests/10-vcdm2.js Show resolved Hide resolved
tests/10-vcdm2.js Show resolved Hide resolved
Copy link
Collaborator

@PatStLouis PatStLouis left a comment

Choose a reason for hiding this comment

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

Happy with the changes, I have a few more inconsistencies to add as well from my fork. Once this is merged in I'll pull these changes and open my PR

tests/10-vcdm2.js Outdated Show resolved Hide resolved
Per the upcoming merge of w3c/vc-data-model#1530

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Pulled up from where these are additionally tests.
Only added reject tests.
Just going to put it right back...but robots... >_<
...which caused an astounding amount of whitespace shifting for one
word to be added... >_<
Copy link
Collaborator

@PatStLouis PatStLouis left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Collaborator

@aljones15 aljones15 left a comment

Choose a reason for hiding this comment

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

approving again provided this PR is checked to ensure commits in it are not duplicates of existing commits in main (might need a rebase from main in other words).

@BigBlueHat BigBlueHat dismissed TallTed’s stale review July 15, 2024 19:23

Addressed the issues per your request. Thanks! Pushing this along given time constraints (and that fact that despite the robot opinion the issues were addressed).

@BigBlueHat BigBlueHat merged commit db10bfd into main Jul 15, 2024
2 checks passed
@BigBlueHat BigBlueHat deleted the quote-updates branch July 15, 2024 19:24
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.

None yet

4 participants