Skip to content

Commit

Permalink
Improve the failure messages from MapSubject.containsExactly and cont…
Browse files Browse the repository at this point in the history
…ainsExactlyElementsIn (and, in a more minor way, isEqualTo).

The most important change is to containsExactly and containsExactlyElementsIn's any-order assertions. These now report separately keys which are missing, unexpected, or present with the wrong value. Previously, they only reported missing or unexpected entries (which they referred to as either 'items').

The other changes to containsExactly and containsExactlyElementsIn are:
 - The in-order assertions now refer to entries as 'entries' rather than 'elements'.
 - They now use the standard Map formatting of {k1=v1, k2=v2} (as used by Map.toString and other MapSubject assertions). Previously, they used the non-standard [k1=v1, k2=v2].
 - The any-order assertions where keys or values are toString-equal now give the types of the keys or values rather than the types of the entries.

We already provide the pairing behaviour for isEqualTo. The changes there are:
 - The text of the messages is a bit more explicit. Previously, it said "missing the following entries" for the cases where there are no entries with the expected keys, "has the following extra entries" for the cases where there are entries with unexpected keys, and "has the following different entries" for the cases where there are matching keys with different values, which all seemed a bit unclear.
 - For a key diff where a key 123 was expected to have value "foo" but had value "bar", it formats the diff as "123=(expected foo but got bar)". Previously, it said "123=(foo, bar)" which was rather opaque (especially if the values have long toString()s which only differ slightly).
 - It says "It has..." instead of "The subject has...", to be more consistent with other assertions (and avoid the disfavoured "subject" terminology).
 - Where keys or values are toString-equal it now give the types of the keys or values rather than the types of the entries.
 - It has special handling for the case where the diff is empty but equals returns false (i.e. the Map implementation violates its contract). Previously, it would say "Not true that <...> is equal to <...>. The subject" and then just stop.

The test coverage is also improved.

This also fixes a couple of tests of test helpers which made assertions about the failure messages Truth would produce.

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=177790307
  • Loading branch information
peteg authored and ronshapiro committed Dec 4, 2017
1 parent 4066d17 commit aad4f3a
Show file tree
Hide file tree
Showing 2 changed files with 366 additions and 49 deletions.
163 changes: 143 additions & 20 deletions core/src/main/java/com/google/common/truth/MapSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@
import static com.google.common.truth.SubjectUtils.objectToTypeName;
import static com.google.common.truth.SubjectUtils.retainMatchingToString;

