From 948f3edca4dd788dd20065a6582561d510471578 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Sat, 23 Jan 2021 08:30:43 -0800 Subject: [PATCH] Throw a plain AssertionError if ComparisonFailure is unavailable, for 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 https://github.com/google/truth/issues/333#issuecomment-765616282 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: 353416036 --- .../google/common/truth/FailureMetadata.java | 36 ++++------- .../com/google/common/truth/Platform.java | 64 ++++++++++++++++++- .../com/google/common/truth/Platform.java | 27 ++++++-- 3 files changed, 96 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/com/google/common/truth/FailureMetadata.java b/core/src/main/java/com/google/common/truth/FailureMetadata.java index b8dbc3cd0..00c890cfd 100644 --- a/core/src/main/java/com/google/common/truth/FailureMetadata.java +++ b/core/src/main/java/com/google/common/truth/FailureMetadata.java @@ -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; @@ -171,31 +171,17 @@ void failEqualityCheck( ImmutableList tailFacts, String expected, String actual) { - ImmutableList messages = evaluateAll(this.messages); - ImmutableList 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 facts) { diff --git a/core/src/main/java/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/Platform.java index 7515e442d..698e4bd2a 100644 --- a/core/src/main/java/com/google/common/truth/Platform.java +++ b/core/src/main/java/com/google/common/truth/Platform.java @@ -15,6 +15,7 @@ */ 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; @@ -22,6 +23,7 @@ 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; @@ -231,7 +233,65 @@ private static boolean isInferDescriptionDisabled() { } } - static boolean isLinkageError(Error e) { - return e instanceof LinkageError; + static AssertionError makeComparisonFailure( + ImmutableList messages, + ImmutableList 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 asAssertionErrorSubclass = + comparisonFailureClass.asSubclass(AssertionError.class); + + Constructor 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; } } diff --git a/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java b/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java index 1519b9515..ed2eb2c47 100644 --- a/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java +++ b/core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java @@ -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, @@ -214,4 +210,27 @@ public boolean getUseGrouping() { return false; } } + + static AssertionError makeComparisonFailure( + ImmutableList messages, + ImmutableList 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); + } }