Add annotation tests from @hyperjump/json-schema

[jdesrosiers]

Resolves #652

It seems there was agreement in the discussion to add these tests, but it never got done. So, here it is.

A couple of things to note that weren't discussed in the issue ...

I made some minor changes to the format that was proposed in the issue. Nothing significant.

The original tests I shared were written for 2020-12, but I don't want these tests to be like the validation test where we keep making copies of the whole suite every time there's a new release. So, I included a compatibility property to declare the dialects each test is compatible with. This allows test runners to filter the suite for only the tests that apply to the dialect they are currently testing. See the included README for more details.

I want these tests to include all dialects, but I did not add tests to cover keywords from old releases that no longer exist. For example, dependencies isn't currently covered. Hopefully they will get filled in at some point.

I added schemas to describe the annotation test structure, but I wrote them in draft-07 😱. I chose draft-07 to be compatible with the JSON language server (included by default in vscode and many other editors) so users can get in-editor documentation, completions, and validation when writing tests. That's also the reason they don't have $ids (not because I'm trying to pick that fight again).

I added automation to check that the tests are valid according to the included schemas. I wrote the validation script in JavaScript, but I did it in a way that keeps JavaScript contained to just that script. No need for a package.json or node_modules. Since this is otherwise a Python repo, I could translate to Python if there's a request for that, but my Python is pretty rusty.

[karenetheridge]

Some comments about how these tests are expressed and what things we can actually test.

[karenetheridge]

I don't think we can dictate an order for this, because there is no keyword execution order defined in the specification.

For example, we don't say whether applicator keywords are evaluated first before validation, or vice versa. It is up to the individual implementation to decide whether to approach schemas in a breadth-first order (leaf nodes, e.g. validation keywords, first, then descend into subschemas via applicator keywords) or depth-first (applicators first).

[jdesrosiers]

I meant for this to be a convention to keep the tests consistent, not as a requirement. Test harnesses can match on these as an unordered set rather than an ordered array. However, there is an intuitive ordering that people expect when they read a schema and I'd like the tests to stay close to that expectation.

For example, here's a common example of a schema extension scenario where the user would expect their annotation to take precedence over the annotation from the extended schema.

{
  "$ref": "#/$defs/base",
  "properties": {
    "foo": { "title": "Extended Foo" }
  },
  "$defs": {
    "base": {
      "type": "object",
      "properties": {
        "foo": { "title": "Base Foo" }
      }
    }
  }
}

An implementation could choose to evaluate properties before $ref and the order would be ["Base Foo", "Extended Foo"] instead of the expected ["Extended Foo", "Base Foo"]. I'm just saying that I want the tests to stick to that expected order even though it's valid for it to be the other way around.

[karenetheridge]

Test harnesses can match on these as an unordered set rather than an ordered array. ... I'm just saying that I want the tests to stick to that expected order

Both of these things should be stated explicitly.

[jdesrosiers]

Already done commit. Let me know if the new text is sufficient.

[jdesrosiers]

I've updated the suite to include schema locations. Please let me know what you think.

[gregsdennis]

I like this. I think it gives all the information that's really required. Just a couple comments.

[gregsdennis]

I'm not sure this can be enforced. (I see you do have "as a convention".) If a schema uses `properties: {foo: ..., bar: ...}, who's to say which property is evaluated first? As a result, the annotations can be produced in any order.

If this is just for the sake of test organization, then maybe the request needs some more clarity (e.g. "sorted alphabetically by location pointer" or something)

[jdesrosiers]

This is about the expected property which means it applies to a single location. So, your example isn't relevant because each property would be part of a different "assertion". No ordering is recommended for assertions.

Implementations can evaluate keywords in any order they want. If everyone who contributes tests with expected ordered consistent with their implementation, there will soon be no consistency with the suite as a whole. Inconsistency makes the tests harder to reason about and harder to review. Top-down based ordering is both an intuitive and neutral choice for us to stick to.

Maybe I'm the only one who thinks maintaining this consistency has value. The vast majority of tests will only have one expected value per assertion, so maybe it won't even come up enough to be an issue anyway. In any case, it will be up to us to enforce it or not in the review process. From the perspective of test harness implementations, the order shouldn't change the pass/fail result for a test.

[gregsdennis]

Is this a conflict of message?

Ref: https://github.com/orgs/json-schema-org/discussions/473

[jdesrosiers]

I don't think so. I don't think we decided that it's wrong to use $schema this way. We just decided that we don't want to standardize using it that way. The point is to make use of the tooling that's in common use. I think it's fine.