On Thu, 25 Apr 2024 12:45:28 GMT, robert engels <[email protected]> wrote:
>> test/jdk/com/sun/net/httpserver/bugs/TcpNoDelayNotRequired.java line 99:
>>
>>> 97: t.sendResponseHeaders(200,5);
>>> 98:
>>> t.getResponseBody().write("hello".getBytes(StandardCharsets.ISO_8859_1));
>>> 99: t.getResponseBody().close();
>>
>> I don't have a strong preference, but would you be open to updating this
>> (and the ChunkedHandler) to:
>>
>>
>> try (InputStream is = t.getRequestBody()) {
>> is.readAllBytes();
>> }
>> Headers reqHeaders = t.getRequestHeaders();
>> Headers respHeaders = t.getResponseHeaders();
>> respHeaders.add("content-type", "text/plain");
>> t.sendResponseHeaders(200,5);
>> try (OutputStream os = t.getResponseBody()) {
>> os.write("hello".getBytes(StandardCharsets.ISO_8859_1));
>> }
>>
>>
>> (I haven't compiled it or done any testing with the proposed snippet)
>
> Isn’t consistency in the code base more important. I like the change but it
> seems there should be a single PR that changes all of the test cases to this
> format.
We have been using these newer constructs whenever a new test gets added, but
we don't update all tests in one go for such changes. If/when old tests are
updated for some bug fix relevant to that test, depending on the complexity we
either decide to let them stay as-is or update them to use these newer
constructs.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579417278
