Skip to content

draft-04/06: items/additionalItems tests #161

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 25, 2017
Merged

draft-04/06: items/additionalItems tests #161

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65c18d5
draft-04/06: items/additionalItems tests
epoberezkin Jan 28, 2017
96fed74
draft-04/06: updated descriptions for items/additionalItems test cases
epoberezkin Feb 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tests/draft4/additionalItems.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
"additionalItems": false
},
"tests": [
{
"description": "fewer items is valid",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spirit of this is redundant with the test below it for "no additional items present", but if you feel like it's worth having separate tests for "less than" than "equal" it might be nice to reword the descriptions in that way specifically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test below tests array with the same number of items. This tests for array with FEWER items. While it should pass and may seem redundant, some validators fail this test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Julian could you please suggest the description?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "fewer number of items present" and "equal number of items present" is good enough.

It's mostly the generality of the description below that seems out of place to me now, since it says "no additional items present" but that applies mostly equally to this test case.

"data": [ 1, 2 ],
"valid": true
},
{
"description": "no additional items present",
"data": [ 1, 2, 3 ],
Expand Down
17 changes: 17 additions & 0 deletions tests/draft4/items.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
"description": "ignores non-arrays",
"data": {"foo" : "bar"},
"valid": true
},
{
"description": "not array is valid",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is redundant with "ignores non-arrays" above it for what I can tell, unless you're testing something different here, but not seeing it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here I think?

Copy link
Member Author

@epoberezkin epoberezkin Feb 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is relevant for Javascript validators (almost half of all validators) that do not differentiate between arrays and pseudo-array (objects that look like and can be iterated as arrays). Objects should pass even if a similar array would fail. Quite a few validators fail this test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, can you update the description to mention that specifically? This seems even more likely to be JS-specific given the presence of the "length" key, so maybe this one should specifically say something like "Javascript pseudo-arrays are not valid", and it seems likely that that edge case belongs elsewhere as well then possibly.

And if we're already going there, maybe having integer-keyed maps entirely is another reasonable test (i.e. the same as what you have, just without the length key.).

"data": {
"0": "invalid",
"length": 1
},
"valid": true
}
]
},
Expand Down Expand Up @@ -55,6 +63,15 @@
"description": "empty array",
"data": [ ],
"valid": true
},
{
"description": "not array is valid",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above about pseudo arrays

"data": {
"0": "invalid",
"1": "valid",
"length": 2
},
"valid": true
}
]
}
Expand Down
5 changes: 5 additions & 0 deletions tests/draft6/additionalItems.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
"additionalItems": false
},
"tests": [
{
"description": "fewer items is valid",
"data": [ 1, 2 ],
"valid": true
},
{
"description": "no additional items present",
"data": [ 1, 2, 3 ],
Expand Down
17 changes: 17 additions & 0 deletions tests/draft6/items.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
"description": "ignores non-arrays",
"data": {"foo" : "bar"},
"valid": true
},
{
"description": "not array is valid",
"data": {
"0": "invalid",
"length": 1
},
"valid": true
}
]
},
Expand Down Expand Up @@ -55,6 +63,15 @@
"description": "empty array",
"data": [ ],
"valid": true
},
{
"description": "not array is valid",
"data": {
"0": "invalid",
"1": "valid",
"length": 2
},
"valid": true
}
]
}
Expand Down