From 882c6147720227c161a2fb573c79cfc683e70379 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Thu, 8 Oct 2020 23:17:03 +0100 Subject: Split replication plugins tests in two groups Run unit-tests and integration tests in parallel by splitting them into two separate tasks. This also allows to potentially identify which group of tests is flaky, because Bazel would flag one or the other in case of instability. Change-Id: I21f969a17e3653dfc5ab93d71cc6955024fc2d8f --- BUILD | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/BUILD b/BUILD index 50615d8..72a3fc8 100644 --- a/BUILD +++ b/BUILD @@ -23,6 +23,18 @@ junit_tests( name = "replication_tests", srcs = glob([ "src/test/java/**/*Test.java", + ]), + tags = ["replication"], + visibility = ["//visibility:public"], + deps = PLUGIN_TEST_DEPS + PLUGIN_DEPS + [ + ":replication__plugin", + ":replication_util", + ], +) + +junit_tests( + name = "replication_it", + srcs = glob([ "src/test/java/**/*IT.java", ]), tags = ["replication"], -- cgit v1.2.3 From 5529649274286edbb7559a3af13724cdcb90f1c3 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Thu, 1 Oct 2020 12:03:02 -0600 Subject: ReplicationIT: Fix invalid replicationDelay setting Setting config values for a remote in replication.config vs the remote's own config file results in the replication.config values being ignored. Fix this by setting the values in each remote's config file. This test had delays added to avoid any flakiness, but the delays weren't working because of this issue. While the test generally passes, the delay makes it safer from races. Change-Id: Idcdf5f07b3fc91724068ec6216527665c4a48bb3 --- .../gerrit/plugins/replication/ReplicationIT.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 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 d8d3384..f0ea138 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java @@ -229,10 +229,12 @@ public class ReplicationIT extends LightweightPluginDaemonTest { createTestProject("projectreplica1"); createTestProject("projectreplica2"); - setReplicationDestination("foo1", replicaSuffixes, ALL_PROJECTS); - setReplicationDestination("foo2", replicaSuffixes, ALL_PROJECTS); - config.setInt("remote", "foo1", "replicationDelay", TEST_REPLICATION_DELAY * 100); - config.setInt("remote", "foo2", "replicationDelay", TEST_REPLICATION_DELAY * 100); + FileBasedConfig dest1 = setReplicationDestination("foo1", replicaSuffixes, ALL_PROJECTS); + FileBasedConfig dest2 = setReplicationDestination("foo2", replicaSuffixes, ALL_PROJECTS); + dest1.setInt("remote", null, "replicationDelay", TEST_REPLICATION_DELAY * 100); + dest2.setInt("remote", null, "replicationDelay", TEST_REPLICATION_DELAY * 100); + dest1.save(); + dest2.save(); reloadConfig(); createChange(); @@ -439,13 +441,13 @@ public class ReplicationIT extends LightweightPluginDaemonTest { setReplicationDestination(remoteName, Arrays.asList(replicaSuffix), project, replicationDelay); } - private void setReplicationDestination( + private FileBasedConfig setReplicationDestination( String remoteName, List replicaSuffixes, Optional project) throws IOException { - setReplicationDestination(remoteName, replicaSuffixes, project, TEST_REPLICATION_DELAY); + return setReplicationDestination(remoteName, replicaSuffixes, project, TEST_REPLICATION_DELAY); } - private void setReplicationDestination( + private FileBasedConfig setReplicationDestination( String remoteName, List replicaSuffixes, Optional project, @@ -461,6 +463,7 @@ public class ReplicationIT extends LightweightPluginDaemonTest { config.setInt("remote", remoteName, "replicationRetry", TEST_REPLICATION_RETRY); project.ifPresent(prj -> config.setString("remote", remoteName, "projects", prj)); config.save(); + return config; } private void setProjectDeletionReplication(String remoteName, boolean replicateProjectDeletion) -- cgit v1.2.3 From 84d96eb953d51c97b2093d06597bc69812b812e7 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Mon, 12 Oct 2020 11:07:40 -0600 Subject: ReplicationIT: Remove unnecessary storage inspection Integration tests shouldn't need to rely on inspecting the underlying ReplicationTasksStorage layer(s). All of these tests already verify the expected end result. This leaves 4 tests that currently completely rely on inspecting the task storage to verify the expected result. Those tests need further improvement to decouple from the storage layer. Change-Id: I029d63ce7d07414d9bf5d9290d556378beedcabf --- .../googlesource/gerrit/plugins/replication/ReplicationIT.java | 8 -------- 1 file changed, 8 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 f0ea138..89ee20a 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java @@ -107,8 +107,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest { Project.NameKey sourceProject = createTestProject("foo"); - assertThat(listReplicationTasks("refs/meta/config")).hasSize(1); - WaitUtil.waitUntil( () -> nonEmptyProjectExists(new Project.NameKey(sourceProject + "replica.git")), TEST_NEW_PROJECT_TIMEOUT); @@ -156,8 +154,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest { RevCommit sourceCommit = pushResult.getCommit(); String sourceRef = pushResult.getPatchSet().getRefName(); - assertThat(listReplicationTasks("refs/changes/\\d*/\\d*/\\d*")).hasSize(1); - try (Repository repo = repoManager.openRepository(targetProject)) { waitUntil(() -> checkedGetRef(repo, sourceRef) != null); @@ -179,8 +175,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest { input.revision = master; gApi.projects().name(project.get()).branch(newBranch).create(input); - assertThat(listReplicationTasks("refs/heads/(mybranch|master)")).hasSize(2); - try (Repository repo = repoManager.openRepository(targetProject); Repository sourceRepo = repoManager.openRepository(project)) { waitUntil(() -> checkedGetRef(repo, newBranch) != null); @@ -205,8 +199,6 @@ public class ReplicationIT extends LightweightPluginDaemonTest { RevCommit sourceCommit = pushResult.getCommit(); String sourceRef = pushResult.getPatchSet().getRefName(); - assertThat(listReplicationTasks("refs/changes/\\d*/\\d*/\\d*")).hasSize(2); - try (Repository repo1 = repoManager.openRepository(targetProject1); Repository repo2 = repoManager.openRepository(targetProject2)) { waitUntil( -- cgit v1.2.3