3.2 improve schema test coverage #4780
Conversation
Let's wait for an answer to hyperjump-io/json-schema-coverage#1 whether this change is a good idea.
More invalid security scheme objects
This reverts commit 3baeb67.
ralfhandl
changed the title
There was a problem hiding this comment.
I'm concerned that the actual validation coverage is not as thorough as the files make it appear- I have commented on one such example. This all depends on how the JSON Schema implementation handles things, and whether we expect this suite to be usable with other implementations. If we want it to work with any conforming JSON Schema implementation, we need to only test one failure condition per case.
| paths: true | ||
| components: | ||
| schemas: true | ||
| responses: true | ||
| parameters: true | ||
| examples: true | ||
| requestBodies: true | ||
| headers: true | ||
| securitySchemes: true | ||
| links: true | ||
| callbacks: true | ||
| pathItems: true |
There was a problem hiding this comment.
Does the JSON Schema implementation actually check every single one of these? Many if not most stop evaluating on the first error. This is why most of the time negative test cases test only one condition at a time while postive test cases can test many conditions.
Also, why is paths: true?
There was a problem hiding this comment.
whether we expect this suite to be usable with other implementations
I don't expect that. These tests are for us to verify whether the schemas we publish describe what we intend. The schemas did not fully describe what former TSCs intended before we added test coverage and were wrong in "exotic" areas.
Does the JSON Schema implementation actually check every single one of these?
Yes, it does. This single fail test case covers a lot of branches in a negative way.
Many if not most stop evaluating on the first error.
These implementations would then not count the remaining negative cases, and test coverage would noticeably drop. If the tool we use would switch its behaviour, we would immediately know.
why is
paths: true?
Negative test for type: object. paths: 42 would also work, and paths: [], with the latter being a more probable error than the other variants.
I could go over the fail test cases and generally use an object to fail type: array and an array to fail type: object.
There was a problem hiding this comment.
why is paths: true?
Negative test for type: object. paths: 42 would also work, and paths: [], with the latter being a more probable error than the other variants.
I could go over the fail test cases and generally use an object to fail type: array and an array to fail type: object.
No need, this was more "why is this in the components test file" but we have other files that contain test conditions beyond what is mentioned in the file name so that's fine.
We switched to https://github.com/hyperjump-io/json-schema-coverage for schema test coverage, and the new tool is more diligent than the preliminary tool used so far, reporting branches tbat are not covered by the existing schema tests.
This PR brings schema test coverage back up to 100%.
It includes the changes in
which can't be merged due to the insufficient schema test coverage.