opendev/gerrit
Code Issues Proposed changes

SubmitStrategyOp: Set correct new revision in change-merged event

Browse Source 
When using the cherry-pick submit strategy, and submitting a change with
identical tree, the branch does not advance. The change-merged event for
such a change has the new revision set to the change's sha1, which does
not actually exist in the repository. Instead it should be the sha1 of
the head of the (unchanged) branch.

When using any of the other submit strategies, and submitting multiple
changes in a single operation, the change-merged events emitted for the
implicitly merged changes have the new revision set to change's sha1.

Instead, they should have the new revision of the branch resulting from
the whole operation.

Bug: Issue 4193
Bug: Issue 4194
Change-Id: If41ea05e4c75ed8381b884238a46113ddd8c0b48
This commit is contained in:
David Pursehouse
2016-06-15 16:36:43 +09:00
parent 3e75e79c9d
commit 4668f1c61e
6 changed files with 1 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
View File

@@ -35,7 +35,6 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.Ignore;
import org.junit.Test;
import java.util.List;
@@ -318,9 +317,6 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
assertChangeMergedEvents(change4.getChangeId(), newHead.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged event for
//the second change has the wrong newRev. See issue 4193.
@Test
@TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
public void submitIdenticalTree() throws Exception {

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
View File

@@ -36,7 +36,6 @@ import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushResult;
import org.junit.Ignore;
import org.junit.Test;
import java.util.Map;
@@ -62,9 +61,6 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
assertChangeMergedEvents(change.getChangeId(), updatedHead.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged event for
//the second change has the wrong newRev. See issue 4194.
@Test
public void submitTwoChangesWithFastForward() throws Exception {
RevCommit initialHead = getRemoteHead();

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
View File

@@ -20,7 +20,6 @@ import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.client.SubmitType;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Ignore;
import org.junit.Test;
public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
@@ -47,9 +46,6 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
assertChangeMergedEvents(change.getChangeId(), headAfterSubmit.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged events for
//the second submit have the wrong newRev. See issue 4194.
@Test
public void submitMultipleChanges() throws Exception {
RevCommit initialHead = getRemoteHead();

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
View File

@@ -17,7 +17,6 @@ import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.RefSpec;
import org.junit.Ignore;
import org.junit.Test;
import java.util.List;
@@ -45,9 +44,6 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
assertChangeMergedEvents(change.getChangeId(), updatedHead.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged events for
//the second submit have the wrong newRev. See issue 4194.
@Test
public void submitMultipleChanges() throws Exception {
RevCommit initialHead = getRemoteHead();

10
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java
View File

@@ -37,7 +37,6 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.Ignore;
import org.junit.Test;
public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
@@ -97,9 +96,6 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
change2.getChangeId(), headAfterSecondSubmit.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged events for
//the second submit have the wrong newRev. See issue 4194.
@Test
public void submitWithRebaseMultipleChanges() throws Exception {
RevCommit initialHead = getRemoteHead();
@@ -279,9 +275,6 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
}
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged event for
//the second change has the wrong newRev. See issue 4194.
@Test
public void submitAfterReorderOfCommits() throws Exception {
RevCommit initialHead = getRemoteHead();
@@ -316,9 +309,6 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
id1, headAfterSubmit.name());
}
@Ignore
//TODO(dpursehouse) this test fails because the change-merged event for
//the second change has the wrong newRev. See issue 4194.
@Test
public void submitChangesAfterBranchOnSecond() throws Exception {
RevCommit initialHead = getRemoteHead();

2
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
View File

@@ -526,7 +526,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
try {
args.hooks.doChangeMergedHook(updatedChange,
args.accountCache.get(submitter.getAccountId()).getAccount(),
mergedPatchSet, ctx.getDb(), mergeResultRev.name());
mergedPatchSet, ctx.getDb(), args.mergeTip.getCurrentTip().name());
} catch (OrmException ex) {
logError("Cannot run hook for submitted patch set " + getId(), ex);
}