Reviewer needed: fix for regression test jdk/test/sun/misc/URLClassPath/FileLoaderTest.java in IcedTea6

[Jiri Vanek]

On 06/15/2011 02:12 PM, Pavel Tisnovsky wrote:
> Hi,
>
> here's patch containing fix for another regression test which "forgets"
> to delete all its temporary work files. This patch was tested against
> IcedTea6 HEAD on RHEL 5.6 x86_64.
>
> ChangeLog entry:
>
> 2011-06-15  Pavel Tisnovsky<ptisnovs at redhat.com>
>
>          * Makefile.am: added new patch
>          * patches/jtreg-FileLoaderTest.patch:
>          Make sure that the regression test
>          openjdk/jdk/test/sun/misc/URLClassPath/FileLoaderTest.java
>          deletes all its work files.
>
> Can anybody please review this patch please?
>
> Thank you in advance,
> Pavel
>
>
> hg_diff.patch
>
>
> diff -r 78a0ce4fd551 Makefile.am
> --- a/Makefile.am	Tue Jun 14 17:25:06 2011 +0200
> +++ b/Makefile.am	Tue Jun 14 18:00:07 2011 +0200
> @@ -355,7 +355,8 @@
>   	patches/openjdk/7036148-npe-null-jmenu-name.patch \
>   	patches/jtreg-ChangeDir.patch \
>   	patches/jtreg-TempBuffer.patch \
> -	patches/jtreg-EncodedMultiByteChar.patch
> +	patches/jtreg-EncodedMultiByteChar.patch \
> +	patches/jtreg-FileLoaderTest.patch
>

maybe we should introduce special directory for jtreg patches [RFC?]
But in case they will go into separate repository (?)  thy will bring all the patches with them.

>   if WITH_ALT_HSBUILD
>   ICEDTEA_PATCHES += \
> diff -r 78a0ce4fd551 patches/jtreg-FileLoaderTest.patch
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/patches/jtreg-FileLoaderTest.patch	Tue Jun 14 18:00:07 2011 +0200
> @@ -0,0 +1,36 @@
> +--- openjdk.orig/jdk/test/sun/misc/URLClassPath/FileLoaderTest.java	2011-02-28 17:07:05.000000000 +0100
> ++++ openjdk/jdk/test/sun/misc/URLClassPath/FileLoaderTest.java	2011-06-14 16:49:57.000000000 +0200
> +@@ -31,16 +31,24 @@
> +
> + public class FileLoaderTest {
> +   public static void main (String args[]) throws Exception {
> +-      File tempFile = File.createTempFile("foo", ".txt");
> +-      tempFile.deleteOnExit();
> +-      String basestr = tempFile.toURL().toString();
> +-      basestr = basestr.substring(0, basestr.lastIndexOf("/")+1);
> +-      URL url = new URL(basestr+"."+"/");
> ++      File tempFile = null;
> ++      try
> ++      {
> ++          tempFile = File.createTempFile("foo", ".txt");
> ++          String basestr = tempFile.toURL().toString();
> ++          basestr = basestr.substring(0, basestr.lastIndexOf("/")+1);
> ++          URL url = new URL(basestr+"."+"/");
> +
> +-      ClassLoader cl = new URLClassLoader (new URL[] { url });
> +-      if (cl.getResource (tempFile.getName()) == null) {
> +-          throw new RuntimeException("Returned null instead of " +
> +-                                     tempFile.toURL().toString());
> ++          ClassLoader cl = new URLClassLoader (new URL[] { url });
> ++          if (cl.getResource (tempFile.getName()) == null) {
> ++              throw new RuntimeException("Returned null instead of " +
> ++                      tempFile.toURL().toString());
> ++          }
> ++      }
> ++      finally {
> ++          if (tempFile != null) {
> ++              tempFile.delete();
> ++          }
> +       }
> +    }
> + }

It looks like added try /finally again so it seams obvious and ok.


One question around - are you sure that remaining files should be deleted - eg after testsuite you can see which files remains => you will see files and know a lot (but as I can see names of files and delete on exit then this is more rhetorical confirmation then expected issue).


J.