From ddff89ae9c87be68d17c703859ae0428ff5e0189 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 1 Feb 2024 13:08:12 -0800 Subject: [PATCH 1/9] Cause CI to fail when 1 or more tests fail. --- .../scenario_app/bin/android_integration_tests.dart | 11 ++++++++--- .../bin/utils/process_manager_extension.dart | 7 +++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 02fbdfa265290..908d75e859f26 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -170,15 +170,20 @@ void main(List args) async { }); await step('Running instrumented tests...', () async { - final int exitCode = await pm.runAndForward([ + final (int exitCode, StringBuffer out) = await pm.runAndCapture([ adb.path, 'shell', 'am', 'instrument', - '-w', 'dev.flutter.scenarios.test/dev.flutter.TestRunner', + '-w', + 'dev.flutter.scenarios.test/dev.flutter.TestRunner', ]); if (exitCode != 0) { - panic(['could not install test apk']); + panic(['instrumented tests failed to run']); + } + if (out.toString().contains('FAILURES!!!')) { + stdout.write(out); + panic(['1 or more tests failed']); } }); } finally { diff --git a/testing/scenario_app/bin/utils/process_manager_extension.dart b/testing/scenario_app/bin/utils/process_manager_extension.dart index fb705f8f1144e..02499ae3f5fdd 100644 --- a/testing/scenario_app/bin/utils/process_manager_extension.dart +++ b/testing/scenario_app/bin/utils/process_manager_extension.dart @@ -45,4 +45,11 @@ extension RunAndForward on ProcessManager { Future runAndForward(List cmd) async { return pipeProcessStreams(await start(cmd), out: stdout); } + + /// Runs [cmd], and captures the stdout and stderr pipes. + Future<(int, StringBuffer)> runAndCapture(List cmd) async { + final StringBuffer buffer = StringBuffer(); + final int exitCode = await pipeProcessStreams(await start(cmd), out: buffer); + return (exitCode, buffer); + } } From 3d88a4f8c1325255ed8330bf3403c48321569fd2 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 1 Feb 2024 13:46:34 -0800 Subject: [PATCH 2/9] Fix runIf for scenario_app. --- .ci.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci.yaml b/.ci.yaml index b4173be0de09f..593db7141a087 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -58,6 +58,7 @@ targets: - DEPS - lib/ui/** - shell/platform/android/** + - testing/scenario_app/** # Task to run Linux linux_android_emulator_tests on AVDs running Android 33 # instead of 34 for investigating https://github.com/flutter/flutter/issues/137947. @@ -74,6 +75,7 @@ targets: - DEPS - lib/ui/** - shell/platform/android/** + - testing/scenario_app/** - name: Linux builder_cache enabled_branches: From b17c81b1d106e4bb905df8eb9d93d930d8770402 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 1 Feb 2024 13:47:24 -0800 Subject: [PATCH 3/9] I suck at whitespace. --- .ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci.yaml b/.ci.yaml index 593db7141a087..28c698f97b057 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -58,7 +58,7 @@ targets: - DEPS - lib/ui/** - shell/platform/android/** - - testing/scenario_app/** + - testing/scenario_app/** # Task to run Linux linux_android_emulator_tests on AVDs running Android 33 # instead of 34 for investigating https://github.com/flutter/flutter/issues/137947. From 92a81ccbe34f51ecd587fa19e19c3faf3b248d0e Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 1 Feb 2024 15:31:01 -0800 Subject: [PATCH 4/9] Try explicit exitCode = 1. --- testing/scenario_app/bin/utils/logs.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/scenario_app/bin/utils/logs.dart b/testing/scenario_app/bin/utils/logs.dart index 0c7c8a6873bf3..efcbb3daaa91d 100644 --- a/testing/scenario_app/bin/utils/logs.dart +++ b/testing/scenario_app/bin/utils/logs.dart @@ -27,5 +27,6 @@ void panic(List messages) { for (final String message in messages) { stderr.writeln('$_red$message$_reset'); } + exitCode = 1; throw 'panic'; } From 2b8409f87bb018022fb1c057dcfef67e5219d1c1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Feb 2024 11:56:30 -0800 Subject: [PATCH 5/9] ++ --- .../bin/android_integration_tests.dart | 36 +++++++++++++++---- testing/scenario_app/bin/utils/logs.dart | 5 +-- .../tool/run_android_tests_smoke.sh | 18 ++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-) create mode 100755 testing/scenario_app/tool/run_android_tests_smoke.sh diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 908d75e859f26..46f4ba87dbc0b 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -18,14 +18,36 @@ import 'utils/screenshot_transformer.dart'; const int tcpPort = 3001; void main(List args) async { - const ProcessManager pm = LocalProcessManager(); final ArgParser parser = ArgParser() ..addOption('adb', help: 'absolute path to the adb tool', mandatory: true) - ..addOption('out-dir', help: 'out directory', mandatory: true); + ..addOption('out-dir', help: 'out directory', mandatory: true) + ..addFlag('smoke-test', + help: 'runs a single test to verify the setup', negatable: false); + + runZonedGuarded( + () async { + final ArgResults results = parser.parse(args); + final Directory outDir = Directory(results['out-dir'] as String); + final File adb = File(results['adb'] as String); + final bool smokeTest = results['smoke-test'] as bool; + await _run(outDir: outDir, adb: adb, smokeTest: smokeTest); + }, + (Object error, StackTrace stackTrace) { + if (error is! Panic) { + stderr.writeln(error); + stderr.writeln(stackTrace); + } + exit(1); + }, + ); +} - final ArgResults results = parser.parse(args); - final Directory outDir = Directory(results['out-dir'] as String); - final File adb = File(results['adb'] as String); +Future _run({ + required Directory outDir, + required File adb, + required bool smokeTest, +}) async { + const ProcessManager pm = LocalProcessManager(); if (!outDir.existsSync()) { panic(['out-dir does not exist: $outDir', 'make sure to build the selected engine variant']); @@ -176,6 +198,8 @@ void main(List args) async { 'am', 'instrument', '-w', + if (smokeTest) + '-e class dev.flutter.scenarios.EngineLaunchE2ETest.java', 'dev.flutter.scenarios.test/dev.flutter.TestRunner', ]); if (exitCode != 0) { @@ -227,7 +251,5 @@ void main(List args) async { await step('Flush logcat...', () async { await logcat.flush(); }); - - exit(0); } } diff --git a/testing/scenario_app/bin/utils/logs.dart b/testing/scenario_app/bin/utils/logs.dart index efcbb3daaa91d..3132164300dde 100644 --- a/testing/scenario_app/bin/utils/logs.dart +++ b/testing/scenario_app/bin/utils/logs.dart @@ -23,10 +23,11 @@ void log(String msg) { stdout.writeln('$_gray$msg$_reset'); } +final class Panic extends Error {} + void panic(List messages) { for (final String message in messages) { stderr.writeln('$_red$message$_reset'); } - exitCode = 1; - throw 'panic'; + throw Panic(); } diff --git a/testing/scenario_app/tool/run_android_tests_smoke.sh b/testing/scenario_app/tool/run_android_tests_smoke.sh new file mode 100755 index 0000000000000..f04da36fc6594 --- /dev/null +++ b/testing/scenario_app/tool/run_android_tests_smoke.sh @@ -0,0 +1,18 @@ +#!/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. + +# Run this command and print out the exit code. +../third_party/dart/tools/sdks/dart-sdk/bin/dart ./testing/scenario_app/bin/android_integration_tests.dart \ + --adb="../third_party/android_tools/sdk/platform-tools/adb" \ + --out-dir="../out/android_debug_unopt_arm64" \ + --smoke-test + +echo "Exit code: $?" +echo "Done" From c93d68e8afd9d6e055b9830d185deaef594c5cad Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 2 Feb 2024 14:01:56 -0800 Subject: [PATCH 6/9] Fix the test. --- .../bin/android_integration_tests.dart | 3 +++ testing/scenario_app/lib/main.dart | 15 ++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 46f4ba87dbc0b..dde13958f3cf2 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -205,6 +205,9 @@ Future _run({ if (exitCode != 0) { panic(['instrumented tests failed to run']); } + // Unfortunately adb shell am instrument does not return a non-zero exit + // code when tests fail, but it does seem to print "FAILURES!!!" to + // stdout, so we can use that as a signal that something went wrong. if (out.toString().contains('FAILURES!!!')) { stdout.write(out); panic(['1 or more tests failed']); diff --git a/testing/scenario_app/lib/main.dart b/testing/scenario_app/lib/main.dart index 5e76f1d7306bf..6116bb1a80d82 100644 --- a/testing/scenario_app/lib/main.dart +++ b/testing/scenario_app/lib/main.dart @@ -25,14 +25,19 @@ void main() { channelBuffers.setListener('driver', _handleDriverMessage); channelBuffers.setListener('write_timeline', _handleWriteTimelineMessage); - final FlutterView view = PlatformDispatcher.instance.implicitView!; + // TODO(matanlurey): https://github.com/flutter/flutter/issues/142746. + // This Dart program is used for every test, but there is at least one test + // (EngineLaunchE2ETest.java) that does not create a FlutterView, so the + // implicit view's size is not initialized (and the assert would be tripped). + // + // final FlutterView view = PlatformDispatcher.instance.implicitView!; // Asserting that this is greater than zero since this app runs on different // platforms with different sizes. If it is greater than zero, it has been // initialized to some meaningful value at least. - assert( - view.display.size > Offset.zero, - 'Expected ${view.display} to be initialized.', - ); + // assert( + // view.display.size > Offset.zero, + // 'Expected ${view.display} to be initialized.', + // ); final ByteData data = ByteData(1); data.setUint8(0, 1); From c6e7b5ba4fbfe1c0da0c7595d8b4d33875986e2a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 5 Feb 2024 14:00:16 -0800 Subject: [PATCH 7/9] Try smoke test only. --- .../bin/android_integration_tests.dart | 22 ++++++++++++++----- testing/scenario_app/run_android_tests.sh | 5 ++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index dde13958f3cf2..8759e89d581aa 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -19,10 +19,22 @@ const int tcpPort = 3001; void main(List args) async { final ArgParser parser = ArgParser() - ..addOption('adb', help: 'absolute path to the adb tool', mandatory: true) - ..addOption('out-dir', help: 'out directory', mandatory: true) - ..addFlag('smoke-test', - help: 'runs a single test to verify the setup', negatable: false); + ..addOption( + 'adb', + help: 'absolute path to the adb tool', + mandatory: true, + ) + ..addOption( + 'out-dir', + help: 'out directory', + mandatory: true, + ) + ..addFlag( + 'smoke-test', + help: 'runs a single test to verify the setup', + negatable: false, + defaultsTo: true, + ); runZonedGuarded( () async { @@ -199,7 +211,7 @@ Future _run({ 'instrument', '-w', if (smokeTest) - '-e class dev.flutter.scenarios.EngineLaunchE2ETest.java', + '-e class dev.flutter.scenarios.EngineLaunchE2ETest', 'dev.flutter.scenarios.test/dev.flutter.TestRunner', ]); if (exitCode != 0) { diff --git a/testing/scenario_app/run_android_tests.sh b/testing/scenario_app/run_android_tests.sh index eae473cf4cbdd..ec4abf49cb1c8 100755 --- a/testing/scenario_app/run_android_tests.sh +++ b/testing/scenario_app/run_android_tests.sh @@ -33,7 +33,10 @@ function follow_links() ( ) SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")") -SRC_DIR="$(cd "$SCRIPT_DIR/../../.."; pwd -P)" +SRC_DIR="$( + cd "$SCRIPT_DIR/../../.." + pwd -P +)" OUT_DIR="$SRC_DIR/out/$BUILD_VARIANT" # Dump the logcat and symbolize stack traces before exiting. From aea3c4c87dd125c868ed9ff360b9dc77f8697faa Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 5 Feb 2024 17:16:21 -0800 Subject: [PATCH 8/9] Try closing the IOSink. --- testing/scenario_app/bin/android_integration_tests.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 8759e89d581aa..459cb38438f79 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -265,6 +265,7 @@ Future _run({ await step('Flush logcat...', () async { await logcat.flush(); + await logcat.close(); }); } } From ab30e4324098ec04099475d6554a624c75fe1a3f Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 5 Feb 2024 19:36:24 -0800 Subject: [PATCH 9/9] Try re-adding exit(0). --- testing/scenario_app/bin/android_integration_tests.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 459cb38438f79..ccbd0729df800 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -43,6 +43,7 @@ void main(List args) async { final File adb = File(results['adb'] as String); final bool smokeTest = results['smoke-test'] as bool; await _run(outDir: outDir, adb: adb, smokeTest: smokeTest); + exit(0); }, (Object error, StackTrace stackTrace) { if (error is! Panic) {