Skip to content

Make expErrors use an array (and only an array) #1076

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.

Sign up for GitHub

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

Merged
aphillips merged 12 commits into from
Jul 7, 2025
Merged

Make expErrors use an array (and only an array) #1076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3f11bcb
Issue 993: make expErrors only
mihnita Jun 2, 2025
51a2a4b
Adding `expErrors` values for fallback.json
mihnita Jun 8, 2025
5db6c5d
Update test/README.md
mihnita Jun 8, 2025
7cde5a3
Update test/README.md
mihnita Jun 8, 2025
679ec61
Update test/tests/fallback.json
mihnita Jun 8, 2025
989bdc0
Update test/tests/fallback.json
mihnita Jun 8, 2025
542e6ec
Update test/tests/fallback.json
mihnita Jun 8, 2025
c5f71d3
Update test/tests/fallback.json
mihnita Jun 8, 2025
208f524
Fix 'Bad Operand' to 'Bad Option'
mihnita Jun 30, 2025
ae7b8d7
Update test/README.md
mihnita Jul 7, 2025
08b1cbf
Fix test case with proper error code
mihnita Jul 7, 2025
c43fd62
Fix test case with proper error code
mihnita Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions test/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ the behaviour of calling it as the `rv` value of MatchSelectorKeys(`rv`, `keys`)
depends on its `Input`, `DecimalPlaces` and `FailsSelect` values.

- If `FailsSelect` is `true`,
calling the method will emit a _Message Function Error_
calling the method will emit a _Bad Option_ error
and not return any value.
- If the `Input` is 1 and `DecimalPlaces` is 1,
the method will return some slice of the list « `'1.0'`, `'1'` »,
Expand Down Expand Up @@ -185,8 +185,14 @@ rather than being concatenated into a single string.

If `FailsFormat` is `true`,
attempting to format the _placeholder_ to any formatting target will
emit a _Message Function Error_.

emit a _Bad Option_ error.

> Note that emitting _Bad Option_ here does not indicate an incorrect option.
> Actual functions in your implementation might emit
> an implementation-defined or platform-specific runtime error or exception
> when the function handler is called.
> Your implementation might thus produce a _Message Function Error_
> not provided with a label in the JSON Schema of this test suite.
### `:test:select`

