Re: [teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Chris Hegarty]

> On 27 Jun 2019, at 14:57, Julia Boes  wrote:
> 
> Hi,
> 
> Thanks Brian and Chris for your comments! I incorporated the changes.
> 
> Here's the updated webrev:
> 
> http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev.01/

Thanks for the updated webrev, Julia. You have my Review.

-Chris.

Re: [teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Julia Boes]

Hi,

Thanks Brian and Chris for your comments! I incorporated the changes.

Here's the updated webrev:

http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev.01/


Cheers,

Julia

On 26/06/2019 21:32, Chris Hegarty wrote:

Julia,


On 26 Jun 2019, at 18:33, Julia Boes  wrote:

Hi,

Please find below a patch for:

8226825: Replace wildcard address with loopback or local host in tests - part 19

https://bugs.openjdk.java.net/browse/JDK-8226825


webrev:

http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev/

This is good work, and interesting what one finds when digging into some
of these old tests. I can see that AsyncDisconnect does not operated as
expected ( the currently checked in version passes on my machine,
without the test executing correctly )!

Rather than catching and re-throwing non-test sensitive exceptions, it
is simpler to just let them flow up the stack ( the methods that are
currently catching can just declare `throws Exception` ). Since jtreg
will report a test failure if there is an uncaught exception in the main
thread.

Additionally, any threads that are started by a test, AsyncDisconnect is
one such example, should wait (join) the threads that it has started.
This is good practice, since jtreg considers a test complete when its
main method returns ( could be non-daemon test threads left behind ).

-Chris.

Re: [teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Chris Hegarty]

Julia,

> On 26 Jun 2019, at 18:33, Julia Boes  wrote:
> 
> Hi,
> 
> Please find below a patch for:
> 
> 8226825: Replace wildcard address with loopback or local host in tests - part 
> 19
> 
> https://bugs.openjdk.java.net/browse/JDK-8226825
> 
> 
> webrev:
> 
> http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev/

This is good work, and interesting what one finds when digging into some
of these old tests. I can see that AsyncDisconnect does not operated as
expected ( the currently checked in version passes on my machine,
without the test executing correctly )!

Rather than catching and re-throwing non-test sensitive exceptions, it
is simpler to just let them flow up the stack ( the methods that are
currently catching can just declare `throws Exception` ). Since jtreg
will report a test failure if there is an uncaught exception in the main
thread.

Additionally, any threads that are started by a test, AsyncDisconnect is
one such example, should wait (join) the threads that it has started.
This is good practice, since jtreg considers a test complete when its
main method returns ( could be non-daemon test threads left behind ).

-Chris.

Re: [teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Brian Burkhalter]

Hi Julia,

(part 19 (!))

In AsyncDisconnect line 57 could be replaced with ioe.printStackTrace() to be 
consistent with lines 85 and 88.
Same thing for B6641309 line 58, and B6660405 line 108.

I’ll leave comments on the more substantive changes to the net-dev experts.

Thanks,

Brian

> On Jun 26, 2019, at 10:33 AM, Julia Boes  wrote:
> 
> webrev:
> 
> http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev/ 
> 
> 
> - Replaced wildcard address with loopback or localhost in all tests
> 
> - Replaced URL constructor with URIBuilder
> 
> - Set proxy to null per default for all tests but SetSoLinger
> 
> - Test B6660405 now checks the length of the input stream and throws an 
> AssertionError if the length is not as expected.

[teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Julia Boes]

Hi,

Please find below a patch for:

8226825: Replace wildcard address with loopback or local host in tests - 
part 19


https://bugs.openjdk.java.net/browse/JDK-8226825


webrev:

http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev/

- Replaced wildcard address with loopback or localhost in all tests

- Replaced URL constructor with URIBuilder

- Set proxy to null per default for all tests but SetSoLinger

- Test B6660405 now checks the length of the input stream and throws an 
AssertionError if the length is not as expected.


Cheers,

Julia