Write adb logcat results and screenshots to FLUTTER_LOGS_DIR if able. (flutter/engine#50722)

Fixes https://github.com/flutter/flutter/issues/143586. This should make it easier to understand what is/isn't happening, and will unblock https://github.com/flutter/flutter/issues/143458, and make https://github.com/flutter/flutter/issues/143459 potentially easier to debug.
2026-02-20 02:29:02 +08:00 · 2024-02-18 12:31:28 -08:00 · 2024-02-18 12:31:28 -08:00 · cfac6a5d5f
commit cfac6a5d5f
parent 37acc1a863
6 changed files with 83 additions and 14 deletions
--- a/engine/src/flutter/testing/scenario_app/android/README.md
+++ b/engine/src/flutter/testing/scenario_app/android/README.md
@ -16,6 +16,30 @@ $ ./testing/scenario_app/run_android_tests.sh android_debug_unopt
 $ ./testing/scenario_app/run_android_tests.sh android_debug_unopt_arm64
 ```

+## Debugging
+
+Debugging the tests on CI is not straightforward but is being improved:
+
+- <https://github.com/flutter/flutter/issues/143458>
+- <https://github.com/flutter/flutter/issues/143459>
+
+Locally (or on a temporary PR for CI), you can run the tests with the
+`--smoke-test` argument to run a single test by class name, which can be useful
+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
+```
+
+The result of `adb logcat` and screenshots taken during the test will be stored
+in a logs directory, which is either `FLUTTER_LOGS_DIR` (if set, such as on CI)
+or locally in `out/.../scenario_app/logs`.
+
+You can then view the logs and screenshots on LUCI. [For example](https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20Engine%20Drone/2003164/overview):
+
+![Screenshot of the Logs on LUCI](https://github.com/flutter/engine/assets/168174/79dc864c-c18b-4df9-a733-fd55301cc69c).
+
 ## CI Configuration

 See [`ci/builders/linux_android_emulator.json`](../../../ci/builders/linux_android_emulator.json)
@ -23,16 +47,19 @@ See [`ci/builders/linux_android_emulator.json`](../../../ci/builders/linux_andro

 The following matrix of configurations is tested on the CI:

-| API Version | Graphics Backend    | Skia Gold                                                        | Rationale                                                  |
-| ----------- | ------------------- | ---------------------------------------------------------------- | ---------------------------------------------------------- |
+<!-- TODO(matanlurey): Blocked by https://github.com/flutter/flutter/issues/143471.
 | 28          | Skia                | [Android 28 + Skia][skia-gold-skia-28]                           | Older Android devices (without `ImageReader`) on Skia.     |
 | 28          | Impeller (OpenGLES) | [Android 28 + Impeller OpenGLES][skia-gold-impeller-opengles-28] | Older Android devices (without `ImageReader`) on Impeller. |
-| 34          | Skia                | [Android 34 + Skia][skia-gold-skia-34]                           | Newer Android devices on Skia.                             |
-| 34          | Impeller (OpenGLES) | [Android 34 + Impeller OpenGLES][skia-gold-impeller-opengles-34] | Newer Android devices on Impeller with OpenGLES.           |
-| 34          | Impeller (Vulkan)   | [Android 34 + Impeller Vulkan][skia-gold-impeller-vulkan-34]     | Newer Android devices on Impeller.                         |
-
 [skia-gold-skia-28]: https://flutter-engine-gold.skia.org/search?left_filter=AndroidAPILevel%3D28%26GraphicsBackend%3Dskia&negative=true&positive=true&right_filter=AndroidAPILevel%3D28%26GraphicsBackend%3Dskia
 [skia-gold-impeller-opengles-28]: https://flutter-engine-gold.skia.org/search?left_filter=AndroidAPILevel%3D28%26GraphicsBackend%3Dimpeller-opengles&negative=true&positive=true&right_filter=AndroidAPILevel%3D28%26GraphicsBackend%3Dimpeller-opengles
+-->
+
+| API Version | Graphics Backend    | Skia Gold                                                        | Rationale                                        |
+| ----------- | ------------------- | ---------------------------------------------------------------- | ------------------------------------------------ |
+| 34          | Skia                | [Android 34 + Skia][skia-gold-skia-34]                           | Newer Android devices on Skia.                   |
+| 34          | Impeller (OpenGLES) | [Android 34 + Impeller OpenGLES][skia-gold-impeller-opengles-34] | Newer Android devices on Impeller with OpenGLES. |
+| 34          | Impeller (Vulkan)   | [Android 34 + Impeller Vulkan][skia-gold-impeller-vulkan-34]     | Newer Android devices on Impeller.               |
+
 [skia-gold-skia-34]: https://flutter-engine-gold.skia.org/search?left_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dskia&negative=true&positive=true&right_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dskia
 [skia-gold-impeller-opengles-34]: https://flutter-engine-gold.skia.org/search?left_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dimpeller-opengles&negative=true&positive=true&right_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dimpeller-opengles
 [skia-gold-impeller-vulkan-34]: https://flutter-engine-gold.skia.org/search?left_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dimpeller-vulkan&negative=true&positive=true&right_filter=AndroidAPILevel%3D34%26GraphicsBackend%3Dimpeller-vulkan
--- a/engine/src/flutter/testing/scenario_app/bin/README.md
+++ b/engine/src/flutter/testing/scenario_app/bin/README.md
@ -30,6 +30,9 @@ dart bin/android_integration_tests.dart --smoke-test dev.flutter.scenarios.Engin
 - `--out-dir`: The directory containing the build artifacts. Defaults to the
  last updated build directory in `out/` that starts with `android_`.

+- `--logs-dir`: The directory to store logs and screenshots. Defaults to
+  `FLUTTER_LOGS_DIR` if set, or `out/.../scenario_app/logs` otherwise.
+
 - `--use-skia-gold`: Use Skia Gold to compare screenshots. Defaults to true
  when running on CI, and false otherwise (i.e. when running locally). If
  set to true, `isSkiaGoldClientAvailable` must be true.
--- a/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart
+++ b/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart
@ -81,6 +81,13 @@ void main(List<String> args) async {
      help: 'The Impeller backend to use for the Android app.',
      allowed: <String>['vulkan', 'opengles'],
      defaultsTo: 'vulkan',
+    )
+    ..addOption(
+      'logs-dir',
+      help: 'The directory to store the logs and screenshots. Defaults to '
+            'the value of the FLUTTER_LOGS_DIR environment variable, if set, '
+            'otherwise it defaults to a path within out-dir.',
+      defaultsTo: Platform.environment['FLUTTER_LOGS_DIR'],
    );

  runZonedGuarded(
@ -108,6 +115,7 @@ void main(List<String> args) async {
      if (enableImpeller && impellerBackend == null) {
        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'));
      await _run(
        outDir: outDir,
        adb: adb,
@ -115,6 +123,7 @@ void main(List<String> args) async {
        useSkiaGold: useSkiaGold,
        enableImpeller: enableImpeller,
        impellerBackend: impellerBackend,
+        logsDir: logsDir,
        contentsGolden: contentsGolden,
      );
      exit(0);
@ -152,6 +161,7 @@ Future<void> _run({
  required bool useSkiaGold,
  required bool enableImpeller,
  required _ImpellerBackend? impellerBackend,
+  required Directory logsDir,
  required String? contentsGolden,
 }) async {
  const ProcessManager pm = LocalProcessManager();
@ -165,11 +175,14 @@ Future<void> _run({
  }

  final String scenarioAppPath = join(outDir.path, 'scenario_app');
-  final String logcatPath = join(scenarioAppPath, 'logcat.txt');
-  final String screenshotPath = join(scenarioAppPath, 'screenshots');
+  final String logcatPath = join(logsDir.path, 'logcat.txt');
+
+  // TODO(matanlurey): Use screenshots/ sub-directory (https://github.com/flutter/flutter/issues/143604).
+  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'));
  final File appApk = File(join(apkOutPath, 'debug', 'app-debug.apk'));
+  log('writing logs and screenshots to ${logsDir.path}');

  if (!testApk.existsSync()) {
    panic(<String>['test apk does not exist: ${testApk.path}', 'make sure to build the selected engine variant']);
@ -347,14 +360,14 @@ Future<void> _run({
      }
    });

-    await step('Uinstalling app APK...', () async {
+    await step('Uninstalling app APK...', () async {
      final int exitCode = await pm.runAndForward(<String>[adb.path, 'uninstall', 'dev.flutter.scenarios']);
      if (exitCode != 0) {
        panic(<String>['could not uninstall app apk']);
      }
    });

-    await step('Uinstalling test APK...', () async {
+    await step('Uninstalling test APK...', () async {
      final int exitCode = await pm.runAndForward(<String>[adb.path, 'uninstall', 'dev.flutter.scenarios.test']);
      if (exitCode != 0) {
        panic(<String>['could not uninstall app apk']);
--- a/engine/src/flutter/testing/scenario_app/bin/utils/logs.dart
+++ b/engine/src/flutter/testing/scenario_app/bin/utils/logs.dart
@ -14,6 +14,9 @@ Future<void> step(String msg, Future<void> Function() fn) async {
  stdout.writeln('-> $_green$msg$_reset');
  try {
    await fn();
+  } catch (_) {
+    stderr.writeln('~~ ${_red}Failed$_reset');
+    rethrow;
  } finally {
    stdout.writeln('<- ${_gray}Done$_reset');
  }
--- a/engine/src/flutter/testing/scenario_app/run_android_tests.sh
+++ b/engine/src/flutter/testing/scenario_app/run_android_tests.sh
@ -46,6 +46,19 @@ SRC_DIR="$(
 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"
@ -55,18 +68,25 @@ function dumpLogcat {
    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 "$OUT_DIR"/scenario_app/logcat.txt
+    -dump "$logcat_file"
  echo "<- Done"

  echo "-> Dump full logcat"
-  cat "$OUT_DIR"/scenario_app/logcat.txt
+  cat "$logcat_file"
  echo "<- Done"
+
+  # Output the directory for the logs.
+  echo "TIP: Full logs are in $LOGS_DIR"
 }

-trap dumpLogcat EXIT
+# On error, dump the logcat and symbolize stack traces.
+trap dumpLogcat ERR

 cd $SCRIPT_DIR

@ -75,5 +95,6 @@ cd $SCRIPT_DIR
 "$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" \
  "$@"
--- a/engine/src/flutter/testing/scenario_app_android_output.txt
+++ b/engine/src/flutter/testing/scenario_app_android_output.txt
@ -57,4 +57,6 @@ PlatformViewWithTextureViewUiTest_testPlatformViewRotate.png
 PlatformViewWithTextureViewUiTest_testPlatformViewTransform.png
 PlatformViewWithTextureViewUiTest_testPlatformViewTwoIntersectingOverlays.png
 PlatformViewWithTextureViewUiTest_testPlatformViewWithoutOverlayIntersection.png
-SpawnEngineTests_testSpawnedEngine.png
+SpawnEngineTests_testSpawnedEngine.png
+logcat.txt
+noop.txt