Do not abort at first error when tests fail. (#108936)

dev/bots/run_command.dart

@@ -149,8 +149,8 @@ Future<Command> startCommand(String executable, List<String> arguments, {
 
 /// Runs the `executable` and waits until the process exits.
 ///
-/// If the process exits with a non-zero exit code, exits this process with
-/// exit code 1, unless `expectNonZeroExit` is set to true.
+/// If the process exits with a non-zero exit code and `expectNonZeroExit` is
+/// false, calls foundError (which does not terminate execution!).
 ///
 /// `outputListener` is called for every line of standard output from the
 /// process, and is given the [Process] object. This can be used to interrupt
@@ -195,7 +195,7 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, {
         io.stdout.writeln(result.flattenedStderr);
         break;
     }
-    exitWithError(<String>[
+    foundError(<String>[
       if (failureMessage != null)
         failureMessage
       else
@@ -203,8 +203,9 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, {
       '${bold}Command: $green$commandDescription$reset',
       '${bold}Relative working directory: $cyan$relativeWorkingDir$reset',
     ]);
-  }
+  } else {
     print('$clock ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset');
+  }
   return result;
 }
 

dev/bots/service_worker_test.dart

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
-import 'dart:io';
+import 'dart:core' hide print;
+import 'dart:io' hide exit;
 
 import 'package:path/path.dart' as path;
 import 'package:shelf/shelf.dart';
@@ -43,6 +43,10 @@ Future<void> main() async {
   await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJsShort);
   await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJsEntrypointLoadedEvent);
   await runWebServiceWorkerTestWithBlockedServiceWorkers(headless: false);
+  if (hasError) {
+    print('One or more tests failed.');
+    reportErrorsAndExit();
+  }
 }
 
 Future<void> _setAppVersion(int version) async {

dev/bots/test.dart

@@ -4,7 +4,8 @@
 
 import 'dart:async';
 import 'dart:convert';
-import 'dart:io';
+import 'dart:core' hide print;
+import 'dart:io' hide exit;
 import 'dart:math' as math;
 import 'dart:typed_data';
 
@@ -170,7 +171,6 @@ String get shuffleSeed {
 /// bin/cache/dart-sdk/bin/dart dev/bots/test.dart --local-engine=host_debug_unopt
 Future<void> main(List<String> args) async {
   print('$clock STARTING ANALYSIS');
-  try {
   flutterTestArgs.addAll(args);
   final Set<String> removeArgs = <String>{};
   for (final String arg in args) {
@@ -214,8 +214,9 @@ Future<void> main(List<String> args) async {
     'skp_generator': _runSkpGeneratorTests,
     kTestHarnessShardName: _runTestHarnessTests, // Used for testing this script.
   });
-  } on ExitException catch (error) {
-    error.apply();
+  if (hasError) {
+    print('$clock ${bold}Test failed.$reset');
+    reportErrorsAndExit();
   }
   print('$clock ${bold}Test successful.$reset');
 }
@@ -241,9 +242,7 @@ Future<void> _validateEngineHash() async {
     return line.startsWith('Flutter Engine Version:');
   });
   if (!actualVersion.contains(expectedVersion)) {
-    print('${red}Expected "Flutter Engine Version: $expectedVersion", '
-          'but found "$actualVersion".');
-    exit(1);
+    foundError(<String>['${red}Expected "Flutter Engine Version: $expectedVersion", but found "$actualVersion".']);
   }
 }
 
@@ -332,7 +331,7 @@ Future<void> _runTestHarnessTests() async {
   // Verify that we correctly generated the version file.
   final String? versionError = await verifyVersion(File(path.join(flutterRoot, 'version')));
   if (versionError != null) {
-    exitWithError(<String>[versionError]);
+    foundError(<String>[versionError]);
   }
 }
 
@@ -652,9 +651,10 @@ Future<void> _flutterBuild(
     );
     final File file = File(path.join(flutterRoot, relativePathToApplication, 'perf.json'));
     if (!_allTargetsCached(file)) {
-      print('${red}Not all build targets cached after second run.$reset');
-      print('The target performance data was: ${file.readAsStringSync().replaceAll('},', '},\n')}');
-      exit(1);
+      foundError(<String>[
+        '${red}Not all build targets cached after second run.$reset',
+        'The target performance data was: ${file.readAsStringSync().replaceAll('},', '},\n')}',
+      ]);
     }
   }
 }
@@ -840,8 +840,7 @@ Future<void> _runFrameworkTests() async {
       },
     ));
     if (results.isNotEmpty) {
-      print(results.join('\n'));
-      exit(1);
+      foundError(results);
     }
   }
 
