You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2014/02/15 10:52:06 UTC
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
HI Bernd,
2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
> Author: ecki
> Date: Fri Feb 14 22:44:04 2014
> New Revision: 1568538
>
> URL: http://svn.apache.org/r1568538
> Log:
> [VFS-500][tests] use a non-delegating parent class loader to fix continuum
> CI test failure due to found system resource.
>
> Modified:
>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>
> Modified:
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
>
> ==============================================================================
> ---
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> (original)
> +++
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> Fri Feb 14 22:44:04 2014
> @@ -17,8 +17,10 @@
> package org.apache.commons.vfs2.impl.test;
>
> import java.io.File;
> +import java.io.IOException;
> import java.net.URL;
> import java.net.URLConnection;
> +import java.util.Collections;
> import java.util.Enumeration;
>
> import org.apache.commons.AbstractVfsTestCase;
> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
> assertTrue("nested.jar is required for testing",
> nestedJar.getType() == FileType.FILE);
> assertTrue("test.jar is required for testing", testJar.getType()
> == FileType.FILE);
>
> - final VFSClassLoader loader = new VFSClassLoader(search,
> getManager());
> + // System class loader (null) might be unpredictable in regards
> + // to returning resources for META-INF/MANIFEST.MF (see VFS-500)
> + // so we use our own which is guaranteed to not return any hit
> + final ClassLoader mockClassloader = new MockClassloader();
> +
> + final VFSClassLoader loader = new VFSClassLoader(search,
> getManager(), mockClassloader);
> +
> final Enumeration<URL> urls =
> loader.getResources("META-INF/MANIFEST.MF");
> final URL url1 = urls.nextElement();
> final URL url2 = urls.nextElement();
> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
> }
> }
>
> +
> + /**
> + * Non-Delegating Class Loader.
> + */
> + public static class MockClassloader extends ClassLoader
> + {
>
If this class is not used elsewhere, it can be made private.
Benedikt
> + MockClassloader()
> + {
> + super(null);
> + }
> +
> + /**
> + * This method will not return any hit to
> + * VFSClassLoader#testGetResourcesJARs.
> + */
> + @Override
> + public Enumeration<URL> getResources(String name)
> + throws IOException
> + {
> + return Collections.enumeration(Collections.<URL> emptyList());
> + }
> +
> + @Override
> + protected Class<?> findClass(String name)
> + throws ClassNotFoundException
> + {
> + fail("Not intended to be used for class loading.");
> + return null;
> + }
> + }
> }
>
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by Gary Gregory <ga...@gmail.com>.
I agree with Benedikt.
Gary
On Sat, Feb 15, 2014 at 11:25 AM, Benedikt Ritter <br...@apache.org>wrote:
> 2014-02-15 16:55 GMT+01:00 sebb <se...@gmail.com>:
>
> > On 15 February 2014 15:44, Benedikt Ritter <br...@apache.org> wrote:
> > > 2014-02-15 16:29 GMT+01:00 sebb <se...@gmail.com>:
> > >
> > >> On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org>
> wrote:
> > >> > HI Bernd,
> > >> >
> > >> >
> > >> > 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
> > >> >
> > >> >> Author: ecki
> > >> >> Date: Fri Feb 14 22:44:04 2014
> > >> >> New Revision: 1568538
> > >> >>
> > >> >> URL: http://svn.apache.org/r1568538
> > >> >> Log:
> > >> >> [VFS-500][tests] use a non-delegating parent class loader to fix
> > >> continuum
> > >> >> CI test failure due to found system resource.
> > >> >>
> > >> >> Modified:
> > >> >>
> > >> >>
> > >>
> >
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> > >> >>
> > >> >> Modified:
> > >> >>
> > >>
> >
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> > >> >> URL:
> > >> >>
> > >>
> >
> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
> > >> >>
> > >> >>
> > >>
> >
> ==============================================================================
> > >> >> ---
> > >> >>
> > >>
> >
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> > >> >> (original)
> > >> >> +++
> > >> >>
> > >>
> >
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> > >> >> Fri Feb 14 22:44:04 2014
> > >> >> @@ -17,8 +17,10 @@
> > >> >> package org.apache.commons.vfs2.impl.test;
> > >> >>
> > >> >> import java.io.File;
> > >> >> +import java.io.IOException;
> > >> >> import java.net.URL;
> > >> >> import java.net.URLConnection;
> > >> >> +import java.util.Collections;
> > >> >> import java.util.Enumeration;
> > >> >>
> > >> >> import org.apache.commons.AbstractVfsTestCase;
> > >> >> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
> > >> >> assertTrue("nested.jar is required for testing",
> > >> >> nestedJar.getType() == FileType.FILE);
> > >> >> assertTrue("test.jar is required for testing",
> > >> testJar.getType()
> > >> >> == FileType.FILE);
> > >> >>
> > >> >> - final VFSClassLoader loader = new VFSClassLoader(search,
> > >> >> getManager());
> > >> >> + // System class loader (null) might be unpredictable in
> > regards
> > >> >> + // to returning resources for META-INF/MANIFEST.MF (see
> > >> VFS-500)
> > >> >> + // so we use our own which is guaranteed to not return any
> > hit
> > >> >> + final ClassLoader mockClassloader = new MockClassloader();
> > >> >> +
> > >> >> + final VFSClassLoader loader = new VFSClassLoader(search,
> > >> >> getManager(), mockClassloader);
> > >> >> +
> > >> >> final Enumeration<URL> urls =
> > >> >> loader.getResources("META-INF/MANIFEST.MF");
> > >> >> final URL url1 = urls.nextElement();
> > >> >> final URL url2 = urls.nextElement();
> > >> >> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
> > >> >> }
> > >> >> }
> > >> >>
> > >> >> +
> > >> >> + /**
> > >> >> + * Non-Delegating Class Loader.
> > >> >> + */
> > >> >> + public static class MockClassloader extends ClassLoader
> > >> >> + {
> > >> >>
> > >> >
> > >> > If this class is not used elsewhere, it can be made private.
> > >>
> > >> True, but this is test code, so we don't need to worry about binary
> > >> compatibility.
> > >>
> > >
> > > Agreed, but test code should meet the same quality requirements as
> > > production code.
> >
> > The requirements for test code are rather less exacting.
> > In particular, there is no need to maintain binary compatibility,
> > which is one of the main reasons for limiting class visibility.
> >
>
> I was talking about quality requirements. You're talking about requirements
> for BC.
>
> You're right, one reason for hiding classes is, that it's easier to
> maintain BC. But to me far more important is to provide users with a sound
> API, that hides away all the nasty details. Part to achieve this is to hide
> classes that are not needed outside of the library.
>
> The same applies when I'm working with test code. When I'm writing test
> code and type "new " and then hit space, I don't want to see all the
> classes that are in the test class path. If a class only is useful for one
> test, make it private. Please, hide this implementation detail away from
> me.
>
>
> >
> > There are still good reasons for ensuring that test code meets basic
> > quality standards, but IMO it does not make sense to be as stringent
> > as for the main code.
> >
> > For example, I think test code variables should have minimal
> > visibility, as that avoids accidental misuse.
> > It's much harder to accidentally misuse a test class, so minimising
> > class visibility is not as vital.
> >
>
> In the end I don't understand why you're arguing with me :-) Hiding the
> class doesn't hurt and to the contrary it will make the test code a little
> bit easier to understand (since a implementation detail of one of the tests
> is hidden away).
>
>
> >
> > >
> > >>
> > >> > Benedikt
> > >> >
> > >> >
> > >> >> + MockClassloader()
> > >> >> + {
> > >> >> + super(null);
> > >> >> + }
> > >> >> +
> > >> >> + /**
> > >> >> + * This method will not return any hit to
> > >> >> + * VFSClassLoader#testGetResourcesJARs.
> > >> >> + */
> > >> >> + @Override
> > >> >> + public Enumeration<URL> getResources(String name)
> > >> >> + throws IOException
> > >> >> + {
> > >> >> + return Collections.enumeration(Collections.<URL>
> > >> emptyList());
> > >> >> + }
> > >> >> +
> > >> >> + @Override
> > >> >> + protected Class<?> findClass(String name)
> > >> >> + throws ClassNotFoundException
> > >> >> + {
> > >> >> + fail("Not intended to be used for class loading.");
> > >> >> + return null;
> > >> >> + }
> > >> >> + }
> > >> >> }
> > >> >>
> > >> >>
> > >> >>
> > >> >
> > >> >
> > >> > --
> > >> > http://people.apache.org/~britter/
> > >> > http://www.systemoutprintln.de/
> > >> > http://twitter.com/BenediktRitter
> > >> > http://github.com/britter
> > >>
> > >> ---------------------------------------------------------------------
> > >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> For additional commands, e-mail: dev-help@commons.apache.org
> > >>
> > >>
> > >
> > >
> > > --
> > > http://people.apache.org/~britter/
> > > http://www.systemoutprintln.de/
> > > http://twitter.com/BenediktRitter
> > > http://github.com/britter
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by Benedikt Ritter <br...@apache.org>.
2014-02-15 16:55 GMT+01:00 sebb <se...@gmail.com>:
> On 15 February 2014 15:44, Benedikt Ritter <br...@apache.org> wrote:
> > 2014-02-15 16:29 GMT+01:00 sebb <se...@gmail.com>:
> >
> >> On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org> wrote:
> >> > HI Bernd,
> >> >
> >> >
> >> > 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
> >> >
> >> >> Author: ecki
> >> >> Date: Fri Feb 14 22:44:04 2014
> >> >> New Revision: 1568538
> >> >>
> >> >> URL: http://svn.apache.org/r1568538
> >> >> Log:
> >> >> [VFS-500][tests] use a non-delegating parent class loader to fix
> >> continuum
> >> >> CI test failure due to found system resource.
> >> >>
> >> >> Modified:
> >> >>
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >>
> >> >> Modified:
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
> >> >>
> >> >>
> >>
> ==============================================================================
> >> >> ---
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> (original)
> >> >> +++
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> Fri Feb 14 22:44:04 2014
> >> >> @@ -17,8 +17,10 @@
> >> >> package org.apache.commons.vfs2.impl.test;
> >> >>
> >> >> import java.io.File;
> >> >> +import java.io.IOException;
> >> >> import java.net.URL;
> >> >> import java.net.URLConnection;
> >> >> +import java.util.Collections;
> >> >> import java.util.Enumeration;
> >> >>
> >> >> import org.apache.commons.AbstractVfsTestCase;
> >> >> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
> >> >> assertTrue("nested.jar is required for testing",
> >> >> nestedJar.getType() == FileType.FILE);
> >> >> assertTrue("test.jar is required for testing",
> >> testJar.getType()
> >> >> == FileType.FILE);
> >> >>
> >> >> - final VFSClassLoader loader = new VFSClassLoader(search,
> >> >> getManager());
> >> >> + // System class loader (null) might be unpredictable in
> regards
> >> >> + // to returning resources for META-INF/MANIFEST.MF (see
> >> VFS-500)
> >> >> + // so we use our own which is guaranteed to not return any
> hit
> >> >> + final ClassLoader mockClassloader = new MockClassloader();
> >> >> +
> >> >> + final VFSClassLoader loader = new VFSClassLoader(search,
> >> >> getManager(), mockClassloader);
> >> >> +
> >> >> final Enumeration<URL> urls =
> >> >> loader.getResources("META-INF/MANIFEST.MF");
> >> >> final URL url1 = urls.nextElement();
> >> >> final URL url2 = urls.nextElement();
> >> >> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
> >> >> }
> >> >> }
> >> >>
> >> >> +
> >> >> + /**
> >> >> + * Non-Delegating Class Loader.
> >> >> + */
> >> >> + public static class MockClassloader extends ClassLoader
> >> >> + {
> >> >>
> >> >
> >> > If this class is not used elsewhere, it can be made private.
> >>
> >> True, but this is test code, so we don't need to worry about binary
> >> compatibility.
> >>
> >
> > Agreed, but test code should meet the same quality requirements as
> > production code.
>
> The requirements for test code are rather less exacting.
> In particular, there is no need to maintain binary compatibility,
> which is one of the main reasons for limiting class visibility.
>
I was talking about quality requirements. You're talking about requirements
for BC.
You're right, one reason for hiding classes is, that it's easier to
maintain BC. But to me far more important is to provide users with a sound
API, that hides away all the nasty details. Part to achieve this is to hide
classes that are not needed outside of the library.
The same applies when I'm working with test code. When I'm writing test
code and type "new " and then hit space, I don't want to see all the
classes that are in the test class path. If a class only is useful for one
test, make it private. Please, hide this implementation detail away from
me.
>
> There are still good reasons for ensuring that test code meets basic
> quality standards, but IMO it does not make sense to be as stringent
> as for the main code.
>
> For example, I think test code variables should have minimal
> visibility, as that avoids accidental misuse.
> It's much harder to accidentally misuse a test class, so minimising
> class visibility is not as vital.
>
In the end I don't understand why you're arguing with me :-) Hiding the
class doesn't hurt and to the contrary it will make the test code a little
bit easier to understand (since a implementation detail of one of the tests
is hidden away).
>
> >
> >>
> >> > Benedikt
> >> >
> >> >
> >> >> + MockClassloader()
> >> >> + {
> >> >> + super(null);
> >> >> + }
> >> >> +
> >> >> + /**
> >> >> + * This method will not return any hit to
> >> >> + * VFSClassLoader#testGetResourcesJARs.
> >> >> + */
> >> >> + @Override
> >> >> + public Enumeration<URL> getResources(String name)
> >> >> + throws IOException
> >> >> + {
> >> >> + return Collections.enumeration(Collections.<URL>
> >> emptyList());
> >> >> + }
> >> >> +
> >> >> + @Override
> >> >> + protected Class<?> findClass(String name)
> >> >> + throws ClassNotFoundException
> >> >> + {
> >> >> + fail("Not intended to be used for class loading.");
> >> >> + return null;
> >> >> + }
> >> >> + }
> >> >> }
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >> > --
> >> > http://people.apache.org/~britter/
> >> > http://www.systemoutprintln.de/
> >> > http://twitter.com/BenediktRitter
> >> > http://github.com/britter
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
> >
> >
> > --
> > http://people.apache.org/~britter/
> > http://www.systemoutprintln.de/
> > http://twitter.com/BenediktRitter
> > http://github.com/britter
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by Bernd <ec...@zusammenkunft.net>.
Hello,
Yes I agree it is a good thing to make this class private (or promote it
toplevel if needed in other TestCases).
If nobody does the commit I will do it next time I toch the class (there
need to be better testing of the classloader, it is currently only
exercised by a single provider).
Bernd
BTW: is there a easiy method to get shell access to that build server, I
would like to investigate why the resources.jar is only showing up on that
machine.
Am 15.02.2014 16:55 schrieb "sebb" <se...@gmail.com>:
> On 15 February 2014 15:44, Benedikt Ritter <br...@apache.org> wrote:
> > 2014-02-15 16:29 GMT+01:00 sebb <se...@gmail.com>:
> >
> >> On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org> wrote:
> >> > HI Bernd,
> >> >
> >> >
> >> > 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
> >> >
> >> >> Author: ecki
> >> >> Date: Fri Feb 14 22:44:04 2014
> >> >> New Revision: 1568538
> >> >>
> >> >> URL: http://svn.apache.org/r1568538
> >> >> Log:
> >> >> [VFS-500][tests] use a non-delegating parent class loader to fix
> >> continuum
> >> >> CI test failure due to found system resource.
> >> >>
> >> >> Modified:
> >> >>
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >>
> >> >> Modified:
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
> >> >>
> >> >>
> >>
> ==============================================================================
> >> >> ---
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> (original)
> >> >> +++
> >> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> >> Fri Feb 14 22:44:04 2014
> >> >> @@ -17,8 +17,10 @@
> >> >> package org.apache.commons.vfs2.impl.test;
> >> >>
> >> >> import java.io.File;
> >> >> +import java.io.IOException;
> >> >> import java.net.URL;
> >> >> import java.net.URLConnection;
> >> >> +import java.util.Collections;
> >> >> import java.util.Enumeration;
> >> >>
> >> >> import org.apache.commons.AbstractVfsTestCase;
> >> >> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
> >> >> assertTrue("nested.jar is required for testing",
> >> >> nestedJar.getType() == FileType.FILE);
> >> >> assertTrue("test.jar is required for testing",
> >> testJar.getType()
> >> >> == FileType.FILE);
> >> >>
> >> >> - final VFSClassLoader loader = new VFSClassLoader(search,
> >> >> getManager());
> >> >> + // System class loader (null) might be unpredictable in
> regards
> >> >> + // to returning resources for META-INF/MANIFEST.MF (see
> >> VFS-500)
> >> >> + // so we use our own which is guaranteed to not return any
> hit
> >> >> + final ClassLoader mockClassloader = new MockClassloader();
> >> >> +
> >> >> + final VFSClassLoader loader = new VFSClassLoader(search,
> >> >> getManager(), mockClassloader);
> >> >> +
> >> >> final Enumeration<URL> urls =
> >> >> loader.getResources("META-INF/MANIFEST.MF");
> >> >> final URL url1 = urls.nextElement();
> >> >> final URL url2 = urls.nextElement();
> >> >> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
> >> >> }
> >> >> }
> >> >>
> >> >> +
> >> >> + /**
> >> >> + * Non-Delegating Class Loader.
> >> >> + */
> >> >> + public static class MockClassloader extends ClassLoader
> >> >> + {
> >> >>
> >> >
> >> > If this class is not used elsewhere, it can be made private.
> >>
> >> True, but this is test code, so we don't need to worry about binary
> >> compatibility.
> >>
> >
> > Agreed, but test code should meet the same quality requirements as
> > production code.
>
> The requirements for test code are rather less exacting.
> In particular, there is no need to maintain binary compatibility,
> which is one of the main reasons for limiting class visibility.
>
> There are still good reasons for ensuring that test code meets basic
> quality standards, but IMO it does not make sense to be as stringent
> as for the main code.
>
> For example, I think test code variables should have minimal
> visibility, as that avoids accidental misuse.
> It's much harder to accidentally misuse a test class, so minimising
> class visibility is not as vital.
>
> >
> >>
> >> > Benedikt
> >> >
> >> >
> >> >> + MockClassloader()
> >> >> + {
> >> >> + super(null);
> >> >> + }
> >> >> +
> >> >> + /**
> >> >> + * This method will not return any hit to
> >> >> + * VFSClassLoader#testGetResourcesJARs.
> >> >> + */
> >> >> + @Override
> >> >> + public Enumeration<URL> getResources(String name)
> >> >> + throws IOException
> >> >> + {
> >> >> + return Collections.enumeration(Collections.<URL>
> >> emptyList());
> >> >> + }
> >> >> +
> >> >> + @Override
> >> >> + protected Class<?> findClass(String name)
> >> >> + throws ClassNotFoundException
> >> >> + {
> >> >> + fail("Not intended to be used for class loading.");
> >> >> + return null;
> >> >> + }
> >> >> + }
> >> >> }
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >> > --
> >> > http://people.apache.org/~britter/
> >> > http://www.systemoutprintln.de/
> >> > http://twitter.com/BenediktRitter
> >> > http://github.com/britter
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
> >
> >
> > --
> > http://people.apache.org/~britter/
> > http://www.systemoutprintln.de/
> > http://twitter.com/BenediktRitter
> > http://github.com/britter
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by sebb <se...@gmail.com>.
On 15 February 2014 15:44, Benedikt Ritter <br...@apache.org> wrote:
> 2014-02-15 16:29 GMT+01:00 sebb <se...@gmail.com>:
>
>> On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org> wrote:
>> > HI Bernd,
>> >
>> >
>> > 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
>> >
>> >> Author: ecki
>> >> Date: Fri Feb 14 22:44:04 2014
>> >> New Revision: 1568538
>> >>
>> >> URL: http://svn.apache.org/r1568538
>> >> Log:
>> >> [VFS-500][tests] use a non-delegating parent class loader to fix
>> continuum
>> >> CI test failure due to found system resource.
>> >>
>> >> Modified:
>> >>
>> >>
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> >>
>> >> Modified:
>> >>
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> >> URL:
>> >>
>> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
>> >>
>> >>
>> ==============================================================================
>> >> ---
>> >>
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> >> (original)
>> >> +++
>> >>
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> >> Fri Feb 14 22:44:04 2014
>> >> @@ -17,8 +17,10 @@
>> >> package org.apache.commons.vfs2.impl.test;
>> >>
>> >> import java.io.File;
>> >> +import java.io.IOException;
>> >> import java.net.URL;
>> >> import java.net.URLConnection;
>> >> +import java.util.Collections;
>> >> import java.util.Enumeration;
>> >>
>> >> import org.apache.commons.AbstractVfsTestCase;
>> >> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
>> >> assertTrue("nested.jar is required for testing",
>> >> nestedJar.getType() == FileType.FILE);
>> >> assertTrue("test.jar is required for testing",
>> testJar.getType()
>> >> == FileType.FILE);
>> >>
>> >> - final VFSClassLoader loader = new VFSClassLoader(search,
>> >> getManager());
>> >> + // System class loader (null) might be unpredictable in regards
>> >> + // to returning resources for META-INF/MANIFEST.MF (see
>> VFS-500)
>> >> + // so we use our own which is guaranteed to not return any hit
>> >> + final ClassLoader mockClassloader = new MockClassloader();
>> >> +
>> >> + final VFSClassLoader loader = new VFSClassLoader(search,
>> >> getManager(), mockClassloader);
>> >> +
>> >> final Enumeration<URL> urls =
>> >> loader.getResources("META-INF/MANIFEST.MF");
>> >> final URL url1 = urls.nextElement();
>> >> final URL url2 = urls.nextElement();
>> >> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
>> >> }
>> >> }
>> >>
>> >> +
>> >> + /**
>> >> + * Non-Delegating Class Loader.
>> >> + */
>> >> + public static class MockClassloader extends ClassLoader
>> >> + {
>> >>
>> >
>> > If this class is not used elsewhere, it can be made private.
>>
>> True, but this is test code, so we don't need to worry about binary
>> compatibility.
>>
>
> Agreed, but test code should meet the same quality requirements as
> production code.
The requirements for test code are rather less exacting.
In particular, there is no need to maintain binary compatibility,
which is one of the main reasons for limiting class visibility.
There are still good reasons for ensuring that test code meets basic
quality standards, but IMO it does not make sense to be as stringent
as for the main code.
For example, I think test code variables should have minimal
visibility, as that avoids accidental misuse.
It's much harder to accidentally misuse a test class, so minimising
class visibility is not as vital.
>
>>
>> > Benedikt
>> >
>> >
>> >> + MockClassloader()
>> >> + {
>> >> + super(null);
>> >> + }
>> >> +
>> >> + /**
>> >> + * This method will not return any hit to
>> >> + * VFSClassLoader#testGetResourcesJARs.
>> >> + */
>> >> + @Override
>> >> + public Enumeration<URL> getResources(String name)
>> >> + throws IOException
>> >> + {
>> >> + return Collections.enumeration(Collections.<URL>
>> emptyList());
>> >> + }
>> >> +
>> >> + @Override
>> >> + protected Class<?> findClass(String name)
>> >> + throws ClassNotFoundException
>> >> + {
>> >> + fail("Not intended to be used for class loading.");
>> >> + return null;
>> >> + }
>> >> + }
>> >> }
>> >>
>> >>
>> >>
>> >
>> >
>> > --
>> > http://people.apache.org/~britter/
>> > http://www.systemoutprintln.de/
>> > http://twitter.com/BenediktRitter
>> > http://github.com/britter
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by Benedikt Ritter <br...@apache.org>.
2014-02-15 16:29 GMT+01:00 sebb <se...@gmail.com>:
> On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org> wrote:
> > HI Bernd,
> >
> >
> > 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
> >
> >> Author: ecki
> >> Date: Fri Feb 14 22:44:04 2014
> >> New Revision: 1568538
> >>
> >> URL: http://svn.apache.org/r1568538
> >> Log:
> >> [VFS-500][tests] use a non-delegating parent class loader to fix
> continuum
> >> CI test failure due to found system resource.
> >>
> >> Modified:
> >>
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >>
> >> Modified:
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> (original)
> >> +++
> >>
> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
> >> Fri Feb 14 22:44:04 2014
> >> @@ -17,8 +17,10 @@
> >> package org.apache.commons.vfs2.impl.test;
> >>
> >> import java.io.File;
> >> +import java.io.IOException;
> >> import java.net.URL;
> >> import java.net.URLConnection;
> >> +import java.util.Collections;
> >> import java.util.Enumeration;
> >>
> >> import org.apache.commons.AbstractVfsTestCase;
> >> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
> >> assertTrue("nested.jar is required for testing",
> >> nestedJar.getType() == FileType.FILE);
> >> assertTrue("test.jar is required for testing",
> testJar.getType()
> >> == FileType.FILE);
> >>
> >> - final VFSClassLoader loader = new VFSClassLoader(search,
> >> getManager());
> >> + // System class loader (null) might be unpredictable in regards
> >> + // to returning resources for META-INF/MANIFEST.MF (see
> VFS-500)
> >> + // so we use our own which is guaranteed to not return any hit
> >> + final ClassLoader mockClassloader = new MockClassloader();
> >> +
> >> + final VFSClassLoader loader = new VFSClassLoader(search,
> >> getManager(), mockClassloader);
> >> +
> >> final Enumeration<URL> urls =
> >> loader.getResources("META-INF/MANIFEST.MF");
> >> final URL url1 = urls.nextElement();
> >> final URL url2 = urls.nextElement();
> >> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
> >> }
> >> }
> >>
> >> +
> >> + /**
> >> + * Non-Delegating Class Loader.
> >> + */
> >> + public static class MockClassloader extends ClassLoader
> >> + {
> >>
> >
> > If this class is not used elsewhere, it can be made private.
>
> True, but this is test code, so we don't need to worry about binary
> compatibility.
>
Agreed, but test code should meet the same quality requirements as
production code.
>
> > Benedikt
> >
> >
> >> + MockClassloader()
> >> + {
> >> + super(null);
> >> + }
> >> +
> >> + /**
> >> + * This method will not return any hit to
> >> + * VFSClassLoader#testGetResourcesJARs.
> >> + */
> >> + @Override
> >> + public Enumeration<URL> getResources(String name)
> >> + throws IOException
> >> + {
> >> + return Collections.enumeration(Collections.<URL>
> emptyList());
> >> + }
> >> +
> >> + @Override
> >> + protected Class<?> findClass(String name)
> >> + throws ClassNotFoundException
> >> + {
> >> + fail("Not intended to be used for class loading.");
> >> + return null;
> >> + }
> >> + }
> >> }
> >>
> >>
> >>
> >
> >
> > --
> > http://people.apache.org/~britter/
> > http://www.systemoutprintln.de/
> > http://twitter.com/BenediktRitter
> > http://github.com/britter
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Posted by sebb <se...@gmail.com>.
On 15 February 2014 09:52, Benedikt Ritter <br...@apache.org> wrote:
> HI Bernd,
>
>
> 2014-02-14 23:44 GMT+01:00 <ec...@apache.org>:
>
>> Author: ecki
>> Date: Fri Feb 14 22:44:04 2014
>> New Revision: 1568538
>>
>> URL: http://svn.apache.org/r1568538
>> Log:
>> [VFS-500][tests] use a non-delegating parent class loader to fix continuum
>> CI test failure due to found system resource.
>>
>> Modified:
>>
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>>
>> Modified:
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> URL:
>> http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java?rev=1568538&r1=1568537&r2=1568538&view=diff
>>
>> ==============================================================================
>> ---
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> (original)
>> +++
>> commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
>> Fri Feb 14 22:44:04 2014
>> @@ -17,8 +17,10 @@
>> package org.apache.commons.vfs2.impl.test;
>>
>> import java.io.File;
>> +import java.io.IOException;
>> import java.net.URL;
>> import java.net.URLConnection;
>> +import java.util.Collections;
>> import java.util.Enumeration;
>>
>> import org.apache.commons.AbstractVfsTestCase;
>> @@ -140,7 +142,13 @@ public class VfsClassLoaderTests
>> assertTrue("nested.jar is required for testing",
>> nestedJar.getType() == FileType.FILE);
>> assertTrue("test.jar is required for testing", testJar.getType()
>> == FileType.FILE);
>>
>> - final VFSClassLoader loader = new VFSClassLoader(search,
>> getManager());
>> + // System class loader (null) might be unpredictable in regards
>> + // to returning resources for META-INF/MANIFEST.MF (see VFS-500)
>> + // so we use our own which is guaranteed to not return any hit
>> + final ClassLoader mockClassloader = new MockClassloader();
>> +
>> + final VFSClassLoader loader = new VFSClassLoader(search,
>> getManager(), mockClassloader);
>> +
>> final Enumeration<URL> urls =
>> loader.getResources("META-INF/MANIFEST.MF");
>> final URL url1 = urls.nextElement();
>> final URL url2 = urls.nextElement();
>> @@ -178,4 +186,34 @@ public class VfsClassLoaderTests
>> }
>> }
>>
>> +
>> + /**
>> + * Non-Delegating Class Loader.
>> + */
>> + public static class MockClassloader extends ClassLoader
>> + {
>>
>
> If this class is not used elsewhere, it can be made private.
True, but this is test code, so we don't need to worry about binary
compatibility.
> Benedikt
>
>
>> + MockClassloader()
>> + {
>> + super(null);
>> + }
>> +
>> + /**
>> + * This method will not return any hit to
>> + * VFSClassLoader#testGetResourcesJARs.
>> + */
>> + @Override
>> + public Enumeration<URL> getResources(String name)
>> + throws IOException
>> + {
>> + return Collections.enumeration(Collections.<URL> emptyList());
>> + }
>> +
>> + @Override
>> + protected Class<?> findClass(String name)
>> + throws ClassNotFoundException
>> + {
>> + fail("Not intended to be used for class loading.");
>> + return null;
>> + }
>> + }
>> }
>>
>>
>>
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org