ReviewersIT: Stop using Thread#sleep
Instead run reviewer adding with a direct executor in tests.
Feature: Issue 12610
Change-Id: I234f2d0abb3be8de86884cf88b3a828ae26a69c3
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
index 5f5b8ae..bcf586e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -47,6 +47,7 @@
@Override
protected void configure() {
+ bindWorkQueue();
bind(CapabilityDefinition.class)
.annotatedWith(Exports.named(MODIFY_REVIEWERS_CONFIG))
.to(ModifyReviewersConfigCapability.class);
@@ -89,4 +90,8 @@
}
});
}
+
+ protected void bindWorkQueue() {
+ bind(ReviewerWorkQueue.class).to(ReviewerWorkQueue.Scheduled.class);
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerWorkQueue.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerWorkQueue.java
new file mode 100644
index 0000000..d4241c7
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerWorkQueue.java
@@ -0,0 +1,46 @@
+// Copyright (C) 2020 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.reviewers;
+
+import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
+
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.inject.Inject;
+
+interface ReviewerWorkQueue {
+ void submit(AddReviewers addReviewers);
+
+ static class Scheduled implements ReviewerWorkQueue {
+ private final WorkQueue workQueue;
+
+ @Inject
+ Scheduled(WorkQueue workQueue) {
+ this.workQueue = workQueue;
+ }
+
+ @Override
+ public void submit(AddReviewers addReviewers) {
+ workQueue.getDefaultQueue().submit(addReviewers);
+ }
+ }
+
+ static class Direct implements ReviewerWorkQueue {
+
+ @Override
+ public void submit(AddReviewers addReviewers) {
+ directExecutor().execute(addReviewers);
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index b7c5f49..1aea95e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -36,7 +36,6 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestedReviewer;
-import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Inject;
@@ -44,7 +43,6 @@
import com.google.inject.Singleton;
import java.util.List;
import java.util.Set;
-import java.util.concurrent.Future;
/** Handles automatic adding of reviewers and reviewer suggestions. */
@Singleton
@@ -57,7 +55,7 @@
private final ReviewersResolver resolver;
private final AddReviewers.Factory addReviewersFactory;
- private final WorkQueue workQueue;
+ private final ReviewerWorkQueue workQueue;
private final ReviewersConfig config;
private final Provider<CurrentUser> user;
private final ChangeQueryBuilder queryBuilder;
@@ -67,7 +65,7 @@
Reviewers(
ReviewersResolver resolver,
AddReviewers.Factory addReviewersFactory,
- WorkQueue workQueue,
+ ReviewerWorkQueue workQueue,
ReviewersConfig config,
Provider<CurrentUser> user,
ChangeQueryBuilder queryBuilder,
@@ -159,9 +157,7 @@
final AddReviewers addReviewers =
addReviewersFactory.create(
c, resolver.resolve(reviewers, projectName, changeNumber, uploader));
-
- @SuppressWarnings("unused")
- Future<?> ignored = workQueue.getDefaultQueue().submit(addReviewers);
+ workQueue.submit(addReviewers);
} catch (QueryParseException e) {
logger.atWarning().log(
"Could not add default reviewers for change %d of project %s, filter is invalid: %s",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/TestModule.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/TestModule.java
new file mode 100644
index 0000000..380280a
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/TestModule.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2020 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.reviewers;
+
+public class TestModule extends Module {
+
+ public TestModule() {
+ super(true, false);
+ }
+
+ @Override
+ protected void bindWorkQueue() {
+ bind(ReviewerWorkQueue.class).to(ReviewerWorkQueue.Direct.class);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
index 592f9ca..df25b27 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.reviewers;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
@@ -33,17 +32,14 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
-import java.util.Collection;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@NoHttpd
-@TestPlugin(name = "reviewers", sysModule = "com.googlesource.gerrit.plugins.reviewers.Module")
+@TestPlugin(name = "reviewers", sysModule = "com.googlesource.gerrit.plugins.reviewers.TestModule")
public class ReviewersIT extends LightweightPluginDaemonTest {
@Inject private ProjectOperations projectOperations;
@@ -101,36 +97,14 @@
testRepo.reset(projectOperations.project(project).getHead("master"));
}
- private Set<Account.Id> reviewersFor(String changeId)
- throws RestApiException, InterruptedException {
- Collection<AccountInfo> reviewers;
- // Wait for 100 ms until the create patch set event
- // is processed by the reviewers plugin
- long wait = 0;
- do {
- reviewers = gApi.changes().id(changeId).get().reviewers.get(REVIEWER);
- if (reviewers == null) {
- Thread.sleep(10);
- wait += 10;
- if (wait > 100) {
- assertWithMessage("Timeout of 100 ms exceeded").fail();
- }
- }
- } while (reviewers == null);
- return reviewers.stream().map(a -> Account.id(a._accountId)).collect(toSet());
+ private Set<Account.Id> reviewersFor(String changeId) throws Exception {
+ return gApi.changes().id(changeId).get().reviewers.get(REVIEWER).stream()
+ .map(a -> Account.id(a._accountId))
+ .collect(toSet());
}
- private void assertNoReviewersAddedFor(String changeId)
- throws InterruptedException, RestApiException {
- Collection<AccountInfo> reviewers;
- long wait = 0;
- do {
- Thread.sleep(10);
- wait += 10;
- reviewers = gApi.changes().id(changeId).get().reviewers.get(REVIEWER);
- } while (reviewers == null && wait < 100);
-
- assertThat(reviewers).isNull();
+ private void assertNoReviewersAddedFor(String changeId) throws Exception {
+ assertThat(gApi.changes().id(changeId).get().reviewers.get(REVIEWER)).isNull();
}
private Filter filter(String filter) {
