Extend submit tests with more assertions on events
Refactor the EventRecorder class to make it more usable.
Add more tests to verify that submitting changes causes the expected
ref-updated and change-merged events to be emitted.
Remove testing of the change-merged event from the submit() method
since it's now explicitly covered by the actual test cases.
Some of the updated tests are failing due to the change-merged event
not having expected values. These tests are annotated with @Ignore,
and the problems are tracked in issue 4193 and issue 4194.
The following cases are still not yet covered:
- ref-updated and change-merged events on projects/branches other than
the default project's master branch.
- events for cases involving "submit whole topic".
Change-Id: Iaea6b17aa40660be1b4ef74d2ce1d88881dee9f2
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java
index ac6930d..466488c 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java
@@ -15,11 +15,10 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
-import static org.eclipse.jgit.lib.Constants.R_HEADS;
-import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
+import com.google.common.base.Function;
import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.UserScopedEventListener;
@@ -27,6 +26,7 @@
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.data.RefUpdateAttribute;
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.RefEvent;
@@ -34,8 +34,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import java.util.Collection;
-import java.util.List;
+import org.eclipse.jgit.revwalk.RevCommit;
public class EventRecorder {
private final RegistrationHandle eventListenerRegistration;
@@ -85,42 +84,74 @@
return String.format("%s-%s-%s", type, project, ref);
}
- public RefUpdatedEvent getOneRefUpdate(String project, String refName) {
- String key = key(RefUpdatedEvent.TYPE, project, refName);
- assertThat(recordedEvents).containsKey(key);
- Collection<RefEvent> events = recordedEvents.get(key);
- assertThat(events).hasSize(1);
- Event e = events.iterator().next();
- assertThat(e).isInstanceOf(RefUpdatedEvent.class);
- return (RefUpdatedEvent) e;
+ private static class RefEventTransformer<T extends RefEvent>
+ implements Function<RefEvent, T> {
+
+ @SuppressWarnings("unchecked")
+ @Override
+ public T apply(RefEvent e) {
+ return (T) e;
+ }
}
- public ImmutableList<RefEvent> getRefUpdates(String project, String refName,
- int expectedSize) {
+ private ImmutableList<RefUpdatedEvent> getRefUpdatedEvents(String project,
+ String refName, int expectedSize) {
String key = key(RefUpdatedEvent.TYPE, project, refName);
- assertThat(recordedEvents).containsKey(key);
- Collection<RefEvent> events = recordedEvents.get(key);
- assertThat(events).hasSize(expectedSize);
- return ImmutableList.copyOf(events);
- }
+ if (expectedSize == 0) {
+ assertThat(recordedEvents).doesNotContainKey(key);
+ return ImmutableList.of();
+ }
- public ChangeMergedEvent getOneChangeMerged(String project, String branch,
- final String changeNumber) throws Exception {
- String key = key(ChangeMergedEvent.TYPE, project,
- branch.startsWith(R_HEADS) ? branch : R_HEADS + branch);
assertThat(recordedEvents).containsKey(key);
- List<RefEvent> events = FluentIterable
+ ImmutableList<RefUpdatedEvent> events = FluentIterable
.from(recordedEvents.get(key))
- .filter(new Predicate<RefEvent>() {
- @Override
- public boolean apply(RefEvent input) {
- assertThat(input).isInstanceOf(ChangeMergedEvent.class);
- ChangeMergedEvent e = (ChangeMergedEvent) input;
- return e.change.get().number.equals(changeNumber);
- }})
+ .transform(new RefEventTransformer<RefUpdatedEvent>())
.toList();
- assertThat(events).hasSize(1);
- return (ChangeMergedEvent) events.get(0);
+ assertThat(events).hasSize(expectedSize);
+ return events;
+ }
+
+ private ImmutableList<ChangeMergedEvent> getChangeMergedEvents(String project,
+ String branch, int expectedSize) {
+ String key = key(ChangeMergedEvent.TYPE, project, branch);
+ if (expectedSize == 0) {
+ assertThat(recordedEvents).doesNotContainKey(key);
+ return ImmutableList.of();
+ }
+
+ assertThat(recordedEvents).containsKey(key);
+ ImmutableList<ChangeMergedEvent> events = FluentIterable
+ .from(recordedEvents.get(key))
+ .transform(new RefEventTransformer<ChangeMergedEvent>())
+ .toList();
+ assertThat(events).hasSize(expectedSize);
+ return events;
+ }
+
+ public void assertRefUpdatedEvents(String project, String branch,
+ RevCommit... expected) throws Exception {
+ ImmutableList<RefUpdatedEvent> events = getRefUpdatedEvents(project,
+ branch, expected.length / 2);
+ int i = 0;
+ for (RefUpdatedEvent event : events) {
+ RefUpdateAttribute actual = event.refUpdate.get();
+ assertThat(actual.oldRev).isEqualTo(expected[i].name());
+ assertThat(actual.newRev).isEqualTo(expected[i+1].name());
+ i += 2;
+ }
+ }
+
+ public void assertChangeMergedEvents(String project, String branch,
+ String... expected) throws Exception {
+ ImmutableList<ChangeMergedEvent> events = getChangeMergedEvents(project,
+ branch, expected.length / 2);
+ int i = 0;
+ for (ChangeMergedEvent event : events) {
+ String id = event.change.get().id;
+ assertThat(id).isEqualTo(expected[i]);
+ assertThat(event.newRev).isEqualTo(expected[i+1]);
+ i += 2;
+ }
}
public void close() {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 0fc3fa5..8aab521 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -32,7 +32,6 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.extensions.api.changes.SubmitInput;
-import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -52,7 +51,6 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
-import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.TestTimeUtil;
@@ -203,22 +201,22 @@
}
protected void submit(String changeId) throws Exception {
- submit(changeId, new SubmitInput(), null, null, true);
+ submit(changeId, new SubmitInput(), null, null);
}
protected void submit(String changeId, SubmitInput input) throws Exception {
- submit(changeId, input, null, null, true);
+ submit(changeId, input, null, null);
}
protected void submitWithConflict(String changeId,
String expectedError) throws Exception {
submit(changeId, new SubmitInput(), ResourceConflictException.class,
- expectedError, true);
+ expectedError);
}
protected void submit(String changeId, SubmitInput input,
Class<? extends RestApiException> expectedExceptionType,
- String expectedExceptionMsg, boolean checkMergeResult) throws Exception {
+ String expectedExceptionMsg) throws Exception {
approve(changeId);
if (expectedExceptionType == null) {
assertSubmittable(changeId);
@@ -247,9 +245,6 @@
}
ChangeInfo change = gApi.changes().id(changeId).info();
assertMerged(change.changeId);
- if (checkMergeResult) {
- checkMergeResult(change);
- }
}
protected void assertSubmittable(String changeId) throws Exception {
@@ -262,17 +257,15 @@
assertThat(desc.isEnabled()).named("enabled bit on submit action").isTrue();
}
- private void checkMergeResult(ChangeInfo change) throws Exception {
- // Get the revision of the branch after the submit to compare with the
- // newRev of the ChangeMergedEvent.
- BranchInfo branch = gApi.projects().name(change.project)
- .branch(change.branch).get();
- ChangeMergedEvent event = eventRecorder.getOneChangeMerged(
- change.project,
- change.branch,
- Integer.toString(change._number));
- assertThat(event.newRev).isNotNull();
- assertThat(branch.revision).isEqualTo(event.newRev);
+ protected void assertChangeMergedEvents(String... expected) throws Exception {
+ eventRecorder.assertChangeMergedEvents(
+ project.get(), "refs/heads/master", expected);
+ }
+
+ protected void assertRefUpdatedEvents(RevCommit... expected)
+ throws Exception {
+ eventRecorder.assertRefUpdatedEvents(
+ project.get(), "refs/heads/master", expected);
}
protected void assertCurrentRevision(String changeId, int expectedNum,
@@ -373,8 +366,7 @@
}
}
- protected RevCommit getRemoteHead()
- throws Exception {
+ protected RevCommit getRemoteHead() throws Exception {
return getRemoteHead(project, "master");
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByMerge.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByMerge.java
index 12afff2..741864a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByMerge.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByMerge.java
@@ -145,7 +145,7 @@
SubmitInput failAfterRefUpdates =
new TestSubmitInput(new SubmitInput(), true);
submit(change2.getChangeId(), failAfterRefUpdates,
- ResourceConflictException.class, "Failing after ref updates", true);
+ ResourceConflictException.class, "Failing after ref updates");
// Bad: ref advanced but change wasn't updated.
PatchSet.Id psId1 = new PatchSet.Id(id2, 1);
@@ -165,9 +165,7 @@
assertThat(tip.getParent(1)).isEqualTo(change2.getCommit());
}
- // Skip checking the merge result; in the fixup case, the newRev in
- // ChangeMergedEvent won't match the current branch tip.
- submit(change2.getChangeId(), new SubmitInput(), null, null, false);
+ submit(change2.getChangeId(), new SubmitInput(), null, null);
// Change status and patch set entities were updated, and branch tip stayed
// the same.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index 6acbcf3..0c9d6c9 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -29,14 +29,13 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.Submit.TestSubmitInput;
-import com.google.gerrit.server.data.RefUpdateAttribute;
-import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
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;
@@ -50,11 +49,16 @@
@Test
public void submitWithCherryPickIfFastForwardPossible() throws Exception {
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange();
submit(change.getChangeId());
assertCherryPick(testRepo, false);
- assertThat(getRemoteHead().getParent(0))
+ RevCommit newHead = getRemoteHead();
+ assertThat(newHead.getParent(0))
.isEqualTo(change.getCommit().getParent(0));
+
+ assertRefUpdatedEvents(initialHead, newHead);
+ assertChangeMergedEvents(change.getChangeId(), newHead.name());
}
@Test
@@ -64,7 +68,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "b.txt", "other content");
@@ -72,36 +76,51 @@
assertCherryPick(testRepo, false);
RevCommit newHead = getRemoteHead();
assertThat(newHead.getParentCount()).isEqualTo(1);
- assertThat(newHead.getParent(0)).isEqualTo(oldHead);
+ assertThat(newHead.getParent(0)).isEqualTo(headAfterFirstSubmit);
assertCurrentRevision(change2.getChangeId(), 2, newHead);
assertSubmitter(change2.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 2);
assertPersonEquals(admin.getIdent(), newHead.getAuthorIdent());
assertPersonEquals(admin.getIdent(), newHead.getCommitterIdent());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, newHead);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), newHead.name());
}
@Test
@TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
public void submitWithContentMerge() throws Exception {
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change =
createChange("Change 1", "a.txt", "aaa\nbbb\nccc\n");
submit(change.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteHead();
PushOneCommit.Result change2 =
createChange("Change 2", "a.txt", "aaa\nbbb\nccc\nddd\n");
submit(change2.getChangeId());
+ RevCommit headAfterSecondSubmit = getRemoteHead();
- RevCommit oldHead = getRemoteHead();
testRepo.reset(change.getCommit());
PushOneCommit.Result change3 =
createChange("Change 3", "a.txt", "bbb\nccc\n");
submit(change3.getChangeId());
assertCherryPick(testRepo, true);
- RevCommit newHead = getRemoteHead();
- assertThat(newHead.getParent(0)).isEqualTo(oldHead);
+ RevCommit headAfterThirdSubmit = getRemoteHead();
+ assertThat(headAfterThirdSubmit.getParent(0))
+ .isEqualTo(headAfterSecondSubmit);
assertApproved(change3.getChangeId());
- assertCurrentRevision(change3.getChangeId(), 2, newHead);
+ assertCurrentRevision(change3.getChangeId(), 2, headAfterThirdSubmit);
assertSubmitter(change2.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 2);
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit,
+ headAfterSecondSubmit, headAfterThirdSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name(),
+ change3.getChangeId(), headAfterThirdSubmit.name());
}
@Test
@@ -112,7 +131,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit newHead = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "a.txt", "other content");
@@ -122,9 +141,12 @@
"merged due to a path conflict. Please rebase the change locally and " +
"upload the rebased commit for review.");
- assertThat(getRemoteHead()).isEqualTo(oldHead);
+ assertThat(getRemoteHead()).isEqualTo(newHead);
assertCurrentRevision(change2.getChangeId(), 1, change2.getCommit());
assertNoSubmitter(change2.getChangeId(), 1);
+
+ assertRefUpdatedEvents(initialHead, newHead);
+ assertChangeMergedEvents(change.getChangeId(), newHead.name());
}
@Test
@@ -134,19 +156,25 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
createChange("Change 2", "b.txt", "other content");
PushOneCommit.Result change3 =
createChange("Change 3", "c.txt", "different content");
submit(change3.getChangeId());
assertCherryPick(testRepo, false);
- RevCommit newHead = getRemoteHead();
- assertThat(newHead.getParent(0)).isEqualTo(oldHead);
+ RevCommit headAfterSecondSubmit = getRemoteHead();
+ assertThat(headAfterSecondSubmit.getParent(0))
+ .isEqualTo(headAfterFirstSubmit);
assertApproved(change3.getChangeId());
- assertCurrentRevision(change3.getChangeId(), 2, newHead);
+ assertCurrentRevision(change3.getChangeId(), 2, headAfterSecondSubmit);
assertSubmitter(change3.getChangeId(), 1);
assertSubmitter(change3.getChangeId(), 2);
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change3.getChangeId(), headAfterSecondSubmit.name());
}
@Test
@@ -156,7 +184,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit newHead = getRemoteHead();
testRepo.reset(initialHead);
createChange("Change 2", "b.txt", "other content");
PushOneCommit.Result change3 =
@@ -167,9 +195,12 @@
"merged due to a path conflict. Please rebase the change locally and " +
"upload the rebased commit for review.");
- assertThat(getRemoteHead()).isEqualTo(oldHead);
+ assertThat(getRemoteHead()).isEqualTo(newHead);
assertCurrentRevision(change3.getChangeId(), 1, change3.getCommit());
assertNoSubmitter(change3.getChangeId(), 1);
+
+ assertRefUpdatedEvents(initialHead, newHead);
+ assertChangeMergedEvents(change.getChangeId(), newHead.name());
}
@Test
@@ -197,11 +228,8 @@
assertNew(change2.getChangeId());
assertNew(change3.getChangeId());
- RefUpdatedEvent event = eventRecorder.getOneRefUpdate(
- project.get(), "refs/heads/master");
- RefUpdateAttribute refUpdate = event.refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
- assertThat(refUpdate.newRev).isEqualTo(log.get(0).name());
+ assertRefUpdatedEvents(initialHead, log.get(0));
+ assertChangeMergedEvents(change4.getChangeId(), log.get(0).name());
}
@Test
@@ -215,9 +243,11 @@
// Submit succeeds; change3 is successfully cherry-picked onto head.
submit(change3.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteHead();
// Submit succeeds; change2 is successfully cherry-picked onto head
// (which was change3's cherry-pick).
submit(change2.getChangeId());
+ RevCommit headAfterSecondSubmit = getRemoteHead();
// change2 is the new tip.
List<RevCommit> log = getRemoteLog();
@@ -230,6 +260,11 @@
assertThat(log.get(1).getParent(0)).isEqualTo(log.get(2));
assertThat(log.get(2).getId()).isEqualTo(initialHead.getId());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(change3.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name());
}
@Test
@@ -256,6 +291,9 @@
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0)).isEqualTo(initialHead.getId());
assertNoSubmitter(change3.getChangeId(), 1);
+
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
}
@Test
@@ -271,11 +309,18 @@
// by topic or ancestor (due to cherrypicking!)
approve(change3.getChangeId());
submit(change4.getChangeId());
+ RevCommit newHead = getRemoteHead();
assertNew(change2.getChangeId());
assertNew(change3.getChangeId());
+
+ assertRefUpdatedEvents(initialHead, newHead);
+ 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 {
@@ -287,18 +332,21 @@
PushOneCommit.Result change2 = createChange("Change 2", "a.txt", "a");
submit(change1.getChangeId());
- RevCommit oldHead = getRemoteHead();
- assertThat(oldHead.getShortMessage()).isEqualTo("Change 1");
+ RevCommit headAfterFirstSubmit = getRemoteHead();
+ assertThat(headAfterFirstSubmit.getShortMessage()).isEqualTo("Change 1");
- // Don't check merge result, since ref isn't updated.
- submit(change2.getChangeId(), new SubmitInput(), null, null, false);
+ submit(change2.getChangeId(), new SubmitInput(), null, null);
- assertThat(getRemoteHead()).isEqualTo(oldHead);
+ assertThat(getRemoteHead()).isEqualTo(headAfterFirstSubmit);
ChangeInfo info2 = get(change2.getChangeId());
assertThat(info2.status).isEqualTo(ChangeStatus.MERGED);
assertThat(Iterables.getLast(info2.messages).message)
.isEqualTo(CommitMergeStatus.SKIPPED_IDENTICAL_TREE.getMessage());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change1.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterFirstSubmit.name());
}
@Test
@@ -308,7 +356,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "b.txt", "other content");
@@ -316,7 +364,8 @@
SubmitInput failAfterRefUpdates =
new TestSubmitInput(new SubmitInput(), true);
submit(change2.getChangeId(), failAfterRefUpdates,
- ResourceConflictException.class, "Failing after ref updates", true);
+ ResourceConflictException.class, "Failing after ref updates");
+ RevCommit headAfterFailedSubmit = getRemoteHead();
// Bad: ref advanced but change wasn't updated.
PatchSet.Id psId1 = new PatchSet.Id(id2, 1);
@@ -335,7 +384,8 @@
rev2 = repo.exactRef(psId2.toRefName()).getObjectId();
assertThat(rev2).isNotNull();
assertThat(rev2).isNotEqualTo(rev1);
- assertThat(rw.parseCommit(rev2).getParent(0)).isEqualTo(oldHead);
+ assertThat(rw.parseCommit(rev2).getParent(0))
+ .isEqualTo(headAfterFirstSubmit);
assertThat(repo.exactRef("refs/heads/master").getObjectId())
.isEqualTo(rev2);
@@ -345,6 +395,8 @@
// Change status and patch set entities were updated, and branch tip stayed
// the same.
+ RevCommit headAfterSecondSubmit = getRemoteHead();
+ assertThat(headAfterSecondSubmit).isEqualTo(headAfterFailedSubmit);
info = gApi.changes().id(id2.get()).get();
assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
assertThat(info.revisions.get(info.currentRevision)._number).isEqualTo(2);
@@ -359,5 +411,9 @@
assertThat(repo.exactRef("refs/heads/master").getObjectId())
.isEqualTo(rev2);
}
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name());
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
index 00638a2..de2579a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
@@ -30,13 +30,13 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.Submit.TestSubmitInput;
-import com.google.gerrit.server.events.RefUpdatedEvent;
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.eclipse.jgit.transport.PushResult;
+import org.junit.Ignore;
import org.junit.Test;
import java.util.Map;
@@ -50,18 +50,24 @@
@Test
public void submitWithFastForward() throws Exception {
- RevCommit oldHead = getRemoteHead();
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange();
submit(change.getChangeId());
- RevCommit head = getRemoteHead();
- assertThat(head.getId()).isEqualTo(change.getCommit());
- assertThat(head.getParent(0)).isEqualTo(oldHead);
+ RevCommit updatedHead = getRemoteHead();
+ assertThat(updatedHead.getId()).isEqualTo(change.getCommit());
+ assertThat(updatedHead.getParent(0)).isEqualTo(initialHead);
assertSubmitter(change.getChangeId(), 1);
+
+ assertRefUpdatedEvents(initialHead, updatedHead);
+ 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 originalHead = getRemoteHead();
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange();
PushOneCommit.Result change2 = createChange();
@@ -81,15 +87,15 @@
assertSubmittedTogether(id1, id2, id1);
assertSubmittedTogether(id2, id2, id1);
- RefUpdatedEvent refUpdate = eventRecorder.getOneRefUpdate(
- project.get(), "refs/heads/master");
- assertThat(refUpdate.refUpdate.get().oldRev).isEqualTo(originalHead.name());
- assertThat(refUpdate.refUpdate.get().newRev).isEqualTo(updatedHead.name());
+ assertRefUpdatedEvents(initialHead, updatedHead);
+ //TODO(dpursehouse) why are change-merged events in reverse order?
+ assertChangeMergedEvents(change2.getChangeId(), updatedHead.name(),
+ change.getChangeId(), updatedHead.name());
}
@Test
public void submitTwoChangesWithFastForward_missingDependency() throws Exception {
- RevCommit oldHead = getRemoteHead();
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change1 = createChange();
PushOneCommit.Result change2 = createChange();
@@ -98,8 +104,10 @@
"Failed to submit 2 changes due to the following problems:\n"
+ "Change " + id1 + ": needs Code-Review");
- RevCommit head = getRemoteHead();
- assertThat(head.getId()).isEqualTo(oldHead.getId());
+ RevCommit updatedHead = getRemoteHead();
+ assertThat(updatedHead.getId()).isEqualTo(initialHead.getId());
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
}
@Test
@@ -109,7 +117,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "b.txt", "other content");
@@ -126,8 +134,11 @@
"Change " + change2.getChange().getId() + ": Project policy requires " +
"all submissions to be a fast-forward. Please rebase the change " +
"locally and upload again for review.");
- assertThat(getRemoteHead()).isEqualTo(oldHead);
+ assertThat(getRemoteHead()).isEqualTo(headAfterFirstSubmit);
assertSubmitter(change.getChangeId(), 1);
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name());
}
@Test
@@ -137,7 +148,7 @@
SubmitInput failAfterRefUpdates =
new TestSubmitInput(new SubmitInput(), true);
submit(change.getChangeId(), failAfterRefUpdates,
- ResourceConflictException.class, "Failing after ref updates", true);
+ ResourceConflictException.class, "Failing after ref updates");
// Bad: ref advanced but change wasn't updated.
PatchSet.Id psId = new PatchSet.Id(id, 1);
@@ -167,10 +178,15 @@
assertThat(repo.exactRef("refs/heads/master").getObjectId())
.isEqualTo(rev);
}
+
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents(change.getChangeId(), getRemoteHead().name());
}
@Test
public void submitSameCommitsAsInExperimentalBranch() throws Exception {
+ RevCommit initialHead = getRemoteHead();
+
grant(Permission.CREATE, project, "refs/heads/*");
grant(Permission.PUSH, project, "refs/heads/experimental");
@@ -189,8 +205,12 @@
.isEqualTo(c1.getId());
submit(id1);
+ RevCommit headAfterSubmit = getRemoteHead();
assertThat(getRemoteHead().getId()).isEqualTo(c1.getId());
assertSubmitter(id1, 1);
+
+ assertRefUpdatedEvents(initialHead, headAfterSubmit);
+ assertChangeMergedEvents(id1, headAfterSubmit.name());
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
index a41eb18..f460f71 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
@@ -18,15 +18,11 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.client.SubmitType;
-import com.google.gerrit.server.data.RefUpdateAttribute;
-import com.google.gerrit.server.events.RefEvent;
-import com.google.gerrit.server.events.RefUpdatedEvent;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Ignore;
import org.junit.Test;
-import java.util.List;
-
public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
@Override
@@ -36,18 +32,24 @@
@Test
public void submitWithMergeIfFastForwardPossible() throws Exception {
- RevCommit oldHead = getRemoteHead();
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange();
submit(change.getChangeId());
- RevCommit head = getRemoteHead();
- assertThat(head.getParentCount()).isEqualTo(2);
- assertThat(head.getParent(0)).isEqualTo(oldHead);
- assertThat(head.getParent(1)).isEqualTo(change.getCommit());
+ RevCommit headAfterSubmit = getRemoteHead();
+ assertThat(headAfterSubmit.getParentCount()).isEqualTo(2);
+ assertThat(headAfterSubmit.getParent(0)).isEqualTo(initialHead);
+ assertThat(headAfterSubmit.getParent(1)).isEqualTo(change.getCommit());
assertSubmitter(change.getChangeId(), 1);
- assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
- assertPersonEquals(serverIdent.get(), head.getCommitterIdent());
+ assertPersonEquals(admin.getIdent(), headAfterSubmit.getAuthorIdent());
+ assertPersonEquals(serverIdent.get(), headAfterSubmit.getCommitterIdent());
+
+ assertRefUpdatedEvents(initialHead, headAfterSubmit);
+ 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();
@@ -88,17 +90,11 @@
assertPersonEquals(serverIdent.get(),
headAfterSecondSubmit.getCommitterIdent());
- // The two submit operations should have resulted in two ref-update events
- List<RefEvent> refUpdates = eventRecorder.getRefUpdates(
- project.get(), "refs/heads/master", 2);
-
- RefUpdateAttribute refUpdate =
- ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterFirstSubmit.name());
-
- refUpdate = ((RefUpdatedEvent)(refUpdates.get(1))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(headAfterFirstSubmit.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterSecondSubmit.name());
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ //TODO(dpursehouse) why are change-merged events in reverse order?
+ assertChangeMergedEvents(change2.getChangeId(), headAfterFirstSubmit.name(),
+ change4.getChangeId(), headAfterSecondSubmit.name(),
+ change3.getChangeId(), headAfterSecondSubmit.name());
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
index 19cf436..6c850f1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
@@ -13,13 +13,11 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.data.RefUpdateAttribute;
-import com.google.gerrit.server.events.RefEvent;
-import com.google.gerrit.server.events.RefUpdatedEvent;
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;
@@ -33,17 +31,23 @@
@Test
public void submitWithFastForward() throws Exception {
- RevCommit oldHead = getRemoteHead();
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange();
submit(change.getChangeId());
- RevCommit head = getRemoteHead();
- assertThat(head.getId()).isEqualTo(change.getCommit());
- assertThat(head.getParent(0)).isEqualTo(oldHead);
+ RevCommit updatedHead = getRemoteHead();
+ assertThat(updatedHead.getId()).isEqualTo(change.getCommit());
+ assertThat(updatedHead.getParent(0)).isEqualTo(initialHead);
assertSubmitter(change.getChangeId(), 1);
- assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
- assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
+ assertPersonEquals(admin.getIdent(), updatedHead.getAuthorIdent());
+ assertPersonEquals(admin.getIdent(), updatedHead.getCommitterIdent());
+
+ assertRefUpdatedEvents(initialHead, updatedHead);
+ 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();
@@ -87,18 +91,13 @@
assertNew(change2.getChangeId());
// The two submit operations should have resulted in two ref-update events
- List<RefEvent> refUpdates = eventRecorder.getRefUpdates(
- project.get(), "refs/heads/master", 2);
-
- RefUpdateAttribute refUpdate =
- ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterFirstSubmit.name());
-
- refUpdate = ((RefUpdatedEvent)(refUpdates.get(1))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(headAfterFirstSubmit.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterSecondSubmit.name());
-
+ // and three change-merged events.
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ //TODO(dpursehouse) why are change-merged events in reverse order?
+ assertChangeMergedEvents(change3.getChangeId(), headAfterFirstSubmit.name(),
+ change5.getChangeId(), headAfterSecondSubmit.name(),
+ change4.getChangeId(), headAfterSecondSubmit.name());
}
@Test
@@ -245,10 +244,13 @@
@Test
public void submitWithMergedAncestorsOnOtherBranch() throws Exception {
+ RevCommit initialHead = getRemoteHead();
+
PushOneCommit.Result change1 = createChange(testRepo, "master",
"base commit",
"a.txt", "1", "");
submit(change1.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteHead();
gApi.projects()
.name(project.get())
@@ -261,8 +263,8 @@
submit(change2.getChangeId());
- RevCommit tip1 = getRemoteLog(project, "master").get(0);
- assertThat(tip1.getShortMessage()).isEqualTo(
+ RevCommit headAfterSecondSubmit = getRemoteLog(project, "master").get(0);
+ assertThat(headAfterSecondSubmit.getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
RevCommit tip2 = getRemoteLog(project, "branch").get(0);
@@ -281,14 +283,21 @@
change3.getCommit().getShortMessage());
assertThat(log3.get(1).getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(change1.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name());
}
@Test
public void submitWithOpenAncestorsOnOtherBranch() throws Exception {
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change1 = createChange(testRepo, "master",
"base commit",
"a.txt", "1", "");
submit(change1.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteHead();
gApi.projects()
.name(project.get())
@@ -316,7 +325,7 @@
Project.NameKey p3 = createProject("project-related-to-change3");
TestRepository<?> repo3 = cloneProject(p3);
- RevCommit initialHead = getRemoteHead(p3, "master");
+ RevCommit repo3Head = getRemoteHead(p3, "master");
PushOneCommit.Result change3b = createChange(repo3, "master",
"some accompanying changes for change3a in another repo "
+ "tied together via topic",
@@ -335,11 +344,16 @@
RevCommit tipmaster = getRemoteLog(p3, "master").get(0);
assertThat(tipmaster.getShortMessage()).isEqualTo(
- initialHead.getShortMessage());
+ repo3Head.getShortMessage());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change1.getChangeId(), headAfterFirstSubmit.name());
}
@Test
public void testGerritWorkflow() throws Exception {
+ RevCommit initialHead = getRemoteHead();
+
// We'll setup a master and a stable branch.
// Then we create a change to be applied to master, which is
// then cherry picked back to stable. The stable branch will
@@ -350,21 +364,22 @@
.create(new BranchInput());
// Push a change to master
- PushOneCommit change2 =
+ PushOneCommit push =
pushFactory.create(db, user.getIdent(), testRepo,
"small fix", "a.txt", "2");
- PushOneCommit.Result change2result = change2.to("refs/for/master");
- submit(change2result.getChangeId());
- RevCommit tipmaster = getRemoteLog(project, "master").get(0);
- assertThat(tipmaster.getShortMessage()).isEqualTo(
- change2result.getCommit().getShortMessage());
+ PushOneCommit.Result change = push.to("refs/for/master");
+ submit(change.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteLog(project, "master").get(0);
+ assertThat(headAfterFirstSubmit.getShortMessage()).isEqualTo(
+ change.getCommit().getShortMessage());
// Now cherry pick to stable
CherryPickInput in = new CherryPickInput();
in.destination = "stable";
- in.message = "This goes to stable as well\n" + tipmaster.getFullMessage();
+ in.message = "This goes to stable as well\n"
+ + headAfterFirstSubmit.getFullMessage();
ChangeApi orig = gApi.changes()
- .id(change2result.getChangeId());
+ .id(change.getChangeId());
String cherryId = orig.current().cherryPick(in).id();
gApi.changes().id(cherryId).current().review(ReviewInput.approve());
gApi.changes().id(cherryId).current().submit();
@@ -399,8 +414,15 @@
String changeId = GitUtil.getChangeId(testRepo, merge).get();
approve(changeId);
submit(changeId);
- tipmaster = getRemoteLog(project, "master").get(0);
- assertThat(tipmaster.getShortMessage()).isEqualTo(merge.getShortMessage());
+ RevCommit headAfterSecondSubmit = getRemoteLog(project, "master").get(0);
+ assertThat(headAfterSecondSubmit.getShortMessage())
+ .isEqualTo(merge.getShortMessage());
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(
+ change.getChangeId(), headAfterFirstSubmit.name(),
+ changeId, headAfterSecondSubmit.name());
}
@Test
@@ -435,6 +457,9 @@
"Failed to submit 1 change due to the following problems:\n" +
"Change " + change3.getPatchSetId().getParentKey().get() +
": depends on change that was not submitted");
+
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
}
@Test
@@ -451,6 +476,9 @@
// approve and submit the change
submit(changeResult.getChangeId(), new SubmitInput(),
- ResourceConflictException.class, "nothing to merge", false);
+ ResourceConflictException.class, "nothing to merge");
+
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java
index 80f63d9..f75032c 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java
@@ -18,7 +18,6 @@
import static com.google.gerrit.acceptance.GitUtil.getChangeId;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
@@ -33,14 +32,12 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.Submit.TestSubmitInput;
-import com.google.gerrit.server.data.RefUpdateAttribute;
-import com.google.gerrit.server.events.RefEvent;
-import com.google.gerrit.server.events.RefUpdatedEvent;
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 {
@@ -64,6 +61,8 @@
assertSubmitter(change.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
+ assertRefUpdatedEvents(oldHead, head);
+ assertChangeMergedEvents(change.getChangeId(), head.name());
}
@Test
@@ -92,19 +91,15 @@
assertPersonEquals(admin.getIdent(),
headAfterSecondSubmit.getCommitterIdent());
- ImmutableList<RefEvent> refUpdates = eventRecorder.getRefUpdates(
- project.get(), "refs/heads/master", 2);
-
- RefUpdateAttribute refUpdate =
- ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterFirstSubmit.name());
-
- refUpdate = ((RefUpdatedEvent)(refUpdates.get(1))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(headAfterFirstSubmit.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterSecondSubmit.name());
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ 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();
@@ -143,40 +138,45 @@
assertThat(grandparent).isEqualTo(change1.getCommit());
assertCurrentRevision(change1.getChangeId(), 1, grandparent);
- ImmutableList<RefEvent> refUpdates = eventRecorder.getRefUpdates(
- project.get(), "refs/heads/master", 2);
- RefUpdateAttribute refUpdate =
- ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterFirstSubmit.name());
-
- refUpdate = ((RefUpdatedEvent)(refUpdates.get(1))).refUpdate.get();
- assertThat(refUpdate.oldRev).isEqualTo(headAfterFirstSubmit.name());
- assertThat(refUpdate.newRev).isEqualTo(headAfterSecondSubmit.name());
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit);
+ assertChangeMergedEvents(change1.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name(),
+ change3.getChangeId(), headAfterSecondSubmit.name());
}
@Test
@TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
public void submitWithContentMerge() throws Exception {
+ RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change =
createChange("Change 1", "a.txt", "aaa\nbbb\nccc\n");
submit(change.getChangeId());
+ RevCommit headAfterFirstSubmit = getRemoteHead();
PushOneCommit.Result change2 =
createChange("Change 2", "a.txt", "aaa\nbbb\nccc\nddd\n");
submit(change2.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterSecondSubmit = getRemoteHead();
testRepo.reset(change.getCommit());
PushOneCommit.Result change3 =
createChange("Change 3", "a.txt", "bbb\nccc\n");
submit(change3.getChangeId());
assertRebase(testRepo, true);
- RevCommit head = getRemoteHead();
- assertThat(head.getParent(0)).isEqualTo(oldHead);
+ RevCommit headAfterThirdSubmit = getRemoteHead();
+ assertThat(headAfterThirdSubmit.getParent(0))
+ .isEqualTo(headAfterSecondSubmit);
assertApproved(change3.getChangeId());
- assertCurrentRevision(change3.getChangeId(), 2, head);
+ assertCurrentRevision(change3.getChangeId(), 2, headAfterThirdSubmit);
assertSubmitter(change3.getChangeId(), 1);
assertSubmitter(change3.getChangeId(), 2);
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+ headAfterFirstSubmit, headAfterSecondSubmit,
+ headAfterSecondSubmit, headAfterThirdSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name(),
+ change3.getChangeId(), headAfterThirdSubmit.name());
}
@Test
@@ -187,7 +187,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "a.txt", "other content");
@@ -195,9 +195,12 @@
"Cannot rebase " + change2.getCommit().name()
+ ": The change could not be rebased due to a conflict during merge.");
RevCommit head = getRemoteHead();
- assertThat(head).isEqualTo(oldHead);
+ assertThat(head).isEqualTo(headAfterFirstSubmit);
assertCurrentRevision(change2.getChangeId(), 1, change2.getCommit());
assertNoSubmitter(change2.getChangeId(), 1);
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name());
}
@Test
@@ -207,7 +210,7 @@
createChange("Change 1", "a.txt", "content");
submit(change.getChangeId());
- RevCommit oldHead = getRemoteHead();
+ RevCommit headAfterFirstSubmit = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 =
createChange("Change 2", "b.txt", "other content");
@@ -215,7 +218,8 @@
SubmitInput failAfterRefUpdates =
new TestSubmitInput(new SubmitInput(), true);
submit(change2.getChangeId(), failAfterRefUpdates,
- ResourceConflictException.class, "Failing after ref updates", true);
+ ResourceConflictException.class, "Failing after ref updates");
+ RevCommit headAfterFailedSubmit = getRemoteHead();
// Bad: ref advanced but change wasn't updated.
PatchSet.Id psId1 = new PatchSet.Id(id2, 1);
@@ -234,13 +238,15 @@
rev2 = repo.exactRef(psId2.toRefName()).getObjectId();
assertThat(rev2).isNotNull();
assertThat(rev2).isNotEqualTo(rev1);
- assertThat(rw.parseCommit(rev2).getParent(0)).isEqualTo(oldHead);
+ assertThat(rw.parseCommit(rev2).getParent(0)).isEqualTo(headAfterFirstSubmit);
assertThat(repo.exactRef("refs/heads/master").getObjectId())
.isEqualTo(rev2);
}
submit(change2.getChangeId());
+ RevCommit headAfterSecondSubmit = getRemoteHead();
+ assertThat(headAfterSecondSubmit).isEqualTo(headAfterFailedSubmit);
// Change status and patch set entities were updated, and branch tip stayed
// the same.
@@ -258,6 +264,10 @@
assertThat(repo.exactRef("refs/heads/master").getObjectId())
.isEqualTo(rev2);
}
+
+ assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+ assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+ change2.getChangeId(), headAfterSecondSubmit.name());
}
private RevCommit parse(ObjectId id) throws Exception {
@@ -269,8 +279,13 @@
}
}
+ @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();
+
// Create two commits and push.
RevCommit c1 = commitBuilder()
.add("a.txt", "1")
@@ -294,10 +309,20 @@
approve(id1);
approve(id2);
submit(id1);
+ RevCommit headAfterSubmit = getRemoteHead();
+
+ assertRefUpdatedEvents(initialHead, headAfterSubmit);
+ assertChangeMergedEvents(id2, headAfterSubmit.name(),
+ 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();
+
PushOneCommit.Result change = createChange();
approve(change.getChangeId());
@@ -309,5 +334,10 @@
gApi.changes().id(change2nd.getChangeId()).current().submit();
assertMerged(change2nd.getChangeId());
assertMerged(change.getChangeId());
+
+ RevCommit newHead = getRemoteHead();
+ assertRefUpdatedEvents(initialHead, newHead);
+ assertChangeMergedEvents(change.getChangeId(), newHead.name(),
+ change2nd.getChangeId(), newHead.name());
}
}
