From 5f541f8112bd611ba11d7d79c632044473b26293 Mon Sep 17 00:00:00 2001 From: curfew-marathon Date: Sun, 28 Jun 2026 14:07:37 -0400 Subject: [PATCH 1/2] test: improve coverage and fix stale comments - Add FgaErrorTest covering getMessage() format variants and all helper methods (isRetryable, isClientError, isServerError, etc.) - Add HttpStatusCodeTest with boundary parameterized tests for isSuccessful, isServerError, and isRetryable including 501 exclusion - Add ClientAssertionTest covering asAssertion() and asAssertions() including null and empty list cases - Upgrade MetricsTest existence assertions to verify caching and delegation (credentialsRequest, requestDuration, queryDuration) - Remove stale "Current code will FAIL" comments in RetryStrategyTest --- .../sdk/api/client/ClientAssertionTest.java | 70 +++++++++ .../dev/openfga/sdk/errors/FgaErrorTest.java | 144 ++++++++++++++++++ .../sdk/errors/HttpStatusCodeTest.java | 37 +++++ .../openfga/sdk/telemetry/MetricsTest.java | 8 + .../openfga/sdk/util/RetryStrategyTest.java | 3 - 5 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 src/test/java/dev/openfga/sdk/api/client/ClientAssertionTest.java create mode 100644 src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java create mode 100644 src/test/java/dev/openfga/sdk/errors/HttpStatusCodeTest.java diff --git a/src/test/java/dev/openfga/sdk/api/client/ClientAssertionTest.java b/src/test/java/dev/openfga/sdk/api/client/ClientAssertionTest.java new file mode 100644 index 00000000..2e907c55 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/api/client/ClientAssertionTest.java @@ -0,0 +1,70 @@ +package dev.openfga.sdk.api.client; + +import static org.assertj.core.api.Assertions.assertThat; + +import dev.openfga.sdk.api.model.Assertion; +import java.util.List; +import org.junit.jupiter.api.Test; + +class ClientAssertionTest { + + @Test + void asAssertion_convertsAllFields() { + ClientAssertion client = new ClientAssertion() + .user("user:anne") + .relation("viewer") + ._object("document:budget") + .expectation(true); + + Assertion result = client.asAssertion(); + + assertThat(result.getTupleKey().getUser()).isEqualTo("user:anne"); + assertThat(result.getTupleKey().getRelation()).isEqualTo("viewer"); + assertThat(result.getTupleKey().getObject()).isEqualTo("document:budget"); + assertThat(result.getExpectation()).isTrue(); + } + + @Test + void asAssertion_falseExpectation() { + ClientAssertion client = new ClientAssertion() + .user("user:bob") + .relation("editor") + ._object("document:budget") + .expectation(false); + + assertThat(client.asAssertion().getExpectation()).isFalse(); + } + + @Test + void asAssertions_withNullList_returnsEmpty() { + assertThat(ClientAssertion.asAssertions(null)).isEmpty(); + } + + @Test + void asAssertions_withEmptyList_returnsEmpty() { + assertThat(ClientAssertion.asAssertions(List.of())).isEmpty(); + } + + @Test + void asAssertions_convertsEachItem() { + List input = List.of( + new ClientAssertion() + .user("user:anne") + .relation("viewer") + ._object("doc:1") + .expectation(true), + new ClientAssertion() + .user("user:bob") + .relation("editor") + ._object("doc:2") + .expectation(false)); + + List result = ClientAssertion.asAssertions(input); + + assertThat(result).hasSize(2); + assertThat(result.get(0).getTupleKey().getUser()).isEqualTo("user:anne"); + assertThat(result.get(0).getExpectation()).isTrue(); + assertThat(result.get(1).getTupleKey().getUser()).isEqualTo("user:bob"); + assertThat(result.get(1).getExpectation()).isFalse(); + } +} diff --git a/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java new file mode 100644 index 00000000..8534fd66 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java @@ -0,0 +1,144 @@ +package dev.openfga.sdk.errors; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.http.HttpHeaders; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class FgaErrorTest { + + private static FgaError error(int status) { + return new FgaError("original", status, HttpHeaders.of(Map.of(), (a, b) -> true), null); + } + + // --- getMessage() format --- + + @Test + void getMessage_withAllFields() { + FgaError error = error(400); + error.setOperationName("write"); + error.setApiErrorMessage("type 'invalid_type' not found"); + error.setApiErrorCode("validation_error"); + error.setRequestId("abc-123"); + + assertThat(error.getMessage()) + .isEqualTo("[write] HTTP 400 type 'invalid_type' not found (validation_error) [request-id: abc-123]"); + } + + @Test + void getMessage_withoutRequestId() { + FgaError error = error(400); + error.setOperationName("write"); + error.setApiErrorMessage("type not found"); + error.setApiErrorCode("validation_error"); + + assertThat(error.getMessage()).isEqualTo("[write] HTTP 400 type not found (validation_error)"); + } + + @Test + void getMessage_withoutOperationName() { + FgaError error = error(400); + error.setApiErrorMessage("type not found"); + error.setApiErrorCode("validation_error"); + error.setRequestId("abc-123"); + + assertThat(error.getMessage()).isEqualTo("HTTP 400 type not found (validation_error) [request-id: abc-123]"); + } + + @Test + void getMessage_withoutApiErrorCode() { + FgaError error = error(400); + error.setOperationName("write"); + error.setApiErrorMessage("some message"); + error.setRequestId("abc-123"); + + assertThat(error.getMessage()).isEqualTo("[write] HTTP 400 some message [request-id: abc-123]"); + } + + @Test + void getMessage_withOnlyStatusCode() { + FgaError error = new FgaError(null, 500, HttpHeaders.of(Map.of(), (a, b) -> true), null); + + assertThat(error.getMessage()).isEqualTo("HTTP 500"); + } + + // --- helper method correctness --- + + @Test + void isValidationError_trueWhenCodeMatches() { + FgaError error = error(400); + error.setApiErrorCode("validation_error"); + assertThat(error.isValidationError()).isTrue(); + } + + @Test + void isValidationError_falseForOtherCode() { + FgaError error = error(400); + error.setApiErrorCode("auth_error"); + assertThat(error.isValidationError()).isFalse(); + } + + @Test + void isUnknownError_trueForUnparseable() { + FgaError error = error(400); + error.setApiErrorCode("unknown_error"); + assertThat(error.isUnknownError()).isTrue(); + } + + @Test + void isRateLimitError_trueFor429() { + FgaError error = error(429); + assertThat(error.isRateLimitError()).isTrue(); + } + + @Test + void isRateLimitError_trueForCode() { + FgaError error = error(400); + error.setApiErrorCode("rate_limit_exceeded"); + assertThat(error.isRateLimitError()).isTrue(); + } + + @Test + void isRetryable_trueFor429() { + assertThat(error(429).isRetryable()).isTrue(); + } + + @Test + void isRetryable_trueFor500() { + assertThat(error(500).isRetryable()).isTrue(); + } + + @Test + void isRetryable_falseFor400() { + assertThat(error(400).isRetryable()).isFalse(); + } + + @Test + void isRetryable_falseFor501() { + assertThat(error(501).isRetryable()).isFalse(); + } + + @Test + void isClientError_trueFor4xx() { + assertThat(error(400).isClientError()).isTrue(); + assertThat(error(422).isClientError()).isTrue(); + assertThat(error(429).isClientError()).isTrue(); + } + + @Test + void isClientError_falseFor5xx() { + assertThat(error(500).isClientError()).isFalse(); + } + + @Test + void isServerError_trueFor5xx() { + assertThat(error(500).isServerError()).isTrue(); + assertThat(error(503).isServerError()).isTrue(); + } + + @Test + void isServerError_falseFor4xx() { + assertThat(error(400).isServerError()).isFalse(); + } +} diff --git a/src/test/java/dev/openfga/sdk/errors/HttpStatusCodeTest.java b/src/test/java/dev/openfga/sdk/errors/HttpStatusCodeTest.java new file mode 100644 index 00000000..058b1783 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/errors/HttpStatusCodeTest.java @@ -0,0 +1,37 @@ +package dev.openfga.sdk.errors; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class HttpStatusCodeTest { + + @ParameterizedTest + @CsvSource({"199, false", "200, true", "299, true", "300, false"}) + void isSuccessful_boundaryValues(int status, boolean expected) { + assertThat(HttpStatusCode.isSuccessful(status)).isEqualTo(expected); + } + + @ParameterizedTest + @CsvSource({"499, false", "500, true", "599, true", "600, false"}) + void isServerError_boundaryValues(int status, boolean expected) { + assertThat(HttpStatusCode.isServerError(status)).isEqualTo(expected); + } + + @ParameterizedTest + @CsvSource({ + "400, false", + "429, true", + "500, true", + "501, false", + "502, true", + "503, true", + "504, true", + "599, true", + "600, false" + }) + void isRetryable(int status, boolean expected) { + assertThat(HttpStatusCode.isRetryable(status)).isEqualTo(expected); + } +} diff --git a/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java b/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java index ffe313ca..1b3bacd2 100644 --- a/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java +++ b/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java @@ -55,6 +55,7 @@ void shouldGetCounter() { // then assertThat(longCounter).isNotNull(); + assertThat(metrics.getCounter(counter, value, attributes)).isSameAs(longCounter); } @Test @@ -70,6 +71,7 @@ void shouldGetHistogram() { // then assertThat(doubleHistogram).isNotNull(); + assertThat(metrics.getHistogram(histogram, value, attributes)).isSameAs(doubleHistogram); } @Test @@ -84,6 +86,8 @@ void shouldCredentialsRequest() { // then assertThat(longCounter).isNotNull(); + assertThat(metrics.getCounter(Counters.CREDENTIALS_REQUEST, value, attributes)) + .isSameAs(longCounter); } @Test @@ -98,6 +102,8 @@ void shouldRequestDuration() { // then assertThat(doubleHistogram).isNotNull(); + assertThat(metrics.getHistogram(Histograms.REQUEST_DURATION, value, attributes)) + .isSameAs(doubleHistogram); } @Test @@ -112,6 +118,8 @@ void shouldQueryDuration() { // then assertThat(doubleHistogram).isNotNull(); + assertThat(metrics.getHistogram(Histograms.QUERY_DURATION, value, attributes)) + .isSameAs(doubleHistogram); } @Test diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index 978be27e..dd6de710 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -120,7 +120,6 @@ void calculateRetryDelay_withZeroRetryAfter_shouldEnforceMinimumDelay() { Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); // Then - Should enforce minimum delay to prevent hot-loop retries - // Current code will FAIL this test by returning Duration.ZERO assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms to prevent hot-loops } @@ -135,7 +134,6 @@ void calculateRetryDelay_withNegativeRetryAfter_shouldEnforceMinimumDelay() { Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); // Then - Should enforce minimum delay to handle malformed server responses - // Current code will FAIL this test by returning negative duration assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms for safety } @@ -150,7 +148,6 @@ void calculateRetryDelay_withVerySmallRetryAfter_shouldEnforceMinimumDelay() { Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); // Then - Should enforce reasonable minimum delay - // Current code will FAIL this test by returning tiny delay assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms for system stability } } From 0a8cd89675fb185f15004be0ad33dc922e72777d Mon Sep 17 00:00:00 2001 From: curfew-marathon Date: Sun, 28 Jun 2026 14:19:13 -0400 Subject: [PATCH 2/2] test: address review comments on caching assertions and test naming --- src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java | 2 +- .../java/dev/openfga/sdk/telemetry/MetricsTest.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java index 8534fd66..01863807 100644 --- a/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java +++ b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java @@ -80,7 +80,7 @@ void isValidationError_falseForOtherCode() { } @Test - void isUnknownError_trueForUnparseable() { + void isUnknownError_trueWhenCodeIsUnknownError() { FgaError error = error(400); error.setApiErrorCode("unknown_error"); assertThat(error.isUnknownError()).isTrue(); diff --git a/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java b/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java index 1b3bacd2..10d91809 100644 --- a/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java +++ b/src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java @@ -55,7 +55,7 @@ void shouldGetCounter() { // then assertThat(longCounter).isNotNull(); - assertThat(metrics.getCounter(counter, value, attributes)).isSameAs(longCounter); + assertThat(metrics.getCounter(counter, null, attributes)).isSameAs(longCounter); } @Test @@ -71,7 +71,7 @@ void shouldGetHistogram() { // then assertThat(doubleHistogram).isNotNull(); - assertThat(metrics.getHistogram(histogram, value, attributes)).isSameAs(doubleHistogram); + assertThat(metrics.getHistogram(histogram, null, attributes)).isSameAs(doubleHistogram); } @Test @@ -86,7 +86,7 @@ void shouldCredentialsRequest() { // then assertThat(longCounter).isNotNull(); - assertThat(metrics.getCounter(Counters.CREDENTIALS_REQUEST, value, attributes)) + assertThat(metrics.getCounter(Counters.CREDENTIALS_REQUEST, null, attributes)) .isSameAs(longCounter); } @@ -102,7 +102,7 @@ void shouldRequestDuration() { // then assertThat(doubleHistogram).isNotNull(); - assertThat(metrics.getHistogram(Histograms.REQUEST_DURATION, value, attributes)) + assertThat(metrics.getHistogram(Histograms.REQUEST_DURATION, null, attributes)) .isSameAs(doubleHistogram); } @@ -118,7 +118,7 @@ void shouldQueryDuration() { // then assertThat(doubleHistogram).isNotNull(); - assertThat(metrics.getHistogram(Histograms.QUERY_DURATION, value, attributes)) + assertThat(metrics.getHistogram(Histograms.QUERY_DURATION, null, attributes)) .isSameAs(doubleHistogram); }