diff options
| author | Jukka Jokiniva <jukka.jokiniva@qt.io> | 2019-09-13 12:02:24 +0300 |
|---|---|---|
| committer | Jukka Jokiniva <jukka.jokiniva@qt.io> | 2019-09-13 12:03:16 +0300 |
| commit | 761f4f66c0b7a21cdac3cd7025a175fef25865fa (patch) | |
| tree | 4d1258de57a3b28333d04d537a78e08911540752 | |
| parent | beb4572f62fb2d759dcb621410bfcb222e69d702 (diff) | |
| parent | 143f1d30cc7f9d72d46c5992fc576573dba65d9c (diff) | |
Merge remote-tracking branch 'origin/v2.16.9-based' into v3.0-based
Change-Id: I97acd00f65032d263529c3344595434c4f38e9d1
20 files changed, 612 insertions, 428 deletions
diff --git a/qt-gerrit-ui-plugin/qt-gerrit-ui-plugin.html b/qt-gerrit-ui-plugin/qt-gerrit-ui-plugin.html index 3bd29f6..9bedbb9 100644 --- a/qt-gerrit-ui-plugin/qt-gerrit-ui-plugin.html +++ b/qt-gerrit-ui-plugin/qt-gerrit-ui-plugin.html @@ -8,33 +8,16 @@ <script> 'use strict'; - var BUTTONS = { - 'gerrit-plugin-qt-workflow~abandon' : { - header: 'Abandon the change?', - action_name: 'abandon' - }, - 'gerrit-plugin-qt-workflow~defer' : { - header: 'Defer the change?', - action_name: 'defer' - }, - 'gerrit-plugin-qt-workflow~reopen' : { - header: 'Reopen the change?', - action_name: 'reopen' - }, - 'gerrit-plugin-qt-workflow~stage' : { - header: 'Submit to staging?', - action_name: 'stage' - }, - 'gerrit-plugin-qt-workflow~unstage' : { - header: 'Unstage the change?', - action_name: 'unstage' - } - }; + var BUTTONS = [ + 'gerrit-plugin-qt-workflow~abandon', + 'gerrit-plugin-qt-workflow~defer', + 'gerrit-plugin-qt-workflow~reopen', + 'gerrit-plugin-qt-workflow~stage', + 'gerrit-plugin-qt-workflow~unstage' + ]; Gerrit.install(plugin => { - plugin.custom_popup = null; - plugin.custom_popup_promise = null; plugin.buttons = null; function htmlToElement(html) { @@ -126,32 +109,28 @@ // Remove any existing buttons if (plugin.buttons) { - for (var key in BUTTONS) { - if (BUTTONS.hasOwnProperty(key)) { - if(typeof plugin.buttons[key] !== 'undefined' && plugin.buttons[key] !== null) { - plugin.ca.removeTapListener(plugin.buttons[key], (param) => {} ); - plugin.ca.remove(plugin.buttons[key]); - plugin.buttons[key] = null; - } + BUTTONS.forEach((key) => { + if(typeof plugin.buttons[key] !== 'undefined' && plugin.buttons[key] !== null) { + plugin.ca.removeTapListener(plugin.buttons[key], (param) => {} ); + plugin.ca.remove(plugin.buttons[key]); + plugin.buttons[key] = null; } - } + }); } else plugin.buttons = []; // Add buttons based on server response - for (var key in BUTTONS) { - if (BUTTONS.hasOwnProperty(key)) { - var action = plugin.ca.getActionDetails(key); - if (action) { - // hide dropdown action - plugin.ca.setActionHidden(action.__type, action.__key, true); - - // add button - plugin.buttons[key] = plugin.ca.add(action.__type, action.label); - plugin.ca.setTitle(plugin.buttons[key], action.title); - plugin.ca.addTapListener(plugin.buttons[key], buttonEventCallback); - } + BUTTONS.forEach((key) => { + var action = plugin.ca.getActionDetails(key); + if (action) { + // hide dropdown action + plugin.ca.setActionHidden(action.__type, action.__key, true); + + // add button + plugin.buttons[key] = plugin.ca.add(action.__type, action.label); + plugin.ca.setTitle(plugin.buttons[key], action.title); + plugin.ca.addTapListener(plugin.buttons[key], buttonEventCallback); } - } + }); function buttonEventCallback(event) { var button_key = event.type.substring(0, event.type.indexOf('-tap')); @@ -165,106 +144,22 @@ } } if (button_action) { - plugin.popup('qt-gerrit-ui-confirm-dialog').then( (param) => { - plugin.custom_popup_promise = param; - plugin.custom_popup.set('header', BUTTONS[button_index].header); - plugin.custom_popup.set('subject', changeInfo.subject); - plugin.custom_popup.set('action_name', BUTTONS[button_index].action_name); - plugin.custom_popup.set('api_url', button_action.__url); - }).catch( (param) => { console.log('unexpected error: promise failed');}); + const buttonEl = this.$$(`[data-action-key="${button_key}"]`); + buttonEl.setAttribute('loading', true); + buttonEl.disabled = true; + plugin.restApi().post(button_action.__url, {}) + .then((ok_resp) => { + buttonEl.removeAttribute('loading'); + buttonEl.disabled = false; + window.location.reload(true); + }).catch((failed_resp) => { + buttonEl.removeAttribute('loading'); + buttonEl.disabled = false; + this.fire('show-alert', {message: 'FAILED: ' + failed_resp}); + }); } else console.log('unexpected error: no action'); } }); }); </script> </dom-module> - -<dom-module id="qt-gerrit-ui-confirm-dialog"> - <template> - <style include="shared-styles"> - #dialog { - min-width: 40em; - } - p { - margin-bottom: 1em; - } - @media screen and (max-width: 50em) { - #dialog { - min-width: inherit; - width: 100%; - } - } - </style> - <gr-dialog - id="dialog" - confirm-label="Continue" - confirm-on-enter - on-cancel="_handleCancelTap" - on-confirm="_handleConfirmTap"> - <div class="header" slot="header">[[header]]</div> - <div class="main" slot="main"> - <p>Ready to [[action_name]] “<strong>[[subject]]</strong>”?</p> - <p class="main" style="color: red;font-weight: bold;">[[errorMessage]]</p> - </div> - </gr-dialog> - </template> - <script> - 'use strict'; - - var qtgerrituiconfirmdialog = Polymer({ - is: 'qt-gerrit-ui-confirm-dialog', - - properties: { - header: { - type: String, - value: '...wait...' - }, - action_name: { - type: String, - value: '' - }, - api_url: { - type: String, - value: '' - }, - subject: { - type: String, - value: '' - }, - errorMessage: { - type: String, - value: '' - } - }, - - attached: function() { - this.plugin.custom_popup = this; - }, - - resetFocus(e) { - this.$.dialog.resetFocus(); - }, - - _handleConfirmTap(e) { - this.$.dialog.disabled = true; - e.preventDefault(); - this.plugin.restApi().post(this.get('api_url'), {}) - .then((ok_resp) => { - this.$.dialog.disabled = false; - this.plugin.custom_popup_promise.close(); - this.plugin.custom_popup_promise = null; - window.location.reload(true); - }).catch((failed_resp) => { - this.$.dialog.disabled = false; - this.set('errorMessage', 'FAILED: ' + failed_resp); - }); - }, - - _handleCancelTap(e) { - e.preventDefault(); - this.plugin.custom_popup_promise.close(); - this.plugin.custom_popup_promise = null; - }, - }); - </script> -</dom-module> diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java index 3ad64f4..ddbff1d 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java @@ -5,7 +5,6 @@ package com.googlesource.gerrit.plugins.qtcodereview; import com.google.common.flogger.FluentLogger; -import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MergeConflictException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; @@ -13,15 +12,11 @@ import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.change.NotifyResolver; -import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; -import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchRefException; import com.google.gerrit.server.project.ProjectCache; @@ -35,12 +30,10 @@ import com.google.gerrit.server.util.time.TimeUtil; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; -import java.io.IOException; import java.sql.Timestamp; import java.util.Arrays; import java.util.Date; import java.util.List; -import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; @@ -56,7 +49,6 @@ public class QtCherryPickPatch { private final BatchUpdate.Factory batchUpdateFactory; private final GitRepositoryManager gitManager; private final Provider<IdentifiedUser> user; - private final PatchSetInserter.Factory patchSetInserterFactory; private final MergeUtil.Factory mergeUtilFactory; private final ProjectCache projectCache; private final QtChangeUpdateOp.Factory qtUpdateFactory; @@ -65,14 +57,12 @@ public class QtCherryPickPatch { QtCherryPickPatch(BatchUpdate.Factory batchUpdateFactory, GitRepositoryManager gitManager, Provider<IdentifiedUser> user, - PatchSetInserter.Factory patchSetInserterFactory, MergeUtil.Factory mergeUtilFactory, ProjectCache projectCache, QtChangeUpdateOp.Factory qtUpdateFactory) { this.batchUpdateFactory = batchUpdateFactory; this.gitManager = gitManager; this.user = user; - this.patchSetInserterFactory = patchSetInserterFactory; this.mergeUtilFactory = mergeUtilFactory; this.projectCache = projectCache; this.qtUpdateFactory = qtUpdateFactory; @@ -114,7 +104,6 @@ public class QtCherryPickPatch { commitToCherryPick.setNotes(changeData.notes()); CodeReviewCommit cherryPickCommit; - boolean mergeCommit = false; ProjectState projectState = projectCache.checkedGet(project); if (projectState == null) throw new NoSuchProjectException(project); @@ -123,14 +112,21 @@ public class QtCherryPickPatch { if (commitToCherryPick.getParentCount() > 1) { // Merge commit cannot be cherrypicked logger.atInfo().log("qtcodereview: merge commit detected %s", commitToCherryPick); - mergeCommit = true; - RevCommit commit = QtUtil.merge(committerIdent, - git, oi, - revWalk, - commitToCherryPick, - baseCommit, - true /* merge always */); - cherryPickCommit = revWalk.parseCommit(commit); + + if (commitToCherryPick.getParent(0).equals(baseCommit)) { + // allow fast forward, when parent index 0 is correct + logger.atInfo().log("qtcodereview: merge commit fast forwarded"); + cherryPickCommit = commitToCherryPick; + } else { + logger.atInfo().log("qtcodereview: merge of merge created"); + RevCommit commit = QtUtil.merge(committerIdent, + git, oi, + revWalk, + commitToCherryPick, + baseCommit, + true); + cherryPickCommit = revWalk.parseCommit(commit); + } } else { String commitMessage = mergeUtil.createCommitMessageOnSubmit(commitToCherryPick, baseCommit); cherryPickCommit = mergeUtil.createCherryPickFromCommit(oi, @@ -147,30 +143,17 @@ public class QtCherryPickPatch { boolean patchSetNotChanged = cherryPickCommit.equals(commitToCherryPick); if (!patchSetNotChanged) { - logger.atInfo().log("qtcodereview: new patch %s -> %s", commitToCherryPick, cherryPickCommit); + logger.atInfo().log("qtcodereview: %s cherrypicked as %s", commitToCherryPick, cherryPickCommit); oi.flush(); } Timestamp commitTimestamp = new Timestamp(committerIdent.getWhen().getTime()); BatchUpdate bu = batchUpdateFactory.create(project, identifiedUser, commitTimestamp); - bu.setRepository(git, revWalk, oi); - if (!patchSetNotChanged && !mergeCommit) { - Change.Id changeId = insertPatchSet(bu, git, changeData.notes(), cherryPickCommit); - bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus, - null, - defaultMessage, - inputMessage, - tag, - commitToCherryPick)); - logger.atInfo().log("qtcodereview: cherrypick new patch %s for %s", cherryPickCommit.toObjectId(), changeId); - } else { - bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus, - null, - defaultMessage, - inputMessage, - tag, - null)); - } - + bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus, + null, + defaultMessage, + inputMessage, + tag, + null)); bu.execute(); logger.atInfo().log("qtcodereview: cherrypick done %s", changeData.getId()); return cherryPickCommit; @@ -179,19 +162,4 @@ public class QtCherryPickPatch { } } - private Change.Id insertPatchSet(BatchUpdate bu, - Repository git, - ChangeNotes destNotes, - CodeReviewCommit cherryPickCommit) - throws IOException, BadRequestException, ConfigInvalidException { - Change destChange = destNotes.getChange(); - PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); - PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, cherryPickCommit); - inserter.setAllowClosed(true); - // .setCopyApprovals(true) doesn't work, so copying done in QtChangeUpdateOp - bu.setNotify(NotifyResolver.Result.none()); - bu.addOp(destChange.getId(), inserter); - return destChange.getId(); - } - } 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 4979323..83fcb71 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java @@ -5,6 +5,8 @@ package com.googlesource.gerrit.plugins.qtcodereview; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Branch; @@ -12,11 +14,15 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.ChangeMessagesUtil; +import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.extensions.events.ChangeMerged; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.mail.send.MergedSender; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.ProjectPermission; @@ -33,6 +39,7 @@ import com.google.gerrit.sshd.CommandMetaData; import com.google.inject.Inject; import com.google.inject.Provider; +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.ObjectId; @@ -81,6 +88,9 @@ class QtCommandBuildApprove extends SshCommand { private BatchUpdate.Factory updateFactory; @Inject + private PatchSetInserter.Factory patchSetInserterFactory; + + @Inject private GitReferenceUpdated referenceUpdated; @Inject @@ -185,7 +195,7 @@ class QtCommandBuildApprove extends SshCommand { throw die("invalid branch " + e.getMessage()); } catch (NoSuchRefException e) { throw die("invalid reference " + e.getMessage()); - } catch (UpdateException | RestApiException e) { + } catch (UpdateException | RestApiException | ConfigInvalidException e ) { logger.atSevere().log("qtcodereview: staging-napprove failed to update change status %s", e); throw die("Failed to update change status"); } catch (QtUtil.MergeConflictException e) { @@ -198,12 +208,19 @@ class QtCommandBuildApprove extends SshCommand { } private void approveBuildChanges() throws QtUtil.MergeConflictException, NoSuchRefException, - IOException, UpdateException, RestApiException { + IOException, UpdateException, RestApiException, + ConfigInvalidException { if (message == null) message = String.format("Change merged into branch %s", destBranchKey); 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); @@ -219,7 +236,8 @@ class QtCommandBuildApprove extends SshCommand { } private void rejectBuildChanges() throws QtUtil.MergeConflictException, UpdateException, - RestApiException { + RestApiException, IOException, + ConfigInvalidException { if (message == null) message = String.format("Change rejected for branch %s", destBranchKey); updateChanges(affectedChanges, Change.Status.NEW, Change.Status.INTEGRATING, @@ -238,7 +256,8 @@ class QtCommandBuildApprove extends SshCommand { String changeMessage, String tag, Boolean passed) - throws UpdateException, RestApiException { + throws UpdateException, RestApiException, + IOException, ConfigInvalidException { List<Entry<ChangeData,RevCommit>> emailingList = new ArrayList<Map.Entry<ChangeData, RevCommit>>(); @@ -246,10 +265,29 @@ class QtCommandBuildApprove extends SshCommand { QtChangeUpdateOp op = qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, null); try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) { for (Entry<ChangeData,RevCommit> item : list) { - Change change = item.getKey().change(); + ChangeData cd = item.getKey(); + Change change = cd.change(); if ((oldStatus == null || change.getStatus() == oldStatus) && change.getStatus() != Change.Status.MERGED) { - u.addOp(change.getId(), op); + if (status == Change.Status.MERGED) { + ObjectId obj = git.resolve(cd.currentPatchSet().getRevision().get()); + CodeReviewCommit currCommit = new CodeReviewCommit(obj); + currCommit.setPatchsetId(cd.currentPatchSet().getId()); + CodeReviewCommit newCommit = new CodeReviewCommit(item.getValue()); + Change.Id changeId = insertPatchSet(u, git, cd.notes(), newCommit); + if (!changeId.equals(cd.getId())) { + logger.atWarning().log("staging-approve wrong changeId for new patchSet %s != %s", + changeId, cd.getId()); + } + u.addOp(changeId, qtUpdateFactory.create(status, + oldStatus, + changeMessage, + null, + tag, + currCommit)); + } else { + u.addOp(change.getId(), op); + } emailingList.add(item); } } @@ -271,6 +309,22 @@ class QtCommandBuildApprove extends SshCommand { change, destBranchKey); } } + + } + + private Change.Id insertPatchSet(BatchUpdate bu, + Repository git, + ChangeNotes destNotes, + CodeReviewCommit cherryPickCommit) + throws IOException, BadRequestException, ConfigInvalidException { + Change destChange = destNotes.getChange(); + PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); + PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, cherryPickCommit); + inserter.setSendEmail(false) + .setAllowClosed(true); + // .setCopyApprovals(true) doesn't work, so copying done in QtChangeUpdateOp + bu.addOp(destChange.getId(), inserter); + return destChange.getId(); } private void sendMergeEvent(ChangeData changeData) { diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java index 4af35d3..881cdc7 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java @@ -122,7 +122,7 @@ class QtCommandNewBuild extends SshCommand { throw die("No changes in staging branch. Not creating a build reference"); } - QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, Change.Status.STAGED, message, null, null, null); + QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, Change.Status.STAGED, message, null, QtUtil.TAG_CI, null); try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) { for (Entry<ChangeData, RevCommit> item: openChanges) { Change change = item.getKey().change(); diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java index a48110c..748b155 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java @@ -26,6 +26,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.ChangeJson; @@ -85,7 +86,8 @@ class QtDefer extends RetryingRestModifyView<ChangeResource, AbandonInput, Chang throw new ResourceConflictException("change is " + ChangeUtil.status(change)); } - QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, null, "Deferred", input.message, null, null); + + QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, null, "Deferred", input.message, ChangeMessagesUtil.TAG_ABANDON, null); try (BatchUpdate u = updateFactory.create(change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { u.addOp(rsrc.getId(), op).execute(); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java index 205337a..0367e90 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java @@ -79,7 +79,7 @@ class QtReOpen extends RetryingRestModifyView<ChangeResource, RestoreInput, Chan throw new ResourceConflictException("change is " + ChangeUtil.status(change)); } - QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.DEFERRED, "Reopened", input.message, null, null); + QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.DEFERRED, "Reopened", input.message, QtUtil.TAG_REOPENED, null); try (BatchUpdate u = updateFactory.create(change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { u.addOp(rsrc.getId(), op).execute(); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtStage.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtStage.java index bcee0f3..2dd1d97 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtStage.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtStage.java @@ -116,7 +116,7 @@ public class QtStage implements RestModifyView<RevisionResource, SubmitInput>, throws RestApiException, RepositoryNotFoundException, IOException, PermissionBackendException, UpdateException, ConfigInvalidException { - logger.atInfo().log("qtcodereview: submit %s to staging", rsrc.getChange().toString()); + logger.atInfo().log("qtcodereview: stage %s", rsrc.getChange().toString()); IdentifiedUser submitter = rsrc.getUser().asIdentifiedUser(); change = rsrc.getChange(); @@ -181,7 +181,7 @@ public class QtStage implements RestModifyView<RevisionResource, SubmitInput>, Change.Status.STAGED, "Staged for CI", // defaultMessage null, // inputMessage - null // tag + QtUtil.TAG_CI // tag ); Result result = qtUtil.updateRef(git, stagingBranchKey.get(), commit.toObjectId(), false); referenceUpdated.fire(projectKey, stagingBranchKey.get(), destId, commit.toObjectId(), submitter.state()); diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java index b6d5f63..872edb5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java @@ -137,7 +137,7 @@ class QtUnStage implements RestModifyView<RevisionResource, SubmitInput>, UiActi throw new ResourceConflictException("Invalid Revision: " + patchSet); } - QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, "Unstaged", null, null, null); + QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, "Unstaged", null, QtUtil.TAG_CI, null); BatchUpdate u = updateFactory.create(projectKey, submitter, TimeUtil.nowTs()); u.addOp(rsrc.getChange().getId(), op).execute(); 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 c8fd8e7..246a653 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java +++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java @@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.project.NoSuchRefException; @@ -74,6 +75,10 @@ public class QtUtil { public static final String R_HEADS = "refs/heads/"; public static final String R_STAGING = "refs/staging/"; public static final String R_BUILDS = "refs/builds/"; + public static final String TAG_CI = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX + "qt:ci"; + public static final String TAG_ADMINCHANGE = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX + "qt:adminchange"; + public static final String TAG_REOPENED = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX + "qt:reopened"; + private final Provider<InternalChangeQuery> queryProvider; private final GitReferenceUpdated referenceUpdated; @@ -282,6 +287,7 @@ public class QtUtil { return null; } + // Step backwards from the ref and return change list in the same order private List<ChangeData> arrangeOrderLikeInRef(Repository git, ObjectId refObj, ObjectId tipObj, @@ -307,6 +313,7 @@ public class QtUtil { ChangeData change = findChangeFromList(changeId, changeList); if (change != null) results.add(0, change); + // It can always be trusted that parent in index 0 is the correct one commit = revWalk.parseCommit(commit.getParent(0)); } } while (commit != null && !commit.equals(tipObj) && count < 100); @@ -330,16 +337,76 @@ public class QtUtil { projectKey, srcId, newId, - true, // allowFastForward + false, // allowFastForward null, // newStatus null, // defaultMessage null, // inputMessage - null // tag + TAG_CI // tag ).toObjectId(); } return newId; } + // Step backwards from staging head and return 1st commit in integrating status + private ObjectId findIntegrationHead(Repository git, + ObjectId stagingHead, + ObjectId branchHead, + List<ChangeData> integratingChanges) + throws MissingObjectException, IOException { + + if (stagingHead.equals(branchHead)) return branchHead; + + RevWalk revWalk = new RevWalk(git); + RevCommit commit = revWalk.parseCommit(stagingHead); + int count = 0; + do { + count++; + String changeId = getChangeId(commit); + ChangeData change = findChangeFromList(changeId, integratingChanges); + if (change != null) return commit; + + if (commit.getParentCount() > 0) { + // It can always be trusted that parent in index 0 is the correct one + commit = revWalk.parseCommit(commit.getParent(0)); + } else commit = null; + + } while (commit != null && !commit.equals(branchHead) && count < 100); + + return branchHead; + } + + // Step backwards from staging head and find commit that can be reused + private ObjectId findReusableStagingHead(Repository git, + ObjectId stagingHead, + ObjectId integrationHead, + List<ChangeData> stagedChanges) + throws MissingObjectException, IOException { + + if (stagingHead.equals(integrationHead)) return integrationHead; + + ObjectId reusableHead = null; + RevWalk revWalk = new RevWalk(git); + RevCommit commit = revWalk.parseCommit(stagingHead); + int count = 0; + do { + count++; + String changeId = getChangeId(commit); + ChangeData change = findChangeFromList(changeId, stagedChanges); + if (change != null) { + if (reusableHead == null) reusableHead = commit; + } else reusableHead = null; + + if (commit.getParentCount() > 0) { + // It can always be trusted that parent in index 0 is the correct one + commit = revWalk.parseCommit(commit.getParent(0)); + } else commit = null; + + } while (commit != null && !commit.equals(integrationHead) && count < 100); + + if (reusableHead == null) reusableHead = integrationHead; + return reusableHead; + } + public void rebuildStagingBranch(Repository git, IdentifiedUser user, final Project.NameKey projectKey, @@ -349,87 +416,73 @@ public class QtUtil { InternalChangeQuery query = null; List<ChangeData> changes_integrating = null; List<ChangeData> changes_staged = null; - ObjectId oldStageRefObjId = null; - ObjectId branchObjId = null; - ObjectId newStageRefObjId = null; - ObjectId newObjId = null; - String branchName = null; + List<ChangeData> changes_to_cherrypick = null; + ObjectId oldStageRef = null; + ObjectId branchRef = null; + ObjectId newStageRef = null; + ObjectId integratingRef = null; + String stagingBranchName = null; try { - branchName = stagingBranchKey.get(); - oldStageRefObjId = git.resolve(branchName); - branchObjId = git.resolve(destBranchShortKey.get()); + stagingBranchName = stagingBranchKey.get(); + oldStageRef = git.resolve(stagingBranchName); + branchRef = git.resolve(destBranchShortKey.get()); query = queryProvider.get(); - List<ChangeData> unsorted_list = query.byBranchStatus(destBranchShortKey, Change.Status.INTEGRATING); - changes_integrating = arrangeOrderLikeInRef(git, oldStageRefObjId, branchObjId, unsorted_list); + changes_integrating = query.byBranchStatus(destBranchShortKey, Change.Status.INTEGRATING); query = queryProvider.get(); - unsorted_list = query.byBranchStatus(destBranchShortKey, Change.Status.STAGED); - changes_staged = arrangeOrderLikeInRef(git, oldStageRefObjId, branchObjId, unsorted_list); + changes_staged = query.byBranchStatus(destBranchShortKey, Change.Status.STAGED); } catch (IOException e) { - logger.atSevere().log("qtcodereview: rebuild staging ref %s db failed. IOException %s", + logger.atSevere().log("qtcodereview: rebuild staging ref %s db query failed. Exception %s", stagingBranchKey, e); - throw new MergeConflictException("fatal: IOException"); + throw new MergeConflictException("fatal: " + e.getMessage()); } try { logger.atInfo().log("qtcodereview: rebuild staging ref reset %s back to %s", stagingBranchKey, destBranchShortKey); Result result = QtUtil.createStagingBranch(git, destBranchShortKey); - if (result == null) throw new NoSuchRefException("Cannot create staging ref: " + branchName); - logger.atInfo().log("qtcodereview: rebuild staging ref reset result %s", result); - newStageRefObjId = git.resolve(branchName); - } catch (NoSuchRefException e) { - logger.atSevere().log("qtcodereview: rebuild staging ref reset %s failed. No such ref %s", - stagingBranchKey, e); - throw new MergeConflictException("fatal: NoSuchRefException"); - } catch (IOException e) { - logger.atSevere().log("qtcodereview: rebuild staging ref reset %s failed. IOException %s", - stagingBranchKey, e); - throw new MergeConflictException("fatal: IOException"); + if (result == null) throw new NoSuchRefException("Cannot create staging ref: " + stagingBranchName); + logger.atInfo().log("qtcodereview: rebuild staging ref reset to %s with result %s", branchRef, result); + integratingRef = findIntegrationHead(git, oldStageRef, branchRef, changes_integrating); + logger.atInfo().log("qtcodereview: rebuild staging integration ref is %s", integratingRef); + newStageRef = findReusableStagingHead(git, oldStageRef, integratingRef, changes_staged); + logger.atInfo().log("qtcodereview: rebuild staging reused staging ref is %s", newStageRef); + changes_to_cherrypick = arrangeOrderLikeInRef(git, oldStageRef, newStageRef, changes_staged); + } catch (NoSuchRefException | IOException e) { + logger.atSevere().log("qtcodereview: rebuild staging ref reset %s failed. Exception %s", + stagingBranchKey, e); + throw new MergeConflictException("fatal: " + e.getMessage()); } try { - newObjId = pickChangesToStagingRef(git, projectKey, changes_integrating, newStageRefObjId); - newStageRefObjId = newObjId; + newStageRef = pickChangesToStagingRef(git, projectKey, changes_to_cherrypick, newStageRef); } catch(Exception e) { - logger.atSevere().log("qtcodereview: rebuild staging ref %s. failed to cherry pick integrating changes %s", - stagingBranchKey, e); - newObjId = null; - } - - if (newObjId != null) { - try { - newObjId = pickChangesToStagingRef(git, projectKey, changes_staged, newObjId); - newStageRefObjId = newObjId; - } catch(Exception e) { - newObjId = null; - logger.atInfo().log("qtcodereview: rebuild staging ref %s merge conflict", stagingBranchKey); - String message = "Merge conflict in staging branch. Status changed back to new. Please stage again."; - QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, message, null, null, null); - try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) { - for (ChangeData item: changes_staged) { - Change change = item.change(); - logger.atInfo().log("qtcodereview: staging ref rebuild merge conflict. Change %s back to NEW", change); - u.addOp(change.getId(), op); - } - u.execute(); - } catch (UpdateException | RestApiException ex) { - logger.atSevere().log("qtcodereview: staging ref rebuild. Failed to update change status %s", ex); + logger.atInfo().log("qtcodereview: rebuild staging ref %s merge conflict", stagingBranchKey); + newStageRef = integratingRef; + String message = "Merge conflict in staging branch. Status changed back to new. Please stage again."; + QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, message, null, null, null); + try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) { + for (ChangeData item: changes_staged) { + Change change = item.change(); + logger.atInfo().log("qtcodereview: staging ref rebuild merge conflict. Change %s back to NEW", change); + u.addOp(change.getId(), op); } + u.execute(); + } catch (UpdateException | RestApiException ex) { + logger.atSevere().log("qtcodereview: staging ref rebuild. Failed to update change status %s", ex); } } try { - RefUpdate refUpdate = git.updateRef(branchName); - refUpdate.setNewObjectId(newStageRefObjId); + RefUpdate refUpdate = git.updateRef(stagingBranchName); + refUpdate.setNewObjectId(newStageRef); refUpdate.update(); // send ref updated event only if it changed - if (!newStageRefObjId.equals(oldStageRefObjId)) { - referenceUpdated.fire(projectKey, branchName, oldStageRefObjId, - newStageRefObjId, user.state()); + if (!newStageRef.equals(oldStageRef)) { + referenceUpdated.fire(projectKey, stagingBranchName, oldStageRef, newStageRef, user.state()); } } catch (IOException e) { logger.atSevere().log("qtcodereview: rebuild %s failed to update ref %s", stagingBranchKey, e); @@ -468,7 +521,14 @@ public class QtUtil { Iterator<RevCommit> i = revWalk.iterator(); while (i.hasNext()) { RevCommit commit = i.next(); - List<ChangeData> changes = queryProvider.get().byBranchCommit(destination, commit.name()); + String changeId = getChangeId(commit); + List<ChangeData> changes = null; + + if (changeId != null) { + Change.Key key = new Change.Key(changeId); + changes = queryProvider.get().byBranchKey(destination, key); + } + if (changes != null && !changes.isEmpty()) { if (changes.size() > 1) logger.atWarning().log("qtcodereview: commit belongs to multiple changes: %s", commit.name()); ChangeData cd = changes.get(0); @@ -517,7 +577,7 @@ public class QtUtil { final CommitBuilder mergeCommit = new CommitBuilder(); mergeCommit.setTreeId(merger.getResultTreeId()); - mergeCommit.setParentIds(mergeTip, toMerge); + mergeCommit.setParentIds(mergeTip, toMerge); // important: mergeTip must be parent index 0 mergeCommit.setAuthor(toMerge.getAuthorIdent()); mergeCommit.setCommitter(committerIdent); mergeCommit.setMessage(message); @@ -570,6 +630,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/QtCodeReviewIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java index ec57b33..9c3be66 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java @@ -14,8 +14,11 @@ import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseSsh; +import com.google.gerrit.common.FooterConstants; +import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; @@ -96,7 +99,10 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { RestResponse response = call_REST_API_Stage(c.getChangeId(), c.getCommit().getName()); response.assertOK(); Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(change); + String branch = getBranchNameFromRef(change.getDest().get()); + RevCommit stagingHead = getRemoteHead(project, R_STAGING + branch); + assertReviewedByFooter(stagingHead, true); resetEvents(); } @@ -109,8 +115,7 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { protected void QtUnStage(PushOneCommit.Result c) throws Exception { RestResponse response = call_REST_API_UnStage(c.getChangeId(), getCurrentPatchId(c)); response.assertOK(); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); } protected RestResponse call_REST_API_UnStage(String changeId, String revisionId) throws Exception { @@ -127,6 +132,8 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { commandStr += " --build-id " + buildId; String resultStr = adminSshSession.exec(commandStr); assertThat(adminSshSession.getError()).isNull(); + RevCommit buildHead = getRemoteHead(project, R_BUILDS + buildId); + assertReviewedByFooter(buildHead, true); resetEvents(); } @@ -140,6 +147,8 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { commandStr += " --message " + BUILD_PASS_MESSAGE; String resultStr = adminSshSession.exec(commandStr); assertThat(adminSshSession.getError()).isNull(); + RevCommit branchHead = getRemoteHead(project, R_HEADS + branch); + assertReviewedByFooter(branchHead, true); } protected void QtFailBuild(String branch, String buildId) throws Exception { @@ -161,8 +170,7 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { throws Exception { String pushRef = R_PUSH + branch; PushOneCommit.Result c = createUserChange(pushRef, message, file, content); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); return c; } @@ -185,32 +193,87 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest { return result; } - protected void assertCherryPick(RevCommit head, RevCommit source, String cherrypickSHA) { - assertThat(head.getName()).isEqualTo(cherrypickSHA); + protected void assertCherryPick(RevCommit head, RevCommit source, RevCommit base) { assertThat(head).isNotEqualTo(source); - assertThat(cherrypickSHA).isNotEqualTo(source.getName()); + assertThat(head.getName()).isNotEqualTo(source.getName()); assertThat(head.getShortMessage()).isEqualTo(source.getShortMessage()); assertThat(head.getFooterLines("Change-Id")).isEqualTo(source.getFooterLines("Change-Id")); assertThat(head.getParentCount()).isEqualTo(1); + + if (base != null) assertThat(head.getParent(0)).isEqualTo(base); } - protected void assertApproval(String changeId, TestAccount user) throws Exception { - ChangeInfo c = gApi.changes().id(changeId).get(DETAILED_LABELS, CURRENT_REVISION, CURRENT_COMMIT); + private void assertStatus(Change change, ChangeStatus status, boolean approved, boolean footer) + throws Exception { + ChangeInfo cf; - String label = "Code-Review"; - int expectedVote = 2; + if (approved) { + cf = gApi.changes().id(change.getChangeId()).get(DETAILED_LABELS, CURRENT_REVISION, CURRENT_COMMIT); + Integer vote = getLabelValue(cf, "Code-Review", admin); + assertThat(vote).named("label = Code-Review").isEqualTo(2); + } else { + cf = gApi.changes().id(change.getChangeId()).get(CURRENT_REVISION, CURRENT_COMMIT); + } + + assertThat(cf.status).isEqualTo(status); + String commitMsg = cf.revisions.get(cf.currentRevision).commit.message; + + if (footer && cf.revisions.get(cf.currentRevision).commit.parents.size() == 1) { + assertThat(commitMsg).contains("Reviewed-by"); + } else { + assertThat(commitMsg).doesNotContain("Reviewed-by"); + } + } + + protected void assertStatusNew(Change change) throws Exception { + assertStatus(change, ChangeStatus.NEW, false, false); + } + + protected void assertStatusStaged(Change change) throws Exception { + assertStatus(change, ChangeStatus.STAGED, true, false); + } + + protected void assertStatusIntegrating(Change change) throws Exception { + assertStatus(change, ChangeStatus.INTEGRATING, true, false); + } + + protected void assertStatusMerged(Change change) throws Exception { + assertStatus(change, ChangeStatus.MERGED, true, true); + } + + private Integer getLabelValue(ChangeInfo c, String label, TestAccount user) { Integer vote = 0; - if (c.labels.get(label) != null && c.labels.get(label).all != null) { - for (ApprovalInfo approval : c.labels.get(label).all) { + LabelInfo labels = c.labels.get(label); + + if (labels != null && labels.all != null) { + for (ApprovalInfo approval : labels.all) { if (approval._accountId == user.id().get()) { vote = approval.value; break; } } } + return vote; + } + + protected void assertApproval(String changeId, TestAccount user) throws Exception { + ChangeInfo c = gApi.changes().id(changeId).get(DETAILED_LABELS, CURRENT_REVISION, CURRENT_COMMIT); + + String label = "Code-Review"; + int expectedVote = 2; + Integer vote = 0; + + vote = getLabelValue(c, label, user); + assertThat(vote).named("label = " + label).isEqualTo(expectedVote); + } + + protected void assertReviewedByFooter(RevCommit commit, boolean exists) { + + // Skip initial commit and merge commits + if (commit.getParentCount() != 1) return; - String name = "label = " + label; - assertThat(vote).named(name).isEqualTo(expectedVote); + List<String> changeIds = commit.getFooterLines(FooterConstants.REVIEWED_BY); + assertThat(!changeIds.isEmpty()).isEqualTo(exists); } protected void assertRefUpdatedEvents(String refName, RevCommit ... expected) throws Exception { 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 1c35265..22b55cd 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java @@ -71,10 +71,8 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { QtNewBuild("master", "test-build-101"); RevCommit updatedHead = qtApproveBuild("master", "test-build-101", c3, null); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED); + assertStatusMerged(c1.getChange().change()); + assertStatusMerged(c2.getChange().change()); } @Test @@ -108,10 +106,8 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { QtNewBuild("master", "test-build-201"); RevCommit updatedHead = qtFailBuild("master", "test-build-201", c3, initialHead); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c1.getChange().change()); + assertStatusNew(c2.getChange().change()); } @Test @@ -196,6 +192,44 @@ 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 + + assertStatusNew(c.getChange().change()); + Change 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()); @@ -244,11 +278,12 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { RevCommit buildHead = getRemoteHead(project, buildRef); assertThat(buildHead.getId()).isNotNull(); // build ref is still there + assertReviewedByFooter(buildHead, true); RevCommit updatedHead = getRemoteHead(project, branchRef); if (expectedContent != null && expectedHead == null) { RevCommit commit = expectedContent.getCommit(); - assertCherryPick(updatedHead, commit, getCurrentPatchSHA(expectedContent)); + assertCherryPick(updatedHead, commit, null); expectedHead = updatedHead; } else { assertThat(updatedHead).isEqualTo(expectedHead); // master is updated @@ -260,8 +295,7 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { assertRefUpdatedEvents(branchRef, initialHead, expectedHead); resetEvents(); - Change change = expectedContent.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED); + assertStatusMerged(expectedContent.getChange().change()); ArrayList<ChangeMessage> messages = new ArrayList(expectedContent.getChange().messages()); assertThat(messages.get(messages.size()-1).getMessage()).isEqualTo(MERGED_MSG); // check last message @@ -296,7 +330,7 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { RevCommit stagingHead = getRemoteRefHead(project, stagingRef); if (c != null && expectedStagingHead == null) { - assertCherryPick(stagingHead, c.getCommit(), getCurrentPatchSHA(c)); + assertCherryPick(stagingHead, c.getCommit(), null); expectedStagingHead = stagingHead; } assertThat(stagingHead).isEqualTo(expectedStagingHead); // staging is rebuild @@ -306,8 +340,7 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT { assertRefUpdatedEvents(stagingRef, stagingHeadOld, stagingHead); // staging is rebuild - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); ArrayList<ChangeMessage> messages = new ArrayList<ChangeMessage>(c.getChange().messages()); assertThat(messages.get(messages.size()-1).getMessage()).isEqualTo(FAILED_MSG); // check last message diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuildIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuildIT.java index f4d3efb..db07c8d 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuildIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuildIT.java @@ -66,10 +66,8 @@ public class QtCommandNewBuildIT extends QtCodeReviewIT { QtStage(c3); RevCommit buildHead = qtNewBuild("master", "test-build-101", c3, null); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(c1.getChange().change()); + assertStatusIntegrating(c2.getChange().change()); } @@ -184,18 +182,18 @@ public class QtCommandNewBuildIT extends QtCodeReviewIT { assertThat(stagingHead.getId()).isNotEqualTo(initialHead.getId()); // staging is not master if (expectedHead == null) { - assertCherryPick(stagingHead, c.getCommit(), getCurrentPatchSHA(c)); + assertCherryPick(stagingHead, c.getCommit(), null); expectedHead = stagingHead; } RevCommit buildHead = getRemoteHead(project, buildRef); assertThat(buildHead).isEqualTo(expectedHead); // build ref is updated assertRefUpdatedEvents(buildRef, null, expectedHead); + assertReviewedByFooter(buildHead, true); resetEvents(); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(c.getChange().change()); ArrayList<ChangeMessage> messages = new ArrayList(c.getChange().messages()); assertThat(messages.get(messages.size() - 1).getMessage()).contains(BUILDING_MSG + buildId); // check last message @@ -227,5 +225,4 @@ public class QtCommandNewBuildIT extends QtCodeReviewIT { return adminSshSession.getError(); } - } diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandRebuildStagingIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandRebuildStagingIT.java index 233aa5b..637b9fa 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandRebuildStagingIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandRebuildStagingIT.java @@ -51,8 +51,9 @@ public class QtCommandRebuildStagingIT extends QtCodeReviewIT { QtStage(c1); QtStage(c2); QtStage(c3); + RevCommit stagingExpected = getRemoteHead(project, R_STAGING + "master"); - RevCommit stagingHead = qtRebuildStaging("master", c3, null); + RevCommit stagingHead = qtRebuildStaging("master", null, stagingExpected); } @Test @@ -71,8 +72,9 @@ public class QtCommandRebuildStagingIT extends QtCodeReviewIT { QtStage(c2); QtNewBuild("master", "test-build-250"); QtStage(c3); + RevCommit stagingExpected = getRemoteHead(project, R_STAGING + "master"); - RevCommit stagingHead = qtRebuildStaging("master", c3, null); + RevCommit stagingHead = qtRebuildStaging("master", null, stagingExpected); } @Test @@ -135,9 +137,11 @@ public class QtCommandRebuildStagingIT extends QtCodeReviewIT { assertThat(updatedHead.getId()).isEqualTo(initialHead.getId()); // master is not updated RevCommit stagingHead = getRemoteRefHead(project, stagingRef); + assertReviewedByFooter(stagingHead, true); - if (expectedStagingHead==null && expectedContent != null) { - assertCherryPick(stagingHead, expectedContent.getCommit(), getCurrentPatchSHA(expectedContent)); + + if (expectedStagingHead == null && expectedContent != null) { + assertCherryPick(stagingHead, expectedContent.getCommit(), null); expectedStagingHead = stagingHead; } else { assertThat(stagingHead).isEqualTo(expectedStagingHead); // staging is updated diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandStageIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandStageIT.java index 8f0171f..a9e381b 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandStageIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandStageIT.java @@ -160,7 +160,7 @@ public class QtCommandStageIT extends QtCodeReviewIT { Change change = c.getChange().change(); if (expectFail) assertThat(change.getStatus()).isNotEqualTo(Change.Status.STAGED); - else assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + else assertStatusStaged(change); return adminSshSession.getError(); } diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommitFooterIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommitFooterIT.java index fda4ba1..952d170 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommitFooterIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommitFooterIT.java @@ -3,8 +3,8 @@ package com.googlesource.gerrit.plugins.qtcodereview; import static com.google.common.truth.Truth.assertThat; -import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS; -import static com.google.gerrit.extensions.client.ListChangesOption.COMMIT_FOOTERS; +import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT; +import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.project.testing.Util.category; import static com.google.gerrit.server.project.testing.Util.value; @@ -75,8 +75,8 @@ public class QtCommitFooterIT extends LightweightPluginDaemonTest { requestScopeOperations.setApiUser(admin.id()); gApi.changes().id(change.getChangeId()).current().submit(); - ChangeInfo cf = gApi.changes().id(change.getChangeId()).get(ALL_REVISIONS, COMMIT_FOOTERS); - String commitMsg = cf.revisions.get(cf.currentRevision).commitWithFooters; + ChangeInfo cf = gApi.changes().id(change.getChangeId()).get(CURRENT_REVISION, CURRENT_COMMIT); + String commitMsg = cf.revisions.get(cf.currentRevision).commit.message; assertThat(commitMsg).contains("Reviewed-by"); assertThat(commitMsg).doesNotContain("Reviewed-on"); assertThat(commitMsg).doesNotContain("Sanity-Review"); diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommonFlowsIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommonFlowsIT.java index fd35c6a..da48465 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommonFlowsIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommonFlowsIT.java @@ -55,17 +55,14 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { c = pushCommit("master", "no content", "afile", ""); approve(c.getChangeId()); QtStage(c); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c.getChange().change()); RevCommit stagingHead = getRemoteHead(project, "refs/staging/master"); QtNewBuild("master", "master-build-000"); - change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(c.getChange().change()); QtApproveBuild("master", "master-build-000"); - change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED); + assertStatusMerged(c.getChange().change()); String gitLog = getRemoteLog("refs/staging/master").toString(); assertThat(gitLog).contains(stagingHead.getId().name()); @@ -96,16 +93,13 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { m.assertOkStatus(); approve(m.getChangeId()); QtStage(m); - Change change = m.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(m.getChange().change()); QtNewBuild("master", "master-build-001"); - change = m.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(m.getChange().change()); QtApproveBuild("master", "master-build-001"); - change = m.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.MERGED); + assertStatusMerged(m.getChange().change()); // check that all commits are in staging ref String gitLog = getRemoteLog("refs/heads/master").toString(); @@ -134,17 +128,13 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { QtStage(c3); QtNewBuild("master", "master-build-100"); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusIntegrating(c1.getChange().change()); + assertStatusStaged(c2.getChange().change()); QtNewBuild("5.12", "5.12-build-100"); QtApproveBuild("5.12", "5.12-build-100"); - change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(c1.getChange().change()); + assertStatusIntegrating(c3.getChange().change()); QtApproveBuild("master", "master-build-100"); RevCommit branchHeadMaster = getRemoteHead(project, R_HEADS + "master"); @@ -170,17 +160,13 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { QtStage(c3); QtNewBuild("5.12", "5.12-build-101"); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusIntegrating(c1.getChange().change()); + assertStatusStaged(c3.getChange().change()); QtNewBuild("master", "master-build-101"); QtFailBuild("master", "master-build-101"); - change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); + assertStatusIntegrating(c1.getChange().change()); + assertStatusIntegrating(c2.getChange().change()); QtFailBuild("5.12", "5.12-build-101"); RevCommit branchHeadMaster = getRemoteHead(project, R_HEADS + "master"); @@ -202,8 +188,7 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { QtStage(c2); QtUnStage(c1); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); QtUnStage(c2); RevCommit stagingHeadMaster = getRemoteHead(project, R_STAGING + "master"); @@ -221,18 +206,15 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { approve(c2.getChangeId()); QtStage(c1); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtNewBuild("master", "master-build-401"); RevCommit build1Head = getRemoteHead(project, R_BUILDS + "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtStage(c2); QtApproveBuild("master", "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); QtNewBuild("master", "master-build-402"); RevCommit build2Head = getRemoteHead(project, R_BUILDS + "master-build-402"); @@ -256,18 +238,15 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { approve(c2.getChangeId()); QtStage(c1); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtNewBuild("master", "master-build-401"); RevCommit build1Head = getRemoteHead(project, R_BUILDS + "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtStage(c2); QtApproveBuild("master", "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); QtNewBuild("master", "master-build-402"); RevCommit build2Head = getRemoteHead(project, R_BUILDS + "master-build-402"); @@ -291,24 +270,20 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { approve(c2.getChangeId()); QtStage(c1); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtNewBuild("master", "master-build-401"); RevCommit build1Head = getRemoteHead(project, R_BUILDS + "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtStage(c2); QtFailBuild("master", "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); QtNewBuild("master", "master-build-402"); RevCommit build2Head = getRemoteHead(project, R_BUILDS + "master-build-402"); QtApproveBuild("master", "master-build-402"); - change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c1.getChange().change());; String gitLog = getRemoteLog("refs/heads/master").toString(); assertThat(gitLog).doesNotContain(build1Head.getId().name()); @@ -328,17 +303,14 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { approve(c2.getChangeId()); QtStage(c1); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtNewBuild("master", "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); QtStage(c2); QtFailBuild("master", "master-build-401"); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); QtNewBuild("master", "master-build-402"); QtFailBuild("master", "master-build-402"); @@ -348,6 +320,32 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { } @Test + public void unStageWhileBuilding() throws Exception { + RevCommit initialHead = getRemoteHead(); + PushOneCommit.Result c1 = pushCommit("master", "commitmsg1", "file1", "content1"); + approve(c1.getChangeId()); + QtStage(c1); + QtNewBuild("master", "master-build-402"); + + PushOneCommit.Result c2 = pushCommit("master", "commitmsg2", "file2", "content2"); + approve(c2.getChangeId()); + QtStage(c2); + RevCommit stagingExpected = getRemoteHead(project, R_STAGING + "master"); + + PushOneCommit.Result c3 = pushCommit("master", "commitmsg3", "file3", "content3"); + approve(c3.getChangeId()); + QtStage(c3); + + QtUnStage(c3); + RevCommit stagingHead = getRemoteHead(project, R_STAGING + "master"); + assertThat(stagingHead).isEqualTo(stagingExpected); + + assertStatusIntegrating(c1.getChange().change()); + assertStatusStaged(c2.getChange().change()); + assertStatusNew(c3.getChange().change()); + } + + @Test public void unStage_MergeConflict_While_Building() throws Exception { RevCommit initialHead = getRemoteHead(); PushOneCommit.Result c1 = pushCommit("master", "commitmsg1", "file1", "content1"); @@ -371,14 +369,10 @@ public class QtCommonFlowsIT extends QtCodeReviewIT { QtStage(c4); QtUnStage(c3); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.INTEGRATING); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); - change = c4.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusIntegrating(c1.getChange().change()); + assertStatusNew(c2.getChange().change()); + assertStatusNew(c3.getChange().change()); + assertStatusNew(c4.getChange().change()); RevCommit stagingHeadMaster = getRemoteHead(project, R_STAGING + "master"); assertThat(stagingHeadMaster.getId()).isEqualTo(stagingExpected.getId()); diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java index b785c59..e4ef0f2 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtEmailSendingIT.java @@ -113,7 +113,7 @@ public class QtEmailSendingIT extends QtCodeReviewIT { private void QtStageByAdmin(PushOneCommit.Result c) throws Exception { RestResponse response = call_REST_API_Stage_By_Admin(c.getChangeId(), c.getCommit().getName()); response.assertOK(); - assertThat(c.getChange().change().getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c.getChange().change()); } private RestResponse call_REST_API_Stage_By_Admin(String changeId, String revisionId) throws Exception { diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpenIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpenIT.java index 0a1710e..5b1ca54 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpenIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpenIT.java @@ -60,8 +60,7 @@ public class QtReOpenIT extends QtCodeReviewIT { String url = "/changes/" + changeId + "/gerrit-plugin-qt-workflow~reopen"; RestResponse response = adminRestSession.post(url, restoreInput); response.assertOK(); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); } @Test @@ -100,8 +99,7 @@ public class QtReOpenIT extends QtCodeReviewIT { assertRefUpdatedEvents(masterRef); // no events assertRefUpdatedEvents(stagingRef); // no events - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); return masterHead; } diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtStageIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtStageIT.java index 626032d..8e07afa 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtStageIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtStageIT.java @@ -73,8 +73,8 @@ public class QtStageIT extends QtCodeReviewIT { approve(c3.getChangeId()); stagingHead = qtStage(c1); - stagingHead = qtStage(c2); - stagingHead = qtStage(c3); + stagingHead = qtStage(c2, stagingHead); + stagingHead = qtStage(c3, stagingHead); } @Test @@ -101,7 +101,42 @@ public class QtStageIT extends QtCodeReviewIT { PushOneCommit.Result m = mm.to("refs/for/master"); m.assertOkStatus(); approve(m.getChangeId()); - RevCommit stagingHead = qtStageExpectMerge(m); + RevCommit stagingHead = qtStageExpectMergeFastForward(m); + + // check that all commits are in staging ref + String gitLog = getRemoteLog("refs/staging/master").toString(); + assertThat(gitLog).contains(initialHead.getId().name()); + assertThat(gitLog).contains(c1.getCommit().getId().name()); + assertThat(gitLog).contains(f1.getCommit().getId().name()); + assertThat(gitLog).contains(f2.getCommit().getId().name()); + assertThat(gitLog).contains(m.getCommit().getId().name()); + } + + @Test + public void mergeCommit_Stage_ExpectMergeOfMerge() throws Exception { + RevCommit initialHead = getRemoteHead(); + + // make changes on feature branch + PushOneCommit.Result f1 = pushCommit("feature", "commitmsg1", "file1", "content1"); + PushOneCommit.Result f2 = pushCommit("feature", "commitmsg2", "file2", "content2"); + approve(f1.getChangeId()); + gApi.changes().id(f1.getChangeId()).current().submit(); + approve(f2.getChangeId()); + gApi.changes().id(f2.getChangeId()).current().submit(); + + // make a change on master branch + testRepo.reset(initialHead); + PushOneCommit.Result c1 = pushCommit("master", "commitmsg3", "file3", "content3"); + approve(c1.getChangeId()); + gApi.changes().id(c1.getChangeId()).current().submit(); + + // merge feature branch into master + PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo); + mm.setParents(ImmutableList.of(f2.getCommit(), c1.getCommit())); + PushOneCommit.Result m = mm.to("refs/for/master"); + m.assertOkStatus(); + approve(m.getChangeId()); + RevCommit stagingHead = qtStageExpectMergeOfMerge(m); // check that all commits are in staging ref String gitLog = getRemoteLog("refs/staging/master").toString(); @@ -123,7 +158,7 @@ public class QtStageIT extends QtCodeReviewIT { // no changes in this commit c = pushCommit("master", "no content", "afile", ""); approve(c.getChangeId()); - stagingHead = qtStage(c); + stagingHead = qtStage(c, stagingHead); assertApproval(c.getChangeId(), admin); } @@ -219,19 +254,26 @@ public class QtStageIT extends QtCodeReviewIT { RestResponse response = qtStageExpectFail(c2, initialHead, stagingHead1, HttpStatus.SC_CONFLICT); assertThat(response.getEntityContent()).contains("merge conflict"); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); } private RevCommit qtStage(PushOneCommit.Result c) throws Exception { - return qtStage(c, false); + return qtStage(c, false, false, null); + } + + private RevCommit qtStage(PushOneCommit.Result c, RevCommit base) throws Exception { + return qtStage(c, false, false, base); + } + + private RevCommit qtStageExpectMergeFastForward(PushOneCommit.Result c) throws Exception { + return qtStage(c, true, true, null); } - private RevCommit qtStageExpectMerge(PushOneCommit.Result c) throws Exception { - return qtStage(c, true); + private RevCommit qtStageExpectMergeOfMerge(PushOneCommit.Result c) throws Exception { + return qtStage(c, true, false, null); } - private RevCommit qtStage(PushOneCommit.Result c, boolean merge) throws Exception { + private RevCommit qtStage(PushOneCommit.Result c, boolean merge, boolean fastForward, RevCommit base) throws Exception { String branch = getBranchNameFromRef(c.getChange().change().getDest().get()); String stagingRef = R_STAGING + branch; String branchRef = R_HEADS + branch; @@ -247,19 +289,21 @@ public class QtStageIT extends QtCodeReviewIT { assertThat(branchHead.getId()).isEqualTo(initialHead.getId()); // master is not updated RevCommit stagingHead = getRemoteRefHead(project, stagingRef); + assertReviewedByFooter(stagingHead, true); - if (merge) { + if (fastForward) { + assertThat(stagingHead).isEqualTo(originalCommit); + } else if (merge) { assertThat(stagingHead.getParentCount()).isEqualTo(2); assertThat(stagingHead.getParent(1)).isEqualTo(originalCommit); } else { - assertCherryPick(stagingHead, originalCommit, getCurrentPatchSHA(c)); + assertCherryPick(stagingHead, originalCommit, base); } assertThat(stagingHead.getParent(0)).isEqualTo(oldStagingHead); assertRefUpdatedEvents(stagingRef, oldStagingHead, stagingHead); resetEvents(); - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c.getChange().change()); ArrayList<ChangeMessage> messages = new ArrayList(c.getChange().messages()); assertThat(messages.get(messages.size()-1).getMessage()).isEqualTo(STAGED_MSG); // check last message diff --git a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStageIT.java b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStageIT.java index 4c09d65..291fb96 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStageIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStageIT.java @@ -54,6 +54,21 @@ public class QtUnStageIT extends QtCodeReviewIT { } @Test + public void multiChange_UnStage_NoFastForwardOnStagingRefRebuild() throws Exception { + RevCommit initialHead = getRemoteHead(); + PushOneCommit.Result c1 = pushCommit("master", "commitmsg1", "file1", "content1"); + approve(c1.getChangeId()); + QtStage(c1); + + testRepo.reset(initialHead); + PushOneCommit.Result c2 = pushCommit("master", "commitmsg2", "file2", "content2"); + approve(c2.getChangeId()); + QtStage(c2); + + RevCommit stagingHead = qtUnStageExpectCherryPick(c1, c2); + } + + @Test public void multiChange_UnStage_First() throws Exception { // Push 3 independent commits RevCommit initialHead = getRemoteHead(); @@ -71,10 +86,8 @@ public class QtUnStageIT extends QtCodeReviewIT { QtStage(c3); RevCommit stagingHead = qtUnStageExpectCherryPick(c1, c3); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c2.getChange().change()); + assertStatusStaged(c3.getChange().change()); } @Test @@ -95,10 +108,8 @@ public class QtUnStageIT extends QtCodeReviewIT { QtStage(c3); RevCommit stagingHead = qtUnStageExpectCherryPick(c2, c3); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c1.getChange().change()); + assertStatusStaged(c3.getChange().change()); } @Test @@ -120,10 +131,8 @@ public class QtUnStageIT extends QtCodeReviewIT { QtStage(c3); RevCommit stagingHead = qtUnStageExpectCommit(c3, expectedFastForward); - Change change = c1.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); - change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.STAGED); + assertStatusStaged(c1.getChange().change()); + assertStatusStaged(c2.getChange().change()); } @Test @@ -143,6 +152,7 @@ public class QtUnStageIT extends QtCodeReviewIT { PushOneCommit.Result c1 = pushCommit("master", "commitmsg3", "file3", "content3"); approve(c1.getChangeId()); gApi.changes().id(c1.getChangeId()).current().submit(); + RevCommit stagingHead = getRemoteHead(project, R_HEADS + "master"); // merge feature branch into master PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo); @@ -158,7 +168,7 @@ public class QtUnStageIT extends QtCodeReviewIT { approve(c2.getChangeId()); QtStage(c2); - RevCommit stagingHead = qtUnStageExpectCherryPick(m, c2); + stagingHead = qtUnStageExpectCherryPick(m, c2, stagingHead); String gitLog = getRemoteLog("refs/staging/master").toString(); assertThat(gitLog).contains(initialHead.getId().name()); assertThat(gitLog).contains(c1.getCommit().getId().name()); @@ -169,7 +179,7 @@ public class QtUnStageIT extends QtCodeReviewIT { } @Test - public void multiChange_UnStage_Before_MergeCommit() throws Exception { + public void multiChange_UnStage_Before_MergeCommit_ExpectFastForward() throws Exception { RevCommit initialHead = getRemoteHead(); // make changes on feature branch @@ -197,10 +207,54 @@ public class QtUnStageIT extends QtCodeReviewIT { mm.setParents(ImmutableList.of(c1.getCommit(), f2.getCommit())); PushOneCommit.Result m = mm.to("refs/for/master"); m.assertOkStatus(); + RevCommit originalMergeCommit = m.getCommit(); + approve(m.getChangeId()); + QtStage(m); + + RevCommit stagingHead = qtUnStageExpectMergeFastForward(c2, originalMergeCommit); + String gitLog = getRemoteLog("refs/staging/master").toString(); + assertThat(gitLog).contains(initialHead.getId().name()); + assertThat(gitLog).contains(c1.getCommit().getId().name()); + assertThat(gitLog).contains(stagingHead.getId().name()); + assertThat(gitLog).contains(f1.getCommit().getId().name()); + assertThat(gitLog).contains(f2.getCommit().getId().name()); + assertThat(gitLog).contains(m.getCommit().getId().name()); + assertThat(gitLog).doesNotContain(c2.getCommit().getId().name()); + } + + @Test + public void multiChange_UnStage_Before_MergeCommit_ExpectMergeOfMerge() throws Exception { + RevCommit initialHead = getRemoteHead(); + + // make changes on feature branch + PushOneCommit.Result f1 = pushCommit("feature", "commitmsg1", "file1", "content1"); + PushOneCommit.Result f2 = pushCommit("feature", "commitmsg2", "file2", "content2"); + approve(f1.getChangeId()); + gApi.changes().id(f1.getChangeId()).current().submit(); + approve(f2.getChangeId()); + gApi.changes().id(f2.getChangeId()).current().submit(); + + // make a change on master branch + testRepo.reset(initialHead); + PushOneCommit.Result c1 = pushCommit("master", "commitmsg3", "file3", "content3"); + approve(c1.getChangeId()); + gApi.changes().id(c1.getChangeId()).current().submit(); + + // Stage a change + testRepo.reset(initialHead); + PushOneCommit.Result c2 = pushCommit("master", "commitmsg4", "file4", "content4"); + approve(c2.getChangeId()); + QtStage(c2); + + // merge feature branch and stage it on top + PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo); + mm.setParents(ImmutableList.of(f2.getCommit(),c1.getCommit())); + PushOneCommit.Result m = mm.to("refs/for/master"); + m.assertOkStatus(); approve(m.getChangeId()); QtStage(m); - RevCommit stagingHead = qtUnStageExpectMerge(c2, m); + RevCommit stagingHead = qtUnStageExpectMergeOfMerge(c2, m); String gitLog = getRemoteLog("refs/staging/master").toString(); assertThat(gitLog).contains(initialHead.getId().name()); assertThat(gitLog).contains(c1.getCommit().getId().name()); @@ -228,10 +282,8 @@ public class QtUnStageIT extends QtCodeReviewIT { QtStage(c3); RevCommit stagingHead = qtUnStageExpectCommit(c1, initialHead); - Change change = c2.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); - change = c3.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c2.getChange().change()); + assertStatusNew(c3.getChange().change()); } @Test @@ -295,25 +347,40 @@ public class QtUnStageIT extends QtCodeReviewIT { private RevCommit qtUnStageExpectCherryPick(PushOneCommit.Result c, PushOneCommit.Result expectedContent) throws Exception { - return qtUnStage(c, null, expectedContent, false); + return qtUnStage(c, null, expectedContent, null, false, false); + } + + private RevCommit qtUnStageExpectCherryPick(PushOneCommit.Result c, + PushOneCommit.Result expectedContent, + RevCommit base) + throws Exception { + return qtUnStage(c, null, expectedContent, base, false, false); } private RevCommit qtUnStageExpectCommit(PushOneCommit.Result c, RevCommit expectedStagingHead) throws Exception { - return qtUnStage(c, expectedStagingHead, null, false); + return qtUnStage(c, expectedStagingHead, null, null, false, false); } - private RevCommit qtUnStageExpectMerge(PushOneCommit.Result c, - PushOneCommit.Result expectedContent) - throws Exception { - return qtUnStage(c, null, expectedContent, true); + private RevCommit qtUnStageExpectMergeFastForward(PushOneCommit.Result c, + RevCommit expectedStagingHead) + throws Exception { + return qtUnStage(c, expectedStagingHead, null, null, true, true); + } + + private RevCommit qtUnStageExpectMergeOfMerge(PushOneCommit.Result c, + PushOneCommit.Result expectedContent) + throws Exception { + return qtUnStage(c, null, expectedContent, null, true, false); } private RevCommit qtUnStage(PushOneCommit.Result c, RevCommit expectedStagingHead, PushOneCommit.Result expectedContent, - boolean merge) + RevCommit base, + boolean merge, + boolean fastForward) throws Exception { String branch = getBranchNameFromRef(c.getChange().change().getDest().get()); String stagingRef = R_STAGING + branch; @@ -331,13 +398,16 @@ public class QtUnStageIT extends QtCodeReviewIT { RevCommit stagingHead = getRemoteRefHead(project, stagingRef); assertThat(stagingHead).isNotEqualTo(oldStagingHead); + assertReviewedByFooter(stagingHead, true); - if (merge) { + if (fastForward) { + assertThat(stagingHead).isEqualTo(expectedStagingHead); + } else if (merge) { assertThat(stagingHead.getParentCount()).isEqualTo(2); assertThat(stagingHead.getParent(1)).isEqualTo(expectedContent.getCommit()); expectedStagingHead = stagingHead; } else if (expectedStagingHead == null && expectedContent != null) { - assertCherryPick(stagingHead, expectedContent.getCommit(), getCurrentPatchSHA(expectedContent)); + assertCherryPick(stagingHead, expectedContent.getCommit(), base); expectedStagingHead = stagingHead; } else { assertThat(stagingHead).isEqualTo(expectedStagingHead); // staging is updated @@ -350,8 +420,7 @@ public class QtUnStageIT extends QtCodeReviewIT { resetEvents(); } - Change change = c.getChange().change(); - assertThat(change.getStatus()).isEqualTo(Change.Status.NEW); + assertStatusNew(c.getChange().change()); ArrayList<ChangeMessage> messages = new ArrayList(c.getChange().messages()); assertThat(messages.get(messages.size() - 1).getMessage()).isEqualTo(UNSTAGED_MSG); |