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

Document the test inclusion guidance/criteria #439

Open
Julian opened this issue Oct 4, 2020 · 0 comments
Open

Document the test inclusion guidance/criteria #439

Julian opened this issue Oct 4, 2020 · 0 comments
Labels
documentation A request for new or updated documentation for the suite enhancement An enhancement to the tooling or structure of the suite (as opposed to a new test).

Comments

@Julian
Copy link
Member

Julian commented Oct 4, 2020

From #436 (comment) --

The bar is low for new additions to the suite. As broad guidelines:

  • additional tests MUST be correct behavior as far as is dictated by the specification
  • additional tests MUST NOT be impossible to run for an implementation that is correct under the specification
  • additional tests which appear at the top-level of each version folder MUST test required behavior for all implementations. In particular they MUST NOT rely on behavior which the specification proscribes via SHOULD, MAY RECOMMENDED or equivalent.
  • additional tests MUST NOT attempt to clarify the specification itself independently for behavior that was not considered or proscribed by the specification. In the case of ambiguous text in the specification, the specification team SHOULD be consulted to confirm what behavior was intended. If the relevant scenario was clearly and specifically considered but the wording was unclear, tests MAY be added. Otherwise, the test MUST be deferred (i.e. not added with any expected result) until a specification with explicit decision on its behavior is published.
  • additional tests SHOULD NOT be directly covered by already existing tests
  • additional tests SHOULD be in "minimal form" -- by which we call a test "minimal" if it does not include additional behavior beyond which is required for the behavior the test covers, regardless of whether the schema is stylistically optimal
  • additional tests SHOULD prefer "simpler" keywords to more complicated ones when testing unrelated behavior. As a concrete example, a test for the anyOf keyword will need to reference other keywords as part of its schema. It should prefer to use "simple" keywords like const whenever possible, rather than more complex (to implement) ones like patternProperties or $ref. This allows implementers access to more tests even whilst their implementation is incomplete or in-progress.
  • large numbers of autogenerated or programmatically generated tests which cover many relatively similar scenarios SHOULD NOT be added. Instead, a small representative sample should be added, with the full generative script made available for those who wish to run all combinations.
  • additional tests MUST NOT break or modify API guarantees of the test suite itself without specific discussion and migration plan (this relies on The suite should explicitly document its backwards compatibility policy #223, but the point here as an example is "you can't add tests in which the object in the schema property of the test is not a valid JSON Schema. This is a guarantee that all tests follow intentionally, and which was intended to be true indefinitely. If we want to add tests for scenarios in which that does not hold, it must be after discussion and design.")
  • additional tests MUST NOT reference external schemas or resources other than within the refRemote.json file.

If for a particular set of tests the above is met, generally tests should be merged at which point:

  • additional tests SHOULD NOT be merged so long as the sanity checks ("CI for the suite itself") is failing (which generally aims only to test some of the above API guarantees of the suite). If a bug in the sanity checks themselves is suspected, reviewers SHOULD file an issue to track the sanity check bug and MAY choose to merge the original PR immediately if they are confident that it is correct.
  • additional tests MUST be added to all active drafts they apply to, including the draft-next/ folder, and authors SHOULD review each version to ensure the test is applicable before copying them.

Including some guidelines for modification or removal are likely also nice.

Proposed changes or improvements to the above are of course welcome via issue or discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A request for new or updated documentation for the suite enhancement An enhancement to the tooling or structure of the suite (as opposed to a new test).
Projects
None yet
Development

No branches or pull requests

1 participant