I agree with Benedikt. Gary
On Sat, Feb 15, 2014 at 11:25 AM, Benedikt Ritter <brit...@apache.org>wrote: > 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 > -- E-Mail: garydgreg...@gmail.com | ggreg...@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
