You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2009/03/05 05:49:11 UTC

svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Author: bodewig
Date: Thu Mar  5 04:49:10 2009
New Revision: 750313

URL: http://svn.apache.org/viewvc?rev=750313&view=rev
Log:
make tests pass on Linux

Modified:
    commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Modified: commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java
URL: http://svn.apache.org/viewvc/commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java?rev=750313&r1=750312&r2=750313&view=diff
==============================================================================
--- commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java (original)
+++ commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java Thu Mar  5 04:49:10 2009
@@ -23,8 +23,10 @@
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.InputStream;
+import java.io.IOException;
 import java.io.OutputStream;
 import java.lang.reflect.Method;
+import java.net.URI;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Iterator;
@@ -51,8 +53,17 @@
         addURL(new File("src/test/resources").toURL());
     }
 
-    protected File getFile(String path) {
-        return new File(getClass().getClassLoader().getResource(path).getFile());
+    protected File getFile(String path) throws IOException {
+        URL url = getClass().getResource(path);
+        if (url == null) {
+            throw new java.io.FileNotFoundException(path + " doesn't exist");
+        }
+        try {
+            return new File(new URI(url.toString()));
+        } catch (java.net.URISyntaxException ex) {
+            // impossible since URL.toString() should always yield a valid URI
+            throw new IOException(ex.getMessage(), ex);
+        }
     }
 
     protected void tearDown() throws Exception {
@@ -71,11 +82,11 @@
      */
     public void addURL(URL url) throws Exception {
         URLClassLoader classLoader = (URLClassLoader) ClassLoader
-                .getSystemClassLoader();
+            .getSystemClassLoader();
         Class clazz = URLClassLoader.class;
 
         Method method = clazz.getDeclaredMethod("addURL",
-                new Class[] { URL.class });
+                                                new Class[] { URL.class });
         method.setAccessible(true);
         method.invoke(classLoader, new Object[] { url });
     }
@@ -109,7 +120,7 @@
 
             final OutputStream stream = new FileOutputStream(temp);
             out = new ArchiveStreamFactory().createArchiveOutputStream(
-                    archivename, stream);
+                                                                       archivename, stream);
 
             final File file1 = getFile("test1.xml");
             final File file2 = getFile("test2.xml");
@@ -189,16 +200,16 @@
      * @throws Exception
      */
     protected void checkArchiveContent(File archive, List expected)
