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

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.base.Function;
-import com.google.common.collect.ImmutableList;
 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.DynamicSet;
 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.gerrit.server.events.RefUpdatedEvent;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
-import java.util.Collection;
+import org.eclipse.jgit.revwalk.RevCommit;
-import java.util.List;
 
 public class EventRecorder {
   private final RegistrationHandle eventListenerRegistration;
@@ -85,42 +84,74 @@ public class EventRecorder {
     return String.format("%s-%s-%s", type, project, ref);
   }
 
-  public RefUpdatedEvent getOneRefUpdate(String project, String refName) {
+  private static class RefEventTransformer<T extends RefEvent>
-    String key = key(RefUpdatedEvent.TYPE, project, refName);
+      implements Function<RefEvent, T> {
-    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;
-  }
 
-  public ImmutableList<RefEvent> getRefUpdates(String project, String refName,
+    @SuppressWarnings("unchecked")
-      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);
-  }
-
-  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
-        .from(recordedEvents.get(key))
-        .filter(new Predicate<RefEvent>() {
     @Override
-          public boolean apply(RefEvent input) {
+    public T apply(RefEvent e) {
-            assertThat(input).isInstanceOf(ChangeMergedEvent.class);
+      return (T) e;
-            ChangeMergedEvent e = (ChangeMergedEvent) input;
+    }
-            return e.change.get().number.equals(changeNumber);
+  }
-          }})
+
+  private ImmutableList<RefUpdatedEvent> getRefUpdatedEvents(String project,
+      String refName, int expectedSize) {
+    String key = key(RefUpdatedEvent.TYPE, project, refName);
+    if (expectedSize == 0) {
+      assertThat(recordedEvents).doesNotContainKey(key);
+      return ImmutableList.of();
+    }
+
+    assertThat(recordedEvents).containsKey(key);
+    ImmutableList<RefUpdatedEvent> events = FluentIterable
+        .from(recordedEvents.get(key))
+        .transform(new RefEventTransformer<RefUpdatedEvent>())
         .toList();
-    assertThat(events).hasSize(1);
+    assertThat(events).hasSize(expectedSize);
-    return (ChangeMergedEvent) events.get(0);
+    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() {

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java

@@ -32,7 +32,6 @@ import com.google.gerrit.acceptance.NoHttpd;
 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.reviewdb.client.Project;
 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 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
   }
 
   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 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     }
     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 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     assertThat(desc.isEnabled()).named("enabled bit on submit action").isTrue();
   }
 
-  private void checkMergeResult(ChangeInfo change) throws Exception {
+  protected void assertChangeMergedEvents(String... expected) throws Exception {
-    // Get the revision of the branch after the submit to compare with the
+    eventRecorder.assertChangeMergedEvents(
-    // newRev of the ChangeMergedEvent.
+        project.get(), "refs/heads/master", expected);
-    BranchInfo branch = gApi.projects().name(change.project)
+  }
-        .branch(change.branch).get();
+
-    ChangeMergedEvent event = eventRecorder.getOneChangeMerged(
+  protected void assertRefUpdatedEvents(RevCommit... expected)
-        change.project,
+      throws Exception {
-        change.branch,
+    eventRecorder.assertRefUpdatedEvents(
-        Integer.toString(change._number));
+        project.get(), "refs/heads/master", expected);
-    assertThat(event.newRev).isNotNull();
-    assertThat(branch.revision).isEqualTo(event.newRev);
   }
 
   protected void assertCurrentRevision(String changeId, int expectedNum,
@@ -373,8 +366,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     }
   }
 
