[rfc][icedtea-web] fixing failing unittests CodeBaseClassLoaderTest.java
Deepak Bhole
dbhole at redhat.com
Wed Aug 8 13:25:58 PDT 2012
* Jiri Vanek <jvanek at redhat.com> [2012-08-08 08:35]:
> On 08/07/2012 09:39 PM, Deepak Bhole wrote:
> >* Jiri Vanek <jvanek at redhat.com> [2012-08-07 12:06]:
> >>On 08/07/2012 05:53 PM, Deepak Bhole wrote:
> >>>* Jiri Vanek <jvanek at redhat.com> [2012-08-01 14:29]:
> >>>>
> >>>>Those three tests have started to fail with NPE. I'm not sure if test is just badly written or
> >>>>IcedTea web is broken (but as there are no regressions in reproducers then I'm turning to badly
> >>>>written test)
> >>>>
> >>>>Please note once more, I'm not sure if fix this test this way is correct, or we have really got
> >>>>regression in CodeBaseClassLoader
> >>>>
> >>>>But IMHO we should not have jnlp file null for anything except pure applet.
> >>>>If this is correct, then this null should be probably thrown from
> >>>> new SecurityDesc(HERE, SecurityDesc.SANDBOX_PERMISSIONS, null) this contcructor (after check for
> >>>>javaws/applet) ??
> >>>>
> >>>
> >>>Hi Jiri,
> >>>
> >>>I am not sure I fully understand. The test tries to find a class in the
> >>>codebase classloader and then asserts that the exception is an instance
> >>>of NPE? Shouldn't it be an instance of CNFE when not found?
> >>>
> >>>findClass should never throw NPE IMO.
> >>
> >>The crucial difference in test is this:
> >>- return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
> >>---->> -|-
> >>+ return new SecurityDesc(this, SecurityDesc.SANDBOX_PERMISSIONS, null);
> >>
> >>The mock JNLPfile used in this class to test classlaoder is creating
> >>the security description with null jnlp file. Imho it is not
> >>possible in "real life".
> >>
> >>So I have replaced null by (imho correct) itself.
> >>
> >
> >I am referring to this part:
>
> Ah, sorry.
> >
> >>>>+
> >>>>+ Exception ex2=null;
> >>>>+ try{
> >>>>+ classLoader.findClass("foo");
> >>>>+ }catch (Exception exx){
> >>>>+ ex2=exx;
> >>>>+ }
> >>>>+ Assert.assertNotNull(ex2);
> >>>>+ Assert.assertTrue(ex2 instanceof NullPointerException);
> >>>>+
> >>>>+ Exception ex=null;
> >>>>+ try{
> >>>>+ classLoader.findResource("net/sourceforge/jnlp/about/Main.class");
> >>>>+ }catch (Exception exx){
> >>>>+ ex=exx;
> >>>>+ }
> >>>>+ Assert.assertNotNull(ex);
> >>>>+ Assert.assertTrue(ex instanceof NullPointerException);
> >>>>+
> >
> >Why is an NPE expected and asserted when findClass should never be
> >throwing an NPE?
>
> Because security manager is asked, and he is taking SecurityDesc
> from JNLPfile. But it is null in this test.
>
> If you will ask me to prevent this (and so add if
> securityDesc.getJnlpFile()==null){...) Then I must admit I'm not
> sure what behaviour should be allowed - as jnlp file is quite
> crucial for this.
>
> Does it sounds better now?
If findClass() runs into any exception, it should also convert it to a
CNFE and throw that IMO. Callers should not have to worry about
implementation specific issues when dealing with a classloader.
Cheers,
Deepak
> >
> >Cheers,
> >Deepak
> >
>
More information about the distro-pkg-dev
mailing list