> On Nov 25, 2015, at 7:10 AM, Peter <j...@zeus.net.au> wrote:
> 
> Interesting, why not change the test to check that npe isn't thrown?

Excellent question!  I’m open to argument on this (which is why the test is 
currently commented out, not removed), but here’s my logic:

- The behavior being tested here is not part of River’s specification (the 
documentation simply refers to the JDK documentation)
- The behavior is not actually implemented within River code (River’s 
PolicyProvider delegates to an instance of java.security.Policy)
- I can’t find any evidence that River code depends on this behavior.

If the test stayed in, with the sense flipped, the test’s success would be 
dependent on the JDK version.  That would make sense if this was a bug that we 
(River) cared about, but since it’s not our behavior and it’s not behavior we 
depend on, that seems like we would just be making the test suite more fragile 
without any benefit.  

Cheers,

Greg Trasuk

> 
> I did notice while reimplimenting the file policy provider, that rivers tests 
> were very thorough.  Now you've confirmed they even helped duplicate the 
> bugs.  So I'll have to fix that, thanks for the info.
> 
> I was approached by the openjdk devs about improving Java's policy provider 
> recently and i passed on some tips.  I can say however, with the utmost 
> confidence that River has the worlds fastest and most scalable policy 
> provider, by a good margin.
> 
> Regards,
> 
> Peter.
> 
> 
> Sent from my Samsung device.
>   Include original message
> ---- Original message ----
> From: gtra...@apache.org
> Sent: 25/11/2015 01:44:21 pm
> To: comm...@river.apache.org
> Subject: svn commit: r1716294 - in 
> /river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider: 
> dynamicPolicyProvider/NullCases.java policyFileProvider/NullCases.java
> 
> Author: gtrasuk 
> Date: Wed Nov 25 03:44:21 2015 
> New Revision: 1716294 
> 
> URL: http://svn.apache.org/viewvc?rev=1716294&view=rev 
> Log: 
> 
>         The tests below check that if you call getPermissions(codesource)  
>         with a null codesource, a NullPointerException should be thrown.  The 
>         implementation of it relies not with DynamicPolicyProvider or 
> PolicyFileProvider, but with the 
>         underlying Policy object (java.security.Policy), which used to do 
> just  
>         that. 
>          
>         However, this behaviour was reported as a bug in JDK-7147830  
>         (2012-02-22), and resolved only in JDK8, as of 2012-07-17. 
>         So it's no longer appropriate to do this test.  Commented out for 
> now, 
>         remove it completely if you happen to see this far in the future. 
> 
> 
> Modified: 
>     
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/dynamicPolicyProvider/NullCases.java
>  
>     
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/policyFileProvider/NullCases.java
>  
> 
> Modified: 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/dynamicPolicyProvider/NullCases.java
>  
> URL: 
> http://svn.apache.org/viewvc/river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/dynamicPolicyProvider/NullCases.java?rev=1716294&r1=1716293&r2=1716294&view=diff
>  
> ==============================================================================
>  
> --- 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/dynamicPolicyProvider/NullCases.java
>  (original) 
> +++ 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/dynamicPolicyProvider/NullCases.java
>  Wed Nov 25 03:44:21 2015 
> @@ -204,10 +204,22 @@ public class NullCases extends DynamicPo 
>          msg = "policy.getPermissions((ProtectionDomain) null)"; 
>          callGetPermissions((ProtectionDomain) null, pmGranted, msg); 
>   
> +        /* (Greg Trasuk - 20151124) 
> +        The test below checks that if you call getPermissions(codesource)  
> +        with a null codesource, a NullPointerException should be thrown.  
> The 
> +        implementation of it relies not with DynamicPolicyProvider but with 
> the 
> +        underlying Policy object (java.security.Policy), which used to do 
> just  
> +        that. 
> +         
> +        However, this behaviour was reported as a bug in JDK-7147830  
> +        (2012-02-22), and resolved in in JDK8, as of 2012-07-17. 
> +        So it's no longer appropriate to do this test.  Commented out for 
> now, 
> +        remove it completely if you happen to see this far in the future. 
> +        */ 
>          // Call getPermissions() passing null as CodeSource 
>          // and verify that NullPointerException is thrown; 
> -        msg = "policy.getPermissions((CodeSource) null)"; 
> -        callGetPermissionsNPE((CodeSource) null, msg); 
> +        //msg = "policy.getPermissions((CodeSource) null)"; 
> +        //callGetPermissionsNPE((CodeSource) null, msg); 
>   
>          // Call policy.implies(null, null) 
>          // and verify that NullPointerException is thrown; 
> 
> Modified: 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/policyFileProvider/NullCases.java
>  
> URL: 
> http://svn.apache.org/viewvc/river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/policyFileProvider/NullCases.java?rev=1716294&r1=1716293&r2=1716294&view=diff
>  
> ==============================================================================
>  
> --- 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/policyFileProvider/NullCases.java
>  (original) 
> +++ 
> river/jtsk/branches/2.2/qa/src/com/sun/jini/test/spec/policyprovider/policyFileProvider/NullCases.java
>  Wed Nov 25 03:44:21 2015 
> @@ -112,12 +112,24 @@ public class NullCases extends PolicyFil 
>           */ 
>          callGetPermissionsNPD(); 
>   
> +        /* (Greg Trasuk - 20151124) 
> +        The test below checks that if you call getPermissions(codesource)  
> +        with a null codesource, a NullPointerException should be thrown.  
> The 
> +        implementation of it relies not with DynamicPolicyProvider but with 
> the 
> +        underlying Policy object (java.security.Policy), which used to do 
> just  
> +        that. 
> +         
> +        However, this behaviour was reported as a bug in JDK-7147830  
> +        (2012-02-22), and resolved in in JDK8, as of 2012-07-17. 
> +        So it's no longer appropriate to do this test.  Commented out for 
> now, 
> +        remove it completely if you happen to see this far in the future. 
> +        */ 
>          /* 
>           * Call getPermissions() passing null as CodeSource 
>           * and verify that NullPointerException is thrown; 
>           */ 
> -        msg = "policy.getPermissions((CodeSource) null)"; 
> -        callGetPermissionsNPE((CodeSource) null, msg); 
> +        //msg = "policygetPermissions((CodeSource) null)"; 
> +        //callGetPermissionsNPE((CodeSource) null, msg); 
>   
>          /* 
>           * Call policy.implies(null, null) 
> 
> 
> 

Reply via email to