-  protected RevCommit getRemoteHead()
+  protected RevCommit getRemoteHead() throws Exception {
-      throws Exception {
     return getRemoteHead(project, "master");
   }
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmitByMerge.java

@@ -145,7 +145,7 @@ public abstract class AbstractSubmitByMerge extends AbstractSubmit {
     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 @@ public abstract class AbstractSubmitByMerge extends AbstractSubmit {
       assertThat(tip.getParent(1)).isEqualTo(change2.getCommit());
     }
 
-    // Skip checking the merge result; in the fixup case, the newRev in
+    submit(change2.getChangeId(), new SubmitInput(), null, null);
-    // ChangeMergedEvent won't match the current branch tip.
-    submit(change2.getChangeId(), new SubmitInput(), null, null, false);
 
     // Change status and patch set entities were updated, and branch tip stayed
     // the same.

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

@@ -29,14 +29,13 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
 
   @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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     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();
+    RevCommit headAfterThirdSubmit = getRemoteHead();
-    assertThat(newHead.getParent(0)).isEqualTo(oldHead);
+    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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         "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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         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();
+    RevCommit headAfterSecondSubmit = getRemoteHead();
-    assertThat(newHead.getParent(0)).isEqualTo(oldHead);
+    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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         "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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     assertNew(change2.getChangeId());
     assertNew(change3.getChangeId());
 
-    RefUpdatedEvent event = eventRecorder.getOneRefUpdate(
+    assertRefUpdatedEvents(initialHead, log.get(0));
-        project.get(), "refs/heads/master");
+    assertChangeMergedEvents(change4.getChangeId(), log.get(0).name());
-    RefUpdateAttribute refUpdate = event.refUpdate.get();
-    assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
-    assertThat(refUpdate.newRev).isEqualTo(log.get(0).name());
   }
 
   @Test
@@ -215,9 +243,11 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
 
     // 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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     List<RevCommit> log = getRemoteLog();
     assertThat(log.get(0)).isEqualTo(initialHead.getId());
     assertNoSubmitter(change3.getChangeId(), 1);
+
+    assertRefUpdatedEvents();
+    assertChangeMergedEvents();
   }
 
   @Test
@@ -271,11 +309,18 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     // 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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     PushOneCommit.Result change2 = createChange("Change 2", "a.txt", "a");
 
     submit(change1.getChangeId());
-    RevCommit oldHead = getRemoteHead();
+    RevCommit headAfterFirstSubmit = getRemoteHead();
-    assertThat(oldHead.getShortMessage()).isEqualTo("Change 1");
+    assertThat(headAfterFirstSubmit.getShortMessage()).isEqualTo("Change 1");
 
-    // Don't check merge result, since ref isn't updated.
+    submit(change2.getChangeId(), new SubmitInput(), null, null);
-    submit(change2.getChangeId(), new SubmitInput(), null, null, false);
 
-    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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
         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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
       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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
 
     // 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 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
       assertThat(repo.exactRef("refs/heads/master").getObjectId())
           .isEqualTo(rev2);
     }
+
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit);
+    assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
+        change2.getChangeId(), headAfterSecondSubmit.name());
   }
 }

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

@@ -30,13 +30,13 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
 
   @Test
   public void submitWithFastForward() throws Exception {
-    RevCommit oldHead = getRemoteHead();
+    RevCommit initialHead = getRemoteHead();
     PushOneCommit.Result change = createChange();
     submit(change.getChangeId());
-    RevCommit head = getRemoteHead();
+    RevCommit updatedHead = getRemoteHead();
-    assertThat(head.getId()).isEqualTo(change.getCommit());
+    assertThat(updatedHead.getId()).isEqualTo(change.getCommit());
-    assertThat(head.getParent(0)).isEqualTo(oldHead);
+    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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
     assertSubmittedTogether(id1, id2, id1);
     assertSubmittedTogether(id2, id2, id1);
 
-    RefUpdatedEvent refUpdate = eventRecorder.getOneRefUpdate(
+    assertRefUpdatedEvents(initialHead, updatedHead);
-        project.get(), "refs/heads/master");
+    //TODO(dpursehouse) why are change-merged events in reverse order?
-    assertThat(refUpdate.refUpdate.get().oldRev).isEqualTo(originalHead.name());
+    assertChangeMergedEvents(change2.getChangeId(), updatedHead.name(),
-    assertThat(refUpdate.refUpdate.get().newRev).isEqualTo(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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
         "Failed to submit 2 changes due to the following problems:\n"
         + "Change " + id1 + ": needs Code-Review");
 
-    RevCommit head = getRemoteHead();
+    RevCommit updatedHead = getRemoteHead();
-    assertThat(head.getId()).isEqualTo(oldHead.getId());
+    assertThat(updatedHead.getId()).isEqualTo(initialHead.getId());
+    assertRefUpdatedEvents();
+    assertChangeMergedEvents();
   }
 
   @Test
@@ -109,7 +117,7 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
         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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
         "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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
     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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
       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 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
         .isEqualTo(c1.getId());
 
     submit(id1);
+    RevCommit headAfterSubmit = getRemoteHead();
 
     assertThat(getRemoteHead().getId()).isEqualTo(c1.getId());
     assertSubmitter(id1, 1);
+
+    assertRefUpdatedEvents(initialHead, headAfterSubmit);
+    assertChangeMergedEvents(id1, headAfterSubmit.name());
   }
 }

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

