diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index 9d66ecf742..a799ed26ca 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -45,10 +45,10 @@ Future main(List arguments) async { dart = path.join(dartSdk, 'bin', Platform.isWindows ? 'dart.exe' : 'dart'); pub = path.join(dartSdk, 'bin', Platform.isWindows ? 'pub.bat' : 'pub'); print('$clock STARTING ANALYSIS'); - try { - await run(arguments); - } on ExitException catch (error) { - error.apply(); + await run(arguments); + if (hasError) { + print('$clock ${bold}Test failed.$reset'); + reportErrorsAndExit(); } print('$clock ${bold}Analysis successful.$reset'); } @@ -60,17 +60,20 @@ String? _getDartSdkFromArguments(List arguments) { for (int i = 0; i < arguments.length; i += 1) { if (arguments[i] == '--dart-sdk') { if (result != null) { - exitWithError(['The --dart-sdk argument must not be used more than once.']); + foundError(['The --dart-sdk argument must not be used more than once.']); + return null; } if (i + 1 < arguments.length) { result = arguments[i + 1]; } else { - exitWithError(['--dart-sdk must be followed by a path.']); + foundError(['--dart-sdk must be followed by a path.']); + return null; } } if (arguments[i].startsWith('--dart-sdk=')) { if (result != null) { - exitWithError(['The --dart-sdk argument must not be used more than once.']); + foundError(['The --dart-sdk argument must not be used more than once.']); + return null; } result = arguments[i].substring('--dart-sdk='.length); } @@ -82,7 +85,7 @@ Future run(List arguments) async { bool assertsEnabled = false; assert(() { assertsEnabled = true; return true; }()); if (!assertsEnabled) { - exitWithError(['The analyze.dart script must be run with --enable-asserts.']); + foundError(['The analyze.dart script must be run with --enable-asserts.']); } print('$clock No Double.clamp'); @@ -268,7 +271,7 @@ Future verifyNoDoubleClamp(String workingDirectory) async { } } if (errors.isNotEmpty) { - exitWithError([ + foundError([ ...errors, '\n${bold}See: https://github.com/flutter/flutter/pull/103559', ]); @@ -315,7 +318,7 @@ Future verifyToolTestsEndInTestDart(String workingDirectory) async { violations.add(file.path); } if (violations.isNotEmpty) { - exitWithError([ + foundError([ '${bold}Found flutter_tools tests that do not end in `_test.dart`; these will not be run by the test runner$reset', ...violations, ]); @@ -354,7 +357,7 @@ Future verifyNoSyncAsyncStar(String workingDirectory, {int minimumMatches } } if (errors.isNotEmpty) { - exitWithError([ + foundError([ '${bold}Do not use sync*/async* methods. See https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-syncasync for details.$reset', ...errors, ]); @@ -420,7 +423,7 @@ Future verifyGoldenTags(String workingDirectory, { int minimumMatches = 20 } } if (errors.isNotEmpty) { - exitWithError([ + foundError([ ...errors, '${bold}See: https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter$reset', ]); @@ -529,7 +532,7 @@ Future verifyDeprecations(String workingDirectory, { int minimumMatches = } // Fail if any errors if (errors.isNotEmpty) { - exitWithError([ + foundError([ ...errors, '${bold}See: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes$reset', ]); @@ -561,7 +564,7 @@ Future verifyNoMissingLicense(String workingDirectory, { bool checkMinimum failed += await _verifyNoMissingLicenseForExtension(workingDirectory, 'xml', overrideMinimumMatches ?? 1, '', header: r'(<\?xml version="1.0" encoding="utf-8"\?>\n)?'); failed += await _verifyNoMissingLicenseForExtension(workingDirectory, 'frag', overrideMinimumMatches ?? 1, _generateLicense('// '), header: r'#version 320 es(\n)+'); if (failed > 0) { - exitWithError(['License check failed.']); + foundError(['License check failed.']); } } @@ -670,7 +673,7 @@ Future verifySkipTestComments(String workingDirectory) async { // Fail if any errors if (errors.isNotEmpty) { - exitWithError([ + foundError([ ...errors, '\n${bold}See: https://github.com/flutter/flutter/wiki/Tree-hygiene#skipped-tests$reset', ]); @@ -700,7 +703,7 @@ Future verifyNoTestImports(String workingDirectory) async { // Fail if any errors if (errors.isNotEmpty) { final String s = errors.length == 1 ? '' : 's'; - exitWithError([ + foundError([ '${bold}The following file$s import a test directly. Test utilities should be in their own file.$reset', ...errors, ]); @@ -769,7 +772,7 @@ Future verifyNoBadImportsInFlutter(String workingDirectory) async { } // Fail if any errors if (errors.isNotEmpty) { - exitWithError([ + foundError([ if (errors.length == 1) '${bold}An error was detected when looking at import dependencies within the Flutter package:$reset' else @@ -789,7 +792,7 @@ Future verifyNoBadImportsInFlutterTools(String workingDirectory) async { } // Fail if any errors if (errors.isNotEmpty) { - exitWithError([ + foundError([ if (errors.length == 1) '${bold}An error was detected when looking at import dependencies within the flutter_tools package:$reset' else @@ -814,7 +817,7 @@ Future verifyIntegrationTestTimeouts(String workingDirectory) async { } } if (errors.isNotEmpty) { - exitWithError([ + foundError([ if (errors.length == 1) '${bold}An error was detected when looking at integration test timeouts:$reset' else @@ -850,7 +853,7 @@ Future verifyInternationalizations(String workingDirectory, String dartExe final String expectedCupertinoResult = await File(cupertinoLocalizationsFile).readAsString(); if (materialGenResult.stdout.trim() != expectedMaterialResult.trim()) { - exitWithError([ + foundError([ '<<<<<<< $materialLocalizationsFile', expectedMaterialResult.trim(), '=======', @@ -862,7 +865,7 @@ Future verifyInternationalizations(String workingDirectory, String dartExe ]); } if (cupertinoGenResult.stdout.trim() != expectedCupertinoResult.trim()) { - exitWithError([ + foundError([ '<<<<<<< $cupertinoLocalizationsFile', expectedCupertinoResult.trim(), '=======', @@ -893,7 +896,7 @@ Future verifyNoCheckedMode(String workingDirectory) async { } } if (problems.isNotEmpty) { - exitWithError(problems); + foundError(problems); } } @@ -947,7 +950,7 @@ Future verifyNoRuntimeTypeInToString(String workingDirectory) async { } } if (problems.isNotEmpty) { - exitWithError(problems); + foundError(problems); } } @@ -977,7 +980,7 @@ Future verifyNoTrailingSpaces(String workingDirectory, { int minimumMatche } } if (problems.isNotEmpty) { - exitWithError(problems); + foundError(problems); } } @@ -1050,7 +1053,7 @@ Future verifyIssueLinks(String workingDirectory) async { } assert(problems.isEmpty == suggestions.isEmpty); if (problems.isNotEmpty) { - exitWithError([ + foundError([ ...problems, ...suggestions, ]); @@ -1507,7 +1510,7 @@ Future verifyNoBinaries(String workingDirectory, { Set? legacyBin } } if (problems.isNotEmpty) { - exitWithError([ + foundError([ ...problems, 'All files in this repository must be UTF-8. In particular, images and other binaries', 'must not be checked into this repository. This is because we are very sensitive to the', @@ -1545,7 +1548,7 @@ Future> _gitFiles(String workingDirectory, {bool runSilently = true}) runSilently: runSilently, ); if (evalResult.exitCode != 0) { - exitWithError([ + foundError([ 'git ls-files failed with exit code ${evalResult.exitCode}', '${bold}stdout:$reset', evalResult.stdout, @@ -1671,7 +1674,7 @@ Future _evalCommand(String executable, List arguments, { if (exitCode != 0 && !allowNonZeroExit) { stderr.write(result.stderr); - exitWithError([ + foundError([ '${bold}ERROR:$red Last command exited with $exitCode.$reset', '${bold}Command:$red $commandDescription$reset', '${bold}Relative working directory:$red $relativeWorkingDir$reset', @@ -1702,9 +1705,11 @@ Future _checkConsumerDependencies() async { '--directory=${path.join(flutterRoot, 'packages', package)}', ]); if (result.exitCode != 0) { - print(result.stdout as Object); - print(result.stderr as Object); - exit(result.exitCode); + foundError([ + result.stdout.toString(), + result.stderr.toString(), + ]); + return; } final Map rawJson = json.decode(result.stdout as String) as Map; final Map> dependencyTree = >{ @@ -1734,7 +1739,7 @@ Future _checkConsumerDependencies() async { String plural(int n, String s, String p) => n == 1 ? s : p; if (added.isNotEmpty) { - exitWithError([ + foundError([ 'The transitive closure of package dependencies contains ${plural(added.length, "a non-allowlisted package", "non-allowlisted packages")}:', ' ${added.join(', ')}', 'We strongly desire to keep the number of dependencies to a minimum and', @@ -1745,7 +1750,7 @@ Future _checkConsumerDependencies() async { } if (removed.isNotEmpty) { - exitWithError([ + foundError([ 'Excellent news! ${plural(removed.length, "A package dependency has been removed!", "Multiple package dependencies have been removed!")}', ' ${removed.join(', ')}', 'To make sure we do not accidentally add ${plural(removed.length, "this dependency", "these dependencies")} back in the future,', @@ -1778,7 +1783,7 @@ Future verifyNullInitializedDebugExpensiveFields(String workingDirectory, } if (errors.isNotEmpty) { - exitWithError([ + foundError([ '${bold}ERROR: ${red}fields annotated with @_debugOnly must null initialize.$reset', 'to ensure both the field and initializer are removed from profile/release mode.', 'These fields should be written as:\n', diff --git a/dev/bots/run_command.dart b/dev/bots/run_command.dart index f443198958..0f51658cf9 100644 --- a/dev/bots/run_command.dart +++ b/dev/bots/run_command.dart @@ -149,8 +149,8 @@ Future startCommand(String executable, List 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 runCommand(String executable, List arguments, { io.stdout.writeln(result.flattenedStderr); break; } - exitWithError([ + foundError([ if (failureMessage != null) failureMessage else @@ -203,8 +203,9 @@ Future runCommand(String executable, List 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'); } - print('$clock ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); return result; } diff --git a/dev/bots/service_worker_test.dart b/dev/bots/service_worker_test.dart index ce148d2823..698847778d 100644 --- a/dev/bots/service_worker_test.dart +++ b/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 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 _setAppVersion(int version) async { diff --git a/dev/bots/test.dart b/dev/bots/test.dart index b8913fc38e..0240f89457 100644 --- a/dev/bots/test.dart +++ b/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,52 +171,52 @@ String get shuffleSeed { /// bin/cache/dart-sdk/bin/dart dev/bots/test.dart --local-engine=host_debug_unopt Future main(List args) async { print('$clock STARTING ANALYSIS'); - try { - flutterTestArgs.addAll(args); - final Set removeArgs = {}; - for (final String arg in args) { - if (arg.startsWith('--local-engine=')) { - localEngineEnv['FLUTTER_LOCAL_ENGINE'] = arg.substring('--local-engine='.length); - } - if (arg.startsWith('--local-engine-src-path=')) { - localEngineEnv['FLUTTER_LOCAL_ENGINE_SRC_PATH'] = arg.substring('--local-engine-src-path='.length); - } - if (arg.startsWith('--test-randomize-ordering-seed=')) { - _shuffleSeed = arg.substring('--test-randomize-ordering-seed='.length); - removeArgs.add(arg); - } - if (arg == '--no-smoke-tests') { - // This flag is deprecated, ignore it. - removeArgs.add(arg); - } + flutterTestArgs.addAll(args); + final Set removeArgs = {}; + for (final String arg in args) { + if (arg.startsWith('--local-engine=')) { + localEngineEnv['FLUTTER_LOCAL_ENGINE'] = arg.substring('--local-engine='.length); } - flutterTestArgs.removeWhere((String arg) => removeArgs.contains(arg)); - if (Platform.environment.containsKey(CIRRUS_TASK_NAME)) { - print('Running task: ${Platform.environment[CIRRUS_TASK_NAME]}'); + if (arg.startsWith('--local-engine-src-path=')) { + localEngineEnv['FLUTTER_LOCAL_ENGINE_SRC_PATH'] = arg.substring('--local-engine-src-path='.length); } - print('═' * 80); - await selectShard({ - 'add_to_app_life_cycle_tests': _runAddToAppLifeCycleTests, - 'build_tests': _runBuildTests, - 'framework_coverage': _runFrameworkCoverage, - 'framework_tests': _runFrameworkTests, - 'tool_tests': _runToolTests, - // web_tool_tests is also used by HHH: https://dart.googlesource.com/recipes/+/refs/heads/master/recipes/dart/flutter_engine.py - 'web_tool_tests': _runWebToolTests, - 'tool_integration_tests': _runIntegrationToolTests, - 'tool_host_cross_arch_tests': _runToolHostCrossArchTests, - // All the unit/widget tests run using `flutter test --platform=chrome --web-renderer=html` - 'web_tests': _runWebHtmlUnitTests, - // All the unit/widget tests run using `flutter test --platform=chrome --web-renderer=canvaskit` - 'web_canvaskit_tests': _runWebCanvasKitUnitTests, - // All web integration tests - 'web_long_running_tests': _runWebLongRunningTests, - 'flutter_plugins': _runFlutterPluginsTests, - 'skp_generator': _runSkpGeneratorTests, - kTestHarnessShardName: _runTestHarnessTests, // Used for testing this script. - }); - } on ExitException catch (error) { - error.apply(); + if (arg.startsWith('--test-randomize-ordering-seed=')) { + _shuffleSeed = arg.substring('--test-randomize-ordering-seed='.length); + removeArgs.add(arg); + } + if (arg == '--no-smoke-tests') { + // This flag is deprecated, ignore it. + removeArgs.add(arg); + } + } + flutterTestArgs.removeWhere((String arg) => removeArgs.contains(arg)); + if (Platform.environment.containsKey(CIRRUS_TASK_NAME)) { + print('Running task: ${Platform.environment[CIRRUS_TASK_NAME]}'); + } + print('═' * 80); + await selectShard({ + 'add_to_app_life_cycle_tests': _runAddToAppLifeCycleTests, + 'build_tests': _runBuildTests, + 'framework_coverage': _runFrameworkCoverage, + 'framework_tests': _runFrameworkTests, + 'tool_tests': _runToolTests, + // web_tool_tests is also used by HHH: https://dart.googlesource.com/recipes/+/refs/heads/master/recipes/dart/flutter_engine.py + 'web_tool_tests': _runWebToolTests, + 'tool_integration_tests': _runIntegrationToolTests, + 'tool_host_cross_arch_tests': _runToolHostCrossArchTests, + // All the unit/widget tests run using `flutter test --platform=chrome --web-renderer=html` + 'web_tests': _runWebHtmlUnitTests, + // All the unit/widget tests run using `flutter test --platform=chrome --web-renderer=canvaskit` + 'web_canvaskit_tests': _runWebCanvasKitUnitTests, + // All web integration tests + 'web_long_running_tests': _runWebLongRunningTests, + 'flutter_plugins': _runFlutterPluginsTests, + 'skp_generator': _runSkpGeneratorTests, + kTestHarnessShardName: _runTestHarnessTests, // Used for testing this script. + }); + if (hasError) { + print('$clock ${bold}Test failed.$reset'); + reportErrorsAndExit(); } print('$clock ${bold}Test successful.$reset'); } @@ -241,9 +242,7 @@ Future _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(['${red}Expected "Flutter Engine Version: $expectedVersion", but found "$actualVersion".']); } } @@ -332,7 +331,7 @@ Future _runTestHarnessTests() async { // Verify that we correctly generated the version file. final String? versionError = await verifyVersion(File(path.join(flutterRoot, 'version'))); if (versionError != null) { - exitWithError([versionError]); + foundError([versionError]); } } @@ -652,9 +651,10 @@ Future _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([ + '${red}Not all build targets cached after second run.$reset', + 'The target performance data was: ${file.readAsStringSync().replaceAll('},', '},\n')}', + ]); } } } @@ -840,8 +840,7 @@ Future _runFrameworkTests() async { }, )); if (results.isNotEmpty) { - print(results.join('\n')); - exit(1); + foundError(results); } } @@ -941,20 +940,24 @@ Future _runFrameworkTests() async { Future _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([ + '${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 ['--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([ + '${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 _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([ + result, + '${red}Web stack trace integration test failed.$reset', + ]); } } @@ -1545,12 +1547,11 @@ Future _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([ + result, + '${red}Web release mode test failed.$reset', + ]); } } @@ -1599,13 +1600,12 @@ Future _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([ + result.flattenedStdout!, + result.flattenedStderr!, + '${red}Web stack trace integration test failed.$reset', + ]); } } @@ -1652,9 +1652,11 @@ Future _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([ + '${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 _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([ + '${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 _runFlutterTest(String workingDirectory, { if (outputChecker != null) { final String? message = outputChecker(result); if (message != null) { - exitWithError([message]); + foundError([message]); } } return; @@ -1906,15 +1909,19 @@ List _selectIndexOfTotalSubshard(List 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([ + '${red}Invalid subshard name "$subshardName". Expected format "[int]_[int]" ex. "1_3"', + ]); + return []; } // 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([ + '${red}Invalid subshard name "$subshardName". Index number must be greater or equal to total.', + ]); + return []; } final int testsPerShard = (tests.length / total).ceil(); @@ -1965,9 +1972,11 @@ Future _runFromList(Map 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([ + '${red}Invalid $name: $item$reset', + 'The available ${name}s are: ${items.keys.join(", ")}', + ]); + return; } print('$bold$key=$item$reset'); await items[item]!(); diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart index b5c747f575..603d9aae51 100644 --- a/dev/bots/test/analyze_test.dart +++ b/dev/bots/test/analyze_test.dart @@ -12,21 +12,22 @@ import 'common.dart'; typedef AsyncVoidCallback = Future Function(); -Future capture(AsyncVoidCallback callback, { int exitCode = 0 }) async { +Future 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); - } + await callback(); + 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 = [ '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(['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 = [ '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: {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'))); diff --git a/dev/bots/utils.dart b/dev/bots/utils.dart index e174431f14..9474ca0b30 100644 --- a/dev/bots/utils.dart +++ b/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,37 +18,42 @@ 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' : ''; - -class ExitException implements Exception { - ExitException(this.exitCode); - - final int exitCode; - - void apply() { - io_internals.exit(exitCode); - } -} - -// 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); -} - -void exitWithError(List messages) { - final String redLine = '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset'; - print(redLine); - messages.forEach(print); - print(redLine); - exit(1); -} +final String redLine = '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset'; typedef PrintCallback = void Function(Object line); // Allow print() to be overridden, for tests. PrintCallback print = core_internals.print; +bool get hasError => _hasError; +bool _hasError = false; + +Iterable get errorMessages => _errorMessages; +List _errorMessages = []; + +void foundError(List messages) { + assert(messages.isNotEmpty); + print(redLine); + messages.forEach(print); + print(redLine); + _errorMessages.addAll(messages); + _hasError = true; +} + +@visibleForTesting +void resetErrorStatus() { + _hasError = false; + _errorMessages.clear(); +} + +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(); return '$reverse▌'