Skip to content

Commit

Permalink
Next steps in Fuzzy Truth.
Browse files Browse the repository at this point in the history
1. Add UsingCorrespondence.doesNotContain for iterables and doesNotContainEntry for maps and multimaps.

2. Improve the grammatical correctness of failure messages.
BEFORE: Correspondence.toString() was expected to be written for a plural subject (e.g. "are finite numbers within 0.00001 of") and this played badly with situations where the subject is singular — in particular, in maps where by definition there's only one value per key, and in particularly particular there was one failure message which would have read "but the value is <2.71828> which does not are finite numbers within 0.00001 <3.14159>". (I hadn't noticed this until now because it didn't show up in the example used in the tests.)
AFTER: Correspondence.toString() was expected to be written for a singular subject (e.g. "is a finite number within 0.00001 of"). After spending ages playing with these things, I've concluded that it's easier to phrase the statements so that they always refer to a singular subject (if we have a collection we can refer to "each element") than that they always refer to a plural subject. (I don't really want to have two methods on Correspondence, one for each case, and require implementers to have to think about this grammar stuff.)

3. Minor: Make the failure messages in the tests easier to understand by putting a + sign in front of each string-which-parses-as-an-integer, so that we can tell the difference between the string "+456" and the integer 456. (Although StringSubject quotes strings for display, IterableSubject et al do not. Perhaps they should: that's a separate question...)

4. Very minor: Fix up some more which/that errors. I know this isn't really the end of the world, but once I'd started picking at it I kinda wanted to be consistent.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=130520851
  • Loading branch information
peteg authored and cpovirk committed Aug 17, 2016
1 parent 7324605 commit 8ef8343
Show file tree
Hide file tree
Showing 10 changed files with 241 additions and 95 deletions.
20 changes: 13 additions & 7 deletions core/src/main/java/com/google/common/truth/Correspondence.java
Expand Up @@ -45,7 +45,7 @@
public abstract class Correspondence<A, E> { public abstract class Correspondence<A, E> {


/** /**
* Returns a {@link Correspondence} between {@link Number} instances which considers instances to * Returns a {@link Correspondence} between {@link Number} instances that considers instances to
* correspond (i.e. {@link Correspondence#compare(Number, Number)} returns {@code true}) if the * correspond (i.e. {@link Correspondence#compare(Number, Number)} returns {@code true}) if the
* double values of each instance (i.e. the result of calling {@link Number#doubleValue()} on * double values of each instance (i.e. the result of calling {@link Number#doubleValue()} on
* them) are finite values within {@code tolerance} of each other. * them) are finite values within {@code tolerance} of each other.
Expand Down Expand Up @@ -83,7 +83,7 @@ public boolean compare(Number actual, Number expected) {


@Override @Override
public String toString() { public String toString() {
return "are finite numbers within " + tolerance + " of"; return "is a finite number within " + tolerance + " of";
} }
} }


Expand All @@ -95,12 +95,18 @@ public String toString() {


/** /**
* Returns a description of the correspondence, suitable to fill the gap in a failure message of * Returns a description of the correspondence, suitable to fill the gap in a failure message of
* the form {@code "Not true that <[a1, a2, a3]> contains exactly elements which ... <expected>"}. * the form {@code "<some actual element> is an element that ... <some expected element>"}. Note
* that this is a fragment of a verb phrase which takes a singular subject.
* *
* <p>For example, for a {@code Correspondence<String, Integer>} which tests whether the actual * <p>Example 1: For a {@code Correspondence<String, Integer>} that tests whether the actual
* string parses to the expected integer, this would return {@code "parse to"} to result in a * string parses to the expected integer, this would return {@code "parses to"} to result in a
* failure message of the form {@code "Not true that <[foo, 123, bar]> contains exactly elements * failure message of the form {@code "<some actual string> is an element that parses to <some
* which parse to <[123, 456]>"}. * expected integer>"}.
*
* <p>Example 2: For the {@code Correspondence<Number, Number>} returns by {@link #tolerance} this
* returns {@code "is a finite number within " + tolerance + " of"} to result in a failure message
* of the form {@code "<some actual number> is an element that is a finite number within 0.0001 of
* <some expected number>"}.
*/ */
@Override @Override
public abstract String toString(); public abstract String toString();
Expand Down
48 changes: 29 additions & 19 deletions core/src/main/java/com/google/common/truth/IterableSubject.java
Expand Up @@ -522,7 +522,7 @@ private void pairwiseCheck(PairwiseChecker checker) {
* expected} is an {@code E}. * expected} is an {@code E}.
* *
* <p>Any of the methods on the returned object may throw {@link ClassCastException} if they * <p>Any of the methods on the returned object may throw {@link ClassCastException} if they
* encounter an actual element which is not of type {@code A}. * encounter an actual element that is not of type {@code A}.
*/ */
public <A, E> UsingCorrespondence<A, E> comparingElementsUsing( public <A, E> UsingCorrespondence<A, E> comparingElementsUsing(
Correspondence<A, E> correspondence) { Correspondence<A, E> correspondence) {
Expand Down Expand Up @@ -550,28 +550,38 @@ private UsingCorrespondence(Correspondence<A, E> correspondence) {
} }


/** /**
* Attests that the subject contains at least one element which corresponds to the given * Attests that the subject contains at least one element that corresponds to the given expected
* expected element. * element.
*/ */
public void contains(@Nullable E expected) { public void contains(@Nullable E expected) {
for (A actual : getCastSubject()) { for (A actual : getCastSubject()) {
if (correspondence.compare(actual, expected)) { if (correspondence.compare(actual, expected)) {
return; return;
} }
} }
fail("contains one or more elements that " + correspondence, expected); fail("contains at least one element that " + correspondence, expected);
} }


/** Attests that none of the actual elements correspond to the given element. */ /** Attests that none of the actual elements correspond to the given element. */
@SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public. public void doesNotContain(@Nullable E excluded) {
private void doesNotContain(@Nullable E element) { List<A> matchingElements = new ArrayList<A>();
throw new UnsupportedOperationException(); for (A actual : getCastSubject()) {
if (correspondence.compare(actual, excluded)) {
matchingElements.add(actual);
}
}
if (!matchingElements.isEmpty()) {
failWithRawMessage(
"%s should not have contained an element that %s <%s>. "
+ "It contained the following such elements: <%s>",
getDisplaySubject(), correspondence, excluded, matchingElements);
}
} }


/** /**
* Attests that subject contains exactly elements which correspond to the expected elements, * Attests that subject contains exactly elements that correspond to the expected elements, i.e.
* i.e. that there is a 1:1 mapping between the actual elements and the expected elements where * that there is a 1:1 mapping between the actual elements and the expected elements where each
* each pair of elements correspond. * pair of elements correspond.
* *
* <p>To also test that the contents appear in the given order, make a call to {@code inOrder()} * <p>To also test that the contents appear in the given order, make a call to {@code inOrder()}
* on the object returned by this method. * on the object returned by this method.
Expand All @@ -583,9 +593,9 @@ private Ordered containsExactly(@Nullable E... expected) {
} }


/** /**
* Attests that subject contains exactly elements which correspond to the expected elements, * Attests that subject contains exactly elements that correspond to the expected elements, i.e.
* i.e. that there is a 1:1 mapping between the actual elements and the expected elements where * that there is a 1:1 mapping between the actual elements and the expected elements where each
* each pair of elements correspond. * pair of elements correspond.
* *
* <p>To also test that the contents appear in the given order, make a call to {@code inOrder()} * <p>To also test that the contents appear in the given order, make a call to {@code inOrder()}
* on the object returned by this method. * on the object returned by this method.
Expand All @@ -597,7 +607,7 @@ private Ordered containsExactlyElementsIn(Iterable<E> expected) {
} }


/** /**
* Attests that the subject contains elements which corresponds to all of the expected elements, * Attests that the subject contains elements that corresponds to all of the expected elements,
* i.e. that there is a 1:1 mapping between any subset of the actual elements and the expected * i.e. that there is a 1:1 mapping between any subset of the actual elements and the expected
* elements where each pair of elements correspond. * elements where each pair of elements correspond.
* *
Expand All @@ -612,7 +622,7 @@ private Ordered containsAllOf(@Nullable E first, @Nullable E second, @Nullable E
} }


/** /**
* Attests that the subject contains elements which corresponds to all of the expected elements, * Attests that the subject contains elements that corresponds to all of the expected elements,
* i.e. that there is a 1:1 mapping between any subset of the actual elements and the expected * i.e. that there is a 1:1 mapping between any subset of the actual elements and the expected
* elements where each pair of elements correspond. * elements where each pair of elements correspond.
* *
Expand All @@ -627,7 +637,7 @@ private Ordered containsAllIn(Iterable<E> expected) {
} }


/** /**
* Attests that the subject contains at least one element which corresponds to at least one of * Attests that the subject contains at least one element that corresponds to at least one of
* the expected elements. * the expected elements.
*/ */
@SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public. @SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public.
Expand All @@ -636,7 +646,7 @@ private void containsAnyOf(@Nullable E first, @Nullable E second, @Nullable E...
} }


/** /**
* Attests that the subject contains at least one element which corresponds to at least one of * Attests that the subject contains at least one element that corresponds to at least one of
* the expected elements. * the expected elements.
*/ */
@SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public. @SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public.
Expand All @@ -645,7 +655,7 @@ private void containsAnyIn(Iterable<E> expected) {
} }


/** /**
* Attests that the subject contains no elements which correspond to any of the given elements. * Attests that the subject contains no elements that correspond to any of the given elements.
* (Duplicates are irrelevant to this test, which fails if any of the subject elements * (Duplicates are irrelevant to this test, which fails if any of the subject elements
* correspond to any of the given elements.) * correspond to any of the given elements.)
*/ */
Expand All @@ -655,7 +665,7 @@ private void containsNoneOf(@Nullable E first, @Nullable E second, @Nullable E..
} }


/** /**
* Attests that the subject contains no elements which correspond to any of the given elements. * Attests that the subject contains no elements that correspond to any of the given elements.
* (Duplicates are irrelevant to this test, which fails if any of the subject elements * (Duplicates are irrelevant to this test, which fails if any of the subject elements
* correspond to any of the given elements.) * correspond to any of the given elements.)
*/ */
Expand Down
38 changes: 22 additions & 16 deletions core/src/main/java/com/google/common/truth/MapSubject.java
Expand Up @@ -199,7 +199,7 @@ public Ordered containsExactlyEntriesIn(Map<?, ?> expectedMap) {
* <p>Note that keys will always be compared with regular object equality ({@link Object#equals}). * <p>Note that keys will always be compared with regular object equality ({@link Object#equals}).
* *
* <p>Any of the methods on the returned object may throw {@link ClassCastException} if they * <p>Any of the methods on the returned object may throw {@link ClassCastException} if they
* encounter an actual value which is not of type {@code A} or an expected value which is not of * encounter an actual value that is not of type {@code A} or an expected value that is not of
* type {@code E}. * type {@code E}.
*/ */
public <A, E> UsingCorrespondence<A, E> comparingValuesUsing( public <A, E> UsingCorrespondence<A, E> comparingValuesUsing(
Expand All @@ -223,8 +223,8 @@ private UsingCorrespondence(Correspondence<A, E> correspondence) {
} }


/** /**
* Fails if the map does not contain an entry with the given key and a value which corresponds * Fails if the map does not contain an entry with the given key and a value that corresponds to
* to the given value. * the given value.
*/ */
public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValue) { public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValue) {
if (getSubject().containsKey(expectedKey)) { if (getSubject().containsKey(expectedKey)) {
Expand All @@ -236,9 +236,9 @@ public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValu
} }
// Found matching key with non-matching value. // Found matching key with non-matching value.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it contains the key <%s>, but the value " "Not true that %s contains an entry with key <%s> and a value that %s <%s>. "
+ "is <%s> which does not %s <%s>", + "However, it has a mapping from that key to <%s>",
getDisplaySubject(), expectedKey, actualValue, correspondence, expectedValue); getDisplaySubject(), expectedKey, correspondence, expectedValue, actualValue);
} else { } else {
// Did not find matching key. // Did not find matching key.
Set<Object> keys = new LinkedHashSet<Object>(); Set<Object> keys = new LinkedHashSet<Object>();
Expand All @@ -250,26 +250,32 @@ public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValu
if (!keys.isEmpty()) { if (!keys.isEmpty()) {
// Found matching values with non-matching keys. // Found matching values with non-matching keys.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it does not contain the key <%s>, " "Not true that %s contains an entry with key <%s> and a value that %s <%s>. "
+ "but does contain values which %s <%s> at the following keys: <%s>", + "However, the following keys are mapped to such values: <%s>",
getDisplaySubject(), expectedKey, correspondence, expectedValue, keys); getDisplaySubject(), expectedKey, correspondence, expectedValue, keys);
} else { } else {
// Did not find matching key or value. // Did not find matching key or value.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it does not contain the key <%s>, " "Not true that %s contains an entry with key <%s> and a value that %s <%s>",
+ "and it does not contain any values which %s <%s>",
getDisplaySubject(), expectedKey, correspondence, expectedValue); getDisplaySubject(), expectedKey, correspondence, expectedValue);
} }
} }
} }