@@ -941,20 +940,24 @@ Future<void> _runFrameworkTests() async {
 Future<void> _runFrameworkCoverage() async {
   final File coverageFile = File(path.join(flutterRoot, 'packages', 'flutter', 'coverage', 'lcov.info'));
   if (!coverageFile.existsSync()) {
-    print('${red}Coverage file not found.$reset');
-    print('Expected to find: $cyan${coverageFile.absolute}$reset');
-    print('This file is normally obtained by running `${green}flutter update-packages$reset`.');
-    exit(1);
+    foundError(<String>[
+      '${red}Coverage file not found.$reset',
+      'Expected to find: $cyan${coverageFile.absolute}$reset',
+      'This file is normally obtained by running `${green}flutter update-packages$reset`.',
+    ]);
+    return;
   }
   coverageFile.deleteSync();
   await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter'),
     options: const <String>['--coverage'],
   );
   if (!coverageFile.existsSync()) {
-    print('${red}Coverage file not found.$reset');
-    print('Expected to find: $cyan${coverageFile.absolute}$reset');
-    print('This file should have been generated by the `${green}flutter test --coverage$reset` script, but was not.');
-    exit(1);
+    foundError(<String>[
+      '${red}Coverage file not found.$reset',
+      'Expected to find: $cyan${coverageFile.absolute}$reset',
+      'This file should have been generated by the `${green}flutter test --coverage$reset` script, but was not.',
+    ]);
+    return;
   }
 }
 
@@ -1496,12 +1499,11 @@ Future<void> _runWebStackTraceTest(String buildMode, String entrypoint) async {
     browserDebugPort: browserDebugPort,
   );
 
-  if (result.contains('--- TEST SUCCEEDED ---')) {
-    print('${green}Web stack trace integration test passed.$reset');
-  } else {
-    print(result);
-    print('${red}Web stack trace integration test failed.$reset');
-    exit(1);
+  if (!result.contains('--- TEST SUCCEEDED ---')) {
+    foundError(<String>[
+      result,
+      '${red}Web stack trace integration test failed.$reset',
+    ]);
   }
 }
 