-            throws Exception {
+        throws Exception {
         final InputStream is = new FileInputStream(archive);
         final BufferedInputStream buf = new BufferedInputStream(is);
         final ArchiveInputStream in = new ArchiveStreamFactory()
-                .createArchiveInputStream(buf);
+            .createArchiveInputStream(buf);
         this.checkArchiveContent(in, expected);
     }
 
     protected void checkArchiveContent(ArchiveInputStream in, List expected)
-            throws Exception {
+        throws Exception {
         File result = File.createTempFile("dir-result", "");
         result.delete();
         result.mkdir();
@@ -206,7 +217,7 @@
         ArchiveEntry entry = null;
         while ((entry = in.getNextEntry()) != null) {
             File outfile = new File(result.getCanonicalPath() + "/result/"
-                    + entry.getName());
+                                    + entry.getName());
             outfile.getParentFile().mkdirs();
             OutputStream out = new FileOutputStream(outfile);
             IOUtils.copy(in, out);



Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by sebb <se...@gmail.com>.
On 06/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
> On 2009-03-06, sebb <se...@gmail.com> wrote:
>
>  > On 06/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
>  >> On 2009-03-06, sebb <se...@gmail.com> wrote:
>
>
> >>> Adding back the .getClassLoader() stage improves matters,
>
>  >> That really isn't supposed to make any difference, strange.
>
>  > Class.getResource() makes these changes to the resource name:
>
>  > "if the resource name starts with "/", it is unchanged; otherwise, the
>  > package name is prepended to the resource name after converting "." to
>  > "/". "
>
>  > ClassLoader.getResource() does not do this.
>
>
> I knew that (but assumed the path names were startng with a slash), it
>  shouldn't make any difference WRT whitespace was what I meant.

Probably not - but the test does not get that far, as it fails on the
first file...

>  Sorry I was unclear

Likewise, I could have been more explicit...

>
>       Stefan
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-06, sebb <se...@gmail.com> wrote:

> On 06/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
>> On 2009-03-06, sebb <se...@gmail.com> wrote:

>>> Adding back the .getClassLoader() stage improves matters,

>> That really isn't supposed to make any difference, strange.

> Class.getResource() makes these changes to the resource name:

> "if the resource name starts with "/", it is unchanged; otherwise, the
> package name is prepended to the resource name after converting "." to
> "/". "

> ClassLoader.getResource() does not do this.

I knew that (but assumed the path names were startng with a slash), it
shouldn't make any difference WRT whitespace was what I meant.

Sorry I was unclear

      Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by sebb <se...@gmail.com>.
On 06/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
> On 2009-03-06, sebb <se...@gmail.com> wrote:
>
>  > On 05/03/2009, bodewig@apache.org <bo...@apache.org> wrote:
>
>
> >>  URL: http://svn.apache.org/viewvc?rev=750313&view=rev
>  >>  Log:
>  >>  make tests pass on Linux
>
>  > However, they now fail on Windows (and compilation fails on Java
>  > 1.4/1.5, but I fixed that).
>
>
> Sorry about the Java6ism.
>
>
>  > Adding back the .getClassLoader() stage improves matters,
>
>
> That really isn't supposed to make any difference, strange.
>

Class.getResource() makes these changes to the resource name:

"if the resource name starts with "/", it is unchanged; otherwise, the
package name is prepended to the resource name after converting "." to
"/". "

ClassLoader.getResource() does not do this.

I've no idea why, but that's what the Javadoc for 1.4 says.

>
>  > but then the ChangeSetTestCase tests fail with
>
>  >      java.io.IOException: Illegal character in path
>
>  > which is caused by the space character.
>
>
> The File -> URI and URI -> File conversions have always been
>  problematic.  I had hoped new File(URI) would work - didn't test on
>  Windows, obviously).
>
>  The IOException probably is wrapping an URISyntaxException, which
>  means URL.toString() has returned an illegal URI on your JDK, great.
>

Looks like it.

This was on Win/XP using

java version "1.4.2_17"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_17-b06)
Java HotSpot(TM) Client VM (build 1.4.2_17-b06, mixed mode)

Works fine using:

Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_17-b04)

So it does appear to be a Java bug.

>
>  > I'm not sure that getResource() is the correct method to use here.
>
>
> Well, it works great for files that don't contain spaces in their name
>  8-)

Only if you use the ClassLoader version.

I checked the Continuum build and it reports file not found for test1.xml.

>  A hack could be to check whether the file name contains spaces and
>  locate the parent directory instead of the file itself - or use
>  getResourceAsStream in all tests that want to use files with spaces in
>  their names - neither approach looks nice, but I'm not sure how to
>  locate the files from the resources without hardcoding paths.
>
>  Stefan
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by sebb <se...@gmail.com>.
On 06/03/2009, Christian Grobmeier <gr...@gmail.com> wrote:
> > Could anybody using Eclipse please check whether the appended trivial
>  > patch causes problems there?
>
>
> Your patch works for me with Eclipse Ganymede and Mac OSX 10.5.

Also on WinXP.

>  The addUrl Method looks like not used anymore, it can be removed imho.

Agreed.


>  It has appeared cause of the getClass.getResource approach. The new
>  stuff is lots better, so lets kick it out.
>
>  Best,
>
> Christian
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-06, Christian Grobmeier <gr...@gmail.com> wrote:

>> Could anybody using Eclipse please check whether the appended trivial
>> patch causes problems there?

