Skip to content

Commit

Permalink
(#160) Improve messages in Assertion and Mismatches
Browse files Browse the repository at this point in the history
  • Loading branch information
victornoel committed Feb 14, 2021
1 parent ce70d22 commit 110d404
Show file tree
Hide file tree
Showing 26 changed files with 129 additions and 144 deletions.
13 changes: 6 additions & 7 deletions src/main/java/org/llorllale/cactoos/matchers/Assertion.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@
* @todo #18:30min Replace all uses of MatcherAssert.assertThat() with
* Assertion, and ban all overloads of the former in forbidden-apis.txt.
* We should also look into banning common matchers like Matchers.is(), etc.
* @todo #67:30min Assertion should rely on mismatch description when forming
* error message (as assertThat does). Currently 'was' is being used by this
* class as part of description, it causes duplication of the word with
* Matchers derived from BaseMatcher, such as IsEqual.
*/
public final class Assertion<T> {

Expand Down Expand Up @@ -109,10 +105,13 @@ public Assertion(
public void affirm() throws AssertionError {
if (!this.matcher.matches(this.test)) {
final Description text = new StringDescription();
text.appendText(this.msg)
.appendText(String.format("%nExpected: "))
text
.appendText(this.msg)
.appendText(System.lineSeparator())
.appendText("Expected: ")
.appendDescriptionOf(this.matcher)
.appendText(String.format("%n but was: "));
.appendText(System.lineSeparator())
.appendText(" but: ");
this.matcher.describeMismatch(this.test, text);
throw new AssertionError(text.toString());
}
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/org/llorllale/cactoos/matchers/HasLines.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,12 @@ private HasLines(
super(
new MatcherOf<>(
actual -> match.apply(split.apply(actual), expected),
desc -> desc.appendText("Lines are ").appendValue(expected),
(actual, desc) -> desc.appendValue(split.apply(actual))
desc -> desc
.appendText("lines are ")
.appendValue(expected),
(actual, desc) -> desc
.appendText("lines were ")
.appendValue(split.apply(actual))
)
);
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/llorllale/cactoos/matchers/HasValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ public HasValues(final Iterable<X> expected) {
actual -> new ListOf<>(actual).containsAll(new ListOf<>(expected)),
desc -> desc.appendText("contains ")
.appendValue(new TextOf(expected)),
(actual, desc) -> desc.appendValue(new TextOf(actual))
(actual, desc) -> desc
.appendText("was ")
.appendValue(actual)
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@

import org.cactoos.Func;
import org.cactoos.scalar.Or;
import org.cactoos.text.FormattedText;
import org.cactoos.text.TextOf;

/**
* Matcher to check that {@link Iterable} has elements matched by particular
Expand Down Expand Up @@ -57,13 +55,10 @@ public HasValuesMatching(final Func<X, Boolean> fnc) {
super(
new MatcherOf<>(
actual -> new Or(fnc, actual).value(),
desc -> desc.appendText("The function matches at least 1 element."),
(actual, desc) -> desc.appendText(
new FormattedText(
"No any elements from [%s] matches by the function",
new TextOf(actual)
).asString()
)
desc -> desc.appendText("matches at least 1 element"),
(actual, desc) -> desc
.appendText("no match in ")
.appendValue(actual)
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ public IsApplicable(final X input, final Matcher<Y> mtr) {
func -> mtr.matches(
new UncheckedFunc<>(func).apply(input)
),
desc -> desc.appendText("Func with ")
desc -> desc
.appendText("Func output matches ")
.appendDescriptionOf(mtr),
(func, desc) -> desc.appendText("Func with ")
(func, desc) -> desc
.appendText("Func returned ")
.appendValue(func.apply(input))
)
);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/llorllale/cactoos/matchers/IsBlank.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public IsBlank() {
new MatcherOf<>(
text -> text.trim().isEmpty(),
desc -> desc.appendText("is blank"),
(text, desc) -> desc.appendValue(text)
(text, desc) -> desc.appendText("was ").appendValue(text)
)
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/llorllale/cactoos/matchers/IsTrue.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public IsTrue() {
new MatcherOf<>(
bool -> bool,
desc -> desc.appendValue(true),
(bool, desc) -> desc.appendValue(bool)
(bool, desc) -> desc.appendText("was ").appendValue(bool)
)
);
}
Expand Down
46 changes: 25 additions & 21 deletions src/main/java/org/llorllale/cactoos/matchers/Mismatches.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
package org.llorllale.cactoos.matchers;

import org.cactoos.Text;
import org.cactoos.text.Concatenated;
import org.cactoos.text.FormattedText;
import org.cactoos.text.Joined;
import org.cactoos.text.TextOf;
Expand Down Expand Up @@ -54,24 +55,20 @@
* @param <X> Type of item.
* @param <M> Type of tested matcher.
* @since 1.0.0
* @todo #106:30min Add extra tests for this class to validate all
* the different constructors (also add a constructor taking message
* as a String) and ensure they are coherent with how Assertion is
* working and throwing errors.
* @checkstyle ProtectedMethodInFinalClassCheck (200 lines)
*/
public final class Mismatches<X, M extends Matcher<X>> extends
TypeSafeDiagnosingMatcher<M> {

/**
* Expected key word.
* Multiline start.
*/
private static final String EXPECTED = "Expected: ";
private static final String ML_START = "<<<";

/**
* But was key word.
* Multiline end.
*/
private static final String BUT_WAS = " but was: ";
private static final String ML_END = ">>>";

/**
* The testing arguments for the target matcher.
Expand Down Expand Up @@ -113,8 +110,8 @@ public Mismatches(
new Joined(
new FormattedText("%n"),
new TextOf(""),
new Joined(new TextOf(""), new TextOf(Mismatches.EXPECTED), expected),
new Joined(new TextOf(""), new TextOf(Mismatches.BUT_WAS), actual)
new Concatenated(new TextOf("Expected: "), expected),
new Concatenated(new TextOf(" but: "), actual)
)
);
}
Expand All @@ -124,18 +121,22 @@ public Mismatches(
* @param args The testing arguments for the matcher.
* @param message The expected mismatch message.
*/
public Mismatches(final X args, final Text message) {
private Mismatches(final X args, final Text message) {
super();
this.args = args;
this.message = message;
}

@Override
public void describeTo(final Description desc) {
desc.appendText("Mismatches ")
desc.appendText("mismatches ")
.appendValue(this.args)
.appendText(" with message ")
.appendValue(this.message);
.appendText(" with message")
.appendText(System.lineSeparator())
.appendText(Mismatches.ML_START)
.appendText(new UncheckedText(this.message).asString())
.appendText(System.lineSeparator())
.appendText(Mismatches.ML_END);
}

@Override
Expand All @@ -144,18 +145,21 @@ protected boolean matchesSafely(final M matcher, final Description dsc) {
try {
new Assertion<>("", this.args, matcher).affirm();
mismatch = false;
dsc.appendText(String.format("%n"));
dsc.appendText(Mismatches.EXPECTED);
matcher.describeTo(dsc);
dsc.appendText(String.format("%n"));
dsc.appendText(Mismatches.BUT_WAS);
matcher.describeMismatch(this.args, dsc);
dsc
.appendText("matched ")
.appendValue(this.args);
} catch (final AssertionError err) {
mismatch = err.getMessage().equals(
new UncheckedText(this.message).asString()
);
if (!mismatch) {
dsc.appendText(err.getMessage());
dsc
.appendText("mismatched with message")
.appendText(System.lineSeparator())
.appendText(Mismatches.ML_START)
.appendText(err.getMessage())
.appendText(System.lineSeparator())
.appendText(Mismatches.ML_END);
}
}
return mismatch;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public boolean matchesSafely(
service.shutdown();
if (matching != this.total) {
desc
.appendText("Ran successfuly in ")
.appendText("ran successfuly in ")
.appendValue(matching)
.appendText(" threads");
}
Expand All @@ -123,7 +123,7 @@ public boolean matchesSafely(
@Override
public void describeTo(final Description description) {
description
.appendText("Runs in ")
.appendText("runs in ")
.appendValue(this.total)
.appendText(" threads successfuly");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public TextMatcher(
final BiFunc<String, String, Boolean> func,
final String expected
) {
this(text, func, expected, "Text with value");
this(text, func, expected, "was Text with value");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void mismatches() {
new Mismatches<>(
new TextOf("The sentence."),
"Text ending with \"!\"",
"Text with value \"The sentence.\""
"was Text with value \"The sentence.\""
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ void failed() {
new HasLines("Tom", "Mike"),
new Mismatches<>(
String.format("Tom%nJohn%n"),
"Lines are <[Tom, Mike]>",
"<[Tom, John]>"
"lines are <[Tom, Mike]>",
"lines were <[Tom, John]>"
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void mismatch() {
new Mismatches<>(
new TextOf("abc123"),
"Text containing \"xyz456\"",
"Text with value \"abc123\""
"was Text with value \"abc123\""
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ void mismatches() {
new HasValuesMatching<>(value -> value > 5),
new Mismatches<>(
new ListOf<>(1, 2, 3),
"The function matches at least 1 element.",
"No any elements from [1, 2, 3] matches by the function"
"matches at least 1 element",
"no match in <[1, 2, 3]>"
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void mismatches() {
new Mismatches<>(
new ListOf<>(1, 2, 3),
"contains <5>",
"<1, 2, 3>"
"was <[1, 2, 3]>"
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ void mismatches() {
new IsApplicable<>(1, 1),
new Mismatches<>(
new FuncOf<>(x -> 3 * x),
"Func with <1>",
"Func with <3>"
"Func output matches <1>",
"Func returned <3>"
)
).affirm();
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/llorllale/cactoos/matchers/IsBlankTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void notBlank() {
new Mismatches<>(
"-.$%",
"is blank",
"\"-.$%\""
"was \"-.$%\""
)
).affirm();
}
Expand All @@ -67,7 +67,7 @@ void nonBlankMessage() {
new Mismatches<>(
"text",
"is blank",
"\"text\""
"was \"text\""
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void noMatch() {
new Mismatches<>(
new TextOf("abcd"),
"Text with value \"xyz\"",
"Text with value \"abcd\""
"was Text with value \"abcd\""
)
).affirm();
}
Expand Down
15 changes: 1 addition & 14 deletions src/test/java/org/llorllale/cactoos/matchers/IsTrueTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

package org.llorllale.cactoos.matchers;

import org.hamcrest.core.IsNot;
import org.junit.jupiter.api.Test;

/**
Expand All @@ -49,18 +48,6 @@ void matchPositive() {
).affirm();
}

/**
* Give the negative testing result for the invalid arguments.
*/
@Test
void matchNegative() {
new Assertion<>(
"mismatches 'false'",
new IsTrue(),
new IsNot<>(new Matches<>(false))
).affirm();
}

@Test
void describesCorrectly() {
new Assertion<>(
Expand All @@ -69,7 +56,7 @@ void describesCorrectly() {
new Mismatches<>(
false,
"<true>",
"<false>"
"was <false>"
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*/
package org.llorllale.cactoos.matchers;

import org.cactoos.text.Joined;
import org.cactoos.text.Concatenated;
import org.junit.jupiter.api.Test;

/**
Expand Down Expand Up @@ -82,8 +82,8 @@ void mismatches() {
),
new Mismatches<>(
provided,
new Joined("", "<", expected.toString(), ">"),
new Joined("", "<", provided.toString(), ">")
new Concatenated("<", expected.toString(), ">"),
new Concatenated("<", provided.toString(), ">")
)
).affirm();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void mismatchesFromMatcher() {
new Mismatches<>(
new TextOf("b"),
"Text with value \"a\" runs in less than <1000L> milliseconds",
"Text with value \"b\""
"was Text with value \"b\""
)
).affirm();
}
Expand Down

0 comments on commit 110d404

Please sign in to comment.