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

Add a `specification` property to test cases + tests #699

Open Julian opened 8 months ago

Julian commented 8 months ago

There isn't a fixed spot in tests right now which indicates which portion, section, or text of the specification it covers. The suite's main purpose is of course to faithfully represent the specification, so essentially all tests should have some direct basis in the specification, but we leave that for someone interested to have to re-locate, even though this work should have been done before adding each individual test (by at least 2 people presumably -- the test author as well as the reviewer). Right now we occasionally use comments to indicate this information, but having a specific spot for it could open up possibilities, but we should earmark a spot in each test where we include this information.

Considerations:

Era-cell commented 3 months ago

Hi @Julian I am interested, and I think this will useful for upcoming project. I have some doubts: For cases like escaped pointer: https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/604f5f99bfb669b38487e627890dd07b79780b18/tests/draft2020-12/ref.json#L374

Where should the re-location be done?

and for cases like $ref to else : https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/604f5f99bfb669b38487e627890dd07b79780b18/tests/draft2020-12/ref.json#L920

Should we point the location for both ref and else?

gregsdennis commented 3 months ago

Where should the re-location be done?

"Re-locate" here means "locate again;" as opposed to "relocate" which means "move". English is dumb.

The point is that the reader of the test has to find the requirement in the specification text, when we would have known where the requirement was when writing the test and could have just pointed to it from the beginning.

Era-cell commented 3 months ago

Where should the re-location be done?

"Re-locate" here means "locate again;" as opposed to "relocate" which means "move". English is dumb.

The point is that the reader of the test has to find the requirement in the specification text, when we would have known where the requirement was when writing the test and could have just pointed to it from the beginning.

I intended to say where should we point to, for the first case. If I had used the word "refer to" that would have caused ambiguity with $ref. So😅 And yeah, we can use the urls of the spec and rfc right? Inorder to point the location where particular test case belongs to...

Eg,

{
"specification":{
    "jsonschema spec":"https://json-schema.org/draft/2020-12/json-schema-core#name-direct-references-with-ref",
    "rfc spec":"url for rfc spec location"
    }
}
Era-cell commented 3 months ago

@Julian @gregsdennis I have some doubts:

Julian commented 3 months ago

can I get any usage example of these $comments.

I'm afraid I don't understand your other questions, particularly what you mean by "relocation".

The spot that goes there should be the "canonical place in the specification which defines the behavior". If there doesn't seem to be one, that should be raised as an issue.

Era-cell commented 3 months ago

Hey! I previously thought that we should be using URLs which will be redirecting to location where test case belongs. Canonical place is good, I understood that. I'll just reframe unclear questions:

Julian commented 3 months ago

Can I provide the canonical location for an example which belongs to an other section - like ("output" format ) section. For the "additionalProperties" test - case.

No, an example is never a canonical location for something. "Canonical" means "the authoritative place where it is defined". The specification never uses an example to define something, an example is simply to illustrate something.

And for the test case which has explanation in RFC but not in jsonschema spec(also no explicit mention of that RFC in jsonschema spec) will be considered to raise an issue?

No, it's expected that those are defined in their respective RFCs. The JSON Schema spec will however contain a spot where it mentions which RFC it is referencing, and that's interesting to include.

Era-cell commented 3 months ago

No, it's expected that those are defined in their respective RFCs. The JSON Schema spec will however contain a spot where it mentions which RFC it is referencing, and that's interesting to include.

Actually for the test-case: "additionalProperties with non-ascii characters" jsonschema spec doesn't say about property names. But rfc 8259 tells about object names for any json document. In that case--> should I give the reference to that RFC or should I raise an issue?

Julian commented 3 months ago

Linking to simply the definition of additionalProperties (i.e. https://json-schema.org/draft/2020-12/json-schema-core#additionalProperties) seems like the right thing there, as there's no special behavior whatsoever about non-ASCII characters. Yes you're right the JSON spec does say that non-ASCII characters are allowed, but given that every single test in the repo will also somehow depend on the JSON Spec it seems like quite a bit of work to link to the JSON RFC, work that I don't think seems very useful at first glance.

Era-cell commented 3 months ago

Linking to simply the definition of additionalProperties (i.e. https://json-schema.org/draft/2020-12/json-schema-core#additionalProperties) seems like the right thing there, as there's no special behavior whatsoever about non-ASCII characters. Yes you're right the JSON spec does say that non-ASCII characters are allowed, but given that every single test in the repo will also somehow depend on the JSON Spec it seems like quite a bit of work to link to the JSON RFC, work that I don't think seems very useful at first glance.

Thanks @Julian , this task should be much easier then. Can I be assigned or just make PRs? I you can review my updated PR that would be great

Julian commented 3 months ago

Just sending PRs I think is fine, as just one PR is certainly not going to cover everything, so presumably more than one person can work on it at once. I'll have a look, though I saw both Greg and Karen had left some comments there, their review is certainly as good as mine, but I'll have a look as well.

Julian commented 3 months ago

This work was started in #726 so we now have an initial schema here to work on top of!