More flexible timeout logic in flutter_test (#18256)

This should reduce the number of flakes without actually increasing
the timeout, so we'll still find out quickly if a test is hanging.

The numbers here might need tweaking. Maybe the default two seconds is
too short for CI bots.

dev/automated_tests/test_smoke_test/timeout_fail_test.dart

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  testWidgets('flutter_test timeout logic - addTime - negative', (WidgetTester tester) async {
+    await tester.runAsync(() async {
+      await new Future<void>.delayed(const Duration(milliseconds: 3500));
+    }, additionalTime: const Duration(milliseconds: 200));
+  });
+}

dev/automated_tests/test_smoke_test/timeout_pass_test.dart

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  testWidgets('flutter_test timeout logic - addTime - positive', (WidgetTester tester) async {
+    await tester.runAsync(() async {
+      await new Future<void>.delayed(const Duration(milliseconds: 3500));
+    }, additionalTime: const Duration(milliseconds: 2000));
+  });
+}

dev/bots/test.dart

@@ -50,8 +50,11 @@ Future<Null> main(List<String> args) async {
 
   final String shard = Platform.environment['SHARD'];
   if (shard != null) {
-    if (!_kShards.containsKey(shard))
-      throw new ArgumentError('Invalid shard: $shard');
+    if (!_kShards.containsKey(shard)) {
+      print('Invalid shard: $shard');
+      print('The available shards are: ${_kShards.keys.join(", ")}');
+      exit(1);
+    }
     print('${bold}SHARD=$shard$reset');
     await _kShards[shard]();
   } else {
@@ -132,55 +135,79 @@ Future<Null> _analyzeRepo() async {
 }
 
 Future<Null> _runTests() async {
-  // Verify that the tests actually return failure on failure and success on success.
+  // Verify that the tests actually return failure on failure and success on
+  // success.
   final String automatedTests = path.join(flutterRoot, 'dev', 'automated_tests');
-  await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'fail_test.dart'),
-    expectFailure: true,
-    printOutput: false,
-    timeout: _kShortTimeout,
-  );
+  // We run the "pass" and "fail" smoke tests first, and alone, because those
+  // are particularly critical and sensitive. If one of these fails, there's no
+  // point even trying the others.
   await _runFlutterTest(automatedTests,
     script: path.join('test_smoke_test', 'pass_test.dart'),
     printOutput: false,
     timeout: _kShortTimeout,
   );
   await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'crash1_test.dart'),
-    expectFailure: true,
-    printOutput: false,
-    timeout: _kShortTimeout,
-  );
-  await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'crash2_test.dart'),
-    expectFailure: true,
-    printOutput: false,
-    timeout: _kShortTimeout,
-  );
-  await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'syntax_error_test.broken_dart'),
+    script: path.join('test_smoke_test', 'fail_test.dart'),
     expectFailure: true,
     printOutput: false,
     timeout: _kShortTimeout,
   );
+  // We run the timeout tests individually because they are timing-sensitive.
   await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'missing_import_test.broken_dart'),
-    expectFailure: true,
+    script: path.join('test_smoke_test', 'timeout_pass_test.dart'),
+    expectFailure: false,
     printOutput: false,
     timeout: _kShortTimeout,
   );
   await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'disallow_error_reporter_modification_test.dart'),
+    script: path.join('test_smoke_test', 'timeout_fail_test.dart'),
     expectFailure: true,
     printOutput: false,
     timeout: _kShortTimeout,
   );
-  await _runCommand(flutter,
-    <String>['drive', '--use-existing-app', '-t', path.join('test_driver', 'failure.dart')],
-    workingDirectory: path.join(flutterRoot, 'packages', 'flutter_driver'),
-    expectFailure: true,
-    printOutput: false,
-    timeout: _kShortTimeout,
+  // We run the remaining smoketests in parallel, because they each take some
+  // time to run (e.g. compiling), so we don't want to run them in series,
+  // especially on 20-core machines...
+  await Future.wait<void>(
+    <Future<void>>[
+      _runFlutterTest(automatedTests,
+        script: path.join('test_smoke_test', 'crash1_test.dart'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+      _runFlutterTest(automatedTests,
+        script: path.join('test_smoke_test', 'crash2_test.dart'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+      _runFlutterTest(automatedTests,
+        script: path.join('test_smoke_test', 'syntax_error_test.broken_dart'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+      _runFlutterTest(automatedTests,
+        script: path.join('test_smoke_test', 'missing_import_test.broken_dart'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+      _runFlutterTest(automatedTests,
+        script: path.join('test_smoke_test', 'disallow_error_reporter_modification_test.dart'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+      _runCommand(flutter,
+        <String>['drive', '--use-existing-app', '-t', path.join('test_driver', 'failure.dart')],
+        workingDirectory: path.join(flutterRoot, 'packages', 'flutter_driver'),
+        expectFailure: true,
+        printOutput: false,
+        timeout: _kShortTimeout,
+      ),
+    ],
   );
 
   // Verify that we correctly generated the version file.
@@ -369,8 +396,20 @@ Future<Null> _runFlutterTest(String workingDirectory, {
   final List<String> args = <String>['test']..addAll(options);
   if (flutterTestArgs != null && flutterTestArgs.isNotEmpty)
     args.addAll(flutterTestArgs);
-  if (script != null)
+  if (script != null) {
+    final String fullScriptPath = path.join(workingDirectory, script);
+    if (!FileSystemEntity.isFileSync(fullScriptPath)) {
+      print('Could not find test: $fullScriptPath');
+      print('Working directory: $workingDirectory');
+      print('Script: $script');
+      if (!printOutput)
+        print('This is one of the tests that does not normally print output.');
+      if (skip)
+        print('This is one of the tests that is normally skipped in this configuration.');
+      exit(1);
+    }
     args.add(script);
+  }
   return _runCommand(flutter, args,
     workingDirectory: workingDirectory,
     expectFailure: expectFailure,

packages/flutter_test/lib/src/matchers.dart

@@ -1248,7 +1248,10 @@ class _MatchesGoldenFile extends AsyncMatcher {
     final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized();
     return binding.runAsync<String>(() async {
       final ui.Image image = await imageFuture;
-      final ByteData bytes = await image.toByteData(format: ui.ImageByteFormat.png);
+      final ByteData bytes = await image.toByteData(format: ui.ImageByteFormat.png)
+        .timeout(const Duration(seconds: 10), onTimeout: () => null);
+      if (bytes == null)
+        return 'Failed to generate screenshot from engine within the 10,000ms timeout.';
       if (autoUpdateGoldenFiles) {
         await goldenFileComparator.update(key, bytes.buffer.asUint8List());
       } else {
@@ -1259,7 +1262,7 @@ class _MatchesGoldenFile extends AsyncMatcher {
           return ex.message;
         }
       }
-    });
+    }, additionalTime: const Duration(seconds: 11));
   }
 
   @override

packages/flutter_test/lib/src/widget_tester.dart

@@ -317,7 +317,9 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker
   /// are required to wait for the returned future to complete before calling
   /// this method again. Attempts to do otherwise will result in a
   /// [TestFailure] error being thrown.
-  Future<T> runAsync<T>(Future<T> callback()) => binding.runAsync(callback);
+  Future<T> runAsync<T>(Future<T> callback(), {
+    Duration additionalTime = const Duration(milliseconds: 250),
+  }) => binding.runAsync(callback, additionalTime: additionalTime);
 
   /// Whether there are any any transient callbacks scheduled.
   ///