Rate limit warnings spewed by RepositoryTestCase.recursiveDelete
RFC PATCH 3/3 Rate limit warnings spewed by RepositoryTestCase.recursiveDelete
On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are printed. In most cases the path is in fact deleted and it seems to just be a timing bug or something Windows or NTFS specific. Also, many of the warnings are for the same paths and therefore a bit redundant. Furthermore, when a lot of warnings are printed it gives the feeling of slowing down the tests. By rate limiting the warnings the problem is still made visible for developers to notice.
--- .../org/spearce/jgit/lib/RepositoryTestCase.java | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
This is mostly an annoyance fix, but might be acceptable.
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +3 -1
@@ -51,6 +51,8 @@ public abstract class RepositoryTestCase extends TestCase { + private static long deleteFailures; + protected final File trashParent = new File("trash"); protected File trash;
@@ -79,7 +81,7 @@ protected static void recursiveDelete(final File dir) { } } dir.delete(); - if (dir.exists()) { + if (dir.exists() && deleteFailures++ < 42) { System.out.println("Warning: Failed to delete " + dir); } }
Signature · 4 lines
1.6.0.2.1166.g8d97a.dirty -- Jonas Fonseca
onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:
Quoted 7 lines
On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are printed. In most cases the path is in fact deleted and it seems to just be a timing bug or something Windows or NTFS specific. Also, many of the warnings are for the same paths and therefore a bit redundant. Furthermore, when a lot of warnings are printed it gives the feeling of slowing down the tests. By rate limiting the warnings the problem is still made visible for developers to notice.
The problem is actually flaws in the unit tests and in the supporting RepositoryTestCase. I think I'll fix that instead.
-- robin
Robin Rosenberg <[email protected]> wrote Fri, Oct 24, 2008:
Quoted 7 lines
onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:
Quoted 3 lines
On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are printed. In most cases the path is in fact deleted and it seems to just be a timing bug or something Windows or NTFS specific.
The problem is actually flaws in the unit tests and in the supporting RepositoryTestCase. I think I'll fix that instead.
Thanks. Any progress on this? Can I help in any way?
Signature · 1 line
Jonas Fonseca
torsdagen den 30 oktober 2008 11.46.20 skrev Jonas Fonseca:
Quoted 10 lines
Robin Rosenberg <[email protected]> wrote Fri, Oct 24, 2008:
Quoted 7 lines
onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:
Quoted 3 lines
On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are printed. In most cases the path is in fact deleted and it seems to just be a timing bug or something Windows or NTFS specific.
The problem is actually flaws in the unit tests and in the supporting RepositoryTestCase. I think I'll fix that instead.
Thanks. Any progress on this? Can I help in any way?
Ok here is a W-I-P. One test case is not cleaning up properly. You may note that I still give some room for gc not to unmap immediately, but at the end of the program we hope that will be the case and I think the missing case is just a bug and not GC being delayed. The counter testcount is only here for debugging this problem.
-- robin
Quoted 1 line
From
b3907a11bec0158a99bc77a7655379725203da9cMon Sep 17 00:00:00 2001
From: Robin Rosenberg <[email protected]>
Date: Thu, 23 Oct 2008 01:27:50 +0200
Subject: [EGIT PATCH] Make the cleanup less verbose when it fails to delete temporary stuff.
--- .../org/spearce/jgit/lib/RepositoryTestCase.java | 57 +++++++++++++++----- 1 files changed, 44 insertions(+), 13 deletions(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +44 -13
@@ -47,6 +47,7 @@ import java.io.Reader; import junit.framework.TestCase; + import org.spearce.jgit.util.JGitTestUtil; public abstract class RepositoryTestCase extends TestCase {
@@ -66,22 +67,34 @@ jcommitter = new PersonIdent("J. Committer", "[email protected]"); } - protected static void recursiveDelete(final File dir) { + protected static boolean recursiveDelete(final File dir) { + return recursiveDelete(dir, false); + } + + protected static boolean recursiveDelete(final File dir, boolean silent) { + 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); } else { - e.delete(); + if (!e.delete()) { + if (!silent) + System.out.println("Warning: Failed to delete " + e); + silent = true; + } } } } - dir.delete(); - if (dir.exists()) { - System.out.println("Warning: Failed to delete " + dir); + if (!dir.delete()) { + if (!silent) + System.out.println("Warning: Failed to delete " + dir); + silent = true; } + return silent; } protected static void copyFile(final File src, final File dst)
@@ -121,19 +134,31 @@ 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,"xtrash"+System.currentTimeMillis()+"."+(testcount++)); + recursiveDelete(trashParent, true); trash_git = new File(trash, ".git"); - + if (shutdownhook == null) { + shutdownhook = new Thread() { + @Override + public void run() { + System.gc(); + recursiveDelete(trashParent, false); + } + }; + Runtime.getRuntime().addShutdownHook(shutdownhook); + } Runtime.getRuntime().addShutdownHook(new Thread() { @Override public void run() { - recursiveDelete(trashParent); + recursiveDelete(trash); } }); - db = new Repository(trash_git); db.create();
@@ -170,12 +195,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()+"."+(testcount++)+"/.git"); assertFalse(newTestRepo.exists()); - File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis()); + File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis()+"."+(testcount++)); assertTrue(unusedDir.mkdirs()); final Repository newRepo = new Repository(newTestRepo); newRepo.create(); + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + recursiveDelete(newTestRepo); + } + }); return newRepo; }
Signature · 1 line
1.6.0.2.308.gef4a