Skip to content

Commit 3e6489b

Browse files
jimmyjamesjimmyjames
committed
fix: test optimization
1 parent abcd97d commit 3e6489b

1 file changed

Lines changed: 34 additions & 39 deletions

File tree

‎src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java‎

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ void shouldRetryWith429AndRetryAfterHeader() throws Exception {
6767
.whenScenarioStateIs("Started")
6868
.willReturn(aResponse()
6969
.withStatus(429)
70-
.withHeader("Retry-After", "1")
70+
.withHeader("Retry-After", "0.05") // Fast retry for test performance - timing not verified
7171
.withBody("{\"error\":\"rate limited\"}"))
7272
.willSetStateTo("First Retry"));
7373

@@ -102,7 +102,7 @@ void shouldRetryWith500AndRetryAfterHeaderForGetRequest() throws Exception {
102102
.whenScenarioStateIs("Started")
103103
.willReturn(aResponse()
104104
.withStatus(500)
105-
.withHeader("Retry-After", "1")
105+
.withHeader("Retry-After", "0.05") // Fast retry for test performance - timing not verified
106106
.withBody("{\"error\":\"server error\"}"))
107107
.willSetStateTo("First Retry"));
108108

@@ -164,7 +164,7 @@ void shouldRetryWith500WithRetryAfterHeaderForPostRequest() throws Exception {
164164
.whenScenarioStateIs("Started")
165165
.willReturn(aResponse()
166166
.withStatus(500)
167-
.withHeader("Retry-After", "1")
167+
.withHeader("Retry-After", "0.05") // Fast retry for test performance - timing not verified
168168
.withBody("{\"error\":\"server error\"}"))
169169
.willSetStateTo("First Retry"));
170170

@@ -195,10 +195,7 @@ void shouldRetryWith500WithRetryAfterHeaderForPostRequest() throws Exception {
195195
void shouldNotRetryWith501() throws Exception {
196196
// Given
197197
wireMockServer.stubFor(get(urlEqualTo("/test"))
198-
.willReturn(aResponse()
199-
.withStatus(501)
200-
.withHeader("Retry-After", "1")
201-
.withBody("{\"error\":\"not implemented\"}")));
198+
.willReturn(aResponse().withStatus(501).withBody("{\"error\":\"not implemented\"}")));
202199

203200
HttpRequest request = HttpRequest.newBuilder()
204201
.uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test"))
@@ -224,10 +221,7 @@ void shouldNotRetryWith501() throws Exception {
224221
void shouldRespectMaxRetries() throws Exception {
225222
// Given
226223
wireMockServer.stubFor(get(urlEqualTo("/test"))
227-
.willReturn(aResponse()
228-
.withStatus(429)
229-
.withHeader("Retry-After", "1")
230-
.withBody("{\"error\":\"rate limited\"}")));
224+
.willReturn(aResponse().withStatus(429).withBody("{\"error\":\"rate limited\"}")));
231225

232226
HttpRequest request = HttpRequest.newBuilder()
233227
.uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test"))
@@ -416,7 +410,7 @@ void shouldUseExponentialBackoffForNetworkErrorsWithPreciseTiming() throws Excep
416410
HttpRequest request = HttpRequest.newBuilder()
417411
.uri(java.net.URI.create("http://invalid-hostname-that-does-not-exist.local/test"))
418412
.GET()
419-
.timeout(Duration.ofSeconds(1)) // Reasonable timeout
413+
.timeout(Duration.ofMillis(500)) // Reasonable timeout
420414
.build();
421415

422416
HttpRequestAttempt<Void> attempt = new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig);
@@ -515,7 +509,7 @@ void shouldRespectGlobalMinimumRetryDelayWithExponentialBackoff() throws Excepti
515509
ClientConfiguration globalConfig = new ClientConfiguration()
516510
.apiUrl("http://localhost:" + wireMockServer.port())
517511
.maxRetries(2)
518-
.minimumRetryDelay(Duration.ofSeconds(2)); // Should act as floor for exponential backoff
512+
.minimumRetryDelay(Duration.ofMillis(100)); // Should act as floor for exponential backoff
519513

520514
HttpRequestAttempt<Void> attempt =
521515
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig);
@@ -535,8 +529,8 @@ void shouldRespectGlobalMinimumRetryDelayWithExponentialBackoff() throws Excepti
535529
// Verify that it retried the expected number of times
536530
wireMockServer.verify(1 + globalConfig.getMaxRetries(), getRequestedFor(urlEqualTo("/test")));
537531

