search

w3c / vc-test-suite

Verifiable Credentials WG Test Suite
https://w3c.github.io/vc-test-suite/
BSD 3-Clause "New" or "Revised" License
69 stars 39 forks source link

[Tracking Issue - Proposed Corrections Feedback] Test suite improvements are needed #126

Open brentzundel opened 2 years ago

brentzundel commented 2 years ago

@jyasskin I've raised this issue to track the WG response to the feedback you provided on the test suite and to track the work required to address the limitations you've uncovered.

From feedback on VC Data Model v1.1

The implementation testing described in
https://w3c.github.io/vc-test-suite/implementations/#testing-methodology is
insufficient for these changes, as it doesn't test that the full set of
values that can be generated for the `id`, `proof`, and `credentialSchema`
properties by one implementation can be understood and used by all the
other conforming implementations.

@clehner has volunteered to work on this, but assistance from others is also welcome.

iherman commented 2 years ago

The issue was discussed in a meeting on 2022-01-26

View the transcript #### 5.1. [Tracking Issue - Proposed Corrections Feedback] Test suite improvements are needed (issue vc-test-suite#126) _See github issue [vc-test-suite#126](https://github.com/w3c/vc-test-suite/issues/126)._ **Brent Zundel:** Some of the corrections we made were not reflected in the test suite, that needs to be addressed. Charles, can you give us a status on this issue?. **Charles Lehner:** I'm still a little stumped by what changes to make. **Brent Zundel:** I think. … one of your PRs from sept addresses this already. **David Chadwick:** as written, this feels impossible-- "full range"/"all possible" is infinite; "representative values" or "one example of each" would be a better wording. **Brent Zundel:** maybe a good first pass would be making a current list of all normative statements; i think tests for anything else (like proof and credential schema properties) would be overkill. … but still appreciated as a stretch goal. **Charles Lehner:** sure, normative statements sounds a good first step. **Brent Zundel:** i'm not sure any second step is mandatory, that can all wait til v2 WG. **Juan Caballero:** where is the old list of normative statements, and how out-of-date is it?. … just to ask if normative statements have ever been compiled. **Brent Zundel:** I don't know if there is such a list. … Could ask Manu or Dmitri. **Juan Caballero:** ok sounds good. … i'll do that before meeting with Charles to continue this. **Brent Zundel:** Any other questions or comments?.
bumblefudge commented 2 years ago

I volunteered last week to help scope or specify the requirements here, but I have to cry uncle and request additional assistance because I'm having even more trouble than Charles understanding the scope of

the full set of values that can be generated for the id, proof, and credentialSchema properties

Which of the following need to be included in the test vectors to say we have fully addressed this?

A.) How many of the following have to be represented as valid (positive-case) values for a top-level id property:

  1. a URL
  2. a URI but not a URL?
  3. a non-ASCII IRI?
  4. a non-fetchable URI?
  5. a DID?
  6. a non-DID, non-URL URI?
  7. a UUID with the prefix urn:uuid:?

B.) Does the [optional] credentialStatus section of the test suite have to cover the same exhaustive range of options for credentialSchema.id ?

C.) As discussed on today's call, it seems that the requirements of refreshService.id are determined by refreshService.type- so the behavior of a consuming party as to refreshService.id seems to me untestable without a normative reference as to how one or more specific types further constrain refreshService.id.

D.) As for the proof section, this seems even harder to define vectors for, since these vary even more depending on proof.type. Changes 3.1 and 3.2 make this even more the case than before, but as the NOTE under example 25 already stated explicitly, the ZKP section does not specify how to evaluate or parse a ZKP VP normatively. Am I wrong in concluding that the only two normative statements it makes can't be evaluated by this test suite alone?

iherman commented 2 years ago

The issue was discussed in a meeting on 2022-02-02

