Skip to content

more test cases for unevaluatedItems, unevaluatedProperties #472

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

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

karenetheridge
Copy link
Member

Only annotations from sibling keywords (or children of sibling keywords)
should be used by unevaluatedItems, unevaluatedProperties, rather than all
annotations collected so far.

see https://stackoverflow.com/questions/66936884/deeply-nested-unevaluatedproperties-and-their-expectations

@karenetheridge karenetheridge requested review from jdesrosiers and a team April 6, 2021 06:45
karenetheridge added a commit to karenetheridge/JSON-Schema-Modern that referenced this pull request Apr 6, 2021
This prevents annotations collected so far from being improperly used by
unevaluatedItems, unevaluatedProperties when only annotations from sibling
keywords (or children of sibling keywords) should be used.

See https://stackoverflow.com/questions/66936884/deeply-nested-unevaluatedproperties-and-their-expectations
and json-schema-org/JSON-Schema-Test-Suite#472
karenetheridge added a commit to karenetheridge/JSON-Schema-Modern that referenced this pull request Apr 6, 2021
This prevents annotations collected so far from being improperly used by
unevaluatedItems, unevaluatedProperties when only annotations from sibling
keywords (or children of sibling keywords) should be used.

See https://stackoverflow.com/questions/66936884/deeply-nested-unevaluatedproperties-and-their-expectations
and json-schema-org/JSON-Schema-Test-Suite#472
Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

I don't think all of these test are necessary. It looks to me like we really only need the one test for each keyword. I might be missing something.

]
},
{
"description": "item is evaluated in an uncle schema to unevaluatedItems",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "item is evaluated in an uncle schema to unevaluatedItems",
"description": "item is evaluated in a separate keyword branch to unevaluatedItems",

"valid": true
},
{
"description": "uncle keyword evaluation is not significant",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "uncle keyword evaluation is not significant",
"description": "evaluation of items in a separate keyword branch is not significant",

]
},
{
"description": "in-place applicator siblings, allOf has unevaluated",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "in-place applicator siblings, allOf has unevaluated",
"description": "property is evaluated in a separate keyword branch to unevaluatedProperties",

@karenetheridge karenetheridge force-pushed the ether/unevaluatedProperties_uncles branch from cd99094 to 78feea2 Compare April 7, 2021 21:14
@karenetheridge
Copy link
Member Author

I have removed the redundant tests (two in each file). I've left the test descriptions alone, as "uncle" is used to indicate the distinction from the other "sibling" and "cousin" tests.

@karenetheridge karenetheridge requested a review from a team April 7, 2021 21:32
Only annotations from sibling keywords (or children of sibling keywords)
should be used by unevaluatedItems, unevaluatedProperties, rather than all
annotations collected so far.

see https://stackoverflow.com/questions/66936884/deeply-nested-unevaluatedproperties-and-their-expectations
@karenetheridge karenetheridge force-pushed the ether/unevaluatedProperties_uncles branch from 78feea2 to ed4cf5f Compare April 7, 2021 21:38
@karenetheridge
Copy link
Member Author

This all seems to be sorted now, so I'm merging.

@karenetheridge karenetheridge merged commit fc68499 into master Apr 8, 2021
@karenetheridge karenetheridge deleted the ether/unevaluatedProperties_uncles branch April 8, 2021 19:32
karenetheridge added a commit to karenetheridge/JSON-Schema-Modern that referenced this pull request Apr 8, 2021
This prevents annotations collected so far from being improperly used by
unevaluatedItems, unevaluatedProperties when only annotations from sibling
keywords (or children of sibling keywords) should be used.

See https://stackoverflow.com/questions/66936884/deeply-nested-unevaluatedproperties-and-their-expectations
and json-schema-org/JSON-Schema-Test-Suite#472
@Relequestual
Copy link
Member

Thanks for handling this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants