Bug 328789

Comment 1 Greg Wilkins 2010-11-02 01:43:37 EDT

Michael,

can you have a look at what tests are leaving temporary files around and make sure the tests remove them.

Also check if any of them are actually coming from non-test code (eg Multipart???)

Comment 2 Greg Wilkins 2010-11-22 15:49:02 EST

Thomas,

here is a minor issue that will give you some reasons to tour the tests

Comment 3 Thomas Becker 2010-11-24 08:37:46 EST

First small fix to get rid of copyjar and copyto:

Index: src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java
===================================================================
--- src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java     (revision 2553)
+++ src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java     (working copy)
@@ -25,8 +25,6 @@
 import java.net.URL;
 import java.util.jar.JarInputStream;
 
-import junit.framework.TestSuite;
-
 import org.eclipse.jetty.util.IO;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -228,6 +226,8 @@
         Resource r =Resource.newResource("/tmp/a file with,spe#ials/");
         assertTrue(r.getURL().toString().indexOf("a%20file%20with,spe%23ials")>0);
         assertTrue(r.getFile().toString().indexOf("a file with,spe#ials")>0);
+        r.delete();
+        assertFalse("File should have been deleted.",r.exists());
     }
 
     /* ------------------------------------------------------------ */
@@ -314,7 +314,12 @@
             }
         };
         assertEquals(1, dest.listFiles(currentDirectoryFilenameFilter).length);
-        assertEquals(0, dest.getParentFile().listFiles(currentDirectoryFilenameFilter).length);        
+        assertEquals(0, dest.getParentFile().listFiles(currentDirectoryFilenameFilter).length); 
+        
+        for (File file : dest.listFiles()) {
+                       file.delete();
+               }
+        
     }
     
     /**

Comment 4 Greg Wilkins 2010-11-25 00:27:13 EST

Thomas,

thanks for the patch.

Normally we ask for patches from the top level of the jetty project and for them to include an entry in VERSION.txt noting the issue they are fixing.

Note also that we have an o.e.j.util.IO.delete(File) method that does a recursive delete of a directory (probably not useful here, but maybe for other left over tmp files).

Comment 5 Thomas Becker 2010-12-01 06:22:46 EST

Created attachment 184240 [details]
git diff fixing all tests which didn't cleanup tmp files before

Hi Greg,

attached you'll find a patch fixing all tmp files which have been left over by the test harnesses. 

Cheers,
Thomas

Comment 6 Greg Wilkins 2010-12-01 09:29:57 EST

patch applied, but still getting /tmp/copyto6921733889102830253.tmp/ directories left over after tests.

Comment 7 Thomas Becker 2010-12-01 12:06:19 EST

Created attachment 184277 [details]
additional fix

Hi Greg,

I missed that one. Here you go. Let me know if there's still tmp files being left open for you. On my box it's clean now.

Regards,
Thomas

Comment 8 Greg Wilkins 2010-12-01 12:32:45 EST

patches applied r2567

Comment 9 Thomas Becker 2010-12-02 03:07:56 EST

thx greg