On Fri, 10 May 2024 07:56:38 GMT, Jaikiran Pai <[email protected]> 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 94:
>
>> 92: " used to identify authentication scheme sent
>> by client parsed incorrectly");
>> 93: }
>> 94: assert ServerAuthenticator.wasChecked() : "Authenticator was
>> not correctly invoked";
>
> As far as I know, the jtreg tests that we launch (through make) will always
> have asserts enabled. So I think using `assert` here is OK. However, to be
> consistent with other conditional checks in this test, I think it would be
> better to change this to a if block, something like:
>
>
> if (!ServerAuthenticator.wasChecked()) {
> throw new RuntimeException("Authenticator wasn't invoked");
> }
Fixed it! And yes, all tests passed using `assert`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19133#discussion_r1596785358