/** /**
* Fails if the map contains an entry with the given key and a value which corresponds to the * Fails if the map contains an entry with the given key and a value that corresponds to the
* given value. * given value.
*/ */
@SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public. public void doesNotContainEntry(@Nullable Object excludedKey, @Nullable E excludedValue) {
private void doesNotContainEntry(@Nullable Object key, @Nullable E value) { if (getSubject().containsKey(excludedKey)) {
throw new UnsupportedOperationException(); A actualValue = getCastSubject().get(excludedKey);
if (correspondence.compare(actualValue, excludedValue)) {
failWithRawMessage(
"Not true that %s does not contain an entry with key <%s> and a value that %s <%s>. "
+ "It maps that key to <%s>",
getDisplaySubject(), excludedKey, correspondence, excludedValue, actualValue);
}
}
} }


/** Fails if the map is not empty. */ /** Fails if the map is not empty. */
Expand All @@ -280,7 +286,7 @@ private Ordered containsExactly() {
} }


/** /**
* Fails if the map does not contain exactly the given set of keys mapping to values which * Fails if the map does not contain exactly the given set of keys mapping to values that
* correspond to the given values. * correspond to the given values.
* *
* <p>The values must all be of type {@code E}, and a {@link ClassCastException} will be thrown * <p>The values must all be of type {@code E}, and a {@link ClassCastException} will be thrown
Expand All @@ -297,7 +303,7 @@ private Ordered containsExactly(@Nullable Object k0, @Nullable E v0, Object... r
} }


/** /**
* Fails if the map does not contain exactly the keys in the given map, mapping to values which * Fails if the map does not contain exactly the keys in the given map, mapping to values that
* correspond to the values of the given map. * correspond to the values of the given map.
*/ */
@CanIgnoreReturnValue @CanIgnoreReturnValue
Expand Down
40 changes: 26 additions & 14 deletions core/src/main/java/com/google/common/truth/MultimapSubject.java
Expand Up @@ -341,7 +341,7 @@ private static <K, V> String countDuplicatesMultimap(Multimap<K, V> multimap) {
* <p>Note that keys will always be compared with regular object equality ({@link Object#equals}). * <p>Note that keys will always be compared with regular object equality ({@link Object#equals}).
* *
* <p>Any of the methods on the returned object may throw {@link ClassCastException} if they * <p>Any of the methods on the returned object may throw {@link ClassCastException} if they
* encounter an actual value which is not of type {@code A}. * encounter an actual value that is not of type {@code A}.
*/ */
public <A, E> UsingCorrespondence<A, E> comparingValuesUsing( public <A, E> UsingCorrespondence<A, E> comparingValuesUsing(
Correspondence<A, E> correspondence) { Correspondence<A, E> correspondence) {
Expand All @@ -365,7 +365,7 @@ private UsingCorrespondence(Correspondence<A, E> correspondence) {
} }


/** /**
* Fails if the multimap does not contain an entry with the given key and a value which * Fails if the multimap does not contain an entry with the given key and a value that
* corresponds to the given value. * corresponds to the given value.
*/ */
public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValue) { public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValue) {
Expand All @@ -380,9 +380,9 @@ public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValu
} }
// Found matching key with non-matching values. // Found matching key with non-matching values.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it contains the key <%s>, but the " "Not true that %s contains at least one entry with key <%s> and a value that %s <%s>. "
+ "values are <%s> none of which %s <%s>", + "However, it has a mapping from that key to <%s>",
getDisplaySubject(), expectedKey, actualValues, correspondence, expectedValue); getDisplaySubject(), expectedKey, correspondence, expectedValue, actualValues);
} else { } else {
// Did not find matching key. // Did not find matching key.
Set<Object> keys = new LinkedHashSet<Object>(); Set<Object> keys = new LinkedHashSet<Object>();
Expand All @@ -394,31 +394,43 @@ public void containsEntry(@Nullable Object expectedKey, @Nullable E expectedValu
if (!keys.isEmpty()) { if (!keys.isEmpty()) {
// Found matching values with non-matching keys. // Found matching values with non-matching keys.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it does not contain the key <%s>, " "Not true that %s contains at least one entry with key <%s> and a value that %s <%s>."
+ "but does contain values which %s <%s> at the following keys: <%s>", + " However, the following keys are mapped to such values: <%s>",
getDisplaySubject(), expectedKey, correspondence, expectedValue, keys); getDisplaySubject(), expectedKey, correspondence, expectedValue, keys);
} else { } else {
// Did not find matching key or value. // Did not find matching key or value.
failWithRawMessage( failWithRawMessage(
"Not true that %s contains the expected entry: it does not contain the key <%s>, " "Not true that %s contains at least one entry with key <%s> and a value that %s <%s>",
+ "and it does not contain any values which %s <%s>",
getDisplaySubject(), expectedKey, correspondence, expectedValue); getDisplaySubject(), expectedKey, correspondence, expectedValue);
} }
} }
} }


