Skip to content

Commit

Permalink
Change all containsExactly() fail() calls to go through a helper method.
Browse files Browse the repository at this point in the history 
This reduces duplication a bit, and it should be more natural when we switch to the key-value message format.

Also, IntelliJ static imported some methods everywhere when I chose to static import in one place, so that's in this CL, too, because why not?

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191443280
  • Loading branch information
@cpovirk @ronshapiro
cpovirk authored and ronshapiro committed Apr 4, 2018
1 parent 0b38d17 commit 7d5868b
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 106 deletions.
194 changes: 88 additions & 106 deletions core/src/main/java/com/google/common/truth/IterableSubject.java
View file
Expand Up @@ -17,7 +17,9 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.truth.Fact.factWithoutValue;
import static com.google.common.truth.StringUtil.format;
import static com.google.common.truth.SubjectUtils.accumulate;
import static com.google.common.truth.SubjectUtils.annotateEmptyStrings;
import static com.google.common.truth.SubjectUtils.countDuplicates;
Expand Down Expand Up @@ -234,8 +236,8 @@ public final Ordered containsAllIn(Iterable<?> expectedIterable) {
List<?> actual = Lists.newLinkedList(actual());
Collection<?> expected = iterableToCollection(expectedIterable);

List<Object> missing = Lists.newArrayList();
List<Object> actualNotInOrder = Lists.newArrayList();
List<Object> missing = newArrayList();
List<Object> actualNotInOrder = newArrayList();

boolean ordered = true;
// step through the expected elements...
Expand Down Expand Up @@ -316,7 +318,7 @@ private static void moveElements(List<?> input, Collection<Object> output, int m
*/
@CanIgnoreReturnValue
public final Ordered containsExactly(@Nullable Object... varargs) {
List<Object> expected = (varargs == null) ? Lists.newArrayList((Object) null) : asList(varargs);
List<Object> expected = (varargs == null) ? newArrayList((Object) null) : asList(varargs);
return containsExactlyElementsIn(
expected, varargs != null && varargs.length == 1 && varargs[0] instanceof Iterable);
}
Expand Down Expand Up @@ -351,12 +353,6 @@ public final Ordered containsExactlyElementsIn(Object[] expected) {
}

private Ordered containsExactlyElementsIn(Iterable<?> required, boolean addElementsInWarning) {
String failSuffix =
addElementsInWarning
? ". Passing an iterable to the varargs method containsExactly(Object...) is "
+ "often not the correct thing to do. Did you mean to call "
+ "containsExactlyElementsIn(Iterable) instead?"
: "";
Iterator<?> actualIter = actual().iterator();
Iterator<?> requiredIter = required.iterator();

Expand All @@ -380,12 +376,12 @@ private Ordered containsExactlyElementsIn(Iterable<?> required, boolean addEleme
// effect on the result now.
if (!Objects.equal(actualElement, requiredElement)) {
// Missing elements; elements that are not missing will be removed as we iterate.
Collection<Object> missing = Lists.newArrayList();
Collection<Object> missing = newArrayList();
missing.add(requiredElement);
Iterators.addAll(missing, requiredIter);

// Extra elements that the subject had but shouldn't have.
Collection<Object> extra = Lists.newArrayList();
Collection<Object> extra = newArrayList();

// Remove all actual elements from missing, and add any that weren't in missing
// to extra.
Expand All @@ -399,97 +395,85 @@ private Ordered containsExactlyElementsIn(Iterable<?> required, boolean addEleme
}
}

// Fail if there are either missing or extra elements.

// TODO(kak): Possible enhancement: Include "[1 copy]" if the element does appear in
// the subject but not enough times. Similarly for unexpected extra items.
if (!missing.isEmpty()) {
if (!extra.isEmpty()) {
// Subject is both missing required elements and contains extra elements
boolean addTypeInfo = hasMatchingToStringPair(missing, extra);
failWithRawMessage(
"Not true that %s contains exactly <%s>. "
+ "It is missing <%s> and has unexpected items <%s>%s",
actualAsString(),
annotateEmptyStrings(required),
addTypeInfo
? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(missing))
: countDuplicates(annotateEmptyStrings(missing)),
addTypeInfo
? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(extra))
: countDuplicates(annotateEmptyStrings(extra)),
failSuffix);
return ALREADY_FAILED;
} else {
failWithBadResultsAndSuffix(
"contains exactly",
annotateEmptyStrings(required),
"is missing",
countDuplicates(annotateEmptyStrings(missing)),
failSuffix);
return ALREADY_FAILED;
}
if (missing.isEmpty() && extra.isEmpty()) {
/*
* This containsExactly() call is a success. But the iterables were not in the same order,
* so return an object that will fail the test if the user calls inOrder().
*/
return new NotInOrder(this, "contains exactly these elements in order", required);
}
if (!extra.isEmpty()) {
failWithBadResultsAndSuffix(
"contains exactly",
annotateEmptyStrings(required),
"has unexpected items",
countDuplicates(annotateEmptyStrings(extra)),
failSuffix);
return ALREADY_FAILED;
}

// Since we know the iterables were not in the same order, inOrder() can just fail.
return new NotInOrder(this, "contains exactly these elements in order", required);
return failExactly(required, addElementsInWarning, missing, extra);
}
}

// Here, we must have reached the end of one of the iterators without finding any
// pairs of elements that differ. If the actual iterator still has elements, they're
// extras. If the required iterator has elements, they're missing elements.
if (actualIter.hasNext()) {
failWithBadResultsAndSuffix(
"contains exactly",
annotateEmptyStrings(required),
"has unexpected items",
countDuplicates(annotateEmptyStrings(Lists.newArrayList(actualIter))),
failSuffix);
return ALREADY_FAILED;
return failExactly(
required,
addElementsInWarning,
/* missing= */ ImmutableList.of(),
/* extra= */ newArrayList(actualIter));
} else if (requiredIter.hasNext()) {
failWithBadResultsAndSuffix(
"contains exactly",
annotateEmptyStrings(required),
"is missing",
countDuplicates(annotateEmptyStrings(Lists.newArrayList(requiredIter))),
failSuffix);
return ALREADY_FAILED;
return failExactly(
required,
addElementsInWarning,
/* missing= */ newArrayList(requiredIter),
/* extra= */ ImmutableList.of());
}

// If neither iterator has elements, we reached the end and the elements were in
// order, so inOrder() can just succeed.
return IN_ORDER;
}

