RFR: 8278398: jwebserver: Add test to confirm maximum request time [v2]

Jaikiran Pai jai.forums2013 at gmail.com
Fri Jan 14 01:33:25 UTC 2022


Thank you Julia, these new changes look fine to me.

-Jaikiran

On 13/01/22 9:09 pm, Julia Boes wrote:
> On Thu, 13 Jan 2022 04:52:25 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>
>>> Julia Boes has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    address PR comments:
>>>    * remove redundant null check
>>>    * use try-finally to stop process in test
>> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 525:
>>
>>> 523:             logger.log(Level.TRACE, "exchange started");
>>> 524:
>>> 525:             if (dispatcherThread != null && dispatcherThread == Thread.currentThread()) {
>> Hello Julia,
>> Minor thing - With the `dispatcherThread == Thread.currentThread()` check, the `dispatcherThread != null` would be redundant and perhaps can be removed?
> You're right, removed.
>
>> test/jdk/com/sun/net/httpserver/simpleserver/jwebserver/MaxRequestTimeTest.java line 104:
>>
>>> 102:         sendHTTPRequest();   // server expected to respond successfully
>>> 103:
>>> 104:         p.destroy();
>> Since the `sendXXXRequest()` methods above could potentially throw exceptions, do you think it would be better to destroy the process in a finally block? Or maybe manage the process start and destroy as testng before/after lifecycle methods?
> Good point, I'll update that. Thanks!
>
> -------------
>
> PR: https://git.openjdk.java.net/jdk/pull/7052


More information about the net-dev mailing list