Tests for propertyNames with additionalProperties/unevaluatedProperties

json-schema-org / JSON-Schema-Test-Suite

A language agnostic test suite for the JSON Schema specifications

MIT License

603 stars 205 forks source link

Tests for propertyNames with additionalProperties/unevaluatedProperties #733

Closed Era-cell closed 3 months ago

Era-cell commented 3 months ago

Addressing the issue: 305. Also inculded "specification" property, I have added it such a way that property names cover pattern and length constraints too. Please review my PR

Era-cell commented 3 months ago

As I described in #732, were testing additionalProperties and unevaluatedProperties here, not propertyNames. These tests need to be moved to the appropriate files.

H.. Actually

As I described in #732, were testing additionalProperties and unevaluatedProperties here, not propertyNames. These tests need to be moved to the appropriate files.

Huh.. this PR is related to a different issue than issue #626. The issue #305 description says:

Verify that "propertyNames" and "additionalProperties"/"unevaluatedProperties" don't interact.
This has come up as a question a few times. propertyNames doesn't interact with other keywords. I feel this issue wants to address more on properttyNames rather than additionalProperties and unevaluatedProperties. So, can you please confirm on that...

MeastroZI commented 3 months ago

@Era-cell, propertyNames is functioning as expected, but there is change in the behavior of additionalProperties and unevaluatedProperties for propertyNames. So because of that tests related to these properties should be included in the additionalProperties and unevaluatedProperties files.

gregsdennis commented 3 months ago

but there is change in the behavior of additionalProperties and unevaluatedProperties for propertyNames.

No. There is no change in behavior. The tests are to verify that.

Era-cell commented 3 months ago

These look good. Please add them to 2019 and "next".

I saw just now, that the tests are present already for unevaluatedProperties from 2019 - draft-next, already. So, I have considered to add tests only for additionalProperties

MeastroZI commented 3 months ago

but the spec could be misread that propertyNames defines properties that additionalProperties would then skip. That's what the issue is about.

So for this schema
{"propertyNames": {
"maxLength": 5
},
"additionalProperties": false
}
Should the data { apple: "apple" } be considered valid or not? because It appears we don't have tests for this specific case also.

gregsdennis commented 3 months ago

Correct, { "apple": "apple" } is an invalid instance for that schema.

We do have that test with the latest update: see line 175 in the first file.

Era-cell commented 3 months ago

I have updated the changes, and Thank you