Implement remaining DID Resolution tests. #86

peacekeeper · 2021-05-03T15:24:43Z

This implements the remaining DID Resolution and DID URL Dereferencing tests.

…RL including the DID fragment MUST be used."

…ID resolution functions for at least one DID method and MUST be able to return a DID document in at least one conformant representation."

msporny · 2021-05-03T15:44:05Z

packages/did-core-test-server/suites/did-resolution/did-resolution.js

+      it.skip('A conforming DID Method specification MUST guarantee that each equivalentId value is logically equivalent to the id property value.', () => {
+      });
+      // As discussed on the 2021-04-13 DID WG topic call, the following test can be skipped (see https://www.w3.org/2019/did-wg/Meetings/Minutes/2021-04-13-did-topic)
+      it.skip('equivalentId is a much stronger form of equivalence than alsoKnownAs because the equivalence MUST be guaranteed by the governing DID method.', () => {
+      });


Suggested change

it.skip('A conforming DID Method specification MUST guarantee that each equivalentId value is logically equivalent to the id property value.', () => {

});

// As discussed on the 2021-04-13 DID WG topic call, the following test can be skipped (see https://www.w3.org/2019/did-wg/Meetings/Minutes/2021-04-13-did-topic)

it.skip('equivalentId is a much stronger form of equivalence than alsoKnownAs because the equivalence MUST be guaranteed by the governing DID method.', () => {

});

I suggest we just not have .skip() and .todo()s in the test suite. We haven't done that elsewhere and it'll be more work than necessary at this point. We do have an accouting/audit of this in the issues that have been raised. I suggest that doing so should be enough for the purposes of CR/PR/REC.

msporny · 2021-05-03T15:44:45Z

packages/did-core-test-server/suites/did-resolution/did-resolution.js

+      it.skip('A conforming DID Method specification MUST guarantee that the canonicalId value is logically equivalent to the id property value.', () => {
+      });


Suggested change

it.skip('A conforming DID Method specification MUST guarantee that the canonicalId value is logically equivalent to the id property value.', () => {

});

Suggest removal per https://github.com/w3c/did-test-suite/pull/86/files#r625178855

msporny · 2021-05-03T15:47:27Z