/**
* Fails with the bad results and a suffix.
*
* @param verb the check being asserted
* @param expected the expectations against which the subject is compared
* @param failVerb the failure of the check being asserted
* @param actual the actual value the subject was compared against
* @param suffix a suffix to append to the failure message
*/
private void failWithBadResultsAndSuffix(
String verb, Object expected, String failVerb, Object actual, String suffix) {
failWithRawMessage(
"Not true that %s %s <%s>. It %s <%s>%s",
actualAsString(),
verb,
expected,
failVerb,
(actual == null) ? "null reference" : actual,
suffix);
private Ordered failExactly(
Iterable<?> required,
boolean addElementsInWarning,
Collection<?> missing,
Collection<?> extra) {
// TODO(kak): Possible enhancement: Include "[1 copy]" if the element does appear in
// the subject but not enough times. Similarly for unexpected extra items.
// Subject is both missing required elements and contains extra elements
boolean addTypeInfo = hasMatchingToStringPair(missing, extra);
StringBuilder message =
new StringBuilder(
format(
"Not true that %s contains exactly <%s>. It ",
actualAsString(), annotateEmptyStrings(required)));
/*
* Fact keys like "missing (1)" and "unexpected (2)" violate our recommendation that keys
* should be fixed strings. This violation lets the fact value contain only the elements
* (instead of also containing the count), so it feels worthwhile.
*/
if (!missing.isEmpty()) {
message.append(
format(
"is missing <%s>",
addTypeInfo
? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(missing))
: countDuplicates(annotateEmptyStrings(missing))));
}
if (!extra.isEmpty()) {
if (!missing.isEmpty()) {
message.append(" and ");
}
message.append(
format(
"has unexpected items <%s>",
addTypeInfo
? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(extra))
: countDuplicates(annotateEmptyStrings(extra))));
}
if (addElementsInWarning) {
message.append(
". Passing an iterable to the varargs method containsExactly(Object...) is "
+ "often not the correct thing to do. Did you mean to call "
+ "containsExactlyElementsIn(Iterable) instead?");
}
failWithRawMessage(message.toString());
return ALREADY_FAILED;
}

