AbstractDaemonTest: Don't delete common server path too early
Chaining the TemporaryFolder with the custom TestRule results in the directory for the common server being deleted after the first test method, even though it might still be needed. This means a @UseLocalDisk class cannot have more than one method, or things break, including but not limited to the NRT reopen threads throwing exceptions due to the index files disappearing out from under them. We just got lucky that none of our local disk tests were actually affected by this. To make this work, we can't tie the lifetime of the TemporaryFolder to a single AbstractDaemonTest method invocation. Instead, create the tempdir within GerritServer#initAndStart, and allow callers to get it back out. We have to be a bit more careful about managing the lifetime of the tempdir since some GerritServers, particularly in StandaloneSiteTest, have a shorter lifetime than their tempdirs. Fortunately, TempFileUtil make this relatively easy. As a nice side effect, we can get rid of the hack of passing the tempdir in via a Gerrit config option. Change-Id: I80ac66a27bc990f2cef966c9b372c86ce277c471
This commit is contained in:
parent
938398f9dc
commit
91d8fe01f9
@ -151,7 +151,6 @@ import org.junit.AfterClass;
|
||||
import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
import org.junit.rules.ExpectedException;
|
||||
import org.junit.rules.TemporaryFolder;
|
||||
import org.junit.rules.TestRule;
|
||||
import org.junit.runner.Description;
|
||||
import org.junit.runner.RunWith;
|
||||
@ -268,8 +267,6 @@ public abstract class AbstractDaemonTest {
|
||||
}
|
||||
};
|
||||
|
||||
@Rule public TemporaryFolder tempSiteDir = new TemporaryFolder();
|
||||
|
||||
@Before
|
||||
public void clearSender() {
|
||||
sender.clear();
|
||||
@ -337,7 +334,6 @@ public abstract class AbstractDaemonTest {
|
||||
GerritServer.Description methodDesc =
|
||||
GerritServer.Description.forTestMethod(description, configName);
|
||||
|
||||
baseConfig.setString("gerrit", null, "tempSiteDir", tempSiteDir.getRoot().getPath());
|
||||
baseConfig.setInt("receive", null, "changeUpdateThreads", 4);
|
||||
if (classDesc.equals(methodDesc) && !classDesc.sandboxed() && !methodDesc.sandboxed()) {
|
||||
if (commonServer == null) {
|
||||
|
@ -36,6 +36,7 @@ import com.google.gerrit.testutil.FakeEmailSender;
|
||||
import com.google.gerrit.testutil.NoteDbChecker;
|
||||
import com.google.gerrit.testutil.NoteDbMode;
|
||||
import com.google.gerrit.testutil.SshMode;
|
||||
import com.google.gerrit.testutil.TempFileUtil;
|
||||
import com.google.inject.Injector;
|
||||
import com.google.inject.Key;
|
||||
import com.google.inject.Module;
|
||||
@ -45,7 +46,6 @@ import java.net.InetAddress;
|
||||
import java.net.InetSocketAddress;
|
||||
import java.net.URI;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.BrokenBarrierException;
|
||||
@ -217,20 +217,29 @@ public class GerritServer implements AutoCloseable {
|
||||
/**
|
||||
* Initializes new Gerrit site and returns started server.
|
||||
*
|
||||
* <p>A new temporary directory for the site will be created with {@link TempFileUtil}, even in
|
||||
* the server is otherwise configured in-memory. Closing the server stops the daemon but does not
|
||||
* delete the temporary directory. Callers may either get the directory with {@link
|
||||
* #getSitePath()} and delete it manually, or call {@link TempFileUtil#cleanup()}.
|
||||
*
|
||||
* @param desc server description.
|
||||
* @param baseConfig default config values; merged with config from {@code desc}. Must contain a
|
||||
* value for {@code gerrit.tempSiteDir} pointing to a temporary directory. This directory is
|
||||
* only actually used for on-disk sites ({@link Description#memory()} returns false), but it
|
||||
* must nonetheless exist for in-memory sites.
|
||||
* @param baseConfig default config values; merged with config from {@code desc}.
|
||||
* @return started server.
|
||||
* @throws Exception
|
||||
*/
|
||||
public static GerritServer initAndStart(Description desc, Config baseConfig) throws Exception {
|
||||
Path site = Paths.get(baseConfig.getString("gerrit", null, "tempSiteDir"));
|
||||
if (!desc.memory()) {
|
||||
init(desc, baseConfig, site);
|
||||
Path site = TempFileUtil.createTempDirectory().toPath();
|
||||
baseConfig = new Config(baseConfig);
|
||||
baseConfig.setString("gerrit", null, "tempSiteDir", site.toString());
|
||||
try {
|
||||
if (!desc.memory()) {
|
||||
init(desc, baseConfig, site);
|
||||
}
|
||||
return start(desc, baseConfig, site);
|
||||
} catch (Exception e) {
|
||||
TempFileUtil.recursivelyDelete(site.toFile());
|
||||
throw e;
|
||||
}
|
||||
return start(desc, baseConfig, site);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -267,13 +276,13 @@ public class GerritServer implements AutoCloseable {
|
||||
daemon.setEnableSshd(SshMode.useSsh());
|
||||
|
||||
if (desc.memory()) {
|
||||
return startInMemory(desc, baseConfig, daemon);
|
||||
return startInMemory(desc, site, baseConfig, daemon);
|
||||
}
|
||||
return startOnDisk(desc, site, daemon, serverStarted);
|
||||
}
|
||||
|
||||
private static GerritServer startInMemory(Description desc, Config baseConfig, Daemon daemon)
|
||||
throws Exception {
|
||||
private static GerritServer startInMemory(
|
||||
Description desc, Path site, Config baseConfig, Daemon daemon) throws Exception {
|
||||
Config cfg = desc.buildConfig(baseConfig);
|
||||
mergeTestConfig(cfg);
|
||||
// Set the log4j configuration to an invalid one to prevent system logs
|
||||
@ -285,9 +294,10 @@ public class GerritServer implements AutoCloseable {
|
||||
cfg.setString("gitweb", null, "cgi", "");
|
||||
daemon.setEnableHttpd(desc.httpd());
|
||||
daemon.setLuceneModule(LuceneIndexModule.singleVersionAllLatest(0));
|
||||
daemon.setDatabaseForTesting(ImmutableList.<Module>of(new InMemoryTestingDatabaseModule(cfg)));
|
||||
daemon.setDatabaseForTesting(
|
||||
ImmutableList.<Module>of(new InMemoryTestingDatabaseModule(cfg, site)));
|
||||
daemon.start();
|
||||
return new GerritServer(desc, createTestInjector(daemon), daemon, null);
|
||||
return new GerritServer(desc, null, createTestInjector(daemon), daemon, null);
|
||||
}
|
||||
|
||||
private static GerritServer startOnDisk(
|
||||
@ -317,7 +327,7 @@ public class GerritServer implements AutoCloseable {
|
||||
}
|
||||
System.out.println("Gerrit Server Started");
|
||||
|
||||
return new GerritServer(desc, createTestInjector(daemon), daemon, daemonService);
|
||||
return new GerritServer(desc, site, createTestInjector(daemon), daemon, daemonService);
|
||||
}
|
||||
|
||||
private static void mergeTestConfig(Config cfg) {
|
||||
@ -370,6 +380,7 @@ public class GerritServer implements AutoCloseable {
|
||||
}
|
||||
|
||||
private final Description desc;
|
||||
private final Path sitePath;
|
||||
|
||||
private Daemon daemon;
|
||||
private ExecutorService daemonService;
|
||||
@ -379,10 +390,15 @@ public class GerritServer implements AutoCloseable {
|
||||
private InetSocketAddress httpAddress;
|
||||
|
||||
private GerritServer(
|
||||
Description desc, Injector testInjector, Daemon daemon, ExecutorService daemonService) {
|
||||
this.desc = desc;
|
||||
this.testInjector = testInjector;
|
||||
this.daemon = daemon;
|
||||
Description desc,
|
||||
@Nullable Path sitePath,
|
||||
Injector testInjector,
|
||||
Daemon daemon,
|
||||
@Nullable ExecutorService daemonService) {
|
||||
this.desc = checkNotNull(desc);
|
||||
this.sitePath = sitePath;
|
||||
this.testInjector = checkNotNull(testInjector);
|
||||
this.daemon = checkNotNull(daemon);
|
||||
this.daemonService = daemonService;
|
||||
|
||||
Config cfg = testInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
|
||||
@ -428,6 +444,10 @@ public class GerritServer implements AutoCloseable {
|
||||
}
|
||||
}
|
||||
|
||||
public Path getSitePath() {
|
||||
return sitePath;
|
||||
}
|
||||
|
||||
private void checkNoteDbState() throws Exception {
|
||||
NoteDbMode mode = NoteDbMode.get();
|
||||
if (mode != NoteDbMode.CHECK && mode != NoteDbMode.PRIMARY) {
|
||||
|
@ -51,16 +51,18 @@ import com.google.inject.TypeLiteral;
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
import org.apache.sshd.common.keyprovider.KeyPairProvider;
|
||||
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
|
||||
class InMemoryTestingDatabaseModule extends LifecycleModule {
|
||||
private final Config cfg;
|
||||
private final Path sitePath;
|
||||
|
||||
InMemoryTestingDatabaseModule(Config cfg) {
|
||||
InMemoryTestingDatabaseModule(Config cfg, Path sitePath) {
|
||||
this.cfg = cfg;
|
||||
this.sitePath = sitePath;
|
||||
makeSiteDirs(sitePath);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -68,9 +70,7 @@ class InMemoryTestingDatabaseModule extends LifecycleModule {
|
||||
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(cfg);
|
||||
|
||||
// TODO(dborowitz): Use jimfs.
|
||||
Path p = Paths.get(cfg.getString("gerrit", null, "tempSiteDir"));
|
||||
bind(Path.class).annotatedWith(SitePath.class).toInstance(p);
|
||||
makeSiteDirs(p);
|
||||
bind(Path.class).annotatedWith(SitePath.class).toInstance(sitePath);
|
||||
|
||||
bind(GitRepositoryManager.class).to(InMemoryRepositoryManager.class);
|
||||
bind(InMemoryRepositoryManager.class).in(SINGLETON);
|
||||
|
@ -27,12 +27,15 @@ import com.google.gerrit.extensions.common.ServerInfo;
|
||||
import com.google.gerrit.server.config.AllProjectsNameProvider;
|
||||
import com.google.gerrit.server.config.AllUsersNameProvider;
|
||||
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
|
||||
import com.google.gerrit.server.config.SitePaths;
|
||||
import com.google.inject.Inject;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import org.junit.Test;
|
||||
|
||||
@NoHttpd
|
||||
public class ServerInfoIT extends AbstractDaemonTest {
|
||||
@Inject private SitePaths sitePaths;
|
||||
|
||||
@Test
|
||||
// auth
|
||||
@ -131,7 +134,8 @@ public class ServerInfoIT extends AbstractDaemonTest {
|
||||
@UseSsh
|
||||
@GerritConfig(name = "plugins.allowRemoteAdmin", value = "true")
|
||||
public void serverConfigWithPlugin() throws Exception {
|
||||
Path plugins = tempSiteDir.newFolder("plugins").toPath();
|
||||
Path plugins = sitePaths.plugins_dir;
|
||||
Files.createDirectory(plugins);
|
||||
Path jsplugin = plugins.resolve("js-plugin-1.js");
|
||||
Files.write(jsplugin, "Gerrit.install(function(self){});\n".getBytes(UTF_8));
|
||||
adminSshSession.exec("gerrit plugin reload");
|
||||
|
@ -38,7 +38,7 @@ public class TempFileUtil {
|
||||
allDirsCreated.clear();
|
||||
}
|
||||
|
||||
private static void recursivelyDelete(File dir) throws IOException {
|
||||
public static void recursivelyDelete(File dir) throws IOException {
|
||||
if (!dir.getPath().equals(dir.getCanonicalPath())) {
|
||||
// Directory symlink reaching outside of temporary space.
|
||||
return;
|
||||
|
Loading…
Reference in New Issue
Block a user