/** /**
* Fails if the multimap contains an entry with the given key and a value which corresponds to * Fails if the multimap contains an entry with the given key and a value that corresponds to
* the given value. * the given value.
*/ */
@SuppressWarnings("unused") // TODO(b/29966314): Implement this and make it public. public void doesNotContainEntry(@Nullable Object excludedKey, @Nullable E excludedValue) {
private void doesNotContainEntry(@Nullable Object key, @Nullable E value) { if (getSubject().containsKey(excludedKey)) {
throw new UnsupportedOperationException(); Collection<A> actualValues = getCastSubject().asMap().get(excludedKey);
List<A> matchingValues = new ArrayList<A>();
for (A actualValue : actualValues) {
if (correspondence.compare(actualValue, excludedValue)) {
matchingValues.add(actualValue);
}
}
if (!matchingValues.isEmpty()) {
failWithRawMessage(
"Not true that %s did not contain an entry with key <%s> and a value that %s <%s>. "
+ "It maps that key to the following such values: <%s>",
getDisplaySubject(), excludedKey, correspondence, excludedValue, matchingValues);
}
}
} }


/** /**
* Fails if the map does not contain exactly the keys in the given multimap, mapping to values * Fails if the map does not contain exactly the keys in the given multimap, mapping to values
* which correspond to the values of the given multimap. * that correspond to the values of the given multimap.
* *
* <p>A subsequent call to {@link Ordered#inOrder} may be made if the caller wishes to verify * <p>A subsequent call to {@link Ordered#inOrder} may be made if the caller wishes to verify
* that the two Multimaps iterate fully in the same order. That is, their key sets iterate in * that the two Multimaps iterate fully in the same order. That is, their key sets iterate in
Expand Down
Expand Up @@ -44,7 +44,7 @@ public boolean compare(Object actual, Object expected) {


@Override @Override
public String toString() { public String toString() {
return "example"; return "has example property";
} }
}; };


Expand Down Expand Up @@ -143,8 +143,8 @@ public void testTolerance_viaIterableSubjectContains_failure() {
} catch (AssertionError expected) { } catch (AssertionError expected) {
assertThat(expected) assertThat(expected)
.hasMessage( .hasMessage(
"Not true that <[1.02, 2.04, 3.08]> contains one or more elements that " "Not true that <[1.02, 2.04, 3.08]> contains at least one element that "
+ "are finite numbers within 0.05 of <3.0>"); + "is a finite number within 0.05 of <3.0>");
} }
} }
} }

0 comments on commit 8ef8343

Please sign in to comment.