From 84bcb65dd15929bd16deee090be669b9d8249781 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= <stephane.nicoll@broadcom.com>
Date: Tue, 18 Jun 2024 06:23:13 +0200
Subject: [PATCH] Streamline assertions on exceptions

This commit simplifies assertions on MockMvc requests that have failed
to process. A now general hasFailed/doesNotHaveFailed/failure provides
the necessary to assert the exception.

Any attempt to access anything from the result with an unresolved
exception still fails as before.

Closes gh-33060
---
 .../web/servlet/assertj/MockMvcTester.java    | 18 +++++--
 .../servlet/assertj/MvcTestResultAssert.java  | 33 ++++++++-----
 .../MockMvcTesterIntegrationTests.java        | 49 ++++++++++++++-----
 3 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java
index 5dd258ee987..736219b8a27 100644
--- a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java
+++ b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MockMvcTester.java
@@ -89,12 +89,20 @@ import org.springframework.web.context.WebApplicationContext;
  * <p>One main difference between {@link MockMvc} and {@code MockMvcTester} is
  * that an unresolved exception is not thrown directly when using
  * {@code MockMvcTester}. Rather an {@link MvcTestResult} is available with an
- * {@linkplain MvcTestResult#getUnresolvedException() unresolved exception}
- * which allows you to assert that a request failed unexpectedly:
+ * {@linkplain MvcTestResult#getUnresolvedException() unresolved exception}.
+ * Both resolved and unresolved exceptions are considered a failure that can
+ * be asserted as follows:
  * <pre><code class="java">
- * // perform a GET on /boom and assert the message for the the unresolved exception
- * assertThat(mvc.get().uri("/boom")).hasUnresolvedException())
- *         .withMessage("Test exception");
+ * // perform a GET on /boom and assert the message for the the exception
+ * assertThat(mvc.get().uri("/boom")).hasFailed()
+ *         .failure().hasMessage("Test exception");
+ * </code></pre>
+ *
+ * <p>Any attempt to access the result with an unresolved exception will
+ * throw an {@link AssertionError}:
+ * <pre><code class="java">
+ * // throw an AssertionError with an unresolved exception
+ * assertThat(mvc.get().uri("/boom")).hasStatus5xxServerError();
  * </code></pre>
  *
  * <p>{@code MockMvcTester} can be configured with a list of
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java
index 810d4a8d2eb..20725cb1781 100644
--- a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java
+++ b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java
@@ -59,13 +59,13 @@ public class MvcTestResultAssert extends AbstractMockHttpServletResponseAssert<M
 	}
 
 	/**
-	 * Verify that the request failed with an unresolved exception, and
-	 * return a new {@linkplain AbstractThrowableAssert assertion} object
-	 * that uses the unresolved {@link Exception} as the object to test.
+	 * Verify that the request has failed and return a new
+	 * {@linkplain AbstractThrowableAssert assertion} object that uses the
+	 * failure as the object to test.
 	 */
