RFR 8050427 LoginContext tests to cover JDK-4703361

[Amanda Jiang]

Hi Max,

Thanks for your comments ,please check updated webrev and my replies 
inline.
webrev: http://cr.openjdk.java.net/~amjiang/8050427/webrev.02/

Regards,
Amanda

On 8/17/15, 12:29 AM, Weijun Wang wrote:
> There are something I don't understand.
>
> MyConfiguration.java:
>
> - optionOrder: Is it possible to make this an argument of the 
> constructor?
     Fixed, made this an argument of the constructor.
>
> - getConfiguration/setConfiguration: If these are useless, why adding 
> these methods?
     Removed useless methods.
>
> SmartLoginModule.java:
>
> - initialize: Why don't you use the callbackHandler argument?
     See below.
>
> - shouldSucceed: It is always very confusing to make a field 
> accessible from outside a class.
     Originally the test tried to test with different password , 
"shouldSucceed" is used to control which password will be set.
     MycallbackHandler is called in initialize( ) method and 
"shouldSucceed" is parsed as an argument to control which password would 
be set to "PasswdCallback"

  123         this.callbackHandler = new MyCallbackHandler(myUsername, myPassword,
  124                 shouldSucceed);

  341     public MyCallbackHandler(String username, char[] password, boolean action) {
  342         super();
  343         userName = username;
  344         this.password = password;
  345         this.action = action;
  346     }

  361                 PasswordCallback pc = (PasswordCallback) callback;
  362                 if (action) {
  363                     pc.setPassword(password);
  364                 } else {
  365                     pc.setPassword(wrongpd);
  366                 }

     I agree with you this may be confusing, so I simplify this test, 
"shouldSucceed" value is remove, please check updated webrev.
>
> DynamicConfigurationTest.java:
>
> - test: why the if checks are based on both isNegative and success? 
> Why not only on isNegative? If you want to test 2 stages (initialize 
> and login), you can provide 2 isNegative flags.
          "success" was for controlling correct/wrong passwords, 
"isNegative" was for checking if exception is expected.
           Tests are re-organized , so I do not use these two flags 
now,  please check updated webrev.
>
> Thanks
> Max
>
>
> On 08/17/2015 01:39 PM, Amanda Jiang wrote:
>> Hi All,
>>
>> Please be free to review these new tests for Dynamic configuration of
>> authentication modules.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8050427
>> webrev:http://cr.openjdk.java.net/~amjiang/8050427/webrev.01/
>>
>>
>> Thanks,
>> Amanda
>>
>>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20150922/ba1c92ef/attachment.htm>