Refactor Android `scenario_app` to remove shell entrypoint, simplify. (#50922)
Closes https://github.com/flutter/flutter/issues/144045.
There is still more work I want to do, like pulling args parsing into it's own class, and potentially cleanup the [golden file collection](https://github.com/flutter/flutter/issues/144047), but those seem reasonable for future (lower priority) PRs.
diff --git a/ci/builders/linux_android_emulator.json b/ci/builders/linux_android_emulator.json
index 7217a14..ad9763b 100644
--- a/ci/builders/linux_android_emulator.json
+++ b/ci/builders/linux_android_emulator.json
@@ -59,7 +59,7 @@
]
},
{
- "language": "bash",
+ "language": "dart",
"name": "Android Scenario App Integration Tests (Skia)",
"test_dependencies": [
{
@@ -74,14 +74,14 @@
"contexts": [
"android_virtual_device"
],
- "script": "flutter/testing/scenario_app/run_android_tests.sh",
+ "script": "flutter/testing/scenario_app/bin/run_android_tests.dart",
"parameters": [
- "android_emulator_debug_x64",
+ "--out-dir=../out/android_emulator_debug_x64",
"--no-enable-impeller"
]
},
{
- "language": "bash",
+ "language": "dart",
"name": "Android Scenario App Integration Tests (Impeller/Vulkan)",
"test_dependencies": [
{
@@ -96,9 +96,9 @@
"contexts": [
"android_virtual_device"
],
- "script": "flutter/testing/scenario_app/run_android_tests.sh",
+ "script": "flutter/testing/scenario_app/bin/run_android_tests.dart",
"parameters": [
- "android_emulator_debug_x64",
+ "--out-dir=../out/android_emulator_debug_x64",
"--enable-impeller",
"--impeller-backend=vulkan"
]
diff --git a/ci/builders/linux_android_emulator_api_33.json b/ci/builders/linux_android_emulator_api_33.json
index 104793a..ed91c3a 100644
--- a/ci/builders/linux_android_emulator_api_33.json
+++ b/ci/builders/linux_android_emulator_api_33.json
@@ -59,7 +59,7 @@
]
},
{
- "language": "bash",
+ "language": "dart",
"name": "Scenario App Integration Tests",
"test_dependencies": [
{
@@ -74,9 +74,9 @@
"contexts": [
"android_virtual_device"
],
- "script": "flutter/testing/scenario_app/run_android_tests.sh",
+ "script": "flutter/testing/scenario_app/bin/run_android_tests.dart",
"parameters": [
- "android_debug_api33_x64"
+ "--out-dir=../out/android_debug_api33_x64"
]
}
]
diff --git a/ci/builders/linux_android_emulator_opengles.json b/ci/builders/linux_android_emulator_opengles.json
index 4f74f62..0f2fd3f 100644
--- a/ci/builders/linux_android_emulator_opengles.json
+++ b/ci/builders/linux_android_emulator_opengles.json
@@ -34,7 +34,7 @@
},
"tests": [
{
- "language": "bash",
+ "language": "dart",
"name": "Android Scenario App Integration Tests (Impeller/OpenGLES)",
"test_dependencies": [
{
@@ -49,9 +49,9 @@
"contexts": [
"android_virtual_device"
],
- "script": "flutter/testing/scenario_app/run_android_tests.sh",
+ "script": "flutter/testing/scenario_app/bin/run_android_tests.dart",
"parameters": [
- "android_emulator_opengles_debug_x64",
+ "--out-dir=../out/android_emulator_opengles_debug_x64",
"--enable-impeller",
"--impeller-backend=opengles"
]
diff --git a/testing/scenario_app/README.md b/testing/scenario_app/README.md
index 8d866ce..3a7a2b2 100644
--- a/testing/scenario_app/README.md
+++ b/testing/scenario_app/README.md
@@ -6,17 +6,20 @@
in conjunction with Android and iOS-specific embedding code that simulates the
use of the engine in a real app (such as plugins and platform views).
-The [`run_android_tests.sh`](run_android_tests.sh) and
+The [`bin/run_android_tests.dart`](bin/run_android_tests.dart) and
[`run_ios_tests.sh`](run_ios_tests.sh) are then used to run the tests on a
connected device or emulator.
See also:
+- [File an issue][file_issue] with the `e: scenario-app` label.
- [`bin/`](bin/), the entry point for running Android integration tests.
- [`lib/`](lib/), the Dart code and instrumentation for the scenario app.
- [`ios/`](ios/), the iOS-side native code and tests.
- [`android/`](android/), the Android-side native code and tests.
+[file_issue]: https://github.com/flutter/flutter/issues/new?labels=e:%20scenario-app,engine,platform-android,fyi-android,team-engine
+
## Running a smoke test on Firebase TestLab
To run the smoke test on Firebase TestLab test, build `android_profile_arm64`,
@@ -41,10 +44,3 @@
Then set the scenario from the Android or iOS app by calling `set_scenario` on
platform channel `driver`.
-
-## Output validation
-
-When using `//flutter/testing/scenario_app/run_android_tests.sh` the generated
-output will be checked against a golden file at
-`//flutter/testing/scenario_app_android_output.txt` to make sure all output was
-generated. A patch will be printed to stdout if they don't match.
diff --git a/testing/scenario_app/android/README.md b/testing/scenario_app/android/README.md
index 164039a..4dd6d2f 100644
--- a/testing/scenario_app/android/README.md
+++ b/testing/scenario_app/android/README.md
@@ -5,15 +5,16 @@
run the tests, you will need to build the engine with the appropriate
configuration.
-For example, `android_debug_unopt` or `android_debug_unopt_arm64` was built,
-run:
+For example, for the latest `android` build you've made locally:
```sh
-# From the root of the engine repository
-$ ./testing/scenario_app/run_android_tests.sh android_debug_unopt
+dart ./testing/scenario_app/bin/run_android_tests.dart
+```
-# Or, for arm64
-$ ./testing/scenario_app/run_android_tests.sh android_debug_unopt_arm64
+Or for a specific, build, such as `android_debug_unopt_arm64`:
+
+```sh
+dart ./testing/scenario_app/bin/run_android_tests.dart --out-dir=../out/android_debug_unopt_arm64
```
## Debugging
@@ -28,8 +29,7 @@
to verify the setup:
```sh
-# From the root of the engine repository
-$ ./testing/scenario_app/run_android_tests.sh android_debug_unopt_arm64 --smoke-test dev.flutter.scenarios.EngineLaunchE2ETest
+dart ./testing/scenario_app/bin/run_android_tests.dart --smoke-test dev.flutter.scenarios.EngineLaunchE2ETest
```
The result of `adb logcat` and screenshots taken during the test will be stored
@@ -43,7 +43,7 @@
## CI Configuration
See [`ci/builders/linux_android_emulator.json`](../../../ci/builders/linux_android_emulator.json)
-, and grep for `run_android_tests.sh`.
+, and grep for `run_android_tests.dart`.
The following matrix of configurations is tested on the CI:
@@ -67,3 +67,9 @@
## Updating Gradle dependencies
See [Updating the Embedding Dependencies](../../../tools/cipd/android_embedding_bundle/README.md).
+
+## Output validation
+
+The generated output will be checked against a golden file
+([`expected_golden_output.txt`](./expected_golden_output.txt)) to make sure all
+output was generated. A patch will be printed to stdout if they don't match.
diff --git a/testing/scenario_app_android_output.txt b/testing/scenario_app/android/expected_golden_output.txt
similarity index 100%
rename from testing/scenario_app_android_output.txt
rename to testing/scenario_app/android/expected_golden_output.txt
diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/run_android_tests.dart
similarity index 87%
rename from testing/scenario_app/bin/android_integration_tests.dart
rename to testing/scenario_app/bin/run_android_tests.dart
index 0812f4a..f0ad827 100644
--- a/testing/scenario_app/bin/android_integration_tests.dart
+++ b/testing/scenario_app/bin/run_android_tests.dart
@@ -46,7 +46,7 @@
)
..addOption(
'adb',
- help: 'Absolute path to the adb tool',
+ help: 'Path to the adb tool',
defaultsTo: engine != null ? join(
engine.srcDir.path,
'third_party',
@@ -57,6 +57,30 @@
) : null,
)
..addOption(
+ 'ndk-stack',
+ help: 'Path to the ndk-stack tool',
+ defaultsTo: engine != null ? join(
+ engine.srcDir.path,
+ 'third_party',
+ 'android_tools',
+ 'ndk',
+ 'prebuilt',
+ () {
+ if (Platform.isLinux) {
+ return 'linux-x86_64';
+ } else if (Platform.isMacOS) {
+ return 'darwin-x86_64';
+ } else if (Platform.isWindows) {
+ return 'windows-x86_64';
+ } else {
+ throw UnsupportedError('Unsupported platform: ${Platform.operatingSystem}');
+ }
+ }(),
+ 'bin',
+ 'ndk-stack',
+ ) : null,
+ )
+ ..addOption(
'out-dir',
help: 'Out directory',
defaultsTo:
@@ -68,7 +92,7 @@
)
..addOption(
'smoke-test',
- help: 'runs a single test to verify the setup',
+ help: 'Runs a single test to verify the setup',
)
..addFlag(
'use-skia-gold',
@@ -79,8 +103,17 @@
'enable-impeller',
help: 'Enable Impeller for the Android app.',
)
- ..addOption('output-contents-golden',
- help: 'Path to a file that will be used to check the contents of the output to make sure everything was created.',
+ ..addOption(
+ 'output-contents-golden',
+ help: 'Path to a file that contains the expected filenames of golden files.',
+ defaultsTo: engine != null ? join(
+ engine.srcDir.path,
+ 'flutter',
+ 'testing',
+ 'scenario_app',
+ 'android',
+ 'expected_golden_output.txt',
+ ) : null,
)
..addOption(
'impeller-backend',
@@ -123,6 +156,10 @@
panic(<String>['invalid graphics-backend', results['impeller-backend'] as String? ?? '<null>']);
}
final Directory logsDir = Directory(results['logs-dir'] as String? ?? join(outDir.path, 'scenario_app', 'logs'));
+ final String? ndkStack = results['ndk-stack'] as String?;
+ if (ndkStack == null) {
+ panic(<String>['--ndk-stack is required']);
+ }
await _run(
verbose: verbose,
outDir: outDir,
@@ -133,6 +170,7 @@
impellerBackend: impellerBackend,
logsDir: logsDir,
contentsGolden: contentsGolden,
+ ndkStack: ndkStack,
);
exit(0);
},
@@ -172,6 +210,7 @@
required _ImpellerBackend? impellerBackend,
required Directory logsDir,
required String? contentsGolden,
+ required String ndkStack,
}) async {
const ProcessManager pm = LocalProcessManager();
@@ -187,6 +226,9 @@
final String logcatPath = join(logsDir.path, 'logcat.txt');
// TODO(matanlurey): Use screenshots/ sub-directory (https://github.com/flutter/flutter/issues/143604).
+ if (!logsDir.existsSync()) {
+ logsDir.createSync(recursive: true);
+ }
final String screenshotPath = logsDir.path;
final String apkOutPath = join(scenarioAppPath, 'app', 'outputs', 'apk');
final File testApk = File(join(apkOutPath, 'androidTest', 'debug', 'app-debug-androidTest.apk'));
@@ -279,6 +321,10 @@
case null:
break;
case 'ActivityManager':
+ // These are mostly noise, i.e. "D ActivityManager: freezing 24632 com.blah".
+ if (adbLogLine!.severity == 'D') {
+ break;
+ }
// TODO(matanlurey): Figure out why this isn't 'flutter.scenario' or similar.
// Also, why is there two different names?
case 'utter.scenario':
@@ -388,6 +434,33 @@
} finally {
await server.close();
+ await step('Killing logcat process...', () async {
+ final bool delivered = logcatProcess.kill(ProcessSignal.sigkill);
+ assert(delivered);
+ await logcatProcessExitCode;
+ });
+
+ await step('Flush logcat...', () async {
+ await logcat.flush();
+ await logcat.close();
+ log('wrote logcat to $logcatPath');
+ });
+
+ await step('Symbolize stack traces', () async {
+ final ProcessResult result = await pm.run(
+ <String>[
+ ndkStack,
+ '-sym',
+ outDir.path,
+ '-dump',
+ logcatPath,
+ ],
+ );
+ if (result.exitCode != 0) {
+ panic(<String>['Failed to symbolize stack traces']);
+ }
+ });
+
await step('Remove reverse port...', () async {
final int exitCode = await pm.runAndForward(<String>[
adb.path,
@@ -413,12 +486,6 @@
}
});
- await step('Killing logcat process...', () async {
- final bool delivered = logcatProcess.kill(ProcessSignal.sigkill);
- assert(delivered);
- await logcatProcessExitCode;
- });
-
await step('Wait for Skia gold comparisons...', () async {
await Future.wait(pendingComparisons);
});
@@ -426,6 +493,8 @@
if (contentsGolden != null) {
// Check the output here.
await step('Check output files...', () async {
+ // TODO(matanlurey): Resolve this in a better way. On CI this file always exists.
+ File(join(screenshotPath, 'noop.txt')).writeAsStringSync('');
// TODO(gaaclarke): We should move this into dir_contents_diff.
_withTemporaryCwd(contentsGolden, () {
final int exitCode = dirContentsDiff(basename(contentsGolden), screenshotPath);
@@ -435,11 +504,5 @@
});
});
}
-
- await step('Flush logcat...', () async {
- await logcat.flush();
- await logcat.close();
- log('wrote logcat to $logcatPath');
- });
}
}
diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart
index def78fb..ca67ab1 100644
--- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart
+++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart
@@ -60,6 +60,9 @@
/// The full line of `adb logcat` output.
String get line => _match.group(0)!;
+ /// The character representing the severity of the log message, such as `I`.
+ String get severity => _match.group(2)!;
+
/// The process name, such as `ActivityManager`.
String get process => _match.group(3)!;
diff --git a/testing/scenario_app/run_android_tests.sh b/testing/scenario_app/run_android_tests.sh
deleted file mode 100755
index 18d14a7..0000000
--- a/testing/scenario_app/run_android_tests.sh
+++ /dev/null
@@ -1,96 +0,0 @@
-#!/bin/bash
-# Copyright 2013 The Flutter Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-# Runs the Android scenario tests on a connected device.
-# To run the test on a x64 emulator, build `android_debug_unopt_x64`, and then run
-# `./run_android_tests.sh android_debug_unopt_x64`.
-
-set -e
-
-# Check number of args.
-if [ $# -lt 1 ]; then
- echo "Usage: $0 <variant> [flags*]"
- exit 1
-fi
-
-# Needed because if it is set, cd may print the path it changed to.
-unset CDPATH
-
-BUILD_VARIANT=$1
-
-# On Mac OS, readlink -f doesn't work, so follow_links traverses the path one
-# link at a time, and then cds into the link destination and find out where it
-# ends up.
-#
-# The function is enclosed in a subshell to avoid changing the working directory
-# of the caller.
-function follow_links() (
- cd -P "$(dirname -- "$1")"
- file="$PWD/$(basename -- "$1")"
- while [[ -L "$file" ]]; do
- cd -P "$(dirname -- "$file")"
- file="$(readlink -- "$file")"
- cd -P "$(dirname -- "$file")"
- file="$PWD/$(basename -- "$file")"
- done
- echo "$file"
-)
-
-SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")")
-SRC_DIR="$(
- cd "$SCRIPT_DIR/../../.."
- pwd -P
-)"
-OUT_DIR="$SRC_DIR/out/$BUILD_VARIANT"
-CONTENTS_GOLDEN="$SRC_DIR/flutter/testing/scenario_app_android_output.txt"
-
-# TODO(matanlurey): If the test runner was purely in Dart, this would not have
-# been necesesary to repeat. However my best guess is the Dart script was seen
-# as potentially crashing, so it was wrapped in a shell script. If we can change
-# this, we should.
-#
-# Define a logs directory for ADB and screenshots.
-# By default, it should be the environment variable FLUTTER_LOGS_DIR, but if
-# it's not set, use the output directory and append "scenario_app/logs".
-LOGS_DIR=${FLUTTER_LOGS_DIR:-"$OUT_DIR/scenario_app/logs"}
-
-# Create the logs directory if it doesn't exist.
-mkdir -p "$LOGS_DIR"
-
-# Dump the logcat and symbolize stack traces before exiting.
-function dumpLogcat {
- ndkstack="windows-x86_64"
- if [ "$(uname)" == "Darwin" ]; then
- ndkstack="darwin-x86_64"
- elif [ "$(expr substr $(uname -s) 1 5)" == "Linux" ]; then
- ndkstack="linux-x86_64"
- fi
-
- # Get the expected location of logcat.txt.
- logcat_file="$LOGS_DIR/logcat.txt"
-
- echo "-> Symbolize stack traces"
- "$SRC_DIR"/third_party/android_tools/ndk/prebuilt/"$ndkstack"/bin/ndk-stack \
- -sym "$OUT_DIR" \
- -dump "$logcat_file"
- echo "<- Done"
-
- # Output the directory for the logs.
- echo "TIP: Full logs are in $LOGS_DIR"
-}
-
-# On error, dump the logcat and symbolize stack traces.
-trap dumpLogcat ERR
-
-cd $SCRIPT_DIR
-
-"$SRC_DIR"/third_party/dart/tools/sdks/dart-sdk/bin/dart pub get
-
-"$SRC_DIR"/third_party/dart/tools/sdks/dart-sdk/bin/dart run \
- "$SCRIPT_DIR"/bin/android_integration_tests.dart \
- --out-dir="$OUT_DIR" \
- --logs-dir="$LOGS_DIR" \
- --output-contents-golden="$CONTENTS_GOLDEN" \
- "$@"
diff --git a/testing/scenario_app/tool/run_android_tests_smoke.sh b/testing/scenario_app/tool/run_android_tests_smoke.sh
deleted file mode 100755
index 87ab86e..0000000
--- a/testing/scenario_app/tool/run_android_tests_smoke.sh
+++ /dev/null
@@ -1,37 +0,0 @@
-#!/bin/bash
-# Copyright 2013 The Flutter Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-# This is a debugging script that runs a single Android E2E test on a connected
-# device or emulator, and reports the exit code. It was largely created to debug
-# why `./testing/scenario_app/run_android_tests.sh` did or did not report
-# failures correctly.
-
-ADB="../third_party/android_tools/sdk/platform-tools/adb"
-OUT_DIR="../out/android_debug_unopt_arm64"
-SMOKE_TEST="dev.flutter.scenarios.EngineLaunchE2ETest"
-
-# Optionally skip installation if -s is passed.
-if [ "$1" != "-s" ]; then
- # Install the app and test APKs.
- echo "Installing app and test APKs..."
- $ADB install -r $OUT_DIR/scenario_app/app/outputs/apk/debug/app-debug.apk
- $ADB install -r $OUT_DIR/scenario_app/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk
-fi
-
-# Configure the device for testing.
-echo "Configuring device for testing..."
-$ADB shell settings put secure immersive_mode_confirmations confirmed
-
-# Reverse port 3000 to the device.
-echo "Reversing port 3000 to the device..."
-$ADB reverse tcp:3000 tcp:3000
-
-# Run the test.
-echo "Running test..."
-$ADB shell am instrument -w -r -e class $SMOKE_TEST dev.flutter.scenarios.test/dev.flutter.TestRunner
-
-# Reverse port 3000 to the device.
-echo "Reversing port 3000 to the device..."
-$ADB reverse --remove tcp:3000