packages/did-core-test-server/suites/did-resolution/did-resolution.js

      });
      it('This structure is REQUIRED, and in the case of an error in the resolution process, this MUST NOT be empty.', async () => {
        expect(didResolutionMetadata).not.toBeFalsy();
-        if (isErrorExpectedOutcome(expectedOutcome)) {
+        if (utils.isErrorExpectedOutcome(expectedOutcome)) {
          expect(Object.keys(didResolutionMetadata)).not.toHaveLength(0);
        }
      });
      it('If resolveRepresentation was called, this structure MUST contain a contentType property containing the Media Type of the representation found in the didDocumentStream.', async () => {


This goes for all it() statements in the test suite. I'm afraid that if we don't prefix each test with the section number, that it's going to be even more difficult for the person doing the test reporting logic to match each test appropriately and automatically tally the "multiple independent implementations" of each feature.

Please see https://github.com/w3c/did-test-suite/blob/main/packages/did-core-test-server/suites/did-core-properties/did-core-properties.js#L14-L16 for an example.

msporny

While there are some suggestions that could impact the test report generation, this is an improvement over the current suite and thus should be merged even if those changes aren't made.

@msporny

Suggested by @msporny in #86 (comment).

@msporny

Suggested by @msporny in #86 (comment).

peacekeeper · 2021-05-03T16:22:22Z

@msporny I applied your suggestions.

OR13 · 2021-05-03T18:03:14Z

packages/jest-did-matcher/src/matchers/toBeMediaType/predicate.js

@@ -0,0 +1,5 @@
+export default expected => {
+    const regex = /^\w+\/[-+.\w]+/


minor nit that we might prefer this to be something like application/did+......

Agree. Do we have the complete list?

minor nit that we might prefer this to be something like application/did+......

I think I disagree, because there are statements in the spec that say something must be a media type, and this is what the predicate is meant for.

Additional tests (such as whether a media type is for a conformant representation of the DID document data model) happen elsewhere.

OR13 · 2021-05-03T18:04:06Z

packages/did-core-test-server/suites/resolution-utils.js

+};
+
+const consumeRepresentation = (representation, contentType) => {
+  // TODO: improve this by re-using other test code


nit: open an issue an link to it in todo... same could be said for the other todos.

I agree, done: #87

shigeya · 2021-05-04T12:00:50Z

packages/did-core-test-server/suites/did-url-dereferencing/did-url-dereferencing.js

-        expectConformantMetadataStructure(dereferenceOptions);
+    describe('dereferencingOptions', () => {
+      it('7.2 DID URL Dereferencing - A metadata structure.', async () => {
+        utils.expectConformantMetadataStructure(dereferenceOptions);


This can be merged with my part of the test.

I agree, I was planning to do this later (i.e. better re-use existing test code), after this PR is merged.

shigeya · 2021-05-04T12:04:32Z

packages/jest-did-matcher/src/matchers/toBeMediaType/predicate.js

@@ -0,0 +1,5 @@
+export default expected => {
+    const regex = /^\w+\/[-+.\w]+/


Agree. Do we have the complete list?

shigeya · 2021-05-04T12:10:16Z

packages/did-core-test-server/suites/did-resolution/did-resolution.spec.js

+  describe(implementationName, () => {
+    it('All conformant DID resolvers MUST implement the DID resolution functions for at least one DID method and MUST be able to return a DID document in at least one conformant representation.', async () => {
+      expect(implementation.executions).not.toBeEmpty();
+      const execution = implementation.executions.find((execution) => (execution.function === 'resolveRepresentation'));


I like the structure of the test 👍

@msporny

Suggested by @msporny in #86 (comment).

peacekeeper requested review from msporny and OR13 as code owners May 3, 2021 15:24

peacekeeper mentioned this pull request May 3, 2021

Normative statements checklist: 7. Resolution #76

Closed

72 tasks

peacekeeper force-pushed the peacekeeper-finalize-resolution-dereferencing branch from ab6306e to d295bd6 Compare May 3, 2021 15:32

peacekeeper added 9 commits May 3, 2021 17:32

Fix invalidDidUrl error string.

0f80f82

Use toBeValidDid() and toBeValidDidUrl() matchers.

269e969

Use toBeAsciiString() matcher.

cabfd14

Use toBeDidCoreDatetime() matcher.

d1b24a0

Use toBeMediaType() matcher.

e02864f

Implement test for "To dereference a DID fragment, the complete DID U…

8897f63

…RL including the DID fragment MUST be used."

Implement test for "All conformant DID resolvers MUST implement the D…

b6f1716

…ID resolution functions for at least one DID method and MUST be able to return a DID document in at least one conformant representation."

Remove unused input file.

caf5934

Implement remaining DID Resolution tests.

241315a

peacekeeper force-pushed the peacekeeper-finalize-resolution-dereferencing branch from d295bd6 to 241315a Compare May 3, 2021 15:36

msporny reviewed May 3, 2021

View reviewed changes

msporny approved these changes May 3, 2021

View reviewed changes

peacekeeper added 2 commits May 3, 2021 18:00

Remove skipped tests (but keep comments).

0322e4c

Suggested by @msporny in #86 (comment).

Add section numbers.

582511a

Suggested by @msporny in #86 (comment).

msporny requested review from clehner and shigeya May 3, 2021 17:36

OR13 approved these changes May 3, 2021

View reviewed changes

OR13 reviewed May 3, 2021

View reviewed changes

shigeya approved these changes May 4, 2021

View reviewed changes

shigeya reviewed May 4, 2021

View reviewed changes

peacekeeper mentioned this pull request May 5, 2021

Better re-use test code between sections #87

Open

msporny merged commit c2b1e47 into main May 6, 2021

msporny pushed a commit that referenced this pull request May 6, 2021

Remove skipped tests (but keep comments).

5cdd06f

Suggested by @msporny in #86 (comment).

msporny deleted the peacekeeper-finalize-resolution-dereferencing branch June 18, 2021 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement remaining DID Resolution tests. #86

Implement remaining DID Resolution tests. #86

peacekeeper commented May 3, 2021

msporny May 3, 2021

msporny May 3, 2021

msporny May 3, 2021 •

edited

Loading

msporny left a comment

peacekeeper commented May 3, 2021

OR13 May 3, 2021 •

edited

Loading

shigeya May 4, 2021

peacekeeper May 5, 2021 •

edited

Loading

OR13 May 3, 2021

peacekeeper May 5, 2021

shigeya May 4, 2021

peacekeeper May 5, 2021

shigeya May 4, 2021

shigeya May 4, 2021

		it.skip('A conforming DID Method specification MUST guarantee that the canonicalId value is logically equivalent to the id property value.', () => {
		});

		@@ -0,0 +1,5 @@
		export default expected => {
		const regex = /^\w+\/[-+.\w]+/

Implement remaining DID Resolution tests. #86

Implement remaining DID Resolution tests. #86

Conversation

peacekeeper commented May 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msporny May 3, 2021 • edited Loading

Choose a reason for hiding this comment

msporny left a comment

Choose a reason for hiding this comment

peacekeeper commented May 3, 2021

OR13 May 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peacekeeper May 5, 2021 • edited Loading

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

msporny May 3, 2021 •

edited

Loading

OR13 May 3, 2021 •

edited

Loading

peacekeeper May 5, 2021 •

edited

Loading