commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bernd <e...@zusammenkunft.net>
Subject Re: svn commit: r1568538 - /commons/proper/vfs/trunk/core/src/test/java/org/apache/commons/vfs2/impl/test/VfsClassLoaderTests.java
Date Sat, 15 Feb 2014 16:06:51 GMT
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" <sebbaz@gmail.com>:

> On 15 February 2014 15:44, Benedikt Ritter <britter@apache.org> wrote:
> > 2014-02-15 16:29 GMT+01:00 sebb <sebbaz@gmail.com>:
> >
> >> On 15 February 2014 09:52, Benedikt Ritter <britter@apache.org> wrote:
> >> > HI Bernd,
> >> >
> >> >
> >> > 2014-02-14 23:44 GMT+01:00 <ecki@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
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message