From 7855a2036950b0e9a47ac82b867bcaff2276d90e Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 15 Dec 2021 11:22:52 -0500 Subject: [PATCH] [flutter_plugin_tools] Auto-retry failed FTL tests (#4610) Currently the flake situation for Firebase Test Lab tests is very bad, and the task running those tests are some of the slowest tasks in the CI. Re-running failed tests is slow, manual work that is signficantly affecting productivity. There are plans to actually address the flake in the short-to-medium term, but in the immediate term this will improve the CI situation, as well as reducing the drag on engineering time that could be spent on the root causes. Part of https://github.com/flutter/flutter/issues/95063 --- script/tool/CHANGELOG.md | 2 + .../lib/src/firebase_test_lab_command.dart | 74 +++++++++++++------ .../test/firebase_test_lab_command_test.dart | 41 +++++++++- 3 files changed, 93 insertions(+), 24 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 72539c24c5..12ccf17d4d 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -13,6 +13,8 @@ - Fix `federation-safety-check` handling of plugin deletion, and of top-level files in unfederated plugins whose names match federated plugin heuristics (e.g., `packages/foo/foo_android.iml`). +- Add an auto-retry for failed Firebase Test Lab tests as a short-term patch + for flake issues. ## 0.7.3 diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 4e53ee8fba..e824d8ad1a 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -176,30 +176,22 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { final String testRunId = getStringArg('test-run-id'); final String resultsDir = 'plugins_android_test/${package.displayName}/$buildId/$testRunId/${resultsCounter++}/'; - final List args = [ - 'firebase', - 'test', - 'android', - 'run', - '--type', - 'instrumentation', - '--app', - 'build/app/outputs/apk/debug/app-debug.apk', - '--test', - 'build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk', - '--timeout', - '7m', - '--results-bucket=${getStringArg('results-bucket')}', - '--results-dir=$resultsDir', - ]; - for (final String device in getStringListArg('device')) { - args.addAll(['--device', device]); - } - final int exitCode = await processRunner.runAndStream('gcloud', args, - workingDir: example.directory); - if (exitCode != 0) { - printError('Test failure for $testName'); + // Automatically retry failures; there is significant flake with these + // tests whose cause isn't yet understood, and having to re-run the + // entire shard for a flake in any one test is extremely slow. This should + // be removed once the root cause of the flake is understood. + // See https://github.com/flutter/flutter/issues/95063 + const int maxRetries = 2; + bool passing = false; + for (int i = 1; i <= maxRetries && !passing; ++i) { + if (i > 1) { + logWarning('$testName failed on attempt ${i - 1}. Retrying...'); + } + passing = await _runFirebaseTest(example, test, resultsDir: resultsDir); + } + if (!passing) { + printError('Test failure for $testName after $maxRetries attempts'); errors.add('$testName failed tests'); } } @@ -238,6 +230,42 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { return true; } + /// Runs [test] from [example] as a Firebase Test Lab test, returning true if + /// the test passed. + /// + /// [resultsDir] should be a unique-to-the-test-run directory to store the + /// results on the server. + Future _runFirebaseTest( + RepositoryPackage example, + File test, { + required String resultsDir, + }) async { + final List args = [ + 'firebase', + 'test', + 'android', + 'run', + '--type', + 'instrumentation', + '--app', + 'build/app/outputs/apk/debug/app-debug.apk', + '--test', + 'build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk', + '--timeout', + '7m', + '--results-bucket=${getStringArg('results-bucket')}', + '--results-dir=$resultsDir', + for (final String device in getStringListArg('device')) ...[ + '--device', + device + ], + ]; + final int exitCode = await processRunner.runAndStream('gcloud', args, + workingDir: example.directory); + + return exitCode == 0; + } + /// Builds [target] using Gradle in the given [project]. Assumes Gradle is /// already configured. /// diff --git a/script/tool/test/firebase_test_lab_command_test.dart b/script/tool/test/firebase_test_lab_command_test.dart index 65f398b32c..1dfd8ba66b 100644 --- a/script/tool/test/firebase_test_lab_command_test.dart +++ b/script/tool/test/firebase_test_lab_command_test.dart @@ -271,7 +271,7 @@ public class MainActivityTest { ); }); - test('fails if a test fails', () async { + test('fails if a test fails twice', () async { const String javaTestFileRelativePath = 'example/android/app/src/androidTest/MainActivityTest.java'; final Directory pluginDir = @@ -287,6 +287,7 @@ public class MainActivityTest { MockProcess(), // auth MockProcess(), // config MockProcess(exitCode: 1), // integration test #1 + MockProcess(exitCode: 1), // integration test #1 retry MockProcess(), // integration test #2 ]; @@ -315,6 +316,44 @@ public class MainActivityTest { ); }); + test('passes with warning if a test fails once, then passes on retry', + () async { + const String javaTestFileRelativePath = + 'example/android/app/src/androidTest/MainActivityTest.java'; + final Directory pluginDir = + createFakePlugin('plugin', packagesDir, extraFiles: [ + 'example/integration_test/bar_test.dart', + 'example/integration_test/foo_test.dart', + 'example/android/gradlew', + javaTestFileRelativePath, + ]); + _writeJavaTestFile(pluginDir, javaTestFileRelativePath); + + processRunner.mockProcessesForExecutable['gcloud'] = [ + MockProcess(), // auth + MockProcess(), // config + MockProcess(exitCode: 1), // integration test #1 + MockProcess(), // integration test #1 retry + MockProcess(), // integration test #2 + ]; + + final List output = await runCapturingPrint(runner, [ + 'firebase-test-lab', + '--device', + 'model=redfin,version=30', + ]); + + expect( + output, + containsAllInOrder([ + contains('Testing example/integration_test/bar_test.dart...'), + contains('bar_test.dart failed on attempt 1. Retrying...'), + contains('Testing example/integration_test/foo_test.dart...'), + contains('Ran for 1 package(s) (1 with warnings)'), + ]), + ); + }); + test('fails for packages with no androidTest directory', () async { createFakePlugin('plugin', packagesDir, extraFiles: [ 'example/integration_test/foo_test.dart',