search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

flutter-analyze bot might be analyzing too much #54361

Open bwilkerson opened 9 months ago

bwilkerson commented 9 months ago

The flutter-analyze bot is currently red. I spoke to the flutter team and it appears that one of the issues is a result of a mismatch between what flutter analyzes and our bot analyzes. In particular, I was told that

The engine has a script that it uses for analysis (ci/analyze.sh) which runs over the following directories:

"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/ci"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/flutter_frontend_server"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/impeller/golden_tests_harvester"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/impeller/tessellator/dart"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/lib/gpu"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/lib/ui"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/testing"
"$DART" analyze --fatal-infos --fatal-warnings "$FLUTTER_DIR/tools"

Notably, it doesn't analyze web_ui.

Please verify that this information is correct, and if so consider reducing the amount analyzed by the flutter-analyze bot to match.

bwilkerson commented 9 months ago

I suspect that the other issue might be similar, given that this breakage doesn't appear to be blocking the roll.

whesse commented 9 months ago

The other issue was a real issue, and did block the roll, and the Flutter rollers felt very strongly in their discussion that the Dart team should have seen this redness on the flutter-analyze bot and on the monorepo bots, and reverted quickly. The CL causing the analyzer failure was https://dart-review.googlesource.com/c/sdk/+/337923

bwilkerson commented 9 months ago

I'm not disagreeing with the position taken by the Flutter rollers. The CL probably should have been reverted until the Flutter code had been cleaned up.

However, I will point out that a failure on this bot is not as good a signal as we might want it to be. It is not infrequently the case that this bot begins failing for reasons that are unrelated to the Dart CL that prompted it to run. This happens because a PR on the Flutter side causes the breakage, but the bot isn't run again until the next Dart CL is run through the bot. The Dart CL isn't responsible for the breakage; the real cause is invisible.

I have suggested a few times that it would be nice to have a corresponding bot on the Flutter side that would test every Flutter PR against the HEAD of the Dart SDK. That would allow this kind of breakage to be caught before the problem is introduced. I don't know how feasible such a bot would be, but it would be nice.