Second attempt at adding test output #269
Conversation
|
Looks like the runner still needs to be updated to allow for the |
Unblocks #269. Really we should use the output schema to validate the keys are one of the known output types. But I can't figure out how to ref https://github.com/json-schema-org/json-schema-spec/blob/master/output/schema.json plus that URL is going to change. So... later.
|
Still trying to get through this, but the reformatting actually makes this a lot harder to review (as well as inconsistent style-wise with the rest of the suite). Would definitely help to back that out. |
|
Sorry, though I just notice that you mentioned the key changes are in their own commit, which definitely makes that easier, but yeah the reformatting makes these inconsistent with the earlier drafts. |
|
I was starting to feel bad that this was taking me so long to merge (which I suppose I still do, so definitely sorry for that), but then I got a bit of vindication for my stubbornness to accept these kinds of patches :) In the process of the reformatting from this PR, it essentially completely ruined a test. Namely this one: https://github.com/gregsdennis/JSON-Schema-Test-Suite/blob/595b773ca3053eb3bec066f62e1f98c63f629ef1/tests/draft2019-06/uniqueItems.json#L66 where whatever JSON parser ran that through decided to write out different JSON (equivalent to itself JSON) that completely makes the point of the test disappear. So in the face of both my general intuition and a small bit of evidence telling me it's at least possible it's correct... Can you please remove all the formatting changes and do this "from scratch"? I.e. the only change that should appear in the diff is the addition of that output property with the correct values. I assume you're using some script (your own validator?) to add that, so hopefully that's not too much trouble, but yeah it's just too risky (not to mention inconsistent) to make large scale changes all at once and hope that you're not introducing bugs. @gregsdennis sorry for the delay and stubbornness obviously. |
|
@Julian I can't output the exact same formatting that I read in. I don't know of a JSON parser that will do that TBH. To accommodate this, I split the reformat and the actual change so that it can be better reviewed commit-by-commit. The reformat was performed by a VS Code extension, and I'm surprised that it would alter the data like that. All that said, the change at this stage is a first pass. I never intended to merge in this state. We should be reviewing it for unintended alterations like this so that they can be fixed manually. Please let me know if you find anything else, and I'll put up manual fixes for these items. |
|
Since this is greatly out of date, I'm going to close it, recreate my my fork, and try again. |
|
Thanks, that'd definitely help me, and apologies again for this taking so long... |
|
@gregsdennis it's definitely possible in Python to preserve JSON ordering. Its a slightly weird thing you have to do but it works, I can dig it up if needed. JavaScript will usually preserve it up to some size, but it's annoyingly un-guaranteed. |
|
Another thing to make diffs easier - we could reformat all files to a consistent format first. Sorted properties with four column indent should be simple and reasonable enough, yes? I wrote up a oneliner to do that and can apply it to all the test files in a PR whenever the time is right. |
|
I tried to do that in one of the commits (maybe on the other PR), but was told that it was still affecting the tests somehow. |
Resolves #247 (partially)
I did some reformatting in a separate commit. Looking at 892120b (added test output) will give you just the output additions.
Also, these tests are rather simplistic (as they're designed to be), giving just a single error. This means that for the vast majority of them, the
basicanddetailedoutput version are essentially the same. More complex tests with multiple failure points would yield output that shows the difference between these two output levels.