Skip to content

oneOf tests for success-caused failures #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
RitoG09 wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

oneOf tests for success-caused failures #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af6d60f
oneOf tests for success-caused failures
RitoG09 Feb 3, 2026
4cbf3f0
not, contain test cases added
RitoG09 Feb 4, 2026
2c497cf
desc update & not nested test added
RitoG09 Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions src/test-suite/tests/contains.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,88 @@
},
"instance": ["foo", 42],
"errors": []
},
{
"description": "contains fails due to too many matching items with success explanations",
"compatibility": "2019",
"schema": {
"contains": { "type": "string" },
"maxContains": 1
},
"instance": [42, "foo", "bar"],
"errors": [
{
"messageId": "contains-exact-message",
"messageParams": {
"minContains": 1,
"maxContains": 1
},
"instanceLocation": "#",
"schemaLocations": ["#/contains", "#/maxContains"],
"alternatives": [
[
{
"messageId": "type-success",
"messageParams": {
"actualType": "string"
},
"instanceLocation": "#/1",
"schemaLocations": ["#/contains/type"]
}
],
[
{
"messageId": "type-success",
"messageParams": {
"actualType": "string"
},
"instanceLocation": "#/2",
"schemaLocations": ["#/contains/type"]
}
]
]
},
{
"description": "contains fails with multiple object matches",
"compatibility": "2019",
"schema": {
"contains": {
"required": ["a"]
},
"maxContains": 1
},
"instance": [{ "a": 1 }, { "a": 2 }],
"errors": [
{
"messageId": "contains-exact-message",
"messageParams": {
"minContains": 1,
"maxContains": 1
},
"instanceLocation": "#",
"schemaLocations": ["#/contains", "#/maxContains"],
"alternatives": [
[
{
"messageId": "required-success",
"messageParams": { "property": "a" },
"instanceLocation": "#/0",
"schemaLocations": ["#/contains/required"]
}
],
[
{
"messageId": "required-success",
"messageParams": { "property": "a" },
"instanceLocation": "#/1",
"schemaLocations": ["#/contains/required"]
}
]
]
}
]
}
]
}
]
}
89 changes: 89 additions & 0 deletions src/test-suite/tests/not.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,95 @@
},
"instance": "foo",
"errors": []
},
{
"description": "not fails with success explanation",
"schema": {
"not": { "pattern": "^a" }
},
"instance": "apple",
"errors": [
{
"messageId": "not-message",
"instanceLocation": "#",
"schemaLocations": ["#/not"],
"alternatives": [
[
{
"messageId": "pattern-success",
"messageParams": {
"pattern": "^a"
},
"instanceLocation": "#",
"schemaLocations": ["#/not/pattern"]
}
]
]
}
]
},
{
"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"]
}
]
]
}
]
},
Comment on lines +54 to +79
Copy link
Collaborator

@jdesrosiers jdesrosiers Feb 9, 2026

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

@RitoG09 RitoG09 Feb 12, 2026

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.

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

@jdesrosiers jdesrosiers Feb 9, 2026

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

@RitoG09 RitoG09 Feb 12, 2026

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

@jdesrosiers jdesrosiers Feb 17, 2026

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.

Copy link
Author

@RitoG09 RitoG09 Feb 26, 2026

Choose a reason for hiding this comment

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

Ok, How i am thinking about the negated message is ->

let keywords optionally define a negated form of their message at the localization level, rather than introducing a global success or failure mode flag.

In this case:
current format: The '{property}' property is required.
what we need: The '{property}' property should not be present

Then not wouldn’t need to understand the internal logic of required or oneOf it would just delegate to the inner keyword and request its negated form when available. If a keyword doesn’t define a negated message, we could fall back to the generic not message plus structured alternatives.

So we are avoiding: requiredErrorHandler(normalizedErrors, instance, localization, { negated: true })
(it introduces the boolean flag)
Rather we can do: getRequiredNegatedMessage(property)
(let localization decide)

I’m not sure if this fully avoids complexity, but it seems more localized than introducing parallel success or negated handlers everywhere.

Copy link
Collaborator

@jdesrosiers jdesrosiers Mar 3, 2026

Choose a reason for hiding this comment

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

I don't think any of that helps us avoid needing to pass a negated flag around. I'm thinking we'll need some sort of getPassing analog of getErrors. Then, getPassing would have to have a negated flag that could be passed to each keyword so it can be passed to the Localization message function that decides which version of the message to send.

But I'm pretty sure that requires a separate pass and fail handler for each keyword. So, the thing we have to figure out is if we can make handlers be able to handle both pass and fail cases given a flag. That might not be be too hard, but applicator keywords might have some complications.

Copy link
Author

@RitoG09 RitoG09 Mar 9, 2026

