Skip to content

Commit

Permalink
Annotate empty strings in collections printed by IterableSubject.
Browse files Browse the repository at this point in the history
Example error message:

  Not true that <[]> contains exactly <["" (empty String)]>. It is missing <["" (empty String)]>

=== Note: ===

Some methods such as `contains()` and `containsAnyOf()` can't be changed. The reason is that their error message has the form "Not true that <[]> contains any of <[]>" where the first list is actualAsString(). Changing only the second list could lead to confusion because sometimes it'd show "" (empty String) and sometimes it wouldn't.

Consequently, I adopted the criterion of only changing error messages where I can at annotate at least the expected list and some derived list, such as "Not true that <[]> contains all of <[]>. It is missing <[]>".

Obviously, this would be solved if we stop using actualAsString(), but IIRC this is not currently the plan.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=163937787
  • Loading branch information
nymanjens authored and ronshapiro committed Aug 3, 2017
1 parent 14a23d5 commit d26c072
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 19 deletions.
45 changes: 30 additions & 15 deletions core/src/main/java/com/google/common/truth/IterableSubject.java
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.SubjectUtils.accumulate; import static com.google.common.truth.SubjectUtils.accumulate;
import static com.google.common.truth.SubjectUtils.annotateEmptyStrings;
import static com.google.common.truth.SubjectUtils.countDuplicates; import static com.google.common.truth.SubjectUtils.countDuplicates;
import static com.google.common.truth.SubjectUtils.countDuplicatesAndAddTypeInfo; import static com.google.common.truth.SubjectUtils.countDuplicatesAndAddTypeInfo;
import static com.google.common.truth.SubjectUtils.hasMatchingToStringPair; import static com.google.common.truth.SubjectUtils.hasMatchingToStringPair;
Expand Down Expand Up @@ -240,12 +241,17 @@ private Ordered containsAll(String failVerb, Iterable<?> expectedIterable) {
"Not true that %s %s <%s>. It is missing <%s>. However, it does contain <%s>.", "Not true that %s %s <%s>. It is missing <%s>. However, it does contain <%s>.",
actualAsString(), actualAsString(),
failVerb, failVerb,
expected, annotateEmptyStrings(expected),
countDuplicatesAndAddTypeInfo(missing), countDuplicatesAndAddTypeInfo(annotateEmptyStrings(missing)),
countDuplicatesAndAddTypeInfo( countDuplicatesAndAddTypeInfo(
retainMatchingToString(actual(), missing /* itemsToCheck */))); annotateEmptyStrings(
retainMatchingToString(actual(), missing /* itemsToCheck */))));
} else { } else {
failWithBadResults(failVerb, expected, "is missing", countDuplicates(missing)); failWithBadResults(
failVerb,
annotateEmptyStrings(expected),
"is missing",
countDuplicates(annotateEmptyStrings(missing)));
} }
} }
return ordered ? IN_ORDER : new NotInOrder("contains all elements in order", expected); return ordered ? IN_ORDER : new NotInOrder("contains all elements in order", expected);
Expand Down Expand Up @@ -353,23 +359,31 @@ private Ordered containsExactlyElementsIn(Iterable<?> required, boolean addEleme
"Not true that %s contains exactly <%s>. " "Not true that %s contains exactly <%s>. "
+ "It is missing <%s> and has unexpected items <%s>%s", + "It is missing <%s> and has unexpected items <%s>%s",
actualAsString(), actualAsString(),
required, annotateEmptyStrings(required),
addTypeInfo ? countDuplicatesAndAddTypeInfo(missing) : countDuplicates(missing), addTypeInfo
addTypeInfo ? countDuplicatesAndAddTypeInfo(extra) : countDuplicates(extra), ? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(missing))
: countDuplicates(annotateEmptyStrings(missing)),
addTypeInfo
? countDuplicatesAndAddTypeInfo(annotateEmptyStrings(extra))
: countDuplicates(annotateEmptyStrings(extra)),
failSuffix); failSuffix);
return ALREADY_FAILED; return ALREADY_FAILED;
} else { } else {
failWithBadResultsAndSuffix( failWithBadResultsAndSuffix(
"contains exactly", required, "is missing", countDuplicates(missing), failSuffix); "contains exactly",
annotateEmptyStrings(required),
"is missing",
countDuplicates(annotateEmptyStrings(missing)),
failSuffix);
return ALREADY_FAILED; return ALREADY_FAILED;
} }
} }
if (!extra.isEmpty()) { if (!extra.isEmpty()) {
failWithBadResultsAndSuffix( failWithBadResultsAndSuffix(
"contains exactly", "contains exactly",
required, annotateEmptyStrings(required),
"has unexpected items", "has unexpected items",
countDuplicates(extra), countDuplicates(annotateEmptyStrings(extra)),
failSuffix); failSuffix);
return ALREADY_FAILED; return ALREADY_FAILED;
} }
Expand All @@ -385,17 +399,17 @@ private Ordered containsExactlyElementsIn(Iterable<?> required, boolean addEleme
if (actualIter.hasNext()) { if (actualIter.hasNext()) {
failWithBadResultsAndSuffix( failWithBadResultsAndSuffix(
"contains exactly", "contains exactly",
required, annotateEmptyStrings(required),
"has unexpected items", "has unexpected items",
countDuplicates(Lists.newArrayList(actualIter)), countDuplicates(annotateEmptyStrings(Lists.newArrayList(actualIter))),
failSuffix); failSuffix);
return ALREADY_FAILED; return ALREADY_FAILED;
} else if (requiredIter.hasNext()) { } else if (requiredIter.hasNext()) {
failWithBadResultsAndSuffix( failWithBadResultsAndSuffix(
"contains exactly", "contains exactly",
required, annotateEmptyStrings(required),
"is missing", "is missing",
countDuplicates(Lists.newArrayList(requiredIter)), countDuplicates(annotateEmptyStrings(Lists.newArrayList(requiredIter))),
failSuffix); failSuffix);
return ALREADY_FAILED; return ALREADY_FAILED;
} }
Expand Down Expand Up @@ -455,7 +469,8 @@ private void containsNone(String failVerb, Iterable<?> excluded) {
} }
} }
if (!present.isEmpty()) { if (!present.isEmpty()) {
failWithBadResults(failVerb, excluded, "contains", present); failWithBadResults(
failVerb, annotateEmptyStrings(excluded), "contains", annotateEmptyStrings(present));
} }
} }


