Add static test vectors #33

aljones15 · 2024-01-30T15:41:25Z

Adds configurable static test vectors to project. This does not use the vectors from the ecdsa spec yet, but can be configured to use them in the future.

Partially Addresses this issue: #29

Features:

A new credentials section in runner.json that allows configuring static test vectors.
A new script that clones the vc-di-ecdsa repo and TestVectors into the project.
A new dir mocks that contains static test vectors and respective pointers.
A new config property issuerName for configuring the issuer used to create test data.
A new config property holderName for configuring the holder used to create sd test data.

TallTed

tiny

tests/50-sd-verify.js

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

TallTed

small, human-facing

CHANGELOG.md

README.md

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

BigBlueHat

Mostly just need clarity around some things.

BigBlueHat · 2024-02-13T19:39:56Z

.gitignore

@@ -23,3 +23,4 @@ reports/**
 credentials/*.json
 .vscode
 .localImplementationsConfig.cjs
+tests/input/**


What's in this directory? Is it populated by someone/something?

this is where the static test vectors from vc-di-ecdsa are being stored. This follows the existing convention of storing test vectors in /tests/input such as in the VC 2.0 Data Model tests. We're not using anything in that DIR yet, but we do plan on using those vectors in the future.

BigBlueHat · 2024-02-13T19:41:21Z

README.md

+the issuer name using an environment variable or setting `issuerName` in `./config/runner.json`. 
+
+For the `ecdsa-rdfc-2019` suite use `RDFC_ISSUER_NAME`.
+For the `ecdsa-sd-2023` suite use `SD_ISSUER_NAME`.


So...set RDFC_ISSUER_NAME to some string value?

should be improved here: c97f3e4

BigBlueHat · 2024-02-13T19:43:18Z

README.md

 To generate test data used in the test suite, testers are required to specify
-the issuer name using the environment variable `ISSUER_NAME`.
+the issuer name using an environment variable or setting `issuerName` in `./config/runner.json`. 


Why is issuerName in config/runner.json, but the other variables are environment variables?

Also, this says you can do that either way, but it removes what environment variable name to use. Should that be SD_ISSUER_NAME? And if so, why the name change? And is issuerName in config/runner.json the same thing?

hopefully improved the explanation here: c97f3e4

BigBlueHat · 2024-02-13T19:44:41Z

README.md

-If `$HOLDER_NAME` is not specified, `Digital Bazaar` will be used.
+In addition, the environment variable `SD_HOLDER_NAME` or the setting `holderName` in `./config/runner.json` 
+can be used to specify the VC holder name for generating disclosed test credentials for ECDSA-SD tests.
+If `$SD_HOLDER_NAME` or `holderName` is not specified, `Digital Bazaar` will be used.


Is this just for reporting purposes? Or is this a lookup of some kind? If it's a lookup (from the implementations list?), we should spell that out here. If it's just a string for reporting purposes, we should set it to something generic to avoid confusion.

it needs to match an implementation name. The implementation used it not noted in the report unless it fails to create the test data in some suites. Anyways, addressed here: c97f3e4

BigBlueHat · 2024-02-13T19:45:33Z

README.md

-### Changing the Test Tag
-These test suites use tags to identify which implementation's endpoints are used in tests.
+### Configuring the Tests
+The suites call on a common config file stored at `./config/runner.json`.


This particular sentence should likely be moved up to the top of "Usage"

moved to the top and expanded here: a356456

BigBlueHat · 2024-02-13T19:51:25Z

README.md

+  "suites": {
+    "ecdsa-rdfc-2019": {
+      "tags": ["ecdsa-rdfc-2019"],
+      "issuerName": "Digital Bazaar",


This file increasingly feels confused in purpose. If it's a committed file in git history, we don't want local test running folks to ever edit it--which essentially causes a fork.

So, any "runtime" config should be kept elsewhere.

So there are now vector.json and runner.json files here: a6ebbef

BigBlueHat · 2024-02-13T19:54:40Z

tests/50-sd-verify.js

                  // select less than full subarrays
+                  const pointers2 = credentialHasArrays.selectivePointers.slice(


Not real keen on the incrementing variable name. Can we use something clearer?

as discussed this test data is has been historically a little weird, anyways improved the pointer names here: 67418b3 in the vc-generator PR these test vectors should be created programmatically outside of the before statement.

BigBlueHat · 2024-02-13T19:54:58Z

tests/50-sd-verify.js

                    signedCredential: signedAchievementCredential,
                    vcHolder
                  });
                  disclosedCredentialsWithLessThanFullSubArray.push(
                    revealedAchievementCredential2);
-
-                  // select w/o first array element
+                  // select w/o first 7 array element


Why remove the first 7 elements? No need to explain it in a reply here, but explaining it in a comment in the code would help folks who work with you on this or who use it as reference when testing their implementations.

this is hopefully addressed somewhat here: 67418b3

as the original 2 pieces of test data appear to have been identical, this change at the least seems to improve the diversity of test data.

BigBlueHat · 2024-02-13T19:56:49Z

tests/helpers.js

+export const createMultipleVcs = async ({}) => {
+
+};


This should come out until it's in use.

Excellent catch, was going to move test data generation outside of the before statements and into helpers to clean up the suites, but to much going on in this PR. Removed createMultipleVc here: da1f798

BigBlueHat · 2024-02-13T20:16:30Z

tests/test-config.js

@@ -1,5 +1,73 @@
 /*!
- * Copyright 2023 Digital Bazaar, Inc. All Rights Reserved
+ * Copyright 2023-2024 Digital Bazaar, Inc. All Rights Reserved


appended this as an additional task to this issue: #27

Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

BigBlueHat

One small thing that can wait, so please plow ahead.

CHANGELOG.md

BigBlueHat · 2024-02-15T19:34:56Z

testVectorGuide.md

+Copyright 2023 Digital Bazaar, Inc.
+
+SPDX-License-Identifier: BSD-3-Clause
+-->


Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

aljones15 self-assigned this Jan 30, 2024

aljones15 force-pushed the add-static-test-vectors branch 2 times, most recently from e6a55b2 to d6d3f9c Compare February 7, 2024 16:03

TallTed requested changes Feb 8, 2024

View reviewed changes

tests/50-sd-verify.js Outdated Show resolved Hide resolved

aljones15 and others added 26 commits February 13, 2024 15:53

Add tests/input and ignore subdirs of it.

16f0f1f

Clone shallow copy of vc-di-ecdsa spec and test vectors.

9c6ebbc

Add postinstall script for vc-di-ecdsa test vectors.

0a909e8

Use windDoc.json as test vector.

fb36b9b

Use require for runner.json with package specific path.

707ac41

Make document for issuer tests configurable.

a54e9dd

Use test-config.js with issuerDoc prop.

919852c

Remove validVc from mock-data.js move to mocks dir.

af5e73c

Reduce use of klona in test-config & check issuerDoc type.

56ddba6

Add README Section on configuring the suites.

cf18e1a

Expand settings to user issuer and holder name.

2a1bc07

Get issuerName & holderName for getSuiteConfig.

27c934e

Add SPDX clause to test-config.js.

084a467

Check that holderName is defined before overwriting holderName.

2bb3026

Add mandatoryPointers to initial VC issuance.

23e74af

Better comment & improve naming in 50-sd-verify suite.

0f3780d

Better docs on createInitialVc & improve mandatoryPointer checks.

e58d43a

Start verifierDocs in runner.json.

175218d

Comment sd 50 for better clarity.

497e5e6

Add credentials section to config for test suites.

38698e2

Convert existent test data to static vectors.

5affbb7

Add config cache & start using configurable VC files.

ff6d7c3

Update tests/50-sd-verify.js use standard VC in comments.

fbfc6c0

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Create no id VC from validVc.

723b4cb

Add empty selective & mandatory pointers.

e20b57a

Open arrays of suiteFiles.

6de799b

aljones15 added 4 commits February 13, 2024 16:17

Add initial test vector config section.

e19cc51

Correct anchor link to test data section & space config sections.

52bd222

Add MINOR CHANGELOG entry for test vector config feature.

8d87cd5

Further expand CHANGELOG entry.

d71cae1

aljones15 requested review from BigBlueHat and Wind4Greg February 13, 2024 18:31

aljones15 marked this pull request as ready for review February 13, 2024 18:33

TallTed requested changes Feb 13, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

aljones15 requested a review from TallTed February 13, 2024 19:39

aljones15 and others added 2 commits February 13, 2024 19:44

Update CHANGELOG.md Improve language about config props and directories.

fd35025

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update README.md Use JSON not json.

517035d

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

BigBlueHat requested changes Feb 13, 2024

View reviewed changes

aljones15 and others added 12 commits February 14, 2024 14:45

Update README.md include path when referencing runner.json.

6095e2a

Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

Update README.md prefer SD over sd.

a69aa23

Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

Update README.md use SD over sd.

506c18e

Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

Remove unused method createMultipleVcs.

da1f798

Improve naming of pointers in sd-verify tests.

67418b3

Improve README docs on issuer and holder name env vars.

c97f3e4

Separate Test Vectors from runner docs in README.

ab8d3c3

Separate runner & vector json files.

a6ebbef

Try relative URL for test vectors md file link.

593e4e5

Prefer testVectorGuide to TESTVECTORS.md.

4ac71eb

Clarify README and testVectorGuide more.

a356456

Add note on test vector defaults.

ae009ca

aljones15 requested a review from BigBlueHat February 15, 2024 13:20

Pluralize config intro statements.

02f028d

BigBlueHat approved these changes Feb 15, 2024

View reviewed changes

Update CHANGELOG.md Prefer uppercase for acronyms.

d61169d

Co-authored-by: BigBlueHat <byoung@digitalbazaar.com>

aljones15 merged commit e20df31 into main Feb 16, 2024
2 checks passed

aljones15 deleted the add-static-test-vectors branch February 16, 2024 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add static test vectors #33

Add static test vectors #33

aljones15 commented Jan 30, 2024 •

edited

Loading

TallTed left a comment

TallTed left a comment

BigBlueHat left a comment

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat Feb 13, 2024

aljones15 Feb 14, 2024

BigBlueHat left a comment

BigBlueHat Feb 15, 2024

		// select less than full subarrays
		const pointers2 = credentialHasArrays.selectivePointers.slice(

Add static test vectors #33

Add static test vectors #33

Conversation

aljones15 commented Jan 30, 2024 • edited Loading

TallTed left a comment

Choose a reason for hiding this comment

TallTed left a comment

Choose a reason for hiding this comment

BigBlueHat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BigBlueHat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aljones15 commented Jan 30, 2024 •

edited

Loading