On Wed, 28 May 2025 11:26:17 GMT, p-nima <d...@openjdk.org> wrote: > The AuthenticationFilter did not respect the default retry limit of 3 retries > in case of invalid credentials supplied. > > This PR helps to resolve the bug and tests it with default and updated retry > limit set via `jdk.httpclient.auth.retrylimit=1`. > > The test is green with tiers 1, 2, 3 and the test is stable.
test/jdk/java/net/httpclient/HttpClientRetryLimitTest.java line 34: > 32: * @build jdk.httpclient.test.lib.http2.Http2TestServer > 33: * @run junit HttpClientRetryLimitTest > 34: * @run junit/othervm -Djdk.httpclient.auth.retrylimit=1 > HttpClientRetryLimitTest Do you think adding a retrylimit=0 would be beneficial too? This way every scenario would be covered test/jdk/java/net/httpclient/HttpClientRetryLimitTest.java line 47: > 45: import java.net.http.HttpResponse; > 46: > 47: import static jdk.test.lib.Asserts.assertEquals; minor: Did you mean `import static org.junit.jupiter.api.Assertions.assertEquals;` here, as you're using junit? test/jdk/java/net/httpclient/HttpClientRetryLimitTest.java line 52: > 50: > 51: private static final int DEFAULT_RETRY_LIMIT = 3; > 52: private final int retryLimit = > Integer.getInteger("jdk.httpclient.auth.retrylimit", DEFAULT_RETRY_LIMIT); Nit: This feels a bit confusing to me when reading it the first time, why not have a retry limit of 1 or 0 as a default and then specify if you want more retries in `@test`? I think it might be a bit easier to read, but if you want to keep it, it's fine with me. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25490#discussion_r2111682350 PR Review Comment: https://git.openjdk.org/jdk/pull/25490#discussion_r2111670821 PR Review Comment: https://git.openjdk.org/jdk/pull/25490#discussion_r2111681002