From cbdfd581d5783a9c2b577f4abfdaa8d37b053fc3 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Thu, 5 Sep 2019 09:26:46 +0200 Subject: [PATCH] Test that creating 2 changes for the same commit on the same branch is not possible Gerrit rejects the creation of a change for a commit that already exists in the repository. The only exception is for changes on different branches if CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET is set to true. Add push tests to verify this. In addition verify that it's also not possible to end up with 2 changes for the same commit on the same branch by using the Create Change REST endpoint, or by doing and publishing identical change edits on different changes which were created on the same base. Signed-off-by: Edwin Kempin Change-Id: I6ed37042475250b23a08ec3318f30d538055d166 --- .../gerrit/acceptance/edit/ChangeEditIT.java | 23 ++++++ .../acceptance/git/AbstractPushForReview.java | 70 +++++++++++++++++++ .../rest/change/CreateChangeIT.java | 8 +++ 3 files changed, 101 insertions(+) diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 070b98c5d9..df4b6d6c53 100644 --- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -47,9 +47,11 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.PublishChangeEditInput; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.client.ChangeEditDetailOption; +import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.common.ApprovalInfo; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.common.DiffInfo; import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.common.FileInfo; @@ -734,6 +736,27 @@ public class ChangeEditIT extends AbstractDaemonTest { .contains(String.format("change %s is abandoned", change._number)); } + @Test + public void sha1sOfTwoChangesWithSameContentAfterEditDiffer() throws Exception { + ChangeInput changeInput = new ChangeInput(); + changeInput.project = project.get(); + changeInput.branch = "master"; + changeInput.subject = "Empty change"; + changeInput.status = ChangeStatus.NEW; + + ChangeInfo info1 = gApi.changes().create(changeInput).get(); + gApi.changes().id(info1._number).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW)); + gApi.changes().id(info1._number).edit().publish(new PublishChangeEditInput()); + info1 = gApi.changes().id(info1._number).get(); + + ChangeInfo info2 = gApi.changes().create(changeInput).get(); + gApi.changes().id(info2._number).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW)); + gApi.changes().id(info2._number).edit().publish(new PublishChangeEditInput()); + info2 = gApi.changes().id(info2._number).get(); + + assertThat(info1.currentRevision).isNotEqualTo(info2.currentRevision); + } + private void createArbitraryEditFor(String changeId) throws Exception { createEmptyEditFor(changeId); arbitrarilyModifyEditOf(changeId); diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java index f82dcd7ee9..89dbabff5a 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -2608,6 +2608,76 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { + "\n"); } + @Test + public void cannotPushTheSameCommitTwiceForReviewToTheSameBranch() throws Exception { + testCannotPushTheSameCommitTwiceForReviewToTheSameBranch(); + } + + @Test + public void cannotPushTheSameCommitTwiceForReviewToTheSameBranchCreateNewChangeForAllNotInTarget() + throws Exception { + enableCreateNewChangeForAllNotInTarget(); + testCannotPushTheSameCommitTwiceForReviewToTheSameBranch(); + } + + private void testCannotPushTheSameCommitTwiceForReviewToTheSameBranch() throws Exception { + setRequireChangeId(InheritableBoolean.FALSE); + + // create a commit without Change-Id + testRepo + .branch("HEAD") + .commit() + .author(user.newIdent()) + .committer(user.newIdent()) + .add(PushOneCommit.FILE_NAME, PushOneCommit.FILE_CONTENT) + .message(PushOneCommit.SUBJECT) + .create(); + + // push the commit for review to create a change + PushResult r = pushHead(testRepo, "refs/for/master"); + assertPushOk(r, "refs/for/master"); + + // try to push the same commit for review again to create another change on the same branch, + // it's expected that this is rejected with "no new changes" + r = pushHead(testRepo, "refs/for/master"); + assertPushRejected(r, "refs/for/master", "no new changes"); + } + + @Test + public void pushTheSameCommitTwiceForReviewToDifferentBranches() throws Exception { + setRequireChangeId(InheritableBoolean.FALSE); + + // create a commit without Change-Id + testRepo + .branch("HEAD") + .commit() + .author(user.newIdent()) + .committer(user.newIdent()) + .add(PushOneCommit.FILE_NAME, PushOneCommit.FILE_CONTENT) + .message(PushOneCommit.SUBJECT) + .create(); + + // push the commit for review to create a change + PushResult r = pushHead(testRepo, "refs/for/master"); + assertPushOk(r, "refs/for/master"); + + // create another branch + gApi.projects().name(project.get()).branch("otherBranch").create(new BranchInput()); + + // try to push the same commit for review again to create a change on another branch, + // it's expected that this is rejected with "no new changes" since + // CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET is false + r = pushHead(testRepo, "refs/for/otherBranch"); + assertPushRejected(r, "refs/for/otherBranch", "no new changes"); + + enableCreateNewChangeForAllNotInTarget(); + + // try to push the same commit for review again to create a change on another branch, + // now it should succeed since CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET is true + r = pushHead(testRepo, "refs/for/otherBranch"); + assertPushOk(r, "refs/for/otherBranch"); + } + private DraftInput newDraft(String path, int line, String message) { DraftInput d = new DraftInput(); d.path = path; diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index b5f3d06145..b718d843b9 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -436,6 +436,14 @@ public class CreateChangeIT extends AbstractDaemonTest { assertCreateFails(input, ResourceNotFoundException.class, "ref refs/heads/foo not found"); } + @Test + public void sha1sOfTwoNewChangesDiffer() throws Exception { + ChangeInput changeInput = newChangeInput(ChangeStatus.NEW); + ChangeInfo info1 = assertCreateSucceeds(changeInput); + ChangeInfo info2 = assertCreateSucceeds(changeInput); + assertThat(info1.currentRevision).isNotEqualTo(info2.currentRevision); + } + private ChangeInput newChangeInput(ChangeStatus status) { ChangeInput in = new ChangeInput(); in.project = project.get();