Skip to content

Commit 48b31f7

Browse files
JiangJironshapiro
JiangJi
authored and
ronshapiro
committed
Omitting common stack frames for multiple failures when using 'Expect'.
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
1 parent e600383 commit 48b31f7

File tree

8 files changed

+214
-17
lines changed

8 files changed

+214
-17
lines changed

‎core/src/main/java/com/google/common/truth/Expect.java‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
import static com.google.common.truth.Expect.TestPhase.DURING;
2424

2525
import com.google.common.annotations.GwtIncompatible;
26-
import com.google.common.truth.Truth.AssertionErrorWithCause;
26+
import com.google.common.base.Throwables;
27+
import com.google.common.truth.Truth.SimpleAssertionError;
2728
import com.google.errorprone.annotations.concurrent.GuardedBy;
2829
import java.util.ArrayList;
2930
import java.util.List;
@@ -116,6 +117,9 @@ synchronized boolean hasFailures() {
116117

117118
@Override
118119
public synchronized String toString() {
120+
if (failures.isEmpty()) {
121+
return "No expectation failed.";
122+
}
119123
int numFailures = failures.size();
120124
StringBuilder message =
121125
new StringBuilder(
@@ -126,13 +130,29 @@ public synchronized String toString() {
126130
message.append(" ");
127131
message.append(count);
128132
message.append(". ");
129-
message.append(showStackTrace ? getStackTraceAsString(failure) : failure.getMessage());
133+
if (count == 1) {
134+
message.append(showStackTrace ? getStackTraceAsString(failure) : failure.getMessage());
135+
} else {
136+
message.append(
137+
showStackTrace
138+
? printSubsequentFailure(failures.get(0).getStackTrace(), failure)
139+
: failure.getMessage());
140+
}
130141
message.append("\n");
131142
}
132143

133144
return message.toString();
134145
}
135146

147+
private String printSubsequentFailure(
148+
StackTraceElement[] baseTraceFrames, AssertionError toPrint) {
149+
Exception e = new RuntimeException("__EXCEPTION_MARKER__", toPrint);
150+
e.setStackTrace(baseTraceFrames);
151+
String s = Throwables.getStackTraceAsString(e);
152+
// Force single line reluctant matching
153+
return s.replaceFirst("(?s)^.*?__EXCEPTION_MARKER__.*?Caused by:\\s+", "");
154+
}
155+
136156
@GuardedBy("this")
137157
private void doCheckInRuleContext(@Nullable AssertionError failure) {
138158
switch (inRuleContext) {
@@ -158,7 +178,7 @@ private void doCheckInRuleContext(@Nullable AssertionError failure) {
158178
@GuardedBy("this")
159179
private void doLeaveRuleContext() {
160180
if (hasFailures()) {
161-
throw new AssertionError(this);
181+
throw SimpleAssertionError.createWithNoStack(this.toString());
162182
}
163183
}
164184

@@ -169,8 +189,8 @@ private void doLeaveRuleContext(Throwable caught) throws Throwable {
169189
caught instanceof AssumptionViolatedException
170190
? "Failures occurred before an assumption was violated"
171191
: "Failures occurred before an exception was thrown while the test was running";
172-
record(new AssertionErrorWithCause(message + ": " + caught, caught));
173-
throw new AssertionError(this);
192+
record(SimpleAssertionError.createWithNoStack(message + ": " + caught, caught));
193+
throw SimpleAssertionError.createWithNoStack(this.toString());
174194
} else {
175195
throw caught;
176196
}

‎core/src/main/java/com/google/common/truth/ExpectFailure.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import static com.google.common.truth.StringUtil.format;
2121

2222
import com.google.common.annotations.GwtIncompatible;
23-
import com.google.common.truth.Truth.AssertionErrorWithCause;
23+
import com.google.common.truth.Truth.SimpleAssertionError;
2424
import javax.annotation.Nullable;
2525
import org.junit.runner.Description;
2626
import org.junit.runners.model.Statement;
@@ -89,7 +89,7 @@ public ExpectFailure() {}
8989
public StandardSubjectBuilder whenTesting() {
9090
checkState(inRuleContext, "ExpectFailure must be used as a JUnit @Rule");
9191
if (failure != null) {
92-
throw new AssertionErrorWithCause("ExpectFailure already captured a failure", failure);
92+
throw SimpleAssertionError.create("ExpectFailure already captured a failure", failure);
9393
}
9494
if (failureExpected) {
9595
throw new AssertionError(

‎core/src/main/java/com/google/common/truth/FailureMetadata.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import com.google.common.annotations.VisibleForTesting;
2424
import com.google.common.base.Joiner;
2525
import com.google.common.collect.ImmutableList;
26-
import com.google.common.truth.Truth.AssertionErrorWithCause;
26+
import com.google.common.truth.Truth.SimpleAssertionError;
2727
import javax.annotation.Nullable;
2828

2929
/**
@@ -127,11 +127,11 @@ public String toString() {
127127
}
128128

129129
void fail(String message) {
130-
doFail(new AssertionErrorWithCause(addToMessage(message), rootCause()));
130+
doFail(SimpleAssertionError.create(addToMessage(message), rootCause()));
131131
}
132132

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

‎core/src/main/java/com/google/common/truth/Platform.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ private static final class ComparisonFailureWithCause extends ComparisonFailure
9999
try {
100100
initCause(cause);
101101
} catch (IllegalStateException alreadyInitializedBecauseOfHarmonyBug) {
102-
// See Truth.AssertionErrorWithCause.
102+
// See Truth.SimpleAssertionError.
103103
}
104104
}
105105

‎core/src/main/java/com/google/common/truth/Truth.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,15 @@ public static AtomicLongMapSubject assertThat(@Nullable AtomicLongMap<?> actual)
260260
return assert_().that(actual);
261261
}
262262

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

267-
AssertionErrorWithCause(String message, Throwable cause) {
271+
private SimpleAssertionError(String message, @Nullable Throwable cause) {
268272
super(message);
269273
this.cause = cause;
270274

@@ -278,6 +282,20 @@ static final class AssertionErrorWithCause extends AssertionError {
278282
}
279283
}
280284

285+
static SimpleAssertionError create(String message, Throwable cause) {
286+
return new SimpleAssertionError(message, cause);
287+
}
288+
289+
static SimpleAssertionError createWithNoStack(String message, Throwable cause) {
290+
SimpleAssertionError error = new SimpleAssertionError(message, cause);
291+
error.setStackTrace(new StackTraceElement[0]);
292+
return error;
293+
}
294+
295+
static SimpleAssertionError createWithNoStack(String message) {
296+
return createWithNoStack(message, null);
297+
}
298+
281299
@Override
282300
@SuppressWarnings("UnsynchronizedOverridesSynchronized")
283301
public Throwable getCause() {

‎core/src/main/java/com/google/common/truth/super/com/google/common/truth/Platform.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import static java.lang.Float.parseFloat;
2121
import static jsinterop.annotations.JsPackage.GLOBAL;
2222

23-
import com.google.common.truth.Truth.AssertionErrorWithCause;
23+
import com.google.common.truth.Truth.SimpleAssertionError;
2424
import jsinterop.annotations.JsProperty;
2525
import jsinterop.annotations.JsType;
2626

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

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

‎core/src/test/java/com/google/common/truth/ExpectWithStackTest.java‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/*
2+
* Copyright (c) 2018 Google, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.common.truth;
17+
18+
import static com.google.common.truth.Truth.assertThat;
19+
import static com.google.common.truth.Truth.assertWithMessage;
20+
21+
import com.google.common.base.Predicate;
22+
import com.google.common.base.Predicates;
23+
import org.junit.Rule;
24+
import org.junit.Test;
25+
import org.junit.rules.TestRule;
26+
import org.junit.runner.Description;
27+
import org.junit.runner.RunWith;
28+
import org.junit.runners.JUnit4;
29+
import org.junit.runners.model.Statement;
30+
31+
@RunWith(JUnit4.class)
32+
public class ExpectWithStackTest {
33+
private final Expect expectWithTrace = Expect.createAndEnableStackTrace();
34+
35+
@Rule public final TestRuleVerifier verifyAssertionError = new TestRuleVerifier(expectWithTrace);
36+
37+
@Test
38+
public void testExpectTrace_simpleCase() {
39+
verifyAssertionError.setErrorVerifier(
40+
new Predicate<AssertionError>() {
41+
@Override
42+
public boolean apply(AssertionError expected) {
43+
assertThat(expected.getStackTrace()).hasLength(0);
44+
assertThat(expected).hasMessageThat().startsWith("3 expectations failed:");
45+
return true;
46+
}
47+
});
48+
49+
expectWithTrace.that(true).isFalse();
50+
expectWithTrace.that("Hello").isNull();
51+
expectWithTrace.that(1).isEqualTo(2);
52+
}
53+
54+
@Test
55+
public void testExpectTrace_loop() {
56+
verifyAssertionError.setErrorVerifier(
57+
new Predicate<AssertionError>() {
58+
@Override
59+
public boolean apply(AssertionError expected) {
60+
assertThat(expected.getStackTrace()).hasLength(0);
61+
assertThat(expected).hasMessageThat().startsWith("4 expectations failed:");
62+
assertWithMessage("test method name should only show up once with following omitted")
63+
.that(expected.getMessage().split("testExpectTrace_loop"))
64+
.hasLength(2);
65+
return true;
66+
}
67+
});
68+
69+
for (int i = 0; i < 4; i++) {
70+
expectWithTrace.that(true).isFalse();
71+
}
72+
}
73+
74+
@Test
75+
public void testExpectTrace_callerException() {
76+
verifyAssertionError.setErrorVerifier(
77+
new Predicate<AssertionError>() {
78+
@Override
79+
public boolean apply(AssertionError expected) {
80+
assertThat(expected.getStackTrace()).hasLength(0);
81+
assertThat(expected).hasMessageThat().startsWith("2 expectations failed:");
82+
return true;
83+
}
84+
});
85+
86+
expectWithTrace.that(true).isFalse();
87+
expectWithTrace
88+
.that(alwaysFailWithCause(getFirstException("First", getSecondException("Second", null))))
89+
.isEqualTo(5);
90+
}
91+
92+
@Test
93+
public void testExpectTrace_onlyCallerException() {
94+
verifyAssertionError.setErrorVerifier(
95+
new Predicate<AssertionError>() {
96+
@Override
97+
public boolean apply(AssertionError expected) {
98+
assertWithMessage("Should throw exception as it is if only caller exception")
99+
.that(expected.getStackTrace().length)
100+
.isAtLeast(2);
101+
return true;
102+
}
103+
});
104+
105+
expectWithTrace
106+
.that(alwaysFailWithCause(getFirstException("First", getSecondException("Second", null))))
107+
.isEqualTo(5);
108+
}
109+
110+
private static long alwaysFailWithCause(Throwable throwable) {
111+
throw new AssertionError("Always fail", throwable);
112+
}
113+
114+
private static Exception getFirstException(String messsage, Throwable cause) {
115+
if (cause != null) {
116+
return new RuntimeException(messsage, cause);
117+
} else {
118+
return new RuntimeException(messsage);
119+
}
120+
}
121+
122+
private static Exception getSecondException(String messsage, Throwable cause) {
123+
if (cause != null) {
124+
return new RuntimeException(messsage, cause);
125+
} else {
126+
return new RuntimeException(messsage);
127+
}
128+
}
129+
130+
private static class TestRuleVerifier implements TestRule {
131+
protected Predicate<AssertionError> errorVerifier = Predicates.alwaysFalse();
132+
133+
private final TestRule ruleToVerify;
134+
135+
public TestRuleVerifier(TestRule ruleToVerify) {
136+
this.ruleToVerify = ruleToVerify;
137+
}
138+
139+
public void setErrorVerifier(Predicate<AssertionError> verifier) {
140+
this.errorVerifier = verifier;
141+
}
142+
143+
@Override
144+
public Statement apply(final Statement base, final Description description) {
145+
return new Statement() {
146+
@Override
147+
public void evaluate() throws Throwable {
148+
try {
149+
ruleToVerify.apply(base, description).evaluate();
150+
} catch (AssertionError caught) {
151+
if (!errorVerifier.apply(caught)) {
152+
throw new AssertionError("Caught error doesn't meet expectation", caught);
153+
}
154+
}
155+
}
156+
};
157+
}
158+
}
159+
}

‎core/src/test/java/com/google/common/truth/StackTraceCleanerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void collapseStreaks() {
6464
* Stripping doesn't actually work under j2cl (and presumably GWT):
6565
* StackTraceElement.getClassName() doesn't have real data. Some data is available in toString(),
6666
* albeit along the lines of
67-
* "AssertionErrorWithCause.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner
67+
* "SimpleAssertionError.m_createError__java_lang_String_$pp_java_lang." StackTraceCleaner
6868
* could maybe look through the toString() representations to count how many frames to strip, but
6969
* that's a bigger project. (While we're at it, we could strip the j2cl-specific boilerplate from
7070
* the _bottom_ of the stack, too.) And sadly, it's not necessarily as simple as looking at just

0 commit comments

Comments
 (0)