RFR[8217825]: Verify @AfterTest is used correctly in WebSocket tests

[Patrick Concannon]

Hi Pavel,

Thanks for the feedback. I've incorporated the changes you suggested, 
and you can find them in the new webrev below.
http://cr.openjdk.java.net/~pconcannon/8217825/webrevs/webrev.02/

Kind regards,
Patrick

On 20/09/2019 13:21, Pavel Rappo wrote:
> Patrick,
>
> Thank you for taking care of that!
> Before you push, please add missing spaces to
>
>      WebSocketTest.java:612
>      WebSocketTest.java:713
>
> (no need to update the webrev)
>
> Nits:
>
> WebSocketTest.java:808 does not abort WebSocket. Now, I understand that in normal circumstances we never expect a WebSocket to be created. However, for uniformity's sake, I would use abort here as well.
>
> I understand that "single line" method formatting used in AutomaticPong.java:140+ might not be everyone's cup of tea, but it might add to readability in the case of a lengthy definition like that.
>
> -Pavel
>
>> On 20 Sep 2019, at 11:36, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>
>> Hi Patrick,
>>
>> Looks good to me!
>>
>> best regards,
>>
>> -- daniel
>>
>> On 20/09/2019 11:27, Patrick Concannon wrote:
>>> Hi Daniel,
>>> Thanks for the feedback. That's a good idea. I've made those changes and I've included them in the webrev below.
>>> http://cr.openjdk.java.net/~pconcannon/8217825/webrevs/webrev.01/
>>> Kind regards,
>>> Patrick
>>> On 13/09/2019 15:47, Daniel Fuchs wrote:
>>>> Hi Patrick,
>>>>
>>>> I wonder if you should be using `try { } finally { }` to ensure
>>>> that the websocket is closed too:
>>>>
>>>>     var websocket = .....;
>>>>     try {
>>>>      // send some messages etc...
>>>>     } finally {
>>>>      websocket.abort();
>>>>     }
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>>
>>>> On 13/09/2019 15:36, Patrick Concannon wrote:
>>>>> Hi,
>>
>>>>>
>>>>> Would it be possible to have my fix for JDK-8217825 - Verify @AfterTest is used correctly in WebSocket tests - reviewed?
>>>>>
>>>>> Following on from the discussion had here: https://mail.openjdk.java.net/pipermail/net-dev/2019-January/012141.html, I’ve refactored several websocket tests to explicitly close resources opened during each test method i.e. test servers and websockets.
>>>>>
>>>>> Tests refactored:
>>>>> - test/jdk/java/net/httpclient/websocket/AutomaticPong.java
>>>>> - test/jdk/java/net/httpclient/websocket/WebsocketTest.java
>>>>> - test/jdk/java/net/httpclient/websocket/SendTest.java
>>>>> - test/jdk/java/net/httpclient/websocket/Abort.java
>>>>>
>>>>>
>>>>> Further information on this bug can be found here: https://bugs.openjdk.java.net/browse/JDK-8217825
>>>>>
>> Webrev for fix:
>>>>> http://cr.openjdk.java.net/~pconcannon/8217825/webrevs/webrev.00/
>>>>>
>>>>>
>> Kind regards,
>>
>>>>> Patrick
>>>>>