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

Julia Boes jboes at openjdk.java.net
Thu Jan 13 15:39:29 UTC 2022


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