[infra] Add support for pre-approvals in approved_results.json.
This change adds a 'preapprovals' field to approved_results.json, which is
a map from gerrit Change-Id values to a pre-approval record, containing:
* 'from': What the previous approval was.
* 'result': What the new approval is.
* 'matches': Whether the new approval matches the expectation.
* 'expected': The expectation for the new approval.
* 'preapprover': Who did the preapproval.
* 'preapproved_at': When the preapproval occurred.
tools/approve_results.dart now produces pre-approval records instead of
updating the actual approvals. This ensures pre-approvals don't take
effect immediately and fixes the problem of pre-approving one kind of
failure becoming another kind of failure. The script needed to query the
gerrit API in order to determine the Change-Id of changelists. This query
also lets approve_results know what the latest changelist and lifts the
restriction that the patchset must be supplied when pre-approving.
The innards of approve_results has been refactored so it better can handle
selective modification of approve_results.dart.
approve_results now also checks for race conditions with any other
concurrent approval in interactive mode. This should catch most cases where
people leave approve_results running for long. There still is a small race
condition, so there is no check done in the non-interactive mode as the
results would just have been downloaded anyway.
The new tools/bots/apply_preapprovals.dart script will be used by the
recipe. It locates the outstanding pre-approved changelists and checks
the git history for whether they have landed by searching for their
Change-Id. It then applies their pre-approvals in order, warning about any
merge conflicts. Optionally it uploads the pre-approvals. This will work
for the commit queue where the pre-approvals can be applied temporarily,
and in the continuous integration where the pre-approvals become permanent
approvals.
This change allows empty approval records that only contain preapprovals but
with no actual base approval. tools/bots/compare_results.dart has been
updared to handle that case.
This changelist addresses https://github.com/dart-lang/sdk/issues/36279 by
ensuring only verified fields make it into new approved_results.json records
and adds a temporary workaround for 14 days to remove existing needless
existing fields.
Change-Id: I7b3ff59756dc0d28721c6db480782b4ea983b55d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97308
Reviewed-by: William Hesse <whesse@google.com>
diff --git a/tools/approve_results.dart b/tools/approve_results.dart
index 9d11f48..c577d5d 100755
--- a/tools/approve_results.dart
+++ b/tools/approve_results.dart
@@ -18,6 +18,37 @@
import 'bots/results.dart';
+/// Returns whether two decoded JSON objects are identical.
+bool isIdenticalJson(dynamic a, dynamic b) {
+ if (a is Map<String, dynamic> && b is Map<String, dynamic>) {
+ if (a.length != b.length) return false;
+ for (final key in a.keys) {
+ if (!b.containsKey(key)) return false;
+ if (!isIdenticalJson(a[key], b[key])) return false;
+ }
+ return true;
+ } else if (a is List<dynamic> && b is List<dynamic>) {
+ if (a.length != b.length) return false;
+ for (int i = 0; i < a.length; i++) {
+ if (!isIdenticalJson(a[i], b[i])) return false;
+ }
+ return true;
+ } else {
+ return a == b;
+ }
+}
+
+/// Returns whether two sets of approvals are identical.
+bool isIdenticalApprovals(
+ Map<String, Map<String, dynamic>> a, Map<String, Map<String, dynamic>> b) {
+ if (a.length != b.length) return false;
+ for (final key in a.keys) {
+ if (!b.containsKey(key)) return false;
+ if (!isIdenticalJson(a[key], b[key])) return false;
+ }
+ return true;
+}
+
/// The bot names and named configurations are highly redundant if both are
/// listed. This function returns a simplified named configuration that doesn't
/// contain any aspects that's part of the bots name. This is used to get a more
@@ -30,16 +61,16 @@
.join("-");
}
-/// Represents a test on a bot with the current result, the current approved
-/// result, and flakiness data.
+/// Represents a test on a bot with the baseline results (if tryrun), the
+/// current result, the current approved result, and flakiness data.
class Test implements Comparable {
final String bot;
- final String name;
+ final Map<String, dynamic> baselineData;
final Map<String, dynamic> resultData;
final Map<String, dynamic> approvedResultData;
final Map<String, dynamic> flakinessData;
- Test(this.bot, this.name, this.resultData, this.approvedResultData,
+ Test(this.bot, this.baselineData, this.resultData, this.approvedResultData,
this.flakinessData);
int compareTo(Object other) {
@@ -54,12 +85,17 @@
return 0;
}
- String get configuration => resultData["configuration"];
- String get result => resultData["result"];
- String get expected => resultData["expected"];
+ Map<String, dynamic> get _sharedData =>
+ resultData ?? baselineData ?? approvedResultData;
+ String get name => _sharedData["name"];
+ String get configuration => _sharedData["configuration"];
+ String get key => "$configuration:$name";
+ String get expected => _sharedData["expected"];
+ String get result => (resultData ?? const {})["result"];
bool get matches => resultData["matches"];
- String get approvedResult =>
- approvedResultData != null ? approvedResultData["result"] : null;
+ String get baselineResult => (baselineData ?? const {})["result"];
+ String get approvedResult => (approvedResultData ?? const {})["result"];
+ bool get isDifferent => result != null && result != baselineResult;
bool get isApproved => result == approvedResult;
List<String> get flakyModes =>
flakinessData != null ? flakinessData["outcomes"].cast<String>() : null;
@@ -133,7 +169,7 @@
/// Loads the results from the bot.
Future<List<Test>> loadResultsFromBot(String bot, ArgResults options,
- Map<String, dynamic> changelistBuild) async {
+ String changeId, Map<String, dynamic> changelistBuild) async {
if (options["verbose"]) {
print("Loading $bot...");
}
@@ -146,7 +182,7 @@
/// TODO(https://github.com/dart-lang/sdk/issues/36015): The step name
/// changed incompatibly, allow both temporarily to reduce the user
/// breakage. Remove this 2019-03-25.
- final build = (changelistBuild != null
+ final build = (changeId != null
? await todoFallbackLoadLog(
changelistBuild["id"],
"download_previous_results/0/steps/gsutil_find_latest_build/0/logs/"
@@ -164,7 +200,7 @@
"$approvedResultsStoragePath/$bot/approved_results.json",
"${tmpdir.path}/approved_results.json"),
new Future(() async {
- if (changelistBuild != null) {
+ if (changeId != null) {
tryResults.addAll(parseResultsMap(await loadLog(
changelistBuild["id"], "test_results/0/logs/results.json/0")));
}
@@ -196,68 +232,24 @@
// Construct an object for every test containing its current result,
// what the last approved result was, and whether it's flaky.
final tests = <Test>[];
- for (final key in results.keys) {
- final result = results[key];
+ final testResults = changeId != null ? tryResults : results;
+ for (final key in testResults.keys) {
+ final baselineResult = changeId != null ? results[key] : null;
+ final testResult = testResults[key];
final approvedResult = approvedResults[key];
final flakiness = flaky[key];
- // If preapproving results, allow new non-matching results that are
- // different from the baseline. The approved results will be the current
- // approved results, plus the difference between the tryrun's baseline and
- // the tryrun's results.
- if (tryResults.containsKey(key)) {
- final tryResult = tryResults[key];
- final wasFlake = flakiness != null &&
- (flakiness["outcomes"] as List<dynamic>)
- .contains(tryResult["result"]);
- // Pick the try run result if the try result was not a flake and it's a
- // non-matching result that's different than the approved result. If
- // there is no approved result yet, use the latest result from the
- // builder instead.
- final baseResult = approvedResult ?? result;
- if (!wasFlake &&
- !tryResult["matches"] &&
- tryResult["result"] != result["result"]) {
- // The approved_results.json format currently does not natively
- // support preapproval, so preapproving turning one failure into
- // another will turn the builder in question red until the CL lands.
- if (!baseResult["matches"] &&
- tryResult["result"] != baseResult["result"]) {
- print("Warning: Preapproving changed failure modes will turn the "
- "CI red until the CL is submitted: $bot: $key: "
- "${baseResult["result"]} -> ${tryResult["result"]}");
- }
- result.clear();
- result.addAll(tryResult);
- } else {
- if (approvedResult != null) {
- result.clear();
- result.addAll(approvedResult);
- }
- }
- } else if (tryResults.isNotEmpty && approvedResult != null) {
- result.clear();
- result.addAll(approvedResult);
- }
- final name = result["name"];
- final test = new Test(bot, name, result, approvedResult, flakiness);
- final dropApproval =
- test.matches ? options["failures-only"] : options["successes-only"];
- if (dropApproval && !test.isApproved) {
- if (approvedResult == null) continue;
- result.clear();
- result.addAll(approvedResult);
- }
+ final test =
+ new Test(bot, baselineResult, testResult, approvedResult, flakiness);
tests.add(test);
}
- // If preapproving and the CL has introduced new tests, add the new tests
- // as well to the approved data.
- final newTestKeys = new Set<String>.from(tryResults.keys)
- .difference(new Set<String>.from(results.keys));
- for (final key in newTestKeys) {
- final result = tryResults[key];
+ // Add in approvals whose test was no longer in the results.
+ for (final key in approvedResults.keys) {
+ if (testResults.containsKey(key)) continue;
+ final baselineResult = changeId != null ? results[key] : null;
+ final approvedResult = approvedResults[key];
final flakiness = flaky[key];
- final name = result["name"];
- final test = new Test(bot, name, result, null, flakiness);
+ final test =
+ new Test(bot, baselineResult, null, approvedResult, flakiness);
tests.add(test);
}
if (options["verbose"]) {
@@ -270,6 +262,33 @@
}
}
+Future<Map<String, dynamic>> loadJsonPrefixedAPI(String url) async {
+ final client = new HttpClient();
+ try {
+ final request = await client
+ .getUrl(Uri.parse(url))
+ .timeout(const Duration(seconds: 30));
+ final response = await request.close().timeout(const Duration(seconds: 30));
+ if (response.statusCode != HttpStatus.ok) {
+ throw new Exception("Failed to request $url: ${response.statusCode}");
+ }
+ final text = await response
+ .transform(utf8.decoder)
+ .join()
+ .timeout(const Duration(seconds: 30));
+ return jsonDecode(text.substring(5 /* ")]}'\n" */));
+ } finally {
+ client.close();
+ }
+}
+
+Future<Map<String, dynamic>> loadChangelistDetails(
+ String gerritHost, String changeId) async {
+ // ?O=516714 requests the revisions field.
+ final url = "https://$gerritHost/changes/$changeId/detail?O=516714";
+ return await loadJsonPrefixedAPI(url);
+}
+
main(List<String> args) async {
final parser = new ArgParser();
parser.addFlag("automated-approver",
@@ -382,9 +401,11 @@
// Determine which builders have run for the changelist.
final changelistBuilds = <String, Map<String, dynamic>>{};
- if (options["preapprove"] != null) {
+ final isPreapproval = options["preapprove"] != null;
+ String changeId;
+ if (isPreapproval) {
if (options["verbose"]) {
- print("Loading list of try runs...");
+ print("Loading changelist details...");
}
final gerritHost = "dart-review.googlesource.com";
final gerritProject = "sdk";
@@ -396,16 +417,30 @@
return;
}
final components = gerrit.substring(prefix.length).split("/");
- if (components.length != 2 ||
- int.tryParse(components[0]) == null ||
- int.tryParse(components[1]) == null) {
+ if (!((components.length == 1 && int.tryParse(components[0]) != null) ||
+ (components.length == 2 &&
+ int.tryParse(components[0]) != null &&
+ int.tryParse(components[1]) != null))) {
stderr.writeln("error: $gerrit must be in the form of "
- "$prefix<changelist>/<patchset>");
+ "$prefix<changelist> or $prefix<changelist>/<patchset>");
exitCode = 1;
return;
}
final changelist = int.parse(components[0]);
- final patchset = int.parse(components[1]);
+ final details =
+ await loadChangelistDetails(gerritHost, changelist.toString());
+ changeId = details["change_id"];
+ final patchset = 2 <= components.length
+ ? int.parse(components[1])
+ : details["revisions"][details["current_revision"]]["_number"];
+ if (2 <= components.length) {
+ print("Using Change-Id $changeId patchset $patchset");
+ } else {
+ print("Using Change-Id $changeId with the latest patchset $patchset");
+ }
+ if (options["verbose"]) {
+ print("Loading list of try runs...");
+ }
final buildset = "buildset:patch/gerrit/$gerritHost/$changelist/$patchset";
final url = Uri.parse(
"https://cr-buildbucket.appspot.com/_ah/api/buildbucket/v1/search"
@@ -519,7 +554,8 @@
for (final String bot in bots) {
testListFutures.add(new Future(() async {
try {
- return await loadResultsFromBot(bot, options, changelistBuilds[bot]);
+ return await loadResultsFromBot(
+ bot, options, changeId, changelistBuilds[bot]);
} on NoResultsException catch (e) {
print(
"Error: Failed to find results for $bot build <${e.buildUrl}>: $e");
@@ -538,19 +574,28 @@
print("");
// Compute statistics and the set of interesting tests.
- final flakyTestsCount = tests.where((test) => test.isFlake).length;
- final failingTestsCount =
- tests.where((test) => !test.isFlake && !test.matches).length;
- final unapprovedTests =
- tests.where((test) => !test.isFlake && !test.isApproved).toList();
- final fixedTests = unapprovedTests.where((test) => test.matches).toList();
- final brokenTests = unapprovedTests.where((test) => !test.matches).toList();
+ final flakyTestsCount =
+ tests.where((test) => test.resultData != null && test.isFlake).length;
+ final failingTestsCount = tests
+ .where(
+ (test) => test.resultData != null && !test.isFlake && !test.matches)
+ .length;
+ final differentTests = tests
+ .where((test) =>
+ (isPreapproval ? test.isDifferent : !test.isApproved) &&
+ !test.isFlake)
+ .toList();
+ final selectedTests = differentTests
+ .where((test) => !(test.matches
+ ? options["failures-only"]
+ : options["successes-only"]))
+ .toList();
+ final fixedTests = selectedTests.where((test) => test.matches).toList();
+ final brokenTests = selectedTests.where((test) => !test.matches).toList();
// Find out which bots have multiple configurations.
- final outcomes = new Set<String>();
final configurationsForBots = <String, Set<String>>{};
for (final test in tests) {
- outcomes.add(test.result);
var configurationSet = configurationsForBots[test.bot];
if (configurationSet == null) {
configurationsForBots[test.bot] = configurationSet = new Set<String>();
@@ -577,7 +622,7 @@
int longestTest = "TEST".length;
int longestResult = "RESULT".length;
int longestExpected = "EXPECTED".length;
- for (final test in unapprovedTests) {
+ for (final test in selectedTests) {
unapprovedBots.add(test.bot);
final botDisplayName = getBotDisplayName(test.bot, test.configuration);
longestBot = max(longestBot, botDisplayName.length);
@@ -680,10 +725,10 @@
// Stop if this is a dry run.
if (options["no"]) {
- if (unapprovedTests.length == 1) {
+ if (selectedTests.length == 1) {
print("1 test has a changed result and needs approval");
} else {
- print("${unapprovedTests.length} "
+ print("${selectedTests.length} "
"tests have changed results and need approval");
}
return;
@@ -695,16 +740,15 @@
print("Note: It is assumed bugs have been filed about the above failures "
"before they are approved here.");
if (brokenTests.isNotEmpty) {
- final botPlural = bots.length == 1 ? "bot" : "bots";
+ final builderPlural = bots.length == 1 ? "builder" : "builders";
+ final tryBuilders = isPreapproval ? "try$builderPlural" : builderPlural;
+ final tryCommit = isPreapproval ? "tryrun" : "commit";
print("Note: Approving the failures will turn the "
- "$botPlural green on the next commit.");
- }
- if (options["preapprove"] != null) {
- print("Warning: Preapproval is currently not sticky and somebody else "
- "approving before your CL has landed will undo your preapproval.");
+ "$tryBuilders green on the next $tryCommit.");
}
while (true) {
- stdout.write("Do you want to approve? (yes/no) [yes] ");
+ final approve = isPreapproval ? "pre-approve" : "approve";
+ stdout.write("Do you want to $approve? (yes/no) [yes] ");
final line = stdin.readLineSync();
// End of file condition is considered no.
if (line == null) {
@@ -739,45 +783,194 @@
}
final now = new DateTime.now().toUtc().toIso8601String();
- // Update approved_results.json for each bot with unapproved changes.
+ // Deep clones a decoded json object.
+ dynamic deepClone(dynamic object) {
+ if (object is Map<String, dynamic>) {
+ final result = <String, dynamic>{};
+ for (final key in object.keys) {
+ result[key] = deepClone(object[key]);
+ }
+ return result;
+ } else if (object is List<dynamic>) {
+ final result = <dynamic>[];
+ for (final value in object) {
+ result.add(deepClone(value));
+ }
+ return result;
+ } else {
+ return object;
+ }
+ }
+
+ // Reconstruct the old approvals so we can double check there was no race
+ // condition when uploading.
+ final oldApprovalsForBuilders = <String, Map<String, Map<String, dynamic>>>{};
+ for (final test in tests) {
+ if (test.approvedResultData == null) continue;
+ final newApprovals = oldApprovalsForBuilders.putIfAbsent(
+ test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
+ newApprovals[test.key] = test.approvedResultData;
+ }
+
+ // Build the new approval data with the changes in test results applied.
+ final newApprovalsForBuilders = <String, Map<String, Map<String, dynamic>>>{};
+
+ if (isPreapproval) {
+ // Import all the existing approval data, keeping tests that don't exist
+ // anymore.
+ for (final test in tests) {
+ if (test.approvedResultData == null) continue;
+ final approvalData = deepClone(test.approvedResultData);
+ // TODO(https://github.com/dart-lang/sdk/issues/36279): Remove needless
+ // fields that shouldn't be in the approvals data. Remove this 2019-04-03.
+ approvalData.remove("bot_name");
+ approvalData.remove("builder_name");
+ approvalData.remove("build_number");
+ approvalData.remove("changed");
+ approvalData.remove("commit_hash");
+ approvalData.remove("commit_time");
+ approvalData.remove("commit_hash");
+ approvalData.remove("flaky");
+ approvalData.remove("previous_build_number");
+ approvalData.remove("previous_commit_hash");
+ approvalData.remove("previous_commit_time");
+ approvalData.remove("previous_flaky");
+ approvalData.remove("previous_result");
+ approvalData.remove("time_ms");
+ // Discard all the existing pre-approvals for this changelist.
+ final preapprovals =
+ approvalData.putIfAbsent("preapprovals", () => <String, dynamic>{});
+ preapprovals.remove(changeId);
+ final newApprovals = newApprovalsForBuilders.putIfAbsent(
+ test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
+ newApprovals[test.key] = approvalData;
+ }
+
+ // Pre-approve all the regressions (no need to pre-approve fixed tests).
+ for (final test in brokenTests) {
+ final newApprovals = newApprovalsForBuilders.putIfAbsent(
+ test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
+ final approvalData =
+ newApprovals.putIfAbsent(test.key, () => <String, dynamic>{});
+ final preapprovals =
+ approvalData.putIfAbsent("preapprovals", () => <String, dynamic>{});
+ final preapproval =
+ preapprovals.putIfAbsent(changeId, () => <String, dynamic>{});
+ preapproval["from"] = test.approvedResult;
+ preapproval["result"] = test.result;
+ preapproval["matches"] = test.matches;
+ preapproval["expected"] = test.expected;
+ preapproval["preapprover"] = username;
+ preapproval["preapproved_at"] = now;
+ }
+ } else {
+ // Import all the existing approval data for tests, removing tests that
+ // don't exist anymore unless they have pre-approvals.
+ for (final test in tests) {
+ if (test.approvedResultData == null) continue;
+ if (test.result == null &&
+ test.approvedResultData["preapprovals"].isEmpty) continue;
+ final approvalData = deepClone(test.approvedResultData);
+ // TODO(https://github.com/dart-lang/sdk/issues/36279): Remove needless
+ // fields that shouldn't be in the approvals data. Remove this 2019-04-03.
+ approvalData.remove("bot_name");
+ approvalData.remove("builder_name");
+ approvalData.remove("build_number");
+ approvalData.remove("changed");
+ approvalData.remove("commit_hash");
+ approvalData.remove("commit_time");
+ approvalData.remove("commit_hash");
+ approvalData.remove("flaky");
+ approvalData.remove("previous_build_number");
+ approvalData.remove("previous_commit_hash");
+ approvalData.remove("previous_commit_time");
+ approvalData.remove("previous_flaky");
+ approvalData.remove("previous_result");
+ approvalData.remove("time_ms");
+ approvalData.putIfAbsent("preapprovals", () => <String, dynamic>{});
+ final newApprovals = newApprovalsForBuilders.putIfAbsent(
+ test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
+ newApprovals[test.key] = approvalData;
+ }
+
+ // Approve the changes in test results.
+ for (final test in selectedTests) {
+ final newApprovals = newApprovalsForBuilders.putIfAbsent(
+ test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
+ final approvalData =
+ newApprovals.putIfAbsent(test.key, () => <String, dynamic>{});
+ approvalData["name"] = test.name;
+ approvalData["configuration"] = test.configuration;
+ approvalData["suite"] = test.resultData["suite"];
+ approvalData["test_name"] = test.resultData["test_name"];
+ approvalData["result"] = test.result;
+ approvalData["expected"] = test.expected;
+ approvalData["matches"] = test.matches;
+ approvalData["approver"] = username;
+ approvalData["approved_at"] = now;
+ approvalData.putIfAbsent("preapprovals", () => <String, dynamic>{});
+ }
+ }
+
+ for (final builder in newApprovalsForBuilders.keys) {
+ final newApprovals = newApprovalsForBuilders[builder];
+ for (final key in newApprovals.keys) {
+ final approvalData = newApprovals[key];
+ if (!approvalData.containsKey("preapprovals")) {
+ throw new Exception(approvalData.toString());
+ }
+ }
+ }
+
+ // Update approved_results.json for each builder with unapproved changes.
final outDirectory =
await Directory.systemTemp.createTemp("approved_results.");
+ bool raceCondition = false;
try {
- final testsForBots = <String, List<Test>>{};
- for (final test in tests) {
- if (!testsForBots.containsKey(test.bot)) {
- testsForBots[test.bot] = <Test>[test];
- } else {
- testsForBots[test.bot].add(test);
- }
- }
print("Uploading approved results...");
final futures = <Future>[];
- for (final String bot in unapprovedBots) {
- Map<String, dynamic> approveData(Test test) {
- if (test.isApproved) {
- return test.approvedResultData;
- } else {
- final data = new Map<String, dynamic>.from(test.resultData);
- data["approver"] = username;
- data["approved_at"] = now;
- return data;
- }
- }
-
- final dataList = testsForBots[bot].map(approveData).toList();
- final localPath = "${outDirectory.path}/$bot.json";
+ for (final String builder in newApprovalsForBuilders.keys) {
+ final approvals = newApprovalsForBuilders[builder].values;
+ final localPath = "${outDirectory.path}/$builder.json";
await new File(localPath).writeAsString(
- dataList.map((data) => jsonEncode(data) + "\n").join(""));
+ approvals.map((approval) => jsonEncode(approval) + "\n").join(""));
final remotePath =
- "$approvedResultsStoragePath/$bot/approved_results.json";
- futures.add(cpGsutil(localPath, remotePath)
- .then((_) => print("Uploaded approved results for $bot")));
+ "$approvedResultsStoragePath/$builder/approved_results.json";
+ futures.add(new Future(() async {
+ if (!options["yes"]) {
+ if (options["verbose"]) {
+ print("Checking for race condition on $builder...");
+ }
+ final oldApprovedResults = oldApprovalsForBuilders[builder];
+ final oldApprovalPath = "${outDirectory.path}/$builder.json.old";
+ await cpGsutil(remotePath, oldApprovalPath);
+ final checkApprovedResults =
+ await loadResultsMapIfExists(oldApprovalPath);
+ if (!isIdenticalApprovals(oldApprovedResults, checkApprovedResults)) {
+ print("error: Race condition: "
+ "$builder approvals have changed, please try again.");
+ raceCondition = true;
+ return;
+ }
+ }
+ if (options["verbose"]) {
+ print("Uploading approved results for $builder...");
+ }
+ await cpGsutil(localPath, remotePath);
+ print("Uploaded approved results for $builder");
+ }));
}
await Future.wait(futures);
+ if (raceCondition) {
+ exitCode = 1;
+ print("error: Somebody else has approved, please try again");
+ return;
+ }
if (brokenTests.isNotEmpty) {
- print(
- "Successfully approved results, the next commit will turn bots green");
+ final approved = isPreapproval ? "pre-approved" : "approved";
+ final commit = isPreapproval ? "tryrun" : "commit";
+ print("Successfully $approved results, the next $commit "
+ "will turn builders green");
} else {
print("Successfully approved results");
}
diff --git a/tools/bots/apply_preapprovals.dart b/tools/bots/apply_preapprovals.dart
new file mode 100755
index 0000000..ed15d61
--- /dev/null
+++ b/tools/bots/apply_preapprovals.dart
@@ -0,0 +1,238 @@
+#!/usr/bin/env dart
+// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Applies pending pre-approvals for any changelists that have landed according
+// to the git history of HEAD.
+
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:args/args.dart';
+
+import 'results.dart';
+
+main(List<String> args) async {
+ final parser = new ArgParser();
+ parser.addFlag("dry",
+ abbr: "n",
+ help: "Don't write out the updated approvals.",
+ negatable: false);
+ parser.addFlag("help", help: "Show the program usage.", negatable: false);
+ parser.addOption("upload",
+ abbr: "u",
+ help: "Upload the updated results to this cloud storage location");
+
+ final options = parser.parse(args);
+ if (options["help"]) {
+ print("""
+Usage: apply_preapprovals.dart [OPTION]... APPROVALS
+Applies pending pre-approvals for any changelists that have landed according to
+the git history of HEAD.
+
+The options are as follows:
+
+${parser.usage}""");
+ return;
+ }
+
+ final parameters = options.rest;
+ if (parameters.length != 1) {
+ print("error: Expected one parameter");
+ exitCode = 2;
+ return;
+ }
+
+ // Locate gsutil.py.
+ gsutilPy = Platform.script
+ .resolve("../../third_party/gsutil/gsutil.py")
+ .toFilePath();
+
+ final approvalsPath = parameters[0];
+ final approvals = await loadResultsMap(approvalsPath);
+
+ // Find the changelists with pre-approvals.
+ final allChangelists = <String>{};
+ for (final key in approvals.keys) {
+ final record = approvals[key];
+ final preapprovals =
+ record.putIfAbsent("preapprovals", () => <String, dynamic>{});
+ allChangelists.addAll(preapprovals.keys);
+ }
+ if (allChangelists.isEmpty) {
+ print("No pre-approvals are pending");
+ }
+
+ // Find the order the pre-approved changelists landed in.
+ final joinedChangelistsPattern = allChangelists.join("\\|");
+ final pattern = "^Change-Id: \\($joinedChangelistsPattern\\)\$";
+ final arguments = [
+ "rev-list",
+ "--pretty=medium",
+ "--grep=$pattern",
+ "--reverse",
+ "HEAD"
+ ];
+ final processOutput = await Process.run("git", arguments, runInShell: true);
+ if (processOutput.exitCode != 0) {
+ throw new Exception("Failed to run git $arguments\n"
+ "exitCode: ${processOutput.exitCode}\n"
+ "stdout: ${processOutput.stdout}\n"
+ "stderr: ${processOutput.stderr}");
+ }
+ final landedChangelists = <String>[];
+ final commitOfChangelist = <String, String>{};
+ String currentCommit;
+ for (final line in LineSplitter.split(processOutput.stdout)) {
+ if (line.startsWith("commit ")) {
+ currentCommit = line.substring("commit ".length);
+ } else if (line.startsWith(" Change-Id: ")) {
+ final changeId = line.substring(" Change-Id: ".length);
+ if (allChangelists.contains(changeId)) {
+ landedChangelists.add(changeId);
+ commitOfChangelist[changeId] = currentCommit;
+ }
+ }
+ }
+ if (processOutput.stdout != "") {
+ print(processOutput.stdout);
+ }
+
+ // Report the status of each of the pre-approved changelists.
+ final unlandedChangelists =
+ allChangelists.difference(landedChangelists.toSet());
+ for (final changelist in unlandedChangelists) {
+ final changelistUrl = "https://dart-review.googlesource.com/q/$changelist";
+ print("Pending: Changelist $changelistUrl hasn't landed yet");
+ }
+ if (allChangelists.isNotEmpty && landedChangelists.isEmpty) {
+ print("No pre-approved changelists have landed.");
+ }
+ for (final changelist in landedChangelists) {
+ final changelistUrl = "https://dart-review.googlesource.com/q/$changelist";
+ final commit = commitOfChangelist[changelist];
+ print("Landed: Changelist $changelistUrl landed in commit $commit");
+ }
+
+ // Apply the pre-approvals for landed changes.
+ bool updated = false;
+ final conflictsForKey = <String, List<String>>{};
+ final changelistsWithMergeConflicts = <String>{};
+ int totalNumberOfPreapprovals = 0;
+ int totalNumberOfMergeConflicts = 0;
+ for (final changelist in landedChangelists) {
+ final changelistUrl = "https://dart-review.googlesource.com/q/$changelist";
+ final commit = commitOfChangelist[changelist];
+ print("\nApplying pre-approvals for changelist "
+ "$changelistUrl landed in commit $commit");
+ int numberOfPreapprovals = 0;
+ int numberOfMergeConflicts = 0;
+ for (final key in approvals.keys) {
+ final record = approvals[key];
+ final preapprovals = record["preapprovals"];
+ final preapproval = preapprovals.remove(changelist);
+ if (preapproval == null) continue;
+ updated = true;
+ final conflicts = conflictsForKey.putIfAbsent(key, () => <String>[]);
+ if (record["result"] == preapproval["from"]) {
+ print("$changelist: $key: "
+ "${record["result"]} -> ${preapproval["result"]}");
+ conflicts.add("$changelist/$commit had changed approval from "
+ "${record["result"]} to ${preapproval["result"]}");
+ record["result"] = preapproval["result"];
+ record["matches"] = preapproval["matches"];
+ record["expected"] = preapproval["expected"];
+ record["approver"] = preapproval["preapprover"];
+ record["approved_at"] = preapproval["preapproved_at"];
+ numberOfPreapprovals++;
+ totalNumberOfPreapprovals++;
+ } else {
+ print("$changelist: $key: MERGE CONFLICT:");
+ for (final conflict in conflicts) {
+ print(" * $conflict");
+ }
+ print(" * MERGE CONFLICT: Cannot change approval from "
+ "${preapproval["from"]} to ${preapproval["result"]} "
+ "because it's currently ${record["result"]}");
+ changelistsWithMergeConflicts.add(changelist);
+ numberOfMergeConflicts++;
+ totalNumberOfMergeConflicts++;
+ }
+ }
+ if (0 < numberOfPreapprovals) {
+ print("$numberOfPreapprovals "
+ "pre-approvals applied from $changelistUrl commit $commit");
+ }
+ if (0 < numberOfMergeConflicts) {
+ print("Warning: $numberOfMergeConflicts "
+ "merge conflicts in pre-approvals for $changelistUrl commit $commit");
+ }
+ }
+
+ // Expire old pre-approvals.
+ final now = new DateTime.now().toUtc();
+ final expiredChangelists = <String>{};
+ for (final record in approvals.values) {
+ final preapprovals = record["preapprovals"];
+ final changelists = preapprovals.keys.toList();
+ for (final changelist in changelists) {
+ final preapproval = preapprovals[changelist];
+ final expires = DateTime.parse(preapproval["expires"]);
+ if (expires.isBefore(now)) {
+ updated = true;
+ preapprovals.remove(changelist);
+ expiredChangelists.add(changelist);
+ }
+ }
+ }
+ if (expiredChangelists.isNotEmpty) {
+ print("");
+ }
+ for (final changelist in expiredChangelists) {
+ final changelistUrl = "https://dart-review.googlesource.com/q/$changelist";
+ print("Expired: Pre-approvals for changelist $changelistUrl have expired");
+ }
+
+ // Format a final report.
+ print("");
+ final landedChangelistsCount = landedChangelists.length;
+ if (0 < landedChangelistsCount) {
+ print("$landedChangelistsCount changelists have landed");
+ }
+ final expiredChangelistsCount = expiredChangelists.length;
+ if (0 < expiredChangelistsCount) {
+ print("$expiredChangelistsCount changelists have expired");
+ }
+ final unlandedChangelistsCount =
+ unlandedChangelists.length - expiredChangelistsCount;
+ if (0 < unlandedChangelistsCount) {
+ print("$unlandedChangelistsCount changelists are pending");
+ }
+ if (0 < totalNumberOfPreapprovals) {
+ print("$totalNumberOfPreapprovals pre-approvals applied");
+ }
+ if (0 < totalNumberOfPreapprovals) {
+ print("Warning: $totalNumberOfMergeConflicts "
+ "pre-approvals had merge conflicts");
+ }
+
+ // Save the updated approvals and upload them to cloud storage.
+ print("");
+ if (!updated) {
+ print("Approvals are unchanged");
+ return;
+ }
+ if (options["dry"]) {
+ print("Dry run, not saving the updated approvals");
+ return;
+ }
+ await new File(approvalsPath).writeAsString(
+ approvals.values.map((data) => jsonEncode(data) + "\n").join(""));
+ print("Wrote updated approvals to $approvalsPath");
+ if (options["upload"] != null) {
+ print("Uploading updated approvals to ${options["upload"]}...");
+ await cpGsutil(approvalsPath, options["upload"]);
+ print("Uploaded updated approvals to ${options["upload"]}");
+ }
+}
diff --git a/tools/bots/compare_results.dart b/tools/bots/compare_results.dart
index c3820a0..792a473 100755
--- a/tools/bots/compare_results.dart
+++ b/tools/bots/compare_results.dart
@@ -273,7 +273,7 @@
? new Result.fromMap(mapBefore, flakinessData[name])
: null;
final resultAfter = new Result.fromMap(mapAfter, flakinessData[name]);
- final resultApproved = mapApproved != null
+ final resultApproved = mapApproved != null && mapApproved["result"] != null
? new Result.fromMap(mapApproved, flakinessData[name])
: null;
final event = new Event(resultBefore, resultAfter, resultApproved);
