2014-02-15 16:55 GMT+01:00 sebb <seb...@gmail.com>:
> On 15 February 2014 15:44, Benedikt Ritter <brit...@apache.org> wrote:
> > 2014-02-15 16:29 GMT+01:00 sebb <seb...@gmail.com>:
> >
> >> On 15 February 2014 09:52, Benedikt Ritter <brit...@apache.org> wrote:
> >> > HI Bernd,
> >> >
> >> >
> >> > 2014-02-14 23:44 GMT+01:00 <e...@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-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@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-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