@@ -18,15 +18,11 @@ import static com.google.common.truth.Truth.assertThat;
 
 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 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
 
   @Test
   public void submitWithMergeIfFastForwardPossible() throws Exception {
-    RevCommit oldHead = getRemoteHead();
+    RevCommit initialHead = getRemoteHead();
     PushOneCommit.Result change = createChange();
     submit(change.getChangeId());
-    RevCommit head = getRemoteHead();
+    RevCommit headAfterSubmit = getRemoteHead();
-    assertThat(head.getParentCount()).isEqualTo(2);
+    assertThat(headAfterSubmit.getParentCount()).isEqualTo(2);
-    assertThat(head.getParent(0)).isEqualTo(oldHead);
+    assertThat(headAfterSubmit.getParent(0)).isEqualTo(initialHead);
-    assertThat(head.getParent(1)).isEqualTo(change.getCommit());
+    assertThat(headAfterSubmit.getParent(1)).isEqualTo(change.getCommit());
     assertSubmitter(change.getChangeId(), 1);
-    assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
+    assertPersonEquals(admin.getIdent(), headAfterSubmit.getAuthorIdent());
-    assertPersonEquals(serverIdent.get(), head.getCommitterIdent());
+    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 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
     assertPersonEquals(serverIdent.get(),
         headAfterSecondSubmit.getCommitterIdent());
 
-    // The two submit operations should have resulted in two ref-update events
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
-    List<RefEvent> refUpdates = eventRecorder.getRefUpdates(
+        headAfterFirstSubmit, headAfterSecondSubmit);
-        project.get(), "refs/heads/master", 2);
+    //TODO(dpursehouse) why are change-merged events in reverse order?
-
+    assertChangeMergedEvents(change2.getChangeId(), headAfterFirstSubmit.name(),
-    RefUpdateAttribute refUpdate =
+        change4.getChangeId(), headAfterSecondSubmit.name(),
-        ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
+        change3.getChangeId(), headAfterSecondSubmit.name());
-    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());
   }
 }

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

@@ -13,13 +13,11 @@ import com.google.gerrit.extensions.api.projects.BranchInput;
 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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
   @Test
   public void submitWithFastForward() throws Exception {
-    RevCommit oldHead = getRemoteHead();
+    RevCommit initialHead = getRemoteHead();
     PushOneCommit.Result change = createChange();
     submit(change.getChangeId());
-    RevCommit head = getRemoteHead();
+    RevCommit updatedHead = getRemoteHead();
-    assertThat(head.getId()).isEqualTo(change.getCommit());
+    assertThat(updatedHead.getId()).isEqualTo(change.getCommit());
-    assertThat(head.getParent(0)).isEqualTo(oldHead);
+    assertThat(updatedHead.getParent(0)).isEqualTo(initialHead);
     assertSubmitter(change.getChangeId(), 1);
-    assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
+    assertPersonEquals(admin.getIdent(), updatedHead.getAuthorIdent());
-    assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
+    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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertNew(change2.getChangeId());
 
     // The two submit operations should have resulted in two ref-update events
-    List<RefEvent> refUpdates = eventRecorder.getRefUpdates(
+    // and three change-merged events.
-        project.get(), "refs/heads/master", 2);
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
-
+        headAfterFirstSubmit, headAfterSecondSubmit);
-    RefUpdateAttribute refUpdate =
+    //TODO(dpursehouse) why are change-merged events in reverse order?
-        ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
+    assertChangeMergedEvents(change3.getChangeId(), headAfterFirstSubmit.name(),
-    assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
+        change5.getChangeId(), headAfterSecondSubmit.name(),
-    assertThat(refUpdate.newRev).isEqualTo(headAfterFirstSubmit.name());
+        change4.getChangeId(), headAfterSecondSubmit.name());
-
-    refUpdate = ((RefUpdatedEvent)(refUpdates.get(1))).refUpdate.get();
-    assertThat(refUpdate.oldRev).isEqualTo(headAfterFirstSubmit.name());
-    assertThat(refUpdate.newRev).isEqualTo(headAfterSecondSubmit.name());
-
   }
 
   @Test
