ethereum-oasis-op / baseline-standard

Repository for the Baseline standards team and specification work
Creative Commons Zero v1.0 Universal
18 stars 33 forks source link

Testability statements for Section 7 of the Core Baseline Specification #207

Closed Therecanbeonlyone1969 closed 1 year ago

mehranshakeri commented 1 year ago

Added a PR to this branch for types

Therecanbeonlyone1969 commented 1 year ago

Added a PR to this branch for types

all typos are fixed @mehranshakeri ... you can close the other PR. All issues should be resolved in one PR to avoid duplication.

Thank you so much for your review!!

Therecanbeonlyone1969 commented 1 year ago

There is a lot of "some tool you might use for this has a big test suite and so this can be tested". I'm a bit uncomfortable with it - the key issue is whether the extensive test suite covers the properties required in each specific requirement, and in some cases the text as written looks like it's skating over that rather than justifying it clearly.

@chaals the requirements are core requirements for DB systems which are covered by test suites. What you are saying means that each testability statement must contain a test fixture which is not necessary since the statement is not normative. It just says there is a system that meets this requirement and it has a test suite and if it does not have such a test it would not be a solution with audited tests. covering all functionality which would mean the audit missed stuff. To make such a claim you should prove that. otherwise it is sufficient IMHO

chaals commented 1 year ago

There is a lot of "some tool you might use for this has a big test suite and so this can be tested". I'm a bit uncomfortable with it - the key issue is whether the extensive test suite covers the properties required in each specific requirement, and in some cases the text as written looks like it's skating over that rather than justifying it clearly.

@chaals the requirements are core requirements for DB systems which are covered by test suites. What you are saying means that each testability statement must contain a test fixture

No. I am saying that claims of "an extensive test suite" seem to be general, and ignore the question of whether specific tests exist for the requirement in question. And yet the existence of a tool that could be used, along with such general claims, are used as the sole substantiation that the requirement is testable.

which is not necessary since the statement is not normative. It just says there is a system that meets this requirement and it has a test suite and if it does not have such a test it would not be a solution with audited tests.

It seems there is a circular argument here. There are commonly two assertions of the nature "This tool can be used to solve this problem", and "There is an extensive test suite for this tool". This is apparently used to claim that "therefore we can test that at least this tool solves this problem", apparently whether or not any tests exist to show that the tool solves the problem. Which means that most of the statements contain extraneous information ("there are lots of tests") and the written basis for demonstrating testability is "this tool is asserted to meet the requirements, so you could test whether you're using this tool and thus meeting the requirements".

If there are sufficient specific tests for the relevant requirement, it makes sense to mention that. If there are not, then noting that some software is a big test suite is superfluous text of dubious value; its effect is likely misleading.

covering all functionality which would mean the audit missed stuff. To make such a claim you should prove that. otherwise it is sufficient IMHO

chaals commented 1 year ago

To be clear, in general I suspect that it is possible to justify that most requirements are testable. (I do not think that applies to R309 as written).

But it seems that many of the explanations in the PR are not strong enough to justify the assertions, often because the text provided doesn't address the specifics of the requirement.

Therecanbeonlyone1969 commented 1 year ago

To be clear, in general I suspect that it is possible to justify that most requirements are testable. (I do not think that applies to R309 as written).

But it seems that many of the explanations in the PR are not strong enough to justify the assertions, often because the text provided doesn't address the specifics of the requirement.

@chaals To address the specifics you would have to either write the test fixture for the requirement or find the exact test in the test suite that does that one thing.

I hope we agree that

If we do not then, then there is a significant disagreement on what a testability statement means.

chaals commented 1 year ago

@chaals To address the specifics you would have to either write the test fixture for the requirement or find the exact test in the test suite that does that one thing.

That's one way of doing it.

I think there is a reasonable middle ground: If you assert that something demonstrably meets the requirement, because it has tests that show it does, and that also shows the requirement is testable, then your assertion that the relevant tests exist is enough IMHO. I expect writing a spec not to be an exercise in the importance of trustlessness.

Aside:

I hope we agree that

  • test fixtures should not be in a standard but be included with a reference implementation, and
  • the standard should not provide tests but rather enable someone reasonably skilled in the art to take the given examples as directional guidance and design their own tests for their implementation for a given requirement.

If we do not then, then there is a significant disagreement on what a testability statement means.

Hmm. We don't agree in detail.

Despite that, I suspect we agree on what testability means.

Therecanbeonlyone1969 commented 1 year ago

@chaals To address the specifics you would have to either write the test fixture for the requirement or find the exact test in the test suite that does that one thing.

That's one way of doing it.

I think there is a reasonable middle ground: If you assert that something demonstrably meets the requirement, because it has tests that show it does, and that also shows the requirement is testable, then your assertion that the relevant tests exist is enough IMHO. I expect writing a spec not to be an exercise in the importance of trustlessness.

@chaals how is your middle-ground suggestion different from identifying an exact test in the test suite?

Therecanbeonlyone1969 commented 1 year ago

Hmm. We don't agree in detail.

  • I don't think tests should be included with a reference implementation but should ideally be developed independently of any implementation, failing which developed by a community of implementers reaching agreement, and
  • I think it is good if the spec does provide tests, although it isn't always necessary.

Despite that, I suspect we agree on what testability means.

ok, let's park the discussion. A test for me is expressed in code not in words. A test fixture also requires a schema in a requirement otherwise it presupposes a specific implementation of a requirement that is not desirable, IMHO.

chaals commented 1 year ago

@chaals how is your middle-ground suggestion different from identifying an exact test in the test suite?

Depends what you mean.

It does require that the assertion is based on having identified appropriate tests, but I don't think it is necessary to explicitly include the identification in the text added to the specification. (I had interpreted "identify exact test" as meaning the latter, in the conversation so far).

Therecanbeonlyone1969 commented 1 year ago

@chaals could you take a look at R279 through R281 testability statements (last commit) and tell me if I am close enough to what we discussed on Friday? Thank you!

chaals commented 1 year ago

@Therecanbeonlyone1969

@chaals could you take a look at R279 through R281 testability statements (last commit) and tell me if I am close enough to what we discussed on Friday? Thank you!

Looks excellent! When every requirement has this amount of information, I think it's worth asking if we should split "testing" into a separate document, but for now these are great.

Thank You!

Therecanbeonlyone1969 commented 1 year ago

@chaals @Kasshern @Ybittan @mehranshakeri finished the rewrite of testability statements for section 7 after the first 3 of the section were deemed to be of the right content and structure.. Please, review and approve if ok with content.