Add $schema to 2019-09 and 2020-12, update README for draft configuration.

[handrews]

Fixes #311. To recap so folks don't have to dig through that whole discussion:

• starting with 2019-09, implementations are explicitly allowed to refuse to process schemas without $schema, therefore all tests for 2019-09 and 2020-12 need to have the correct $schema
• in draft-03, using $schema to determine behavior was only a MAY, and therefore cannot be used to control the behavior of required tests
• in draft-04 through draft-07, no normative language provides guidance on the usage of $schema to control behavior, and nothing indicates that an implementation can refuse to process a $schema-less schema, therefore adding $schema does not provide benefits over the long-standing practice of configuring the draft based on the directory name.

Therefore, we are only adding $schema to 2019-09 and 2020-12. I am unsure what to do with draft-next, as there is certainly no normative language around detecting unpublished work, so I have not added $schema to those tests, and do not plan to unless someone makes an argument in favor of that.

@Julian I updated the README as best I could, but I would be happy to change it. I think it's important to convey that 2019-09 and later are intended to function through $schema-detection (particularly since that is a change), but I avoided mandating the behavior.

[gregsdennis]

Cursory glance through these looks good to me. I'll run these through my validator soon-ish.

EDIT: yes, all tests still pass ✔️

[Julian]

Nice, thanks! README changes seem fine to me, will have a last look in a bit -- did you also re-add the optional test not containing $schema for implementations choosing to indeed process them?

[handrews]

@Julian

did you also re-add the optional test not containing $schema for implementations choosing to indeed process them?

Oops forgot about this, I'll look into it and update the PR.

[handrews]

