-
-
Notifications
You must be signed in to change notification settings - Fork 216
Additional $ref tests for draft-04/06 #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
} | ||
}, | ||
"required": ["meta", "nodes"], | ||
"definitions": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see definitions used in this schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Relequestual {"$ref": "node"}
points to http://localhost:1234/node
which is defined in #/definitions/node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How exactly does {"$ref": "node"}
points to http://localhost:1234/node
? I don't follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base URI is http://localhost:1234/tree
when you resolve relative uri node
it becomes http://localhost:1234/node
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's section 5.2 of RFC 3986
} | ||
}, | ||
"required": ["meta", "nodes"], | ||
"definitions": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see definitions used in this schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Relequestual it's the same test as above, just in draft6 folder - please see the comment above
"baz": { | ||
"id": "folder/", | ||
"type": "array", | ||
"items": {"$ref": "folderInteger.json"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is folderInteger.json? Maybe I just don't understand this schema or the use of id, I don't know =/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Relequestual there is a convention already used in other tests to assume that schemas at http://localhost:1234/
are files in remotes
folder, folderInteger.json
is in remotes/folder
, its full URI is http://localhost:1234/folder/folderInteger.json
(and there is another existing test that points to it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Relequestual thank you for the feedback. I didn't introduce any new conventions/assumptions by these additional tests, they use only existing conventions and test only what is required by the spec. |
@epoberezkin I guess I don't understand this part of the spec that well. I don't really have the time to review / read up on it either. Probably best to get someone else to review this further. Thanks. |
@Relequestual @epoberezkin I'll take a look- I've gone through the first few which look fine so far, need to wake up a bit more to work through the base URI change tests which I will do later today. |
@handrews could you please have a look at this one so we can merge it too? |
tests/draft6/ref.json
Outdated
{ | ||
"description": "Recursive references between schemas", | ||
"schema": { | ||
"id": "http://localhost:1234/tree", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is draft 6 why is this id
instead of $id
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right :) will fix
@handrews I updated draft-06 tests - replaced ids with $ids. Thank you. |
@epoberezkin @handrews @Relequestual I found errors with these tests - see #424. |
No description provided.