Skip to content

Commit

Permalink
Make ServiceException and RemoteException assertions better (#1058)
Browse files Browse the repository at this point in the history
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)
```
  • Loading branch information
jdm2212 authored Nov 21, 2023
1 parent 63e1190 commit 5914e09
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand All @@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand Down Expand Up @@ -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));
}
}

Expand All @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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}");
}
}

0 comments on commit 5914e09

Please sign in to comment.