Address frame policy benchmark flakes (#155130)

Recently the microbenchmarks were flakey, but from an older bug. Turns out, `LiveTestWidgetsFlutterBindingFramePolicy` is defaulted to `fadePointers` with this fun note:

> This can result in additional frames being pumped beyond those that
the test itself requests, which can cause differences in behavior

Both `text_intrinsic_bench` and `build_bench` use a similar pattern:
* Load stocks app
* Open the menu
* Switch to `benchmark` frame policy

What happens, rarely, is that
`LiveTestWidgetsFlutterBinding.pumpBenchmark()` will call (async) `handleBeginFrame` and `handleDrawFrame`. `handleDrawFrame` juggles a tri-state boolean (null, false, true). This boolean is only reset to `null` when handleDrawFrame is called back to back, say, from an extra frame that was scheduled.

1. Switch tri-state boolean to an enum, its easier to read
2. remove asserts that compile away in benchmarks (`--profile`)
3. use `Error.throwWithStackTrace` to keep stack traces.

I've been running this test on device lab hardware for hundreds of runs and have not hit a failure yet.

Fixes #150542
Fixes #150543 - throw stack!
This commit is contained in:
John McDole 2024-09-12 16:19:15 -07:00 committed by GitHub
parent 74caead4cd
commit b755641559
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 25 additions and 11 deletions

View File

@ -33,7 +33,7 @@ Future<void> execute() async {
await benchmarkWidgets((WidgetTester tester) async {
runApp(intrinsicTextHeight);
// Wait for the UI to stabilize.
await tester.pump(const Duration(seconds: 1));
await tester.pumpAndSettle(const Duration(seconds: 1));
final TestViewConfiguration big = TestViewConfiguration.fromView(
size: const Size(360.0, 640.0),

View File

@ -29,7 +29,7 @@ Future<List<double>> runBuildBenchmark() async {
await tester.pump(const Duration(seconds: 1)); // Complete startup animation
await tester.tapAt(const Offset(20.0, 40.0)); // Open drawer
await tester.pump(); // Start drawer animation
await tester.pump(const Duration(seconds: 1)); // Complete drawer animation
await tester.pumpAndSettle(const Duration(seconds: 1)); // Complete drawer animation
final Element appState = tester.element(find.byType(stocks.StocksApp));
binding.framePolicy = LiveTestWidgetsFlutterBindingFramePolicy.benchmark;

View File

@ -1649,6 +1649,12 @@ enum LiveTestWidgetsFlutterBindingFramePolicy {
benchmarkLive,
}
enum _HandleDrawFrame {
reset,
drawFrame,
skipFrame,
}
/// A variant of [TestWidgetsFlutterBinding] for executing tests
/// on a device, typically via `flutter run`, or via integration tests.
/// This is intended to allow interactive test development.
@ -1779,31 +1785,35 @@ class LiveTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
return super.reassembleApplication();
}
bool? _doDrawThisFrame;
_HandleDrawFrame _drawFrame = _HandleDrawFrame.reset;
@override
void handleBeginFrame(Duration? rawTimeStamp) {
assert(_doDrawThisFrame == null);
if (_drawFrame != _HandleDrawFrame.reset) {
throw StateError('handleBeginFrame() called before previous handleDrawFrame()');
}
if (_expectingFrame ||
_expectingFrameToReassemble ||
(framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.fullyLive) ||
(framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmarkLive) ||
(framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.benchmark) ||
(framePolicy == LiveTestWidgetsFlutterBindingFramePolicy.fadePointers && _viewNeedsPaint)) {
_doDrawThisFrame = true;
_drawFrame = _HandleDrawFrame.drawFrame;
super.handleBeginFrame(rawTimeStamp);
} else {
_doDrawThisFrame = false;
_drawFrame = _HandleDrawFrame.skipFrame;
}
}
@override
void handleDrawFrame() {
assert(_doDrawThisFrame != null);
if (_doDrawThisFrame!) {
if (_drawFrame == _HandleDrawFrame.reset) {
throw StateError('handleDrawFrame() called without paired handleBeginFrame()');
}
if (_drawFrame == _HandleDrawFrame.drawFrame) {
super.handleDrawFrame();
}
_doDrawThisFrame = null;
_drawFrame = _HandleDrawFrame.reset;
_viewNeedsPaint = false;
_expectingFrameToReassemble = false;
if (_expectingFrame) { // set during pump

View File

@ -683,7 +683,11 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
}());
dynamic caughtException;
void handleError(dynamic error, StackTrace stackTrace) => caughtException ??= error;
StackTrace? stackTrace;
void handleError(dynamic error, StackTrace trace) {
caughtException ??= error;
stackTrace ??= trace;
}
await Future<void>.microtask(() { binding.handleBeginFrame(duration); }).catchError(handleError);
await idle();
@ -691,7 +695,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
await idle();
if (caughtException != null) {
throw caughtException as Object; // ignore: only_throw_errors, rethrowing caught exception.
Error.throwWithStackTrace(caughtException as Object, stackTrace!);
}
}