Expand Down
29 changes: 28 additions & 1 deletion core/src/main/java/com/google/common/truth/SubjectUtils.java
Expand Up @@ -18,6 +18,7 @@
import com.google.common.base.Objects; import com.google.common.base.Objects;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.SetMultimap; import com.google.common.collect.SetMultimap;
import java.util.ArrayList; import java.util.ArrayList;
Expand Down Expand Up @@ -60,7 +61,7 @@ static <T> int countOf(T t, Iterable<T> items) {
return count; return count;
} }


static <T> List<Object> countDuplicates(Collection<T> items) { static <T> List<Object> countDuplicates(Iterable<T> items) {
// We use a List to de-dupe instead of a Set in case the elements don't have a proper // We use a List to de-dupe instead of a Set in case the elements don't have a proper
// .hashCode() method (e.g., MessageSet from old versions of protobuf). // .hashCode() method (e.g., MessageSet from old versions of protobuf).
List<T> itemSet = new ArrayList<T>(); List<T> itemSet = new ArrayList<T>();
Expand Down Expand Up @@ -206,4 +207,30 @@ static <T> List<T> iterableToList(Iterable<T> iterable) {
return Lists.newArrayList(iterable); return Lists.newArrayList(iterable);
} }
} }

/**
* Returns a collection with all empty strings replaced by a non-empty human understandable
* indicator for an empty string.
*
* <p>Returns the given collection if it contains no empty strings.
*/
static <T> Iterable<T> annotateEmptyStrings(Iterable<T> items) {
if (Iterables.contains(items, "")) {
List<T> annotatedItems = Lists.newArrayList();
for (T item : items) {
if (Objects.equal(item, "")) {
// This is a safe cast because know that at least one instance of T (this item) is a
// String.
@SuppressWarnings("unchecked")
T newItem = (T) "\"\" (empty String)";
annotatedItems.add(newItem);
} else {
annotatedItems.add(item);
}
}
return annotatedItems;
} else {
return items;
}
}
} }
Expand Up @@ -29,6 +29,7 @@
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.JUnit4; import org.junit.runners.JUnit4;
Expand All @@ -41,6 +42,8 @@
*/ */
@RunWith(JUnit4.class) @RunWith(JUnit4.class)
public class IterableSubjectTest { public class IterableSubjectTest {
@Rule public final ExpectFailure expectFailure = new ExpectFailure();

@Test @Test
public void hasSize() { public void hasSize() {
assertThat(ImmutableList.of(1, 2, 3)).hasSize(3); assertThat(ImmutableList.of(1, 2, 3)).hasSize(3);
Expand Down Expand Up @@ -169,7 +172,7 @@ public void doesNotContainDuplicates() {
public void doesNotContainDuplicatesMixedTypes() { public void doesNotContainDuplicatesMixedTypes() {
assertThat(asList(1, 2, 2L, 3)).containsNoDuplicates(); assertThat(asList(1, 2, 2L, 3)).containsNoDuplicates();
} }

@Test @Test
public void doesNotContainDuplicatesFailure() { public void doesNotContainDuplicatesFailure() {
try { try {
Expand Down Expand Up @@ -426,7 +429,7 @@ public void iterableContainsAllOfFailsWithSameToStringAndHomogeneousListWithNull
+ "<[null (null type)]>. However, it does contain <[null] (java.lang.String)>."); + "<[null (null type)]>. However, it does contain <[null] (java.lang.String)>.");
} }
} }

@Test @Test
public void iterableContainsAllOfFailsWithSameToStringAndHeterogeneousListWithDuplicates() { public void iterableContainsAllOfFailsWithSameToStringAndHeterogeneousListWithDuplicates() {
try { try {
Expand All @@ -442,6 +445,16 @@ public void iterableContainsAllOfFailsWithSameToStringAndHeterogeneousListWithDu
} }
} }


@Test
public void iterableContainsAllOfFailsWithEmptyString() {
expectFailure.whenTesting().that(asList("a", null)).containsAllOf("", null);

assertThat(expectFailure.getFailure().getMessage())
.isEqualTo(
"Not true that <[a, null]> contains all of <[\"\" (empty String), null]>. "
+ "It is missing <[\"\" (empty String)]>");
}

@Test @Test
public void iterableContainsAllOfInOrder() { public void iterableContainsAllOfInOrder() {
assertThat(asList(3, 2, 5)).containsAllOf(3, 2, 5).inOrder(); assertThat(asList(3, 2, 5)).containsAllOf(3, 2, 5).inOrder();
Expand Down Expand Up @@ -566,6 +579,16 @@ public void iterableContainsNoneOfFailureWithDuplicateInExpected() {
} }
} }


@Test
public void iterableContainsNoneOfFailureWithEmptyString() {
expectFailure.whenTesting().that(asList("")).containsNoneOf("", null);

assertThat(expectFailure.getFailure().getMessage())
.isEqualTo(
"Not true that <[]> contains none of <[\"\" (empty String), null]>. "
+ "It contains <[\"\" (empty String)]>");
}

@Test @Test
public void iterableContainsExactlyArray() { public void iterableContainsExactlyArray() {
String[] stringArray = {"a", "b"}; String[] stringArray = {"a", "b"};
Expand Down Expand Up @@ -628,12 +651,42 @@ public void iterableContainsExactlyWithNullThird() {
public void iterableContainsExactlyWithNull() { public void iterableContainsExactlyWithNull() {
assertThat(asList(1, null, 3)).containsExactly(1, null, 3); assertThat(asList(1, null, 3)).containsExactly(1, null, 3);
} }

@Test @Test
public void iterableContainsExactlyWithNullOutOfOrder() { public void iterableContainsExactlyWithNullOutOfOrder() {
assertThat(asList(1, null, 3)).containsExactly(1, 3, (Integer) null); assertThat(asList(1, null, 3)).containsExactly(1, 3, (Integer) null);
} }


@Test
public void iterableContainsExactlyWithEmptyString() {
expectFailure.whenTesting().that(asList()).containsExactly("");

assertThat(expectFailure.getFailure().getMessage())
.isEqualTo(
"Not true that <[]> contains exactly <[\"\" (empty String)]>. "
+ "It is missing <[\"\" (empty String)]>");
}

@Test
public void iterableContainsExactlyWithEmptyStringAndUnexpectedItem() {
expectFailure.whenTesting().that(asList("a", null)).containsExactly("");

assertThat(expectFailure.getFailure().getMessage())
.isEqualTo(
"Not true that <[a, null]> contains exactly <[\"\" (empty String)]>. "
+ "It is missing <[\"\" (empty String)]> and has unexpected items <[a, null]>");
}

@Test
public void iterableContainsExactlyWithEmptyStringAndMissingItem() {
expectFailure.whenTesting().that(asList("")).containsExactly("a", null);

assertThat(expectFailure.getFailure().getMessage())
.isEqualTo(
"Not true that <[]> contains exactly <[a, null]>. "
+ "It is missing <[a, null]> and has unexpected items <[\"\" (empty String)]>");
}

@Test @Test
public void iterableContainsExactlyWithElementsThatThrowWhenYouCallHashCode() { public void iterableContainsExactlyWithElementsThatThrowWhenYouCallHashCode() {
HashCodeThrower one = new HashCodeThrower(); HashCodeThrower one = new HashCodeThrower();
Expand Down

0 comments on commit d26c072

Please sign in to comment.