Update unevaluatedProperties and unevaluatedItems tests

[jdesrosiers]

These are the tests I used to implement unevaluatedProperties and unevaluatedItems. They cover a lot more than the current tests, so now I'm sharing if you want them.

[Julian]

Amazing, thanks! Will have a look and leave any comments but this looks great!

[gregsdennis]

It looks like you're replacing the existing tests rather than adding new ones. Is that correct?

[jdesrosiers]

Yeah, I hope that's ok. Let me know if there's something you think I should add back in.

[gregsdennis]

Those tests highlighted a failure in my lib, so I'd like the scenarios (or similar) to remain.

[jdesrosiers]

I think everything in the existing tests are covered and more. I'm reluctant to just restore the tests as they were because I think they will at least mostly duplicate the other tests. I'd rather add tests focused on what's missing. I'm just not sure what's missing.

[Julian]

Yeah I think we basically need to go through each old test and ensure each is represented in the new ones.

[Julian]

I may not have a chance to do so myself for a few more days -- if either of you (or anyone else) give it a shot it'd obviously be super appreciated otherwise yeah will try to follow up with some comments as soon as I can.

[jdesrosiers]

I went over it a third time to make a matrix to help others do their own analysis.

This time, I noticed something that was missing. It doesn't test that properties declared in a failed if are counted as unevaluated. I'll update the PR to cover that case.

unevaluatedProperties.json

• old#/0/tests/0 - covered by new#/8/tests/1

• old#/0/tests/1 - covered by new#/8/tests/0

• old#/0/tests/2 - duplicate of old#/0/tests/0

• old#/1/tests/0 - covered by new#/1/tests/0

• old#/1/tests/1 - covered by new#/13/tests/0 and new#/1/tests/0

• old#/1/tests/2 - covered by new#/13/tests/1 and new#/1/tests/1

• old#/1/tests/3 - covered by new#/13/tests/1 and new#/1/tests/0

• old#/1/tests/4 - covered by new#/13/tests/3 and new#/1/tests/0

• old#/1/tests/5 - covered by new#/1/tests/1 and new#/13/tests/3

• old#/1/tests/6 - covered by new#/13/tests/3 and new#/1/tests/0

unevaluatedItems.json

• old#/0/tests/0 - covered by new#/9/tests/2

• old#/0/tests/1 - covered by new#/9/tests/0

• old#/0/tests/2 - covered by new#/9/tests/0

• old#/0/tests/3 - covered by new#/9/tests/1

• old#/0/tests/4 - covered by new#/9/tests/0

• old#/1/tests/0 - covered by new#/2/tests/0

• old#/1/tests/1 - covered by new#/12/tests/0 and new#/2/tests/0

• old#/1/tests/2 - covered by new#/12/tests/0 and new#/2/tests/0

• old#/1/tests/3 - covered by new#/12/tests/1 and new#/2/tests/1

• old#/1/tests/4 - covered by new#/12/tests/1 and new#/2/tests/2

• old#/1/tests/5 - covered by new#/12/tests/3 and new#/2/tests/2

• old#/1/tests/6 - covered by new#/12/tests/3 and new#/2/tests/2

[ssilverman]

@jdesrosiers I notice these lines in your matrix:

• unevaluatedProperties.json:
  • old#/1/tests/0 - covered by new#/1/tests/0 and new#/1/tests/0
• unevaluatedItems.json:
  • old#/1/tests/0 - covered by new#/2/tests/0 and new#/2/tests/0

These look like they may have typos since there's duplicates.

[jdesrosiers]

@ssilverman It looks like it's just a duplicate. I edited the matrix. Thanks!

[jdesrosiers]

I found something that was missing and added a couple more tests.

[jdesrosiers]

I know there's a lot going on right now, but I wanted to bump this issue so it doesn't get lost. I've been over it in detail four times now and I'm very confident that there isn't anything in the old tests that isn't covered in the new tests.

[Julian]

Yeah I had a quick look myself too and I think it looked good to me too. Merging, and thanks, appreciated!

[jviotti]

My library is failing due to one of these tests, and I'm struggling to find the reasoning behind it. This is the test case:

{
    "description": "unevaluatedProperties with not",
    "schema": {
        "type": "object",
        "properties": {
            "foo": { "type": "string" }
        },
        "not": {
            "not": {
                "properties": {
                    "bar": { "const": "bar" }
                },
                "required": ["bar"]
            }
        },
        "unevaluatedProperties": false
    },
    "tests": [
        {
            "description": "with unevaluated properties",
            "data": {
                "foo": "foo",
                "bar": "bar"
            },
            "valid": false
        }
    ]
}

As I understand it, double negation is the same as not using negation. I.e. not not X = X. If that holds, then the schema:

{
    "type": "object",
    "properties": {
        "foo": { "type": "string" }
    },
    "not": {
        "not": {
            "properties": {
                "bar": { "const": "bar" }
            },
            "required": ["bar"]
        }
    },
    "unevaluatedProperties": false
}

Is semantically equivalent to the following schema:

{
    "type": "object",
    "properties": {
        "foo": { "type": "string" },
        "bar": { "const": "bar" }
    },
    "required": ["bar"],
    "unevaluatedProperties": false
}

Which means that the result is true, as there are no unevaluated properties (my library is behaving in this way).

Is the intention that schemas inside a not do not count as evaluating properties?

[karenetheridge]

Is the intention that schemas inside a not do not count as evaluating properties?

No, not quite. I wonder how you are implementing the not keyword, because things "should" work properly without any special treatment: the unevaluated keywords are implemented on the basis of annotations being generated against the relevant instance data locations, but not can never return any annotations: in order for annotations to be returned from not, the subschema inside the not must have failed, and failed schemas can't return annotations. So even though the property did get evaluated inside the not-not, schemas outside have no way of knowing because the double-not ensures they cannot be returned.

I hope this makes sense, and good luck to you in getting to the bottom of it in your implementation 👯‍♀️

[jviotti]

@karenetheridge Thanks for the quick response!

in order for annotations to be returned from not, the subschema inside the not must have failed, and failed schemas can't return annotations

I think that clarifies everything!