From bd995096010c4c84a5e5f1a74ae6d04ab1f2eaa7 Mon Sep 17 00:00:00 2001
From: Jonas Termansen <sortie@google.com>
Date: Wed, 27 Mar 2019 09:56:22 +0000
Subject: [PATCH] [infra] Fix approve_results race condition checking failing
 on new builders.

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>
---
 tools/approve_results.dart | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/tools/approve_results.dart b/tools/approve_results.dart
index afe488f97c7..5912f3b4668 100755
--- a/tools/approve_results.dart
+++ b/tools/approve_results.dart
@@ -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>>>{};
 
@@ -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.