Skip to content

Commit 9b4627b

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Skip bytecode inference of the actual value for assume.
To do so, I've generalized our existing support for skipping bytecode inference, which I had originally set up for `ExpectFailure` in 3caa0e8 + 24b5a31. This change improves the performance of failed assumptions. It makes the messages of the resulting `AssumptionViolatedException` somewhat worse, but my sense is that no one really looks at those. As discussed in b/478281659, we could go further by also avoiding cleaning stack traces, which probably also no one looks at. RELNOTES=n/a PiperOrigin-RevId: 860538616
1 parent feb3bee commit 9b4627b

File tree

8 files changed

+73
-67
lines changed

8 files changed

+73
-67
lines changed

core/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@
145145
<include>**/ActualValueInferenceTest.java</include>
146146
</includes>
147147
<systemPropertyVariables>
148-
<com.google.common.truth.enable_infer_description_for_expect_failure>true</com.google.common.truth.enable_infer_description_for_expect_failure>
148+
<com.google.common.truth.force_infer_description>true</com.google.common.truth.force_infer_description>
149149
</systemPropertyVariables>
150150
</configuration>
151151
<goals>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ public static Expect create() {
234234
}
235235

236236
private Expect(ExpectationGatherer gatherer) {
237-
super(FailureMetadata.forFailureStrategy(gatherer));
237+
super(FailureMetadata.forFailureStrategy(gatherer, /* suppressInferDescription= */ false));
238238
this.gatherer = checkNotNull(gatherer);
239239
}
240240

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,9 @@ public StandardSubjectBuilder whenTesting() {
110110
}
111111
failureExpected = true;
112112
return StandardSubjectBuilder.forCustomFailureStrategy(
113-
(ExpectFailureFailureStrategy) this::captureFailure);
113+
this::captureFailure, /* suppressInferDescription= */ true);
114114
}
115115

