Skip to content

Commit

Permalink
Improved keys assertions in ObservationContextAssert (#3266)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
simonbasle authored Jul 6, 2022
1 parent 21d450b commit 512b1de
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<String> allKeys() {
List<String> result = lowCardinalityKeys();
result.addAll(highCardinalityKeys());
return result;
}

public SELF hasOnlyKeys(String... keys) {
isNotNull();
Set<String> actualKeys = new LinkedHashSet<>(allKeys());
List<String> expectedKeys = Arrays.asList(keys);
boolean sameContent = actualKeys.containsAll(expectedKeys) && actualKeys.size() == expectedKeys.size();

if (!sameContent) {
Set<String> extraKeys = new LinkedHashSet<>(actualKeys);
extraKeys.removeAll(expectedKeys);

Set<String> 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<String> lowCardinalityKeys() {
return this.actual.getLowCardinalityKeyValues().stream().map(KeyValue::getKey).collect(Collectors.toList());
}
Expand Down Expand Up @@ -194,7 +234,6 @@ public SELF doesNotHaveLowCardinalityKeyValueWithKey(String key) {

public SELF doesNotHaveLowCardinalityKeyValue(String key, String value) {
isNotNull();
doesNotHaveLowCardinalityKeyValueWithKey(key);
Optional<KeyValue> optional = this.actual.getLowCardinalityKeyValues().stream()
.filter(tag -> tag.getKey().equals(key)).findFirst();
if (!optional.isPresent()) {
Expand Down Expand Up @@ -241,7 +280,6 @@ public SELF doesNotHaveHighCardinalityKeyValueWithKey(String key) {

public SELF doesNotHaveHighCardinalityKeyValue(String key, String value) {
isNotNull();
doesNotHaveHighCardinalityKeyValueWithKey(key);
Optional<KeyValue> optional = this.actual.getHighCardinalityKeyValues().stream()
.filter(tag -> tag.getKey().equals(key)).findFirst();
if (!optional.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 512b1de

Please sign in to comment.