diff options
author | Ville Suomala <ville.suomala@systencess.fi> | 2020-02-12 18:15:19 +0200 |
---|---|---|
committer | Jukka Jokiniva <jukka.jokiniva@qt.io> | 2020-02-20 13:17:49 +0000 |
commit | 731ba877dde5bed3481b28d3cb24de436f1eea04 (patch) | |
tree | 9f4b475433b437d372016d4a1c9aadb8455dfb94 | |
parent | 4305930afb761ce77dd1f318c5b0c586ef499b08 (diff) |
Keep status of non-staged cherry-picked changes on build approve
A fix on build approve method.
When running 'build approve' after merging a branch to another
branch, an incorrect status change was fulfilled:
The status of cherry picked change (on targed branch) was changed
from new/abandoned to merged.
This occurred because cherry picked change had the same change-id as
one of the merged changes on source branch.
Fixes: QTQAINFRA-3411
Fixes: QTQAINFRA-3374
Change-Id: Ie04c5268b0196f5ff9e43618fe93c300865b9ea6
Reviewed-by: Jukka Jokiniva <jukka.jokiniva@qt.io>
3 files changed, 76 insertions, 7 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 fabbadf..7d7bd40 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java @@ -236,7 +236,7 @@ class QtCommandBuildApprove extends SshCommand { } updateChanges( - affectedChanges, Change.Status.MERGED, null, message, ChangeMessagesUtil.TAG_MERGED, true); + affectedChanges, Change.Status.MERGED, Change.Status.INTEGRATING, message, ChangeMessagesUtil.TAG_MERGED, true); logger.atInfo().log( "qtcodereview: staging-approve build %s merged into branch %s", buildBranch, destBranchKey); @@ -273,7 +273,7 @@ class QtCommandBuildApprove extends SshCommand { private void updateChanges( List<Entry<ChangeData, RevCommit>> list, - Change.Status status, + Change.Status newStatus, Change.Status oldStatus, String changeMessage, String tag, @@ -284,14 +284,13 @@ class QtCommandBuildApprove extends SshCommand { new ArrayList<Map.Entry<ChangeData, RevCommit>>(); // do the db update - QtChangeUpdateOp op = qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, null); + QtChangeUpdateOp op = qtUpdateFactory.create(newStatus, oldStatus, changeMessage, null, tag, null); try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) { for (Entry<ChangeData, RevCommit> item : list) { ChangeData cd = item.getKey(); Change change = cd.change(); - if ((oldStatus == null || change.getStatus() == oldStatus) - && change.getStatus() != Change.Status.MERGED) { - if (status == Change.Status.MERGED) { + if (change.getStatus() == oldStatus) { + if (newStatus == Change.Status.MERGED) { ObjectId obj = git.resolve(cd.currentPatchSet().getRevision().get()); CodeReviewCommit currCommit = new CodeReviewCommit(obj); currCommit.setPatchsetId(cd.currentPatchSet().getId()); @@ -303,7 +302,7 @@ class QtCommandBuildApprove extends SshCommand { } u.addOp( changeId, - qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, currCommit)); + qtUpdateFactory.create(newStatus, oldStatus, changeMessage, null, tag, currCommit)); } else { u.addOp(change.getId(), op); } diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java index bdf398c..8abf40d 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java @@ -14,6 +14,7 @@ import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.common.FooterConstants; +import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.ChangeInfo; @@ -103,6 +104,24 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { return response; } + protected ChangeInfo cherryPick(final PushOneCommit.Result c, final String branch) throws Exception { + // If the commit message does not specify a Change-Id, a new one is picked for the destination change. + final CherryPickInput input = new CherryPickInput(); + input.message = "CHERRY" + c.getCommit().getFullMessage(); + input.destination = branch; + + final RestResponse response = call_REST_API_CherryPick(c.getChangeId(), c.getCommit().getName(), input); + response.assertOK(); + return newGson().fromJson(response.getReader(), ChangeInfo.class); + } + + protected RestResponse call_REST_API_CherryPick( + final String changeId, final String revisionId, final CherryPickInput input) + throws Exception { + final String url = "/changes/" + changeId + "/revisions/" + revisionId + "/cherrypick"; + return userRestSession.post(url, input); + } + protected void QtUnStage(PushOneCommit.Result c) throws Exception { RestResponse response = call_REST_API_UnStage(c.getChangeId(), getCurrentPatchId(c)); response.assertOK(); 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 181d76e..2a54303 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java @@ -3,12 +3,18 @@ package com.googlesource.gerrit.plugins.qtcodereview; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseSsh; import com.google.gerrit.common.data.Permission; +import com.google.gerrit.extensions.api.changes.Changes; +import com.google.gerrit.extensions.client.ChangeStatus; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import java.io.StringBufferInputStream; @@ -29,7 +35,10 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { @Before public void SetDefaultPermissions() throws Exception { + createBranch(new Branch.NameKey(project, "feature")); + grant(project, "refs/heads/master", Permission.QT_STAGE, false, REGISTERED_USERS); + grant(project, "refs/heads/feature", Permission.QT_STAGE, false, REGISTERED_USERS); grant(project, "refs/staging/*", Permission.PUSH, false, adminGroupUuid()); grant(project, "refs/builds/*", Permission.CREATE, false, adminGroupUuid()); } @@ -80,6 +89,48 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { } @Test + public void cherryPicked_Stays_Intact_After_Merge_And_Build() throws Exception { + // make a change on feature branch + final PushOneCommit.Result f1 = pushCommit("feature", "f1-commitmsg", "f1-file", "f1-content"); + approve(f1.getChangeId()); + gApi.changes().id(f1.getCommit().getName()).current().submit(); + + // cherry pick it to the master branch (now there are two changes with same change-id) + final ChangeInfo cp = cherryPick(f1, "master"); + + // make another change on feature branch + final PushOneCommit.Result f2 = pushCommit("feature", "f2-commitmsg", "f2-file", "f2-content"); + approve(f2.getChangeId()); + QtStage(f2); + QtNewBuild("feature", "feature-build-000"); + QtApproveBuild("feature", "feature-build-000"); + + // make a change on master branch + final PushOneCommit.Result m1 = pushCommit("master", "m1-commitmsg", "m1-file", "m1-content"); + approve(m1.getChangeId()); + QtStage(m1); + QtNewBuild("master", "master-build-000"); + QtApproveBuild("master", "master-build-000"); + + // merge feature branch into master + final PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo); + mm.setParents(ImmutableList.of(f2.getCommit(), m1.getCommit())); + final PushOneCommit.Result m = mm.to("refs/for/master"); + m.assertOkStatus(); + approve(m.getChangeId()); + QtStage(m); + QtNewBuild("master", "merge-build-000"); + QtApproveBuild("master", "merge-build-000"); + + final Changes changes = gApi.changes(); + assertThat(changes.id(project.get(), "feature", f1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED); + assertThat(changes.id(project.get(), "feature", f2.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED); + assertThat(changes.id(project.get(), "master", m1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED); + assertThat(changes.id(project.get(), "master", m.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED); + assertThat(changes.id(project.get(), "master", cp.changeId).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.NEW); + } + + @Test public void multiChange_New_Staged_Integrating_Failed() throws Exception { // Push 3 independent commits RevCommit initialHead = getRemoteHead(); |