View the transcript #### 4.1. [Tracking Issue - Proposed Corrections Feedback] Test suite improvements are needed (issue vc-test-suite#126) _See github issue [vc-test-suite#126](https://github.com/w3c/vc-test-suite/issues/126)._ **Brent Zundel:** Charles, can you give us an update on status of this?. **Charles Lehner:** No update yet. Changes might be remove number of tests in test suite.. **Manu Sporny:** Yeah, Charles, your read is correct. Our approach in the group has not been to do tests for MAY statements.. … If it's associated with a MAY statement, it should not be in the test suite. I haven't had a change to dig deep into this, but some of the things they are referring to seem to be non-normative things in the specification, except for `id` that is normative.. … The other thing we'll do in the VCWG 2.0 work is to define more stringent tests for at least `id` and `proof`, `credentialSchema` we still need implementers to step forward and write the tests for.. **Brent Zundel:** cel, does that give you info to move forward on this?. **Charles Lehner:** Kinda, still trying to understand how to test these things. It seems like this is optional, up to suite type rather than make it mandatory.. **Manu Sporny:** Yeah, Charles, your read is correct on that. We shouldn't be testing that stuff. They are asking us to test an extension point; the spec doesn't define it. We can discuss this more in VCWG 2.0. If you just ping me we can go through it one by one and fix or defer to 2.0.. **Charles Lehner:** Ok, thanks.. **Brent Zundel:** Anything else on this topic before we move to Charter..
jyasskin commented 2 years ago

The above discussion matches my impression that this probably can't be fixed before 2.0. The large number of 'MAY' constraints add up to a spec that says that interoperability might or might not be achieved between the actual implementations. Since the purpose of a REC is to ensure that implementations are interoperable, that's a problem. But it's not a problem we expect you to be able to fix under the current charter.

brentzundel commented 2 years ago

I agree that the bulk of the changes mentioned in this thread will need to happen in the next WG under v2.0.

My question to @msporny @clehner @bumblefudge: has the test suite been updated to reflect the normative corrections we made in v1.1? If not, imo that is the scope of work that needs to be undertaken at this point.

clehner commented 2 years ago

has the test suite been updated to reflect the normative corrections we made in v1.1

Two PRs are open for this: #123 for the URL/URI changes, and #127 for the ZKP changes. The test suite still refers to VC Data Model 1.0 though - e.g. the test directory name test/vc-data-model-1.0. Should a new copy be made for 1.1, or the current one renamed in-place?

I agree it looks like a change in methodology may be needed to test a greater range of values. The tests focus on required properties. We have changes where some properties used to be required and are now optional - or may be required in some situations, such as with proof properties - or may be required depending on the object type. Maybe these should have new tests added, to ensure that the omission of those properties is allowed where relevant?

iherman commented 2 years ago

The issue was discussed in a meeting on 2022-02-09

View the transcript #### 4.1. [Tracking Issue - Proposed Corrections Feedback] Test suite improvements are needed (issue vc-test-suite#126) _See github issue [vc-test-suite#126](https://github.com/w3c/vc-test-suite/issues/126)._ **Charles Lehner:** Tallted has commented on this. > *Charles Lehner:* See [Ted's comments](https://github.com/w3c/vc-test-suite/pull/123#discussion_r800718574). _See github pull request [vc-test-suite#123](https://github.com/w3c/vc-test-suite/pull/123)._ **Charles Lehner:** when a test has not been evaluated or when feature did not previously exist, I still need to respond to TallTed. _See github pull request [vc-test-suite#127](https://github.com/w3c/vc-test-suite/pull/127)._ **Brent Zundel:** is there anyone on the call that can review these?. **Manu Sporny:** you can add me. … we did not copy 1.0 test suite to a new one. Instead we should add tests to the 1.0 test suite. … this will not require testers to re-run all their previous tests. … there will be a proposal to change the way we run tests for v2.0.
iherman commented 2 years ago

The issue was discussed in a meeting on 2022-02-16

View the transcript #### 2.1. [Tracking Issue - Proposed Corrections Feedback] Test suite improvements are needed (issue vc-test-suite#126) _See github issue [vc-test-suite#126](https://github.com/w3c/vc-test-suite/issues/126)._ **Brent Zundel:** who can speak about progress on the issue?. **Manu Sporny:** haven't been in touch with Charles. **Brent Zundel:** the changes we can make as a WG are reflected in 127 & 128 pulls. … those could be pulled in after a conversation. **Manu Sporny:** the test suite is non-normative and can be changed anytime.. **Brent Zundel:** next issue.
iherman commented 2 years ago

The issue was discussed in a meeting on 2022-02-23

View the transcript ### 3. test suite feedback. _See github issue [vc-test-suite#126](https://github.com/w3c/vc-test-suite/issues/126)._ **Brent Zundel:** can someone give us an update on this?. **Manu Sporny:** no update, have not met with charles, there are a lot of fairly involved questions... the whole thing is non normative... we should probably not change a lot of stuff until we have 2.0 charter.