-	public AbstractThrowableAssert<?, ? extends Throwable> unresolvedException() {
-		hasUnresolvedException();
-		return Assertions.assertThat(this.actual.getUnresolvedException());
+	public AbstractThrowableAssert<?, ? extends Throwable> failure() {
+		hasFailed();
+		return Assertions.assertThat(getFailure());
 	}
 
 	/**
@@ -137,20 +137,19 @@ public class MvcTestResultAssert extends AbstractMockHttpServletResponseAssert<M
 	}
 
 	/**
-	 * Verify that the request failed with an unresolved exception.
-	 * @see #unresolvedException()
+	 * Verify that the request has failed.
 	 */
-	public MvcTestResultAssert hasUnresolvedException() {
-		Assertions.assertThat(this.actual.getUnresolvedException())
+	public MvcTestResultAssert hasFailed() {
+		Assertions.assertThat(getFailure())
 				.withFailMessage("Expected request to fail, but it succeeded").isNotNull();
 		return this;
 	}
 
 	/**
-	 * Verify that the request did not fail with an unresolved exception.
+	 * Verify that the request has not failed.
 	 */
-	public MvcTestResultAssert doesNotHaveUnresolvedException() {
-		Assertions.assertThat(this.actual.getUnresolvedException())
+	public MvcTestResultAssert doesNotHaveFailed() {
+		Assertions.assertThat(getFailure())
 				.withFailMessage("Expected request to succeed, but it failed").isNull();
 		return this;
 	}
@@ -184,6 +183,14 @@ public class MvcTestResultAssert extends AbstractMockHttpServletResponseAssert<M
 		return this.myself;
 	}
 
+	@Nullable
+	private Throwable getFailure() {
+		Exception unresolvedException = this.actual.getUnresolvedException();
+		if (unresolvedException != null) {
+			return unresolvedException;
+		}
+		return this.actual.getMvcResult().getResolvedException();
+	}
 
 	@SuppressWarnings("NullAway")
 	private ModelAndView getModelAndView() {
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
index 75e3b600b8d..2eafba04ba5 100644
--- a/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
+++ b/spring-test/src/test/java/org/springframework/test/web/servlet/assertj/MockMvcTesterIntegrationTests.java
@@ -53,6 +53,7 @@ import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.bind.annotation.SessionAttributes;
 import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.servlet.DispatcherServlet;
 import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
@@ -278,40 +279,59 @@ public class MockMvcTesterIntegrationTests {
 	class ExceptionTests {
 
 		@Test
-		void doesNotHaveUnresolvedException() {
-			assertThat(mvc.get().uri("/greet")).doesNotHaveUnresolvedException();
+		void hasFailedWithUnresolvedException() {
+			assertThat(mvc.get().uri("/error/1")).hasFailed();
 		}
 
 		@Test
-		void hasUnresolvedException() {
-			assertThat(mvc.get().uri("/error/1")).hasUnresolvedException();
+		void hasFailedWithResolvedException() {
+			assertThat(mvc.get().uri("/error/2")).hasFailed().hasStatus(HttpStatus.PAYMENT_REQUIRED);
 		}
 
 		@Test
-		void doesNotHaveUnresolvedExceptionWithUnresolvedException() {
+		void doesNotHaveFailedWithoutException() {
+			assertThat(mvc.get().uri("/greet")).doesNotHaveFailed();
+		}
+
+		@Test
+		void doesNotHaveFailedWithUnresolvedException() {
 			assertThatExceptionOfType(AssertionError.class)
-					.isThrownBy(() -> assertThat(mvc.get().uri("/error/1")).doesNotHaveUnresolvedException())
+					.isThrownBy(() -> assertThat(mvc.get().uri("/error/1")).doesNotHaveFailed())
 					.withMessage("Expected request to succeed, but it failed");
 		}
 
 		@Test
-		void hasUnresolvedExceptionWithoutUnresolvedException() {
+		void doesNotHaveFailedWithResolvedException() {
 			assertThatExceptionOfType(AssertionError.class)
-					.isThrownBy(() -> assertThat(mvc.get().uri("/greet")).hasUnresolvedException())
+					.isThrownBy(() -> assertThat(mvc.get().uri("/error/2")).doesNotHaveFailed())
+					.withMessage("Expected request to succeed, but it failed");
+		}
+
+		@Test
+		void hasFailedWithoutException() {
+			assertThatExceptionOfType(AssertionError.class)
+					.isThrownBy(() -> assertThat(mvc.get().uri("/greet")).hasFailed())
 					.withMessage("Expected request to fail, but it succeeded");
 		}
 
 		@Test
-		void unresolvedExceptionWithFailedRequest() {
-			assertThat(mvc.get().uri("/error/1")).unresolvedException()
+		void failureWithUnresolvedException() {
+			assertThat(mvc.get().uri("/error/1")).failure()
 					.isInstanceOf(ServletException.class)
 					.cause().isInstanceOf(IllegalStateException.class).hasMessage("Expected");
 		}
 
 		@Test
-		void unresolvedExceptionWithSuccessfulRequest() {
+		void failureWithResolvedException() {
+			assertThat(mvc.get().uri("/error/2")).failure()
+					.isInstanceOfSatisfying(ResponseStatusException.class, ex ->
+							assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.PAYMENT_REQUIRED));
+		}
+
+		@Test
+		void failureWithoutException() {
 			assertThatExceptionOfType(AssertionError.class)
-					.isThrownBy(() -> assertThat(mvc.get().uri("/greet")).unresolvedException())
+					.isThrownBy(() -> assertThat(mvc.get().uri("/greet")).failure())
 					.withMessage("Expected request to fail, but it succeeded");
 		}
 
@@ -524,6 +544,11 @@ public class MockMvcTesterIntegrationTests {
 			throw new IllegalStateException("Expected");
 		}
 
+		@GetMapping("/error/2")
+		public String two() {
+			throw new ResponseStatusException(HttpStatus.PAYMENT_REQUIRED);
+		}
+
 		@GetMapping("/error/validation/{id}")
 		public String validation(@PathVariable @Size(max = 4) String id) {
 			return "Hello " + id;