Use class level @UseLocalDisk annotation Repeated usage of @UseLocalDisk causes the sandboxing of the tests, which in turn starts and stops gerrit instances, consuming more memory and potentially leading to OOM as identified by [1]. Use a class level @UseLocalDisk annotation to avoid such behavior. Using @UseLocalDisk however irrevocably set the cache.directory in configuration, without possibility to override it via a @GerritConfiguration annotation. For this reason, H2MigrationIT tests have been split into two suites, one that runs tests that require local disk (and cache.directory set) and one that runs tests that do not require so. [1] https://bugs.chromium.org/p/gerrit/issues/detail?id=14678 Bug: Issue 14687 Change-Id: I6692438895035501cedb5b383b0c7e022840df70
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java index e7074e0..c3d39f7 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java
@@ -19,6 +19,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; +import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseLocalDisk; import com.google.gerrit.acceptance.UseSsh; @@ -33,6 +34,8 @@ @TestPlugin( name = "cache-chroniclemap", sshModule = "com.googlesource.gerrit.modules.cache.chroniclemap.SSHCommandModule") +@UseLocalDisk +@Sandboxed public class AnalyzeH2CachesIT extends LightweightPluginDaemonTest { @Inject private SitePaths sitePaths; @@ -40,7 +43,6 @@ private String cmd = Joiner.on(" ").join("cache-chroniclemap", "analyze-h2-caches"); @Test - @UseLocalDisk public void shouldAnalyzeH2Cache() throws Exception { createChange(); @@ -55,7 +57,6 @@ } @Test - @UseLocalDisk public void shouldProduceWarningWhenCacheFileIsEmpty() throws Exception { List<String> expected = ImmutableList.of( @@ -73,9 +74,7 @@ } @Test - @UseLocalDisk public void shouldIgnoreNonH2Files() throws Exception { - Path cacheDirectory = sitePaths.resolve(cfg.getString("cache", null, "directory")); Files.write(cacheDirectory.resolve("some.dat"), "some_content".getBytes()); @@ -95,7 +94,6 @@ } @Test - @UseLocalDisk public void shouldFailWhenCacheDirectoryDoesNotExists() throws Exception { cfg.setString("cache", null, "directory", "/tmp/non_existing_directory");
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java new file mode 100644 index 0000000..5480001 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java
@@ -0,0 +1,75 @@ +// Copyright (C) 2021 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.modules.cache.chroniclemap; + +import static com.google.common.net.HttpHeaders.CONTENT_TYPE; +import static com.google.common.truth.Truth.assertThat; +import static org.apache.http.HttpHeaders.ACCEPT; +import static org.eclipse.jgit.util.HttpSupport.TEXT_PLAIN; + +import com.google.gerrit.acceptance.LightweightPluginDaemonTest; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.acceptance.RestSession; +import com.google.gerrit.acceptance.TestPlugin; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.inject.Inject; +import java.io.IOException; +import org.apache.http.message.BasicHeader; +import org.junit.Test; + +@TestPlugin( + name = "cache-chroniclemap", + httpModule = "com.googlesource.gerrit.modules.cache.chroniclemap.HttpModule") +public class MigrateH2CachesInMemoryIT extends LightweightPluginDaemonTest { + private static final String MIGRATION_ENDPOINT = "/plugins/cache-chroniclemap/migrate"; + + @Inject protected GitRepositoryManager repoManager; + + @Test + public void shouldReturnTexPlain() throws Exception { + RestResponse result = runMigration(adminRestSession); + assertThat(result.getHeader(CONTENT_TYPE)).contains(TEXT_PLAIN); + } + + @Test + public void shouldReturnBadRequestWhenTextPlainIsNotAnAcceptedHeader() throws Exception { + runMigrationWithAcceptHeader(adminRestSession, "application/json").assertBadRequest(); + } + + @Test + public void shouldFailWhenUserHasNoAdminServerCapability() throws Exception { + RestResponse result = runMigration(userRestSession); + result.assertForbidden(); + assertThat(result.getEntityContent()) + .contains("administrateServer for plugin cache-chroniclemap not permitted"); + } + + @Test + public void shouldFailWhenCacheDirectoryIsNotDefined() throws Exception { + RestResponse result = runMigration(adminRestSession); + result.assertBadRequest(); + assertThat(result.getEntityContent()) + .contains("Cannot run migration, cache directory is not configured"); + } + + private RestResponse runMigration(RestSession restSession) throws IOException { + return runMigrationWithAcceptHeader(restSession, TEXT_PLAIN); + } + + private RestResponse runMigrationWithAcceptHeader(RestSession restSession, String acceptHeader) + throws IOException { + return restSession.putWithHeader(MIGRATION_ENDPOINT, new BasicHeader(ACCEPT, acceptHeader)); + } +}
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java similarity index 89% rename from src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesIT.java rename to src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java index cc600b5..d2523f0 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.modules.cache.chroniclemap; -import static com.google.common.net.HttpHeaders.CONTENT_TYPE; import static com.google.common.truth.Truth.assertThat; import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.H2_SUFFIX; import static com.googlesource.gerrit.modules.cache.chroniclemap.H2MigrationServlet.DEFAULT_MAX_BLOAT_FACTOR; @@ -61,7 +60,8 @@ @TestPlugin( name = "cache-chroniclemap", httpModule = "com.googlesource.gerrit.modules.cache.chroniclemap.HttpModule") -public class MigrateH2CachesIT extends LightweightPluginDaemonTest { +@UseLocalDisk +public class MigrateH2CachesLocalDiskIT extends LightweightPluginDaemonTest { private final Duration LOAD_CACHE_WAIT_TIMEOUT = Duration.ofSeconds(4); private String ACCOUNTS_CACHE_NAME = "accounts"; private String PERSISTED_PROJECTS_CACHE_NAME = "persisted_projects"; @@ -80,38 +80,21 @@ } @Test - @UseLocalDisk public void shouldRunAndCompleteSuccessfullyWhenCacheDirectoryIsDefined() throws Exception { runMigration(adminRestSession).assertOK(); } @Test - @UseLocalDisk - public void shouldReturnTexPlain() throws Exception { - RestResponse result = runMigration(adminRestSession); - assertThat(result.getHeader(CONTENT_TYPE)).contains(TEXT_PLAIN); - } - - @Test - @UseLocalDisk - public void shouldReturnBadRequestWhenTextPlainIsNotAnAcceptedHeader() throws Exception { - runMigrationWithAcceptHeader(adminRestSession, "application/json").assertBadRequest(); - } - - @Test - @UseLocalDisk public void shouldReturnSuccessWhenAllTextContentsAreAccepted() throws Exception { runMigrationWithAcceptHeader(adminRestSession, "text/*").assertOK(); } @Test - @UseLocalDisk public void shouldReturnSuccessWhenAllContentsAreAccepted() throws Exception { runMigrationWithAcceptHeader(adminRestSession, "*/*").assertOK(); } @Test - @UseLocalDisk public void shouldOutputChronicleMapBloatedDefaultConfiguration() throws Exception { waitForCacheToLoad(ACCOUNTS_CACHE_NAME); waitForCacheToLoad(PERSISTED_PROJECTS_CACHE_NAME); @@ -137,7 +120,6 @@ } @Test - @UseLocalDisk public void shouldOutputChronicleMapBloatedProvidedConfiguration() throws Exception { waitForCacheToLoad(ACCOUNTS_CACHE_NAME); waitForCacheToLoad(PERSISTED_PROJECTS_CACHE_NAME); @@ -164,23 +146,6 @@ } @Test - public void shouldFailWhenCacheDirectoryIsNotDefined() throws Exception { - RestResponse result = runMigration(adminRestSession); - result.assertBadRequest(); - assertThat(result.getEntityContent()) - .contains("Cannot run migration, cache directory is not configured"); - } - - @Test - public void shouldFailWhenUserHasNoAdminServerCapability() throws Exception { - RestResponse result = runMigration(userRestSession); - result.assertForbidden(); - assertThat(result.getEntityContent()) - .contains("administrateServer for plugin cache-chroniclemap not permitted"); - } - - @Test - @UseLocalDisk public void shouldMigrateAccountsCache() throws Exception { waitForCacheToLoad(ACCOUNTS_CACHE_NAME); @@ -195,7 +160,6 @@ } @Test - @UseLocalDisk public void shouldMigratePersistentProjects() throws Exception { waitForCacheToLoad(PERSISTED_PROJECTS_CACHE_NAME);