ReplicationStorageIT: Move shouldMatch* tests from ReplicationIT These tests are focused on verifying storage, so they belong in ReplicationStorageIT. Improve these tests to better verify storage correctness by switching the 'now' parameter to false such that replicationDelay is honored and follow the ReplicationStorageIT pattern using a very long delay. These improvements make these tests much more stable. The tests improve the ref matching slightly by comparing to the PushOne.ALL_REFS constant. Also removes the disableDeleteForTesting flag as there are no users of it now. A later change can add ReplicationIT e2e tests for these use cases. Change-Id: Iaa14a7429a40fb62325259efa1c7d7637deef95a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java index bfb7e95..fd92e27 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
@@ -37,8 +37,6 @@ public class ReplicationTasksStorage { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private boolean disableDeleteForTesting; - public static class ReplicateRefUpdate { public final String project; public final String ref; @@ -91,21 +89,11 @@ return eventKey; } - @VisibleForTesting - public void disableDeleteForTesting(boolean deleteDisabled) { - this.disableDeleteForTesting = deleteDisabled; - } - public void delete(ReplicateRefUpdate r) { String key = r.project + "\n" + r.ref + "\n" + r.uri + "\n" + r.remote; String taskKey = sha1(key).name(); Path file = refUpdates().resolve(taskKey); - if (disableDeleteForTesting) { - logger.atFine().log("DELETE %s (%s:%s => %s) DISABLED", file, r.project, r.ref, r.uri); - return; - } - try { logger.atFine().log("DELETE %s (%s:%s => %s)", file, r.project, r.ref, r.uri); Files.delete(file);
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 1b7de36..225aa3e 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
@@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.testing.GerritJUnit.assertThrows; -import static com.googlesource.gerrit.plugins.replication.PushResultProcessing.NO_OP; import static java.util.stream.Collectors.toList; import com.google.common.flogger.FluentLogger; @@ -24,7 +23,6 @@ import com.google.gerrit.acceptance.PushOneCommit.Result; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseLocalDisk; -import com.google.gerrit.extensions.annotations.PluginData; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.common.ProjectInfo; @@ -33,18 +31,13 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; -import com.google.inject.Key; -import com.googlesource.gerrit.plugins.replication.ReplicationTasksStorage.ReplicateRefUpdate; import java.io.IOException; -import java.nio.file.DirectoryStream; -import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.function.Supplier; -import java.util.regex.Pattern; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -72,31 +65,20 @@ @Inject private SitePaths sitePaths; @Inject private DynamicSet<ProjectDeletedListener> deletedListeners; - private Path pluginDataDir; private Path gitPath; - private Path storagePath; private FileBasedConfig config; - private ReplicationTasksStorage tasksStorage; @Override public void setUpTestPlugin() throws Exception { gitPath = sitePaths.site_path.resolve("git"); - config = new FileBasedConfig(sitePaths.etc_dir.resolve("replication.config").toFile(), FS.DETECTED); + config.save(); setReplicationDestination( "remote1", "suffix1", Optional.of("not-used-project")); // Simulates a full replication.config initialization - config.save(); - super.setUpTestPlugin(); - - pluginDataDir = plugin.getSysInjector().getInstance(Key.get(Path.class, PluginData.class)); - storagePath = pluginDataDir.resolve("ref-updates"); - tasksStorage = plugin.getSysInjector().getInstance(ReplicationTasksStorage.class); - cleanupReplicationTasks(); - tasksStorage.disableDeleteForTesting(true); } @Test @@ -215,49 +197,6 @@ } @Test - public void shouldMatchTemplatedURL() throws Exception { - createTestProject("projectreplica"); - - setReplicationDestination("foo", "replica", ALL_PROJECTS); - reloadConfig(); - - String urlMatch = gitPath.resolve("${name}" + "replica" + ".git").toString(); - String expectedURI = gitPath.resolve(project + "replica" + ".git").toString(); - - plugin - .getSysInjector() - .getInstance(ReplicationQueue.class) - .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true); - - assertThat(listReplicationTasks(".*all.*")).hasSize(1); - for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) { - assertThat(task.uri).isEqualTo(expectedURI); - } - } - - @Test - public void shouldMatchRealURL() throws Exception { - createTestProject("projectreplica"); - - setReplicationDestination("foo", "replica", ALL_PROJECTS); - reloadConfig(); - - String urlMatch = gitPath.resolve(project + "replica" + ".git").toString(); - String expectedURI = urlMatch; - - plugin - .getSysInjector() - .getInstance(ReplicationQueue.class) - .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true); - - assertThat(listReplicationTasks(".*")).hasSize(1); - for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) { - assertThat(task.uri).isEqualTo(expectedURI); - } - assertThat(tasksStorage.list()).isNotEmpty(); - } - - @Test public void shouldReplicateHeadUpdate() throws Exception { setReplicationDestination("foo", "replica", ALL_PROJECTS); reloadConfig(); @@ -397,21 +336,6 @@ plugin.getSysInjector().getInstance(AutoReloadConfigDecorator.class).shutdown(); } - private List<ReplicateRefUpdate> listReplicationTasks(String refRegex) { - Pattern refmaskPattern = Pattern.compile(refRegex); - return tasksStorage.list().stream() - .filter(task -> refmaskPattern.matcher(task.ref).matches()) - .collect(toList()); - } - - private void cleanupReplicationTasks() throws IOException { - try (DirectoryStream<Path> files = Files.newDirectoryStream(storagePath)) { - for (Path path : files) { - path.toFile().delete(); - } - } - } - private boolean nonEmptyProjectExists(Project.NameKey name) { try (Repository r = repoManager.openRepository(name)) { return !r.getAllRefsByPeeledObjectId().isEmpty();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java index 662e57d..da064fb 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java
@@ -195,6 +195,50 @@ assertThat(isPushCompleted(target2, changeRef2, TEST_PUSH_TIMEOUT)).isEqualTo(true); } + @Test + public void shouldMatchTemplatedUrl() throws Exception { + createTestProject("projectreplica"); + + setReplicationDestination("foo", "replica", ALL_PROJECTS, Integer.MAX_VALUE); + reloadConfig(); + + String urlMatch = gitPath.resolve("${name}" + "replica" + ".git").toString(); + String expectedURI = gitPath.resolve(project + "replica" + ".git").toString(); + + plugin + .getSysInjector() + .getInstance(ReplicationQueue.class) + .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), false); + + assertThat(tasksStorage.list()).hasSize(1); + for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) { + assertThat(task.uri).isEqualTo(expectedURI); + assertThat(task.ref).isEqualTo(PushOne.ALL_REFS); + } + } + + @Test + public void shouldMatchRealUrl() throws Exception { + createTestProject("projectreplica"); + + setReplicationDestination("foo", "replica", ALL_PROJECTS, Integer.MAX_VALUE); + reloadConfig(); + + String urlMatch = gitPath.resolve(project + "replica" + ".git").toString(); + String expectedURI = urlMatch; + + plugin + .getSysInjector() + .getInstance(ReplicationQueue.class) + .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), false); + + assertThat(tasksStorage.list()).hasSize(1); + for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) { + assertThat(task.uri).isEqualTo(expectedURI); + assertThat(task.ref).isEqualTo(PushOne.ALL_REFS); + } + } + private void setReplicationDestination( String remoteName, String replicaSuffix, Optional<String> project) throws IOException { setReplicationDestination(