Skip to content

Commit 1edc6d4

Browse files
committed
[infra] Fix approve_results not erroring properly if tryjobs lack results.
Fixes #36157 Change-Id: I9c85cc9e54a5c09353c74897f39d01b0784ab2b8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96650 Reviewed-by: William Hesse <[email protected]>
1 parent 1a196d4 commit 1edc6d4

File tree

1 file changed

+56
-15
lines changed

1 file changed

+56
-15
lines changed

tools/approve_results.dart

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
/// green.
99
1010
import 'dart:async';
11+
import 'dart:collection';
1112
import 'dart:convert';
1213
import 'dart:io';
1314
import 'dart:math';
@@ -73,25 +74,45 @@ Future<Map<String, Map<String, dynamic>>> loadResultsMapIfExists(
7374
? loadResultsMap(path)
7475
: <String, Map<String, dynamic>>{};
7576

77+
/// Exception for when the results for a builder can't be found.
78+
class NoResultsException implements Exception {
79+
final String message;
80+
final String buildUrl;
81+
82+
NoResultsException(this.message, this.buildUrl);
83+
84+
String toString() => message;
85+
}
86+
7687
/// Loads a log from logdog.
7788
Future<String> loadLog(String id, String step) async {
89+
final buildUrl = "https://ci.chromium.org/b/$id";
7890
final logUrl = Uri.parse("https://logs.chromium.org/"
7991
"logs/dart/buildbucket/cr-buildbucket.appspot.com/"
8092
"$id/+/steps/$step?format=raw");
8193
final client = new HttpClient();
82-
final request =
83-
await client.getUrl(logUrl).timeout(const Duration(seconds: 60));
84-
final response = await request.close().timeout(const Duration(seconds: 60));
85-
if (response.statusCode != HttpStatus.ok) {
86-
throw new Exception("The log at $logUrl doesn't exist");
94+
try {
95+
final request =
96+
await client.getUrl(logUrl).timeout(const Duration(seconds: 60));
97+
final response = await request.close().timeout(const Duration(seconds: 60));
98+
if (response.statusCode == HttpStatus.notFound) {
99+
await response.drain();
100+
throw new NoResultsException(
101+
"The log at $logUrl doesn't exist: ${response.statusCode}", buildUrl);
102+
}
103+
if (response.statusCode != HttpStatus.ok) {
104+
await response.drain();
105+
throw new Exception("Failed to download $logUrl: ${response.statusCode}");
106+
}
107+
final contents = (await response
108+
.transform(new Utf8Decoder())
109+
.timeout(const Duration(seconds: 60))
110+
.toList())
111+
.join("");
112+
return contents;
113+
} finally {
114+
client.close();
87115
}
88-
final contents = (await response
89-
.transform(new Utf8Decoder())
90-
.timeout(const Duration(seconds: 60))
91-
.toList())
92-
.join("");
93-
client.close();
94-
return contents;
95116
}
96117

97118
/// TODO(https://github.com/dart-lang/sdk/issues/36015): The step name changed
@@ -493,10 +514,19 @@ ${parser.usage}""");
493514
// Load all the latest results for the selected bots, as well as flakiness
494515
// data, and the set of currently approved results. Each bot's latest build
495516
// is downloaded in parallel to make this phase faster.
496-
final testListFutures = <Future>[];
517+
final testListFutures = <Future<List<Test>>>[];
518+
final noResultsBuilds = new SplayTreeMap<String, String>();
497519
for (final String bot in bots) {
498-
testListFutures
499-
.add(loadResultsFromBot(bot, options, changelistBuilds[bot]));
520+
testListFutures.add(new Future(() async {
521+
try {
522+
return await loadResultsFromBot(bot, options, changelistBuilds[bot]);
523+
} on NoResultsException catch (e) {
524+
print(
525+
"Error: Failed to find results for $bot build <${e.buildUrl}>: $e");
526+
noResultsBuilds[bot] = e.buildUrl;
527+
return <Test>[];
528+
}
529+
}));
500530
}
501531

502532
// Collect all the tests from the synchronous downloads.
@@ -631,6 +661,17 @@ ${parser.usage}""");
631661
statistic(brokenTests.length, tests.length,
632662
"tests were broken since last approval");
633663

664+
// Warn about any builders where results weren't available.
665+
if (noResultsBuilds.isNotEmpty) {
666+
print("");
667+
noResultsBuilds.forEach((String builder, String buildUrl) {
668+
print("Warning: No results were found for $builder: <$buildUrl>");
669+
});
670+
print("Warning: Builders without results are usually due to infrastructure "
671+
"issues, please have a closer look at the affected builders and try "
672+
"the build again.");
673+
}
674+
634675
// Stop if there's nothing to do.
635676
if (unapprovedBots.isEmpty) {
636677
print("\nEvery test result has already been approved.");

0 commit comments

Comments
 (0)