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

Fixes flutter/flutter#143586.

This should make it easier to understand what is/isn't happening, and will unblock flutter/flutter#143458, and make flutter/flutter#143459 potentially easier to debug.

Author: matanlurey

testing/scenario_app/android/README.md

@@ -16,23 +16,50 @@ $ ./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)
 , and grep for `run_android_tests.sh`.
 
 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

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.

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,13 +115,15 @@ 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,
         smokeTestFullPath: smokeTest,
         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']);

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');
   }

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" \
   "$@"

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