From 5914e096b8569a86361b99b392f66feb88b41341 Mon Sep 17 00:00:00 2001
From: JM <36801022+jdm2212@users.noreply.github.com>
Date: Tue, 21 Nov 2023 12:42:50 -0500
Subject: [PATCH] Make ServiceException and RemoteException assertions better
(#1058)
When a ServiceException or RemoteException fails to satisfy test conditions, log the actual failing exception, including error instance ID.
This is useful for tracking down the error in the test container logs.
Errors now look like:
```
"Expected error status to be 500, but found 501; remote exception: com.palantir.conjure.java.api.errors.RemoteException: RemoteException: FAILED_PRECONDITION (Default:FailedPrecondition) with instance ID a7507bd9-fcae-4cee-a888-f38710c30491
at com.palantir.conjure.java.api.testing.RemoteExceptionAssertTest.lambda$testSanity$0(RemoteExceptionAssertTest.java:37)
at org.assertj.core.api.ThrowableAssert.catchThrowable(ThrowableAssert.java:63)
at org.assertj.core.api.AssertionsForClassTypes.catchThrowable(AssertionsForClassTypes.java:892)
at org.assertj.core.api.Assertions.catchThrowable(Assertions.java:1366)
at org.assertj.core.api.Assertions.assertThatThrownBy(Assertions.java:1210)
at com.palantir.conjure.java.api.testing.RemoteExceptionAssertTest.testSanity(RemoteExceptionAssertTest.java:37)
```
---
.../api/testing/RemoteExceptionAssert.java | 24 +++++++++++--------
.../api/testing/ServiceExceptionAssert.java | 14 +++++++----
.../testing/RemoteExceptionAssertTest.java | 8 +++++--
.../testing/ServiceExceptionAssertTest.java | 16 ++++++-------
4 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/test-utils/src/main/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssert.java b/test-utils/src/main/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssert.java
index 625735351..9c6d76f80 100644
--- a/test-utils/src/main/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssert.java
+++ b/test-utils/src/main/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssert.java
@@ -18,7 +18,9 @@
import com.palantir.conjure.java.api.errors.ErrorType;
import com.palantir.conjure.java.api.errors.RemoteException;
+import java.util.Objects;
import org.assertj.core.api.AbstractThrowableAssert;
+import org.assertj.core.util.Throwables;
public class RemoteExceptionAssert extends AbstractThrowableAssert<RemoteExceptionAssert, RemoteException> {
@@ -28,21 +30,23 @@ public class RemoteExceptionAssert extends AbstractThrowableAssert<RemoteExcepti
public final RemoteExceptionAssert isGeneratedFromErrorType(ErrorType type) {
isNotNull();
+
String actualCode = actual.getError().errorCode();
String actualName = actual.getError().errorName();
int actualStatus = actual.getStatus();
- if (!actualCode.equals(type.code().name())) {
- failWithMessage(
- "Expected error code to be %s, but found %s", type.code().name(), actualCode);
- }
- if (!actualName.equals(type.name())) {
- failWithMessage("Expected error name to be %s, but found %s", type.name(), actualName);
- }
- if (!(actualStatus == type.httpErrorCode())) {
- failWithMessage("Expected error status to be %s, but found %s", type.httpErrorCode(), actualStatus);
- }
+ failIfNotEqual("error code", type.code().name(), actualCode);
+ failIfNotEqual("error name", type.name(), actualName);
+ failIfNotEqual("error status", type.httpErrorCode(), actualStatus);
return this;
}
+
+ private <T> void failIfNotEqual(String fieldName, T expectedValue, T actualValue) {
+ if (!Objects.equals(expectedValue, actualValue)) {
+ failWithMessage(
+ "Expected %s to be %s, but found %s; remote exception: %s",
+ fieldName, expectedValue, actualValue, Throwables.getStackTrace(actual));
+ }
+ }
}
diff --git a/test-utils/src/main/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssert.java b/test-utils/src/main/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssert.java
index 9a41253c6..e48d80547 100644
--- a/test-utils/src/main/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssert.java
+++ b/test-utils/src/main/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssert.java
@@ -25,6 +25,7 @@
import java.util.Map;
import java.util.Objects;
import org.assertj.core.api.AbstractThrowableAssert;
+import org.assertj.core.util.Throwables;
public class ServiceExceptionAssert extends AbstractThrowableAssert<ServiceExceptionAssert, ServiceException> {
@@ -58,15 +59,17 @@ public final ServiceExceptionAssert hasNoArgs() {
Map<String, Object> allArgs = new HashMap<>();
allArgs.putAll(actualArgs.safeArgs);
allArgs.putAll(actualArgs.unsafeArgs);
- failWithMessage("Expected no args, but found %s", allArgs);
+ failWithMessage(
+ "Expected no args, but found %s; service exception: %s", allArgs, Throwables.getStackTrace(actual));
}
return this;
}
- private void failIfNotEqual(String message, Object expected, Object actual) {
- if (!Objects.equals(expected, actual)) {
- failWithMessage(message, expected, actual);
+ private <T> void failIfNotEqual(String message, T expectedValue, T actualValue) {
+ if (!Objects.equals(expectedValue, actualValue)) {
+ failWithMessage(
+ message + "; service exception: ", expectedValue, actualValue, Throwables.getStackTrace(actual));
}
}
@@ -87,7 +90,8 @@ public final ServiceExceptionAssert containsArgs(Arg<?>... args) {
private void failIfDoesNotContain(
String message, Map<String, Object> expectedArgs, Map<String, Object> actualArgs) {
if (!actualArgs.entrySet().containsAll(expectedArgs.entrySet())) {
- failWithMessage(message, expectedArgs, actualArgs);
+ failWithMessage(
+ message + "; service exception: %s", expectedArgs, actualArgs, Throwables.getStackTrace(actual));
}
}
diff --git a/test-utils/src/test/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssertTest.java b/test-utils/src/test/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssertTest.java
index 7e8f4cefe..c078e0f73 100644
--- a/test-utils/src/test/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssertTest.java
+++ b/test-utils/src/test/java/com/palantir/conjure/java/api/testing/RemoteExceptionAssertTest.java
@@ -37,8 +37,12 @@ public void testSanity() {
assertThatThrownBy(() -> Assertions.assertThat(new RemoteException(error, actualType.httpErrorCode() + 1))
.isGeneratedFromErrorType(actualType))
.isInstanceOf(AssertionError.class)
- .hasMessage(
+ .hasMessageContaining(
"Expected error status to be %s, but found %s",
- actualType.httpErrorCode(), actualType.httpErrorCode() + 1);
+ actualType.httpErrorCode(), actualType.httpErrorCode() + 1)
+ // Make sure the error type was captured.
+ .hasMessageContaining("FAILED_PRECONDITION")
+ // Make sure the instance ID was captured.
+ .hasMessageContaining("with instance ID");
}
}
diff --git a/test-utils/src/test/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssertTest.java b/test-utils/src/test/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssertTest.java
index 749532ab4..2317967e7 100644
--- a/test-utils/src/test/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssertTest.java
+++ b/test-utils/src/test/java/com/palantir/conjure/java/api/testing/ServiceExceptionAssertTest.java
@@ -43,28 +43,28 @@ public void testSanity() {
assertThatThrownBy(() ->
Assertions.assertThat(new ServiceException(actualType)).hasType(ErrorType.INTERNAL))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected ErrorType to be %s, but found %s", ErrorType.INTERNAL, actualType);
+ .hasMessageContaining("Expected ErrorType to be %s, but found %s", ErrorType.INTERNAL, actualType);
assertThatThrownBy(() -> Assertions.assertThat(new ServiceException(actualType, SafeArg.of("a", "b")))
.hasArgs(SafeArg.of("c", "d")))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected safe args to be {c=d}, but found {a=b}");
+ .hasMessageContaining("Expected safe args to be {c=d}, but found {a=b}");
assertThatThrownBy(() -> Assertions.assertThat(new ServiceException(actualType, UnsafeArg.of("a", "b")))
.hasArgs(UnsafeArg.of("c", "d")))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected unsafe args to be {c=d}, but found {a=b}");
+ .hasMessageContaining("Expected unsafe args to be {c=d}, but found {a=b}");
assertThatThrownBy(() -> Assertions.assertThat(new ServiceException(actualType, SafeArg.of("a", "b")))
.hasNoArgs())
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected no args, but found {a=b}");
+ .hasMessageContaining("Expected no args, but found {a=b}");
assertThatThrownBy(() -> Assertions.assertThat(
new ServiceException(actualType, SafeArg.of("a", "b"), UnsafeArg.of("c", "d")))
.hasNoArgs())
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected no args, but found {a=b, c=d}");
+ .hasMessageContaining("Expected no args, but found {a=b, c=d}");
Assertions.assertThat(new ServiceException(actualType, UnsafeArg.of("a", "b"), UnsafeArg.of("c", "d")))
.containsArgs(UnsafeArg.of("a", "b"));
@@ -74,16 +74,16 @@ public void testSanity() {
new ServiceException(actualType, SafeArg.of("a", "b"), UnsafeArg.of("c", "d")))
.containsArgs(UnsafeArg.of("a", "b")))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected unsafe args to contain {a=b}, but found {c=d}");
+ .hasMessageContaining("Expected unsafe args to contain {a=b}, but found {c=d}");
assertThatThrownBy(() -> Assertions.assertThat(new ServiceException(actualType, SafeArg.of("a", "b")))
.containsArgs(SafeArg.of("c", "d")))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected safe args to contain {c=d}, but found {a=b}");
+ .hasMessageContaining("Expected safe args to contain {c=d}, but found {a=b}");
assertThatThrownBy(() -> Assertions.assertThat(new ServiceException(actualType, UnsafeArg.of("a", "b")))
.containsArgs(UnsafeArg.of("c", "d")))
.isInstanceOf(AssertionError.class)
- .hasMessage("Expected unsafe args to contain {c=d}, but found {a=b}");
+ .hasMessageContaining("Expected unsafe args to contain {c=d}, but found {a=b}");
}
}