Skip to content

Commit 92539fc

Browse files
[flutter_plugin_tools] Auto-retry failed FTL tests (flutter#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 flutter/flutter#95063
1 parent 910055b commit 92539fc

File tree

3 files changed

+93
-24
lines changed

3 files changed

+93
-24
lines changed

script/tool/CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
- Fix `federation-safety-check` handling of plugin deletion, and of top-level
1414
files in unfederated plugins whose names match federated plugin heuristics
1515
(e.g., `packages/foo/foo_android.iml`).
16+
- Add an auto-retry for failed Firebase Test Lab tests as a short-term patch
17+
for flake issues.
1618

1719
## 0.7.3
1820

script/tool/lib/src/firebase_test_lab_command.dart

+51-23
Original file line numberDiff line numberDiff line change
@@ -176,30 +176,22 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
176176
final String testRunId = getStringArg('test-run-id');
177177
final String resultsDir =
178178
'plugins_android_test/${package.displayName}/$buildId/$testRunId/${resultsCounter++}/';
179-
final List<String> args = <String>[
180-
'firebase',
181-
'test',
182-
'android',
183-
'run',
184-
'--type',
185-
'instrumentation',
186-
'--app',
187-
'build/app/outputs/apk/debug/app-debug.apk',
188-
'--test',
189-
'build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk',
190-
'--timeout',
191-
'7m',
192-
'--results-bucket=${getStringArg('results-bucket')}',
193-
'--results-dir=$resultsDir',
194-
];
195-
for (final String device in getStringListArg('device')) {
196-
args.addAll(<String>['--device', device]);
197-
}
198-
final int exitCode = await processRunner.runAndStream('gcloud', args,
199-
workingDir: example.directory);
200179

201-
if (exitCode != 0) {
202-
printError('Test failure for $testName');
180+
// Automatically retry failures; there is significant flake with these
181+
// tests whose cause isn't yet understood, and having to re-run the
182+
// entire shard for a flake in any one test is extremely slow. This should
183+
// be removed once the root cause of the flake is understood.
184+
// See https://github.com/flutter/flutter/issues/95063
185+
const int maxRetries = 2;
186+
bool passing = false;
187+
for (int i = 1; i <= maxRetries && !passing; ++i) {
188+
if (i > 1) {
189+
logWarning('$testName failed on attempt ${i - 1}. Retrying...');
190+
}
191+
passing = await _runFirebaseTest(example, test, resultsDir: resultsDir);
192+
}
193+
if (!passing) {
194+
printError('Test failure for $testName after $maxRetries attempts');
203195
errors.add('$testName failed tests');
204196
}
205197
}
@@ -238,6 +230,42 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
238230
return true;
239231
}
240232

233+
/// Runs [test] from [example] as a Firebase Test Lab test, returning true if
234+
/// the test passed.
235+
///
236+
/// [resultsDir] should be a unique-to-the-test-run directory to store the
237+
/// results on the server.
238+
Future<bool> _runFirebaseTest(
239+
RepositoryPackage example,
240+
File test, {
241+
required String resultsDir,
242+
}) async {
243+
final List<String> args = <String>[
244+
'firebase',
245+
'test',
246+
'android',
247+
'run',
248+
'--type',
249+
'instrumentation',
250+
'--app',
251+
'build/app/outputs/apk/debug/app-debug.apk',
252+
'--test',
253+
'build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk',
254+
'--timeout',
255+
'7m',
256+
'--results-bucket=${getStringArg('results-bucket')}',
257+
'--results-dir=$resultsDir',
258+
for (final String device in getStringListArg('device')) ...<String>[
259+
'--device',
260+
device
261+
],
262+
];
263+
final int exitCode = await processRunner.runAndStream('gcloud', args,
264+
workingDir: example.directory);
265+
266+
return exitCode == 0;
267+
}
268+
241269
/// Builds [target] using Gradle in the given [project]. Assumes Gradle is
242270
/// already configured.
243271
///

script/tool/test/firebase_test_lab_command_test.dart

+40-1
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public class MainActivityTest {
271271
);
272272
});
273273

274-
test('fails if a test fails', () async {
274+
test('fails if a test fails twice', () async {
275275
const String javaTestFileRelativePath =
276276
'example/android/app/src/androidTest/MainActivityTest.java';
277277
final Directory pluginDir =
@@ -287,6 +287,7 @@ public class MainActivityTest {
287287
MockProcess(), // auth
288288
MockProcess(), // config
289289
MockProcess(exitCode: 1), // integration test #1
290+
MockProcess(exitCode: 1), // integration test #1 retry
290291
MockProcess(), // integration test #2
291292
];
292293

@@ -315,6 +316,44 @@ public class MainActivityTest {
315316
);
316317
});
317318

319+
test('passes with warning if a test fails once, then passes on retry',
320+
() async {
321+
const String javaTestFileRelativePath =
322+
'example/android/app/src/androidTest/MainActivityTest.java';
323+
final Directory pluginDir =
324+
createFakePlugin('plugin', packagesDir, extraFiles: <String>[
325+
'example/integration_test/bar_test.dart',
326+
'example/integration_test/foo_test.dart',
327+
'example/android/gradlew',
328+
javaTestFileRelativePath,
329+
]);
330+
_writeJavaTestFile(pluginDir, javaTestFileRelativePath);
331+
332+
processRunner.mockProcessesForExecutable['gcloud'] = <Process>[
333+
MockProcess(), // auth
334+
MockProcess(), // config
335+
MockProcess(exitCode: 1), // integration test #1
336+
MockProcess(), // integration test #1 retry
337+
MockProcess(), // integration test #2
338+
];
339+
340+
final List<String> output = await runCapturingPrint(runner, <String>[
341+
'firebase-test-lab',
342+
'--device',
343+
'model=redfin,version=30',
344+
]);
345+
346+
expect(
347+
output,
348+
containsAllInOrder(<Matcher>[
349+
contains('Testing example/integration_test/bar_test.dart...'),
350+
contains('bar_test.dart failed on attempt 1. Retrying...'),
351+
contains('Testing example/integration_test/foo_test.dart...'),
352+
contains('Ran for 1 package(s) (1 with warnings)'),
353+
]),
354+
);
355+
});
356+
318357
test('fails for packages with no androidTest directory', () async {
319358
createFakePlugin('plugin', packagesDir, extraFiles: <String>[
320359
'example/integration_test/foo_test.dart',

0 commit comments

Comments
 (0)