Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 02263ca

Browse files
authored
Fix Android scenario_app to actually run and block on CI (#50414)
20337ea (now reverted) intentionally re-introduced a bug (#49938) on a now working test runner, and verified that (with some fixes in this PR), we can now catch regressions on CI with SkiaGold: --- <details> <summary>ExternalTextureTests_testCanvasSurface.png</summary> This test shows that what previously was a picture has been reduced down to a single stretched pixel. **BEFORE**: ![ExternalTextureTests_testCanvasSurface](https://github.com/flutter/engine/assets/168174/1509ea21-2887-4a3b-b200-b857bbfb8304) **AFTER**: ![ExternalTextureTests_testCanvasSurface](https://github.com/flutter/engine/assets/168174/d9ae19c8-dd39-4e7f-859c-5d82bd1ba0a3) </details> <details> <summary>ExternalTextureTests_testRotatedMediaSurface_180.png</summary> Similar to above, but shows _another_ bug (kClamp versus kRepeat) **BEFORE**: ![ExternalTextureTests_testRotatedMediaSurface_180](https://github.com/flutter/engine/assets/168174/fee5bc8d-749f-4a59-976a-a573515fecea) **AFTER**: ![ExternalTextureTests_testRotatedMediaSurface_180](https://github.com/flutter/engine/assets/168174/a5249f70-22b4-4d36-8b63-e88c8a6846fe)/cc </details> --- This PR now makes the `scenario_test` useful and blocking for Android engine rolls: - Add support for connecting to SkiaGold by depending on `goldctl` in the appropriate configs. - Add some useful local debugging flags for skipping SkiaGold or running specific tests (not used by default). - Failing to connect to Skia gold is now a test failure.
1 parent e4a5acc commit 02263ca

File tree

4 files changed

+49
-10
lines changed

4 files changed

+49
-10
lines changed

.ci.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ targets:
5454
recipe: engine_v2/engine_v2
5555
properties:
5656
config_name: linux_android_emulator
57+
dependencies: >-
58+
[
59+
{"dependency": "goldctl", "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"}
60+
]
5761
timeout: 60
5862
runIf:
5963
- .ci.yaml
@@ -71,6 +75,10 @@ targets:
7175
recipe: engine_v2/engine_v2
7276
properties:
7377
config_name: linux_android_emulator_api_33
78+
dependencies: >-
79+
[
80+
{"dependency": "goldctl", "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"}
81+
]
7482
timeout: 60
7583
runIf:
7684
- .ci.yaml

ci/builders/linux_android_emulator.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
"--rbe",
1818
"--no-goma"
1919
],
20+
"dependencies": [
21+
{
22+
"dependency": "goldctl",
23+
"version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"
24+
}
25+
],
2026
"name": "android_debug_x64",
2127
"ninja": {
2228
"config": "android_debug_x64",

ci/builders/linux_android_emulator_api_33.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
"--rbe",
1818
"--no-goma"
1919
],
20+
"dependencies": [
21+
{
22+
"dependency": "goldctl",
23+
"version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"
24+
}
25+
],
2026
"name": "android_debug_x64",
2127
"ninja": {
2228
"config": "android_debug_x64",

testing/scenario_app/bin/android_integration_tests.dart

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,41 @@ void main(List<String> args) async {
2121
final ArgParser parser = ArgParser()
2222
..addOption(
2323
'adb',
24-
help: 'absolute path to the adb tool',
24+
help: 'Absolute path to the adb tool',
2525
mandatory: true,
2626
)
2727
..addOption(
2828
'out-dir',
29-
help: 'out directory',
29+
help: 'Out directory',
3030
mandatory: true,
3131
)
32-
..addFlag(
32+
..addOption(
3333
'smoke-test',
3434
help: 'runs a single test to verify the setup',
35-
negatable: false,
35+
valueHelp: 'The class to execute, defaults to dev.flutter.scenarios.EngineLaunchE2ETest',
36+
)
37+
..addFlag(
38+
'use-skia-gold',
39+
help: 'Use Skia Gold to compare screenshots.',
40+
defaultsTo: isLuciEnv,
3641
);
3742

3843
runZonedGuarded(
3944
() async {
4045
final ArgResults results = parser.parse(args);
4146
final Directory outDir = Directory(results['out-dir'] as String);
4247
final File adb = File(results['adb'] as String);
43-
final bool smokeTest = results['smoke-test'] as bool;
44-
await _run(outDir: outDir, adb: adb, smokeTest: smokeTest);
48+
final bool useSkiaGold = results['use-skia-gold'] as bool;
49+
String? smokeTest = results['smoke-test'] as String?;
50+
if (results.wasParsed('smoke-test') && smokeTest!.isEmpty) {
51+
smokeTest = 'dev.flutter.scenarios.EngineLaunchE2ETest';
52+
}
53+
await _run(
54+
outDir: outDir,
55+
adb: adb,
56+
smokeTestFullPath: smokeTest,
57+
useSkiaGold: useSkiaGold,
58+
);
4559
exit(0);
4660
},
4761
(Object error, StackTrace stackTrace) {
@@ -57,7 +71,8 @@ void main(List<String> args) async {
5771
Future<void> _run({
5872
required Directory outDir,
5973
required File adb,
60-
required bool smokeTest,
74+
required String? smokeTestFullPath,
75+
required bool useSkiaGold,
6176
}) async {
6277
const ProcessManager pm = LocalProcessManager();
6378

@@ -178,7 +193,11 @@ Future<void> _run({
178193
await skiaGoldClient!.auth();
179194
log('skia gold client is available');
180195
} else {
181-
log('skia gold client is unavailable');
196+
if (useSkiaGold) {
197+
panic(<String>['skia gold client is unavailable']);
198+
} else {
199+
log('skia gold client is unavaialble');
200+
}
182201
}
183202
});
184203

@@ -210,8 +229,8 @@ Future<void> _run({
210229
'am',
211230
'instrument',
212231
'-w',
213-
if (smokeTest)
214-
'-e class dev.flutter.scenarios.EngineLaunchE2ETest',
232+
if (smokeTestFullPath != null)
233+
'-e class $smokeTestFullPath',
215234
'dev.flutter.scenarios.test/dev.flutter.TestRunner',
216235
]);
217236
if (exitCode != 0) {

0 commit comments

Comments
 (0)