TestEventPublisher: performance improvement
Separate handling of when an event is expected to exist and not.
* Return early if event exists
Checking every 50 ms saves 10-15 seconds on the current execution
time of the entire test-suite since the event is most often
available before the wait-timeout (500 ms) is complete.
* Handle expected missing events in TestEventsPublisher
By calling the TestEventPublisher to check if an event is missing we
can skip the check-every-x-seconds loop.
This has the added benefit that it removes a lot of copy-paste code
from the test-cases.
Change-Id: I764e87100f2f768d628facd4393b0fc0479aeaef
diff --git a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/TestEventPublisher.java b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/TestEventPublisher.java
index 37e93f2..7b4569e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/TestEventPublisher.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/TestEventPublisher.java
@@ -16,6 +16,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
@@ -32,6 +33,9 @@
@Ignore
public class TestEventPublisher implements Runnable, EiffelEventHub.Consumer {
+ /* Milliseconds after which the publisher-thread is expected to have handled the event. */
+ public static int MILLIS_TO_WAIT = 500;
+
Map<EventKey, EiffelEvent> publishedEvents = Maps.newConcurrentMap();
List<EventKey> actualOrder = Lists.newArrayList();
boolean keepTrying = true;
@@ -88,8 +92,20 @@
keys.size());
}
- public EiffelEvent getPublished(EventKey key) {
+ public void assertNotPublished(EventKey key, String message) {
waitForIt();
+ EiffelEvent event = publishedEvents.getOrDefault(key, null);
+ assertNull(message, event);
+ }
+
+ public EiffelEvent getPublished(EventKey key) {
+ int wait = 50;
+ for (int totalWait = 0; totalWait < MILLIS_TO_WAIT; totalWait += wait) {
+ if (publishedEvents.containsKey(key)) {
+ return publishedEvents.get(key);
+ }
+ waitFor(wait);
+ }
return publishedEvents.getOrDefault(key, null);
}
@@ -106,10 +122,13 @@
pause = true;
}
- /* Give the publisher-thread some time to handle events. */
private void waitForIt() {
+ waitFor(MILLIS_TO_WAIT);
+ }
+
+ private void waitFor(int millis) {
try {
- Thread.sleep(500);
+ Thread.sleep(millis);
} catch (InterruptedException e) {
e.printStackTrace();
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/listeners/GerritEventListenersIT.java b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/listeners/GerritEventListenersIT.java
index 126eb50..3d93b80 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/listeners/GerritEventListenersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/listeners/GerritEventListenersIT.java
@@ -19,7 +19,6 @@
import static com.googlesource.gerrit.plugins.eventseiffel.eiffel.dto.EiffelEventType.SCS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -86,8 +85,8 @@
@Test
public void patchSetCreatedForMetaDoesntResultsInEvent() throws Exception {
- EiffelEvent event = publisher.getPublished(toSccKey(createMetaConfigChange()));
- assertNull("refs/meta/config change shouldn't result in event", event);
+ publisher.assertNotPublished(
+ toSccKey(createMetaConfigChange()), "refs/meta/config change shouldn't result in event");
}
@Test
@@ -153,49 +152,45 @@
@Test
public void patchSetSubmittedForMetaDoesntResultsInEvent() throws Exception {
PushOneCommit.Result res = createMetaConfigChange();
- SourceChangeEventKey sccKey = toSccKey(res);
- EiffelEvent sccEvent = publisher.getPublished(sccKey);
- assertNull("refs/meta/config change shouldn't result in event", sccEvent);
+ publisher.assertNotPublished(
+ toSccKey(res), "refs/meta/config change shouldn't result in event");
+
merge(res);
- EiffelEvent scsEvent = publisher.getPublished(sccKey.copy(SCS));
- assertNull("refs/meta/config change shouldn't result in event", scsEvent);
+ publisher.assertNotPublished(
+ toScsKey(res), "refs/meta/config change shouldn't result in event");
}
@Test
public void noEventsCreatedWhenDisabled() throws Exception {
TestEventListenerConfigProvider.disable();
PushOneCommit.Result res = createChange();
- SourceChangeEventKey sccKey = toSccKey(res);
- EiffelEvent sccEvent = publisher.getPublished(sccKey);
- assertNull("Patch-set created shouldn't result in event when disabled", sccEvent);
+ publisher.assertNotPublished(
+ toSccKey(res), "Patch-set created shouldn't result in event when disabled");
merge(res);
- EiffelEvent scsEvent = publisher.getPublished(sccKey.copy(SCS));
- assertNull("Patch-set submitted shouldn't result in event when disabled", scsEvent);
+ publisher.assertNotPublished(
+ toScsKey(res), "Patch-set submitted shouldn't result in event when disabled");
}
@Test
public void noEventsCreatedWhenRefIsBlocked() throws Exception {
TestEventListenerConfigProvider.setBlockedRefPatterns("refs/heads/master");
PushOneCommit.Result res = createChange();
- SourceChangeEventKey sccKey = toSccKey(res);
- EiffelEvent sccEvent = publisher.getPublished(sccKey);
- assertNull("Patch-set created shouldn't result in event when target ref is blocked", sccEvent);
+ publisher.assertNotPublished(
+ toSccKey(res), "Patch-set created shouldn't result in event when target ref is blocked");
merge(res);
- EiffelEvent scsEvent = publisher.getPublished(sccKey.copy(SCS));
- assertNull(
- "Patch-set submitted shouldn't result in event when target ref is blocked", scsEvent);
+ publisher.assertNotPublished(
+ toScsKey(res), "Patch-set submitted shouldn't result in event when target ref is blocked");
}
@Test
public void noEventsCreatedWhenProjectIsBlocked() throws Exception {
TestEventListenerConfigProvider.setBlockedProjectPatterns(project.get());
PushOneCommit.Result res = createChange();
- SourceChangeEventKey sccKey = toSccKey(res);
- EiffelEvent sccEvent = publisher.getPublished(sccKey);
- assertNull("Patch-set created shouldn't result in event when project is blocked", sccEvent);
+ publisher.assertNotPublished(
+ toSccKey(res), "Patch-set created shouldn't result in event when project is blocked");
merge(res);
- EiffelEvent scsEvent = publisher.getPublished(sccKey.copy(SCS));
- assertNull("Patch-set submitted shouldn't result in event when project is blocked", scsEvent);
+ publisher.assertNotPublished(
+ toScsKey(res), "Patch-set submitted shouldn't result in event when project is blocked");
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/rest/EventsEiffelRestIT.java b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/rest/EventsEiffelRestIT.java
index d483a66..cb77c91 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/rest/EventsEiffelRestIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/rest/EventsEiffelRestIT.java
@@ -18,7 +18,6 @@
import static com.googlesource.gerrit.plugins.eventseiffel.eiffel.dto.EiffelEventType.SCC;
import static com.googlesource.gerrit.plugins.eventseiffel.eiffel.dto.EiffelEventType.SCS;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
@@ -84,9 +83,8 @@
@Test
public void createSccsAsNonAdminForbidden() throws Exception {
createSccs("master", false).assertStatus(403);
- SourceChangeEventKey scc = keyFromMaster(SCC);
- EiffelEvent event = publisher.getPublished(scc);
- assertNull(event);
+ publisher.assertNotPublished(
+ keyFromMaster(SCC), "SCC event creation should be blocked for non admin");
}
@Test
@@ -111,9 +109,8 @@
@Test
public void createScssAsNonAdminForbidden() throws Exception {
createScss("master", false).assertStatus(403);
- SourceChangeEventKey scs = keyFromMaster(SCS);
- EiffelEvent event = publisher.getPublished(scs);
- assertNull(event);
+ publisher.assertNotPublished(
+ keyFromMaster(SCS), "SCS event creation should be blocked for non admin");
}
@Test
@@ -136,9 +133,9 @@
createScss("master", true).assertStatus(202);
String tagName = createTagRef(true).substring(RefNames.REFS_TAGS.length());
createArtcs(tagName, false).assertStatus(403);
- ArtifactEventKey artc = ArtifactEventKey.create(tagPURL(project.get(), tagName));
- EiffelEvent event = publisher.getPublished(artc);
- assertNull(event);
+ publisher.assertNotPublished(
+ ArtifactEventKey.create(tagPURL(project.get(), tagName)),
+ "ArtC event creation should be blocked for non admin");
}
@Test
