diff --git a/gerrit-acceptance-tests/pom.xml b/gerrit-acceptance-tests/pom.xml index 2df6cd9cb8..2345819469 100644 --- a/gerrit-acceptance-tests/pom.xml +++ b/gerrit-acceptance-tests/pom.xml @@ -126,6 +126,9 @@ limitations under the License. org.apache.maven.plugins maven-failsafe-plugin 2.5 + + -Djava.io.tmpdir=${project.build.directory} + integration-test 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 65bab6a4ef..fab8397510 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.gerrit.server.config.FactoryModule; 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 @@ class GerritServer { ExecutorService daemonService = Executors.newSingleThreadExecutor(); daemonService.submit(new Callable() { 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 @@ class GerritServer { 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 @@ class GerritServer { 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 @@ class GerritServer { 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 adee361ec7..1dad4795c7 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 c8158c07e4..fbf25762ad 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 @@ public class GitUtil { 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 e13c75528a..04b053c56d 100644 --- a/gerrit-server/pom.xml +++ b/gerrit-server/pom.xml @@ -232,6 +232,14 @@ limitations under the License. + + org.apache.maven.plugins + maven-surefire-plugin + + -Djava.io.tmpdir=${project.build.directory} + + + org.apache.maven.plugins maven-source-plugin 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 9a8fc001ef..0087df6274 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 junit.framework.TestCase; 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 @@ public class SitePathsTest extends TestCase { 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 @@ public class SitePathsTest extends TestCase { 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 b517de7633..47ff6b0cad 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 @@ package com.google.gerrit.server.tools.hooks; 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.FileOutputStream; 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 hooks = Maps.newTreeMap(); + private final List cleanup = Lists.newArrayList(); @Override @Before @@ -79,15 +84,21 @@ public abstract class HookTestCase extends LocalDiskRepositoryTestCase { @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 @@ public abstract class HookTestCase extends LocalDiskRepositoryTestCase { 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 @@ public abstract class HookTestCase extends LocalDiskRepositoryTestCase { } 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() {