Skip to content

Commit

Permalink
[infra] Fix approve_results race condition checking failing on new bu…
Browse files Browse the repository at this point in the history
…ilders.

It would accidentally compare a null old approval map with the new approvals
instead of using an empty map for the old approvals.

While here, remove debug code that shouldn't have been committed.

Change-Id: I071bc6fc830bbb0b0cec310a189c0ef0a46f2bb8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98000
Reviewed-by: Alexander Thomas <athom@google.com>
  • Loading branch information
sortie committed Mar 27, 2019
1 parent 440d287 commit bd99509
Showing 1 changed file with 11 additions and 17 deletions.
28 changes: 11 additions & 17 deletions tools/approve_results.dart
Original file line number Diff line number Diff line change
Expand Up @@ -808,16 +808,6 @@ ${parser.usage}""");
}
}

// 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>>>{};

Expand Down Expand Up @@ -926,14 +916,18 @@ ${parser.usage}""");
}
}

// 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 oldApprovals = oldApprovalsForBuilders.putIfAbsent(
test.bot, () => new SplayTreeMap<String, Map<String, dynamic>>());
oldApprovals[test.key] = test.approvedResultData;
}
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());
}
}
oldApprovalsForBuilders.putIfAbsent(
builder, () => <String, Map<String, dynamic>>{});
}

// Update approved_results.json for each builder with unapproved changes.
Expand Down

0 comments on commit bd99509

Please sign in to comment.