oneOf tests for success-caused failures#159
oneOf tests for success-caused failures#159RitoG09 wants to merge 3 commits intohyperjump-io:mainfrom
Conversation
jdesrosiers
left a comment
jdesrosiers
left a comment
There was a problem hiding this comment.
Thanks. I think that covers the basics. I've converted this to a draft because we can't commit it until we have an implementation or it breaks the test automation.
src/test-suite/tests/not.json
Outdated
| ] | ||
| }, | ||
| { | ||
| "description": "not with nested schema producing a match", |
There was a problem hiding this comment.
This description doesn't match the test. I don't see any nesting.
There was a problem hiding this comment.
I've updated the description and added a not with nested schema.
| { | ||
| "description": "not fails due to subschema match with success explanation", | ||
| "schema": { | ||
| "not": { | ||
| "required": ["a"] | ||
| } | ||
| }, | ||
| "instance": { "a": 1 }, | ||
| "errors": [ | ||
| { | ||
| "messageId": "not-message", | ||
| "instanceLocation": "#", | ||
| "schemaLocations": ["#/not"], | ||
| "alternatives": [ | ||
| [ | ||
| { | ||
| "messageId": "required-success", | ||
| "messageParams": { "property": "a" }, | ||
| "instanceLocation": "#", | ||
| "schemaLocations": ["#/not/required"] | ||
| } | ||
| ] | ||
| ] | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
I see no difference between this test and the one before it. The not schemas are different, but there's nothing fundamentally different about what it's testing.
There was a problem hiding this comment.
sorry for late reply I was travelling, yes this one and the previous one is fundamentally same except the not schema used here is required.
| { | ||
| "description": "not with nested schema producing a match", | ||
| "schema": { | ||
| "not": { | ||
| "oneOf":[ | ||
| { "required": ["a"] }, | ||
| { "required": ["b"] } | ||
| ] | ||
| } | ||
| }, | ||
| "instance": { "a": 1, "b": 2 }, | ||
| "errors": [ | ||
| { | ||
| "messageId": "not-message", | ||
| "instanceLocation": "#", | ||
| "schemaLocations": ["#/not"], | ||
| "alternatives": [ | ||
| [ | ||
| { | ||
| "messageId": "required-success", | ||
| "messageParams": { "property": "a" }, | ||
| "instanceLocation": "#", | ||
| "schemaLocations": ["#/not/oneOf/0/required"] | ||
| } | ||
| ], | ||
| [ | ||
| { | ||
| "messageId": "required-success", | ||
| "messageParams": { "property": "b" }, | ||
| "instanceLocation": "#", | ||
| "schemaLocations": ["#/not/oneOf/1/required"] | ||
| } | ||
| ] | ||
| ] | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
This one's not quite correct. The oneOf will fail in this example, which means not will pass and there should be no errors. You'd have to remove either "a" or "b" from the instance to get a failure.
My first thought was that I expected there to be a oneOf node in the output. But, then I realized that might not make sense in this case. Let's explore this case a bit more. Try writing out the error message and let's see if we can come up with something that's helpful and computable.
There was a problem hiding this comment.
Yes, I got it. I should change the instance to { "a": 1 } which makes oneOf pass and not fail but I think the oneOf node in the output will be too abstract. not does not care about oneOf internal logic. It only cares that the subschema matched. Error message can be something like "The value matches a schema that it should not".
"errors": [
{
"messageId": "not-message",
"instanceLocation": "#",
"schemaLocations": ["#/not"],
"alternatives": [
[
{
"messageId": "required-success",
"messageParams": { "property": "a" },
"instanceLocation": "#",
"schemaLocations": ["#/not/oneOf/0/required"]
}
]
]
}
]
2 participants
Added test cases for
oneOffor now focusing on edge cases where validation fails due to multiple successful matches.Related to issue: #120
Planning to add similar tests for not and contains after review.