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
fix for flakey analyze test #111895
fix for flakey analyze test #111895
Conversation
@@ -40,6 +40,7 @@ const String _kNoPlatformsMessage = "You've created a plugin project that doesn' | |||
const String frameworkRevision = '12345678'; | |||
const String frameworkChannel = 'omega'; | |||
const String _kDisabledPlatformRequestedMessage = 'currently not supported on your local environment.'; | |||
const String _kLintingErrorPattern = r"^\s*(?<type>error|info)(\s*[•|-]\s*)(?<desc>[\(\)`@\w\s'-]+)(\s*[•|-]\s*)(?<path>[\\\w'-\/:]+)(\s*[•|-]\s*)(?<lint>[\w'-]+)$"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern is pretty inscrutable :) Is this approach superior to only collecting errors AFTER 'Analyzing flutter_project'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, the pattern is hard to decipher. But I went with this approach because I wasn't sure if the analyze
tool could potentially output more lines after the Analyzing...
line.
I can make the necessary updates to only check lines after that particular line if you think that is better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To help with deciphering it as well, I added named groups within the regex matches so it could potentially make it easier to debug... but adding the named groups also makes the pattern string pretty long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, the pattern is hard to decipher. But I went with this approach because I wasn't sure if the analyze tool could potentially output more lines after the Analyzing... line.
"Analyzing flutter_project..." occurs after we're done with the pub get flow: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/commands/analyze_once.dart#L115. It's not perfect, but at least if we regress this I'm pretty sure it will be in our own tool code.
instead looking for the first instance of `Analyzing flutter_project...` to determine when to start capturing error outputs from `flutter analyze`
packages/flutter_tools/test/commands.shard/permeable/create_test.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR fixes #111272 by improving how we parse the stdout from
flutter analyze
to find only the relevant linesPre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.