> On Oct 16, 2016, at 11:18 AM, Peter Levart <peter.lev...@gmail.com> wrote:
> I think specifying both is more verbose on one hand, but OTOH it allows one 
> to visually inspect all the cases and think of each and every one in 
> isolation to see if it is valid in a particular caller/member/target 
> arrangement. The test infrastructure verifies that the test case covers all 
> the MemberFactory(s) so one must only verify each individual allowed / denied 
> MemberFactory.

I understand the intention there.  But when each test case enumerates 20 
constants, it’s getting harder to review what’s going on and catch any issue.

>>    Builder::allowAll and Builder::denyAll would be useful.  
>> allowAccessMember of a specific modifier can imply both field and method.
> This is a good idea. Here's a modified test (no changes to patched files, 
> just tests):
> http://cr.openjdk.java.net/~plevart/jdk9-dev/6378384_Reflection.ensureAccess/webrev.03/

I like these grouping and it does help.  One more nit: would be good to replace 
the class name strings with constant variables.  I don’t need a new webrev.


Reply via email to