Skip to content

oneOf tests for success-caused failures#159

Draft
RitoG09 wants to merge 3 commits intohyperjump-io:mainfrom
RitoG09:test/success-explanation-cases
Draft

oneOf tests for success-caused failures#159
RitoG09 wants to merge 3 commits intohyperjump-io:mainfrom
RitoG09:test/success-explanation-cases

Conversation

@RitoG09
Copy link

@RitoG09 RitoG09 commented Feb 3, 2026

Added test cases for oneOf for 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.

@jdesrosiers jdesrosiers marked this pull request as draft February 4, 2026 00:46
Copy link
Collaborator

@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.

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.

]
},
{
"description": "not with nested schema producing a match",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This description doesn't match the test. I don't see any nesting.

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the description and added a not with nested schema.

Comment on lines +54 to +79
{
"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"]
}
]
]
}
]
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Comment on lines +80 to 116
{
"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"]
}
]
]
}
]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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"]
          }
        ]
      ]
    }
  ]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think I agree. If oneOf passes, it means one of the schemas passed and it's effectively a simple applicator in that case.

Error message can be something like "The value matches a schema that it should not".

We can't say "matches a schema". That's the kind of thing we're trying to avoid with this project. The reader shouldn't have to know anything about schemas to understand what's wrong with the data.

We can probably come up with a message for not that works ok, but I wonder if it would be better to provide a negated message for each keyword. For example, not-required would be something like The '{property}' property should not be present. It would probably make for a better experience, but it makes the handling even more complicated and we still don't really have a great solution for success messaging. Adding negation messaging might not be that simple.

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