commit 502f874b70f5bb7cc2fc25ab28cfc5372a012479
author Stephen Li <stephenli@google.com> Tue May 23 15:58:03 2017 -0700
committer Stephen Li <stephenli@google.com> Tue May 23 15:58:03 2017 -0700
tree ae231492075a460d0c07c5f07b631c5cfe2bcd69
parent aba35b095920742aab3361df10cf88a46b17a397

Deflake testDownstreamMergeConflict by abandoning changes correctly.

Previously, we had a race condition where we would abandon all
downstream changes of the initial, source change. Instead, we should
only abandon the obsolete change that hit the merge conflict and all changes downstream of it.

Change-Id: I1e5066ec7c48ba9631d0ab8e07862ddfa1cd24cb

src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
index e7f85da..2920afe 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -285,49 +285,52 @@
     for (String downstreamBranch : mdsMergeInput.dsBranchMap.keySet()) {
       // If there are existing downstream merges, update them
       // Otherwise, create them.
-      try {
-        boolean createDownstreams = true;
-        if (mdsMergeInput.obsoleteRevision != null) {
-          existingDownstream =
-              getExistingMergesOnBranch(
-                  mdsMergeInput.obsoleteRevision, mdsMergeInput.topic, downstreamBranch);
-          if (!existingDownstream.isEmpty()) {
-            log.debug(
-                "Attempting to update downstream merge of {} on branch {}",
-                mdsMergeInput.currentRevision,
-                downstreamBranch);
-            // existingDownstream should almost always be of length one, but
-            // it's possible to construct it so that it's not
-            for (Integer dsChangeNumber : existingDownstream) {
+      boolean createDownstreams = true;
+      if (mdsMergeInput.obsoleteRevision != null) {
+        existingDownstream =
+            getExistingMergesOnBranch(
+                mdsMergeInput.obsoleteRevision, mdsMergeInput.topic, downstreamBranch);
+        if (!existingDownstream.isEmpty()) {
+          log.debug(
+              "Attempting to update downstream merge of {} on branch {}",
+              mdsMergeInput.currentRevision,
+              downstreamBranch);
+          // existingDownstream should almost always be of length one, but
+          // it's possible to construct it so that it's not
+          for (Integer dsChangeNumber : existingDownstream) {
+            try {
               updateDownstreamMerge(
                   mdsMergeInput.currentRevision,
                   mdsMergeInput.subject,
                   dsChangeNumber,
                   mdsMergeInput.dsBranchMap.get(downstreamBranch));
               createDownstreams = false;
+            } catch (MergeConflictException e) {
+              failedMergeBranchMap.put(downstreamBranch, e.getMessage());
+              log.debug("Abandoning existing, obsolete {} due to merge conflict.", dsChangeNumber);
+              abandonChange(dsChangeNumber);
             }
           }
         }
-        if (createDownstreams) {
-          log.debug(
-              "Attempting to create downstream merge of {} on branch {}",
-              mdsMergeInput.currentRevision,
-              downstreamBranch);
-          SingleDownstreamMergeInput sdsMergeInput = new SingleDownstreamMergeInput();
-          sdsMergeInput.currentRevision = mdsMergeInput.currentRevision;
-          sdsMergeInput.changeNumber = mdsMergeInput.changeNumber;
-          sdsMergeInput.project = mdsMergeInput.project;
-          sdsMergeInput.topic = mdsMergeInput.topic;
-          sdsMergeInput.subject = mdsMergeInput.subject;
-          sdsMergeInput.downstreamBranch = downstreamBranch;
-          sdsMergeInput.doMerge = mdsMergeInput.dsBranchMap.get(downstreamBranch);
+      }
+      if (createDownstreams) {
+        log.debug(
+            "Attempting to create downstream merge of {} on branch {}",
+            mdsMergeInput.currentRevision,
+            downstreamBranch);
+        SingleDownstreamMergeInput sdsMergeInput = new SingleDownstreamMergeInput();
+        sdsMergeInput.currentRevision = mdsMergeInput.currentRevision;
+        sdsMergeInput.changeNumber = mdsMergeInput.changeNumber;
+        sdsMergeInput.project = mdsMergeInput.project;
+        sdsMergeInput.topic = mdsMergeInput.topic;
+        sdsMergeInput.subject = mdsMergeInput.subject;
+        sdsMergeInput.downstreamBranch = downstreamBranch;
+        sdsMergeInput.doMerge = mdsMergeInput.dsBranchMap.get(downstreamBranch);
+        try {
           createSingleDownstreamMerge(sdsMergeInput);
+        } catch (MergeConflictException e) {
+          failedMergeBranchMap.put(downstreamBranch, e.getMessage());
         }
-      } catch (MergeConflictException e) {
-        failedMergeBranchMap.put(downstreamBranch, e.getMessage());
-        log.debug("Abandoning downstream of {}", mdsMergeInput.changeNumber);
-        abandonDownstream(
-            gApi.changes().id(mdsMergeInput.changeNumber).info(), mdsMergeInput.currentRevision);
       }
     }