116-
/**
117-
* Marker interface that we check for so that we can automatically disable "value of" lines during
118-
* {@link ExpectFailure} assertions.
119-
*/
120-
interface ExpectFailureFailureStrategy extends FailureStrategy {}
121-
122116
/**
123117
* Enters rule context to be ready to capture failures.
124118
*

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

Lines changed: 58 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,14 @@
2424
import static com.google.common.truth.Fact.simpleFact;
2525
import static com.google.common.truth.LazyMessage.evaluateAll;
2626
import static com.google.common.truth.Platform.cleanStackTrace;
27+
import static com.google.common.truth.Platform.forceInferDescription;
2728
import static com.google.common.truth.Platform.inferDescription;
28-
import static com.google.common.truth.Platform.isInferDescriptionEnabledForExpectFailure;
2929
import static com.google.common.truth.Platform.makeComparisonFailure;
3030
import static com.google.common.truth.SubjectUtils.append;
3131
import static com.google.common.truth.SubjectUtils.concat;
3232

3333
import com.google.common.base.Function;
3434
import com.google.common.collect.ImmutableList;
35-
import com.google.common.truth.ExpectFailure.ExpectFailureFailureStrategy;
3635
import org.jspecify.annotations.Nullable;
3736

3837
/**
@@ -56,55 +55,18 @@
5655
* constructor.)
5756
*/
5857
public final class FailureMetadata {
59-
static FailureMetadata forFailureStrategy(FailureStrategy strategy) {
58+
static FailureMetadata forFailureStrategy(
59+
FailureStrategy strategy, boolean suppressInferDescription) {
6060
return new FailureMetadata(
61-
strategy, /* messages= */ ImmutableList.of(), /* steps= */ ImmutableList.of());
61+
strategy,
62+
suppressInferDescription,
63+
/* messages= */ ImmutableList.of(),
64+
/* steps= */ ImmutableList.of());
6265
}
6366

6467
private final FailureStrategy strategy;
6568

66-
/**
67-
* The data from a call to either (a) a {@link Subject} constructor or (b) {@link Subject#check}.
68-
*/
69-
private static final class Step {
70-
static Step subjectCreation(Subject subject) {
71-
return new Step(checkNotNull(subject), null, null);
72-
}
73-
74-
static Step checkCall(
75-
@Nullable OldAndNewValuesAreSimilar valuesAreSimilar,
76-
@Nullable Function<String, String> descriptionUpdate) {
77-
return new Step(null, descriptionUpdate, valuesAreSimilar);
78-
}
79-
80-
/*
81-
* We store Subject, rather than the actual value itself, so that we can call
82-
* actualCustomStringRepresentation(). Why not call actualCustomStringRepresentation()
83-
* immediately? First, it might be expensive, and second, the Subject isn't initialized at the
84-
* time we receive it. We *might* be able to make it safe to call if it looks only at
85-
* actualForPackageMembersToCall(), but it might try to look at facts initialized by a subclass,
86-
* which aren't ready yet.
87-
*/
88-
final @Nullable Subject subject;
89-
90-
final @Nullable Function<String, String> descriptionUpdate;
91-
92-
// Present only when descriptionUpdate is.
93-
final @Nullable OldAndNewValuesAreSimilar valuesAreSimilar;
94-
95-
private Step(
96-
@Nullable Subject subject,
97-
@Nullable Function<String, String> descriptionUpdate,
98-
@Nullable OldAndNewValuesAreSimilar valuesAreSimilar) {
99-
this.subject = subject;
100-
this.descriptionUpdate = descriptionUpdate;
101-
this.valuesAreSimilar = valuesAreSimilar;
102-
}
103-
104-
boolean isCheckCall() {
105-
return subject == null;
106-
}
107-
}
69+
private final boolean suppressInferDescription;
10870

10971
/*
11072
* TODO(cpovirk): This implementation is wasteful, especially because `steps` is used even by
@@ -119,8 +81,12 @@ boolean isCheckCall() {
11981
private final ImmutableList<Step> steps;
12082

12183
private FailureMetadata(
122-
FailureStrategy strategy, ImmutableList<LazyMessage> messages, ImmutableList<Step> steps) {
84+
FailureStrategy strategy,
85+
boolean suppressInferDescription,
86+
ImmutableList<LazyMessage> messages,
87+
ImmutableList<Step> steps) {
12388
this.strategy = checkNotNull(strategy);
89+
this.suppressInferDescription = suppressInferDescription;
12490
this.messages = checkNotNull(messages);
12591
this.steps = checkNotNull(steps);
12692
}
@@ -212,7 +178,7 @@ private void doFail(AssertionError failure) {
212178
}
213179

214180
private FailureMetadata derive(ImmutableList<LazyMessage> messages, ImmutableList<Step> steps) {
215-
return new FailureMetadata(strategy, messages, steps);
181+
return new FailureMetadata(strategy, suppressInferDescription, messages, steps);
216182
}
217183

218184
/**
@@ -246,10 +212,7 @@ private ImmutableList<Fact> description() {
246212
/** Overload of {@link #description()} that allows passing a custom key for the fact. */
247213
private ImmutableList<Fact> description(String factKey) {
248214
String description =
249-
strategy instanceof ExpectFailureFailureStrategy
250-
&& !isInferDescriptionEnabledForExpectFailure()
251-
? null
252-
: inferDescription();
215+
suppressInferDescription && !forceInferDescription() ? null : inferDescription();
253216
boolean descriptionIsInteresting = description != null;
254217
for (Step step : steps) {
255218
if (step.isCheckCall()) {
@@ -353,4 +316,47 @@ && checkNotNull(step.subject).actualForPackageMembersToCall() instanceof Throwab
353316
}
354317
return null;
355318
}
319+
320+
/**
321+
* The data from a call to either (a) a {@link Subject} constructor or (b) {@link Subject#check}.
322+
*/
323+
private static final class Step {
324+
static Step subjectCreation(Subject subject) {
325+
return new Step(checkNotNull(subject), null, null);
326+
}
327+
328+
static Step checkCall(
329+
@Nullable OldAndNewValuesAreSimilar valuesAreSimilar,
330+
@Nullable Function<String, String> descriptionUpdate) {
331+
return new Step(null, descriptionUpdate, valuesAreSimilar);
332+
}
333+
334+
/*
335+
* We store Subject, rather than the actual value itself, so that we can call
336+
* actualCustomStringRepresentation(). Why not call actualCustomStringRepresentation()
337+
* immediately? First, it might be expensive, and second, the Subject isn't initialized at the
338+
* time we receive it. We *might* be able to make it safe to call if it looks only at
339+
* actualForPackageMembersToCall(), but it might try to look at facts initialized by a subclass,
340+
* which aren't ready yet.
341+
*/
342+
final @Nullable Subject subject;
343+
344+
final @Nullable Function<String, String> descriptionUpdate;
345+
346+
// Present only when descriptionUpdate is.
347+
final @Nullable OldAndNewValuesAreSimilar valuesAreSimilar;
348+
349+
private Step(
350+
@Nullable Subject subject,
351+
@Nullable Function<String, String> descriptionUpdate,
352+
@Nullable OldAndNewValuesAreSimilar valuesAreSimilar) {
353+
this.subject = subject;
354+
this.descriptionUpdate = descriptionUpdate;
355+
this.valuesAreSimilar = valuesAreSimilar;
356+
}
357+
358+
boolean isCheckCall() {
359+
return subject == null;
360+
}
361+
}
356362
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,10 @@ private static boolean isInferDescriptionDisabled() {
242242
}
243243
}
244244

245-
static boolean isInferDescriptionEnabledForExpectFailure() {
245+
static boolean forceInferDescription() {
246246
try {
247247
return Boolean.parseBoolean(
248-
System.getProperty(
249-
"com.google.common.truth.enable_infer_description_for_expect_failure"));
248+
System.getProperty("com.google.common.truth.force_infer_description"));
250249
} catch (SecurityException e) {
251250
return false;
252251
}

core/src/main/java/com/google/common/truth/StandardSubjectBuilder.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,13 @@ public class StandardSubjectBuilder {
9292
* FailureStrategy}.
9393
*/
9494
public static StandardSubjectBuilder forCustomFailureStrategy(FailureStrategy strategy) {
95-
return new StandardSubjectBuilder(FailureMetadata.forFailureStrategy(strategy));
95+
return forCustomFailureStrategy(strategy, /* suppressInferDescription= */ false);
96+
}
97+
98+
static StandardSubjectBuilder forCustomFailureStrategy(
99+
FailureStrategy strategy, boolean suppressInferDescription) {
100+
return new StandardSubjectBuilder(
101+
FailureMetadata.forFailureStrategy(strategy, suppressInferDescription));
96102
}
97103

98104
private final FailureMetadata metadataDoNotReferenceDirectly;

core/src/main/java/com/google/common/truth/TruthJUnit.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ public final class TruthJUnit {
4646
new AssumptionViolatedException(failure.getMessage(), failure.getCause());
4747
assumptionViolated.setStackTrace(failure.getStackTrace());
4848
throw assumptionViolated;
49-
});
49+
},
50+
/* suppressInferDescription= */ true);
5051

5152
/**
5253
* Begins a call chain with the fluent Truth API. If the check made by the chain fails, it will

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ static String expectFailureWarningIfWeb() {
180180
+ " verifying that the expected failure occurred.)";
181181
}
182182

183-
static boolean isInferDescriptionEnabledForExpectFailure() {
183+
static boolean forceInferDescription() {
184184
return false; // irrelevant because we can infer descriptions only under the JVM
185185
}
186186

0 commit comments

Comments
 (0)