Fix flakiness in ReplicationIT for pending events firing

Fix the shouldFirePendingOnlyToStoredUri test by making sure
that events are NOT executed by the replication engine until
the tests has completed the preparation phase.

The Gerrit build on stable-2.16 became flaky right afterward
the merge of the new shouldFirePendingOnlyToStoredUri test which
highlighted the flakiness.

The test wants to simulate a situation where a ref-update needs
to be propagated to two remotes: remote1 and remote2.
For doing so, it configures the two remotes and crates a change
for generating the two replication tasks files on the filesystem.
Then, it looks for the events associated for remote1 and removes
them, so that the next replication queue startup won't find it
and won't replicate the change to remote1.

During the interval of time between the creation of the change
and the removal of the underlying replication task on the filesystem,
the replication task could have been executed already and the
test failed.

Make sure that the replication does not kick in by
setting the replication timeout to Integer.MAX_VALUE at the
beginning. Then, once the replication task file is removed on the
filesystem, set it back to default and reload the configuration to
trigger the firing of the events.

Remove also the explicit start/stop of the replication queue, as
the config reload is already a stop/start process and it
automatically triggering an event replay.

Change-Id: Ifd591da37e94b6ce8f281cb0404f3f3c737489f3

1 files changed, 24 insertions, 24 deletions

diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
index aa15e3a..8da9da2 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
@@ -370,21 +370,23 @@ public class ReplicationIT extends LightweightPluginDaemonTest {
     Project.NameKey target2 = createTestProject("project" + suffix2);
     String remote1 = "foo1";
     String remote2 = "foo2";
-    setReplicationDestination(remote1, suffix1, ALL_PROJECTS);
-    setReplicationDestination(remote2, suffix2, ALL_PROJECTS);
+    setReplicationDestination(remote1, suffix1, ALL_PROJECTS, Integer.MAX_VALUE);
+    setReplicationDestination(remote2, suffix2, ALL_PROJECTS, Integer.MAX_VALUE);
     reloadConfig();
 
     Result pushResult = createChange();
     String sourceRef = pushResult.getPatchSet().getRefName();
 
-    replicationQueueStop();
-
     tasksStorage.disableDeleteForTesting(false);
     listReplicationTasks("refs/changes/\\d*/\\d*/\\d*").stream()
         .filter(task -> remote1.equals(task.remote))
         .forEach(u -> tasksStorage.delete(u));
     tasksStorage.disableDeleteForTesting(true);
 
+    setReplicationDestination(remote1, suffix1, ALL_PROJECTS);
+    setReplicationDestination(remote2, suffix2, ALL_PROJECTS);
+    reloadConfig();
+
     assertThat(
             listReplicationTasks("refs/changes/\\d*/\\d*/\\d*").stream()
                 .filter(task -> remote2.equals(task.remote))
@@ -397,8 +399,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest {
                 .collect(toList()))
         .hasSize(0);
 
-    replicationQueueStart();
-
     assertThat(isPushCompleted(target2, sourceRef, TEST_TIMEOUT)).isEqualTo(true);
     assertThat(isPushCompleted(target1, sourceRef, TEST_TIMEOUT)).isEqualTo(false);
   }
@@ -433,19 +433,35 @@ public class ReplicationIT extends LightweightPluginDaemonTest {
 
   private void setReplicationDestination(
       String remoteName, String replicaSuffix, Optional<String> project) throws IOException {
-    setReplicationDestination(remoteName, Arrays.asList(replicaSuffix), project);
+    setReplicationDestination(
+        remoteName, Arrays.asList(replicaSuffix), project, TEST_REPLICATION_DELAY);
+  }
+
+  private void setReplicationDestination(
+      String remoteName, String replicaSuffix, Optional<String> project, int replicationDelay)
+      throws IOException {
+    setReplicationDestination(remoteName, Arrays.asList(replicaSuffix), project, replicationDelay);
   }
 
   private void setReplicationDestination(
       String remoteName, List<String> replicaSuffixes, Optional<String> project)
       throws IOException {
+    setReplicationDestination(remoteName, replicaSuffixes, project, TEST_REPLICATION_DELAY);
+  }
+
+  private void setReplicationDestination(
+      String remoteName,
+      List<String> replicaSuffixes,
+      Optional<String> project,
+      int replicationDelay)
+      throws IOException {
 
     List<String> replicaUrls =
         replicaSuffixes.stream()
             .map(suffix -> gitPath.resolve("${name}" + suffix + ".git").toString())
             .collect(toList());
     config.setStringList("remote", remoteName, "url", replicaUrls);
-    config.setInt("remote", remoteName, "replicationDelay", TEST_REPLICATION_DELAY);
+    config.setInt("remote", remoteName, "replicationDelay", replicationDelay);
     config.setInt("remote", remoteName, "replicationRetry", TEST_REPLICATION_RETRY);
     project.ifPresent(prj -> config.setString("remote", remoteName, "projects", prj));
     config.save();
@@ -469,22 +485,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest {
     plugin.getSysInjector().getInstance(AutoReloadConfigDecorator.class).shutdown();
   }
 
-  private void replicationQueueStart() {
-    getReplicationQueueInstance().start();
-  }
-
-  private void replicationQueueStop() {
-    getReplicationQueueInstance().stop();
-  }
-
-  private ReplicationQueue getReplicationQueueInstance() {
-    return getInstance(ReplicationQueue.class);
-  }
-
-  private <T> T getInstance(Class<T> classObj) {
-    return plugin.getSysInjector().getInstance(classObj);
-  }
-
   private List<ReplicateRefUpdate> listReplicationTasks(String refRegex) {
     Pattern refmaskPattern = Pattern.compile(refRegex);
     return tasksStorage.list().stream()