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

Need tests for Metadata Structure #31

Closed
msporny opened this issue Feb 22, 2021 · 9 comments
Closed

Need tests for Metadata Structure #31

msporny opened this issue Feb 22, 2021 · 9 comments
Assignees

Comments

@msporny
Copy link
Member

msporny commented Feb 22, 2021

7.3 Metadata Structure

The structure used to communicate this metadata MUST be a map of properties.

Each property name MUST be a string.

Each property value MUST be a string, map, list, ordered set, boolean, or null.

The values within any complex data structures such as maps and lists MUST be one of these data types as well.

All metadata property definitions registered in the DID Specification Registries [DID-SPEC-REGISTRIES] MUST define the value type, including any additional formats or restrictions to that value (for example, a string formatted as a date or as a decimal integer).

The entire metadata structure MUST be serializable according to the JSON serialization rules in the [INFRA] specification.

@peacekeeper peacekeeper changed the title Need tests for Resolution Metadata Structure Need tests for Metadata Structure Mar 25, 2021
@peacekeeper
Copy link
Contributor

peacekeeper commented Mar 26, 2021

@msporny I edited your initial comment to reflect the latest version of the spec.

@shigeya
Copy link
Contributor

shigeya commented Mar 30, 2021

I'm wondering how we can test this statement:

All metadata property definitions registered in the DID Specification Registries [DID-SPEC-REGISTRIES] MUST define the value type, including any additional formats or restrictions to that value (for example, a string formatted as a date or as a decimal integer).

How do we know property definitions on the test suite side? We just assume non-core properties to be in the spec registry?

@iherman
Copy link
Member

iherman commented Mar 31, 2021

The issue was discussed in a meeting on 2021-03-30

  • no resolutions were taken
View the transcript

7.4. Need tests for Metadata Structure

See github issue did-test-suite#31.

Shigeya Suzuki: I almost finished writing tests besides one issue, related to the DID registry, as I commented in the issue.
… Please look at the last comment I made.
… The spec says all metadata property definitions registered in the registry must define a variable type.
… I need a way to find this. Option 1: refer to did-spec-registries
… Or we can assume that properties other than the core properties are registered.
… I need some way to determine the list of types for this test.

Brent Zundel: Thank you for the overview

Charles Lehner: Thank you all for your work.

Brent Zundel: Special Topic call this Thursday


@msporny msporny closed this as completed Apr 3, 2021
@shigeya
Copy link
Contributor

shigeya commented Apr 6, 2021

@msporny We don't want to close this yet.. The test (potentially) referring to DID SPEC REGISTRY is not yet complete..

@msporny msporny reopened this Apr 6, 2021
@msporny
Copy link
Member Author

msporny commented Apr 6, 2021

@msporny We don't want to close this yet.. The test (potentially) referring to DID SPEC REGISTRY is not yet complete..

Ok, reopened.

I think there is a good argument for that statement to be human testable only... unless you have a clear machine-testable way to test that statement?

@shigeya
Copy link
Contributor

shigeya commented Apr 8, 2021

Let me quote the statement again here:

All metadata property definitions registered in the DID Specification Registries [DID-SPEC-REGISTRIES] MUST define the value type, including any additional formats or restrictions to that value (for example, a string formatted as a date or as a decimal integer).

From the core spec point of view, the above statement is not testable within the core spec. This spec text needs to be placed in DID-SPEC-REGISTRIES, not within core spec. I honestly have no idea whether we can treat this statement as "human testable."

From a different perspective: since the current test implementation runs the whole test against the transcript of interaction with multiple method implementations, it is desirable to test properties defined in DID-SPEC-REGISTRIES. In this case, we need a machine-readable -- at least partial -- DID-SPEC-REGISTRY.

I think there are three options:

  1. Eliminating the above statement
  2. Rewrite the above statement to move the test to DID-SPEC-REGISTRIES
  3. Rewrite the above statement to remove (direct) dependency on DID-SPEC-REGISTRIES.

It is possible to create an optional configuration parameter that may hold machine-readable (possibly partial) DID-SPEC-REGISTRIES, which can be usable for the above option 3.

(Note: due to schedule conflict, I can't attend the special topic call this week)

@msporny
Copy link
Member Author

msporny commented Apr 18, 2021

From the core spec point of view, the above statement is not testable within the core spec.

Correct.

This spec text needs to be placed in DID-SPEC-REGISTRIES, not within core spec.

Yes, agreed.

I honestly have no idea whether we can treat this statement as "human testable."

It is human testable. A human can check to see if "All metadata property definitions registered in the DID Specification Registries [DID-SPEC-REGISTRIES] define the value type, including any additional formats or restrictions to that value (for example, a string formatted as a date or as a decimal integer)."

I suggest we do the second option (if we don't have to go through another CR to do it), or just flag the statement as human testable and not something the WG intended to test. We have lots of these sorts of statements in the DID Methods section.

@msporny
Copy link
Member Author

msporny commented Apr 18, 2021

I believe we can raise a new issue just for the one test that @shigeya found that was human testable, and then close this issue.

Raised the issue here: #65

Closing this one, as we now have tests for Metadata Structure.

@msporny msporny closed this as completed Apr 18, 2021
@shigeya
Copy link
Contributor

shigeya commented Apr 18, 2021

Yes, I should have close this and open a new one early. thanks.

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

No branches or pull requests

4 participants