Draft 07 validation test suite #200
Conversation
Covers * Ignoring each keyword in isolation * All paths through if/then, if/else, and if/then/else * Ensuring that keywords across "allOf" do not interact
Also remove the "we'll use draft3 as an example" bit, as there are no actual draft-specific examples in this file.
This does not include any of the other RFC 3339 formats that are reserved but not included in the main set of formats by the validation specification. For the same reason, it also does not include the reserved "full-date" and "full-time" aliases. There is no required behavior for the reserved formats. We may wish to add tests for them in the future, or make an even-more-optional section of some sort. But this should be sufficient for the initial draft-07 test suite.
These are just slightly changed versions of most of the URI and URI-reference test suites. I omitted a number of URI test cases, particularly of different sorts of schemes. It's not entirely clear to me what the scope of the test suite should be, and I just want to get something in place that hopefully someone with a deeper understanding of IRI-related challenges can enhance.
These cover specific RJP syntax elements but do not cover all of the variations from the JP test suite that would also apply to RJP. More can be added later.
Since it is now restored to the formats list.
handrews
added
draft support
missing test
|
Nice! It might be helpful for reviewers (well, me :/) to do the two parts there separately -- i.e. all the stuff that's just copying over draft6 tests unchanged is lgtm immediately, so would be a bit easier to review just the new stuff. I'm happy to do that locally instead though too, looks like you've helpingly done that part in a separate commit. |
| @@ -16,7 +16,7 @@ If you're going to use this suite, you need to know how tests are laid out. The | |||
| tests are contained in the `tests` directory at the root of this repository. | |||
|
|
|||
| Inside that directory is a subdirectory for each draft or version of the | |||
| schema. We'll use `draft3` as an example. | |||
| schema. | |||
There was a problem hiding this comment.
The changes to this file are also lgtm immediately :), thanks for updating it.
|
@Julian they're all separate commits so you can look at the diffs individually and take your time. I didn't want to just have the repo sitting there with the wrong test cases in the folder so I didn't want to merge the copy separately. LMK if it's still a concern. |
|
The only test file that's changed (instead of added) is format.json, and those changes are done over several commits just for that one file. |
|
Ah got it, as in there's actual behavior changes? Makes sense. Will have a look. |
|
@Julian For format.json there are cases added. I don't think anything changed behavior in this draft for validation or dereferencing. |
| { | ||
| "description": "validation of string-encoded content based on media type", | ||
| "schema": { | ||
| "contentMediaType": "application/json" |
There was a problem hiding this comment.
Hmmmm I probably have to read the spec now I assume, but wow, this seems rather large -- are we gonna ask implementors to be able to validate that something is a valid video/mp4?
There was a problem hiding this comment.
@Julian As with format, doing validation for contentMediaType and contentEncoding is entirely options. I suspect many systems will use them as annotations (instructions to the application using the schema and instance) rather than assertions (affecting a boolean outcome, in this case validation). The assertion vs annotation thing is formalized in the new core and validation specs.
Unlike format, which has a set of values that, if you implement validation for the keyword at all, you SHOULD implement that set of values, with content* there is no requirement for any particular set of encodings or media types to be validated. I assume that many validators will provide a generic hook and allow extensions to register handlers for particular media types an/or encodings, as is often done with custom formats already.
At least, that was the intention.
There was a problem hiding this comment.
Got it -- makes sense, and I didn't notice that this was in optional/ so thanks for pointing that out.
tests/draft7/optional/format.json
Outdated
| "tests": [ | ||
| { | ||
| "description": "a valid idn e-mail address ([email protected] in Hangul)", | ||
| "data": "실례@실례.테스트", |
There was a problem hiding this comment.
If you're gonna put the literal bytes here, might want to document that our test suite files themselves are UTF-8, not sure that's documented anywhere already.
There was a problem hiding this comment.
Should that just go in the README or is there something else to do for it? I've always been a bit vague on that- it tends to Just Work(tm) on MacOS whenever I've needed it.
|
Awesome! This looks great. Merge away I think! |
|
Oh, other than there's a failing test it looks like. |
|
@Julian I'm having a bit of trouble figuring out what failed. It says that tox exited with 1 but the test cases look fine? I may just need another cup of tea (late night, not awake yet). |
|
Oh wait, I see it now... line length problem? Blerg. |
|
OK, I abbreviated or removed a word from each of the three long description strings and it's fine now. Given how trivial the fix was and the prior approval, I'm going to go ahead and merge this. |
The first commit copies over the draft-06 test cases, the subsequent commits add various sets of closely related tests and update the README.
The JSON files parse properly, but I haven't done anything else to test the tests yet.
I will file issues for more IRI, IRI-reference, and Relative JSON Pointer tests as those are not thoroughly covered. In the case of IRIs, I did not duplicate all URI tests, mostly because I just want to get something out for now and got tired of messing with it :-)