-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add table of errors to test README (#706) #850
Add table of errors to test README (#706) #850
Conversation
Co-authored-by: Eemeli Aro <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor issues. Thanks for making this PR.
The "Message Function Error" error name used in the spec | ||
is not included in the schema, | ||
as it is intended to be an umbrella category | ||
for implementation-specific errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine writing tests for my own formatting functions that might use such an error. Would we be better off to define the schema name and then say:
The "Message Function Error" error name used in the spec | |
is not included in the schema, | |
as it is intended to be an umbrella category | |
for implementation-specific errors. | |
> [!NOTE] | |
> The error name `message-function-error` does not appear in any of the | |
> tests found in this test suite because the "Message Function Error" | |
> category of errors is intended as an umbrella category | |
> for implementation-specific errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current schema is designed to specifically support our own tests, and it's not really extensible e.g. due to incorporating "additionalProperties": false
requirements in many places. This means that adding message-function-error
is beyond its current intended use cases.
As JSON Schema in general is not extensible (it has no concept of inheritance, for instance), other test suites that would have such a need probably need to copy our schema and patch it locally, or not rely on an explicit JSON Schema document.
I found the PR's proposed language appropriate.
Update README for tests directory to include a table relating error names in the test schema to error names in the spec.