Conversation
jdesrosiers
left a comment
There was a problem hiding this comment.
I feel like it's unfortunate that these tests would be separate from the regular tests, but from a backward compatibility perspective, this is is probably the best approach.
My first thought was that providing an instance wasn't necessary. Implementations that don't have a way to validate a schema without an instance can just hard code null or something for the instance. But, then I realized that there are some cases that may not get evaluated fully without an instance. For example, if the schema is for an object with a property that includes an invalid reference and you pass null as the instance, the property keyword would never get evaluated and the invalid reference wouldn't be caught. So, we'd have to take care to think about different possible implementation choices to make sure we use instances that produce the desired result.
|
Yeah, I ran through all of that same logic when putting this together. |
|
There's also a question that I don't think is addressed by the spec: if the error in the schema isn't exercised by the instance, is the implementation still required to error? For example, a bad I don't think that the spec says anything about this case. If it doesn't, then we really can't provide a test for it. Maybe we can add an |
|
I agree that the spec doesn't say anything about that case. I'm not worried about covering pathological cases, so I don't think an |
This PR adds an idea for tests that check failure scenarios. Please see the included readme for details.
(I'm pretty sure we have an issue for this. I'll come back and reference it, but I have look for it first.)Looks like this starts to address #589.