/**
Expand Down Expand Up @@ -840,7 +824,7 @@ public void doesNotContain(@Nullable E excluded) {
@CanIgnoreReturnValue
public final Ordered containsExactly(@Nullable E... expected) {
return containsExactlyElementsIn(
(expected == null) ? Lists.newArrayList((E) null) : asList(expected));
(expected == null) ? newArrayList((E) null) : asList(expected));
}

/**
Expand Down Expand Up @@ -983,7 +967,7 @@ private String describeMissingOrExtra(List<? extends E> missing, List<? extends
+ "provided and has consequently been ignored.)";
}
} else if (missing.size() == 1 && extra.size() >= 1) {
return StringUtil.format(
return format(
"is missing an element that %s <%s> and has unexpected elements <%s>",
correspondence, missing.get(0), formatExtras(missing.get(0), extra));
} else {
Expand All @@ -993,24 +977,23 @@ private String describeMissingOrExtra(List<? extends E> missing, List<? extends

private String describeMissingOrExtraWithoutPairing(
String verb, List<? extends E> missing, List<? extends A> extra) {
List<String> messages = Lists.newArrayList();
List<String> messages = newArrayList();
if (!missing.isEmpty()) {
messages.add(
StringUtil.format("is missing an element that %s %s", verb, formatMissing(missing)));
messages.add(format("is missing an element that %s %s", verb, formatMissing(missing)));
}
if (!extra.isEmpty()) {
messages.add(StringUtil.format("has unexpected elements <%s>", extra));
messages.add(format("has unexpected elements <%s>", extra));
}
return Joiner.on(" and ").join(messages);
}

private String describeMissingOrExtraWithPairing(Pairing pairing) {
List<String> messages = Lists.newArrayList();
List<String> messages = newArrayList();
for (Object key : pairing.pairedKeysToExpectedValues.keySet()) {
E missing = pairing.pairedKeysToExpectedValues.get(key);
List<A> extras = pairing.pairedKeysToActualValues.get(key);
messages.add(
StringUtil.format(
format(
"is missing an element that corresponds to <%s> and has unexpected elements <%s> "
+ "with key %s",
missing, formatExtras(missing, extras), key));
Expand All @@ -1032,7 +1015,7 @@ private List<String> formatExtras(E missing, List<? extends A> extras) {
for (A extra : extras) {
@Nullable String diff = correspondence.formatDiff(extra, missing);
if (diff != null) {
extrasFormatted.add(StringUtil.format("%s (diff: %s)", extra, diff));
extrasFormatted.add(format("%s (diff: %s)", extra, diff));
} else {
extrasFormatted.add(extra.toString());
}
Expand All @@ -1050,7 +1033,7 @@ private <T> List<T> findNotIndexed(List<T> list, Set<Integer> indexes) {
// index must be in there once.
return ImmutableList.of();
}
List<T> notIndexed = Lists.newArrayList();
List<T> notIndexed = newArrayList();
for (int index = 0; index < list.size(); index++) {
if (!indexes.contains(index)) {
notIndexed.add(list.get(index));
Expand Down Expand Up @@ -1279,16 +1262,16 @@ private String describeMissing(List<? extends E> missing, List<? extends A> extr
}

private String describeMissingWithoutPairing(String verb, List<? extends E> missing) {
return StringUtil.format("is missing an element that %s %s", verb, formatMissing(missing));
return format("is missing an element that %s %s", verb, formatMissing(missing));
}

private String describeMissingWithPairing(Pairing pairing) {
List<String> messages = Lists.newArrayList();
List<String> messages = newArrayList();
for (Object key : pairing.pairedKeysToExpectedValues.keySet()) {
E missing = pairing.pairedKeysToExpectedValues.get(key);
List<A> extras = pairing.pairedKeysToActualValues.get(key);
messages.add(
StringUtil.format(
format(
"is missing an element that corresponds to <%s> (but did have elements <%s> with "
+ "matching key %s)",
missing, formatExtras(missing, extras), key));
Expand Down Expand Up @@ -1334,7 +1317,7 @@ private boolean failIfOneToOneMappingHasMissing(
@SafeVarargs
public final void containsAnyOf(@Nullable E first, @Nullable E second, @Nullable E... rest) {
containsAny(
StringUtil.format("contains at least one element that %s any of", correspondence),
format("contains at least one element that %s any of", correspondence),
accumulate(first, second, rest));
}

Expand All @@ -1344,8 +1327,7 @@ public final void containsAnyOf(@Nullable E first, @Nullable E second, @Nullable
*/
public void containsAnyIn(Iterable<? extends E> expected) {
containsAny(
StringUtil.format("contains at least one element that %s any element in", correspondence),
expected);
format("contains at least one element that %s any element in", correspondence), expected);
}

