Fix android_hardware_smoke_test retry mechanisms by andywolff · Pull Request #195708 · flutter/flutter

andywolff · 2026-10-30T19:36:49Z

Fixes #194928
Fixes #191348
Fixes #193557
Related to #193205

This makes android_hardware_smoke_test recover cleanly from transient emulator SurfaceFlinger wedges and host-GPU-pipe startup crashes instead of timing out or failing the shard after a passing retry. See #193557 (comment) for my root-cause investigation and local emulator reproduction.

What changed

run_android_hardware_smoke_tests.dart already retried failed flutter drive runs once, while invoking adb logcat -c and flutter drive through runCommand, which called foundError on any non-zero exit and marked the shard failed even when attempt 2 passed. runCommand now supports checkExitCode: false so retryable commands do not record a shard error on attempt 1, and startCommand decodes output streams with Utf8Decoder(allowMalformed: true) so raw bytes in native crash tombstones do not throw FormatException when inspecting adb logcat -d.

Retrying with flutter drive --no-build only restarted the app process and left SurfaceFlinger wedged when an emulator QuickBoot snapshot broke display composition. When attempt 1 fails with a blank screenshot or fatal graphics error in adb logcat -d, the runner now resets sys.boot_completed to 0, restarts surfaceflinger and zygote via adb shell stop and adb shell start, waits for sys.boot_completed=1, and unlocks the screen before attempt 2. Normal test assertion failures and golden mismatches still fail immediately on attempt 1.

When the app crashed during native EGL or Vulkan startup right after printing its Dart VM service URI, flutter drive looped against the closed adb forward port until LUCI killed the shard on timeout. startCommand and runCommand now accept a stderrListener alongside outputListener so the runner can watch stdout and stderr for slow-VM-connect warnings, check adb shell pidof, send SIGKILL to flutter drive once the app process has exited, and trigger the userspace-restart retry immediately.

In FlutterActivityTest.kt, querying logcat -d --pid=$pid *:W stripped Log.i attempt markers, while both FlutterActivityTest.kt and run_android_hardware_smoke_tests.dart treated routine HWUI warnings as fatal errors. FlutterActivityTest.kt now passes $TAG:I *:W to keep attempt markers visible in the dump. Both files now match explicit fatal EGL, Vulkan, and host-pipe errors while ignoring benign HWUI capability-probe logs.

Pre-launch Checklist

…rrors Route retryable adb and flutter drive commands through _runUncheckedSubprocess so a failed first attempt does not call foundError before attempt 2 runs. Decode subprocess streams with malformed UTF-8 tolerance so native crash tombstones in logcat do not throw FormatException.

…r EGL failures Reset sys.boot_completed and restart Android userspace via adb shell stop and start before attempt 2 so SurfaceFlinger rebinds cleanly after QuickBoot display wedges. Report the specific retryable failure reason in retry and error logs while keeping non-retryable test assertions failing fast on attempt 1.

…ing VM connect Check adb shell pidof when Dart VM service connection stalls during flutter drive and send SIGKILL once the target process has exited. Treat terminated dead-target runs as retryable with an explicit crash reason so userspace recovery starts immediately instead of hitting shard timeout.

…markers Pass $TAG:I *:W when dumping logcat in FlutterActivityTest so Log.i attempt markers are not stripped. Match explicit fatal EGL, Vulkan, and host-pipe errors across Kotlin and Dart while ignoring benign HWUI capability-probe warnings.

gemini-code-assist

Code Review

This pull request enhances the Android hardware smoke tests by introducing a robust recovery mechanism for emulator graphics and display compositor failures. It adds helper functions to run unchecked ADB commands, monitor the target process during VM connection, and reactively restart Android userspace when fatal graphics errors or blank captures are detected. The review feedback suggests adding a timeout to the _pollAdbUntil function to prevent infinite hangs in CI if the device or ADB gets permanently wedged.

b-luk · 2026-11-05T23:15:41Z

+/// Runs [executable] without recording a shard error via [foundError] on
+/// non-zero exit codes, decoding output with malformed UTF-8 tolerance so
+/// native crash tombstones in `adb logcat` do not throw [FormatException].
+Future<_SubprocessResult> _runUncheckedSubprocess(


This looks like it is almost the same as runCommand. Are the only differences: 1. don't check exit code and 2. allowMalformed in Utf8Decoder?

Should we just modify runCommand instead of making this new function? We could add a bool checkExitCode = true parameter. For allowMalformed, should we just always use true, or do some existing runCommand callers expect it to be false for some reason?

Done. I actually tried putting the changes in runCommand first, but I didn't like the list of parameters this resulted in. But you're right, I can just migrate it to something better in a followup. Filed #196126. As far as I can tell, allowMalformed should be fine for all existing callers.

b-luk · 2026-11-05T23:30:26Z

+    'sys.boot_completed',
+    '0',
+  ], description: 'sys.boot_completed reset');
+  await _pollAdbUntil(


Do adb shell stop and adb shell start need to be polled?

Good catch, they do not. Polling setprop sys.boot_completed 0 already waits for adbd to finish cycling after adb root, and stop and start do not restart adbd. Updated both to call _runAdb directly.

b-luk · 2026-11-05T23:33:27Z

+    // Intentionally scope retries strictly to blank-display compositor wedges or EGL
+    // initialization errors. General test assertion or golden pixel mismatch failures
+    // must fail fast on attempt 1 so real visual regressions are not masked.


nit: I think this comment should go above or inside the _detectRetryableFailureReason function instead of here at the call site. But I'll leave it up to whatever you prefer.

Done, moved into the doc comment on _detectRetryableFailureReason.

b-luk · 2026-11-05T23:39:06Z

+    // Match blank-capture exceptions reported by driver_test.dart (errorBlankScreenshot
+    // in constants.dart). Keep this substring in sync with both files.
+    if (driverOutput.contains('Captured screenshot is blank/empty')) 'blank display capture',


Seems like this should just check for errorBlankScreenshot directly without having to manually keep strings in sync.

If we think the current string of 'blank/empty' is too general and risks getting a false positive from an unrelated output line, then we could expand it to 'Captured screenshot is blank/empty'. And we can just update its throwing source to account for that at

flutter/dev/integration_tests/android_hardware_smoke_test/test_driver/driver_test.dart

Lines 104 to 106 in 1eecf01

throw StateError(

'Captured screenshot is $errorBlankScreenshot after $maxAttempts attempts.',

);

Expanded errorBlankScreenshot to 'Captured screenshot is blank/empty' in constants.dart so constants.dart and run_android_hardware_smoke_tests.dart hold the exact same string, and updated driver_test.dart so it only logs errorBlankScreenshot once screenshot retries are exhausted. dev/bots (tests_on_bots) cannot import constants.dart directly because avoid_relative_lib_imports and depend_on_referenced_packages are enabled and dart test rejects standalone bot packages that depend on flutter: sdk: flutter app packages, so I noted that package boundary in the sync comment.

b-luk · 2026-11-05T23:46:09Z

+      result.output,
+      terminatedDeadDrive: result.terminatedDeadDrive,


Should we just pass result instead of multiple arguments?

Done, updated _detectRetryableFailureReason to take _DriveAttemptResult result directly.

b-luk · 2026-11-05T23:49:29Z

    }

    io.stderr.writeln(
      'flutter drive failed with exit code ${result.exitCode} on attempt $attempt/$maxAttempts.',


This should be updated for the terminatedDeadDrive case. Otherwise it'll be a confusing message about failing with exit code 0.

Done, branched on result.terminatedDeadDrive to log 'flutter drive was terminated after target app crash on attempt ... (exit code ...).' instead.

b-luk · 2026-11-05T23:57:36Z

+        final _SubprocessResult pidResult = await _runAdb(<String>[
+          'shell',
+          'pidof',
+          'com.example.android_hardware_smoke_test',
+        ]);


Should we cache the pid instead of running pidof on every output line? Or does it change during the run?

Oh wait, I see. This is only run once because every line exits early until a line is encountered that matches slowVmConnectPatterns. So I think this is fine. Following the code flow requires a bit too much thinking for my taste, but I'm okay with it.

Extracted killDriveIfTargetExited out of the line callback to make this read a bit easier

Extend runCommand with checkExitCode, malformed UTF-8 decoding, and stderrListener instead of duplicating subprocess plumbing in run_android_hardware_smoke_tests.dart, and cover the new behavior in run_command_test.dart. Call _runAdb directly for adb shell stop and start, expand errorBlankScreenshot in constants.dart, and simplify the driver crash liveness callback and retry diagnostics.

b-luk

Thanks for the changes. LGTM

andywolff · 2026-11-06T20:51:51Z

Thanks for the review!

flutter/flutter@09175cd...e36b370 2026-11-07 [email protected] Roll Fuchsia Linux SDK from KIc0XZbhLmvDpTluZ... to Sf2yvdTiFXAKkfLkD... (flutter/flutter#196160) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196155) 2026-11-07 [email protected] [iOS a11y] Use a real `UIScrollView` as a11y scrollable container (flutter/flutter#195731) 2026-11-07 [email protected] Add ParagraphStyle.hyphenCharacter to dart:ui (flutter/flutter#195345) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196143) 2026-11-06 [email protected] [Linux] Fix layer position when compositing without glBlitFramebuffer (flutter/flutter#196083) 2026-11-06 [email protected] Add support for riscv64 desktop linux engine (flutter/flutter#178712) 2026-11-06 [email protected] Add ResizeImagePolicy.cover (flutter/flutter#194665) 2026-11-06 [email protected] Roll Skia from f533d4034900 to b11315d0a096 (6 revisions) (flutter/flutter#196138) 2026-11-06 [email protected] iOS: Remove dead code/unused imports from FlutterPlatformViewsController (flutter/flutter#196084) 2026-11-06 [email protected] Replace pull_request_target workflows with pull_request + workflow_run (flutter/flutter#195536) 2026-11-06 [email protected] [master] Update Flutter DEPS to Dart 3b174b7217fe09b21da836ea623932b5f041b091 (flutter/flutter#196004) 2026-11-06 [email protected] Fix android_hardware_smoke_test retry mechanisms (flutter/flutter#195708) 2026-11-06 [email protected] Roll Skia from 452b9c1e5db6 to f533d4034900 (1 revision) (flutter/flutter#196129) 2026-11-06 [email protected] [flutter_tools] Use `ErrorHandlingFileSystem.deleteIfExists` for `IOSCoreDeviceControl` temp directory cleanup (flutter/flutter#195913) 2026-11-06 [email protected] Adds guideline tests to flutter create samples (flutter/flutter#195827) 2026-11-06 [email protected] refactor(flutter_tools): unify OptionDescriptor identity and addTo (flutter/flutter#196076) 2026-11-06 [email protected] Roll pub packages (flutter/flutter#196114) 2026-11-06 [email protected] Roll Skia from 5ce64e63a0e7 to 452b9c1e5db6 (5 revisions) (flutter/flutter#196111) 2026-11-06 [email protected] Add PlatformDispatcher.onAlwaysUse24HourFormatChanged callback (flutter/flutter#194670) 2026-11-06 [email protected] Roll Packages from 0478162 to 63881f9 (10 revisions) (flutter/flutter#196108) 2026-11-06 [email protected] Roll Skia from 793b8ac73d4b to 5ce64e63a0e7 (4 revisions) (flutter/flutter#196107) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This was referenced Oct 30, 2026

Add allowNonZeroExit, stderrListener, and malformed UTF-8 tolerance to runCommand #195706

Closed

Fix FlutterActivityTest logcat marker priority and narrow graphics error filtering #195707

Closed

Piinks added engine flutter/engine related. See also e: labels. c: flake Tests that sometimes, but not always, incorrectly pass labels Nov 2, 2026

andywolff changed the title ~~Fix android_hardware_smoke_test retry accounting, userspace recovery, and host EGL filtering~~ Fix android_hardware_smoke_test retry accounting, userspace recovery, and logcat filtering Nov 3, 2026

andywolff changed the title ~~Fix android_hardware_smoke_test retry accounting, userspace recovery, and logcat filtering~~ Fix android_hardware_smoke_test retry mechanisms Nov 3, 2026

github-actions Bot removed the engine flutter/engine related. See also e: labels. label Nov 3, 2026

andywolff force-pushed the apktest-userspace-recovery branch 7 times, most recently from 99e6e2f to f9d6dc0 Compare November 5, 2026 21:02

andywolff added the CICD Run CI/CD label Nov 5, 2026

andywolff added 4 commits November 5, 2026 13:27

andywolff force-pushed the apktest-userspace-recovery branch from f9d6dc0 to 78e400c Compare November 5, 2026 21:27

andywolff marked this pull request as ready for review November 5, 2026 22:14

andywolff requested a review from b-luk November 5, 2026 22:14

gemini-code-assist Bot reviewed Nov 5, 2026

View reviewed changes

Comment thread dev/bots/suite_runners/run_android_hardware_smoke_tests.dart Resolved

b-luk requested changes Nov 6, 2026

View reviewed changes

andywolff mentioned this pull request Nov 6, 2026

Consolidate exit-code parameters on runCommand in dev/bots/run_command.dart #196126

Open

andywolff requested a review from b-luk November 6, 2026 20:06

b-luk approved these changes Nov 6, 2026

View reviewed changes

andywolff added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2026

auto-submit Bot added this pull request to the merge queue Nov 6, 2026

Merged via the queue into flutter:master with commit 1f33e88 Nov 6, 2026
26 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2026

engine-flutter-autoroll mentioned this pull request Nov 7, 2026

Roll Flutter from 09175cdeac89 to e36b37012875 (22 revisions) flutter/packages#13586

Merged

andywolff mentioned this pull request Nov 11, 2026

Linux_android_emu_vulkan_stable android_hardware_smoke_vulkan_instrumented_tests is 3.03% flaky #193205

Closed

	throw StateError(
	'Captured screenshot is $errorBlankScreenshot after $maxAttempts attempts.',
	);

		result.output,
		terminatedDeadDrive: result.terminatedDeadDrive,

Conversation

andywolff commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b-luk left a comment

Choose a reason for hiding this comment

Uh oh!

andywolff commented Nov 6, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andywolff commented Oct 30, 2026 •

edited

Loading