From 2a7c5a6e205e29d42e9886657987893cb03b6875 Mon Sep 17 00:00:00 2001 From: Stefano Cordio Date: Tue, 2 Jan 2024 23:46:42 +0100 Subject: [PATCH] Revert "Provide value when assertThatThrownBy/thenThrownBy fail" (#3318) This reverts commit cf06398587da23ac530b03a941e0efe6cf29aef7 and adds a new test to prevent further regression. --- .../java/org/assertj/core/api/Assertions.java | 43 ---------------- .../org/assertj/core/api/BDDAssertions.java | 43 ---------------- .../org/assertj/core/api/ThrowableAssert.java | 4 -- .../Assertions_assertThatThrownBy_Test.java | 49 ++++++++++++------- ...sert_built_from_ThrowingCallable_Test.java | 11 +---- ...bleAssert_built_with_then_method_Test.java | 9 +--- 6 files changed, 34 insertions(+), 125 deletions(-) diff --git a/assertj-core/src/main/java/org/assertj/core/api/Assertions.java b/assertj-core/src/main/java/org/assertj/core/api/Assertions.java index 4668ddc70e9..9b0bcff165f 100644 --- a/assertj-core/src/main/java/org/assertj/core/api/Assertions.java +++ b/assertj-core/src/main/java/org/assertj/core/api/Assertions.java @@ -12,7 +12,6 @@ */ package org.assertj.core.api; -import static java.lang.String.format; import static org.assertj.core.configuration.ConfigurationProvider.CONFIGURATION_PROVIDER; import static org.assertj.core.data.Percentage.withPercentage; @@ -76,7 +75,6 @@ import java.util.stream.Stream; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; -import org.assertj.core.api.ThrowableAssert.ThrowingCallableWithValue; import org.assertj.core.api.filter.FilterOperator; import org.assertj.core.api.filter.Filters; import org.assertj.core.api.filter.InFilter; @@ -98,7 +96,6 @@ import org.assertj.core.description.Description; import org.assertj.core.groups.Properties; import org.assertj.core.groups.Tuple; -import org.assertj.core.internal.Failures; import org.assertj.core.presentation.BinaryRepresentation; import org.assertj.core.presentation.HexadecimalRepresentation; import org.assertj.core.presentation.Representation; @@ -1212,26 +1209,6 @@ public static AbstractThrowableAssert assertThat(T a return assertThat(catchThrowable(shouldRaiseThrowable)).hasBeenThrown(); } - /** - * Similar to {@link #assertThatThrownBy(ThrowingCallable)}, but when the called code returns a value instead of - * throwing, the assertion error shows the returned value to help understand what went wrong. - * - * @param shouldRaiseThrowable The {@link ThrowingCallableWithValue} or lambda with the code that should raise the throwable. - * @return the created {@link ThrowableAssert}. - * @since 3.25.0 - */ - @CanIgnoreReturnValue - public static AbstractThrowableAssert assertThatThrownBy( - ThrowingCallableWithValue shouldRaiseThrowable) { - Object value; - try { - value = shouldRaiseThrowable.call(); - } catch (Throwable throwable) { - return assertThat(throwable); - } - throw Failures.instance().failure(format("Expecting code to raise a throwable, but it returned [%s] instead", value)); - } - /** * Allows to capture and then assert on a {@link Throwable} like {@code assertThatThrownBy(ThrowingCallable)} but this method * let you set the assertion description the same way you do with {@link AbstractAssert#as(String, Object...) as(String, Object...)}. @@ -1270,26 +1247,6 @@ public static AbstractThrowableAssert assertThat(T a return assertThat(catchThrowable(shouldRaiseThrowable)).as(description, args).hasBeenThrown(); } - /** - * Similar to {@link #assertThatThrownBy(ThrowingCallable, String, Object...)}, but when the called code returns a value - * instead of throwing, the assertion error shows the returned value to help understand what went wrong. - * - * @param shouldRaiseThrowable The {@link ThrowingCallableWithValue} or lambda with the code that should raise the throwable. - * @return the created {@link ThrowableAssert}. - * @since 3.25.0 - */ - @CanIgnoreReturnValue - public static AbstractThrowableAssert assertThatThrownBy(ThrowingCallableWithValue shouldRaiseThrowable, - String description, Object... args) { - Object value; - try { - value = shouldRaiseThrowable.call(); - } catch (Throwable throwable) { - return assertThat(throwable).as(description, args); - } - throw Failures.instance().failure(format("Expecting code to raise a throwable, but it returned [%s] instead", value)); - } - /** * Allows to capture and then assert on a {@link Throwable} (easier done with lambdas). *

diff --git a/assertj-core/src/main/java/org/assertj/core/api/BDDAssertions.java b/assertj-core/src/main/java/org/assertj/core/api/BDDAssertions.java index f14b202fdf0..ee84813bf97 100644 --- a/assertj-core/src/main/java/org/assertj/core/api/BDDAssertions.java +++ b/assertj-core/src/main/java/org/assertj/core/api/BDDAssertions.java @@ -12,8 +12,6 @@ */ package org.assertj.core.api; -import static java.lang.String.format; - import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -74,7 +72,6 @@ import java.util.stream.Stream; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; -import org.assertj.core.api.ThrowableAssert.ThrowingCallableWithValue; import org.assertj.core.api.filter.FilterOperator; import org.assertj.core.api.filter.InFilter; import org.assertj.core.api.filter.NotFilter; @@ -92,7 +89,6 @@ import org.assertj.core.description.Description; import org.assertj.core.groups.Properties; import org.assertj.core.groups.Tuple; -import org.assertj.core.internal.Failures; import org.assertj.core.presentation.BinaryRepresentation; import org.assertj.core.presentation.HexadecimalRepresentation; import org.assertj.core.presentation.Representation; @@ -1334,25 +1330,6 @@ public static AbstractThrowableAssert then(T actual) return assertThat(catchThrowable(shouldRaiseThrowable)).hasBeenThrown(); } - /** - * Similar to {@link #thenThrownBy(ThrowingCallable)}, but when the called code returns a value instead of - * throwing, the assertion error shows the returned value to help understand what went wrong. - * - * @param shouldRaiseThrowable The {@link ThrowingCallableWithValue} or lambda with the code that should raise the throwable. - * @return the created {@link ThrowableAssert}. - * @since 3.25.0 - */ - @CanIgnoreReturnValue - public static AbstractThrowableAssert thenThrownBy(ThrowingCallableWithValue shouldRaiseThrowable) { - Object value; - try { - value = shouldRaiseThrowable.call(); - } catch (Throwable throwable) { - return assertThat(throwable); - } - throw Failures.instance().failure(format("Expecting code to raise a throwable, but it returned [%s] instead", value)); - } - /** * Allows to capture and then assert on a {@link Throwable} like {@code thenThrownBy(ThrowingCallable)} but this method * let you set the assertion description the same way you do with {@link AbstractAssert#as(String, Object...) as(String, Object...)}. @@ -1390,26 +1367,6 @@ public static AbstractThrowableAssert then(T actual) return assertThat(catchThrowable(shouldRaiseThrowable)).as(description, args).hasBeenThrown(); } - /** - * Similar to {@link #thenThrownBy(ThrowingCallable, String, Object...)}, but when the called code returns a value instead of - * throwing, the assertion error shows the returned value to help understand what went wrong. - * - * @param shouldRaiseThrowable The {@link ThrowingCallableWithValue} or lambda with the code that should raise the throwable. - * @return the created {@link ThrowableAssert}. - * @since 3.25.0 - */ - @CanIgnoreReturnValue - public static AbstractThrowableAssert thenThrownBy(ThrowingCallableWithValue shouldRaiseThrowable, - String description, Object... args) { - Object value; - try { - value = shouldRaiseThrowable.call(); - } catch (Throwable throwable) { - return assertThat(throwable).as(description, args); - } - throw Failures.instance().failure(format("Expecting code to raise a throwable, but it returned [%s] instead", value)); - } - /** * Allows to capture and then assert on a {@link Throwable} more easily when used with Java 8 lambdas. * diff --git a/assertj-core/src/main/java/org/assertj/core/api/ThrowableAssert.java b/assertj-core/src/main/java/org/assertj/core/api/ThrowableAssert.java index 0aee0bae1d1..9d6b9057f34 100644 --- a/assertj-core/src/main/java/org/assertj/core/api/ThrowableAssert.java +++ b/assertj-core/src/main/java/org/assertj/core/api/ThrowableAssert.java @@ -33,10 +33,6 @@ public interface ThrowingCallable { void call() throws Throwable; } - public interface ThrowingCallableWithValue { - Object call() throws Throwable; - } - public ThrowableAssert(ACTUAL actual) { super(actual, ThrowableAssert.class); } diff --git a/assertj-core/src/test/java/org/assertj/core/api/Assertions_assertThatThrownBy_Test.java b/assertj-core/src/test/java/org/assertj/core/api/Assertions_assertThatThrownBy_Test.java index e7d713dc28a..5c804c00d0e 100644 --- a/assertj-core/src/test/java/org/assertj/core/api/Assertions_assertThatThrownBy_Test.java +++ b/assertj-core/src/test/java/org/assertj/core/api/Assertions_assertThatThrownBy_Test.java @@ -14,9 +14,8 @@ import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.Assertions_catchThrowableOfType_Test.raisingException; -import static org.assertj.core.api.Assertions_catchThrowable_Test.codeThrowing; import static org.assertj.core.api.BDDAssertions.then; +import static org.assertj.core.error.ShouldHaveMessage.shouldHaveMessage; import static org.assertj.core.util.AssertionsUtil.expectAssertionError; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; @@ -25,47 +24,63 @@ class Assertions_assertThatThrownBy_Test { @Test - void should_build_ThrowableAssert_with_runtime_exception_thrown() { + void should_work_with_runtime_exception_thrown() { + // WHEN/THEN assertThatThrownBy(codeThrowing(new IllegalArgumentException("boom"))).isInstanceOf(IllegalArgumentException.class) .hasMessage("boom"); } @Test - void should_build_ThrowableAssert_with_throwable_thrown() { + void should_work_with_throwable_thrown() { + // WHEN/THEN assertThatThrownBy(codeThrowing(new Throwable("boom"))).isInstanceOf(Throwable.class) .hasMessage("boom"); } @Test - void should_be_able_to_pass_a_description_to_assertThatThrownBy() { + void should_work_with_method_reference_having_vararg_parameter() { + // WHEN/THEN + assertThatThrownBy(Assertions_assertThatThrownBy_Test::methodThrowing).isInstanceOf(Exception.class) + .hasMessage("boom"); + } + + @Test + void should_support_description() { // GIVEN - // make assertThatThrownBy fail to verify the description afterwards - ThrowingCallable code = () -> assertThatThrownBy(raisingException("boom"), "Test %s", "code").hasMessage("bam"); + Throwable throwable = new Exception("boom"); // WHEN - AssertionError assertionError = expectAssertionError(code); + AssertionError assertionError = expectAssertionError(() -> assertThatThrownBy(codeThrowing(throwable), "Test %s", + "code").hasMessage("bam")); // THEN then(assertionError).hasMessageContaining("[Test code]"); } @Test void should_fail_if_no_throwable_was_thrown() { - // GIVEN - ThrowingCallable code = () -> {}; // WHEN - AssertionError assertionError = expectAssertionError(() -> assertThatThrownBy(code).hasMessage("boom ?")); + AssertionError assertionError = expectAssertionError(() -> assertThatThrownBy(() -> {}).hasMessage("boom ?")); // THEN then(assertionError).hasMessage(format("%nExpecting code to raise a throwable.")); } @Test - void should_fail_with_good_message_when_assertion_is_failing() { + void should_fail_with_proper_message_when_assertion_is_failing() { + // GIVEN + Throwable throwable = new Exception("boom"); // WHEN - AssertionError assertionError = expectAssertionError(() -> assertThatThrownBy(raisingException("boom")).hasMessage("bam")); + AssertionError assertionError = expectAssertionError(() -> assertThatThrownBy(codeThrowing(throwable)).hasMessage("bam")); // THEN - then(assertionError).hasMessageContainingAll("Expecting message to be:", - "\"bam\"", - "but was:", - "\"boom\""); + then(assertionError).hasMessage(shouldHaveMessage(throwable, "bam").create()); + } + + private static ThrowingCallable codeThrowing(Throwable t) { + return () -> { + throw t; + }; + } + + private static void methodThrowing(Object... parameters) throws Exception { + throw new Exception("boom"); } } diff --git a/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_from_ThrowingCallable_Test.java b/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_from_ThrowingCallable_Test.java index 30456928fa9..c6a1d8e6bc0 100644 --- a/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_from_ThrowingCallable_Test.java +++ b/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_from_ThrowingCallable_Test.java @@ -12,10 +12,8 @@ */ package org.assertj.core.api.throwable; -import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.util.AssertionsUtil.assertThatAssertionErrorIsThrownBy; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; @@ -52,14 +50,7 @@ public void call() { // no exception } }); - }).withMessage(format("%nExpecting code to raise a throwable.")); + }).withMessage(String.format("%nExpecting code to raise a throwable.")); } - @Test - void should_fail_and_show_value_returned_by_callable_code() { - // GIVEN - ThrowingCallable code = () -> assertThatThrownBy(() -> 42); - // WHEN/THEN - assertThatAssertionErrorIsThrownBy(code).withMessage("Expecting code to raise a throwable, but it returned [42] instead"); - } } diff --git a/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_with_then_method_Test.java b/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_with_then_method_Test.java index e52334a7fe0..b19958ef2f9 100644 --- a/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_with_then_method_Test.java +++ b/assertj-core/src/test/java/org/assertj/core/api/throwable/ThrowableAssert_built_with_then_method_Test.java @@ -14,11 +14,11 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.BDDAssertions.thenThrownBy; -import static org.assertj.core.util.AssertionsUtil.assertThatAssertionErrorIsThrownBy; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; +// TODO build two throwable assert with then and assertThat and compare them. class ThrowableAssert_built_with_then_method_Test { @Test @@ -54,11 +54,4 @@ public void call() { }).withMessage(String.format("%nExpecting code to raise a throwable.")); } - @Test - void should_fail_if_value_is_returned_by_callable_code() { - // GIVEN - ThrowingCallable code = () -> thenThrownBy(() -> 42); - // WHEN/THEN - assertThatAssertionErrorIsThrownBy(code).withMessage("Expecting code to raise a throwable, but it returned [42] instead"); - } }