@Julian updated with optional tests. TIL that type has not always been the same across all drafts! It only took on its current form in draft-03. Prior to that it was always an array, and the array could contain either type names or schemas! 😱 So I used minLength instead, as it would not violate the spec for an implementation to treat a lack of $schema to indicate draft-01 (although why you'd do that I have no idea — but it would be valid).

[handrews]

@Julian now I'm realizing that the remotes need draft-specific $schema updating. I'll work on that and update this PR, unless you'd prefer to merge this and I'll do the remotes as a follow-up. But probably better to do it all at once.

[handrews]

OK, I've looked into the remotes situation, including how bin/jsonschema_suite works with the remotes, and @Julian I could use a bit of advice on how to proceed. I see two major options:

1. Copy the existing remotes under the existing draft20NN-NN directories, update $schema and $id accordingly, and update the corresponding test references to the remotes.
2. Copy the existing remotes under the existing draft20NN-NN directories, update $schema, but *not $id, copy the generic tests (without any changes) under a new draft7 directory, symlink draft3, draft4, and draft6 to draft7

With Option 2, instead of loading from the "remotes" dir, you always load from a particular draft dir (or symlink).

Option 1 pros: No changes to bin/jsonschema_suite, no changes to how people interact with the test suite
Option 1 cons: Need to make sure all remote references from 2019-09 on include the appropriate draft directory in their URIs, which means that forward- and back-porting tests becomes a bit more complex. Not only does $schema need to be updated, but all references to remotes

Option 2 pros: Remote referencing would be stable across drafts, except for the meta-schema remotes which already have the draft in their URI and would probably stay that way (which might require some tweaks as the remotes/draft20NN-NN directories would be treated as the root so if we want a draft directory in the remote meta-schema we'd need another layer of directories, but that's trivial to fix)
Option 2 neutral: jsonschema_suite needs updating, which I'm happy to do, so that it accepts a draft for the appropriate commands, and collects/dumps/serves the remotes from the draft-specific remote directory
Option 2 cons: Requires test suite users to change how they set up remotes. This would be easy enough to document, but I don't know what expectations you have regarding this sort of change.

I have a slight preference for Option 2, because most remote URIs do not need to be draft-specific even if the actual remote in use names a specific draft in $schema. But it has a much bigger downstream impact so I'm happy to go with Option 1 if you prefer. It's much lower-impact, and only slightly expands the existing tedium of updating things with the draft in them.

[karenetheridge]

I am unsure what to do with draft-next, as there is certainly no normative language around detecting unpublished work, so I have not added $schema to those tests, and do not plan to unless someone makes an argument in favor of that.

I would suggest that we explicitly document (i.e. in the README) that draft-next is intended to be a placeholder for whatever identifier we choose for the next publication, and therefore the behaviour for that directory should reflect the current state of the next version in whatever branch development is taking place in. If we haven't changed the semantics of $schema after draft2020-12, then it would be reasonable to add $schema there too -- generally I would assume that tests/draft-next is keeping pace with what's added to tests/draft2020-12, plus getting additional tests for new vocabulary keywords that are being added. (Obviously the next draft is in a state of flux, so there is no expectation whatsoever that an implementation ought to support it in any way, and if tests are incomplete no one should to get too fussed about it, at least until we get closer to publication.)

[Julian]

I have a slight preference for Option 2, because most remote URIs do not need to be draft-specific even if the actual remote in use names a specific draft in $schema. But it has a much bigger downstream impact so I'm happy to go with Option 1 if you prefer. It's much lower-impact, and only slightly expands the existing tedium of updating things with the draft in them.

Option 2 is I think already something we've wanted (at least myself and I think @karenetheridge and a few others in PRs) but perhaps best to do that in a separate PR and keep the minimum amount of changes here. I think for Option2 we anyways have to update some duplication as well. Certainly if you're willing to help with that it's welcome though.

[Julian]

I think I probably agree about draft-next as well, if even just for practical reasons that it's quite annoying to have to go change every single test when draft-next becomes draft-current. If the language for $schema ever changes again ("obviating" this issue) we can always then remove $schema from all the draft-next tests at the time.

[handrews]

@Julian so should I implement option 1 for now, or put this on hold until the necessary support for Option 2 (whoever ends up implementing it) is in place?

Regarding draft-next, do we expect implementations to understand "$schema": "https://json-schema.org/draft/next"? At least in the context of the test suite? If so I'll go ahead and put it in place.

[Julian]

Do option 1 here, we merge, and then if you're still interested, a follow up PR for option 2 after.

Regarding draft-next, do we expect implementations to understand "$schema": "https://json-schema.org/draft/next"? At least in the context of the test suite?

Yes (explicitly saying so in docs is #554, but it's already expected in existing tests).

[Julian]

There's an extra "problem" in the current suite according to the assertions made in #574 that implementations may not enable additional vocabularies, namely that the format tests as they currently appear are invalid in draft2020, because the draft2020 metaschema doesn't enable or mention the assertion vocabulary.

Given that I'm moving things around in the optional directory I won't ask you to do that in this PR (add $schema to them and point that at a dialect with the assertion vocabulary mentioned) but we need to do so once both this and my PR land.

[handrews]

@Julian good to know. I will try to wrap this PR as soon as I can up since it is touching so many files.

As far as the format tests go, the 2020-12 format-annotation vocabulary has assertion behavior off by default, but to retain compatibility, it can be configured on in the historically-consistent not-clearly-specified-way (with historically-consistently vague semantics regarding how much the assertions check) just like before.

So those tests should be as valid as they were in 2019-09, which also had the "by default assertion behavior is off, but it can be turned on by some sort of unique configuration option."

The difference with the format-assertion vocabulary is that the assertion behavior is mandatory, and the expectation of how much validation happens is higher (but still does not mandate complete coverage, because someone objected to that I guess, I don't remember).

[handrews]

@Julian I know that localhost:1234 is where the remotes directory is mounted, but what is localhost:4242? It appears in tests/2019-09/recursiveRef.json?

[Julian]

Sounds like a mistake/poorly chosen fake base URI but can look later when at a computer. I want to add a sanity check that ensures files other than refRemote don't mention http://localhost at all.

[handrews]

@Julian a lot of files reference localhost. What should they be referencing instead? I can fix that (at least for 2019-09, 2020-12 and draft-next) if you'd like, but mostly I'm asking because I'm wondering if I'm missing something here.

I know a lot of the localhost references do not point to actual files, but I was updating all of the URIs consistently (references and ids).

[handrews]

I think this is now complete — @gregsdennis if you'd be willing to run the suite with your validator again that would be fantastic. I still don't have anything set up to run and I just wanted to get the PR to a complete state. If no one has time to run this, I'll set it up, but it may take a few more days.

[gregsdennis]

The suite runs fine.

Caveat: The optional 2019-09 format: uri-template tests all fail where they didn't before because I don't support that format. (.Net didn't port the implementation from .Net Framework to .Net Core. It's my implementation's issue, not a JSON Schema issue.) Interestingly, I think adding this exposed a bug in my implementation around determining whether format should be validated or merely annotated. (I'm checking the schema's meta-schema but not my draft override option.)

This definitely is an issue in my implementation, not anything about this PR.

[Julian]

Is this waiting on anything else? If not I think you can merge at will, I didn't look at the URI question yet but we can handle in a follow up, as you said they could use some cleanup but it doesn't have to happen simultaneously here, if this sits around it's liable to produce merge conflicts.

[Julian]

Oh, we should add a sanity check I guess, enforcing that root schemas all have this in these directories, but we can do that in a follow up as well if needed.

[handrews]

@Julian yes, I was just waiting to make sure you were OK with the further work on the remotes and draft-next. Let's defer the sanity check and other cleanup to later PRs.

@karenetheridge technically merging is blocked because you requested changes and have not re-approved, even though requested change has been resolved. Since it has been resolved, I'm going to go ahead and merge anyway given the high possibility of conflicts with further work if it stays open.