import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.LinkedHashMultiset;
import com.google.common.collect.Lists;
import com.google.common.collect.MapDifference;
import com.google.common.collect.MapDifference.ValueDifference;
import com.google.common.collect.Maps;
import com.google.common.collect.Multiset;
import com.google.common.collect.Sets;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.LinkedHashSet;
import java.util.List;
Expand All @@ -53,25 +57,13 @@ public class MapSubject extends Subject<MapSubject, Map<?, ?>> {
public void isEqualTo(@Nullable Object other) {
if (!Objects.equal(actual(), other)) {
if (other instanceof Map) {
MapDifference<?, ?> diff = Maps.difference((Map<?, ?>) other, (Map<?, ?>) actual());
String errorMsg = "The subject";
if (!diff.entriesOnlyOnLeft().isEmpty()) {
errorMsg += " is missing the following entries: " + diff.entriesOnlyOnLeft();
if (!diff.entriesOnlyOnRight().isEmpty() || !diff.entriesDiffering().isEmpty()) {
errorMsg += " and";
}
}
if (!diff.entriesOnlyOnRight().isEmpty()) {
errorMsg += " has the following extra entries: " + diff.entriesOnlyOnRight();
if (!diff.entriesDiffering().isEmpty()) {
errorMsg += " and";
}
}
if (!diff.entriesDiffering().isEmpty()) {
errorMsg += " has the following different entries: " + diff.entriesDiffering();
boolean mapEquals = containsExactlyEntriesInAnyOrder((Map<?, ?>) other, "is equal to");
if (mapEquals) {
failWithRawMessage(
"Not true that %s is equal to <%s>. It is equal according to the contract of "
+ "Map.equals(Object), but this implementation returned false",
actualAsString(), other);
}
failWithRawMessage(
"Not true that %s is equal to <%s>. " + errorMsg, actualAsString(), other);
} else {
fail("is equal to", other);
}
Expand Down Expand Up @@ -180,14 +172,16 @@ public void doesNotContainEntry(@Nullable Object key, @Nullable Object value) {
/** Fails if the map is not empty. */
@CanIgnoreReturnValue
public Ordered containsExactly() {
return check().that(actual().entrySet()).containsExactly();
return containsExactlyEntriesIn(ImmutableMap.of());
}

/**
* Fails if the map does not contain exactly the given set of key/value pairs.
*
* <p><b>Warning:</b> the use of varargs means that we cannot guarantee an equal number of
* key/value pairs at compile time. Please make sure you provide varargs in key/value pairs!
*
* <p>The arguments must not contain duplicate keys.
*/
@CanIgnoreReturnValue
public Ordered containsExactly(@Nullable Object k0, @Nullable Object v0, Object... rest) {
Expand Down Expand Up @@ -221,9 +215,136 @@ private static Map<Object, Object> accumulateMap(
/** Fails if the map does not contain exactly the given set of entries in the given map. */
@CanIgnoreReturnValue
public Ordered containsExactlyEntriesIn(Map<?, ?> expectedMap) {
return check().that(actual().entrySet()).containsExactlyElementsIn(expectedMap.entrySet());
boolean containsAnyOrder = containsExactlyEntriesInAnyOrder(expectedMap, "contains exactly");
if (containsAnyOrder) {
return new MapInOrder(expectedMap);
} else {
return ALREADY_FAILED;
}
}

@CanIgnoreReturnValue
private boolean containsExactlyEntriesInAnyOrder(Map<?, ?> expectedMap, String failVerb) {
MapDifference<?, ?> diff = Maps.difference(expectedMap, actual());
if (diff.areEqual()) {
return true;
}
boolean includeKeyTypes = includeKeyTypes(diff);
Map<?, ?> missing = diff.entriesOnlyOnLeft();
Map<?, ?> unexpected = diff.entriesOnlyOnRight();
Map<?, ? extends ValueDifference<?>> wrongValues = diff.entriesDiffering();
StringBuilder errorMsg = new StringBuilder();
if (!missing.isEmpty()) {
errorMsg
.append("is missing keys for the following entries: ")
.append(includeKeyTypes ? addKeyTypes(missing) : missing);
}
if (!unexpected.isEmpty()) {
if (errorMsg.length() > 0) {
errorMsg.append(" and ");
}
errorMsg
.append("has the following entries with unexpected keys: ")
.append(includeKeyTypes ? addKeyTypes(unexpected) : unexpected);
}
if (!wrongValues.isEmpty()) {
if (errorMsg.length() > 0) {
errorMsg.append(" and ");
}
Map<?, String> wrongValuesFormatted =
Maps.transformValues(wrongValues, VALUE_DIFFERENCE_FORMATTER);
errorMsg
.append("has the following entries with matching keys but different values: ")
.append(includeKeyTypes ? addKeyTypes(wrongValuesFormatted) : wrongValuesFormatted);
}
failWithRawMessage(
"Not true that %s %s <%s>. It %s", actualAsString(), failVerb, expectedMap, errorMsg);
return false;
}

private static boolean includeKeyTypes(MapDifference<?, ?> diff) {
// We will annotate all the keys in the diff with their types if any of the keys involved have
// the same toString() without being equal.
Set<Object> keys = Sets.newHashSet();
keys.addAll(diff.entriesOnlyOnLeft().keySet());
keys.addAll(diff.entriesOnlyOnRight().keySet());
keys.addAll(diff.entriesDiffering().keySet());
return hasMatchingToStringPair(keys, keys);
}

private static final Map<Object, Object> addKeyTypes(Map<?, ?> in) {
Map<Object, Object> out = Maps.newLinkedHashMap();
for (Map.Entry<?, ?> entry : in.entrySet()) {
out.put(new TypedToStringWrapper(entry.getKey()), entry.getValue());
}
return out;
}

private static final Function<ValueDifference<?>, String> VALUE_DIFFERENCE_FORMATTER =
new Function<ValueDifference<?>, String>() {
@Override
public String apply(ValueDifference<?> diff) {
Object left = diff.leftValue();
Object right = diff.rightValue();
boolean includeTypes = left.toString().equals(right.toString());
return StringUtil.format(
"(expected %s but got %s)",
includeTypes ? new TypedToStringWrapper(left) : left,
includeTypes ? new TypedToStringWrapper(right) : right);
}
};

private static class TypedToStringWrapper {

private final Object delegate;

TypedToStringWrapper(Object delegate) {
this.delegate = delegate;
}

@Override
public boolean equals(Object other) {
return Objects.equal(delegate, other);
}

@Override
public int hashCode() {
return Objects.hashCode(delegate);
}

@Override
public String toString() {
return StringUtil.format("%s (%s)", delegate, objectToTypeName(delegate));
}
}

private class MapInOrder implements Ordered {

private final Map<?, ?> expectedMap;

MapInOrder(Map<?, ?> expectedMap) {
this.expectedMap = expectedMap;
}

@Override
public void inOrder() {
List<?> expectedKeyOrder = Lists.newArrayList(expectedMap.keySet());
List<?> actualKeyOrder = Lists.newArrayList(actual().keySet());
if (!actualKeyOrder.equals(expectedKeyOrder)) {
failWithRawMessage(
"Not true that %s contains exactly these entries in order <%s>",
actualAsString(), expectedMap);
}
}
}

/** Ordered implementation that does nothing because an earlier check already caused a failure. */
private static final Ordered ALREADY_FAILED =
new Ordered() {
@Override
public void inOrder() {}
};

/**
* Starts a method chain for a check in which the actual values (i.e. the values of the {@link
* Map} under test) are compared to expected values using the given {@link Correspondence}. The
Expand Down Expand Up @@ -347,6 +468,8 @@ public Ordered containsExactly(@Nullable Object k0, @Nullable E v0, Object... re
*/
@CanIgnoreReturnValue
public <K, V extends E> Ordered containsExactlyEntriesIn(Map<K, V> expectedMap) {
// TODO(b/69728070): Show missing keys, unexpected keys, and matching keys with wrong values,
// like we do for the vanilla containsExactlyEntriesIn.
return check()
.that(actual().entrySet())
.comparingElementsUsing(new EntryCorrespondence<K, A, V>(correspondence))
Expand Down
Loading

0 comments on commit aad4f3a

Please sign in to comment.