> Your patch works for me with Eclipse Ganymede and Mac OSX 10.5.

It should as long as your current working directory is set correctly,
but then again this is true for the old addURL version as well.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by Christian Grobmeier <gr...@gmail.com>.
> Could anybody using Eclipse please check whether the appended trivial
> patch causes problems there?

Your patch works for me with Eclipse Ganymede and Mac OSX 10.5.

The addUrl Method looks like not used anymore, it can be removed imho.
It has appeared cause of the getClass.getResource approach. The new
stuff is lots better, so lets kick it out.

Best,
Christian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-06, Stefan Bodewig <bo...@apache.org> wrote:

> On 2009-03-06, sebb <se...@gmail.com> wrote:

>> I'm not sure that getResource() is the correct method to use here.

> Well, it works great for files that don't contain spaces in their name
> 8-)

Looking into AbstractTestCase it explicitly adds src/test/resources to
the classloader to make the getFile method work later.  If getFile
simply returned new File("src/test/resources", path) it works fine for
running mvn from the command line - but a comment in the addURL method
indicates the current approach was necessary in Eclipse.

Could anybody using Eclipse please check whether the appended trivial
patch causes problems there?

Stefan

Index: src/test/java/org/apache/commons/compress/AbstractTestCase.java
===================================================================
--- src/test/java/org/apache/commons/compress/AbstractTestCase.java	(Revision 750769)
+++ src/test/java/org/apache/commons/compress/AbstractTestCase.java	(Arbeitskopie)
@@ -50,10 +50,12 @@
         dir.delete();
         dir.mkdir();
 
-        addURL(new File("src/test/resources").toURL());
+        //addURL(new File("src/test/resources").toURL());
     }
 
     protected File getFile(String path) throws IOException {
+        return new File("src/test/resources", path);
+        /*
         URL url = getClass().getResource(path);
         if (url == null) {
             throw new java.io.FileNotFoundException(path + " doesn't exist");
@@ -64,6 +66,7 @@
             // impossible since URL.toString() should always yield a valid URI
             throw new IOException(ex.getMessage());
         }
+        */
     }
 
     protected void tearDown() throws Exception {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-06, sebb <se...@gmail.com> wrote:

> On 05/03/2009, bodewig@apache.org <bo...@apache.org> wrote:

>>  URL: http://svn.apache.org/viewvc?rev=750313&view=rev
>>  Log:
>>  make tests pass on Linux

> However, they now fail on Windows (and compilation fails on Java
> 1.4/1.5, but I fixed that).

Sorry about the Java6ism.

> Adding back the .getClassLoader() stage improves matters,

That really isn't supposed to make any difference, strange.

> but then the ChangeSetTestCase tests fail with

>      java.io.IOException: Illegal character in path

> which is caused by the space character.

The File -> URI and URI -> File conversions have always been
problematic.  I had hoped new File(URI) would work - didn't test on
Windows, obviously).

The IOException probably is wrapping an URISyntaxException, which
means URL.toString() has returned an illegal URI on your JDK, great.

> I'm not sure that getResource() is the correct method to use here.

Well, it works great for files that don't contain spaces in their name
8-)

A hack could be to check whether the file name contains spaces and
locate the parent directory instead of the file itself - or use
getResourceAsStream in all tests that want to use files with spaces in
their names - neither approach looks nice, but I'm not sure how to
locate the files from the resources without hardcoding paths.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r750313 - /commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java

Posted by sebb <se...@gmail.com>.
On 05/03/2009, bodewig@apache.org <bo...@apache.org> wrote:
> Author: bodewig
>  Date: Thu Mar  5 04:49:10 2009
>  New Revision: 750313
>
>  URL: http://svn.apache.org/viewvc?rev=750313&view=rev
>  Log:
>  make tests pass on Linux
>

However, they now fail on Windows (and compilation fails on Java
1.4/1.5, but I fixed that).

