Skip to content

Conversation

@Shristibot
Copy link
Contributor

This PR adds additional tests for correct handling of the false schema within
conditional keywords (if, then, else), as described in Issue #767.

What these tests cover

  • When if matches, then: false must cause validation to fail.
  • When if does not match, else: false must cause validation to fail.
  • When both then and else are false, all data must be invalid regardless
    of the branch taken.
  • Ensures that false is treated as a schema that always fails, matching JSON
    Schema semantics, rather than being interpreted as {}.

These tests fill gaps in draft2020-12 conditional logic coverage, addressing the
behavior described in Issue #767.

Fixes #767.

@Shristibot Shristibot requested a review from a team as a code owner December 12, 2025 20:25
Copy link
Member

@karenetheridge karenetheridge left a comment

Choose a reason for hiding this comment

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

One test case is wrong.

I'm concerned that if it passed for your implementation, you have other issues you need to worry about! e.g. the properties keyword is never invalid when the data instance is not an object.

Also, these tests should be copied to all the version directories where they are relevant (if/then/else was added in draft7).

@jdesrosiers
Copy link
Member

@Zash, do these tests cover the case you were talking about in #767? It's not 100% clear to me what the case was exactly, especially because the example given was for not and these tests cover then/else.

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 think just the third test is sufficient. Or, just the first two. But, the first two and the third seem to be testing the same thing.

@Shristibot
Copy link
Contributor Author

I think just the third test is sufficient. Or, just the first two. But, the first two and the third seem to be testing the same thing.

Thanks for the feedback — that makes sense.

I see how the third test is logically implied by the first two and doesn’t introduce a new failure mode. I’ll remove that case and update the tests accordingly.

I’ll push an update shortly.

@Shristibot
Copy link
Contributor Author

@Zash, do these tests cover the case you were talking about in #767? It's not 100% clear to me what the case was exactly, especially because the example given was for not and these tests cover then/else.

Thanks for raising that.

I didn’t add new tests for not because handling of false with not is already covered in the existing not.json tests, which assert that false is treated as a schema that always fails (rather than {}).

This PR was scoped specifically to fill the gap for conditional keywords (if / then / else), where equivalent coverage didn’t previously exist.

If you’d prefer additional or cross-referenced coverage for not in this context, I’m happy to add it.

@Shristibot
Copy link
Contributor Author

All checks are passing now. I’ve removed the redundant test case from all three files.

Please let me know if anything else needs adjustment.

@Shristibot
Copy link
Contributor Author

hello @jdesrosiers

All checks are passing now. I’ve removed the redundant test case from all three files.

Please let me know if anything else needs adjustment, Thanks

@jdesrosiers jdesrosiers merged commit ece5bd1 into json-schema-org:main Jan 2, 2026
3 checks passed
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.

Handling of false in then or else

4 participants