Skip to content

Commit 03aa059

Browse files
HixieHixie
authored
Do not abort at first error when tests fail. (#108936)
1 parent 274a934 commit 03aa059

6 files changed

Lines changed: 201 additions & 174 deletions

File tree

‎dev/bots/analyze.dart‎

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ Future<void> main(List<String> arguments) async {
4545
dart = path.join(dartSdk, 'bin', Platform.isWindows ? 'dart.exe' : 'dart');
4646
pub = path.join(dartSdk, 'bin', Platform.isWindows ? 'pub.bat' : 'pub');
4747
print('$clock STARTING ANALYSIS');
48-
try {
49-
await run(arguments);
50-
} on ExitException catch (error) {
51-
error.apply();
48+
await run(arguments);
49+
if (hasError) {
50+
print('$clock ${bold}Test failed.$reset');
51+
reportErrorsAndExit();
5252
}
5353
print('$clock ${bold}Analysis successful.$reset');
5454
}
@@ -60,17 +60,20 @@ String? _getDartSdkFromArguments(List<String> arguments) {
6060
for (int i = 0; i < arguments.length; i += 1) {
6161
if (arguments[i] == '--dart-sdk') {
6262
if (result != null) {
63-
exitWithError(<String>['The --dart-sdk argument must not be used more than once.']);
63+
foundError(<String>['The --dart-sdk argument must not be used more than once.']);
64+
return null;
6465
}
6566
if (i + 1 < arguments.length) {
6667
result = arguments[i + 1];
6768
} else {
68-
exitWithError(<String>['--dart-sdk must be followed by a path.']);
69+
foundError(<String>['--dart-sdk must be followed by a path.']);
70+
return null;
6971
}
7072
}
7173
if (arguments[i].startsWith('--dart-sdk=')) {
7274
if (result != null) {
73-
exitWithError(<String>['The --dart-sdk argument must not be used more than once.']);
75+
foundError(<String>['The --dart-sdk argument must not be used more than once.']);
76+
return null;
7477
}
7578
result = arguments[i].substring('--dart-sdk='.length);
7679
}
@@ -82,7 +85,7 @@ Future<void> run(List<String> arguments) async {
8285
bool assertsEnabled = false;
8386
assert(() { assertsEnabled = true; return true; }());
8487
if (!assertsEnabled) {
85-
exitWithError(<String>['The analyze.dart script must be run with --enable-asserts.']);
88+
foundError(<String>['The analyze.dart script must be run with --enable-asserts.']);
8689
}
8790

8891
print('$clock No Double.clamp');
@@ -268,7 +271,7 @@ Future<void> verifyNoDoubleClamp(String workingDirectory) async {
268271
}
269272
}
270273
if (errors.isNotEmpty) {
271-
exitWithError(<String>[
274+
foundError(<String>[
272275
...errors,
273276
'\n${bold}See: https://github.com/flutter/flutter/pull/103559',
274277
]);
@@ -315,7 +318,7 @@ Future<void> verifyToolTestsEndInTestDart(String workingDirectory) async {
315318
violations.add(file.path);
316319
}
317320
if (violations.isNotEmpty) {
318-
exitWithError(<String>[
321+
foundError(<String>[
319322
'${bold}Found flutter_tools tests that do not end in `_test.dart`; these will not be run by the test runner$reset',
320323
...violations,
321324
]);
@@ -354,7 +357,7 @@ Future<void> verifyNoSyncAsyncStar(String workingDirectory, {int minimumMatches
354357
}
355358
}
356359
if (errors.isNotEmpty) {
357-
exitWithError(<String>[
360+
foundError(<String>[
358361
'${bold}Do not use sync*/async* methods. See https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-syncasync for details.$reset',
359362
...errors,
360363
]);
@@ -420,7 +423,7 @@ Future<void> verifyGoldenTags(String workingDirectory, { int minimumMatches = 20
420423
}
421424
}
422425
if (errors.isNotEmpty) {
423-
exitWithError(<String>[
426+
foundError(<String>[
424427
...errors,
425428
'${bold}See: https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter$reset',
426429
]);
@@ -529,7 +532,7 @@ Future<void> verifyDeprecations(String workingDirectory, { int minimumMatches =
529532
}
530533
// Fail if any errors
531534
if (errors.isNotEmpty) {
532-
exitWithError(<String>[
535+
foundError(<String>[
533536
...errors,
534537
'${bold}See: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes$reset',
535538
]);
@@ -561,7 +564,7 @@ Future<void> verifyNoMissingLicense(String workingDirectory, { bool checkMinimum
561564
failed += await _verifyNoMissingLicenseForExtension(workingDirectory, 'xml', overrideMinimumMatches ?? 1, '<!-- ${_generateLicense('')} -->', header: r'(<\?xml version="1.0" encoding="utf-8"\?>\n)?');
562565
failed += await _verifyNoMissingLicenseForExtension(workingDirectory, 'frag', overrideMinimumMatches ?? 1, _generateLicense('// '), header: r'#version 320 es(\n)+');
563566
if (failed > 0) {
564-
exitWithError(<String>['License check failed.']);
567+
foundError(<String>['License check failed.']);
565568
}
566569
}
567570

@@ -670,7 +673,7 @@ Future<void> verifySkipTestComments(String workingDirectory) async {
670673

671674
// Fail if any errors
672675
if (errors.isNotEmpty) {
673-
exitWithError(<String>[
676+
foundError(<String>[
674677
...errors,
675678
'\n${bold}See: https://github.com/flutter/flutter/wiki/Tree-hygiene#skipped-tests$reset',
676679
]);
@@ -700,7 +703,7 @@ Future<void> verifyNoTestImports(String workingDirectory) async {
700703
// Fail if any errors
701704
if (errors.isNotEmpty) {
702705
final String s = errors.length == 1 ? '' : 's';
703-
exitWithError(<String>[
706+
foundError(<String>[
704707
'${bold}The following file$s import a test directly. Test utilities should be in their own file.$reset',
705708
...errors,
706709
]);
@@ -769,7 +772,7 @@ Future<void> verifyNoBadImportsInFlutter(String workingDirectory) async {
769772
}
770773
// Fail if any errors
771774
if (errors.isNotEmpty) {
772-
exitWithError(<String>[
775+
foundError(<String>[
773776
if (errors.length == 1)
774777
'${bold}An error was detected when looking at import dependencies within the Flutter package:$reset'
775778
else
@@ -789,7 +792,7 @@ Future<void> verifyNoBadImportsInFlutterTools(String workingDirectory) async {
789792
}
790793
// Fail if any errors
791794
if (errors.isNotEmpty) {
792-
exitWithError(<String>[
795+
foundError(<String>[
793796
if (errors.length == 1)
794797
'${bold}An error was detected when looking at import dependencies within the flutter_tools package:$reset'
795798
else
@@ -814,7 +817,7 @@ Future<void> verifyIntegrationTestTimeouts(String workingDirectory) async {
814817
}
815818
}
816819
if (errors.isNotEmpty) {
817-
exitWithError(<String>[
820+
foundError(<String>[
818821
if (errors.length == 1)
819822
'${bold}An error was detected when looking at integration test timeouts:$reset'
820823
else
@@ -850,7 +853,7 @@ Future<void> verifyInternationalizations(String workingDirectory, String dartExe
850853
final String expectedCupertinoResult = await File(cupertinoLocalizationsFile).readAsString();
851854

852855
if (materialGenResult.stdout.trim() != expectedMaterialResult.trim()) {
853-
exitWithError(<String>[
856+
foundError(<String>[
854857
'<<<<<<< $materialLocalizationsFile',
855858
expectedMaterialResult.trim(),
856859
'=======',
@@ -862,7 +865,7 @@ Future<void> verifyInternationalizations(String workingDirectory, String dartExe
862865
]);
863866
}
864867
if (cupertinoGenResult.stdout.trim() != expectedCupertinoResult.trim()) {
865-
exitWithError(<String>[
868+
foundError(<String>[
866869
'<<<<<<< $cupertinoLocalizationsFile',
867870
expectedCupertinoResult.trim(),
868871
'=======',
@@ -893,7 +896,7 @@ Future<void> verifyNoCheckedMode(String workingDirectory) async {
893896
}
894897
}
895898
if (problems.isNotEmpty) {
896-
exitWithError(problems);
899+
foundError(problems);
897900
}
898901
}
899902

@@ -947,7 +950,7 @@ Future<void> verifyNoRuntimeTypeInToString(String workingDirectory) async {
947950
}
948951
}
949952
if (problems.isNotEmpty) {
950-
exitWithError(problems);
953+
foundError(problems);
951954
}
952955
}
953956

@@ -977,7 +980,7 @@ Future<void> verifyNoTrailingSpaces(String workingDirectory, { int minimumMatche
977980
}
978981
}
979982
if (problems.isNotEmpty) {
980-
exitWithError(problems);
983+
foundError(problems);
981984
}
982985
}
983986

@@ -1050,7 +1053,7 @@ Future<void> verifyIssueLinks(String workingDirectory) async {
10501053
}
10511054
assert(problems.isEmpty == suggestions.isEmpty);
10521055
if (problems.isNotEmpty) {
1053-
exitWithError(<String>[
1056+
foundError(<String>[
10541057
...problems,
10551058
...suggestions,
10561059
]);
@@ -1507,7 +1510,7 @@ Future<void> verifyNoBinaries(String workingDirectory, { Set<Hash256>? legacyBin
15071510
}
15081511
}
15091512
if (problems.isNotEmpty) {
1510-
exitWithError(<String>[
1513+
foundError(<String>[
15111514
...problems,
15121515
'All files in this repository must be UTF-8. In particular, images and other binaries',
15131516
'must not be checked into this repository. This is because we are very sensitive to the',
@@ -1545,7 +1548,7 @@ Future<List<File>> _gitFiles(String workingDirectory, {bool runSilently = true})
15451548
runSilently: runSilently,
15461549
);
15471550
if (evalResult.exitCode != 0) {
1548-
exitWithError(<String>[
1551+
foundError(<String>[
15491552
'git ls-files failed with exit code ${evalResult.exitCode}',
15501553
'${bold}stdout:$reset',
15511554
evalResult.stdout,
@@ -1671,7 +1674,7 @@ Future<EvalResult> _evalCommand(String executable, List<String> arguments, {
16711674

16721675
if (exitCode != 0 && !allowNonZeroExit) {
16731676
stderr.write(result.stderr);
1674-
exitWithError(<String>[
1677+
foundError(<String>[
16751678
'${bold}ERROR:$red Last command exited with $exitCode.$reset',
16761679
'${bold}Command:$red $commandDescription$reset',
16771680
'${bold}Relative working directory:$red $relativeWorkingDir$reset',
@@ -1702,9 +1705,11 @@ Future<void> _checkConsumerDependencies() async {
17021705
'--directory=${path.join(flutterRoot, 'packages', package)}',
17031706
]);
17041707
if (result.exitCode != 0) {
1705-
print(result.stdout as Object);
1706-
print(result.stderr as Object);
1707-
exit(result.exitCode);
1708+
foundError(<String>[
1709+
result.stdout.toString(),
1710+
result.stderr.toString(),
1711+
]);
1712+
return;
17081713
}
17091714
final Map<String, Object?> rawJson = json.decode(result.stdout as String) as Map<String, Object?>;
17101715
final Map<String, Map<String, Object?>> dependencyTree = <String, Map<String, Object?>>{
@@ -1734,7 +1739,7 @@ Future<void> _checkConsumerDependencies() async {
17341739
String plural(int n, String s, String p) => n == 1 ? s : p;
17351740

17361741
if (added.isNotEmpty) {
1737-
exitWithError(<String>[
1742+
foundError(<String>[
17381743
'The transitive closure of package dependencies contains ${plural(added.length, "a non-allowlisted package", "non-allowlisted packages")}:',
17391744
' ${added.join(', ')}',
17401745
'We strongly desire to keep the number of dependencies to a minimum and',
@@ -1745,7 +1750,7 @@ Future<void> _checkConsumerDependencies() async {
17451750
}
17461751

17471752
if (removed.isNotEmpty) {
1748-
exitWithError(<String>[
1753+
foundError(<String>[
17491754
'Excellent news! ${plural(removed.length, "A package dependency has been removed!", "Multiple package dependencies have been removed!")}',
17501755
' ${removed.join(', ')}',
17511756
'To make sure we do not accidentally add ${plural(removed.length, "this dependency", "these dependencies")} back in the future,',
@@ -1778,7 +1783,7 @@ Future<void> verifyNullInitializedDebugExpensiveFields(String workingDirectory,
17781783
}
17791784

17801785
if (errors.isNotEmpty) {
1781-
exitWithError(<String>[
1786+
foundError(<String>[
17821787
'${bold}ERROR: ${red}fields annotated with @_debugOnly must null initialize.$reset',
17831788
'to ensure both the field and initializer are removed from profile/release mode.',
17841789
'These fields should be written as:\n',

‎dev/bots/run_command.dart‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ Future<Command> startCommand(String executable, List<String> arguments, {
149149

150150
/// Runs the `executable` and waits until the process exits.
151151
///
152-
/// If the process exits with a non-zero exit code, exits this process with
153-
/// exit code 1, unless `expectNonZeroExit` is set to true.
152+
/// If the process exits with a non-zero exit code and `expectNonZeroExit` is
153+
/// false, calls foundError (which does not terminate execution!).
154154
///
155155
/// `outputListener` is called for every line of standard output from the
156156
/// process, and is given the [Process] object. This can be used to interrupt
@@ -195,16 +195,17 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, {
195195
io.stdout.writeln(result.flattenedStderr);
196196
break;
197197
}
198-
exitWithError(<String>[
198+
foundError(<String>[
199199
if (failureMessage != null)
200200
failureMessage
201201
else
202202
'${bold}ERROR: ${red}Last command exited with ${result.exitCode} (expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'}).$reset',
203203
'${bold}Command: $green$commandDescription$reset',
204204
'${bold}Relative working directory: $cyan$relativeWorkingDir$reset',
205205
]);
206+
} else {
207+
print('$clock ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset');
206208
}
207-
print('$clock ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset');
208209
return result;
209210
}
210211

‎dev/bots/service_worker_test.dart‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import 'dart:async';
6-
import 'dart:io';
5+
import 'dart:core' hide print;
6+
import 'dart:io' hide exit;
77

88
import 'package:path/path.dart' as path;
99
import 'package:shelf/shelf.dart';
@@ -43,6 +43,10 @@ Future<void> main() async {
4343
await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJsShort);
4444
await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJsEntrypointLoadedEvent);
4545
await runWebServiceWorkerTestWithBlockedServiceWorkers(headless: false);
46+
if (hasError) {
47+
print('One or more tests failed.');
48+
reportErrorsAndExit();
49+
}
4650
}
4751

4852
Future<void> _setAppVersion(int version) async {

0 commit comments

Comments
 (0)