> 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. thanks Mandy
