From 850012b3aade2865f8311345917a703a79f88bbd Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Thu, 21 Nov 2024 13:07:05 +0900 Subject: [PATCH] Use failWithActualExpectedAndMessage() where possible See gh-5550 --- .../tck/ObservationContextAssert.java | 6 ++-- .../tck/ObservationRegistryAssert.java | 17 +++++++--- .../tck/TestObservationRegistryAssert.java | 24 +++++++------- .../tck/ObservationContextAssertTests.java | 18 +++++++++-- .../tck/ObservationRegistryAssertTests.java | 23 +++++++++++-- .../TestObservationRegistryAssertTests.java | 32 +++++++++++++++---- 6 files changed, 89 insertions(+), 31 deletions(-) diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java index 2d491416f8..aa8a78e7b8 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java +++ b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationContextAssert.java @@ -166,7 +166,8 @@ public SELF hasKeyValuesCount(int size) { isNotNull(); long actualSize = this.actual.getAllKeyValues().stream().count(); if (actualSize != size) { - failWithMessage("Observation expected to have <%s> keys but has <%s>.", size, actualSize); + failWithActualExpectedAndMessage(actualSize, size, "Observation expected to have <%s> keys but has <%s>.", + size, actualSize); } return (SELF) this; } @@ -388,7 +389,8 @@ public SELF hasMapEntry(Object key, Object value) { isNotNull(); Object mapValue = this.actual.get(key); if (!Objects.equals(mapValue, value)) { - failWithMessage("Observation should have an entry for key <%s> with value <%s>. Value was <%s>", key, value, + failWithActualExpectedAndMessage(mapValue, value, + "Observation should have an entry for key <%s> with value <%s>. Value was <%s>", key, value, mapValue); } return (SELF) this; diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryAssert.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryAssert.java index 38c10c0da3..752c23109a 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryAssert.java +++ b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationRegistryAssert.java @@ -116,8 +116,14 @@ public SELF doesNotHaveRemainingCurrentObservationSameAs(Observation observation public SELF hasRemainingCurrentObservationSameAs(Observation observation) { isNotNull(); Observation current = actual.getCurrentObservation(); + if (current == null) { + failWithMessage( + "Expected current observation in the registry to be same as <%s> but there was no current observation", + observation); + } if (current != observation) { - failWithMessage("Expected current observation in the registry to be same as <%s> but was <%s>", observation, + failWithActualExpectedAndMessage(current, observation, + "Expected current observation in the registry to be same as <%s> but was <%s>", observation, current); } return (SELF) this; @@ -183,16 +189,17 @@ public SELF doesNotHaveRemainingCurrentScopeSameAs(Observation.Scope scope) { public SELF hasRemainingCurrentScopeSameAs(Observation.Scope scope) { isNotNull(); Observation.Scope current = actual.getCurrentObservationScope(); + String expectedContextName = scope.getCurrentObservation().getContext().getName(); if (current == null) { failWithMessage( "Expected current Scope in the registry to be same as a provided Scope tied to observation named <%s> but there was no current scope", - scope.getCurrentObservation().getContext().getName()); + expectedContextName); } if (current != scope) { - failWithMessage( + String actualContextName = current.getCurrentObservation().getContext().getName(); + failWithActualExpectedAndMessage(actualContextName, expectedContextName, "Expected current Scope in the registry to be same as a provided Scope tied to observation named <%s> but was a different one (tied to observation named <%s>)", - scope.getCurrentObservation().getContext().getName(), - current.getCurrentObservation().getContext().getName()); + expectedContextName, actualContextName); } return (SELF) this; } diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistryAssert.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistryAssert.java index 3d752394d4..481e248799 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistryAssert.java +++ b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistryAssert.java @@ -240,9 +240,11 @@ public TestObservationRegistryAssert forAllObservationsWithNameEqualToIgnoreCase */ public TestObservationRegistryAssert hasNumberOfObservationsEqualTo(int expectedNumberOfObservations) { isNotNull(); - if (this.actual.getContexts().size() != expectedNumberOfObservations) { - failWithMessage("There should be <%s> Observations but there were <%s>. Found following Observations:\n%s", - expectedNumberOfObservations, this.actual.getContexts().size(), + int actualNumberOfObservations = this.actual.getContexts().size(); + if (actualNumberOfObservations != expectedNumberOfObservations) { + failWithActualExpectedAndMessage(actualNumberOfObservations, expectedNumberOfObservations, + "There should be <%s> Observations but there were <%s>. Found following Observations:\n%s", + expectedNumberOfObservations, actualNumberOfObservations, observationNames(this.actual.getContexts())); } return this; @@ -268,14 +270,14 @@ public TestObservationRegistryAssert hasNumberOfObservationsEqualTo(int expected public TestObservationRegistryAssert hasNumberOfObservationsWithNameEqualTo(String observationName, int expectedNumberOfObservations) { isNotNull(); - long observationsWithNameSize = this.actual.getContexts() + long actualNumberOfObservations = this.actual.getContexts() .stream() .filter(f -> observationName.equals(f.getContext().getName())) .count(); - if (observationsWithNameSize != expectedNumberOfObservations) { - failWithMessage( + if (actualNumberOfObservations != expectedNumberOfObservations) { + failWithActualExpectedAndMessage(actualNumberOfObservations, expectedNumberOfObservations, "There should be <%s> Observations with name <%s> but there were <%s>. Found following Observations:\n%s", - expectedNumberOfObservations, observationName, observationsWithNameSize, + expectedNumberOfObservations, observationName, actualNumberOfObservations, observationNames(this.actual.getContexts())); } return this; @@ -302,14 +304,14 @@ public TestObservationRegistryAssert hasNumberOfObservationsWithNameEqualTo(Stri public TestObservationRegistryAssert hasNumberOfObservationsWithNameEqualToIgnoreCase(String observationName, int expectedNumberOfObservations) { isNotNull(); - long observationsWithNameSize = this.actual.getContexts() + long actualNumberOfObservations = this.actual.getContexts() .stream() .filter(f -> observationName.equalsIgnoreCase(f.getContext().getName())) .count(); - if (observationsWithNameSize != expectedNumberOfObservations) { - failWithMessage( + if (actualNumberOfObservations != expectedNumberOfObservations) { + failWithActualExpectedAndMessage(actualNumberOfObservations, expectedNumberOfObservations, "There should be <%s> Observations with name (ignoring case) <%s> but there were <%s>. Found following Observations:\n%s", - expectedNumberOfObservations, observationName, observationsWithNameSize, + expectedNumberOfObservations, observationName, actualNumberOfObservations, observationNames(this.actual.getContexts())); } return this; diff --git a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationContextAssertTests.java b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationContextAssertTests.java index 84c3577a3e..225afc26b5 100644 --- a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationContextAssertTests.java +++ b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationContextAssertTests.java @@ -201,10 +201,18 @@ void should_throw_exception_when_key_count_differs() { observation.highCardinalityKeyValue("high", "bar"); thenThrownBy(() -> assertThat(context).hasKeyValuesCount(1)).isInstanceOf(AssertionError.class) - .hasMessage("Observation expected to have <1> keys but has <2>."); + .hasMessage("Observation expected to have <1> keys but has <2>.") + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("2"); + then(error.getExpected().getStringRepresentation()).isEqualTo("1"); + }); thenThrownBy(() -> assertThat(context).hasKeyValuesCount(3)).isInstanceOf(AssertionError.class) - .hasMessage("Observation expected to have <3> keys but has <2>."); + .hasMessage("Observation expected to have <3> keys but has <2>.") + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("2"); + then(error.getExpected().getStringRepresentation()).isEqualTo("3"); + }); } @Test @@ -414,7 +422,11 @@ void should_throw_exception_when_tags_present() { void should_throw_exception_when_map_entry_missing() { context.put("foo", "bar"); - thenThrownBy(() -> assertThat(context).hasMapEntry("foo", "baz")).isInstanceOf(AssertionError.class); + thenThrownBy(() -> assertThat(context).hasMapEntry("foo", "baz")) + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("bar"); + then(error.getExpected().getStringRepresentation()).isEqualTo("baz"); + }); } @Test diff --git a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationRegistryAssertTests.java b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationRegistryAssertTests.java index 73d972c68a..42d246319f 100644 --- a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationRegistryAssertTests.java +++ b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationRegistryAssertTests.java @@ -19,8 +19,10 @@ import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.opentest4j.AssertionFailedError; import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.BDDAssertions.then; @SuppressWarnings("rawtypes") class ObservationRegistryAssertTests { @@ -76,7 +78,17 @@ void assertionErrorThrownWhenRemainingObservationNotSameAs() { assertThatThrownBy(() -> this.registryAssert.hasRemainingCurrentObservationSameAs(observation)) .isInstanceOf(AssertionError.class) - .hasMessageContaining("Expected current observation in the registry to be same as <{name=foo"); + .hasMessageContaining("but there was no current observation"); + + Observation anotherObservation = Observation.start("bar", this.registry); + try (Observation.Scope scope = anotherObservation.openScope()) { + assertThatThrownBy(() -> this.registryAssert.hasRemainingCurrentObservationSameAs(observation)) + .hasMessageContaining("Expected current observation in the registry to be same as <{name=foo") + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).contains("bar"); + then(error.getExpected().getStringRepresentation()).contains("foo"); + }); + } } @Test @@ -148,11 +160,16 @@ void failsHasRemainingCurrentScopeSameAs() { Observation.Scope scope1 = o1.openScope(); scope1.close(); try (Observation.Scope scope2 = o2.openScope()) { - assertThatExceptionOfType(AssertionError.class) + assertThatExceptionOfType(AssertionFailedError.class) .isThrownBy(() -> this.registryAssert.hasRemainingCurrentScopeSameAs(scope1)) .withMessage( "Expected current Scope in the registry to be same as a provided Scope tied to observation named " - + " but was a different one (tied to observation named )"); + + " but was a different one (tied to observation named )") + .satisfies(error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("new"); + then(error.getExpected().getStringRepresentation()).isEqualTo("old"); + }); + ; } } diff --git a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/TestObservationRegistryAssertTests.java b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/TestObservationRegistryAssertTests.java index f81b3e2951..6006d2011b 100644 --- a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/TestObservationRegistryAssertTests.java +++ b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/TestObservationRegistryAssertTests.java @@ -23,12 +23,13 @@ import org.assertj.core.api.BDDAssertions; import org.awaitility.Awaitility; import org.junit.jupiter.api.Test; +import org.opentest4j.AssertionFailedError; import java.time.Duration; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.BDDAssertions.thenNoException; -import static org.assertj.core.api.BDDAssertions.thenThrownBy; +import static org.assertj.core.api.BDDAssertions.*; +import static org.assertj.core.api.BDDAssertions.then; class TestObservationRegistryAssertTests { @@ -228,7 +229,12 @@ void should_not_fail_when_all_observations_match_the_assertion_ignore_case() { void should_fail_when_number_of_observations_does_not_match() { Observation.createNotStarted("FOO", registry).start().stop(); - thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsEqualTo(0)).isInstanceOf(AssertionError.class); + thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsEqualTo(0)) + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("1"); + then(error.getExpected().getStringRepresentation()).isEqualTo("0"); + }); + ; } @Test @@ -243,7 +249,10 @@ void should_fail_when_names_match_but_number_is_incorrect() { Observation.createNotStarted("foo", registry).start().stop(); thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsWithNameEqualTo("foo", 0)) - .isInstanceOf(AssertionError.class); + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("1"); + then(error.getExpected().getStringRepresentation()).isEqualTo("0"); + }); } @Test @@ -251,7 +260,10 @@ void should_fail_when_number_is_correct_but_names_do_not_match() { Observation.createNotStarted("foo", registry).start().stop(); thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsWithNameEqualTo("bar", 1)) - .isInstanceOf(AssertionError.class); + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("0"); + then(error.getExpected().getStringRepresentation()).isEqualTo("1"); + }); } @Test @@ -266,7 +278,10 @@ void should_fail_when_names_match_but_number_is_incorrect_ignore_case() { Observation.createNotStarted("FOO", registry).start().stop(); thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsWithNameEqualToIgnoreCase("foo", 0)) - .isInstanceOf(AssertionError.class); + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("1"); + then(error.getExpected().getStringRepresentation()).isEqualTo("0"); + }); } @Test @@ -274,7 +289,10 @@ void should_fail_when_number_is_correct_but_names_do_not_match_ignore_case() { Observation.createNotStarted("FOO", registry).start().stop(); thenThrownBy(() -> assertThat(registry).hasNumberOfObservationsWithNameEqualToIgnoreCase("bar", 1)) - .isInstanceOf(AssertionError.class); + .isInstanceOfSatisfying(AssertionFailedError.class, error -> { + then(error.getActual().getStringRepresentation()).isEqualTo("0"); + then(error.getExpected().getStringRepresentation()).isEqualTo("1"); + }); } @Test