From 512b1dee56164fd4c0f51dd25bc96eeb0905ab39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Wed, 6 Jul 2022 12:32:48 +0200 Subject: [PATCH] Improved keys assertions in ObservationContextAssert (#3266) The first part of this change adds two assertions around keys (considering both low cardinality and high cardinality KeyValues). It is currently possible to individually assert specific keys are present, either focusing on the key itself being there or specifying the expected key and value pair. For example `hasLowCardinalityKeyValueWithKey("k")` or `hasHighCardinalityKeyValue("k", "v")`). But it isn't possible to quickly also make sure that no other keys are present outside of the ones asserted via these methods. Let's imagine my `actual` has 3 keys "A", "B" and "C" but I only expect "A" and "B": without having prior knowledge of "C", its absence cannot be asserted. Adding `hasKeyValuesCount` is a good complement for this: in the example having `hasKeyValuesCount(2)` would fail with a message indicating 3 keys are present instead of 2. Adding `hasOnlyKeys` could also serve the same purpose if the only concern is having the keys, whatever the values are. As such `hasOnlyKeys("A", "B")` in the example would fail with a message saying that extraneous key "C" was found. `hasOnlyKeys("A", "B", "C", "D")` would also fail with a message stating that key "D" is missing. The second part of this change fixes a counterintuitive behaviour of the `doesNotHave[High|Low]LevelKeyValue(String, String)` assertions: if the key is present with another value than the one specified, the assertion currently fails by complaining the key is present. But if both a key _and a value_ are specified then only that specific pair should be undesirable. So for that same key we should accept values that do not match the one specified. (for a similar example in AssertJ core, see `Map`'s `containsEntry` assertion). --- .../tck/ObservationContextAssert.java | 48 +++++++++- .../tck/ObservationContextAssertTests.java | 93 +++++++++++++++++++ 2 files changed, 136 insertions(+), 5 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 8e1a2efcf0..87a6b88c9b 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 @@ -21,9 +21,7 @@ import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.AbstractThrowableAssert; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import java.util.stream.Collectors; /** @@ -153,6 +151,48 @@ public SELF hasAnyKeyValues() { return (SELF) this; } + 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); + } + return (SELF) this; + } + + private List allKeys() { + List result = lowCardinalityKeys(); + result.addAll(highCardinalityKeys()); + return result; + } + + public SELF hasOnlyKeys(String... keys) { + isNotNull(); + Set actualKeys = new LinkedHashSet<>(allKeys()); + List expectedKeys = Arrays.asList(keys); + boolean sameContent = actualKeys.containsAll(expectedKeys) && actualKeys.size() == expectedKeys.size(); + + if (!sameContent) { + Set extraKeys = new LinkedHashSet<>(actualKeys); + extraKeys.removeAll(expectedKeys); + + Set missingKeys = new LinkedHashSet<>(expectedKeys); + missingKeys.removeAll(actualKeys); + + if (!extraKeys.isEmpty() && !missingKeys.isEmpty()) { + failWithMessage("Observation has unexpected keys %s and misses expected keys %s.", extraKeys, + missingKeys); + } + else if (!extraKeys.isEmpty()) { + failWithMessage("Observation has unexpected keys %s.", extraKeys); + } + else { + failWithMessage("Observation is missing expected keys %s.", missingKeys); + } + } + return (SELF) this; + } + private List lowCardinalityKeys() { return this.actual.getLowCardinalityKeyValues().stream().map(KeyValue::getKey).collect(Collectors.toList()); } @@ -194,7 +234,6 @@ public SELF doesNotHaveLowCardinalityKeyValueWithKey(String key) { public SELF doesNotHaveLowCardinalityKeyValue(String key, String value) { isNotNull(); - doesNotHaveLowCardinalityKeyValueWithKey(key); Optional optional = this.actual.getLowCardinalityKeyValues().stream() .filter(tag -> tag.getKey().equals(key)).findFirst(); if (!optional.isPresent()) { @@ -241,7 +280,6 @@ public SELF doesNotHaveHighCardinalityKeyValueWithKey(String key) { public SELF doesNotHaveHighCardinalityKeyValue(String key, String value) { isNotNull(); - doesNotHaveHighCardinalityKeyValueWithKey(key); Optional optional = this.actual.getHighCardinalityKeyValues().stream() .filter(tag -> tag.getKey().equals(key)).findFirst(); if (!optional.isPresent()) { 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 a892413836..0c0d69ab9a 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 @@ -145,6 +145,75 @@ void should_throw_exception_when_contextual_name_ignore_case_correct() { .isInstanceOf(AssertionError.class); } + @Test + void should_not_throw_exception_when_key_count_matches() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("low", "foo"); + observation.highCardinalityKeyValue("high", "bar"); + + thenNoException().isThrownBy(() -> assertThat(context).hasKeyValuesCount(2)); + } + + @Test + void should_throw_exception_when_key_count_differs() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("low", "foo"); + observation.highCardinalityKeyValue("high", "bar"); + + thenThrownBy(() -> assertThat(context).hasKeyValuesCount(1)).isInstanceOf(AssertionError.class) + .hasMessage("Observation expected to have <1> keys but has <2>."); + + thenThrownBy(() -> assertThat(context).hasKeyValuesCount(3)).isInstanceOf(AssertionError.class) + .hasMessage("Observation expected to have <3> keys but has <2>."); + } + + @Test + void should_not_throw_exception_when_keys_match() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("low", "foo"); + observation.highCardinalityKeyValue("high", "bar"); + + thenNoException().isThrownBy(() -> ObservationContextAssert.assertThat(context).hasOnlyKeys("low", "high")); + } + + @Test + void should_throw_exception_when_keys_missing() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("found", "foo"); + + thenThrownBy(() -> ObservationContextAssert.assertThat(context).hasOnlyKeys("found", "low", "high")) + .isInstanceOf(AssertionError.class).hasMessage("Observation is missing expected keys [low, high]."); + } + + @Test + void should_throw_exception_when_keys_extras() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("found", "foo"); + observation.lowCardinalityKeyValue("low", "foo"); + observation.highCardinalityKeyValue("high", "foo"); + + thenThrownBy(() -> ObservationContextAssert.assertThat(context).hasOnlyKeys("found")) + .isInstanceOf(AssertionError.class).hasMessage("Observation has unexpected keys [low, high]."); + } + + @Test + void should_throw_exception_when_keys_both_missing_and_extras() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("found", "foo"); + observation.lowCardinalityKeyValue("low", "foo"); + observation.highCardinalityKeyValue("high", "foo"); + + thenThrownBy(() -> ObservationContextAssert.assertThat(context).hasOnlyKeys("notfound", "found")) + .isInstanceOf(AssertionError.class) + .hasMessage("Observation has unexpected keys [low, high] and misses expected keys [notfound]."); + } + @Test void should_not_throw_exception_when_low_cardinality_tag_exists() { registry.observationConfig().observationHandler(c -> true); @@ -202,11 +271,35 @@ void should_throw_exception_when_high_cardinality_tag_present() { .isInstanceOf(AssertionError.class); } + @Test + void should_not_throw_exception_when_high_cardinality_tag_present_with_other_value() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.highCardinalityKeyValue("foo", "other"); + + thenNoException().isThrownBy(() -> assertThat(context).doesNotHaveHighCardinalityKeyValue("foo", "bar")); + + thenThrownBy(() -> assertThat(context).doesNotHaveHighCardinalityKeyValueWithKey("foo")) + .isInstanceOf(AssertionError.class); + } + @Test void should_not_throw_exception_when_low_cardinality_tag_missing() { thenNoException().isThrownBy(() -> assertThat(context).doesNotHaveLowCardinalityKeyValue("foo", "bar")); } + @Test + void should_not_throw_exception_when_low_cardinality_tag_present_with_other_value() { + registry.observationConfig().observationHandler(c -> true); + Observation observation = Observation.start("foo", context, registry); + observation.lowCardinalityKeyValue("foo", "other"); + + thenNoException().isThrownBy(() -> assertThat(context).doesNotHaveLowCardinalityKeyValue("foo", "bar")); + + thenThrownBy(() -> assertThat(context).doesNotHaveLowCardinalityKeyValueWithKey("foo")) + .isInstanceOf(AssertionError.class); + } + @Test void should_throw_exception_when_low_cardinality_tag_present() { registry.observationConfig().observationHandler(c -> true);