Initial run at draft/2019-04 tests, complete with output

[gregsdennis]

This is essentially the draft-07 test suite with the addition of the expected output. I generated the output as I ran the test suite in Manatee.Json.

It's missing a few things:

1. Some of the optional tests don't pass (for various reasons), so these aren't included. I can look at adding them manually, but it would be a serious undertaking to make all of them pass in .Net (e.g. zeroTerminatedFloats: "a float is not an integer even without fractional part" just isn't going to happen since I don't save the string representation of numeric values, so I can't test for integer vs fraction-less float).
2. I didn't create any tests for the keywords, formats, etc. that are new as of this draft.
3. There should be tests that validate annotation collection. Maybe these should be optional since annotation collection is not required.

Resolves #247

[Julian]

@gregsdennis thanks!

This is obviously too big to review as-is, so still thinking about how to do that, but at least the tests are failing because the schema for tests themselves has now changed to add the output. I suppose the cheap way to fix that is to remove the additionalProperties, which would evade tackling #223 or having per-draft test suite schemas.

[gregsdennis]

I think (maybe) looking at some of the larger, more deeply nested ones then assuming the others follow suit would be okay . Other than that, ¯\_(ツ)_/¯

[Julian]

I think another thing after I sent that is (assuming the new draft is fully backwards compatible) first merging a complete copy over, such that what's left is at least purely the diff from what's already here.

I'm going to try that later, unless you mind giving that a shot perchance and beat me to it.

[handrews]

@Julian off the top of my head the main incompatibilities are is that dependencies was split into dependentSchemas and dependentRequired, and that definitions was renamed $defs

However, it's still entirely valid to use definitions (because it's a placeholder location and doesn't actually do anything), and dependencies effectively becomes an extension keyword that people can support for backwards compatibility if they want to. Both of these old keywords are still present in the schema meta-schema, they're just not present in the individual vocabulary meta-schemas.

Anyway, those are pretty simple changes so I think copy, commit, and then PR the copies would make sense.

[gregsdennis]

Sure. If you'd like to split the migration of draft 7 -> 8 from the addition of the output, that's fine. Just let me know. It's simple enough to generate the output.

I didn't think about the keyword differences. It's probably a good idea to migrate the tests first.

[Julian]

OK, pushed a straight up copy (without yet handling the renames) to master directly, and pushed a rebased version of this @gregsdennis to the output branch, which at least looks slightly more manageable.

Will now need to handle reviewing that, and then handling the changes @handrews mentioned (thanks!)

[gregsdennis]

I'll update this when we're happy that the new draft suite is ready.

[Julian]

Github just closed this for some reason. And the reopen button is gone too.

Thanks GitHub :/

[gregsdennis]

GH merged the changes! ???!!

[Relequestual]

Github closed the associated issue (#247) because the first post says "Resolves #NUMBER".
The "Resolves' word is underlined.

I'm assuming you're referencing the issue as opposed to the PR, which cannot be re-opened once merged.

[gregsdennis]

No, we can't figure out why this was merged. It's understandable that the issue closed when this merged, but why did this merge?

[Relequestual]

Just to confirm @Julian you did NOT merge this PR yourself?

[Relequestual]

If so, I'll contact github =]

[Julian]

You guys seem more interested in the mystery than I was :D

I merged the other PR (#267).

But, now looking, all that it looks like happened here is that @gregsdennis it looks like you put the commits from #265 on your master branch, then put commits after that that essentially reverted the ones adding output (e.g. 1ce104f), then branched off of that to create the defs branch.

So when I merged that, the commits adding output were in fact merged (and then reverted by later commits).

[Julian]

It's still dumb IMHO by the way that GitHub won't let me reopen the PR, as a marker (as if it were an issue), but whatever.

[Relequestual]

Given you can close manually, I would guess you CAN open manually, but only IF the code hasn't been merged.

[gregsdennis]

Oh. Oops.

[awwright]

Hard-reset the branch if you have to, that's simpler than any other option.

[gregsdennis]

I've already opened a new PR.