Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declare "format" assertion test cases in a way that plays more nicely with vocabularies #513

Closed
jviotti opened this issue Sep 7, 2021 · 5 comments

Comments

@jviotti
Copy link
Member

jviotti commented Sep 7, 2021

The test suite currently distinguishes between the assertion and annotation format vocabularies by putting the assertion format test cases inside the optional subdirectory: https://github.com/json-schema-org/JSON-Schema-Test-Suite/tree/master/tests/draft2020-12/optional/format.

However, this setup doesn't play nicely with the concept of vocabularies as it treats this difference as an exceptional case to be handled by the JSON Schema implementor. I think that a more natural way to express this would be the following:

  • Introduce a meta-schema that uses $vocabulary to set the format assertion vocabulary instead of its annotation counterpart
  • Change all the test cases that assert on format to use such meta-schema and the test cases that treat format as an annotation to use the "official" meta-schema that uses the annotation vocabulary
  • Move the assertion format test cases out of optional

This change forces implementors to understand $vocabulary and implement the right semantics accordingly and be closer with the specification rather than just saying that "format" is optional.

@karenetheridge
Copy link
Member

karenetheridge commented Sep 8, 2021

That is the intention, but no one has gotten around to doing it yet :)

Do however note that implementing the format-assertion vocabulary is still optional, so the tests would need to remain where they are regardless.

@Relequestual
Copy link
Member

We probably need to break tests down into vocabulary folders, which also contain optional tests per vocabulary.
We need to do a lot of thinking around tests... but this is a good discussion.

@jviotti
Copy link
Member Author

jviotti commented Sep 8, 2021

Do however note that implementing the format-assertion vocabulary is still optional, so the tests would need to remain where they are regardless.

I'm wondering if there are two levels of "optionality" that we are discussing together. Whether a vocabulary is optional or not depends on how the schema-writer declares $vocabulary, though. So in that sense, a vocabulary optionality is not an intrinsic characteristic of the vocabulary, but rather a characteristic of the context (i.e. on whether the meta-schema declares it as true or false in $vocabulary).

On the other hand, there seems to be an implicit definition of what vocabularies are required to be supported by an implementation with a certain degree of maturity, and format-assertion is not one of them. However, this feels counterintuitive to me regarding the whole vocabulary concept. I believe the specification states that the only "required" vocabulary that any implementation must implement is Core. Yet, the test suite sends the message that all vocabularies except format-assertion are required to be implemented.

In my opinion, the "optional" directory makes sense for test cases that i.e. cannot be implemented in some programming languages (like some floating-point related tests that are not satisfiable on JavaScript), but it feels to me that according to the spec, only the Core vocabulary is strictly required and the rest are all within the same category of required-ness (including format-assertion).

Am I thinking in the right direction?

@jdesrosiers
Copy link
Member

Ha, you're right. The term "optional" is far too overloaded.

I believe the specification states that the only "required" vocabulary that any implementation must implement is Core.

Not quite. If you create a custom JSON Schema dialect, the core and applicator vocabularies are required to be implemented (although it really should just be core). If you are implementing the standard JSON Schema dialect, you are required to implement all the vocabularies listed in the dialect meta-schema. That doesn't include format-assertion. That means format-assertion is "optional" in the context of being compatible with the standard draft release.

Using a custom dialect meta-schema that includes the format-assertion vocab is definitely the right way to enable that feature, but it can't be in the non-optional tests because it requires a feature that is not required by the standard dialect meta-schema. An implementation that doesn't support format-assertion would fail those tests despite being perfectly compatible with the standard dialect.

So, format-assertion tests have to be in "optional" because they are not required by the standard dialect. But, as @Relequestual said, it might make more sense to organize things by vocabulary rather than by dialect. Then we would have a better place to put format-assertion tests rather than lumping them in with "optional" which is arguably not really the best place for them.

@Julian
Copy link
Member

Julian commented Jul 1, 2022

Closing this in favor of #495 which is very similar ("what layout better supports vocabularies and format in particular"). Further thoughts obviously still welcome.

@Julian Julian closed this as completed Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants