Repository navigation
android_hardware_smoke_test: Detect blank image failures or EGL initialization warnings and retry - #190110
Conversation
cae0c79 to
e82fdc5
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a two-layered retry mechanism in the Android hardware smoke tests to mitigate transient EGL/graphics initialization failures and blank screenshots. The changes introduce retry loops in both the host-side suite runner and the on-device JUnit runner, along with logcat inspection, helper utilities, unit tests, and updated documentation. The review feedback recommends wrapping the retry setting deletion in a try-catch block to ensure cleanup runs, closing streams and destroying the logcat process to prevent resource leaks, and aligning the on-device retry logic with the documentation to also retry on EGL errors.
…ly there to validate the new behavior during development. We don't want to burden CI with them
e82fdc5 to
c8f453e
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a two-layered retry mechanism to the Android hardware smoke tests to mitigate flakiness from transient EGL/graphics initialization errors and blank screenshots. The changes span the host-side suite runner, on-device JUnit tests, and the driver test script, along with new image utility functions and tests. The reviewer feedback highlights several critical improvements: using UiAutomation.executeShellCommand instead of Runtime.getRuntime().exec for robust logcat access under SELinux, updating the activity-level retry condition to include EGL failures, replacing magic hex color literals with standard constants, adding try-catch protection around ADB logcat checks, and restoring the nativeDriver.close() call to prevent resource leaks.
…ow in kt, delete obsolete error const, remove teardown egl error check
b-luk
left a comment
There was a problem hiding this comment.
Looks good. Thanks for all the revisions!
|
Thanks for the review! I'll try to merge |
…12420) Manual roll requested by stuartmorgan@google.com flutter/flutter@b766512...27b0988 2026-08-05 kevmoo@users.noreply.github.com reland(tool): remove redundant --enable-experiment=record-use flag (flutter/flutter#190591) 2026-08-05 chris@bracken.jp Windows: Propagate enabled accessibility state (flutter/flutter#190507) 2026-08-05 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from ltbuIH9Z3T_yOuigu... to vcANVO8VIDQHasH1X... (flutter/flutter#190589) 2026-08-05 256906086+mvincentong@users.noreply.github.com Document frozen embedder API structs (flutter/flutter#186842) 2026-08-05 49402500+fahaddoc@users.noreply.github.com Document super call order for State.didChangeDependencies (flutter/flutter#185945) 2026-08-05 dkwingsmt@users.noreply.github.com Move examples of `flutter/widgets` widgets out from `flutter/material` (flutter/flutter#189532) 2026-08-05 93888664+ColeSpringer@users.noreply.github.com [web] Use thread local strike caches in skwasm (flutter/flutter#190048) 2026-08-05 43089218+chika3742@users.noreply.github.com doc: fix typo in see also section for PrimaryScrollController.maybeOf (flutter/flutter#190386) 2026-08-05 jason-simmons@users.noreply.github.com Migrate the shell unit tests from legacy Dart native functions to FFI (flutter/flutter#190473) 2026-08-05 47866232+chunhtai@users.noreply.github.com render proxy box now defaults baseline calculation to null (flutter/flutter#190269) 2026-08-05 36861262+QuncCccccc@users.noreply.github.com Update Widgets Localizations from Translation Console (flutter/flutter#190503) 2026-08-04 154381524+flutteractionsbot@users.noreply.github.com Revert: fix(tool): remove redundant --enable-experiment=record-use flag (flutter/flutter#190583) 2026-08-04 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from 1frGe_KltAJKkeyPg... to ltbuIH9Z3T_yOuigu... (flutter/flutter#190561) 2026-08-04 chris@bracken.jp iOS,macOS: add tsan and ubsan support for Swift (flutter/flutter#190497) 2026-08-04 34465683+rkishan516@users.noreply.github.com fix: update on_message_ to nullptr after window destroy so that dart gets destroy message (flutter/flutter#185807) 2026-08-04 bkonyi@google.com [flutter_tools] Gracefully handle locked Windows files during clean (flutter/flutter#190095) 2026-08-04 chris@bracken.jp iOS,macOS: make Logger thread-safe, conform to Sendable (flutter/flutter#190488) 2026-08-04 chris@bracken.jp iOS: Eliminate use of IOSContextNoop in platform view tests (reland) (flutter/flutter#190509) 2026-08-04 kevmoo@users.noreply.github.com fix(tool): remove redundant --enable-experiment=record-use flag (flutter/flutter#190475) 2026-08-04 awolff@google.com android_hardware_smoke_test: Detect blank image failures or EGL initialization warnings and retry (flutter/flutter#190110) 2026-08-04 30870216+gaaclarke@users.noreply.github.com Bumps text gamma on windows to match skia. (flutter/flutter#190477) 2026-08-04 engine-flutter-autoroll@skia.org Roll Skia from 48b58ee222f1 to a8583a0a2c11 (2 revisions) (flutter/flutter#190537) 2026-08-04 kevmoo@users.noreply.github.com [tool][web] Intercept dart2wasm errors & append JS migration footers (flutter/flutter#190476) 2026-08-04 dacoharkes@google.com [record_use] Migrate IconTreeShaker to `package:record_use` (flutter/flutter#190225) 2026-08-04 s4bre.py@gmail.com Handle unexpected exceptions during Azure metadata detection (flutter/flutter#189457) 2026-08-04 15619084+vashworth@users.noreply.github.com Fix merge conflict from flutter/flutter#190369 (flutter/flutter#190544) 2026-08-04 15619084+vashworth@users.noreply.github.com Prepare device support symbols (flutter/flutter#190369) 2026-08-04 engine-flutter-autoroll@skia.org Roll Packages from ac87e65 to 3498b9d (1 revision) (flutter/flutter#190532) 2026-08-04 engine-flutter-autoroll@skia.org Roll Skia from a08d918ebd6a to 48b58ee222f1 (11 revisions) (flutter/flutter#190527) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com,tarrinneal@google.com 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
Issue flutter#191348 shows that the detection and retry behavior introduced in PR flutter#190110 did not solve the issue. Previously, engine cache cleanup was placed solely in `@AfterClass tearDownClass()`. When a test run failed mid-suite on attempt 1, JUnit aborted before `@AfterClass` could run. When attempt 2 launched, `MainActivity` retrieved the corrupted engine from cache, guaranteeing that subsequent attempts failed with blank screenshots, which matches what we see in the run history. To fix this, we move cache cleanup to `MainActivity.evictEngineCache()`. `FlutterActivityTest.kt` invokes this cleanup during class teardown and whenever an `EglInitializationException` or `BlankScreenshotException` occurs, ensuring any retry starts with a clean engine instance. We also reduce the retry cap in `run_android_hardware_smoke_tests.dart` from 3 attempts to 2 to avoid 30-minute LUCI timeouts on broken emulators, which we saw some of in tryjobs. I think this should be an infrastructure failure. So if an unrecoverable EGL collapse occurs across both attempts, the runner logs an infrastructure failure and exits with code 2 (`INFRA_FAILURE`) We also saw some logs which suggest HWUI repeatedly attempted 10-bit color format negotiation (`101010-2`), which fails on SwiftShader CPU drivers. This could contribute to an increased rate of EGL errors, so `MainActivity.onCreate()` now explicitly locks the window pixel format to `PixelFormat.RGBA_8888`. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This PR addresses flakiness for the android_hardware_smoke_test suite.
Occasionally, due to race conditions during rendering composition, platform view tests will produce blank images. The screenshot occurs before composition finished, even though we try to sync it deterministically. For these cases, we introduce a retry right there in the process. Also, we refactor the blank image detection and the existing image cropping functions into image_utils.dart, and add a unit test for the blank image method.
Occasionally, for reasons I am not totally clear on, CI starts the test suite in a way which contains EGL initialization errors. This also produces blank images, but retrying wouldn't help at all because it's not a timing issue, it's a setup issue. So we introduce a detection mechanism for these which looks for errors in the logcat, then retries by restarting the activity completely, which at least has a theoretical chance to start up again without the EGL initialization problem.
These issues are affecting prod and staging for the instrumented shards, though only for the vulkan tests. OpenGLES tests are passing consistently. I don't see any failures of this type in presubmit, though it may be masked by a different problem related to infrastructure failures during dependency downloads.
This is intended to improve #189079 and #189843 (comment)
Pre-launch Checklist
///).