Skip to content

Commit

Permalink
Throw a plain AssertionError if ComparisonFailure is unavailable, for…
Browse files Browse the repository at this point in the history
… real this time.

This should *really* let users exclude the JUnit 4 dependency -- again, as long as they don't use Expect, ExpectFailure, or TruthJUnit (i.e., assume()).

This change makes the previously failing test procedure described in #333 (comment) pass.

RELNOTES=Made it possible for users to exclude our JUnit 4 dependency and still use standard Truth assertions -- *really* this time. (JUnit 4 is still required for some advanced features, like `Expect`, `ExpectFailure`, and `TruthJUnit.assume()`.)
PiperOrigin-RevId: 353318639
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jan 23, 2021
1 parent 6b01407 commit 16ee13a
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 31 deletions.
36 changes: 11 additions & 25 deletions core/src/main/java/com/google/common/truth/FailureMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import static com.google.common.truth.LazyMessage.evaluateAll;
import static com.google.common.truth.Platform.cleanStackTrace;
import static com.google.common.truth.Platform.inferDescription;
import static com.google.common.truth.Platform.isLinkageError;
import static com.google.common.truth.Platform.makeComparisonFailure;
import static com.google.common.truth.SubjectUtils.append;
import static com.google.common.truth.SubjectUtils.concat;

Expand Down Expand Up @@ -171,31 +171,17 @@ void failEqualityCheck(
ImmutableList<Fact> tailFacts,
String expected,
String actual) {
ImmutableList<String> messages = evaluateAll(this.messages);
ImmutableList<Fact> facts =
makeComparisonFailureFacts(
concat(description(), headFacts),
concat(tailFacts, rootUnlessThrowable()),
doFail(
makeComparisonFailure(
evaluateAll(messages),
makeComparisonFailureFacts(
concat(description(), headFacts),
concat(tailFacts, rootUnlessThrowable()),
expected,
actual),
expected,
actual);
Throwable cause = rootCause();

/*
* We perform as much work as possible outside the `try`. That way, we minimize the chance that
* we'll catch and swallow a LinkageError from any problem except the specific problem that
* JUnit is not on the classpath.
*/
AssertionError failure;
try {
failure = new ComparisonFailureWithFacts(messages, facts, expected, actual, cause);
} catch (Error probablyJunitNotOnClasspath) {
// We can't catch LinkageError directly because of GWT/j2cl.
if (!isLinkageError(probablyJunitNotOnClasspath)) {
throw probablyJunitNotOnClasspath;
}
failure = new AssertionErrorWithFacts(messages, facts, cause);
}
doFail(failure);
actual,
rootCause()));
}

void fail(ImmutableList<Fact> facts) {
Expand Down
64 changes: 62 additions & 2 deletions core/src/main/java/com/google/common/truth/Platform.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@
*/
package com.google.common.truth;

import static com.google.common.base.Throwables.throwIfUnchecked;
import static com.google.common.truth.DiffUtils.generateUnifiedDiff;
import static com.google.common.truth.Fact.fact;

import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.List;
Expand Down Expand Up @@ -231,7 +233,65 @@ private static boolean isInferDescriptionDisabled() {
}
}

static boolean isLinkageError(Error e) {
return e instanceof LinkageError;
static AssertionError makeComparisonFailure(
ImmutableList<String> messages,
ImmutableList<Fact> facts,
String expected,
String actual,
@Nullable Throwable cause) {
Class<?> comparisonFailureClass;
try {
comparisonFailureClass = Class.forName("com.google.common.truth.ComparisonFailureWithFacts");
} catch (LinkageError | ClassNotFoundException probablyJunitNotOnClasspath) {
/*
* LinkageError makes sense, but ClassNotFoundException shouldn't happen:
* ComparisonFailureWithFacts should be there, even if its JUnit 4 dependency is not. But it's
* harmless to catch an "impossible" exception, and if someone decides to strip the class out
* (perhaps along with Platform.PlatformComparisonFailure, to satisfy a tool that is unhappy
* because it can't find the latter's superclass because JUnit 4 is also missing?), presumably
* we should still fall back to a plain AssertionError.
*
* TODO(cpovirk): Consider creating and using yet another class like AssertionErrorWithFacts,
* not actually extending ComparisonFailure but still exposing getExpected() and getActual()
* methods.
*/
return new AssertionErrorWithFacts(messages, facts, cause);
}
Class<? extends AssertionError> asAssertionErrorSubclass =
comparisonFailureClass.asSubclass(AssertionError.class);

Constructor<? extends AssertionError> constructor;
try {
constructor =
asAssertionErrorSubclass.getDeclaredConstructor(
ImmutableList.class,
ImmutableList.class,
String.class,
String.class,
Throwable.class);
} catch (NoSuchMethodException e) {
// That constructor exists.
throw newLinkageError(e);
}

try {
return constructor.newInstance(messages, facts, expected, actual, cause);
} catch (InvocationTargetException e) {
throwIfUnchecked(e.getCause());
// That constructor has no `throws` clause.
throw newLinkageError(e);
} catch (InstantiationException e) {
// The class is a concrete class.
throw newLinkageError(e);
} catch (IllegalAccessException e) {
// We're accessing a class from within its package.
throw newLinkageError(e);
}
}

private static LinkageError newLinkageError(Throwable cause) {
LinkageError error = new LinkageError(cause.toString());
error.initCause(cause);
return error;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ static boolean isInstanceOfType(Object instance, Class<?> clazz) {
return false;
}

static boolean isLinkageError(Error e) {
return false;
}

abstract static class PlatformComparisonFailure extends AssertionError {
PlatformComparisonFailure(
String message,
Expand Down Expand Up @@ -214,4 +210,27 @@ public boolean getUseGrouping() {
return false;
}
}

static AssertionError makeComparisonFailure(
ImmutableList<String> messages,
ImmutableList<Fact> facts,
String expected,
String actual,
@Nullable Throwable cause) {
/*
* Despite the name, the class we're creating extends AssertionError but not ComparisonFailure
* under GWT: See its supertype, PlatformComparisonFailure, above.
*
* We're actually creating the same class as the non-GWT version of this method does. So why do
* we have supersource for this method? It's because we can't run (and, fortunately, don't need
* to run) the reflective code we have for non-GWT users, who might or might not choose to
* exclude JUnit 4 from their classpath.
*
* TODO(cpovirk): Remove ComparisonFailureWithFacts and PlatformComparisonFailure entirely under
* GWT? That would let us merge them into a single class on the server. And as noted in the
* non-GWT copy of Platform, we could consider another custom type that exposes getExpected()
* and getActual(), even in the absence of ComparisonFailure. That type would work under GWT.
*/
return new ComparisonFailureWithFacts(messages, facts, expected, actual, cause);
}
}

0 comments on commit 16ee13a

Please sign in to comment.