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" <[email protected]>: > On 15 February 2014 15:44, Benedikt Ritter <[email protected]> wrote: > > 2014-02-15 16:29 GMT+01:00 sebb <[email protected]>: > > > >> On 15 February 2014 09:52, Benedikt Ritter <[email protected]> wrote: > >> > HI Bernd, > >> > > >> > > >> > 2014-02-14 23:44 GMT+01:00 <[email protected]>: > >> > > >> >> 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: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > > > > > -- > > http://people.apache.org/~britter/ > > http://www.systemoutprintln.de/ > > http://twitter.com/BenediktRitter > > http://github.com/britter > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
