Test that oneOf handles missing optional property

[aznan2]

This shouldn't really be necessary, but at least one validation library fails this test.

{
  "description": "oneOf complex types",
  "schema": {
    "oneOf": [
      {
        "properties": {
          "bar": {"type": "integer"}
         /* , "baz": {"type": "boolean"} <-- new optional propery, not in data */
        },
        "required": ["bar"]
      },
      {
        "properties": {
          "foo": {"type": "string"}
        },
        "required": ["foo"]
      }
    ]
  },
  "tests": [
    {
      "description": "first oneOf valid (complex)",
      "data": {"bar": 2},
      "valid": true
    },
    {
      "description": "second oneOf valid (complex)",
      "data": {"foo": "baz"},
      "valid": true
    },
    {
      "description": "both oneOf valid (complex)",
      "data": {"foo": "baz", "bar": 2},
      "valid": false
    },
    {
      "description": "neither oneOf valid (complex)",
      "data": {"foo": 2, "bar": "quux"},
      "valid": false
    }
  ]
}

[Relequestual]

Yikes, that's concerning!
These tests can probably be simplified by using boolean values as schemas, although I can't test the failure of the library in question easily to confirm that.

[aznan2]

@Relequestual You are correct, the added property could be simplified as

"properties": {
  "bar": {"type": "integer"},
  "baz": true
},
"required": ["bar"]

[Relequestual]

Which of the tests does it fail? all of them?
I feel the name of the test needs to be more descriptive.

[aznan2]

Oh, in my example I've taken the existing test in oneOf.json with the description "oneOf complex types" and augmented it. Maybe it should be its own test.

The library fails test 1 and 3, {"bar": 2} and {"foo": "baz", "bar": 2}.

[Relequestual]

Ugh, sorry for having double standards on test naming...
We should probably have better names for our tests...

Out of interest, what does the library do with the following?:
https://jsonschema.dev/s/N4IgziBcLAOgdgAkbE8D2AXVlFyciiAA4BO6xApqZgJaVg54KGGoBGAhqU-q66kwBPKk1S14mSgHNqqFvwC+AGgUCQXAF681-QSMpiN6dABtKnePIKtFulbtSlKARwCutZwBMmAbV3IHNzWrAC6CnbwiiDKILRQMApB2lCIAEwI0YpAA

[Julian]

Was the bug in that library really rooted in something oneOf specific? No simpler schema exercised it?

…

On Tue, Nov 19, 2019, 10:48 Ben Hutton ***@***.***> wrote: Ugh, sorry for having double standards on test naming... We should probably have better names for our tests... Out of interest, what does the library do with the following: https://jsonschema.dev/s/N4IgziBcLAOgdgAkbE8D2AXVlFyciiAA4BO6xApqZgJaVg54KGGoBGAhqU-q66kwBPKk1S14mSgHNqqFvwC+AGgUCQXAF681-QSMpiN6dABtKnePIKtFulbtSlKARwCutZwBMmAbV3IHNzWrAC6CnbwiiDKILRQMApB2lCIAEwI0YpAA — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#307?email_source=notifications&email_token=AACQQXUYNBMKGVXUUE534CLQUQDEXA5CNFSM4JPEJSRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEOU4YY#issuecomment-555568739>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQQXXRHJT3KL3YKGICNDTQUQDEXANCNFSM4JPEJSRA> .

[aznan2]

@Relequestual The library succeeds in validating your example.

@Julian The bug stems from the fact that the validator, if a property is missing, checks and reacts to whether it is a "complex validator" (allOf/anyOf/oneOf) before checking if the property is required.

[Julian]

Got it -- well, fair enough, if it's a bug that happened in the wild it's something we always have added -- happy to see a PR adding a new test for these -- do you think you'd have time to send one?

[Julian]

Can you just make sure to mention the downstream issue in either or both of the test case name or the commit message?

[Relequestual]

@aznan2 Julian said what I was trying to allude to.
The location of a subschema should not change the validation result of that subschema by itself.
The combination of an array of subschema validation results should be collected and modified by the encasing applicator keyword, if any, such as *Of and not for example.

[aznan2]

Sure. Would you prefer if I modify the existing "complex types" test or should this be a new test case?

[Julian]

It should be new

…

On Thu, Nov 21, 2019, 03:30 aznan2 ***@***.***> wrote: Sure. Would you prefer if I modify the existing "complex types" test or should this be a new test case? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#307?email_source=notifications&email_token=AACQQXS3NDCTIB6CR4N3YBDQUZBJHA5CNFSM4JPEJSRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEZMKMQ#issuecomment-556975410>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACQQXUJKTJFTVBXRAIR4VDQUZBJHANCNFSM4JPEJSRA> .

[aznan2]

Created the PR #308

[aznan2]

Ugh. Can you guys fill me in on why my PR is breaking the build?

[Julian]

It's because you have a boolean schema there, but the test suite still uses draft 4 for its schemas.

Updating that is #238, but for now the easiest thing to do is likely just to switch to using {} instead of true.

[Julian]

Sorry, it's too early in the morning clearly for me to be responding to issues.

Scratch the above, it isn't related to #238, but is you using a boolean schema in the draft 4 folder which draft4 didn't have those -- so you do need to use {} but just in that one file.

[aznan2]

Yay, thank you!