{"thread":{"id":"16007","subject":"[JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete","startedAt":"2008-10-22T08:34:20Z","lastAt":"2008-10-31T00:16:27Z","messageCount":4,"participants":["Jonas Fonseca","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"93664","messageId":"20081022083420.GC17940@diku.dk","threadId":"16007","inReplyTo":null,"subject":"[JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2008-10-22T08:34:20Z","receivedAt":"2008-10-22T08:34:20Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are\nprinted. In most cases the path is in fact deleted and it seems to just\nbe a timing bug or something Windows or NTFS specific. Also, many of the\nwarnings are for the same paths and therefore a bit redundant.\nFurthermore, when a lot of warnings are printed it gives the feeling of\nslowing down the tests. By rate limiting the warnings the problem is\nstill made visible for developers to notice.\n\nSigned-off-by: Jonas Fonseca <fonseca@diku.dk>\n---\n .../org/spearce/jgit/lib/RepositoryTestCase.java   |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\n This is mostly an annoyance fix, but might be acceptable.\n\ndiff --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\nindex 9d7d133..44e24d7 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n@@ -51,6 +51,8 @@\n \n public abstract class RepositoryTestCase extends TestCase {\n \n+\tprivate static long deleteFailures;\n+\n \tprotected final File trashParent = new File(\"trash\");\n \n \tprotected File trash;\n@@ -79,7 +81,7 @@ protected static void recursiveDelete(final File dir) {\n \t\t\t}\n \t\t}\n \t\tdir.delete();\n-\t\tif (dir.exists()) {\n+\t\tif (dir.exists() && deleteFailures++ < 42) {\n \t\t\tSystem.out.println(\"Warning: Failed to delete \" + dir);\n \t\t}\n \t}\n-- \n1.6.0.2.1166.g8d97a.dirty\n\n-- \nJonas Fonseca\n"},{"id":"93809","messageId":"200810240110.21755.robin.rosenberg@dewire.com","threadId":"16007","inReplyTo":"20081022083420.GC17940@diku.dk","subject":"Re: [JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-23T23:10:21Z","receivedAt":"2008-10-23T23:10:21Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:\n> On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are\n> printed. In most cases the path is in fact deleted and it seems to just\n> be a timing bug or something Windows or NTFS specific. Also, many of the\n> warnings are for the same paths and therefore a bit redundant.\n> Furthermore, when a lot of warnings are printed it gives the feeling of\n> slowing down the tests. By rate limiting the warnings the problem is\n> still made visible for developers to notice.\n\nThe problem is actually flaws in the unit tests and in the supporting RepositoryTestCase.\nI think I'll fix that instead.\n\n-- robin\n"},{"id":"94272","messageId":"20081030104620.GB17131@diku.dk","threadId":"16007","inReplyTo":"200810240110.21755.robin.rosenberg@dewire.com","subject":"Re: [JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2008-10-30T10:46:20Z","receivedAt":"2008-10-30T10:46:20Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote Fri, Oct 24, 2008:\n> onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:\n> > On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are\n> > printed. In most cases the path is in fact deleted and it seems to\n> > just be a timing bug or something Windows or NTFS specific.\n>\n> The problem is actually flaws in the unit tests and in the supporting\n> RepositoryTestCase.  I think I'll fix that instead.\n\nThanks. Any progress on this? Can I help in any way?\n\n-- \nJonas Fonseca\n"},{"id":"94355","messageId":"200810310116.28046.robin.rosenberg@dewire.com","threadId":"16007","inReplyTo":"20081030104620.GB17131@diku.dk","subject":"Re: [JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-10-31T00:16:27Z","receivedAt":"2008-10-31T00:16:27Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"torsdagen den 30 oktober 2008 11.46.20 skrev Jonas Fonseca:\n> Robin Rosenberg <robin.rosenberg@dewire.com> wrote Fri, Oct 24, 2008:\n> > onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:\n> > > On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are\n> > > printed. In most cases the path is in fact deleted and it seems to\n> > > just be a timing bug or something Windows or NTFS specific.\n> >\n> > The problem is actually flaws in the unit tests and in the supporting\n> > RepositoryTestCase.  I think I'll fix that instead.\n> \n> Thanks. Any progress on this? Can I help in any way?\n\nOk here is a W-I-P. One test case is not cleaning up properly. You may note that I\nstill give some room for gc not to unmap immediately, but at the end of the program\nwe hope that will be the case and I think the missing case is just a bug and not GC\nbeing delayed.  The counter testcount is only here for debugging this problem.\n\n-- robin\n\n>From b3907a11bec0158a99bc77a7655379725203da9c Mon Sep 17 00:00:00 2001\nFrom: Robin Rosenberg <robin.rosnberg@dewire.com>\nDate: Thu, 23 Oct 2008 01:27:50 +0200\nSubject: [EGIT PATCH] Make the cleanup less verbose when it fails to delete temporary stuff.\n\n---\n .../org/spearce/jgit/lib/RepositoryTestCase.java   |   57 +++++++++++++++-----\n 1 files changed, 44 insertions(+), 13 deletions(-)\n\ndiff --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\nindex 9d7d133..8015a18 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n@@ -47,6 +47,7 @@\n import java.io.Reader;\n \n import junit.framework.TestCase;\n+\n import org.spearce.jgit.util.JGitTestUtil;\n \n public abstract class RepositoryTestCase extends TestCase {\n@@ -66,22 +67,34 @@\n \t\tjcommitter = new PersonIdent(\"J. Committer\", \"jcommitter@example.com\");\n \t}\n \n-\tprotected static void recursiveDelete(final File dir) {\n+\tprotected static boolean recursiveDelete(final File dir) {\n+\t\treturn recursiveDelete(dir, false);\n+\t}\n+\n+\tprotected static boolean recursiveDelete(final File dir, boolean silent) {\n+\t\tif (!dir.exists())\n+\t\t\treturn silent;\n \t\tfinal File[] ls = dir.listFiles();\n \t\tif (ls != null) {\n \t\t\tfor (int k = 0; k < ls.length; k++) {\n \t\t\t\tfinal File e = ls[k];\n \t\t\t\tif (e.isDirectory()) {\n-\t\t\t\t\trecursiveDelete(e);\n+\t\t\t\t\tsilent = recursiveDelete(e, silent);\n \t\t\t\t} else {\n-\t\t\t\t\te.delete();\n+\t\t\t\t\tif (!e.delete()) {\n+\t\t\t\t\t\tif (!silent)\n+\t\t\t\t\t\t\tSystem.out.println(\"Warning: Failed to delete \" + e);\n+\t\t\t\t\t\tsilent = true;\n+\t\t\t\t\t}\n \t\t\t\t}\n \t\t\t}\n \t\t}\n-\t\tdir.delete();\n-\t\tif (dir.exists()) {\n-\t\t\tSystem.out.println(\"Warning: Failed to delete \" + dir);\n+\t\tif (!dir.delete()) {\n+\t\t\tif (!silent)\n+\t\t\t\tSystem.out.println(\"Warning: Failed to delete \" + dir);\n+\t\t\tsilent = true;\n \t\t}\n+\t\treturn silent;\n \t}\n \n \tprotected static void copyFile(final File src, final File dst)\n@@ -121,19 +134,31 @@ protected static void checkFile(File f, final String checkData)\n \n \tprotected Repository db;\n \n+\tprivate static int testcount;\n+\tprivate static Thread shutdownhook;\n+\n \tpublic void setUp() throws Exception {\n \t\tsuper.setUp();\n-\t\trecursiveDelete(trashParent);\n-\t\ttrash = new File(trashParent,\"trash\"+System.currentTimeMillis());\n+\t\tSystem.gc();\n+\t\ttrash = new File(trashParent,\"xtrash\"+System.currentTimeMillis()+\".\"+(testcount++));\n+\t\trecursiveDelete(trashParent, true);\n \t\ttrash_git = new File(trash, \".git\");\n-\n+\t\tif (shutdownhook == null) {\n+\t\t\tshutdownhook = new Thread() {\n+\t\t\t\t@Override\n+\t\t\t\tpublic void run() {\n+\t\t\t\t\tSystem.gc();\n+\t\t\t\t\trecursiveDelete(trashParent, false);\n+\t\t\t\t}\n+\t\t\t};\n+\t\t\tRuntime.getRuntime().addShutdownHook(shutdownhook);\n+\t\t}\n \t\tRuntime.getRuntime().addShutdownHook(new Thread() {\n \t\t\t@Override\n \t\t\tpublic void run() {\n-\t\t\t\trecursiveDelete(trashParent);\n+\t\t\t\trecursiveDelete(trash);\n \t\t\t}\n \t\t});\n-\n \t\tdb = new Repository(trash_git);\n \t\tdb.create();\n \n@@ -170,12 +195,18 @@ protected void tearDown() throws Exception {\n \t * @throws IOException\n \t */\n \tprotected Repository createNewEmptyRepo() throws IOException {\n-\t\tFile newTestRepo = new File(trashParent, \"new\"+System.currentTimeMillis()+\"/.git\");\n+\t\tfinal File newTestRepo = new File(trashParent, \"new\"+System.currentTimeMillis()+\".\"+(testcount++)+\"/.git\");\n \t\tassertFalse(newTestRepo.exists());\n-\t\tFile unusedDir = new File(trashParent, \"tmp\"+System.currentTimeMillis());\n+\t\tFile unusedDir = new File(trashParent, \"tmp\"+System.currentTimeMillis()+\".\"+(testcount++));\n \t\tassertTrue(unusedDir.mkdirs());\n \t\tfinal Repository newRepo = new Repository(newTestRepo);\n \t\tnewRepo.create();\n+\t\tRuntime.getRuntime().addShutdownHook(new Thread() {\n+\t\t\t@Override\n+\t\t\tpublic void run() {\n+\t\t\t\trecursiveDelete(newTestRepo);\n+\t\t\t}\n+\t\t});\n \t\treturn newRepo;\n \t}\n \n-- \n1.6.0.2.308.gef4a\n"}]}