Skip to content

Commit

Permalink
Omitting common stack frames for multiple failures when using 'Expect'.
Browse files Browse the repository at this point in the history
This approach uses a hacky way to omit the common stack frames by instantiating a new exception class with parallel exception as cause, re-using java.lang.Throwable.printStackTrace() to print, and then strip the leading full stack.

RELNOTES=Cleaner stack trace for Expect.java

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184702168
  • Loading branch information
JiangJi authored and ronshapiro committed Feb 7, 2018
1 parent e600383 commit 48b31f7
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 17 deletions.
30 changes: 25 additions & 5 deletions core/src/main/java/com/google/common/truth/Expect.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
import static com.google.common.truth.Expect.TestPhase.DURING;

import com.google.common.annotations.GwtIncompatible;
import com.google.common.truth.Truth.AssertionErrorWithCause;
import com.google.common.base.Throwables;
import com.google.common.truth.Truth.SimpleAssertionError;
import com.google.errorprone.annotations.concurrent.GuardedBy;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -116,6 +117,9 @@ synchronized boolean hasFailures() {

@Override
public synchronized String toString() {
if (failures.isEmpty()) {
return "No expectation failed.";
}
int numFailures = failures.size();
StringBuilder message =
new StringBuilder(
Expand All @@ -126,13 +130,29 @@ public synchronized String toString() {
message.append(" ");
message.append(count);
message.append(". ");
message.append(showStackTrace ? getStackTraceAsString(failure) : failure.getMessage());
if (count == 1) {
message.append(showStackTrace ? getStackTraceAsString(failure) : failure.getMessage());
} else {
message.append(
showStackTrace
? printSubsequentFailure(failures.get(0).getStackTrace(), failure)
: failure.getMessage());
}
message.append("\n");
}

return message.toString();
}

private String printSubsequentFailure(
StackTraceElement[] baseTraceFrames, AssertionError toPrint) {
Exception e = new RuntimeException("__EXCEPTION_MARKER__", toPrint);
e.setStackTrace(baseTraceFrames);
String s = Throwables.getStackTraceAsString(e);
// Force single line reluctant matching
return s.replaceFirst("(?s)^.*?__EXCEPTION_MARKER__.*?Caused by:\\s+", "");
}

@GuardedBy("this")
private void doCheckInRuleContext(@Nullable AssertionError failure) {
switch (inRuleContext) {
Expand All @@ -158,7 +178,7 @@ private void doCheckInRuleContext(@Nullable AssertionError failure) {
@GuardedBy("this")
private void doLeaveRuleContext() {
if (hasFailures()) {
throw new AssertionError(this);
throw SimpleAssertionError.createWithNoStack(this.toString());
}
}

Expand All @@ -169,8 +189,8 @@ private void doLeaveRuleContext(Throwable caught) throws Throwable {
caught instanceof AssumptionViolatedException
? "Failures occurred before an assumption was violated"
: "Failures occurred before an exception was thrown while the test was running";
record(new AssertionErrorWithCause(message + ": " + caught, caught));
throw new AssertionError(this);
record(SimpleAssertionError.createWithNoStack(message + ": " + caught, caught));
throw SimpleAssertionError.createWithNoStack(this.toString());
} else {
throw caught;
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/com/google/common/truth/ExpectFailure.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import static com.google.common.truth.StringUtil.format;

import com.google.common.annotations.GwtIncompatible;
import com.google.common.truth.Truth.AssertionErrorWithCause;
import com.google.common.truth.Truth.SimpleAssertionError;
import javax.annotation.Nullable;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
Expand Down Expand Up @@ -89,7 +89,7 @@ public ExpectFailure() {}
public StandardSubjectBuilder whenTesting() {
checkState(inRuleContext, "ExpectFailure must be used as a JUnit @Rule");
if (failure != null) {
throw new AssertionErrorWithCause("ExpectFailure already captured a failure", failure);
throw SimpleAssertionError.create("ExpectFailure already captured a failure", failure);
}
if (failureExpected) {
throw new AssertionError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.truth.Truth.AssertionErrorWithCause;
import com.google.common.truth.Truth.SimpleAssertionError;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -127,11 +127,11 @@ public String toString() {
}

void fail(String message) {
doFail(new AssertionErrorWithCause(addToMessage(message), rootCause()));
doFail(SimpleAssertionError.create(addToMessage(message), rootCause()));
}

void fail(String message, Throwable cause) {
doFail(new AssertionErrorWithCause(addToMessage(message), cause));
doFail(SimpleAssertionError.create(addToMessage(message), cause));
// TODO(cpovirk): add rootCause() as a suppressed exception?
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/google/common/truth/Platform.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private static final class ComparisonFailureWithCause extends ComparisonFailure
try {
initCause(cause);
} catch (IllegalStateException alreadyInitializedBecauseOfHarmonyBug) {
// See Truth.AssertionErrorWithCause.
// See Truth.SimpleAssertionError.
}
}

Expand Down
24 changes: 21 additions & 3 deletions core/src/main/java/com/google/common/truth/Truth.java
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,15 @@ public static AtomicLongMapSubject assertThat(@Nullable AtomicLongMap<?> actual)
return assert_().that(actual);
}

static final class AssertionErrorWithCause extends AssertionError {
/**
* An {@code AssertionError} that (a) always supports a cause, even under old versions of Android
* and (b) omits "java.lang.AssertionError:" from the beginning of its toString() representation.
*/
static final class SimpleAssertionError extends AssertionError {
/** Separate cause field, in case initCause() fails. */
private final Throwable cause;
@Nullable private final Throwable cause;

AssertionErrorWithCause(String message, Throwable cause) {
private SimpleAssertionError(String message, @Nullable Throwable cause) {
super(message);
this.cause = cause;

Expand All @@ -278,6 +282,20 @@ static final class AssertionErrorWithCause extends AssertionError {
}
}

static SimpleAssertionError create(String message, Throwable cause) {
return new SimpleAssertionError(message, cause);
}

static SimpleAssertionError createWithNoStack(String message, Throwable cause) {
SimpleAssertionError error = new SimpleAssertionError(message, cause);
error.setStackTrace(new StackTraceElement[0]);
return error;
}

static SimpleAssertionError createWithNoStack(String message) {
return createWithNoStack(message, null);
}

@Override
@SuppressWarnings("UnsynchronizedOverridesSynchronized")
public Throwable getCause() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import static java.lang.Float.parseFloat;
import static jsinterop.annotations.JsPackage.GLOBAL;

import com.google.common.truth.Truth.AssertionErrorWithCause;
import com.google.common.truth.Truth.SimpleAssertionError;
import jsinterop.annotations.JsProperty;
import jsinterop.annotations.JsType;

Expand Down Expand Up @@ -52,7 +52,7 @@ static boolean isInstanceOfType(Object instance, Class<?> clazz) {

static AssertionError comparisonFailure(
String message, String expected, String actual, Throwable cause) {
return new AssertionErrorWithCause(
return SimpleAssertionError.create(
format("%s expected:<[%s]> but was:<[%s]>", message, expected, actual), cause);
}

Expand Down
159 changes: 159 additions & 0 deletions core/src/test/java/com/google/common/truth/ExpectWithStackTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/*
* Copyright (c) 2018 Google, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.common.truth;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;

import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;
import org.junit.runner.Description;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.runners.model.Statement;

@RunWith(JUnit4.class)
public class ExpectWithStackTest {
private final Expect expectWithTrace = Expect.createAndEnableStackTrace();

@Rule public final TestRuleVerifier verifyAssertionError = new TestRuleVerifier(expectWithTrace);

@Test
public void testExpectTrace_simpleCase() {
verifyAssertionError.setErrorVerifier(
new Predicate<AssertionError>() {
@Override
public boolean apply(AssertionError expected) {
assertThat(expected.getStackTrace()).hasLength(0);
assertThat(expected).hasMessageThat().startsWith("3 expectations failed:");
return true;
}
});

expectWithTrace.that(true).isFalse();
expectWithTrace.that("Hello").isNull();
expectWithTrace.that(1).isEqualTo(2);
}

@Test
public void testExpectTrace_loop() {
verifyAssertionError.setErrorVerifier(
new Predicate<AssertionError>() {
@Override
public boolean apply(AssertionError expected) {
assertThat(expected.getStackTrace()).hasLength(0);
assertThat(expected).hasMessageThat().startsWith("4 expectations failed:");
assertWithMessage("test method name should only show up once with following omitted")
.that(expected.getMessage().split("testExpectTrace_loop"))
.hasLength(2);
return true;
}
});

for (int i = 0; i < 4; i++) {
expectWithTrace.that(true).isFalse();
}
}

@Test
public void testExpectTrace_callerException() {
verifyAssertionError.setErrorVerifier(
new Predicate<AssertionError>() {
@Override
public boolean apply(AssertionError expected) {
assertThat(expected.getStackTrace()).hasLength(0);
assertThat(expected).hasMessageThat().startsWith("2 expectations failed:");
return true;
}
});

expectWithTrace.that(true).isFalse();
expectWithTrace
.that(alwaysFailWithCause(getFirstException("First", getSecondException("Second", null))))
.isEqualTo(5);
}

@Test
public void testExpectTrace_onlyCallerException() {
verifyAssertionError.setErrorVerifier(
new Predicate<AssertionError>() {
@Override
public boolean apply(AssertionError expected) {
assertWithMessage("Should throw exception as it is if only caller exception")
.that(expected.getStackTrace().length)
.isAtLeast(2);
return true;
}
});

expectWithTrace
.that(alwaysFailWithCause(getFirstException("First", getSecondException("Second", null))))
.isEqualTo(5);
}

private static long alwaysFailWithCause(Throwable throwable) {
throw new AssertionError("Always fail", throwable);
}

private static Exception getFirstException(String messsage, Throwable cause) {
if (cause != null) {
return new RuntimeException(messsage, cause);
} else {
return new RuntimeException(messsage);
}
}

private static Exception getSecondException(String messsage, Throwable cause) {
if (cause != null) {
return new RuntimeException(messsage, cause);
} else {
return new RuntimeException(messsage);
}
}

private static class TestRuleVerifier implements TestRule {
protected Predicate<AssertionError> errorVerifier = Predicates.alwaysFalse();

private final TestRule ruleToVerify;

public TestRuleVerifier(TestRule ruleToVerify) {
this.ruleToVerify = ruleToVerify;
}

public void setErrorVerifier(Predicate<AssertionError> verifier) {
this.errorVerifier = verifier;
}

@Override
public Statement apply(final Statement base, final Description description) {
return new Statement() {
@Override
public void evaluate() throws Throwable {
try {
ruleToVerify.apply(base, description).evaluate();
} catch (AssertionError caught) {
if (!errorVerifier.apply(caught)) {
throw new AssertionError("Caught error doesn't meet expectation", caught);
}
}
}
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void collapseStreaks() {
* Stripping doesn't actually work under j2cl (and presumably GWT):
* StackTraceElement.getClassName() doesn't have real data. Some data is available in toString(),
* albeit along the lines of
* "AssertionErrorWithCause.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner
* "SimpleAssertionError.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner
* could maybe look through the toString() representations to count how many frames to strip, but
* that's a bigger project. (While we're at it, we could strip the j2cl-specific boilerplate from
* the _bottom_ of the stack, too.) And sadly, it's not necessarily as simple as looking at just
Expand Down

0 comments on commit 48b31f7

Please sign in to comment.