Adding back the .getClassLoader() stage improves matters, but then the
ChangeSetTestCase tests fail with

     java.io.IOException: Illegal character in path

which is caused by the space character.

I'm not sure that getResource() is the correct method to use here.

>  Modified:
>     commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java
>
>  Modified: commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java
>  URL: http://svn.apache.org/viewvc/commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java?rev=750313&r1=750312&r2=750313&view=diff
>  ==============================================================================
>  --- commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java (original)
>  +++ commons/sandbox/compress/trunk/src/test/java/org/apache/commons/compress/AbstractTestCase.java Thu Mar  5 04:49:10 2009
>  @@ -23,8 +23,10 @@
>   import java.io.FileInputStream;
>   import java.io.FileOutputStream;
>   import java.io.InputStream;
>  +import java.io.IOException;
>   import java.io.OutputStream;
>   import java.lang.reflect.Method;
>  +import java.net.URI;
>   import java.net.URL;
>   import java.net.URLClassLoader;
>   import java.util.Iterator;
>  @@ -51,8 +53,17 @@
>          addURL(new File("src/test/resources").toURL());
>      }
>
>  -    protected File getFile(String path) {
>  -        return new File(getClass().getClassLoader().getResource(path).getFile());
>  +    protected File getFile(String path) throws IOException {
>  +        URL url = getClass().getResource(path);
>  +        if (url == null) {
>  +            throw new java.io.FileNotFoundException(path + " doesn't exist");
>  +        }
>  +        try {
>  +            return new File(new URI(url.toString()));
>  +        } catch (java.net.URISyntaxException ex) {
>  +            // impossible since URL.toString() should always yield a valid URI
>  +            throw new IOException(ex.getMessage(), ex);
>  +        }
>      }
>
>      protected void tearDown() throws Exception {
>  @@ -71,11 +82,11 @@
>       */
>      public void addURL(URL url) throws Exception {
>          URLClassLoader classLoader = (URLClassLoader) ClassLoader
>  -                .getSystemClassLoader();
>  +            .getSystemClassLoader();
>          Class clazz = URLClassLoader.class;
>
>          Method method = clazz.getDeclaredMethod("addURL",
>  -                new Class[] { URL.class });
>  +                                                new Class[] { URL.class });
>          method.setAccessible(true);
>          method.invoke(classLoader, new Object[] { url });
>      }
>  @@ -109,7 +120,7 @@
>
>              final OutputStream stream = new FileOutputStream(temp);
>              out = new ArchiveStreamFactory().createArchiveOutputStream(
>  -                    archivename, stream);
>  +                                                                       archivename, stream);
>
>              final File file1 = getFile("test1.xml");
>              final File file2 = getFile("test2.xml");
>  @@ -189,16 +200,16 @@
>       * @throws Exception
>       */
>      protected void checkArchiveContent(File archive, List expected)
>  -            throws Exception {
>  +        throws Exception {
>          final InputStream is = new FileInputStream(archive);
>          final BufferedInputStream buf = new BufferedInputStream(is);
>          final ArchiveInputStream in = new ArchiveStreamFactory()
>  -                .createArchiveInputStream(buf);
>  +            .createArchiveInputStream(buf);
>          this.checkArchiveContent(in, expected);
>      }
>
>      protected void checkArchiveContent(ArchiveInputStream in, List expected)
>  -            throws Exception {
>  +        throws Exception {
>          File result = File.createTempFile("dir-result", "");
>          result.delete();
>          result.mkdir();
>  @@ -206,7 +217,7 @@
>          ArchiveEntry entry = null;
>          while ((entry = in.getNextEntry()) != null) {
>              File outfile = new File(result.getCanonicalPath() + "/result/"
>  -                    + entry.getName());
>  +                                    + entry.getName());
>              outfile.getParentFile().mkdirs();
>              OutputStream out = new FileOutputStream(outfile);
>              IOUtils.copy(in, out);
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org