Choose a reason for hiding this comment

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

That makes sense.
Current flow -> jsonSchemaErrors → normalizedOutput → getErrors

Right now handlers only produce messages when the keyword fails. For example, requiredErrorHandler gathers missing properties and calls localization.getRequiredErrorMessage(...). If the keyword passes it simply returns [].
To support cases like not as you said we’d need something like a getPassing analog of getErrors that traverses the same normalized output but collects explanations for successful assertions instead of failures.
Code sample (conceptual):

async function getPassing(normalizedErrors, instance, localization, { negated = false }) {
  const passing = [];

  for (const handler of handlers) {
    const messages = await handler(normalizedErrors, instance, localization, {
      mode: "pass",
      negated
    });
    passing.push(...messages);
  }

  return passing;
}

Then handlers could support both modes:

const requiredHandler = async (normalizedErrors, instance, localization, { mode, negated }) => {
  const required = ["a"]; // simplified

  const present = required.filter((p) => Instance.has(p, instance));

  if (mode === "pass" && present.length) {
    if (negated) {
      return [{
        message: localization.getRequiredNegatedMessage(present),
        instanceLocation: Instance.uri(instance)
      }];
    }

    return [{
      message: localization.getRequiredSuccessMessage(present),
      instanceLocation: Instance.uri(instance)
    }];
  }

  if (mode === "fail") {
    // existing required error logic
  }

  return [];
};

And not could then call something like:
await getPassing(subschemaOutput, instance, localization, { negated: true });

And for applicator keywords as you have said it likely have more complications since they need to coordinate nested schemas, i would suggest to implement incrementally keyword by keyword starting with simple one to see how well the approach works before tackling applicators.

]
}
128 changes: 119 additions & 9 deletions src/test-suite/tests/oneOf.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
{
"description": "oneOf",
"schema": {
"oneOf": [
{ "type": "string" },
{ "type": "number" }
]
"oneOf": [{ "type": "string" }, { "type": "number" }]
},
"instance": null,
"errors": [
Expand Down Expand Up @@ -46,10 +43,7 @@
{
"description": "oneOf more than one match",
"schema": {
"oneOf": [
{ "type": "integer" },
{ "type": "number" }
]
"oneOf": [{ "type": "integer" }, { "type": "number" }]
},
"instance": 1,
"errors": [
Expand Down Expand Up @@ -100,7 +94,7 @@
]
},
{
"description": "oneOf pass",
"description": "oneOf more than one match with an additional failing alternative",
"schema": {
"oneOf": [
{ "type": "string" },
Expand Down Expand Up @@ -272,6 +266,122 @@
]
}
]
},
{
"description": "oneOf more than one match with success explanations",
"schema": {
"oneOf": [{ "type": "integer" }, { "type": "number" }]
},
"instance": 66,
"errors": [
{
"messageId": "oneOf-message",
"messageParams": { "matchCount": 2 },
"alternatives": [
[
{
"messageId": "type-success",
"messageParams": {
"actualType": "integer"
},
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/0/type"]
}
],
[
{
"messageId": "type-success",
"messageParams": {
"actualType": "number"
},
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/1/type"]
}
]
],
"instanceLocation": "#",
"schemaLocations": ["#/oneOf"]
}
]
},
{
"description": "oneOf more than one match with failed alternatives ignored",
"schema": {
"oneOf": [
{ "required": ["a"] },
{ "required": ["b"] },
{ "required": ["c"] }
]
},
"instance": { "a": 1, "b": 2 },
"errors": [
{
"messageId": "oneOf-message",
"messageParams": { "matchCount": 2 },
"alternatives": [
[
{
"messageId": "required-success",
"messageParams": {
"property": "a"
},
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/0/required"]
}
],
[
{
"messageId": "required-success",
"messageParams": {
"property": "b"
},
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/1/required"]
}
]
],
"instanceLocation": "#",
"schemaLocations": ["#/oneOf"]
}
]
},
{
"description": "nested oneOf producing multiple matches in inner oneOf",
"schema": {
"oneOf": [
{
"oneOf": [{ "required": ["a"] }, { "required": ["b"] }]
},
{ "required": ["c"] }
]
},
"instance": { "a": 1, "b": 2 },
"errors": [
{
"messageId": "oneOf-message",
"messageParams": { "matchCount": 2 },
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/0/oneOf"],
"alternatives": [
[
{
"messageId": "required-success",
"messageParams": { "property": "a" },
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/0/oneOf/0/required"]
}
],
[
{
"messageId": "required-success",
"messageParams": { "property": "b" },
"instanceLocation": "#",
"schemaLocations": ["#/oneOf/0/oneOf/1/required"]
}
]
]
}
]
}
]
}