Thanks Chris. All good comments.
new webrev at: http://cr.openjdk.java.net/~dfuchs/webrev_8230526/webrev.01/ I'm not sure all httpclient tests that use jdk.httpserver have the appropriate @module tag. But that should be for some further cleanup. best regards, -- daniel On 09/09/2019 16:35, Chris Hegarty wrote:
The source changes look good. A few small comments on the test: 1) @modules java.net.http This should be jdk.httpserver, right? - since there is a TEST.properties that already specifies the java.net.http module. 2) Maybe refactor some of the initial code in the `test` method into, say, `sanity`? as it is not really testing the bug, but the test itself, right? 3) Trivially, I don’t think that the @library is needed? 4) Maybe reword the comment on the `createHttpsServer` test method, “For convenience the server is used as ...”.