538-
// With 2 retries and minimum 2-second delays, total time should be at least 4 seconds
539-
assertThat(totalTime.toMillis()).isGreaterThan(3500); // Should be at least ~4 seconds
532+
// With 2 retries and minimum 100ms delays, total time should be at least 200ms
533+
assertThat(totalTime.toMillis()).isGreaterThan(150); // Should be at least ~200ms
540534
}
541535

542536
@Test
@@ -547,7 +541,7 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanGlobalMinimumDelay() throws Exc
547541
.whenScenarioStateIs("Started")
548542
.willReturn(aResponse()
549543
.withStatus(429)
550-
.withHeader("Retry-After", "1") // 1 second
544+
.withHeader("Retry-After", "0.05") // 50ms
551545
.withBody("{\"error\":\"rate limited\"}"))
552546
.willSetStateTo("After-First-Request"));
553547

@@ -565,7 +559,7 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanGlobalMinimumDelay() throws Exc
565559
ClientConfiguration globalConfig = new ClientConfiguration()
566560
.apiUrl("http://localhost:" + wireMockServer.port())
567561
.maxRetries(2)
568-
.minimumRetryDelay(Duration.ofSeconds(3)); // Should NOT override Retry-After: 1
562+
.minimumRetryDelay(Duration.ofMillis(150)); // Should NOT override Retry-After
569563

570564
HttpRequestAttempt<Void> attempt =
571565
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig);
@@ -579,9 +573,9 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanGlobalMinimumDelay() throws Exc
579573
Duration totalTime = Duration.between(startTime, endTime);
580574

581575
// Then
582-
// Should have respected the Retry-After header (1 second) instead of minimum retry delay (3 seconds)
583-
assertThat(totalTime.toMillis()).isGreaterThan(800); // Should be at least ~1 second
584-
assertThat(totalTime.toMillis()).isLessThan(2500); // But less than 2.5 seconds (well below the 3s minimum)
576+
// Should have respected the Retry-After header (50ms) instead of minimum retry delay (150ms)
577+
assertThat(totalTime.toMillis()).isGreaterThan(30); // Should be at least ~50ms
578+
assertThat(totalTime.toMillis()).isLessThan(400); // But less than 400ms (well below the 150ms minimum)
585579

586580
// Verify both requests were made
587581
wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test")));
@@ -595,7 +589,7 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception {
595589
.whenScenarioStateIs("Started")
596590
.willReturn(aResponse()
597591
.withStatus(429)
598-
.withHeader("Retry-After", "2") // 2 seconds
592+
.withHeader("Retry-After", "0.1") // 100ms
599593
.withBody("{\"error\":\"rate limited\"}"))
600594
.willSetStateTo("After-First-Request"));
601595

@@ -613,7 +607,7 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception {
613607
ClientConfiguration globalConfig = new ClientConfiguration()
614608
.apiUrl("http://localhost:" + wireMockServer.port())
615609
.maxRetries(2)
616-
.minimumRetryDelay(Duration.ofMillis(500)); // Should NOT override Retry-After: 2
610+
.minimumRetryDelay(Duration.ofMillis(50)); // Should NOT override Retry-After: 100ms
617611

618612
HttpRequestAttempt<Void> attempt =
619613
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig);
@@ -627,13 +621,12 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception {
627621
Duration totalTime = Duration.between(startTime, endTime);
628622

629623
// Then
630-
// Should have respected the Retry-After header (2 seconds) over minimum delay (500ms)
624+
// Should have respected the Retry-After header (100ms) over minimum delay (50ms)
631625
// Note: Using generous bounds due to timing variability in test environments
632626
System.out.println("Actual retry duration: " + totalTime.toMillis() + " ms");
633627

634-
assertThat(totalTime.toMillis())
635-
.isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance)
636-
assertThat(totalTime.toMillis()).isLessThan(15000); // But not excessive
628+
assertThat(totalTime.toMillis()).isGreaterThan(50); // Should be at least ~100ms (with tolerance)
629+
assertThat(totalTime.toMillis()).isLessThan(1000); // But not excessive
637630

