Complete top-to-bottom review of testability of normative statements. #526
Conversation
msporny
requested review from
jricher,
rhiaro,
brentzundel,
dlongley and
peacekeeper
There was a problem hiding this comment.
I'm not sure about most of the MAY->mights. I read MAYs to be explicit warnings to implementers to look out and account for these behaviours in other implementations they're trying to interop with. I think 2119-ing the MAYs really makes that clear, and doesn't add a testing burden since we don't test MAYs (without tons of free time - but they're all things that could be tested). (But I don't feel strongly about this.)
Agree in general. A decent chunk (but not all) of the OPTIONAL/MAYs that I changed were either contradicting a SHOULD earlier in the document, were asserting things about resolving parties, or wouldn't result in testing anything of importance. I tend to not really like MAY/OPTIONAL language as it is rarely ever tested or enforced... it's kinda nothing language -- sure, be aware of this MAY but we're not expecting anyone to do anything about it. In those cases, why have the normative language at all? I know others don't share this viewpoint so happy to put things back in that folks feel strongly about. I may have also made a few mistakes and missed the nuance of some of the normative MAY/OPTIONAL language. Happy to reconsider anything folks feel needs to go back to the way it was. |
Co-authored-by: Amy Guy <amy@rhiaro.co.uk>
| @@ -4045,7 +4033,7 @@ <h2> | |||
| </code></p> | |||
|
|
|||
| <p> | |||
| The input variables of this function MUST be as follows: | |||
| The input variables of this function are as follows: | |||
There was a problem hiding this comment.
Why would we not test this normative statement?
There was a problem hiding this comment.
Eliminating this normative statement has no effect on the number of tests we write because each item in the list asserts a normative test as well and refers to a data structure for which there are already normative tests.
| @@ -4070,7 +4058,7 @@ <h2> | |||
| </dl> | |||
|
|
|||
| <p> | |||
| The output variables of this function MUST be as follows: | |||
| The output variables of this function are as follows: | |||
There was a problem hiding this comment.
Why would we not test this normative statement?
There was a problem hiding this comment.
Eliminating this normative statement has no effect on the number of tests we write because each item in the list asserts a normative test as well and refers to a data structure for which there are already normative tests.
| @@ -4035,7 +4023,7 @@ <h2> | |||
| and fragment. This process depends on <a>DID resolution</a> of the <a>DID</a> | |||
| contained in the <a>DID URL</a>. | |||
| The details of how this process is accomplished are outside the scope of this | |||
| specification, but all conformant implementations MUST implement a function | |||
| specification, but all conformant implementations implement a function | |||
There was a problem hiding this comment.
Why would we not test this normative statement?
There was a problem hiding this comment.
How would we write a test to ensure that "a conformant implementation implements a function with the following abstract form". That is, how do we test to make sure an abstract function is defined... since by it's very nature, the function is abstract and has no testable realization?
| <a>verification method</a> is explicit in the <a>DID document</a>. | ||
| <a>Verification methods</a> that are not associated with a particular | ||
| <a>verification relationship</a> MUST NOT be used for that <a>verification | ||
| <a>verification relationship</a> cannot be used for that <a>verification | ||
| relationship</a>. For example, a <a>verification method</a> in the value of | ||
| the <code><a>authentication</a></code> property cannot be used to engage in |
There was a problem hiding this comment.
Just noting that I tried to remove this normative language in an earlier PR based on a comment by @kdenhartog, and @msporny objected on the basis that it is testable. Given this is a conflict between present manu and past manu, I'm going to suggest manu can make the call :)
There was a problem hiding this comment.
*lol* ... Dammit, past manu... 'causing trouble.
Past manu was wrong, @rhiaro and @kdenhartog are right.
The normative-ness of the statement is being removed because it is a test intended for a conformance class of software that we don't define in DID Core. In other words, it has very little to do with production and consumption conformance classes and instead has more to do with software that implements authentication/authorization protocols.
Present manu is displeased that past manu failed to realize that. :P
| @@ -2189,7 +2189,7 @@ <h2>Service Endpoints</h2> | |||
| <dd> | |||
| <p> | |||
| If a <a>DID document</a> includes a <code>service</code> property, the value | |||
| of the property SHOULD be an <a data-cite="INFRA#ordered-set">ordered set</a> | |||
| of the property MUST be an <a data-cite="INFRA#ordered-set">ordered set</a> | |||
There was a problem hiding this comment.
was this originally a SHOULD because the value could potentially be an ordered set of just URI strings (which dereference to the rest of the data)? Worth checking if this change affects any implementations?
There was a problem hiding this comment.
I'm not sure we ever had that use case... I may be wrong, but as far as I can remember... service was always supposed to be an array of objects (because you have to give the service an id, type, and a serviceEndpoint)... I don't know of any DID Method that doesn't follow that pattern.
|
Normative, multiple reviews, changes requested (some made), no objections, merging. |
This PR contains a complete top-to-bottom review of the testability of the normative statements in the DID Core specification and should address issue #384. The goal with this PR is to reduce the workload for the entire WG wrt. the test suite and the tests we're going to have to write in the coming month or two. With that goal in mind - this PR removes or refactors normative statements that are tested elsewhere in the specification, aren't our job to test, or are too vague to be tested (by a human or a machine).
I expect there are a few that reviewers might find concerning, so while you're considering the change, ask yourself the following questions to suggest alternatives:
If the answer to all of those questions is "yes", then there may be an argument to not make a subset of the changes made in this PR.
Preview | Diff