* [JGIT PATCH 0/4] RepositoryTestCase cleanups @ 2008-11-27 21:15 Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 1/4] Make the cleanup less verbose when it fails to delete temporary stuff Robin Rosenberg 0 siblings, 1 reply; 5+ messages in thread From: Robin Rosenberg @ 2008-11-27 21:15 UTC (permalink / raw) To: spearce; +Cc: git, fonseca, Robin Rosenberg Ok, so here is an attempt to improve the ability of the JGit's unit tests to delete temporary repositories. This has probably been seen by many, but Jonas Fonseca raised the issue. The background is that on Windows you cannot delete files that are open and mmapped files are open until they get unmapped, which in Java is beyond explicit programmer control. You can only free the resources and pray that the GC does the work. Fortunately it usually does. It turned out our testcases weren't even trying to clean up properly. -- robin Robin Rosenberg (4): Make the cleanup less verbose when it fails to delete temporary stuff. Add shutdown hooks to try to clean up after unit tests anyway Cleanup malformed test cases Automatically clean up any repositories created by the test cases .../tst/org/spearce/jgit/lib/PackWriterTest.java | 3 + .../org/spearce/jgit/lib/RepositoryTestCase.java | 82 +++++++++++++++++--- 2 files changed, 73 insertions(+), 12 deletions(-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [JGIT PATCH 1/4] Make the cleanup less verbose when it fails to delete temporary stuff. 2008-11-27 21:15 [JGIT PATCH 0/4] RepositoryTestCase cleanups Robin Rosenberg @ 2008-11-27 21:15 ` Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 2/4] Add shutdown hooks to try to clean up after unit tests anyway Robin Rosenberg 0 siblings, 1 reply; 5+ messages in thread From: Robin Rosenberg @ 2008-11-27 21:15 UTC (permalink / raw) To: spearce; +Cc: git, fonseca, Robin Rosenberg Pass the test case name for easier tracking of the test case that causes problems. Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com> --- .../org/spearce/jgit/lib/RepositoryTestCase.java | 59 ++++++++++++++----- 1 files changed, 43 insertions(+), 16 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java index 9d7d133..9c272f6 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java @@ -66,22 +66,43 @@ jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com"); } - protected static void recursiveDelete(final File dir) { + protected static boolean recursiveDelete(final File dir) { + return recursiveDelete(dir, false, null); + } + + protected static boolean recursiveDelete(final File dir, boolean silent, + final String name) { + if (!dir.exists()) + return silent; final File[] ls = dir.listFiles(); if (ls != null) { for (int k = 0; k < ls.length; k++) { final File e = ls[k]; if (e.isDirectory()) { - recursiveDelete(e); + silent = recursiveDelete(e, silent, name); } else { - e.delete(); + if (!e.delete()) { + if (!silent) { + String msg = "Warning: Failed to delete " + e; + if (name != null) + msg += " in " + name; + System.out.println(msg); + } + silent = true; + } } } } - dir.delete(); - if (dir.exists()) { - System.out.println("Warning: Failed to delete " + dir); + if (!dir.delete()) { + if (!silent) { + String msg = "Warning: Failed to delete " + dir; + if (name != null) + msg += " in " + name; + System.out.println(msg); + } + silent = true; } + return silent; } protected static void copyFile(final File src, final File dst) @@ -121,19 +142,25 @@ protected static void checkFile(File f, final String checkData) protected Repository db; + private static int testcount; + private static Thread shutdownhook; + public void setUp() throws Exception { super.setUp(); - recursiveDelete(trashParent); - trash = new File(trashParent,"trash"+System.currentTimeMillis()); + System.gc(); + trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++)); + final String name = getClass().getName() + "." + getName(); + recursiveDelete(trashParent, true, name); trash_git = new File(trash, ".git"); - - Runtime.getRuntime().addShutdownHook(new Thread() { - @Override - public void run() { - recursiveDelete(trashParent); - } - }); - + if (shutdownhook == null) { + shutdownhook = new Thread() { + @Override + public void run() { + recursiveDelete(trashParent, false, name); + } + }; + Runtime.getRuntime().addShutdownHook(shutdownhook); + } db = new Repository(trash_git); db.create(); -- 1.6.0.3.640.g6331a ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [JGIT PATCH 2/4] Add shutdown hooks to try to clean up after unit tests anyway 2008-11-27 21:15 ` [JGIT PATCH 1/4] Make the cleanup less verbose when it fails to delete temporary stuff Robin Rosenberg @ 2008-11-27 21:15 ` Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 3/4] Cleanup malformed test cases Robin Rosenberg 0 siblings, 1 reply; 5+ messages in thread From: Robin Rosenberg @ 2008-11-27 21:15 UTC (permalink / raw) To: spearce; +Cc: git, fonseca, Robin Rosenberg Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com> --- .../org/spearce/jgit/lib/RepositoryTestCase.java | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java index 9c272f6..ef4fd1b 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java @@ -66,8 +66,8 @@ jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com"); } - protected static boolean recursiveDelete(final File dir) { - return recursiveDelete(dir, false, null); + protected boolean recursiveDelete(final File dir) { + return recursiveDelete(dir, false, getClass().getName() + "." + getName()); } protected static boolean recursiveDelete(final File dir, boolean silent, @@ -161,6 +161,12 @@ public void run() { }; Runtime.getRuntime().addShutdownHook(shutdownhook); } + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + recursiveDelete(trash); + } + }); db = new Repository(trash_git); db.create(); @@ -197,12 +203,18 @@ protected void tearDown() throws Exception { * @throws IOException */ protected Repository createNewEmptyRepo() throws IOException { - File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"/.git"); + final File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"/.git"); assertFalse(newTestRepo.exists()); File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis()); assertTrue(unusedDir.mkdirs()); final Repository newRepo = new Repository(newTestRepo); newRepo.create(); + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + recursiveDelete(newTestRepo); + } + }); return newRepo; } -- 1.6.0.3.640.g6331a ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [JGIT PATCH 3/4] Cleanup malformed test cases 2008-11-27 21:15 ` [JGIT PATCH 2/4] Add shutdown hooks to try to clean up after unit tests anyway Robin Rosenberg @ 2008-11-27 21:15 ` Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 4/4] Automatically clean up any repositories created by the " Robin Rosenberg 0 siblings, 1 reply; 5+ messages in thread From: Robin Rosenberg @ 2008-11-27 21:15 UTC (permalink / raw) To: spearce; +Cc: git, fonseca, Robin Rosenberg --- .../tst/org/spearce/jgit/lib/PackWriterTest.java | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java index e5bce4d..3c02955 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java @@ -309,6 +309,7 @@ public void testWritePack4ThinPack() throws IOException { public void testWritePack2SizeDeltasVsNoDeltas() throws Exception { testWritePack2(); final int sizePack2NoDeltas = cos.getCount(); + tearDown(); setUp(); testWritePack2DeltasReuseRefs(); final int sizePack2DeltasRefs = cos.getCount(); @@ -327,6 +328,7 @@ public void testWritePack2SizeDeltasVsNoDeltas() throws Exception { public void testWritePack2SizeOffsetsVsRefs() throws Exception { testWritePack2DeltasReuseRefs(); final int sizePack2DeltasRefs = cos.getCount(); + tearDown(); setUp(); testWritePack2DeltasReuseOffsets(); final int sizePack2DeltasOffsets = cos.getCount(); @@ -344,6 +346,7 @@ public void testWritePack2SizeOffsetsVsRefs() throws Exception { public void testWritePack4SizeThinVsNoThin() throws Exception { testWritePack4(); final int sizePack4 = cos.getCount(); + tearDown(); setUp(); testWritePack4ThinPack(); final int sizePack4Thin = cos.getCount(); -- 1.6.0.3.640.g6331a ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [JGIT PATCH 4/4] Automatically clean up any repositories created by the test cases 2008-11-27 21:15 ` [JGIT PATCH 3/4] Cleanup malformed test cases Robin Rosenberg @ 2008-11-27 21:15 ` Robin Rosenberg 0 siblings, 0 replies; 5+ messages in thread From: Robin Rosenberg @ 2008-11-27 21:15 UTC (permalink / raw) To: spearce; +Cc: git, fonseca, Robin Rosenberg Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com> --- .../org/spearce/jgit/lib/RepositoryTestCase.java | 21 +++++++++++++++++++- 1 files changed, 20 insertions(+), 1 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java index ef4fd1b..cab65a0 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java @@ -45,6 +45,7 @@ import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io.Reader; +import java.util.ArrayList; import junit.framework.TestCase; import org.spearce.jgit.util.JGitTestUtil; @@ -145,6 +146,8 @@ protected static void checkFile(File f, final String checkData) private static int testcount; private static Thread shutdownhook; + private ArrayList<Repository> repositoriesToClose = new ArrayList<Repository>(); + public void setUp() throws Exception { super.setUp(); System.gc(); @@ -192,6 +195,20 @@ copyFile(JGitTestUtil.getTestResourceFile(packs[k] + ".idx"), new File(packDir, protected void tearDown() throws Exception { db.close(); + for (Repository r : repositoriesToClose) { + r.close(); + } + // Since memory mapping is controlled by the GC we need to + // tell it this is a good time to clean up and unlock + // mmemory mapped files. + System.gc(); + + recursiveDelete(trash, false, getName()); + for (Repository r : repositoriesToClose) { + recursiveDelete(r.getWorkDir(), false, getName()); + } + repositoriesToClose.clear(); + super.tearDown(); } @@ -209,12 +226,14 @@ protected Repository createNewEmptyRepo() throws IOException { assertTrue(unusedDir.mkdirs()); final Repository newRepo = new Repository(newTestRepo); newRepo.create(); + final String name = getClass().getName() + "." + getName(); Runtime.getRuntime().addShutdownHook(new Thread() { @Override public void run() { - recursiveDelete(newTestRepo); + recursiveDelete(newTestRepo, false, name); } }); + repositoriesToClose.add(newRepo); return newRepo; } -- 1.6.0.3.640.g6331a ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-11-27 21:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-27 21:15 [JGIT PATCH 0/4] RepositoryTestCase cleanups Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 1/4] Make the cleanup less verbose when it fails to delete temporary stuff Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 2/4] Add shutdown hooks to try to clean up after unit tests anyway Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 3/4] Cleanup malformed test cases Robin Rosenberg 2008-11-27 21:15 ` [JGIT PATCH 4/4] Automatically clean up any repositories created by the " Robin Rosenberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).