638631
// Verify both requests were made
639632
wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test")));
@@ -651,8 +644,9 @@ void shouldRespectPerRequestMinimumRetryDelayOverride() throws Exception {
651644
.build();
652645

653646
// Override with larger minimum retry delay using per-request configuration
654-
dev.openfga.sdk.api.configuration.Configuration overriddenConfig = configuration.override(
655-
new dev.openfga.sdk.api.configuration.ConfigurationOverride().minimumRetryDelay(Duration.ofSeconds(2)));
647+
dev.openfga.sdk.api.configuration.Configuration overriddenConfig =
648+
configuration.override(new dev.openfga.sdk.api.configuration.ConfigurationOverride()
649+
.minimumRetryDelay(Duration.ofMillis(100)));
656650

657651
HttpRequestAttempt<Void> attempt =
658652
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, overriddenConfig);
@@ -672,8 +666,8 @@ void shouldRespectPerRequestMinimumRetryDelayOverride() throws Exception {
672666
// Verify that it retried the expected number of times
673667
wireMockServer.verify(1 + overriddenConfig.getMaxRetries(), getRequestedFor(urlEqualTo("/test")));
674668

675-
// With 3 retries and minimum 2-second delays, total time should be at least 6 seconds
676-
assertThat(totalTime.toMillis()).isGreaterThan(5500); // Should be at least ~6 seconds
669+
// With 3 retries and minimum 100ms delays, total time should be at least 300ms
670+
assertThat(totalTime.toMillis()).isGreaterThan(250); // Should be at least ~300ms
677671
}
678672

679673
@Test
@@ -718,7 +712,7 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanMinimumDelay() throws Exception
718712
.whenScenarioStateIs("Started")
719713
.willReturn(aResponse()
720714
.withStatus(429)
721-
.withHeader("Retry-After", "1") // 1 second
715+
.withHeader("Retry-After", "0.05") // 50ms
722716
.withBody("{\"error\":\"rate limited\"}"))
723717
.willSetStateTo("After-First-Request"));
724718

@@ -733,8 +727,9 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanMinimumDelay() throws Exception
733727
.build();
734728

735729
// Override with larger minimum retry delay (should NOT take precedence over Retry-After)
736-
dev.openfga.sdk.api.configuration.Configuration overriddenConfig = configuration.override(
737-
new dev.openfga.sdk.api.configuration.ConfigurationOverride().minimumRetryDelay(Duration.ofSeconds(3)));
730+
dev.openfga.sdk.api.configuration.Configuration overriddenConfig =
731+
configuration.override(new dev.openfga.sdk.api.configuration.ConfigurationOverride()
732+
.minimumRetryDelay(Duration.ofMillis(150)));
738733

739734
HttpRequestAttempt<Void> attempt =
740735
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, overriddenConfig);
@@ -748,9 +743,9 @@ void shouldUseRetryAfterHeaderEvenWhenSmallerThanMinimumDelay() throws Exception
748743
Duration totalTime = Duration.between(startTime, endTime);
749744

750745
// Then
751-
// Should have respected the Retry-After header (1 second) instead of minimum retry delay (3 seconds)
752-
assertThat(totalTime.toMillis()).isGreaterThan(800); // Should be at least ~1 second
753-
assertThat(totalTime.toMillis()).isLessThan(2500); // But less than 2.5 seconds (well below the 3s minimum)
746+
// Should have respected the Retry-After header (50ms) instead of minimum retry delay (150ms)
747+
assertThat(totalTime.toMillis()).isGreaterThan(30); // Should be at least ~50ms
748+
assertThat(totalTime.toMillis()).isLessThan(400); // But less than 400ms (well below the 150ms minimum)
754749

755750
// Verify both requests were made
756751
wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test")));
@@ -764,7 +759,7 @@ void shouldNotOverrideRetryAfterWhenItIsLargerThanMinimumDelayPerRequest() throw
764759
.whenScenarioStateIs("Started")
765760
.willReturn(aResponse()
766761
.withStatus(429)
767-
.withHeader("Retry-After", "1") // 1 second delay
762+
.withHeader("Retry-After", "0.05") // 50ms
768763
.withBody("{\"error\":\"rate limited\"}"))
769764
.willSetStateTo("retry-attempted"));
770765

@@ -798,9 +793,9 @@ void shouldNotOverrideRetryAfterWhenItIsLargerThanMinimumDelayPerRequest() throw
798793
Duration totalTime = Duration.between(startTime, endTime);
799794

800795
// Then
801-
// Should have respected the Retry-After header (1 second) for the single retry
796+
// Should have respected the Retry-After header (50ms) for the single retry
802797
// Note: actual timing may vary in test environments, so we use generous bounds
803-
assertThat(totalTime.toMillis()).isGreaterThan(800); // Should be at least ~1 second
798+
assertThat(totalTime.toMillis()).isGreaterThan(30); // Should be at least ~50ms
804799
assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive (was sometimes 4x in CI)
805800

806801
// Verify initial request + 1 retry = 2 total requests

0 commit comments

Comments
 (0)