Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
ReplicationIT: fix flakiness
ReplicationIT: reduce execution time
ReplicationIT: Split waitUntil to a separate class and fix it
ReplicationIT: Fix typo in constant name
Change-Id: Ie8894e1e366647bdc893b89409615c290af52c6b
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 a8d075d..64397f9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
@@ -16,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.flogger.FluentLogger;
import com.google.common.hash.Hashing;
import com.google.gson.Gson;
@@ -35,6 +36,8 @@
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;
@@ -81,11 +84,21 @@
return eventKey;
}
+ @VisibleForTesting
+ public void disableDeleteForTesting(boolean deleteDisabled) {
+ this.disableDeleteForTesting = deleteDisabled;
+ }
+
public void delete(ReplicateRefUpdate r) {
String taskJson = GSON.toJson(r) + "\n";
String taskKey = sha1(taskJson).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 7517c14..777fe47 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import static java.util.stream.Collectors.toList;
-import com.google.common.base.Stopwatch;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit.Result;
@@ -29,22 +28,17 @@
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.SitePaths;
-import com.google.gson.Gson;
import com.google.inject.Inject;
import com.google.inject.Key;
import com.googlesource.gerrit.plugins.replication.ReplicationTasksStorage.ReplicateRefUpdate;
-import java.io.BufferedReader;
import java.io.IOException;
-import java.nio.charset.StandardCharsets;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
-import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import org.eclipse.jgit.lib.Constants;
@@ -62,8 +56,8 @@
public class ReplicationIT extends LightweightPluginDaemonTest {
private static final Optional<String> ALL_PROJECTS = Optional.empty();
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static final int TEST_REPLICATION_DELAY = 5;
- private static final Duration TEST_TIMEMOUT = Duration.ofSeconds(TEST_REPLICATION_DELAY * 10);
+ private static final int TEST_REPLICATION_DELAY = 1;
+ private static final Duration TEST_TIMEOUT = Duration.ofSeconds(TEST_REPLICATION_DELAY * 2);
@Inject private SitePaths sitePaths;
@Inject private ProjectOperations projectOperations;
@@ -71,7 +65,7 @@
private Path gitPath;
private Path storagePath;
private FileBasedConfig config;
- private Gson GSON = new Gson();
+ private ReplicationTasksStorage tasksStorage;
@Override
public void setUpTestPlugin() throws Exception {
@@ -89,18 +83,21 @@
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
public void shouldReplicateNewProject() throws Exception {
setReplicationDestination("foo", "replica", ALL_PROJECTS);
reloadConfig();
- waitForEmptyTasks();
Project.NameKey sourceProject = projectOperations.newProject().name("foo").create();
assertThat(listReplicationTasks("refs/meta/config")).hasSize(1);
- waitUntil(() -> gitPath.resolve(sourceProject + "replica.git").toFile().isDirectory());
+
+ waitUntil(() -> projectExists(new Project.NameKey(sourceProject + "replica.git")));
ProjectInfo replicaProject = gApi.projects().name(sourceProject + "replica").get();
assertThat(replicaProject).isNotNull();
@@ -113,7 +110,6 @@
setReplicationDestination("foo", "replica", ALL_PROJECTS);
reloadConfig();
- waitForEmptyTasks();
Result pushResult = createChange();
RevCommit sourceCommit = pushResult.getCommit();
@@ -134,7 +130,6 @@
public void shouldReplicateNewBranch() throws Exception {
setReplicationDestination("foo", "replica", ALL_PROJECTS);
reloadConfig();
- waitForEmptyTasks();
Project.NameKey targetProject =
projectOperations.newProject().name(project + "replica").create();
@@ -167,7 +162,6 @@
setReplicationDestination("foo1", "replica1", ALL_PROJECTS);
setReplicationDestination("foo2", "replica2", ALL_PROJECTS);
reloadConfig();
- waitForEmptyTasks();
Result pushResult = createChange();
RevCommit sourceCommit = pushResult.getCommit();
@@ -199,12 +193,16 @@
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);
reloadConfig();
- waitForEmptyTasks();
createChange();
assertThat(listReplicationTasks("refs/changes/\\d*/\\d*/\\d*")).hasSize(4);
+
+ setReplicationDestination("foo1", replicaSuffixes, ALL_PROJECTS);
+ setReplicationDestination("foo2", replicaSuffixes, ALL_PROJECTS);
}
@Test
@@ -263,49 +261,33 @@
}
private void waitUntil(Supplier<Boolean> waitCondition) throws InterruptedException {
- Stopwatch stopwatch = Stopwatch.createStarted();
- while (!waitCondition.get() && stopwatch.elapsed().compareTo(TEST_TIMEMOUT) < 0) {
- TimeUnit.SECONDS.sleep(1);
- }
+ WaitUtil.waitUntil(waitCondition, TEST_TIMEOUT);
}
private void reloadConfig() {
plugin.getSysInjector().getInstance(AutoReloadConfigDecorator.class).forceReload();
}
- private void waitForEmptyTasks() throws InterruptedException {
- waitUntil(
- () -> {
- try {
- return listReplicationTasks(".*").size() == 0;
- } catch (Exception e) {
- logger.atSevere().withCause(e).log("Failed to list replication tasks");
- throw new IllegalStateException(e);
- }
- });
+ 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 List<ReplicateRefUpdate> listReplicationTasks(String refRegex) throws IOException {
- Pattern refmaskPattern = Pattern.compile(refRegex);
- List<ReplicateRefUpdate> tasks = new ArrayList<>();
+ private void cleanupReplicationTasks() throws IOException {
try (DirectoryStream<Path> files = Files.newDirectoryStream(storagePath)) {
for (Path path : files) {
- ReplicateRefUpdate task = readTask(path);
- if (refmaskPattern.matcher(task.ref).matches()) {
- tasks.add(readTask(path));
- }
+ path.toFile().delete();
}
}
-
- return tasks;
}
- private ReplicateRefUpdate readTask(Path file) {
- try (BufferedReader reader = Files.newBufferedReader(file, StandardCharsets.UTF_8)) {
- return GSON.fromJson(reader, ReplicateRefUpdate.class);
+ private boolean projectExists(Project.NameKey name) {
+ try (Repository r = repoManager.openRepository(name)) {
+ return true;
} catch (Exception e) {
- logger.atSevere().withCause(e).log("failed to read replication task %s", file);
- throw new IllegalStateException(e);
+ return false;
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtil.java b/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtil.java
new file mode 100644
index 0000000..586b56c
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtil.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 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.googlesource.gerrit.plugins.replication;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import com.google.common.base.Stopwatch;
+import java.time.Duration;
+import java.util.function.Supplier;
+
+public class WaitUtil {
+ public static void waitUntil(Supplier<Boolean> waitCondition, Duration timeout)
+ throws InterruptedException {
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ while (!waitCondition.get()) {
+ if (stopwatch.elapsed().compareTo(timeout) > 0) {
+ throw new InterruptedException();
+ }
+ MILLISECONDS.sleep(50);
+ }
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtilTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtilTest.java
new file mode 100644
index 0000000..0ccb0af
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/WaitUtilTest.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2019 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.googlesource.gerrit.plugins.replication;
+
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.googlesource.gerrit.plugins.replication.WaitUtil.waitUntil;
+
+import java.time.Duration;
+import org.junit.Test;
+
+public class WaitUtilTest {
+
+ @Test
+ public void shouldFailWhenConditionNotMetWithinTimeout() throws Exception {
+ assertThrows(
+ InterruptedException.class,
+ () -> waitUntil(() -> returnTrue() == false, Duration.ofSeconds(1)));
+ }
+
+ @Test
+ public void shouldNotFailWhenConditionIsMetWithinTimeout() throws Exception {
+ waitUntil(() -> returnTrue() == true, Duration.ofSeconds(1));
+ }
+
+ private static boolean returnTrue() {
+ return true;
+ }
+}