This _function_ accepts the same _operands_ and _options_,
Expand Down
5 changes: 1 addition & 4 deletions test/schemas/v0/tests.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,10 +382,7 @@
},
"expErrors": {
"description": "The runtime errors expected to be emitted when formatting the message. If expErrors is either absent or empty, the message must be formatted without errors.",
"type": [
"array",
"boolean"
],
"type": "array",
"items": {
"type": "object",
"additionalProperties": false,
Expand Down
27 changes: 17 additions & 10 deletions test/tests/fallback.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,59 @@
"description": "Test cases for fallback behaviour.",
"defaultTestProperties": {
"bidiIsolation": "none",
"locale": "en-US",
"expErrors": true
"locale": "en-US"
},
"tests": [
{
"description": "function with unquoted literal operand",
"src": "{42 :test:function fails=format}",
"exp": "{|42|}",
"expParts": [{ "type": "fallback", "source": "|42|" }]
"expParts": [{ "type": "fallback", "source": "|42|" }],
"expErrors": [{ "type": "bad-option" }]
},
{
"description": "function with quoted literal operand",
"src": "{|C:\\\\| :test:function fails=format}",
"exp": "{|C:\\\\|}"
"exp": "{|C:\\\\|}",
"expErrors": [{ "type": "bad-operand" }]
},
{
"description": "unannotated implicit input variable",
"src": "{$var}",
"exp": "{$var}"
"exp": "{$var}",
"expErrors": [{ "type": "unresolved-variable" }]
},
{
"description": "annotated implicit input variable",
"src": "{$var :number}",
"exp": "{$var}",
"expParts": [{ "type": "fallback", "source": "$var" }]
"expParts": [{ "type": "fallback", "source": "$var" }],
"expErrors": [{ "type": "unresolved-variable" }, { "type": "bad-operand" }]
},
{
"description": "local variable with unknown function in declaration",
"src": ".local $var = {|val| :test:undefined} {{{$var}}}",
"exp": "{$var}"
"exp": "{$var}",
"expErrors": [{ "type": "unknown-function" }]
},
{
"description": "function with local variable operand with unknown function in declaration",
"src": ".local $var = {|val| :test:undefined} {{{$var :test:function}}}",
"exp": "{$var}"
"exp": "{$var}",
"expErrors": [{ "type": "unknown-function" }, { "type": "bad-operand" }]
},
{
"description": "local variable with unknown function in placeholder",
"src": ".local $var = {|val|} {{{$var :test:undefined}}}",
"exp": "{$var}"
"exp": "{$var}",
"expErrors": [{ "type": "unknown-function" }]
},
{
"description": "function with no operand",
"src": "{:test:undefined}",
"exp": "{:test:undefined}",
"expParts": [{ "type": "fallback", "source": ":test:undefined" }]
"expParts": [{ "type": "fallback", "source": ":test:undefined" }],
"expErrors": [{ "type": "unknown-function" }]
}
]
}
24 changes: 9 additions & 15 deletions test/tests/functions/currency.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "The built-in formatter and selector for currencies.",
"defaultTestProperties": {
"bidiIsolation": "none",
"locale": "en-US"
"locale": "en-US",
"expErrors": []
},
"tests": [
{
Expand All @@ -24,33 +25,26 @@
"expErrors": [{ "type": "bad-operand" }]
},
{
"src": "{42 :currency currency=EUR}",
"expErrors": false
"src": "{42 :currency currency=EUR}"
},
{
"src": ".local $n = {42 :number} {{{$n :currency currency=EUR}}}",
"expErrors": false
"src": ".local $n = {42 :number} {{{$n :currency currency=EUR}}}"
},
{
"src": ".local $n = {42 :integer} {{{$n :currency currency=EUR}}}",
"expErrors": false
"src": ".local $n = {42 :integer} {{{$n :currency currency=EUR}}}"
},
{
"src": ".local $n = {42 :currency currency=EUR} {{{$n :currency}}}",
"expErrors": false
"src": ".local $n = {42 :currency currency=EUR} {{{$n :currency}}}"
},
{
"src": "{42 :currency currency=EUR fractionDigits=auto}",
"expErrors": false
"src": "{42 :currency currency=EUR fractionDigits=auto}"
},
{
"src": "{42 :currency currency=EUR fractionDigits=2}",
"expErrors": false
"src": "{42 :currency currency=EUR fractionDigits=2}"
},
{
"src": "{$x :currency currency=EUR}",
"params": [{ "name": "x", "value": 41 }],
"expErrors": false
"params": [{ "name": "x", "value": 41 }]
},
{
"src": ".local $n = {42 :currency currency=EUR} .match $n * {{other}}",
Expand Down
2 changes: 1 addition & 1 deletion test/tests/functions/date.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"defaultTestProperties": {
"bidiIsolation": "none",
"locale": "en-US",
"expErrors": false
"expErrors": []
},
"tests": [
{
Expand Down
2 changes: 1 addition & 1 deletion test/tests/functions/datetime.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"defaultTestProperties": {
"bidiIsolation": "none",
"locale": "en-US",
"expErrors": false
"expErrors": []
Copy link
Member

@aphillips aphillips Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fallback.md you left out expErrors. Here you supply an empty array. I think we should probably omit this one?

Copy link
Collaborator

@mradbourne mradbourne Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aphillips It's required in this file otherwise some tests would be left without an expectation (e.g. lines 44-55). This would also be the case in time.json.

As Eemeli mentioned in his review comment, I think we need to keep the expErrors in fallback.md.

Copy link
Collaborator Author

@mihnita mihnita Jun 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are talking about fallback.json, not fallback.md?

I updated fallback.json and added errors for each individual case, instead of doing it in defaultTestProperties

Copy link
Collaborator Author

@mihnita mihnita Jun 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For date.json, datetime.json and time.json I added "expErrors": [] to defaultTestProperties because the schema says that entries are required to have one of exp, expParts or expErrors

In these 3 files we had tests with src only.
So I had to add a expErrors (not enough info for exp, expParts, exact date/time formatting not part of the spec)

},
"tests": [
{
Expand Down
2 changes: 1 addition & 1 deletion test/tests/functions/time.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"defaultTestProperties": {
"bidiIsolation": "none",
"locale": "en-US",
"expErrors": false
"expErrors": []
},
"tests": [
{
Expand Down