deflake more shell unittests #25864
deflake more shell unittests #25864
Conversation
|
There is still a race though right? The timings are reported from the raster thread while callback is reset on the UI thread. It may be that it is just less likely to fail now. Can we just disengage the assertions after they have been checked once instead? There are many ways we could do this but one way could be to setup a trampoline that calls a closure that makes the assertions and clearing the variable before making invoking assertions. |
|
The callback is reset after the first time it's called on the UI thread, and we only grab and assert on themf rom the UI thread. I'm missing where there's still a race - the raster thread couldn't let the UI thread know about this again until the UI thread has already reset the callback to not listen anymore. Am I missing something? |
| } | ||
| } | ||
| nativeReportTimingsCallback(timestamps); | ||
| PlatformDispatcher.instance.onReportTimings = (List<FrameTiming> timings) {}; |
There was a problem hiding this comment.
Specifically here - we cannot get called again on the raster thread in here, since we synchronously reset the callback to a no-op after the first call.
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Am I missing something?
No. I was.
|
@godofredoc - do you understand the cirrus error happening here? Has something changed with infra config? |
This was a transient issue related to more restricted ACLs in the flutter-cirrus project. |
* aac3658 Roll Skia from 14efdd3d50db to 3010f3d79193 (4 revisions) (flutter/engine#25857) * f45f52f Move path part files to libraries (flutter/engine#25842) * d96669f Roll Skia from 3010f3d79193 to 5276ba274b38 (1 revision) (flutter/engine#25859) * 43994ab Use "blur_sigma" instead of "blur_radius" in Shadow. (flutter/engine#25760) * 74ac010 Roll Dart SDK from 1e3e5efcd47e to 6397e8b91103 (18 revisions) (flutter/engine#25861) * d462cfd Remove flake inducing timeouts (flutter/engine#25847) * 258f63a Roll Skia from 5276ba274b38 to 31fddc376993 (14 revisions) (flutter/engine#25862) * 1bf01a1 fuchsia: Reliably forward View insets (flutter/engine#25381) * 2740062 Roll Skia from 31fddc376993 to 097263bb5089 (1 revision) (flutter/engine#25865) * 4aae6b3 Roll Dart SDK from 6397e8b91103 to ee8eb0a65efa (1 revision) (flutter/engine#25869) * d1f31c7 Roll Skia from 097263bb5089 to 5dfb3f40684b (1 revision) (flutter/engine#25870) * 428ef10 deflake (flutter/engine#25864) * 01b9db8 Roll Dart SDK from ee8eb0a65efa to 8c109a734bdc (1 revision) (flutter/engine#25872) * e623c09 Roll Dart SDK from 8c109a734bdc to b98a1eec5eb5 (1 revision) (flutter/engine#25873) * 7ce6226 Roll Dart SDK from b98a1eec5eb5 to 6ecae204598d (1 revision) (flutter/engine#25874) * 554c24c Roll Skia from 5dfb3f40684b to 5c95bcb48b9b (1 revision) (flutter/engine#25876) * 03bd2e3 Roll Dart SDK from 6ecae204598d to 3cc6cdab8eaf (2 revisions) (flutter/engine#25877) * 53fdd89 Roll Skia from 5c95bcb48b9b to c779d432f336 (1 revision) (flutter/engine#25880) * 98ee65b Roll Skia from c779d432f336 to ff8b52df55ff (2 revisions) (flutter/engine#25881) * d84caa7 Roll Dart SDK from 3cc6cdab8eaf to b8f4018535fa (2 revisions) (flutter/engine#25885) * 156760e Roll Skia from ff8b52df55ff to ec79349bad50 (1 revision) (flutter/engine#25886) * 23cd8e5 Revert the Dart SDK to 1e3e5efcd47edeb7ae5a69e146c8ea0559305a98 (flutter/engine#25887) * 578449f Fix crash when both FlutterFragmentActivity and FlutterFragment are destroyed and recreated (flutter/engine#25851)
3 participants
Fixes flutter/flutter#81580
Fixes flutter/flutter#43192
I was able to reproduce flakes locally on this about 25-33% of the time without this change, as long as I used
cpulimitto limit the execution. In those cases, we were getting a second report timings callback after the first, which was throwing off test expectations. This way we only get a single initial call.