docs(expr): PORTING.md: engines finding, exact match claims, runner commit, negative zero, lone surrogates (GO-007)
test(expr): parse errors must match the runner's message exactly in TestRunnerOracle
- commit
- edc4d79005ba9c462b120d2062fefae213392985
- author
- brice · · 2026-10-12 18:35 UTC
- committer
- brice · · 2026-10-18 14:59 UTC
- parent
- d706e56
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +72 -37
M expr/PORTING.md +70 -29
| @@ -18,8 +18,11 @@ Three kinds of evidence back the tests: | |||
| 18 | 18 | - `testdata/runner.json.gz`: what the runner's own code answers on the same | |
| 19 | 19 | expressions. `gen/runner` compiles `src/Sdk/DTExpressions2` (plus stubs for | |
| 20 | 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | |
| 21 | - | `TestRunnerOracle` holds the package to it: 34 765 cases, the only | |
| 22 | - | differences are negative zero (below) and error texts of Json.NET. | |
| 21 | + | `TestRunnerOracle` holds the package to it: 34 765 cases with the same | |
| 22 | + | result, string form, `toJSON` text and, for errors, the same message (parse | |
| 23 | + | errors included, to the position); the only exceptions are negative zero | |
| 24 | + | (below) and the message texts of Json.NET failures, which must be `fromJSON` | |
| 25 | + | failures on both sides. | |
| 23 | 26 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | |
| 24 | 27 | and `number_cases.txt`, and checked the number formatting on 200 000 random | |
| 25 | 28 | doubles. | |
| @@ -30,21 +33,53 @@ Three kinds of evidence back the tests: | |||
| 30 | 33 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | |
| 31 | 34 | ||
| 32 | 35 | The runner's own expression tests (`ExpressionParserL0`, `ConditionFunctionsL0`) | |
| 33 | - | are ported in `runner_l0_test.go`. The casing tables of the JavaScript | |
| 34 | - | (`casing.go`, `gen/casing*`) remain for `Lex`/oracle support only. | |
| 36 | + | are ported in `runner_l0_test.go`; they are the only tests of the expression | |
| 37 | + | engine in the runner's `src/Test/L0` (the other files that mention it test | |
| 38 | + | templates and actions). The case-mapping tables of the JavaScript (`casing.go`, | |
| 39 | + | `casing_tables.go`, `gen/casing*`) are kept for `Lex` and the oracle tests; the | |
| 40 | + | code `Parse` and `Evaluate` use for strings is `ordinal.go`, `number.go` and | |
| 41 | + | `json.go`. The WTF-8 helpers in `casing.go` are only reached through `Lex` and | |
| 42 | + | tests. | |
| 35 | 43 | ||
| 36 | - | Regenerating: `dotnet build -c Release expr/gen/runner` then the `*.mjs` scripts | |
| 37 | - | in `expr/gen/runner` (they need a clone of actions/runner and the .NET SDK, not | |
| 38 | - | `go test`). Runner commit used: `5f0b3ddcc6ee9fe9441e00d55e85b94ea4aad798`, | |
| 39 | - | .NET 9.0.20. | |
| 44 | + | Regenerating: see `gen/runner/README.md`. The scripts need a clone of | |
| 45 | + | actions/runner at the commit `5f0b3ddcc6ee9fe9441e00d55e85b94ea4aad798` | |
| 46 | + | (`gen/runner/runner-commit.txt`, asserted by the scripts and by | |
| 47 | + | `TestRunnerCommit`), the .NET SDK and a 9.0.x runtime (recorded: 9.0.20); none of | |
| 48 | + | it is needed by `go test`. | |
| 40 | 49 | ||
| 41 | - | Caveat for all of the above: the runner has two copies of the engine, | |
| 42 | - | `src/Sdk/DTExpressions2` (used by the legacy template evaluator) and | |
| 43 | - | `src/Sdk/Expressions` (used by the newer `WorkflowTemplateEvaluator`, which the | |
| 44 | - | runner currently runs next to the legacy one and compares). They are almost the | |
| 45 | - | same code; the differences found are noted where they matter (negative zero, | |
| 46 | - | `StrictJsonParsing`, `AlwaysTraceRealized`). The differential run uses | |
| 47 | - | `DTExpressions2`. | |
| 50 | + | ### The two engines of the runner, and which one runs | |
| 51 | + | ||
| 52 | + | The runner contains two copies of the expression engine: | |
| 53 | + | `src/Sdk/DTExpressions2/Expressions2` (namespace `GitHub.DistributedTask. | |
| 54 | + | Expressions2`, "legacy") and `src/Sdk/Expressions` (namespace `GitHub.Actions. | |
| 55 | + | Expressions`, "newer", with its own `ExpressionData` types, `JsonParser` and | |
| 56 | + | options such as `StrictJsonParsing`). They are separate implementations, almost | |
| 57 | + | line for line the same. | |
| 58 | + | ||
| 59 | + | - **Which one runs.** The legacy one. `Runner.Worker/PipelineTemplateEvaluatorWrapper.cs` | |
| 60 | + | (used by `ExecutionContext.ToPipelineTemplateEvaluator`) and | |
| 61 | + | `ActionManifestManagerWrapper.cs` call `EvaluateAndCompare(legacy, new, ...)`: | |
| 62 | + | both evaluators run, the **legacy result (or its exception) is returned**, and a | |
| 63 | + | difference is only reported as job telemetry (`TemplateEvaluatorMismatch`, | |
| 64 | + | `RecordMismatch`). So the newer engine is a shadow being validated, and what | |
| 65 | + | users see is the legacy behaviour. This is why `gen/runner` builds the | |
| 66 | + | legacy engine. | |
| 67 | + | - **How they differ.** `gen/runner-new` builds the newer one with the same driver | |
| 68 | + | and `gen/runner-new/engines.mjs` runs 46 148 expressions through both (the | |
| 69 | + | 34 765 of the oracle, the 1 791 `fromJSON` inputs, the number, comparison, | |
| 70 | + | `contains`, `startsWith` and `endsWith` corpora): the results are identical | |
| 71 | + | except for **negative zero** (the newer engine's `EvaluationResult. | |
| 72 | + | ConvertToString` maps `-0` to `0`, the legacy one prints `-0`; 3 expressions) | |
| 73 | + | and the **message of a `fromJSON` failure** (the newer one wraps it in "Error | |
| 74 | + | parsing fromJson"; both are `JsonReaderException`). In the sources the | |
| 75 | + | newer engine also has options the legacy one lacks: `StrictJsonParsing` (a | |
| 76 | + | `System.Text.Json` path in `fromJSON`, off by default, "temporary for | |
| 77 | + | telemetry"), `AlwaysTraceExpanded` and `MaxCacheMemory` (tracing), and its | |
| 78 | + | own `JsonParser`; none changes a result with the default options. | |
| 79 | + | - **Consequence for this package.** It follows the legacy engine, as run today. | |
| 80 | + | Negative zero is the one place where the newer engine, the shared | |
| 81 | + | cross-language fixtures and this package agree against the legacy one; if | |
| 82 | + | the runner switches engines nothing else changes. See "Still open". | |
| 48 | 83 | ||
| 49 | 84 | ## Limits added by this port (not in the JavaScript or the runner) | |
| 50 | 85 | ||
| @@ -315,7 +350,7 @@ package follows the runner. | |||
| 315 | 350 | boundary-delimited run (`ReadKeywordToken`): `github..event` reports `.` | |
| 316 | 351 | where the JS reported `.event`. Lexing and parsing are interleaved, so the | |
| 317 | 352 | first problem in reading order wins (the JS lexed everything first). | |
| 318 | - | `TestParseErrorPositions`, and `TestRunnerOracle` on 1 700 error cases. | |
| 353 | + | `TestParseErrorPositions`, and `TestRunnerOracle` on the 1 715 parse errors of the corpus (identical message, position included). | |
| 319 | 354 | One fixture still differs, see `xlangKnownDifferences`: `case/27` expects | |
| 320 | 355 | the name for an even `case()`, the runner passes the `(`. | |
| 321 | 356 | - [x] **Function names and context names are case-insensitive**: same in the | |
| @@ -345,11 +380,12 @@ a bug. | |||
| 345 | 380 | handle a missing root context. Here it is null. | |
| 346 | 381 | - Recursion limits: JS recursion depth for `fromJSON` input is engine | |
| 347 | 382 | dependent. `FromJSON` follows Json.NET's `MaxDepth` of 64. | |
| 348 | - | - Lone surrogates: JS strings are UTF-16 and can hold them. The runner cannot | |
| 349 | - | (Json.NET reads an unpaired `\ud800` as U+FFFD), and neither can Go strings; | |
| 350 | - | the WTF-8 machinery of the first version (`wtf8Surrogate`, `normalizeWTF8`) | |
| 351 | - | is only used by the case-folding and comparison helpers for robustness and | |
| 352 | - | by the oracle tests. | |
| 383 | + | - Lone surrogates: JS strings are UTF-16 and can hold them. .NET strings can | |
| 384 | + | too, but no value of an expression has one: Json.NET reads an unpaired | |
| 385 | + | `\ud800` escape as U+FFFD (checked: `fromJSON('"\ud800"') == '\ufffd'` is | |
| 386 | + | true), and Go strings cannot hold them either. The WTF-8 helpers of the first | |
| 387 | + | version (`wtf8Surrogate`, `normalizeWTF8` in `casing.go`) are only reached | |
| 388 | + | through `Lex`-era code and the tests. | |
| 353 | 389 | ||
| 354 | 390 | ## Still open | |
| 355 | 391 | ||
| @@ -363,14 +399,19 @@ a bug. | |||
| 363 | 399 | worth doing only if the error text matters ("The maximum allowed memory | |
| 364 | 400 | size was exceeded while evaluating the following expression: ...", which | |
| 365 | 401 | needs `ConvertToExpression` of the root, also not ported). | |
| 366 | - | - [ ] **Negative zero.** `-0` is `0` here. `DTExpressions2/EvaluationResult.cs` | |
| 367 | - | (the legacy engine) prints `-0` for `format('{0}', -0)` and `toJSON(-0)`, | |
| 368 | - | because .NET Core prints `-0`; `Expressions/EvaluationResult.cs` (the engine of | |
| 369 | - | `WorkflowTemplateEvaluator`) maps it to `0`, and the cross-language fixture | |
| 370 | - | `coerce_string.json` expects `0`. If the legacy engine is what a user sees, | |
| 371 | - | `-0` would be right in `format()`; the choice was made for the fixture. | |
| 372 | - | `toJSON` goes through `ConvertToString` in both and so follows the same | |
| 373 | - | choice. | |
| 402 | + | - [ ] **Negative zero.** `-0` is `0` here (`format('{0}', -0)`, `toJSON(-0)`, | |
| 403 | + | `-0` as a result). The legacy engine, the one that runs (see "The two | |
| 404 | + | engines"), prints `-0`: `DTExpressions2/EvaluationResult.ConvertToString` | |
| 405 | + | formats with `G15` and .NET Core writes `-0`. The newer engine | |
| 406 | + | (`Expressions/EvaluationResult.cs`, "keep our behavior consistent") maps | |
| 407 | + | it to `0`, and the cross-language fixture `coerce_string.json` expects | |
| 408 | + | `0`; the choice follows the fixture and the newer engine. Verified by | |
| 409 | + | `engines.mjs`: this is the only result that differs between the engines, | |
| 410 | + | and `TestRunnerOracle` lists the two corpus expressions as a known | |
| 411 | + | difference. If the runner keeps the legacy engine for good and a user | |
| 412 | + | needs `-0`, change `numberToCoercedString`: `x == 0` would return `-0` | |
| 413 | + | for a negative zero (one line, plus the three tests and the two | |
| 414 | + | `.runner` goldens that say `0`). | |
| 374 | 415 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | |
| 375 | 416 | `//` comments and `new Date(1)` constructors; the first is read in a | |
| 376 | 417 | surprising way (`/*x*/1` is the string `x`). They fail here. | |
| @@ -18,8 +18,11 @@ Three kinds of evidence back the tests: | |||||
| 18 | - `testdata/runner.json.gz`: what the runner's own code answers on the same | 18 | - `testdata/runner.json.gz`: what the runner's own code answers on the same | ||
| 19 | expressions. `gen/runner` compiles `src/Sdk/DTExpressions2` (plus stubs for | 19 | expressions. `gen/runner` compiles `src/Sdk/DTExpressions2` (plus stubs for | ||
| 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | ||
| 21 | - | `TestRunnerOracle` holds the package to it: 34 765 cases, the only | 21 | + | `TestRunnerOracle` holds the package to it: 34 765 cases with the same |
| 22 | - | differences are negative zero (below) and error texts of Json.NET. | 22 | + | result, string form, `toJSON` text and, for errors, the same message (parse |
| 23 | + | errors included, to the position); the only exceptions are negative zero | |||
| 24 | + | (below) and the message texts of Json.NET failures, which must be `fromJSON` | |||
| 25 | + | failures on both sides. | |||
| 23 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | 26 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | ||
| 24 | and `number_cases.txt`, and checked the number formatting on 200 000 random | 27 | and `number_cases.txt`, and checked the number formatting on 200 000 random | ||
| 25 | doubles. | 28 | doubles. | ||
| @@ -30,21 +33,53 @@ Three kinds of evidence back the tests: | |||||
| 30 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | 33 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | ||
| 31 | 34 | ||||
| 32 | The runner's own expression tests (`ExpressionParserL0`, `ConditionFunctionsL0`) | 35 | The runner's own expression tests (`ExpressionParserL0`, `ConditionFunctionsL0`) | ||
| 33 | - | are ported in `runner_l0_test.go`. The casing tables of the JavaScript | 36 | + | are ported in `runner_l0_test.go`; they are the only tests of the expression |
| 34 | - | (`casing.go`, `gen/casing*`) remain for `Lex`/oracle support only. | 37 | + | engine in the runner's `src/Test/L0` (the other files that mention it test |
| 38 | + | templates and actions). The case-mapping tables of the JavaScript (`casing.go`, | |||
| 39 | + | `casing_tables.go`, `gen/casing*`) are kept for `Lex` and the oracle tests; the | |||
| 40 | + | code `Parse` and `Evaluate` use for strings is `ordinal.go`, `number.go` and | |||
| 41 | + | `json.go`. The WTF-8 helpers in `casing.go` are only reached through `Lex` and | |||
| 42 | + | tests. | |||
| 35 | 43 | ||||
| 36 | - | Regenerating: `dotnet build -c Release expr/gen/runner` then the `*.mjs` scripts | 44 | + | Regenerating: see `gen/runner/README.md`. The scripts need a clone of |
| 37 | - | in `expr/gen/runner` (they need a clone of actions/runner and the .NET SDK, not | 45 | + | actions/runner at the commit `5f0b3ddcc6ee9fe9441e00d55e85b94ea4aad798` |
| 38 | - | `go test`). Runner commit used: `5f0b3ddcc6ee9fe9441e00d55e85b94ea4aad798`, | 46 | + | (`gen/runner/runner-commit.txt`, asserted by the scripts and by |
| 39 | - | .NET 9.0.20. | 47 | + | `TestRunnerCommit`), the .NET SDK and a 9.0.x runtime (recorded: 9.0.20); none of |
| 48 | + | it is needed by `go test`. | |||
| 40 | 49 | ||||
| 41 | - | Caveat for all of the above: the runner has two copies of the engine, | 50 | + | ### The two engines of the runner, and which one runs |
| 42 | - | `src/Sdk/DTExpressions2` (used by the legacy template evaluator) and | 51 | + | |
| 43 | - | `src/Sdk/Expressions` (used by the newer `WorkflowTemplateEvaluator`, which the | 52 | + | The runner contains two copies of the expression engine: |
| 44 | - | runner currently runs next to the legacy one and compares). They are almost the | 53 | + | `src/Sdk/DTExpressions2/Expressions2` (namespace `GitHub.DistributedTask. |
| 45 | - | same code; the differences found are noted where they matter (negative zero, | 54 | + | Expressions2`, "legacy") and `src/Sdk/Expressions` (namespace `GitHub.Actions. |
| 46 | - | `StrictJsonParsing`, `AlwaysTraceRealized`). The differential run uses | 55 | + | Expressions`, "newer", with its own `ExpressionData` types, `JsonParser` and |
| 47 | - | `DTExpressions2`. | 56 | + | options such as `StrictJsonParsing`). They are separate implementations, almost |
| 57 | + | line for line the same. | |||
| 58 | + | ||||
| 59 | + | - **Which one runs.** The legacy one. `Runner.Worker/PipelineTemplateEvaluatorWrapper.cs` | |||
| 60 | + | (used by `ExecutionContext.ToPipelineTemplateEvaluator`) and | |||
| 61 | + | `ActionManifestManagerWrapper.cs` call `EvaluateAndCompare(legacy, new, ...)`: | |||
| 62 | + | both evaluators run, the **legacy result (or its exception) is returned**, and a | |||
| 63 | + | difference is only reported as job telemetry (`TemplateEvaluatorMismatch`, | |||
| 64 | + | `RecordMismatch`). So the newer engine is a shadow being validated, and what | |||
| 65 | + | users see is the legacy behaviour. This is why `gen/runner` builds the | |||
| 66 | + | legacy engine. | |||
| 67 | + | - **How they differ.** `gen/runner-new` builds the newer one with the same driver | |||
| 68 | + | and `gen/runner-new/engines.mjs` runs 46 148 expressions through both (the | |||
| 69 | + | 34 765 of the oracle, the 1 791 `fromJSON` inputs, the number, comparison, | |||
| 70 | + | `contains`, `startsWith` and `endsWith` corpora): the results are identical | |||
| 71 | + | except for **negative zero** (the newer engine's `EvaluationResult. | |||
| 72 | + | ConvertToString` maps `-0` to `0`, the legacy one prints `-0`; 3 expressions) | |||
| 73 | + | and the **message of a `fromJSON` failure** (the newer one wraps it in "Error | |||
| 74 | + | parsing fromJson"; both are `JsonReaderException`). In the sources the | |||
| 75 | + | newer engine also has options the legacy one lacks: `StrictJsonParsing` (a | |||
| 76 | + | `System.Text.Json` path in `fromJSON`, off by default, "temporary for | |||
| 77 | + | telemetry"), `AlwaysTraceExpanded` and `MaxCacheMemory` (tracing), and its | |||
| 78 | + | own `JsonParser`; none changes a result with the default options. | |||
| 79 | + | - **Consequence for this package.** It follows the legacy engine, as run today. | |||
| 80 | + | Negative zero is the one place where the newer engine, the shared | |||
| 81 | + | cross-language fixtures and this package agree against the legacy one; if | |||
| 82 | + | the runner switches engines nothing else changes. See "Still open". | |||
| 48 | 83 | ||||
| 49 | ## Limits added by this port (not in the JavaScript or the runner) | 84 | ## Limits added by this port (not in the JavaScript or the runner) | ||
| 50 | 85 | ||||
| @@ -315,7 +350,7 @@ package follows the runner. | |||||
| 315 | boundary-delimited run (`ReadKeywordToken`): `github..event` reports `.` | 350 | boundary-delimited run (`ReadKeywordToken`): `github..event` reports `.` | ||
| 316 | where the JS reported `.event`. Lexing and parsing are interleaved, so the | 351 | where the JS reported `.event`. Lexing and parsing are interleaved, so the | ||
| 317 | first problem in reading order wins (the JS lexed everything first). | 352 | first problem in reading order wins (the JS lexed everything first). | ||
| 318 | - | `TestParseErrorPositions`, and `TestRunnerOracle` on 1 700 error cases. | 353 | + | `TestParseErrorPositions`, and `TestRunnerOracle` on the 1 715 parse errors of the corpus (identical message, position included). |
| 319 | One fixture still differs, see `xlangKnownDifferences`: `case/27` expects | 354 | One fixture still differs, see `xlangKnownDifferences`: `case/27` expects | ||
| 320 | the name for an even `case()`, the runner passes the `(`. | 355 | the name for an even `case()`, the runner passes the `(`. | ||
| 321 | - [x] **Function names and context names are case-insensitive**: same in the | 356 | - [x] **Function names and context names are case-insensitive**: same in the | ||
| @@ -345,11 +380,12 @@ a bug. | |||||
| 345 | handle a missing root context. Here it is null. | 380 | handle a missing root context. Here it is null. | ||
| 346 | - Recursion limits: JS recursion depth for `fromJSON` input is engine | 381 | - Recursion limits: JS recursion depth for `fromJSON` input is engine | ||
| 347 | dependent. `FromJSON` follows Json.NET's `MaxDepth` of 64. | 382 | dependent. `FromJSON` follows Json.NET's `MaxDepth` of 64. | ||
| 348 | - | - Lone surrogates: JS strings are UTF-16 and can hold them. The runner cannot | 383 | + | - Lone surrogates: JS strings are UTF-16 and can hold them. .NET strings can |
| 349 | - | (Json.NET reads an unpaired `\ud800` as U+FFFD), and neither can Go strings; | 384 | + | too, but no value of an expression has one: Json.NET reads an unpaired |
| 350 | - | the WTF-8 machinery of the first version (`wtf8Surrogate`, `normalizeWTF8`) | 385 | + | `\ud800` escape as U+FFFD (checked: `fromJSON('"\ud800"') == '\ufffd'` is |
| 351 | - | is only used by the case-folding and comparison helpers for robustness and | 386 | + | true), and Go strings cannot hold them either. The WTF-8 helpers of the first |
| 352 | - | by the oracle tests. | 387 | + | version (`wtf8Surrogate`, `normalizeWTF8` in `casing.go`) are only reached |
| 388 | + | through `Lex`-era code and the tests. | |||
| 353 | 389 | ||||
| 354 | ## Still open | 390 | ## Still open | ||
| 355 | 391 | ||||
| @@ -363,14 +399,19 @@ a bug. | |||||
| 363 | worth doing only if the error text matters ("The maximum allowed memory | 399 | worth doing only if the error text matters ("The maximum allowed memory | ||
| 364 | size was exceeded while evaluating the following expression: ...", which | 400 | size was exceeded while evaluating the following expression: ...", which | ||
| 365 | needs `ConvertToExpression` of the root, also not ported). | 401 | needs `ConvertToExpression` of the root, also not ported). | ||
| 366 | - | - [ ] **Negative zero.** `-0` is `0` here. `DTExpressions2/EvaluationResult.cs` | 402 | + | - [ ] **Negative zero.** `-0` is `0` here (`format('{0}', -0)`, `toJSON(-0)`, |
| 367 | - | (the legacy engine) prints `-0` for `format('{0}', -0)` and `toJSON(-0)`, | 403 | + | `-0` as a result). The legacy engine, the one that runs (see "The two |
| 368 | - | because .NET Core prints `-0`; `Expressions/EvaluationResult.cs` (the engine of | 404 | + | engines"), prints `-0`: `DTExpressions2/EvaluationResult.ConvertToString` |
| 369 | - | `WorkflowTemplateEvaluator`) maps it to `0`, and the cross-language fixture | 405 | + | formats with `G15` and .NET Core writes `-0`. The newer engine |
| 370 | - | `coerce_string.json` expects `0`. If the legacy engine is what a user sees, | 406 | + | (`Expressions/EvaluationResult.cs`, "keep our behavior consistent") maps |
| 371 | - | `-0` would be right in `format()`; the choice was made for the fixture. | 407 | + | it to `0`, and the cross-language fixture `coerce_string.json` expects |
| 372 | - | `toJSON` goes through `ConvertToString` in both and so follows the same | 408 | + | `0`; the choice follows the fixture and the newer engine. Verified by |
| 373 | - | choice. | 409 | + | `engines.mjs`: this is the only result that differs between the engines, |
| 410 | + | and `TestRunnerOracle` lists the two corpus expressions as a known | |||
| 411 | + | difference. If the runner keeps the legacy engine for good and a user | |||
| 412 | + | needs `-0`, change `numberToCoercedString`: `x == 0` would return `-0` | |||
| 413 | + | for a negative zero (one line, plus the three tests and the two | |||
| 414 | + | `.runner` goldens that say `0`). | |||
| 374 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | 415 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | ||
| 375 | `//` comments and `new Date(1)` constructors; the first is read in a | 416 | `//` comments and `new Date(1)` constructors; the first is read in a | ||
| 376 | surprising way (`/*x*/1` is the string `x`). They fail here. | 417 | surprising way (`/*x*/1` is the string `x`). They fail here. | ||
M expr/runner_oracle_test.go +2 -8
| @@ -124,10 +124,8 @@ func goErrorClass(err error) string { | |||
| 124 | 124 | } | |
| 125 | 125 | ||
| 126 | 126 | // sameAnswer reports whether a Go answer matches the runner's. Errors match | |
| 127 | - | // when they are of the same phase and, for parse errors, name the same kind of | |
| 128 | - | // problem (the text before the colon: the runner appends the position, and its | |
| 129 | - | // arity errors name the parenthesis). Evaluation errors must have the same | |
| 130 | - | // message, except a fromJSON failure, whose runner message is Json.NET's: it | |
| 127 | + | // when they are of the same phase and have the same message, parse errors | |
| 128 | + | // included (token, position and expression), except a fromJSON failure, whose runner message is Json.NET's: it | |
| 131 | 129 | // matches only a fromJSON failure of this package (both are JSON reader | |
| 132 | 130 | // failures, never just "the expression mentions fromJSON"). | |
| 133 | 131 | func sameAnswer(g, r runnerCase) bool { | |
| @@ -140,10 +138,6 @@ func sameAnswer(g, r runnerCase) bool { | |||
| 140 | 138 | if g.Err.T != r.Err.T { | |
| 141 | 139 | return false | |
| 142 | 140 | } | |
| 143 | - | if g.Err.T == "parse" { | |
| 144 | - | desc := func(m string) string { d, _, _ := strings.Cut(m, ":"); return d } | |
| 145 | - | return desc(g.Err.M) == desc(r.Err.M) | |
| 146 | - | } | |
| 147 | 141 | if r.Err.X == "JsonReaderException" || g.Err.X == "JsonReaderException" { | |
| 148 | 142 | return r.Err.X == g.Err.X | |
| 149 | 143 | } | |
| @@ -124,10 +124,8 @@ func goErrorClass(err error) string { | |||||
| 124 | } | 124 | } | ||
| 125 | 125 | ||||
| 126 | // sameAnswer reports whether a Go answer matches the runner's. Errors match | 126 | // sameAnswer reports whether a Go answer matches the runner's. Errors match | ||
| 127 | - | // when they are of the same phase and, for parse errors, name the same kind of | 127 | + | // when they are of the same phase and have the same message, parse errors |
| 128 | - | // problem (the text before the colon: the runner appends the position, and its | 128 | + | // included (token, position and expression), except a fromJSON failure, whose runner message is Json.NET's: it |
| 129 | - | // arity errors name the parenthesis). Evaluation errors must have the same | |||
| 130 | - | // message, except a fromJSON failure, whose runner message is Json.NET's: it | |||
| 131 | // matches only a fromJSON failure of this package (both are JSON reader | 129 | // matches only a fromJSON failure of this package (both are JSON reader | ||
| 132 | // failures, never just "the expression mentions fromJSON"). | 130 | // failures, never just "the expression mentions fromJSON"). | ||
| 133 | func sameAnswer(g, r runnerCase) bool { | 131 | func sameAnswer(g, r runnerCase) bool { | ||
| @@ -140,10 +138,6 @@ func sameAnswer(g, r runnerCase) bool { | |||||
| 140 | if g.Err.T != r.Err.T { | 138 | if g.Err.T != r.Err.T { | ||
| 141 | return false | 139 | return false | ||
| 142 | } | 140 | } | ||
| 143 | - | if g.Err.T == "parse" { | |||
| 144 | - | desc := func(m string) string { d, _, _ := strings.Cut(m, ":"); return d } | |||
| 145 | - | return desc(g.Err.M) == desc(r.Err.M) | |||
| 146 | - | } | |||
| 147 | if r.Err.X == "JsonReaderException" || g.Err.X == "JsonReaderException" { | 141 | if r.Err.X == "JsonReaderException" || g.Err.X == "JsonReaderException" { | ||
| 148 | return r.Err.X == g.Err.X | 142 | return r.Err.X == g.Err.X | ||
| 149 | } | 143 | } | ||