@@ -1545,12 +1547,11 @@ Future<void> _runWebReleaseTest(String target, {
     browserDebugPort: browserDebugPort,
   );
 
-  if (result.contains('--- TEST SUCCEEDED ---')) {
-    print('${green}Web release mode test passed.$reset');
-  } else {
-    print(result);
-    print('${red}Web release mode test failed.$reset');
-    exit(1);
+  if (!result.contains('--- TEST SUCCEEDED ---')) {
+    foundError(<String>[
+      result,
+      '${red}Web release mode test failed.$reset',
+    ]);
   }
 }
 
@@ -1599,13 +1600,12 @@ Future<void> _runWebDebugTest(String target, {
     environment: environment,
   );
 
-  if (success) {
-    print('${green}Web stack trace integration test passed.$reset');
-  } else {
-    print(result.flattenedStdout!);
-    print(result.flattenedStderr!);
-    print('${red}Web stack trace integration test failed.$reset');
-    exit(1);
+  if (!success) {
+    foundError(<String>[
+      result.flattenedStdout!,
+      result.flattenedStderr!,
+      '${red}Web stack trace integration test failed.$reset',
+    ]);
   }
 }
 
@@ -1652,9 +1652,11 @@ Future<void> _dartRunTest(String workingDirectory, {
   if (cpuVariable != null) {
     cpus = int.tryParse(cpuVariable, radix: 10);
     if (cpus == null) {
-      print('${red}The CPU environment variable, if set, must be set to the integer number of available cores.$reset');
-      print('Actual value: "$cpuVariable"');
-      exit(1);
+      foundError(<String>[
+        '${red}The CPU environment variable, if set, must be set to the integer number of available cores.$reset',
+        'Actual value: "$cpuVariable"',
+      ]);
+      return;
     }
   } else {
     cpus = 2; // Don't default to 1, otherwise we won't catch race conditions.
@@ -1762,13 +1764,14 @@ Future<void> _runFlutterTest(String workingDirectory, {
   if (script != null) {
     final String fullScriptPath = path.join(workingDirectory, script);
     if (!FileSystemEntity.isFileSync(fullScriptPath)) {
-      print('${red}Could not find test$reset: $green$fullScriptPath$reset');
-      print('Working directory: $cyan$workingDirectory$reset');
-      print('Script: $green$script$reset');
-      if (!printOutput) {
-        print('This is one of the tests that does not normally print output.');
-      }
-      exit(1);
+      foundError(<String>[
+        '${red}Could not find test$reset: $green$fullScriptPath$reset',
+        'Working directory: $cyan$workingDirectory$reset',
+        'Script: $green$script$reset',
+        if (!printOutput)
+          'This is one of the tests that does not normally print output.',
+      ]);
+      return;
     }
     args.add(script);
   }
@@ -1792,7 +1795,7 @@ Future<void> _runFlutterTest(String workingDirectory, {
     if (outputChecker != null) {
       final String? message = outputChecker(result);
       if (message != null) {
-        exitWithError(<String>[message]);
+        foundError(<String>[message]);
       }
     }
     return;
@@ -1906,15 +1909,19 @@ List<T> _selectIndexOfTotalSubshard<T>(List<T> tests, {String subshardKey = kSub
   final RegExp pattern = RegExp(r'^(\d+)_(\d+)$');
   final Match? match = pattern.firstMatch(subshardName);
   if (match == null || match.groupCount != 2) {
-    print('${red}Invalid subshard name "$subshardName". Expected format "[int]_[int]" ex. "1_3"');
-    exit(1);
+    foundError(<String>[
+      '${red}Invalid subshard name "$subshardName". Expected format "[int]_[int]" ex. "1_3"',
+    ]);
+    return <T>[];
   }
   // One-indexed.
-  final int index = int.parse(match!.group(1)!);
+  final int index = int.parse(match.group(1)!);
   final int total = int.parse(match.group(2)!);
   if (index > total) {
-    print('${red}Invalid subshard name "$subshardName". Index number must be greater or equal to total.');
-    exit(1);
+    foundError(<String>[
+      '${red}Invalid subshard name "$subshardName". Index number must be greater or equal to total.',
+    ]);
+    return <T>[];
   }
 
   final int testsPerShard = (tests.length / total).ceil();
@@ -1965,9 +1972,11 @@ Future<void> _runFromList(Map<String, ShardRunner> items, String key, String nam
     }
   } else {
     if (!items.containsKey(item)) {
-      print('${red}Invalid $name: $item$reset');
-      print('The available ${name}s are: ${items.keys.join(", ")}');
-      exit(1);
+      foundError(<String>[
+        '${red}Invalid $name: $item$reset',
+        'The available ${name}s are: ${items.keys.join(", ")}',
+      ]);
+      return;
     }
     print('$bold$key=$item$reset');
     await items[item]!();

dev/bots/test/analyze_test.dart

@@ -12,21 +12,22 @@ import 'common.dart';
 
 typedef AsyncVoidCallback = Future<void> Function();
 
-Future<String> capture(AsyncVoidCallback callback, { int exitCode = 0 }) async {
+Future<String> capture(AsyncVoidCallback callback, { bool shouldHaveErrors = false }) async {
   final StringBuffer buffer = StringBuffer();
   final PrintCallback oldPrint = print;
   try {
     print = (Object line) {
       buffer.writeln(line);
     };
-    try {
     await callback();
-      expect(exitCode, 0);
-    } on ExitException catch (error) {
-      expect(error.exitCode, exitCode);
-    }
+    expect(
+      hasError,
+      shouldHaveErrors,
+      reason: buffer.isEmpty ? '(No output to report.)' : hasError ? 'Unexpected errors:\n$buffer' : 'Unexpected success:\n$buffer',
+    );
   } finally {
     print = oldPrint;
+    resetErrorStatus();
   }
   if (stdout.supportsAnsiEscapes) {
     // Remove ANSI escapes when this test is running on a terminal.
@@ -42,7 +43,7 @@ void main() {
   final String dartPath = path.canonicalize(path.join('..', '..', 'bin', 'cache', 'dart-sdk', 'bin', dartName));
 
   test('analyze.dart - verifyDeprecations', () async {
-    final String result = await capture(() => verifyDeprecations(testRootPath, minimumMatches: 2), exitCode: 1);
+    final String result = await capture(() => verifyDeprecations(testRootPath, minimumMatches: 2), shouldHaveErrors: true);
     final String lines = <String>[
         'test/analyze-test-input/root/packages/foo/deprecation.dart:12: Deprecation notice does not match required pattern.',
         'test/analyze-test-input/root/packages/foo/deprecation.dart:18: Deprecation notice should be a grammatically correct sentence and start with a capital letter; see style guide: STYLE_GUIDE_URL',
@@ -73,7 +74,7 @@ void main() {
   });
 
   test('analyze.dart - verifyGoldenTags', () async {
-    final String result = await capture(() => verifyGoldenTags(testRootPath, minimumMatches: 6), exitCode: 1);
+    final String result = await capture(() => verifyGoldenTags(testRootPath, minimumMatches: 6), shouldHaveErrors: true);
     const String noTag = "Files containing golden tests must be tagged using @Tags(<String>['reduced-test-set']) "
         'at the top of the file before import statements.';
     const String missingTag = "Files containing golden tests must be tagged with 'reduced-test-set'.";
@@ -117,7 +118,7 @@ void main() {
   });
 
   test('analyze.dart - verifyNoMissingLicense', () async {
-    final String result = await capture(() => verifyNoMissingLicense(testRootPath, checkMinimums: false), exitCode: 1);
+    final String result = await capture(() => verifyNoMissingLicense(testRootPath, checkMinimums: false), shouldHaveErrors: true);
     final String file = 'test/analyze-test-input/root/packages/foo/foo.dart'
       .replaceAll('/', Platform.isWindows ? r'\' : '/');
     expect(result,
@@ -137,7 +138,7 @@ void main() {
   });
 
   test('analyze.dart - verifyNoTrailingSpaces', () async {
-    final String result = await capture(() => verifyNoTrailingSpaces(testRootPath, minimumMatches: 2), exitCode: 1);
+    final String result = await capture(() => verifyNoTrailingSpaces(testRootPath, minimumMatches: 2), shouldHaveErrors: true);
     final String lines = <String>[
         'test/analyze-test-input/root/packages/foo/spaces.txt:5: trailing U+0020 space character',
         'test/analyze-test-input/root/packages/foo/spaces.txt:9: trailing blank line',
@@ -155,7 +156,7 @@ void main() {
     final String result = await capture(() => verifyNoBinaries(
       testRootPath,
       legacyBinaries: <Hash256>{const Hash256(0x39A050CD69434936, 0, 0, 0)},
-    ), exitCode: Platform.isWindows ? 0 : 1);
+    ), shouldHaveErrors: !Platform.isWindows);
     if (!Platform.isWindows) {
       expect(result,
         '━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n'
@@ -173,7 +174,7 @@ void main() {
   });
 
   test('analyze.dart - verifyInternationalizations - comparison fails', () async {
-    final String result = await capture(() => verifyInternationalizations(testRootPath, dartPath), exitCode: 1);
+    final String result = await capture(() => verifyInternationalizations(testRootPath, dartPath), shouldHaveErrors: true);
     final String genLocalizationsScript = path.join('dev', 'tools', 'localization', 'bin', 'gen_localizations.dart');
     expect(result,
         contains('$dartName $genLocalizationsScript --cupertino'));
@@ -201,7 +202,7 @@ void main() {
     final String result = await capture(() => verifyNullInitializedDebugExpensiveFields(
       testRootPath,
       minimumMatches: 1,
-    ), exitCode: 1);
+    ), shouldHaveErrors: true);
 
     expect(result, contains('L15'));
     expect(result, isNot(contains('L12')));

dev/bots/utils.dart

@@ -4,9 +4,11 @@
 
 import 'dart:core' as core_internals show print;
 import 'dart:core' hide print;
-import 'dart:io' as io_internals show exit;
+import 'dart:io' as system show exit;
 import 'dart:io' hide exit;
 
+import 'package:meta/meta.dart';
+
 final bool hasColor = stdout.supportsAnsiEscapes;
 
 final String bold = hasColor ? '\x1B[1m' : ''; // used for shard titles
@@ -16,36 +18,41 @@ final String yellow = hasColor ? '\x1B[33m' : ''; // used for skips
 final String cyan = hasColor ? '\x1B[36m' : ''; // used for paths
 final String reverse = hasColor ? '\x1B[7m' : ''; // used for clocks
 final String reset = hasColor ? '\x1B[0m' : '';
+final String redLine = '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset';
 
-class ExitException implements Exception {
-  ExitException(this.exitCode);
+typedef PrintCallback = void Function(Object line);
 
-  final int exitCode;
+// Allow print() to be overridden, for tests.
+PrintCallback print = core_internals.print;
 
-  void apply() {
-    io_internals.exit(exitCode);
-  }
-}
+bool get hasError => _hasError;
+bool _hasError = false;
 
-// We actually reimplement exit() so that it uses exceptions rather
-// than truly immediately terminating the application, so that we can
-// test the exit code in unit tests (see test/analyze_test.dart).
-void exit(int exitCode) {
-  throw ExitException(exitCode);
-}
+Iterable<String> get errorMessages => _errorMessages;
+List<String> _errorMessages = <String>[];
 
-void exitWithError(List<String> messages) {
-  final String redLine = '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset';
+void foundError(List<String> messages) {
+  assert(messages.isNotEmpty);
   print(redLine);
   messages.forEach(print);
   print(redLine);
-  exit(1);
+  _errorMessages.addAll(messages);
+  _hasError = true;
 }
 
-typedef PrintCallback = void Function(Object line);
+@visibleForTesting
+void resetErrorStatus() {
+  _hasError = false;
+  _errorMessages.clear();
+}
 
-// Allow print() to be overridden, for tests.
-PrintCallback print = core_internals.print;
+Never reportErrorsAndExit() {
+  print(redLine);
+  print('For your convenience, the error messages reported above are repeated here:');
+  _errorMessages.forEach(print);
+  print(redLine);
+  system.exit(1);
+}
 
 String get clock {
   final DateTime now = DateTime.now();