On Wed, 8 May 2024 11:44:42 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Nizar Benalla has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Declare `ServerAuthenticator.invoked` as volatile > > test/jdk/com/sun/net/httpserver/BasicAuthToken.java line 28: > >> 26: * @bug 8144100 >> 27: * @run main/othervm BasicAuthToken >> 28: * @summary checking token sent by client should be done in >> case-insensitive manner > > As a guideline, jtreg recommended order of test definition tags is here > https://openjdk.org/jtreg/tag-spec.html#ORDER. As per that the `@summary` > should follow the `@bug` before the `@run`. Done > test/jdk/com/sun/net/httpserver/BasicAuthToken.java line 97: > >> 95: >> 96: static class ServerAuthenticator extends BasicAuthenticator { >> 97: ServerAuthenticator(String realm) { > > Since this test relies on the authenticator being invoked, just as an > additional check, you might want to have a boolean field in this class (named > something like `invoked`) which will be set to `true` only when the > `checkCredentials` gets called. Then after checking the response code in the > test, you can additionally assert that this field's value is `true`, just to > be certain that this authenticator was indeed invoked. Fixed, thanks ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19133#discussion_r1595425463 PR Review Comment: https://git.openjdk.org/jdk/pull/19133#discussion_r1595425320
