RFR: 8223714: HTTPSetAuthenticatorTest could be made more resilient

[Vyom Tiwari]

Hi Jaikiran,

Code changes look OK to me , couple of minor comments
1-> you don't need "this" to access stop(this.stop)
2-> i think super.stop() should be the first line in the overridden stop
method.
3-> it is matter of personal preference but my preference is use the new
variable name 'running' to check if server is running in place of 'stop'
but again it is just a matter of personal  preference.

Thanks,
Vyom

On Sat, Aug 31, 2019 at 9:37 AM Jaikiran Pai <jai.forums2013 at gmail.com>
wrote:

> Hello Daniel,
>
> On 30/08/19 9:14 PM, Daniel Fuchs wrote:
> > Hi Jaikiran,
> >
> > 1075                     System.out.println("Server will stop
> > accepting connections due to an exception");
> > 1076                     io.printStackTrace(System.out);
> > 1077                     break;
> >
> > Two things here:
> >
> >  1.if ss.connect() throws, it may be because ss is closed, or not.
> >    To be on the safe side, we should call ss.close() before break;
>
> You are right. I have now updated this part of the code.
>
>
> >  2.if the application calls stop() I expect that the call to ss.accept()
> >    will throw.
> >
> >    Therefore I think that the exception should be simply skipped
> >    if stop == true (that's normal termination, we don't want to pollute
> >    the log with stack traces of exceptions that are expected),
> >    but we should probably always print it on System.err if stop == false
> >    otherwise we will be reduced to guessing if we see the client side
> >    failing with Connection Refused.
> >
> Agreed. This has now been updated too.
>
> The updated webrev is at
> http://cr.openjdk.java.net/~jpai/webrev/8223714/2/webrev/. I have also
> included you as the reviewer in the commit message.
>
>
> > Otherwise, I believe it looks good. Thanks for another good fix :-)
>
> Thank you :)
>
> -Jaikiran
>
>
>

-- 
Thanks,
Vyom
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20190831/9e11c868/attachment.html>