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

Add table of errors to test README (#706) #850

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

catamorphism
Copy link
Collaborator

Update README for tests directory to include a table relating error names in the test schema to error names in the spec.

@catamorphism catamorphism marked this pull request as ready for review July 30, 2024 19:21
test/README.md Outdated Show resolved Hide resolved
@eemeli eemeli linked an issue Jul 30, 2024 that may be closed by this pull request
Co-authored-by: Eemeli Aro <[email protected]>
Copy link
Member

@aphillips aphillips left a 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.

Comment on lines +46 to +49
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.
Copy link
Member

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:

Suggested change
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.

Copy link
Collaborator

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.

test/README.md Show resolved Hide resolved
@catamorphism catamorphism merged commit 22f5ac5 into unicode-org:main Aug 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We need a list of possible error codes
4 participants