diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 06efbfd8b0..09ee8f0663 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -201,7 +201,12 @@ public abstract class AbstractDaemonTest { firstTest = description; } beforeTest(description); - try (ProjectResetter resetter = resetProjects(projectResetter.builder())) { + ProjectResetter.Config input = resetProjects(); + if (input == null) { + input = defaultResetProjects(); + } + + try (ProjectResetter resetter = projectResetter.builder().build(input)) { AbstractDaemonTest.this.resetter = resetter; base.evaluate(); } finally { @@ -317,8 +322,12 @@ public abstract class AbstractDaemonTest { } /** Controls which project and branches should be reset after each test case. */ - protected ProjectResetter resetProjects(ProjectResetter.Builder resetter) throws IOException { - return resetter + protected ProjectResetter.Config resetProjects() { + return null; + } + + private ProjectResetter.Config defaultResetProjects() { + return new ProjectResetter.Config() // Don't reset all refs so that refs/sequences/changes is not touched and change IDs are // not reused. .reset(allProjects, RefNames.REFS_CONFIG) @@ -331,8 +340,7 @@ public abstract class AbstractDaemonTest { RefNames.REFS_USERS + "*", RefNames.REFS_EXTERNAL_IDS, RefNames.REFS_STARRED_CHANGES + "*", - RefNames.REFS_DRAFT_COMMENTS + "*") - .build(); + RefNames.REFS_DRAFT_COMMENTS + "*"); } protected void restartAsSlave() throws Exception { diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java index 086cacb809..36f9b8231e 100644 --- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java +++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java @@ -25,7 +25,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.truth.FailureMetadata; import com.google.common.truth.Subject; import com.google.common.truth.Truth; -import com.google.gerrit.acceptance.ProjectResetter.Builder; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.ReviewInput; @@ -63,10 +62,10 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { } @Override - protected ProjectResetter resetProjects(Builder resetter) throws IOException { + protected ProjectResetter.Config resetProjects() { // Don't reset anything so that stagedUsers can be cached across all tests. // Without this caching these tests become much too slow. - return resetter.build(); + return new ProjectResetter.Config(); } protected static FakeEmailSenderSubject assertThat(FakeEmailSender sender) { diff --git a/java/com/google/gerrit/acceptance/ProjectResetter.java b/java/com/google/gerrit/acceptance/ProjectResetter.java index 569a0c14b1..0e4ea1991b 100644 --- a/java/com/google/gerrit/acceptance/ProjectResetter.java +++ b/java/com/google/gerrit/acceptance/ProjectResetter.java @@ -88,8 +88,6 @@ public class ProjectResetter implements AutoCloseable { @Nullable private final AccountCache accountCache; @Nullable private final ProjectCache projectCache; - private final Multimap refsByProject; - @Inject public Builder( GitRepositoryManager repoManager, @@ -102,10 +100,27 @@ public class ProjectResetter implements AutoCloseable { this.accountCreator = accountCreator; this.accountCache = accountCache; this.projectCache = projectCache; + } + + public ProjectResetter build(ProjectResetter.Config input) throws IOException { + return new ProjectResetter( + repoManager, + allUsersName, + accountCreator, + accountCache, + projectCache, + input.refsByProject); + } + } + + public static class Config { + private final Multimap refsByProject; + + public Config() { this.refsByProject = MultimapBuilder.hashKeys().arrayListValues().build(); } - public Builder reset(Project.NameKey project, String... refPatterns) { + public Config reset(Project.NameKey project, String... refPatterns) { List refPatternList = Arrays.asList(refPatterns); if (refPatternList.isEmpty()) { refPatternList = ImmutableList.of(RefNames.REFS + "*"); @@ -113,11 +128,6 @@ public class ProjectResetter implements AutoCloseable { refsByProject.putAll(project, refPatternList); return this; } - - public ProjectResetter build() throws IOException { - return new ProjectResetter( - repoManager, allUsersName, accountCreator, accountCache, projectCache, refsByProject); - } } @Inject private GitRepositoryManager repoManager; diff --git a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java index 6c3a4b07ce..74b23a4e4b 100644 --- a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java +++ b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java @@ -71,7 +71,8 @@ public class ProjectResetterTest extends GerritBaseTests { public void resetAllRefs() throws Exception { Ref matchingRef = createRef("refs/any/test"); - try (ProjectResetter resetProject = builder().reset(project).build()) { + try (ProjectResetter resetProject = + builder().build(new ProjectResetter.Config().reset(project))) { updateRef(matchingRef); } @@ -87,7 +88,10 @@ public class ProjectResetterTest extends GerritBaseTests { Ref updatedNonMatchingRef; try (ProjectResetter resetProject = - builder().reset(project, "refs/match/*", "refs/another-match/*").build()) { + builder() + .build( + new ProjectResetter.Config() + .reset(project, "refs/match/*", "refs/another-match/*"))) { updateRef(matchingRef); updateRef(anotherMatchingRef); updatedNonMatchingRef = updateRef(nonMatchingRef); @@ -107,7 +111,10 @@ public class ProjectResetterTest extends GerritBaseTests { Ref anotherMatchingRef; Ref nonMatchingRef; try (ProjectResetter resetProject = - builder().reset(project, "refs/match/*", "refs/another-match/*").build()) { + builder() + .build( + new ProjectResetter.Config() + .reset(project, "refs/match/*", "refs/another-match/*"))) { matchingRef = createRef("refs/match/test"); anotherMatchingRef = createRef("refs/another-match/test"); nonMatchingRef = createRef("refs/no-match/test"); @@ -135,7 +142,11 @@ public class ProjectResetterTest extends GerritBaseTests { Ref updatedNonMatchingRefProject1; Ref updatedNonMatchingRefProject2; try (ProjectResetter resetProject = - builder().reset(project, "refs/foo/*").reset(project2, "refs/bar/*").build()) { + builder() + .build( + new ProjectResetter.Config() + .reset(project, "refs/foo/*") + .reset(project2, "refs/bar/*"))) { updateRef(matchingRefProject1); updatedNonMatchingRefProject1 = updateRef(nonMatchingRefProject1); @@ -162,7 +173,11 @@ public class ProjectResetterTest extends GerritBaseTests { Ref matchingRefProject2; Ref nonMatchingRefProject2; try (ProjectResetter resetProject = - builder().reset(project, "refs/foo/*").reset(project2, "refs/bar/*").build()) { + builder() + .build( + new ProjectResetter.Config() + .reset(project, "refs/foo/*") + .reset(project2, "refs/bar/*"))) { matchingRefProject1 = createRef("refs/foo/test"); nonMatchingRefProject1 = createRef("refs/bar/test"); @@ -183,7 +198,9 @@ public class ProjectResetterTest extends GerritBaseTests { public void onlyDeleteNewlyCreatedWithOverlappingRefPatterns() throws Exception { Ref matchingRef; try (ProjectResetter resetProject = - builder().reset(project, "refs/match/*", "refs/match/test").build()) { + builder() + .build( + new ProjectResetter.Config().reset(project, "refs/match/*", "refs/match/test"))) { // This ref matches 2 ref pattern, ProjectResetter should try to delete it only once. matchingRef = createRef("refs/match/test"); } @@ -206,7 +223,8 @@ public class ProjectResetterTest extends GerritBaseTests { Ref nonMetaConfig = createRef("refs/heads/master"); try (ProjectResetter resetProject = - builder(null, null, projectCache).reset(project).reset(project2).build()) { + builder(null, null, projectCache) + .build(new ProjectResetter.Config().reset(project).reset(project2))) { updateRef(nonMetaConfig); updateRef(repo2, metaConfig); } @@ -225,7 +243,8 @@ public class ProjectResetterTest extends GerritBaseTests { EasyMock.replay(projectCache); try (ProjectResetter resetProject = - builder(null, null, projectCache).reset(project).reset(project2).build()) { + builder(null, null, projectCache) + .build(new ProjectResetter.Config().reset(project).reset(project2))) { createRef("refs/heads/master"); createRef(repo2, RefNames.REFS_CONFIG); } @@ -249,7 +268,8 @@ public class ProjectResetterTest extends GerritBaseTests { Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(2))); try (ProjectResetter resetProject = - builder(null, accountCache, null).reset(project).reset(allUsers).build()) { + builder(null, accountCache, null) + .build(new ProjectResetter.Config().reset(project).reset(allUsers))) { updateRef(nonUserBranch); updateRef(allUsersRepo, userBranch); } @@ -269,7 +289,8 @@ public class ProjectResetterTest extends GerritBaseTests { EasyMock.replay(accountCache); try (ProjectResetter resetProject = - builder(null, accountCache, null).reset(project).reset(allUsers).build()) { + builder(null, accountCache, null) + .build(new ProjectResetter.Config().reset(project).reset(allUsers))) { // Non-user branch because it's not in All-Users. createRef(RefNames.refsUsers(new Account.Id(2))); @@ -300,7 +321,8 @@ public class ProjectResetterTest extends GerritBaseTests { Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3))); try (ProjectResetter resetProject = - builder(null, accountCache, null).reset(project).reset(allUsers).build()) { + builder(null, accountCache, null) + .build(new ProjectResetter.Config().reset(project).reset(allUsers))) { updateRef(nonUserBranch); updateRef(allUsersRepo, externalIds); createRef(allUsersRepo, RefNames.refsUsers(accountId2)); @@ -329,7 +351,8 @@ public class ProjectResetterTest extends GerritBaseTests { Ref nonUserBranch = createRef(RefNames.refsUsers(new Account.Id(3))); try (ProjectResetter resetProject = - builder(null, accountCache, null).reset(project).reset(allUsers).build()) { + builder(null, accountCache, null) + .build(new ProjectResetter.Config().reset(project).reset(allUsers))) { updateRef(nonUserBranch); createRef(allUsersRepo, RefNames.REFS_EXTERNAL_IDS); createRef(allUsersRepo, RefNames.refsUsers(accountId2)); @@ -350,7 +373,8 @@ public class ProjectResetterTest extends GerritBaseTests { EasyMock.replay(accountCreator); try (ProjectResetter resetProject = - builder(accountCreator, null, null).reset(project).reset(allUsers).build()) { + builder(accountCreator, null, null) + .build(new ProjectResetter.Config().reset(project).reset(allUsers))) { createRef(allUsersRepo, RefNames.refsUsers(accountId)); } diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index 2d5703d251..32dc5ddbe7 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java @@ -40,7 +40,6 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.ProjectResetter; -import com.google.gerrit.acceptance.ProjectResetter.Builder; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.TestAccount; @@ -166,11 +165,11 @@ public class GroupsIT extends AbstractDaemonTest { } @Override - protected ProjectResetter resetProjects(Builder resetter) throws IOException { + protected ProjectResetter.Config resetProjects() { // Don't reset All-Users since deleting users makes groups inconsistent (e.g. groups would // contain members that no longer exist) and as result of this the group consistency checker // that is executed after each test would fail. - return resetter.reset(allProjects, RefNames.REFS_CONFIG).build(); + return new ProjectResetter.Config().reset(allProjects, RefNames.REFS_CONFIG); } @Test @@ -1076,7 +1075,9 @@ public class GroupsIT extends AbstractDaemonTest { // Use ProjectResetter to restore the group names ref try (ProjectResetter resetter = - projectResetter.builder().reset(allUsers, RefNames.REFS_GROUPNAMES).build()) { + projectResetter + .builder() + .build(new ProjectResetter.Config().reset(allUsers, RefNames.REFS_GROUPNAMES))) { // Manually delete group names ref try (Repository repo = repoManager.openRepository(allUsers); RevWalk rw = new RevWalk(repo)) { diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index e0c16e0f50..2c95e27812 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -785,7 +785,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest { private ProjectResetter resetGroups() throws IOException { return projectResetter .builder() - .reset(allUsers, RefNames.REFS_GROUPS + "*", RefNames.REFS_GROUPNAMES) - .build(); + .build( + new ProjectResetter.Config() + .reset(allUsers, RefNames.REFS_GROUPS + "*", RefNames.REFS_GROUPNAMES)); } }