Skip to content
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

remove conditional $id tests from draft 6 #562

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

gregsdennis
Copy link
Member

@gregsdennis gregsdennis commented Jun 28, 2022

Fixes https://github.com/json-schema-org/JSON-Schema-Test-Suite/pull/383/files#r907940838

These tests use if/then/else which weren't introduced to draft 7. We need to remove these from draft 6.

@Julian
Copy link
Member

Julian commented Jun 28, 2022

I have to stare at these again tomorrow but I suspect they're still salvageable without using if/then/else -- those are being used out of convenience but I don't think they're part of the purpose of the test -- probably using oneOf or allOf would work

@gregsdennis
Copy link
Member Author

gregsdennis commented Jun 28, 2022

That's fine, but I just wanted to resolve these being incorrect. My build is failing because of these.

I can try to rework them in terms of oneOf. (Would be really nice to have a linter....)

@Julian
Copy link
Member

Julian commented Jun 28, 2022

It looks like the linter that should catch this isn't running properly for draft 2019 :/ I'll look at why tomorrow.

Julian added a commit that referenced this pull request Jun 28, 2022
It was previously skipping validating newer drafts :/

Refs: #562
@Julian
Copy link
Member

Julian commented Jun 28, 2022

#563 updates the sanity checker to properly run but... it wouldn't have helped unfortunately for catching the mistake, since the schemas aren't actually invalid (since the properties are supposed to be ignored).

#540 is really the kind of thing that'd possibly help, which we're working towards.

@Julian Julian merged commit e1dbaeb into main Jun 28, 2022
@Julian Julian deleted the remove-conditional-id-tests-from-draft6 branch June 28, 2022 16:56
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.

2 participants