some tests of the interactions between $id, $anchor and $ref #383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
karenetheridge
force-pushed
the
ether/more-anchor-and-id-tests
branch
from
ddc859e to
9d0c502
Compare
ryangalamb
reviewed
Jun 10, 2020
Julian
force-pushed
the
ether/more-anchor-and-id-tests
branch
from
e760666 to
d593591
Compare
|
Merged, thanks again! |
karenetheridge
commented
Jun 25, 2022
Comment on lines
648
to
666
| { | ||
| "description": "order of evaluation: $id and $ref", | ||
| "schema": { | ||
| "$comment": "$id must be evaluated before $ref to get the proper $ref destination", | ||
| "$id": "/base/base.json", | ||
| "$ref": "int.json", | ||
| "$defs": { | ||
| "bigint": { | ||
| "$comment": "canonical uri: /base/int.json", | ||
| "$id": "int.json", | ||
| "maximum": 10 | ||
| }, | ||
| "smallint": { | ||
| "$comment": "canonical uri: /int.json", | ||
| "$id": "/int.json", | ||
| "maximum": 2 | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
This test is wrong. $id in the presence of $ref in draft7 will do nothing. consequently, attempts to resolve the ref to "/int.json" will fail.
It looks like I copied this test erroneously from draft2019-09, and it was never tested.
Julian
added a commit
that referenced
this pull request
Jun 25, 2022
The intent of them is *not* to express that an implementation might wish to recognize these keywords. The intent of them is precisely the same as the original intent -- to allow an implementation to test itself for *not* recognizing new keywords. Such an implementation should enable these tests, as it is free to do so, and these tests will help ensure it does not accidentally leak newer keywords backwards in time if it doesn't mean to. All of the previous discussion was strictly centered on not putting these in the required directory. The reason is that all drafts *allow* implementations to add extra keywords. So a compliant implementation may indeed decide it actively *wants* to implement new keywords. Then this file would not help them, and they wouldn't run it. They would presumably love a version of this file that *did* contain all the future assertions as correct for future drafts, and in the future we could provide one. But given no implementations really do do this, at least as far as I'm aware, and given that this version of the file is here and written and more common, it would seem perfectly reasonable to have it and hope for the other version if or when someone wants it. Further discussion is in #383 and later #559. This reverts commit f605fbf.
gregsdennis
reviewed
Jun 28, 2022
| @@ -48,6 +48,67 @@ | |||
| "valid": false | |||
| } | |||
| ] | |||
| }, | |||
| { | |||
| "description": "non-schema object containing a plain-name $id property", | |||
There was a problem hiding this comment.
This uses if/then/else which weren't introduced to draft 7. We need to remove these from draft 6.
There was a problem hiding this comment.
Would you mind opening an issue, I'll remove them if someone doesn't get to it first
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
some more weird edge cases with $id, $anchor and $ref that might tickle a sloppy implementation.
I only went back as far as draft6 because the URI semantics are not as well defined before then.