Skip to content

Fixes #3771. Replace multitests by static errors tests#3778

Open
sgrekhov wants to merge 1 commit intodart-lang:masterfrom
sgrekhov:co19-3771
Open

Fixes #3771. Replace multitests by static errors tests#3778
sgrekhov wants to merge 1 commit intodart-lang:masterfrom
sgrekhov:co19-3771

Conversation

@sgrekhov
Copy link
Copy Markdown
Contributor

@sgrekhov sgrekhov commented May 4, 2026

No description provided.

@sgrekhov sgrekhov requested a review from eernstg May 4, 2026 09:33
@eernstg
Copy link
Copy Markdown
Member

eernstg commented May 4, 2026

We have to think about this one: Changing tests from multi-test format to the new outcome expectation format (// ^^^\n// [analyzer] SOME_PROBLEM\n...) is generally useful, and it has already happened with almost all tests. However, the test runner does not recognize syntax errors with the new format, and this means that we can only keep an eye on spec_parser regressions by running the test runner as long as the syntax error tests continue to use the multi-test format. So if this update is motivated by a general policy to switch to the new format whenever possible (and not based on a specific need to change these tests) then we should at least consider the options. @sgrekhov, WDYT?

@sgrekhov
Copy link
Copy Markdown
Contributor Author

sgrekhov commented May 4, 2026

The main motivation for this change is to switch to the new error expectation format (// ^^^\n// ERROR...). I don’t see any advantages in the old format. What are they? Could it be that it always checks a single line with an error and avoids secondary errors on other lines caused by, say, a missing ; on a previous line? But we already have dozens of other tests where we expect syntax errors using the new format. Why should we keep the old format in this limited set of tests?

I checked all these updated tests in the analyzer, we expect errors exactly where they reported.

@eernstg
Copy link
Copy Markdown
Member

eernstg commented May 5, 2026

The main difference that I have in mind is, as you mention, that syntax errors are really tricky to handle in the case where we have several of them in the same library. The parser is perhaps expected to recover from syntax errors and get on track, ready to detect the next syntax error at the location where it occurs. However, we don't specify the recovery at all, and it's really difficult to say whether or not it's OK for a given implementation to report each of the syntax errors in any given test library as specified by our outcome expectations, or there is some other meaningful way to interpret the text as having a different set of syntax errors. Hence, we should probably never have a test with multiple syntax errors using the new test outcome expectations.

Next, commands like this one will work for a test library using the multi-test format:

> tools/test.py -c spec_parser language/syntax/syntax_test.dart

If this test library were migrated to use the new format then it would fail because the test runner is unable to distinguish syntax errors from other compile-time errors. For example, this one fails:

> tools/test.py -c spec_parser co19/Language/Variables/syntax_t03

(I really should create a CL and get this landed, but there's a need to adjust the test runner in pkg/test_runner/lib/src/test_suite.dart, otherwise it will just skip the test.)

So I keep saying that we should not migrate syntax tests to the new test outcome syntax, because this means that running the spec parser on "all tests" (for example, all language tests, which is what I've used most frequently) will get more and more noisy, reporting syntax errors as failed test runs rather than accepting test runs with syntactically wrong tests as successful test runs when a syntax error is reported.

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

Successfully merging this pull request may close these issues.

2 participants