Status changed back to NEW, if build cannot be updated to target branch

When a passed build cannot be updated to target branch, related changes
updated back to NEW status.

Change-Id: I3a8cc34e24bf0d52aa94c5d937c422b4c8050c4a
Fixes: QTQAINFRA-3110
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>

3 files changed, 49 insertions, 1 deletions

diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
index 4dcfa4e..78a62fa 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
@@ -212,7 +212,13 @@ class QtCommandBuildApprove extends SshCommand {
 
         ObjectId oldId = git.resolve(destBranchKey.get());
 
-        QtUtil.mergeBranches(user.asIdentifiedUser(), git, buildBranchKey, destBranchKey);
+        Result result = QtUtil.mergeBranches(user.asIdentifiedUser(), git, buildBranchKey, destBranchKey);
+
+        if (result != Result.FAST_FORWARD) {
+            message = "Branch update failed, changed back to NEW. Either the destination branch was changed externally, or this is an issue in the Qt plugin.";
+            rejectBuildChanges();
+            return;
+        }
 
         updateChanges(affectedChanges, Change.Status.MERGED, null,
                       message, ChangeMessagesUtil.TAG_MERGED, true);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
index 64d50c7..c72f8c3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
@@ -589,6 +589,9 @@ public class QtUtil {
             RefUpdate refUpdate = git.updateRef(destination.get());
             refUpdate.setNewObjectId(mergeCommit);
             return refUpdate.update();
+        } catch (Exception e) {
+            logger.atWarning().log("qtcodereview: merge failed, %s", e);
+            return null;
         } finally {
             revWalk.dispose();
         }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java
index cb3e10a..9c430b9 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java
@@ -196,6 +196,45 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT {
     }
 
     @Test
+    public void errorApproveBuild_FastForwardFail() throws Exception {
+        RevCommit initialHead = getRemoteHead();
+        PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
+        approve(c.getChangeId());
+        QtStage(c);
+        QtNewBuild("master", "test-build-605");
+
+        // direct push that causes fast forward failure
+        testRepo.reset(initialHead);
+        PushOneCommit.Result d = pushCommit("master", "commitmsg2", "file2", "content2");
+        approve(d.getChangeId());
+        gApi.changes().id(d.getChangeId()).current().submit();
+        RevCommit branchHead = getRemoteHead();
+
+
+        String stagingRef = R_STAGING + "master";
+        String branchRef = R_HEADS + "master";
+        String commandStr;
+        commandStr ="gerrit-plugin-qt-workflow staging-approve";
+        commandStr += " --project " + project.get();
+        commandStr += " --branch master";
+        commandStr += " --build-id test-build-605";
+        commandStr += " --result pass";
+        commandStr += " --message " + MERGED_MSG;
+        adminSshSession.exec(commandStr);
+        assertThat(adminSshSession.getError()).isNull();
+
+        RevCommit updatedHead = getRemoteHead(project, branchRef);
+        assertThat(updatedHead).isEqualTo(branchHead); // master is not updated
+        RevCommit stagingHead = getRemoteHead(project, stagingRef);
+        assertThat(stagingHead).isEqualTo(branchHead); // staging is updated to branch head
+
+        Change change = c.getChange().change();
+        assertThat(change.getStatus()).isEqualTo(Change.Status.NEW);
+        change = d.getChange().change();
+        assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED);
+    }
+
+    @Test
     public void approveBuild_MultiLineMessage() throws Exception {
         PushOneCommit.Result c = pushCommit("master", "commitmsg1", "file1", "content1");
         approve(c.getChangeId());