On Tue, 21 Oct 2025 15:14:48 GMT, Josiah Noel <[email protected]> wrote:

>> Now ExchangeImpl will default to having a separate attribute map for the 
>> request duration.
>
> Josiah Noel has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refactor module-info.java for imports and notes
>   
>   Updated import statement and modified implementation notes.

Additional feedback - do we really need two tests? I hadn't realised that they 
were basically the same. I suggest to merge them (and keep the old name)

test/jdk/com/sun/net/httpserver/ExchangeAttributeTest.java line 107:

> 105:                 exchange.setAttribute("attr", "val");
> 106:                 assertEquals("val", exchange.getAttribute("attr"));
> 107:                 assertEquals("val", 
> exchange.getHttpContext().getAttributes().get("attr"));

Suggestion:

                assertEquals("context-val", 
exchange.getHttpContext().getAttributes().get("attr"));

-------------

Changes requested by dfuchs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27652#pullrequestreview-3361503304
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448845528

Reply via email to