Simplify TestAttempt interface
By always requiring a TestResultData instace, we simplify the callers, which
already have to do all the work anyway.
PiperOrigin-RevId: 199639965
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
index 21bf683..7b347c4 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -167,13 +167,10 @@
.post(
TestAttempt.forExecutedTestResult(
action,
- standaloneTestResult.executionInfo(),
+ standaloneTestResult.testResultData(),
attempt,
- standaloneTestResult.testResultData().getStatus(),
- standaloneTestResult.testResultData().getStartTimeMillisEpoch(),
- standaloneTestResult.testResultData().getRunDurationMillis(),
testOutputs,
- standaloneTestResult.testResultData().getWarningList(),
+ standaloneTestResult.executionInfo(),
true));
finalizeTest(actionExecutionContext, action, dataBuilder.build());
@@ -251,13 +248,10 @@
.post(
TestAttempt.forExecutedTestResult(
action,
- result.executionInfo(),
+ data,
attempt,
- data.getStatus(),
- data.getStartTimeMillisEpoch(),
- data.getRunDurationMillis(),
testOutputsBuilder.build(),
- data.getWarningList(),
+ result.executionInfo(),
false));
processTestOutput(actionExecutionContext, new TestResult(action, data, false), testLog);
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestAttempt.java b/src/main/java/com/google/devtools/build/lib/exec/TestAttempt.java
index e6fd3e1..ed70ad2 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestAttempt.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestAttempt.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
import com.google.devtools.build.lib.buildeventstream.BuildEventId;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestStatus;
import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint;
import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
import com.google.devtools.build.lib.buildeventstream.PathConverter;
@@ -40,7 +41,7 @@
public class TestAttempt implements BuildEventWithOrderConstraint {
private final TestRunnerAction testAction;
- private final BlazeTestStatus status;
+ private final TestStatus status;
private final boolean cachedLocally;
private final int attempt;
private final boolean lastAttempt;
@@ -71,7 +72,7 @@
this.testAction = testAction;
this.executionInfo = Preconditions.checkNotNull(executionInfo);
this.attempt = attempt;
- this.status = Preconditions.checkNotNull(status);
+ this.status = BuildEventStreamerUtils.bepStatus(Preconditions.checkNotNull(status));
this.cachedLocally = cachedLocally;
this.startTimeMillis = startTimeMillis;
this.durationMillis = durationMillis;
@@ -86,24 +87,21 @@
*/
public static TestAttempt forExecutedTestResult(
TestRunnerAction testAction,
- BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo,
+ TestResultData attemptData,
int attempt,
- BlazeTestStatus status,
- long startTimeMillis,
- long durationMillis,
Collection<Pair<String, Path>> files,
- List<String> testWarnings,
+ BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo,
boolean lastAttempt) {
return new TestAttempt(
false,
testAction,
executionInfo,
attempt,
- status,
- startTimeMillis,
- durationMillis,
+ attemptData.getStatus(),
+ attemptData.getStartTimeMillisEpoch(),
+ attemptData.getRunDurationMillis(),
files,
- testWarnings,
+ attemptData.getWarningList(),
lastAttempt);
}
@@ -143,7 +141,7 @@
}
@VisibleForTesting
- public BlazeTestStatus getStatus() {
+ public TestStatus getStatus() {
return status;
}
@@ -189,7 +187,7 @@
PathConverter pathConverter = converters.pathConverter();
BuildEventStreamProtos.TestResult.Builder builder =
BuildEventStreamProtos.TestResult.newBuilder();
- builder.setStatus(BuildEventStreamerUtils.bepStatus(status));
+ builder.setStatus(status);
builder.setExecutionInfo(executionInfo);
builder.setCachedLocally(cachedLocally);
builder.setTestAttemptStartMillisEpoch(startTimeMillis);
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 29b35fd..15b05fa 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1392,6 +1392,7 @@
"//src/main/java/com/google/devtools/build/lib:io",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
index 7c89da4..1aa1818 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.analysis.test.TestResult;
import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestStatus;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.TestStrategy.TestOutputFormat;
@@ -252,10 +253,10 @@
TestAttempt failedAttempt = attempts.get(0);
assertThat(failedAttempt.getExecutionInfo().getStrategy()).isEqualTo("test");
assertThat(failedAttempt.getExecutionInfo().getHostname()).isEqualTo("");
- assertThat(failedAttempt.getStatus()).isEqualTo(BlazeTestStatus.FAILED);
+ assertThat(failedAttempt.getStatus()).isEqualTo(TestStatus.FAILED);
assertThat(failedAttempt.getExecutionInfo().getCachedRemotely()).isFalse();
TestAttempt okAttempt = attempts.get(1);
- assertThat(okAttempt.getStatus()).isEqualTo(BlazeTestStatus.PASSED);
+ assertThat(okAttempt.getStatus()).isEqualTo(TestStatus.PASSED);
assertThat(okAttempt.getExecutionInfo().getStrategy()).isEqualTo("test");
assertThat(okAttempt.getExecutionInfo().getHostname()).isEqualTo("");
}
@@ -333,7 +334,7 @@
.filter(TestAttempt.class::isInstance)
.map(TestAttempt.class::cast)
.collect(MoreCollectors.onlyElement());
- assertThat(attempt.getStatus()).isEqualTo(BlazeTestStatus.PASSED);
+ assertThat(attempt.getStatus()).isEqualTo(TestStatus.PASSED);
assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote");
assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("a-remote-host");
}