/**
Expand Down Expand Up @@ -1390,12 +1372,12 @@ private void containsAny(String failVerb, Iterable<? extends E> expected) {
}

private String describeAnyMatchesByKey(Pairing pairing) {
List<String> messages = Lists.newArrayList();
List<String> messages = newArrayList();
for (Object key : pairing.pairedKeysToExpectedValues.keySet()) {
E expected = pairing.pairedKeysToExpectedValues.get(key);
List<A> got = pairing.pairedKeysToActualValues.get(key);
messages.add(
StringUtil.format(
format(
"with key %s, would have accepted %s, but got %s",
key, expected, formatExtras(expected, got)));
}
Expand Down Expand Up @@ -1569,13 +1551,13 @@ private final class Pairing {
* List of the expected values not used in the pairing. Iterates in the order they appear in
* the input.
*/
private final List<E> unpairedExpectedValues = Lists.newArrayList();
private final List<E> unpairedExpectedValues = newArrayList();

/**
* List of the actual values not used in the pairing. Iterates in the order they appear in the
* input.
*/
private final List<A> unpairedActualValues = Lists.newArrayList();
private final List<A> unpairedActualValues = newArrayList();
}
}
}
5 changes: 5 additions & 0 deletions core/src/main/java/com/google/common/truth/SubjectUtils.java
View file
Expand Up @@ -15,6 +15,8 @@
*/
package com.google.common.truth;

import static com.google.common.collect.Iterables.isEmpty;

import com.google.common.base.Objects;
import com.google.common.base.Optional;
import com.google.common.collect.HashMultimap;
Expand Down Expand Up @@ -148,6 +150,9 @@ static List<Object> retainMatchingToString(Iterable<?> items, Iterable<?> itemsT
* <p>Example: {@code hasMatchingToStringPair([1L, 2L], [1]) == true}
*/
static boolean hasMatchingToStringPair(Iterable<?> items1, Iterable<?> items2) {
if (isEmpty(items1) || isEmpty(items2)) {
return false; // Bail early to avoid calling hashCode() on the elements unnecessarily.
}
return !retainMatchingToString(items1, items2).isEmpty();
}

Expand Down

0 comments on commit 7d5868b

Please sign in to comment.