description of additionalItems test case is insufficient / misleading

[gregsdennis]

trafficstars

Test:

https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/83e866b46c9f9e7082fd51e83a61c5f2145a1ab7/tests/draft7/additionalItems.json#L120-L135

While it's true that additionalItems can't see into applicators, the reason this passes validation is actually because the default value for items is an empty schema, which evaluates all items, leaving none for additionalItems to operate on.

We already have a test for default items, though, which kind of makes this one redundant.

We also have an invalid test immediately below the test in question that could easily have a valid test data case added to it. This test properly verifies that the additionalItems is failing on the second element.

[jdesrosiers]

Looks like that discussion was had at the time, but it was merged anyway https://github.com/json-schema-org/JSON-Schema-Test-Suite/pull/344. After thinking about it for a while, I don't think there's a way to write this test that makes sense.

[gregsdennis]

I don't think there's a way to write this test that makes sense.

As I mentioned, the test that immediately follows it has a "failing validation" case, and it's pretty easy to add a "passing validation" case to that test. Then we just remove this one.

[jdesrosiers]

The only way to make a passing test is a one element array, but then it's not testing additionalItems in any way. For the second item in the array, additionalItems is saying it must be a boolean and /allOf/items/1 is saying it must be a string. There's no value that can be a string and a boolean at the same time, so there is no meaningful passing test for this case.

I think just removing the ", valid case" test is the right move. I think the ", invalid case" test effectively covers what it intends to cover without a passing test even if that was possible.