Upgrade android_hardware_smoke_test CI to run instrumented tests (#189390)
This PR adds native Android instrumented tests to the
`android_hardware_smoke_test` CI suite in parallel to existing driver
tests (Part of https://github.com/flutter/flutter/issues/187538).
Adds two new shards temporarily, both with `bringup: true`:
* `android_hardware_smoke_vulkan_instrumented_tests`
* `android_hardware_smoke_opengles_instrumented_tests`
These shards run the driver tests first, similar to existing shards,
then subsequently run the instrumented tests. They do this via a new
optional `runInstrumented` param in
`run_android_hardware_smoke_tests.dart`.
We'll remove the parameter and put this behavior in the existing shards
after it proves stable.
## Why do we need to run the instrumented tests sequentially after the
driver tests?
I generally prefer to avoid doing multiple things sequentially in an
integration test. It tends to introduce flakiness. So for this suite,
why can't we have one shard for driver tests and a separate one for
instrumented tests?
The driver tests use a skia gold comparator to compare result images to
skia gold at runtime. However, no such comparator is available for
instrumented tests. `goldctl` isn't designed to run on android. So we
need to get the goldens somehow at APK build time.
In this PR, the integration tests run in two sequential passes. We first
execute a host-driven driver test to save reference screenshots, and
then run native JUnit tests to compare against those local files. This
is the same way we will generate instrumented test APKs to be delivered
to OEMs. I considered optimizing this by downloading the latest approved
goldens directly from the Skia Gold server. It turns out it's possible
to calculate the public URLs based on the test name and some other
parameters. So we could theoretically write a script which cheaply
fetches all the goldens for the suite. Then we could configure a shard
which runs that script instead of the driver tests, achieving an
independent shard for instrumented tests only.
However, this downloader approach is incompatible with presubmit. Any PR
that introduces a change to the goldens would fail because the script
would only return the latest approved goldens. Even after a developer
approved the change on the dashboard, subsequent presubmit runs would
still download the old goldens, deadlocking the PR checks. Technically
that's not a problem for post-submit runs, but I'd rather keep presubmit
and postsubmit identical instead of optimizing only postsubmit with
something like this.
Running the driver pass first, as we do in this PR, avoids the problem
by dynamically capturing reference screenshots generated by the PR's own
code. This keeps the skew at zero and allows tests to pass once
approved.
Also, I'm not that comfortable relying on calculating URLs, since there
would be no guarantee that the URL scheme will remain stable. The URL
scheme hasn't changed since 2020, but compared to relying only on
goldctl, it just feels more fragile to me. That being said, I've
implemented the download script in a personal agent skill instead, where
I can use it at my own risk, and it isn't weight bearing in any way.
## 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
diff --git a/.ci.yaml b/.ci.yaml
index ed1857b..a737c5a 100644
--- a/.ci.yaml
+++ b/.ci.yaml
@@ -1809,6 +1809,33 @@
{"dependency": "goldctl", "version": "git_revision:c845c41b9b81bfcb11f2f0ab17b5b2386d634c31"}
]
+ - name: Linux_android_emu_vulkan_stable android_hardware_smoke_vulkan_instrumented_tests
+ bringup: true
+ recipe: flutter/flutter_drone
+ timeout: 60
+ properties:
+ shard: android_hardware_smoke_vulkan_instrumented_tests
+ tags: >
+ ["framework", "hostonly", "shard", "linux"]
+ dependencies: >-
+ [
+ {"dependency": "goldctl", "version": "git_revision:c845c41b9b81bfcb11f2f0ab17b5b2386d634c31"}
+ ]
+
+ - name: Linux_android_emu android_hardware_smoke_opengles_instrumented_tests
+ bringup: true
+ recipe: flutter/flutter_drone
+ timeout: 60
+ properties:
+ shard: android_hardware_smoke_opengles_instrumented_tests
+ tags: >
+ ["framework", "hostonly", "shard", "linux"]
+ dependencies: >-
+ [
+ {"dependency": "android_sdk", "version": "version:36v9unmodified"},
+ {"dependency": "goldctl", "version": "git_revision:c845c41b9b81bfcb11f2f0ab17b5b2386d634c31"}
+ ]
+
- name: Linux web_benchmarks_canvaskit
recipe: devicelab/devicelab_drone
presubmit: false
diff --git a/TESTOWNERS b/TESTOWNERS
index 1cf028d..f849047 100644
--- a/TESTOWNERS
+++ b/TESTOWNERS
@@ -364,6 +364,8 @@
# android_engine_vulkan_tests @reidbaker @flutter/android
# android_hardware_smoke_opengles_tests @andywolff @flutter/engine
# android_hardware_smoke_vulkan_tests @andywolff @flutter/engine
+# android_hardware_smoke_opengles_instrumented_tests @andywolff @flutter/engine
+# android_hardware_smoke_vulkan_instrumented_tests @andywolff @flutter/engine
# android_java11_tool_integration_tests @jesswrd @flutter/android
# android_preview_tool_integration_tests @mboetger @flutter/android
# build_tests @bkonyi @flutter/tool
diff --git a/dev/bots/suite_runners/run_android_hardware_smoke_tests.dart b/dev/bots/suite_runners/run_android_hardware_smoke_tests.dart
index a1d05cf..24056c7 100644
--- a/dev/bots/suite_runners/run_android_hardware_smoke_tests.dart
+++ b/dev/bots/suite_runners/run_android_hardware_smoke_tests.dart
@@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'dart:async';
+import 'dart:io' show Platform;
import 'package:file/file.dart';
import 'package:file/local.dart';
import 'package:path/path.dart' as path;
@@ -14,8 +15,34 @@
String _impellerBackendMetadata({required String value}) =>
'<meta-data android:name="io.flutter.embedding.android.ImpellerBackend" android:value="$value" />';
+void _copyDirectory(Directory source, Directory destination) {
+ destination.createSync(recursive: true);
+ for (final FileSystemEntity entity in source.listSync(recursive: true)) {
+ if (entity is File) {
+ final String relativePath = path.relative(entity.path, from: source.path);
+ final String destPath = path.join(destination.path, relativePath);
+ entity.fileSystem.file(destPath).parent.createSync(recursive: true);
+ entity.copySync(destPath);
+ }
+ }
+}
+
+void _cleanGoldensDirectory(Directory directory) {
+ if (!directory.existsSync()) {
+ return;
+ }
+ for (final FileSystemEntity entity in directory.listSync()) {
+ if (path.basename(entity.path) != 'README.md') {
+ entity.deleteSync(recursive: true);
+ }
+ }
+}
+
/// Runs the Android Hardware Smoke Test golden suite in CI.
-Future<void> runAndroidHardwareSmokeTests({required ImpellerBackend backend}) async {
+Future<void> runAndroidHardwareSmokeTests({
+ required ImpellerBackend backend,
+ bool runInstrumented = false,
+}) async {
printProgress('Running Android Hardware Smoke Tests Shard (backend=${backend.name})');
final String testDir = path.join('dev', 'integration_tests', 'android_hardware_smoke_test');
@@ -28,11 +55,19 @@
'.',
], workingDirectory: testDir);
+ final String androidDir = path.join(testDir, 'android');
final File androidManifestXml = const LocalFileSystem().file(
- path.join(testDir, 'android', 'app', 'src', 'main', 'AndroidManifest.xml'),
+ path.join(androidDir, 'app', 'src', 'main', 'AndroidManifest.xml'),
);
final String androidManifestContents = androidManifestXml.readAsStringSync();
+ final Directory destinationDir = const LocalFileSystem().directory(
+ path.join(testDir, 'test_driver', 'goldens'),
+ );
+ final Directory sourceDir = const LocalFileSystem().directory(
+ path.join(testDir, 'android_hardware_smoke_test.${backend.name}.goldens'),
+ );
+
try {
// Replace whatever the current backend is with the specified backend.
final impellerBackendMetadata = RegExp(_impellerBackendMetadata(value: '[^"]*'));
@@ -48,6 +83,7 @@
),
);
+ // 1. Run driver tests to generate reference screenshots
await runCommand('flutter', <String>[
'drive',
'--driver=test_driver/driver_test.dart',
@@ -55,8 +91,36 @@
'--no-dds',
'--no-enable-dart-profiling',
], workingDirectory: testDir);
+
+ if (runInstrumented) {
+ // 2. Copy the generated goldens to the assets directory so they get packaged with the APK.
+ // In CI, the Skia Gold comparator downloads the baseline images into a temporary prefixed
+ // directory (sourceDir) instead of the default assets directory (destinationDir).
+ if (sourceDir.existsSync()) {
+ _copyDirectory(sourceDir, destinationDir);
+ }
+
+ final String gradle = path.absolute(
+ path.join(androidDir, Platform.isWindows ? 'gradlew.bat' : 'gradlew'),
+ );
+
+ // 3. Build and run the instrumented tests.
+ await runCommand(gradle, <String>[
+ ':app:connectedDebugAndroidTest',
+ '-Pandroid.testInstrumentationRunnerArguments.class=com.example.android_hardware_smoke_test.FlutterActivityTest',
+ '-s',
+ ], workingDirectory: androidDir);
+ }
} finally {
// Restore original contents.
androidManifestXml.writeAsStringSync(androidManifestContents);
+
+ // Clean up copied goldens to keep Git worktree completely clean
+ _cleanGoldensDirectory(destinationDir);
+
+ // Clean up the temporary prefixed goldens directory
+ if (sourceDir.existsSync()) {
+ sourceDir.deleteSync(recursive: true);
+ }
}
}
diff --git a/dev/bots/test.dart b/dev/bots/test.dart
index 511ecd3..8ab0284 100644
--- a/dev/bots/test.dart
+++ b/dev/bots/test.dart
@@ -147,6 +147,10 @@
runAndroidHardwareSmokeTests(backend: ImpellerBackend.vulkan),
'android_hardware_smoke_opengles_tests': () =>
runAndroidHardwareSmokeTests(backend: ImpellerBackend.opengles),
+ 'android_hardware_smoke_vulkan_instrumented_tests': () =>
+ runAndroidHardwareSmokeTests(backend: ImpellerBackend.vulkan, runInstrumented: true),
+ 'android_hardware_smoke_opengles_instrumented_tests': () =>
+ runAndroidHardwareSmokeTests(backend: ImpellerBackend.opengles, runInstrumented: true),
'flutter_plugins': flutterPackagesRunner,
'skp_generator': skpGeneratorTestsRunner,
'customer_testing': customerTestingRunner,
diff --git a/dev/integration_tests/android_hardware_smoke_test/README.md b/dev/integration_tests/android_hardware_smoke_test/README.md
index ce6beee..ecc6647 100644
--- a/dev/integration_tests/android_hardware_smoke_test/README.md
+++ b/dev/integration_tests/android_hardware_smoke_test/README.md
@@ -437,3 +437,45 @@
using native system compositor APIs (`adb` / `UiAutomation`) and cropping,
the test asserts on the **true, final composited image**
rendered by the device's GPU and system composer.
+
+### Why the Compile-Online / Run-Offline Split was not desirable
+
+An alternative execution model was considered:
+* **How it worked**: The build is split into
+ two sequential phases: compiling the test APK online
+ (`assembleDebugAndroidTest`), followed by
+ running the test execution completely offline
+ (`connectedDebugAndroidTest --offline`) to
+ insulate the emulator execution from Maven network flakes.
+
+While beneficial for network isolation,
+it was **not desirable** for several reasons:
+1. **UTP Dynamic Resolution Design**: The Android Gradle Plugin
+ (AGP) 8.x/9.x Unified Test Platform (UTP) runner resolves and
+ downloads its host-side runner plugins (like
+ `com.android.tools.utp:android-test-plugin-host-additional-test-output`)
+ dynamically via internal detached configurations at
+ runtime. These dependencies are not resolved during
+ compilation, causing the offline check to fail on
+ cold caches.
+2. **Maintenance Overhead / Double Execution**: To support
+ offline runs, we would either have to run a try-catch
+ online test execution first (which executes the entire
+ test suite twice, wasting CI resources) or manually
+ pin and declare all transitive UTP dependencies in the
+ `build.gradle.kts` file. Hardcoding UTP plugin versions
+ is highly brittle and breaks automatically whenever the
+ project upgrades its AGP version.
+3. **Redundant Safety**: Because the online compilation phase
+ (`assembleDebugAndroidTest`) already queries remote Maven
+ repositories for project dependencies, running the test
+ execution online does not introduce any new network
+ vectors that could flake. Gradle's internal dependency
+ caching guarantees subsequent execution remains
+ extremely fast.
+
+By executing `connectedDebugAndroidTest` online directly in
+a single step, the test suite executes exactly once,
+remains low maintenance across AGP upgrades, and
+cleanly bubbles up any legitimate compiler or runner
+errors without silent try-catch blocks.
