Require a definition named main in test suites
#2569
Conversation
Closes #2567 The error when there is a compilation failure due to a missing `main` is not very clear. Add an explicit check for a definition with that name when parsing metadata. This does not handle cases where the definition is not a method, but those cases should be more rare than omitting main.
| _annotations = directives.isEmpty ? [] : directives.first.metadata; | ||
| _languageVersionComment = result.unit.languageVersionToken?.value(); | ||
| _hasMain = result.unit.declarations.any( | ||
| (d) => d is NamedCompilationUnitMember && d.name.toString() == 'main', |
There was a problem hiding this comment.
@scheglov - I'm pretty sure toString() is not what I'm meant to be using, but stringValue was null and I didn't see a better route to getting this name. Is there a better option here?
There was a problem hiding this comment.
If says main, meaning probably a top-level function?
If so, you want FunctionDeclaration and its Token get name.
There was a problem hiding this comment.
and its
Token get name.
What way should I go from the Token instance to a String? Is the .toString() considered a reliable mechanism to get exactly 'main'?
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
|
If this doesn't break anything internally I think it might be the lowest cost approach. @jakemac53 WDYT about the limitations checking for "correct" definitions of |
| _annotations = directives.isEmpty ? [] : directives.first.metadata; | ||
| _languageVersionComment = result.unit.languageVersionToken?.value(); | ||
| _hasMain = result.unit.declarations.any( | ||
| (d) => d is FunctionDeclaration && d.name.toString() == 'main', |
|
Yeah I think this is the best solution, the edge cases should be very rare and can be worked around. Ultimately if some use case does come up it wouldn't be very difficult to actually add support for manually crawling exports as well. |
|
Will need changelog and such but lgtm |
|
Hmm, I think I'd be fine with updating tests in this repo to use an explicitly forwarding main, but there are also some internal flutter testing use cases that seem to rely on test suites without direct definitions of I'll see if I can gauge how wide the impact would be. |
|
We currently allow a |
3 participants
Closes #2567
The error when there is a compilation failure due to a missing
mainisless direct than it could be and exposes details of wrapping the test
suite in another library. Add an explicit check for a function declared
with that name when parsing metadata before attempting any compiles.
Add a check to
parseMetadatathat throws aFormatExceptionif thereis no function declared directly in the library named
main. This willprevent some previously working designs such as exposing a main in a
part or with
export, but we do not expect those designs are used inpractice.