Merge "Fix temporary file creation and cleanup during testing"
diff --git a/gerrit-acceptance-tests/pom.xml b/gerrit-acceptance-tests/pom.xml
index 2df6cd9..2345819 100644
--- a/gerrit-acceptance-tests/pom.xml
+++ b/gerrit-acceptance-tests/pom.xml
@@ -126,6 +126,9 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<version>2.5</version>
+ <configuration>
+ <argLine>-Djava.io.tmpdir=${project.build.directory}</argLine>
+ </configuration>
<executions>
<execution>
<id>integration-test</id>
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
index 65bab6a..fab8397 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -14,17 +14,6 @@
package com.google.gerrit.acceptance;
-import java.lang.reflect.Field;
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
-import java.util.Date;
-import java.util.concurrent.BrokenBarrierException;
-import java.util.concurrent.Callable;
-import java.util.concurrent.CyclicBarrier;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.pgm.Daemon;
import com.google.gerrit.pgm.Init;
@@ -32,15 +21,21 @@
import com.google.inject.Injector;
import com.google.inject.Module;
+import java.io.File;
+import java.lang.reflect.Field;
+import java.util.concurrent.BrokenBarrierException;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
class GerritServer {
/** Returns fully started Gerrit server */
static GerritServer start() throws Exception {
-
- final String sitePath = initSite();
-
+ final File site = initSite();
final CyclicBarrier serverStarted = new CyclicBarrier(2);
-
final Daemon daemon = new Daemon(new Runnable() {
public void run() {
try {
@@ -56,10 +51,10 @@
ExecutorService daemonService = Executors.newSingleThreadExecutor();
daemonService.submit(new Callable<Void>() {
public Void call() throws Exception {
- int rc = daemon.main(new String[] {"-d", sitePath, "--headless" });
+ int rc = daemon.main(new String[] {"-d", site.getPath(), "--headless" });
if (rc != 0) {
System.out.println("Failed to start Gerrit daemon. Check "
- + sitePath + "/logs/error_log");
+ + site.getPath() + "/logs/error_log");
serverStarted.reset();
}
return null;
@@ -70,18 +65,18 @@
System.out.println("Gerrit Server Started");
Injector i = createTestInjector(daemon);
- return new GerritServer(i, daemon, daemonService);
+ return new GerritServer(site, i, daemon, daemonService);
}
- private static String initSite() throws Exception {
- DateFormat df = new SimpleDateFormat("yyyyMMddHHmmss");
- String path = "target/test_site_" + df.format(new Date());
+ private static File initSite() throws Exception {
+ File tmp = TempFileUtil.createTempDirectory();
Init init = new Init();
- int rc = init.main(new String[] {"-d", path, "--batch", "--no-auto-start"});
+ int rc = init.main(new String[] {
+ "-d", tmp.getPath(), "--batch", "--no-auto-start"});
if (rc != 0) {
throw new RuntimeException("Couldn't initialize site");
}
- return path;
+ return tmp;
}
private static Injector createTestInjector(Daemon daemon) throws Exception {
@@ -103,12 +98,14 @@
return (T) f.get(obj);
}
+ private File sitePath;
private Daemon daemon;
private ExecutorService daemonService;
private Injector testInjector;
- private GerritServer(Injector testInjector,
+ private GerritServer(File sitePath, Injector testInjector,
Daemon daemon, ExecutorService daemonService) {
+ this.sitePath = sitePath;
this.testInjector = testInjector;
this.daemon = daemon;
this.daemonService = daemonService;
@@ -124,5 +121,6 @@
manager.stop();
daemonService.shutdownNow();
daemonService.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS);
+ TempFileUtil.recursivelyDelete(sitePath);
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TempFileUtil.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TempFileUtil.java
index adee361..1dad479 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TempFileUtil.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TempFileUtil.java
@@ -15,27 +15,41 @@
package com.google.gerrit.acceptance;
import java.io.File;
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
-import java.util.Date;
+import java.io.IOException;
public class TempFileUtil {
-
- private static int testCount;
- private static DateFormat df = new SimpleDateFormat("yyyyMMddHHmmss");
- private static final File temp = new File(new File("target"), "temp");
-
- private static String createUniqueTestFolderName() {
- return "test_" + (df.format(new Date()) + "_" + (testCount++));
+ public static File createTempDirectory() throws IOException {
+ File tmp = File.createTempFile("gerrit_test_", "");
+ if (!tmp.delete() || !tmp.mkdir()) {
+ throw new IOException("Cannot create " + tmp.getPath());
+ }
+ return tmp;
}
- public static File createTempDirectory() {
- final String name = createUniqueTestFolderName();
- final File directory = new File(temp, name);
- if (!directory.mkdirs()) {
- throw new RuntimeException("failed to create folder '"
- + directory.getAbsolutePath() + "'");
+ public static void recursivelyDelete(File dir) throws IOException {
+ if (!dir.getPath().equals(dir.getCanonicalPath())) {
+ // Directory symlink reaching outside of temporary space.
+ throw new IOException("Refusing to clear symlink " + dir.getPath());
}
- return directory;
+ File[] contents = dir.listFiles();
+ if (contents != null) {
+ for (File d : contents) {
+ if (d.isDirectory()) {
+ recursivelyDelete(d);
+ } else {
+ deleteNowOrOnExit(d);
+ }
+ }
+ }
+ deleteNowOrOnExit(dir);
+ }
+
+ private static void deleteNowOrOnExit(File dir) {
+ if (!dir.delete()) {
+ dir.deleteOnExit();
+ }
+ }
+
+ private TempFileUtil() {
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
index c8158c07..fbf2576 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitUtil.java
@@ -77,7 +77,7 @@
s.exec("gerrit create-project --empty-commit --name \"" + name + "\"");
}
- public static Git cloneProject(String url) throws GitAPIException {
+ public static Git cloneProject(String url) throws GitAPIException, IOException {
final File gitDir = TempFileUtil.createTempDirectory();
final CloneCommand cloneCmd = Git.cloneRepository();
cloneCmd.setURI(url);
diff --git a/gerrit-server/pom.xml b/gerrit-server/pom.xml
index e13c755..04b053c 100644
--- a/gerrit-server/pom.xml
+++ b/gerrit-server/pom.xml
@@ -234,6 +234,14 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <argLine>-Djava.io.tmpdir=${project.build.directory}</argLine>
+ </configuration>
+ </plugin>
+
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
<executions>
<execution>
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java
index 9a8fc00..0087df6 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java
@@ -21,10 +21,9 @@
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
-import java.util.UUID;
public class SitePathsTest extends TestCase {
- public void testCreate_NotExisting() throws FileNotFoundException {
+ public void testCreate_NotExisting() throws IOException {
final File root = random();
final SitePaths site = new SitePaths(root);
assertTrue(site.isNew);
@@ -32,7 +31,7 @@
assertEquals(new File(root, "etc"), site.etc_dir);
}
- public void testCreate_Empty() throws FileNotFoundException {
+ public void testCreate_Empty() throws IOException {
final File root = random();
try {
assertTrue(root.mkdir());
@@ -91,8 +90,11 @@
assertEquals(new File(pfx + "a").getCanonicalFile(), site.resolve(pfx + "a"));
}
- private File random() {
- final File t = new File("target");
- return new File(t, "random-name-" + UUID.randomUUID().toString());
+ private static File random() throws IOException {
+ File tmp = File.createTempFile("gerrit_test_", "_site");
+ if (!tmp.delete()) {
+ throw new IOException("Cannot create " + tmp.getPath());
+ }
+ return tmp;
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/HookTestCase.java b/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/HookTestCase.java
index b517de7..47ff6b0 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/HookTestCase.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/tools/hooks/HookTestCase.java
@@ -52,6 +52,8 @@
import static org.junit.Assert.fail;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import com.google.common.io.ByteStreams;
import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
@@ -64,10 +66,13 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
+import java.util.List;
+import java.util.Map;
public abstract class HookTestCase extends LocalDiskRepositoryTestCase {
protected Repository repository;
- private File hooksh;
+ private final Map<String, File> hooks = Maps.newTreeMap();
+ private final List<File> cleanup = Lists.newArrayList();
@Override
@Before
@@ -79,15 +84,21 @@
@Override
@After
public void tearDown() throws Exception {
- if (hooksh != null) {
- if (!hooksh.delete()) {
- hooksh.deleteOnExit();
+ super.tearDown();
+ for (File p : cleanup) {
+ if (!p.delete()) {
+ p.deleteOnExit();
}
- hooksh = null;
}
+ cleanup.clear();
}
protected File getHook(final String name) throws IOException {
+ File hook = hooks.get(name);
+ if (hook != null) {
+ return hook;
+ }
+
final String scproot = "com/google/gerrit/server/tools/root";
final String path = scproot + "/hooks/" + name;
URL url = cl().getResource(path);
@@ -95,17 +106,22 @@
fail("Cannot locate " + path + " in CLASSPATH");
}
- File hook;
if ("file".equals(url.getProtocol())) {
hook = new File(url.getPath());
if (!hook.isFile()) {
fail("Cannot locate " + path + " in CLASSPATH");
}
+ long time = hook.lastModified();
+ hook.setExecutable(true);
+ hook.setLastModified(time);
+ hooks.put(name, hook);
+ return hook;
} else if ("jar".equals(url.getProtocol())) {
- hooksh = File.createTempFile("hook_", ".sh");
InputStream in = url.openStream();
try {
- FileOutputStream out = new FileOutputStream(hooksh);
+ hook = File.createTempFile("hook_", ".sh");
+ cleanup.add(hook);
+ FileOutputStream out = new FileOutputStream(hook);
try {
ByteStreams.copy(in, out);
} finally {
@@ -114,21 +130,13 @@
} finally {
in.close();
}
- hook = hooksh;
+ hook.setExecutable(true);
+ hooks.put(name, hook);
+ return hook;
} else {
fail("Cannot invoke " + url);
- hook = null;
+ return null;
}
-
- // The hook was copied out of our source control system into the
- // target area by Java tools. Its not executable in the source
- // are, nor did the copying Java program make it executable in the
- // destination area. So we must force it to be executable.
- //
- final long time = hook.lastModified();
- hook.setExecutable(true);
- hook.setLastModified(time);
- return hook;
}
private ClassLoader cl() {