@@ -245,10 +244,13 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
   @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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
     submit(change2.getChangeId());
 
-    RevCommit tip1 = getRemoteLog(project, "master").get(0);
+    RevCommit headAfterSecondSubmit = getRemoteLog(project, "master").get(0);
-    assertThat(tip1.getShortMessage()).isEqualTo(
+    assertThat(headAfterSecondSubmit.getShortMessage()).isEqualTo(
         change2.getCommit().getShortMessage());
 
     RevCommit tip2 = getRemoteLog(project, "branch").get(0);
@@ -281,14 +283,21 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
         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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
     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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
     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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
         .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");
+    PushOneCommit.Result change = push.to("refs/for/master");
-    submit(change2result.getChangeId());
+    submit(change.getChangeId());
-    RevCommit tipmaster = getRemoteLog(project, "master").get(0);
+    RevCommit headAfterFirstSubmit = getRemoteLog(project, "master").get(0);
-    assertThat(tipmaster.getShortMessage()).isEqualTo(
+    assertThat(headAfterFirstSubmit.getShortMessage()).isEqualTo(
-        change2result.getCommit().getShortMessage());
+        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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     String changeId = GitUtil.getChangeId(testRepo, merge).get();
     approve(changeId);
     submit(changeId);
-    tipmaster = getRemoteLog(project, "master").get(0);
+    RevCommit headAfterSecondSubmit = getRemoteLog(project, "master").get(0);
-    assertThat(tipmaster.getShortMessage()).isEqualTo(merge.getShortMessage());
+    assertThat(headAfterSecondSubmit.getShortMessage())
+        .isEqualTo(merge.getShortMessage());
+
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
+        headAfterFirstSubmit, headAfterSecondSubmit);
+    assertChangeMergedEvents(
+        change.getChangeId(), headAfterFirstSubmit.name(),
+        changeId, headAfterSecondSubmit.name());
   }
 
   @Test
@@ -435,6 +457,9 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
         "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 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
     // approve and submit the change
     submit(changeResult.getChangeId(), new SubmitInput(),
-        ResourceConflictException.class, "nothing to merge", false);
+        ResourceConflictException.class, "nothing to merge");
+
+    assertRefUpdatedEvents();
+    assertChangeMergedEvents();
   }
 }

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

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 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.Change;
 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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     assertPersonEquals(admin.getIdent(),
         headAfterSecondSubmit.getCommitterIdent());
 
-    ImmutableList<RefEvent> refUpdates = eventRecorder.getRefUpdates(
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
-        project.get(), "refs/heads/master", 2);
+        headAfterFirstSubmit, headAfterSecondSubmit);
-
+    assertChangeMergedEvents(change.getChangeId(), headAfterFirstSubmit.name(),
-    RefUpdateAttribute refUpdate =
+        change2.getChangeId(), headAfterSecondSubmit.name());
-        ((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());
   }
 
+  @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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     assertThat(grandparent).isEqualTo(change1.getCommit());
     assertCurrentRevision(change1.getChangeId(), 1, grandparent);
 
-    ImmutableList<RefEvent> refUpdates = eventRecorder.getRefUpdates(
+    assertRefUpdatedEvents(initialHead, headAfterFirstSubmit,
-        project.get(), "refs/heads/master", 2);
+        headAfterFirstSubmit, headAfterSecondSubmit);
-    RefUpdateAttribute refUpdate =
+    assertChangeMergedEvents(change1.getChangeId(), headAfterFirstSubmit.name(),
-        ((RefUpdatedEvent)(refUpdates.get(0))).refUpdate.get();
+        change2.getChangeId(), headAfterSecondSubmit.name(),
-    assertThat(refUpdate.oldRev).isEqualTo(initialHead.name());
+        change3.getChangeId(), headAfterSecondSubmit.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());
   }
 
   @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();
+    RevCommit headAfterThirdSubmit = getRemoteHead();
-    assertThat(head.getParent(0)).isEqualTo(oldHead);
+    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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
         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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
         "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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
         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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
       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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
       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 @@ 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();
+
     // Create two commits and push.
     RevCommit c1 = commitBuilder()
         .add("a.txt", "1")
@@ -294,10 +309,20 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     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 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
     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());
   }
 }