Refactor ReviewersConfigIT and ReviewersIT
Extract common code into AbstractReviewersPluginTest.
Change-Id: I97e98560898ae37c058a2d06e0f6020a013c8f96
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java
new file mode 100644
index 0000000..383dcfd
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/AbstractReviewersPluginTest.java
@@ -0,0 +1,88 @@
+// 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.gerrit.acceptance.GitUtil.fetch;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.RefNames;
+import com.google.inject.Inject;
+import java.util.Arrays;
+import java.util.List;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+
+/** Base class for reviewer plugin tests. */
+public class AbstractReviewersPluginTest extends LightweightPluginDaemonTest {
+ @Inject protected ProjectOperations projectOperations;
+
+ protected void createFilters(FilterData... filters) throws Exception {
+ createFiltersFor(testRepo, filters);
+ }
+
+ protected void createFiltersFor(TestRepository<?> repo, FilterData... filters) throws Exception {
+ String previousHead = repo.getRepository().getBranch();
+ checkoutRefsMetaConfig(repo);
+ Config cfg = new Config();
+ Arrays.stream(filters)
+ .forEach(f -> cfg.setStringList(SECTION_FILTER, f.filter, KEY_REVIEWER, f.reviewers));
+ pushFactory
+ .create(admin.newIdent(), repo, "Add reviewers", FILENAME, cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+ repo.reset(previousHead);
+ }
+
+ protected TestRepository<?> checkoutRefsMetaConfig(TestRepository<?> repo) throws Exception {
+ fetch(repo, RefNames.REFS_CONFIG + ":refs/heads/config");
+ repo.reset("refs/heads/config");
+ return repo;
+ }
+
+ protected FilterData filter(String filter) {
+ return new FilterData(filter);
+ }
+
+ /** Assists tests to define a filter. */
+ protected static class FilterData {
+ List<String> reviewers;
+ String filter;
+
+ FilterData(String filter) {
+ this.filter = filter;
+ this.reviewers = Lists.newArrayList();
+ }
+
+ FilterData reviewer(TestAccount reviewer) {
+ return reviewer(reviewer.email());
+ }
+
+ FilterData reviewer(String reviewer) {
+ reviewers.add(reviewer);
+ return this;
+ }
+
+ public ReviewerFilterSection asSection() {
+ return new ReviewerFilterSection(filter, ImmutableSet.copyOf(reviewers));
+ }
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
index 1a280e2..19bb268 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
@@ -14,97 +14,60 @@
package com.googlesource.gerrit.plugins.reviewers;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.fetch;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
-import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
-import com.google.inject.Inject;
+import java.util.Arrays;
import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
@TestPlugin(name = "reviewers", sysModule = "com.googlesource.gerrit.plugins.reviewers.Module")
-public class ReviewersConfigIT extends LightweightPluginDaemonTest {
+public class ReviewersConfigIT extends AbstractReviewersPluginTest {
private static final String BRANCH_MAIN = "branch:main";
private static final String NO_FILTER = "*";
private static final String JANE_DOE = "jane.doe@example.com";
private static final String JOHN_DOE = "john.doe@example.com";
- @Inject private ProjectOperations projectOperations;
-
@Before
public void setUp() throws Exception {
- fetch(testRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
- testRepo.reset("refs/heads/config");
+ checkoutRefsMetaConfig(testRepo);
}
@Test
public void reviewersConfigSingle() throws Exception {
- Config cfg = new Config();
- cfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JOHN_DOE);
- cfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JANE_DOE);
+ createFilters(filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).reviewer(JANE_DOE));
- pushFactory
- .create(admin.newIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
- .to(RefNames.REFS_CONFIG)
- .assertOkStatus();
-
- assertThat(reviewersConfig().forProject(project).getReviewerFilterSections())
- .containsExactlyElementsIn(
- ImmutableList.of(
- new ReviewerFilterSection(NO_FILTER, ImmutableSet.of(JOHN_DOE)),
- new ReviewerFilterSection(BRANCH_MAIN, ImmutableSet.of(JANE_DOE))))
- .inOrder();
+ assertProjectHasFilters(
+ project, filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).reviewer(JANE_DOE));
}
@Test
public void reviewersConfigWithMergedInheritance() throws Exception {
- Config parentCfg = new Config();
- parentCfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JOHN_DOE);
- parentCfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JOHN_DOE);
-
- pushFactory
- .create(
- admin.newIdent(),
- testRepo,
- "Add reviewers parent project",
- FILENAME,
- parentCfg.toText())
- .to(RefNames.REFS_CONFIG)
- .assertOkStatus();
-
Project.NameKey childProject = projectOperations.newProject().parent(project).create();
- TestRepository<?> childTestRepo = cloneProject(childProject);
- fetch(childTestRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
- childTestRepo.reset("refs/heads/config");
+ TestRepository<?> childTestRepo = checkoutRefsMetaConfig(cloneProject(childProject));
- Config cfg = new Config();
- cfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JANE_DOE);
- cfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JANE_DOE);
+ createFiltersFor(
+ childTestRepo,
+ filter(NO_FILTER).reviewer(JANE_DOE),
+ filter(BRANCH_MAIN).reviewer(JANE_DOE));
- pushFactory
- .create(
- admin.newIdent(), childTestRepo, "Add reviewers child project", FILENAME, cfg.toText())
- .to(RefNames.REFS_CONFIG)
- .assertOkStatus();
+ createFilters(filter(NO_FILTER).reviewer(JOHN_DOE), filter(BRANCH_MAIN).reviewer(JOHN_DOE));
- assertThat(reviewersConfig().forProject(childProject).getReviewerFilterSections())
+ assertProjectHasFilters(
+ childProject,
+ filter(NO_FILTER).reviewer(JOHN_DOE).reviewer(JANE_DOE),
+ filter(BRANCH_MAIN).reviewer(JOHN_DOE).reviewer(JANE_DOE));
+ }
+
+ private void assertProjectHasFilters(Project.NameKey project, FilterData... filters) {
+ assertThat(reviewersConfig().forProject(project).getReviewerFilterSections())
.containsExactlyElementsIn(
- ImmutableList.of(
- new ReviewerFilterSection(NO_FILTER, ImmutableSet.of(JOHN_DOE, JANE_DOE)),
- new ReviewerFilterSection(BRANCH_MAIN, ImmutableSet.of(JOHN_DOE, JANE_DOE))))
+ Arrays.stream(filters).map(f -> f.asSection()).collect(toImmutableList()))
.inOrder();
}
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 77a60d4..a2a7035 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -15,42 +15,30 @@
package com.googlesource.gerrit.plugins.reviewers;
import static com.google.common.truth.Truth.assertThat;
-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;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
-import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestPlugin;
-import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.inject.Inject;
-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.TestModule")
-public class ReviewersIT extends LightweightPluginDaemonTest {
- @Inject private ProjectOperations projectOperations;
+public class ReviewersIT extends AbstractReviewersPluginTest {
@Test
public void addReviewers() throws Exception {
TestAccount user2 = accountCreator.user2();
- setReviewerFilters(filter("*").reviewer(user).reviewer(user2));
+ createFilters(filter("*").reviewer(user).reviewer(user2));
String changeId = createChange().getChangeId();
assertThat(reviewersFor(changeId))
.containsExactlyElementsIn(ImmutableSet.of(user.id(), user2.id()));
@@ -59,7 +47,7 @@
@Test
public void addReviewersMatchMultipleSections() throws Exception {
TestAccount user2 = accountCreator.user2();
- setReviewerFilters(filter("*").reviewer(user), filter("\"^a.txt\"").reviewer(user2));
+ createFilters(filter("*").reviewer(user), filter("\"^a.txt\"").reviewer(user2));
String changeId = createChange().getChangeId();
assertThat(reviewersFor(changeId))
.containsExactlyElementsIn(ImmutableSet.of(user.id(), user2.id()));
@@ -67,7 +55,7 @@
@Test
public void doNotAddReviewersFromNonMatchingFilters() throws Exception {
- setReviewerFilters(filter("branch:master").reviewer(user));
+ createFilters(filter("branch:master").reviewer(user));
createBranch(BranchNameKey.create(project, "other-branch"));
// Create a change that matches the filter section.
createChange("refs/for/master");
@@ -78,7 +66,7 @@
@Test
public void addReviewersFromMatchingFilters() throws Exception {
- setReviewerFilters(filter("branch:other-branch").reviewer(user));
+ createFilters(filter("branch:other-branch").reviewer(user));
// Create a change that doesn't match the filter section.
createChange("refs/for/master");
// The actual change we want to test
@@ -89,7 +77,7 @@
@Test
public void dontAddReviewersForPrivateChange() throws Exception {
- setReviewerFilters(filter("*").reviewer(user));
+ createFilters(filter("*").reviewer(user));
PushOneCommit.Result r = createChange("refs/for/master%private");
assertThat(r.getChange().change().isPrivate()).isTrue();
assertNoReviewersAddedFor(r.getChangeId());
@@ -97,7 +85,7 @@
@Test
public void privateBitFlippedAndReviewersAddedOnSubmit() throws Exception {
- setReviewerFilters(filter("*").reviewer(user));
+ createFilters(filter("*").reviewer(user));
PushOneCommit.Result r = createChange("refs/for/master%private");
assertThat(r.getChange().change().isPrivate()).isTrue();
String changeId = r.getChangeId();
@@ -114,7 +102,7 @@
@Test
public void reviewerAddedOnPrivateBitFlip() throws Exception {
- setReviewerFilters(filter("*").reviewer(user));
+ createFilters(filter("*").reviewer(user));
PushOneCommit.Result r = createChange("refs/for/master%private");
assertThat(r.getChange().change().isPrivate()).isTrue();
String changeId = r.getChangeId();
@@ -125,20 +113,6 @@
assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(user.id()));
}
- private void setReviewerFilters(Filter... filters) throws Exception {
- fetch(testRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
- testRepo.reset("refs/heads/config");
- Config cfg = new Config();
- for (Filter f : filters) {
- cfg.setStringList(SECTION_FILTER, f.filter, KEY_REVIEWER, f.reviewers);
- }
- pushFactory
- .create(admin.newIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
- .to(RefNames.REFS_CONFIG)
- .assertOkStatus();
- testRepo.reset(projectOperations.project(project).getHead("master"));
- }
-
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))
@@ -148,23 +122,4 @@
private void assertNoReviewersAddedFor(String changeId) throws Exception {
assertThat(gApi.changes().id(changeId).get().reviewers.get(REVIEWER)).isNull();
}
-
- private Filter filter(String filter) {
- return new Filter(filter);
- }
-
- private class Filter {
- List<String> reviewers;
- String filter;
-
- Filter(String filter) {
- this.filter = filter;
- this.reviewers = Lists.newArrayList();
- }
-
- Filter reviewer(TestAccount reviewer) {
- reviewers.add(reviewer.email());
- return this;
- }
- }
}
