[flutter_plugin_tools] Auto-retry failed FTL tests (#4610)

stuartmorgan-g · web-flow · commit 92539fc24765 · 2021-12-15T08:22:52.000-08:00
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 flutter/flutter#95063
diff --git 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
@@ -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<String> args = <String>[
-        '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(<String>['--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<bool> _runFirebaseTest(
+    RepositoryPackage example,
+    File test, {
+    required String resultsDir,
+  }) async {
+    final List<String> args = <String>[
+      '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')) ...<String>[
+        '--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
@@ -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: <String>[
+        'example/integration_test/bar_test.dart',
+        'example/integration_test/foo_test.dart',
+        'example/android/gradlew',
+        javaTestFileRelativePath,
+      ]);
+      _writeJavaTestFile(pluginDir, javaTestFileRelativePath);
+
+      processRunner.mockProcessesForExecutable['gcloud'] = <Process>[
+        MockProcess(), // auth
+        MockProcess(), // config
+        MockProcess(exitCode: 1), // integration test #1
+        MockProcess(), // integration test #1 retry
+        MockProcess(), // integration test #2
+      ];
+
+      final List<String> output = await runCapturingPrint(runner, <String>[
+        'firebase-test-lab',
+        '--device',
+        'model=redfin,version=30',
+      ]);
+
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          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: <String>[
         'example/integration_test/foo_test.dart',
