Refactor event capturing in submit tests for more precise assertions
Factor the event capturing out to a new class, EventRecorder, which listens to the events and records those that are of type RefEvent. Events are recorded in a map keyed on a combination of the event type, project name and ref (branch name). In each map entry the received events are stored in a list. This allows us to make assertions about the events that were received for a project and branch, and in which order they were received. Convert the existing tests of change-merged events to use the new class. Also extend the submit-by-fast-forward tests to assert that when a stack of changes is submitted by fast-forward, there was only one ref-updated event for that project/branch and it contains: - old ref: the sha1 of the branch's original HEAD - new ref: the sha1 of the change at the top of the stack Bug: Issue 4123 Change-Id: Ic978fb28fda601c9ab5897e0a0e2f9280d59216c
This commit is contained in:
@@ -0,0 +1,119 @@
|
|||||||
|
// Copyright (C) 2016 The Android Open Source Project
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
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.FluentIterable;
|
||||||
|
import com.google.common.collect.LinkedListMultimap;
|
||||||
|
import com.google.common.collect.Multimap;
|
||||||
|
import com.google.gerrit.common.UserScopedEventListener;
|
||||||
|
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.events.ChangeMergedEvent;
|
||||||
|
import com.google.gerrit.server.events.Event;
|
||||||
|
import com.google.gerrit.server.events.RefEvent;
|
||||||
|
import com.google.gerrit.server.events.RefUpdatedEvent;
|
||||||
|
import com.google.inject.Inject;
|
||||||
|
import com.google.inject.Singleton;
|
||||||
|
|
||||||
|
import java.util.Collection;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
public class EventRecorder {
|
||||||
|
private final RegistrationHandle eventListenerRegistration;
|
||||||
|
private final Multimap<String, RefEvent> recordedEvents;
|
||||||
|
|
||||||
|
@Singleton
|
||||||
|
public static class Factory {
|
||||||
|
private final DynamicSet<UserScopedEventListener> eventListeners;
|
||||||
|
private final IdentifiedUser.GenericFactory userFactory;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
Factory(DynamicSet<UserScopedEventListener> eventListeners,
|
||||||
|
IdentifiedUser.GenericFactory userFactory) {
|
||||||
|
this.eventListeners = eventListeners;
|
||||||
|
this.userFactory = userFactory;
|
||||||
|
}
|
||||||
|
|
||||||
|
public EventRecorder create(TestAccount user) {
|
||||||
|
return new EventRecorder(eventListeners, userFactory.create(user.id));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public EventRecorder(DynamicSet<UserScopedEventListener> eventListeners,
|
||||||
|
final IdentifiedUser user) {
|
||||||
|
recordedEvents = LinkedListMultimap.create();
|
||||||
|
|
||||||
|
eventListenerRegistration = eventListeners.add(
|
||||||
|
new UserScopedEventListener() {
|
||||||
|
@Override
|
||||||
|
public void onEvent(Event e) {
|
||||||
|
if (e instanceof RefEvent) {
|
||||||
|
RefEvent event = (RefEvent) e;
|
||||||
|
String key = key(event.getType(), event.getProjectNameKey().get(),
|
||||||
|
event.getRefName());
|
||||||
|
recordedEvents.put(key, event);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public CurrentUser getUser() {
|
||||||
|
return user;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
private static String key(String type, String project, String ref) {
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
|
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) {
|
||||||
|
assertThat(input).isInstanceOf(ChangeMergedEvent.class);
|
||||||
|
ChangeMergedEvent e = (ChangeMergedEvent) input;
|
||||||
|
return e.change.get().number.equals(changeNumber);
|
||||||
|
}})
|
||||||
|
.toList();
|
||||||
|
assertThat(events).hasSize(1);
|
||||||
|
return (ChangeMergedEvent) events.get(0);
|
||||||
|
}
|
||||||
|
|
||||||
|
public void close() {
|
||||||
|
eventListenerRegistration.remove();
|
||||||
|
}
|
||||||
|
}
|
@@ -27,10 +27,10 @@ import com.google.common.base.Function;
|
|||||||
import com.google.common.collect.Iterables;
|
import com.google.common.collect.Iterables;
|
||||||
import com.google.common.collect.Lists;
|
import com.google.common.collect.Lists;
|
||||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||||
|
import com.google.gerrit.acceptance.EventRecorder;
|
||||||
import com.google.gerrit.acceptance.NoHttpd;
|
import com.google.gerrit.acceptance.NoHttpd;
|
||||||
import com.google.gerrit.acceptance.PushOneCommit;
|
import com.google.gerrit.acceptance.PushOneCommit;
|
||||||
import com.google.gerrit.acceptance.TestProjectInput;
|
import com.google.gerrit.acceptance.TestProjectInput;
|
||||||
import com.google.gerrit.common.UserScopedEventListener;
|
|
||||||
import com.google.gerrit.extensions.api.changes.SubmitInput;
|
import com.google.gerrit.extensions.api.changes.SubmitInput;
|
||||||
import com.google.gerrit.extensions.api.projects.BranchInfo;
|
import com.google.gerrit.extensions.api.projects.BranchInfo;
|
||||||
import com.google.gerrit.extensions.api.projects.ProjectInput;
|
import com.google.gerrit.extensions.api.projects.ProjectInput;
|
||||||
@@ -41,8 +41,6 @@ import com.google.gerrit.extensions.client.SubmitType;
|
|||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
import com.google.gerrit.extensions.common.ChangeMessageInfo;
|
import com.google.gerrit.extensions.common.ChangeMessageInfo;
|
||||||
import com.google.gerrit.extensions.common.LabelInfo;
|
import com.google.gerrit.extensions.common.LabelInfo;
|
||||||
import com.google.gerrit.extensions.registration.DynamicSet;
|
|
||||||
import com.google.gerrit.extensions.registration.RegistrationHandle;
|
|
||||||
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
||||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
import com.google.gerrit.extensions.webui.UiAction;
|
import com.google.gerrit.extensions.webui.UiAction;
|
||||||
@@ -52,14 +50,9 @@ import com.google.gerrit.reviewdb.client.PatchSet;
|
|||||||
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
import com.google.gerrit.reviewdb.client.PatchSetApproval;
|
||||||
import com.google.gerrit.reviewdb.client.Project;
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
import com.google.gerrit.server.ApprovalsUtil;
|
import com.google.gerrit.server.ApprovalsUtil;
|
||||||
import com.google.gerrit.server.CurrentUser;
|
|
||||||
import com.google.gerrit.server.IdentifiedUser;
|
|
||||||
import com.google.gerrit.server.change.RevisionResource;
|
import com.google.gerrit.server.change.RevisionResource;
|
||||||
import com.google.gerrit.server.change.Submit;
|
import com.google.gerrit.server.change.Submit;
|
||||||
import com.google.gerrit.server.data.ChangeAttribute;
|
|
||||||
import com.google.gerrit.server.data.PatchSetAttribute;
|
|
||||||
import com.google.gerrit.server.events.ChangeMergedEvent;
|
import com.google.gerrit.server.events.ChangeMergedEvent;
|
||||||
import com.google.gerrit.server.events.Event;
|
|
||||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||||
import com.google.gerrit.testutil.ConfigSuite;
|
import com.google.gerrit.testutil.ConfigSuite;
|
||||||
import com.google.gerrit.testutil.TestTimeUtil;
|
import com.google.gerrit.testutil.TestTimeUtil;
|
||||||
@@ -77,39 +70,27 @@ import org.eclipse.jgit.revwalk.RevWalk;
|
|||||||
import org.junit.After;
|
import org.junit.After;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
import org.slf4j.Logger;
|
|
||||||
import org.slf4j.LoggerFactory;
|
|
||||||
|
|
||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
import java.util.HashMap;
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
|
||||||
|
|
||||||
@NoHttpd
|
@NoHttpd
|
||||||
public abstract class AbstractSubmit extends AbstractDaemonTest {
|
public abstract class AbstractSubmit extends AbstractDaemonTest {
|
||||||
private static final Logger log =
|
|
||||||
LoggerFactory.getLogger(AbstractSubmit.class);
|
|
||||||
|
|
||||||
@ConfigSuite.Config
|
@ConfigSuite.Config
|
||||||
public static Config submitWholeTopicEnabled() {
|
public static Config submitWholeTopicEnabled() {
|
||||||
return submitWholeTopicEnabledConfig();
|
return submitWholeTopicEnabledConfig();
|
||||||
}
|
}
|
||||||
|
|
||||||
private Map<String, String> changeMergedEvents;
|
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
private ApprovalsUtil approvalsUtil;
|
private ApprovalsUtil approvalsUtil;
|
||||||
|
|
||||||
@Inject
|
|
||||||
private IdentifiedUser.GenericFactory factory;
|
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
private Submit submitHandler;
|
private Submit submitHandler;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
DynamicSet<UserScopedEventListener> eventListeners;
|
private EventRecorder.Factory eventRecorderFactory;
|
||||||
|
|
||||||
private RegistrationHandle eventListenerRegistration;
|
protected EventRecorder eventRecorder;
|
||||||
|
|
||||||
private String systemTimeZone;
|
private String systemTimeZone;
|
||||||
|
|
||||||
@@ -127,31 +108,12 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
|
|||||||
|
|
||||||
@Before
|
@Before
|
||||||
public void setUp() throws Exception {
|
public void setUp() throws Exception {
|
||||||
changeMergedEvents = new HashMap<>();
|
eventRecorder = eventRecorderFactory.create(user);
|
||||||
eventListenerRegistration =
|
|
||||||
eventListeners.add(new UserScopedEventListener() {
|
|
||||||
@Override
|
|
||||||
public void onEvent(Event event) {
|
|
||||||
if (!(event instanceof ChangeMergedEvent)) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
ChangeMergedEvent e = (ChangeMergedEvent) event;
|
|
||||||
ChangeAttribute c = e.change.get();
|
|
||||||
PatchSetAttribute ps = e.patchSet.get();
|
|
||||||
log.debug("Merged {},{} as {}", ps.number, c.number, e.newRev);
|
|
||||||
changeMergedEvents.put(e.change.get().number, e.newRev);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public CurrentUser getUser() {
|
|
||||||
return factory.create(user.id);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@After
|
@After
|
||||||
public void cleanup() {
|
public void cleanup() {
|
||||||
eventListenerRegistration.remove();
|
eventRecorder.close();
|
||||||
db.close();
|
db.close();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -305,10 +267,12 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
|
|||||||
// newRev of the ChangeMergedEvent.
|
// newRev of the ChangeMergedEvent.
|
||||||
BranchInfo branch = gApi.projects().name(change.project)
|
BranchInfo branch = gApi.projects().name(change.project)
|
||||||
.branch(change.branch).get();
|
.branch(change.branch).get();
|
||||||
assertThat(changeMergedEvents).isNotEmpty();
|
ChangeMergedEvent event = eventRecorder.getOneChangeMerged(
|
||||||
String newRev = changeMergedEvents.get(Integer.toString(change._number));
|
change.project,
|
||||||
assertThat(newRev).isNotNull();
|
change.branch,
|
||||||
assertThat(branch.revision).isEqualTo(newRev);
|
Integer.toString(change._number));
|
||||||
|
assertThat(event.newRev).isNotNull();
|
||||||
|
assertThat(branch.revision).isEqualTo(event.newRev);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected void assertCurrentRevision(String changeId, int expectedNum,
|
protected void assertCurrentRevision(String changeId, int expectedNum,
|
||||||
|
@@ -30,6 +30,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
|||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||||
import com.google.gerrit.server.change.Submit.TestSubmitInput;
|
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.ObjectId;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
import org.eclipse.jgit.lib.Repository;
|
||||||
@@ -60,6 +61,8 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void submitTwoChangesWithFastForward() throws Exception {
|
public void submitTwoChangesWithFastForward() throws Exception {
|
||||||
|
RevCommit originalHead = getRemoteHead();
|
||||||
|
|
||||||
PushOneCommit.Result change = createChange();
|
PushOneCommit.Result change = createChange();
|
||||||
PushOneCommit.Result change2 = createChange();
|
PushOneCommit.Result change2 = createChange();
|
||||||
|
|
||||||
@@ -68,15 +71,20 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
|
|||||||
approve(id1);
|
approve(id1);
|
||||||
submit(id2);
|
submit(id2);
|
||||||
|
|
||||||
RevCommit head = getRemoteHead();
|
RevCommit updatedHead = getRemoteHead();
|
||||||
assertThat(head.getId()).isEqualTo(change2.getCommit());
|
assertThat(updatedHead.getId()).isEqualTo(change2.getCommit());
|
||||||
assertThat(head.getParent(0).getId()).isEqualTo(change.getCommit());
|
assertThat(updatedHead.getParent(0).getId()).isEqualTo(change.getCommit());
|
||||||
assertSubmitter(change.getChangeId(), 1);
|
assertSubmitter(change.getChangeId(), 1);
|
||||||
assertSubmitter(change2.getChangeId(), 1);
|
assertSubmitter(change2.getChangeId(), 1);
|
||||||
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
|
assertPersonEquals(admin.getIdent(), updatedHead.getAuthorIdent());
|
||||||
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
|
assertPersonEquals(admin.getIdent(), updatedHead.getCommitterIdent());
|
||||||
assertSubmittedTogether(id1, id2, id1);
|
assertSubmittedTogether(id1, id2, id1);
|
||||||
assertSubmittedTogether(id2, 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());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@@ -19,7 +19,7 @@ import com.google.gerrit.reviewdb.client.Change;
|
|||||||
import com.google.gerrit.server.data.AccountAttribute;
|
import com.google.gerrit.server.data.AccountAttribute;
|
||||||
|
|
||||||
public class ChangeMergedEvent extends PatchSetEvent {
|
public class ChangeMergedEvent extends PatchSetEvent {
|
||||||
static final String TYPE = "change-merged";
|
public static final String TYPE = "change-merged";
|
||||||
public Supplier<AccountAttribute> submitter;
|
public Supplier<AccountAttribute> submitter;
|
||||||
public String newRev;
|
public String newRev;
|
||||||
|
|
||||||
|
@@ -20,7 +20,7 @@ import com.google.gerrit.server.data.AccountAttribute;
|
|||||||
import com.google.gerrit.server.data.RefUpdateAttribute;
|
import com.google.gerrit.server.data.RefUpdateAttribute;
|
||||||
|
|
||||||
public class RefUpdatedEvent extends RefEvent {
|
public class RefUpdatedEvent extends RefEvent {
|
||||||
static final String TYPE = "ref-updated";
|
public static final String TYPE = "ref-updated";
|
||||||
public Supplier<AccountAttribute> submitter;
|
public Supplier<AccountAttribute> submitter;
|
||||||
public Supplier<RefUpdateAttribute> refUpdate;
|
public Supplier<RefUpdateAttribute> refUpdate;
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user