[rfc][icedtea-web] fixing failing unittests CodeBaseClassLoaderTest.java

[Jiri Vanek]

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.


the example of this mock is also  in patch:

  JNLPFile dummyJnlpFile = new JNLPFile() {
  +            @Override
  +            public ResourcesDesc getResources() {
  +                return new ResourcesDesc(null, new Locale[0], new String[0], new String[0]);
  +            }
  +
  +            @Override
  +            public URL getCodeBase() {
  +                return CODEBASE_URL;
  +            }
  +
  +            @Override
  +            public SecurityDesc getSecurity() {
  +                return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
  +            }
  +        };


>
> Deepak
>
>> 2012-08-01  Jiri Vanek  <jvanek at redhat.com>
>>
>> 	* tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:
>> 	(testResourceLoadSuccessCaching) (testResourceLoadFailureCaching)
>> 	(testParentClassLoaderIsAskedForClasses) - internal JNLPFile's
>> 	(getSecurity) null in SecurityDesc constructorrepalced by this.
>> 	(testNullFileSecurityDesc) new test to ensure NPE in null JNLPFile case
>>
>>
>>
>> 	
>>
>> Best regards
>>    J.
>>
>>
>>
>
>> diff -r 504ad6ea95fa tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java
>> --- a/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java	Tue Jul 24 11:44:50 2012 -0400
>> +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java	Wed Aug 01 18:21:35 2012 +0200
>> @@ -54,6 +54,7 @@
>>   import net.sourceforge.jnlp.runtime.JNLPClassLoader;
>>   import net.sourceforge.jnlp.runtime.JNLPClassLoader.CodeBaseClassLoader;
>>   import net.sourceforge.jnlp.annotations.Bug;
>> +import org.junit.Assert;
>>
>>   import org.junit.Test;
>>
>> @@ -80,7 +81,7 @@
>>
>>               @Override
>>               public SecurityDesc getSecurity() {
>> -                return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
>> +                return new SecurityDesc(this, SecurityDesc.SANDBOX_PERMISSIONS, null);
>>               }
>>           };
>>
>> @@ -125,7 +126,7 @@
>>
>>               @Override
>>               public SecurityDesc getSecurity() {
>> -                return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
>> +                return new SecurityDesc(this, SecurityDesc.SANDBOX_PERMISSIONS, null);
>>               }
>>           };
>>
>> @@ -167,7 +168,7 @@
>>
>>               @Override
>>               public SecurityDesc getSecurity() {
>> -                return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
>> +                return new SecurityDesc(this, SecurityDesc.SANDBOX_PERMISSIONS, null);
>>               }
>>           };
>>
>> @@ -188,4 +189,52 @@
>>
>>           assertTrue(parentWasInvoked[0]);
>>       }
>> +
>> +
>> +    @Test
>> +    public void testNullFileSecurityDesc() throws MalformedURLException, LaunchException {
>> +        final URL JAR_URL = new URL("http://icedtea.classpath.org/netx/about.jar");
>> +        final URL CODEBASE_URL = new URL("http://icedtea.classpath.org/netx/");
>> +
>> +        JNLPFile dummyJnlpFile = new JNLPFile() {
>> +            @Override
>> +            public ResourcesDesc getResources() {
>> +                return new ResourcesDesc(null, new Locale[0], new String[0], new String[0]);
>> +            }
>> +
>> +            @Override
>> +            public URL getCodeBase() {
>> +                return CODEBASE_URL;
>> +            }
>> +
>> +            @Override
>> +            public SecurityDesc getSecurity() {
>> +                return new SecurityDesc(null, SecurityDesc.SANDBOX_PERMISSIONS, null);
>> +            }
>> +        };
>> +
>> +        JNLPClassLoader parent = new JNLPClassLoader(dummyJnlpFile, null);
>> +        CodeBaseClassLoader classLoader = new CodeBaseClassLoader(new URL[] { JAR_URL, CODEBASE_URL }, parent);
>> +
>> +        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);
>> +
>> +
>> +
>> +    }
>>   }
>>
>