Inject `TestSummaryOptions` into `TestStrategy` and make the `#processTestOutput` method respect `--print_relative_test_log_paths`
Also refactor some cases in `TestStrategyTest` so that the new functionality is covered.
PiperOrigin-RevId: 628077548
Change-Id: I6679a6dd0f76b3155fea7c5385c4c15df973a246
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 305657b..4a1cd08 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -365,6 +365,7 @@
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib:build-request-options",
"//src/main/java/com/google/devtools/build/lib:runtime/build_event_streamer_utils",
+ "//src/main/java/com/google/devtools/build/lib:runtime/test_summary_options",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
index f634019..aebd288 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.exec.TestLogHelper;
import com.google.devtools.build.lib.exec.TestXmlOutputParser;
import com.google.devtools.build.lib.exec.TestXmlOutputParserException;
+import com.google.devtools.build.lib.runtime.TestSummaryOptions;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.TestAction;
import com.google.devtools.build.lib.server.FailureDetails.TestAction.Code;
@@ -153,10 +154,13 @@
// executable base name.
private final Map<String, Integer> tmpIndex = new HashMap<>();
protected final ExecutionOptions executionOptions;
+ protected final TestSummaryOptions testSummaryOptions;
protected final BinTools binTools;
- public TestStrategy(ExecutionOptions executionOptions, BinTools binTools) {
+ public TestStrategy(
+ ExecutionOptions executionOptions, TestSummaryOptions testSummaryOptions, BinTools binTools) {
this.executionOptions = executionOptions;
+ this.testSummaryOptions = testSummaryOptions;
this.binTools = binTools;
}
@@ -258,7 +262,7 @@
public int getTestAttempts(TestRunnerAction action) {
return action.getTestProperties().isFlaky()
? getTestAttemptsForFlakyTest(action)
- : getTestAttempts(action, /*defaultTestAttempts=*/ 1);
+ : getTestAttempts(action, /* defaultTestAttempts= */ 1);
}
private int getTestAttempts(TestRunnerAction action, int defaultTestAttempts) {
@@ -267,7 +271,7 @@
}
public int getTestAttemptsForFlakyTest(TestRunnerAction action) {
- return getTestAttempts(action, /*defaultTestAttempts=*/ 3);
+ return getTestAttempts(action, /* defaultTestAttempts= */ 3);
}
private static int getTestAttemptsPerLabel(
@@ -360,7 +364,7 @@
ActionExecutionContext actionExecutionContext,
TestResultData testResultData,
String testName,
- Path testLog)
+ @Nullable Path testLog)
throws IOException {
boolean isPassed = testResultData.getTestPassed();
try {
@@ -376,15 +380,29 @@
if (isPassed) {
actionExecutionContext.getEventHandler().handle(Event.of(EventKind.PASS, null, testName));
} else {
+ PathFragment testLogPathToOutput = null;
+ if (testLog != null) {
+ testLogPathToOutput =
+ testSummaryOptions.printRelativeTestLogPaths
+ ? testLog
+ .asFragment()
+ .relativeTo(actionExecutionContext.getExecRoot().asFragment())
+ : testLog.asFragment();
+ }
if (testResultData.hasStatusDetails()) {
actionExecutionContext
.getEventHandler()
.handle(Event.error(testName + ": " + testResultData.getStatusDetails()));
}
if (testResultData.getStatus() == BlazeTestStatus.TIMEOUT) {
+ String message =
+ String.format(
+ "%s%s",
+ testName,
+ testLogPathToOutput != null ? " (see " + testLogPathToOutput + ")" : "");
actionExecutionContext
.getEventHandler()
- .handle(Event.of(EventKind.TIMEOUT, null, testName + " (see " + testLog + ")"));
+ .handle(Event.of(EventKind.TIMEOUT, null, message));
} else if (testResultData.getStatus() == BlazeTestStatus.INCOMPLETE) {
actionExecutionContext
.getEventHandler()
@@ -395,7 +413,12 @@
.setWaitResponse(testResultData.getExitCode())
.setTimedOut(testResultData.getStatus() == BlazeTestStatus.TIMEOUT)
.build();
- String message = String.format("%s (%s) (see %s)", testName, ts.toShortString(), testLog);
+ String message =
+ String.format(
+ "%s (%s)%s",
+ testName,
+ ts.toShortString(),
+ testLogPathToOutput != null ? " (see " + testLogPathToOutput + ")" : "");
actionExecutionContext.getEventHandler().handle(Event.of(EventKind.FAIL, null, message));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index d66c59f..220c071 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -391,6 +391,7 @@
":spawn_strategy_resolver",
":standalone_test_result",
":test_policy",
+ "//src/main/java/com/google/devtools/build/lib:runtime/test_summary_options",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
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 39e77f1..22e3b22 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
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.runtime.TestSummaryOptions;
import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.TestAction;
@@ -89,8 +90,11 @@
protected final Path tmpDirRoot;
public StandaloneTestStrategy(
- ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) {
- super(executionOptions, binTools);
+ ExecutionOptions executionOptions,
+ TestSummaryOptions testSummaryOptions,
+ BinTools binTools,
+ Path tmpDirRoot) {
+ super(executionOptions, testSummaryOptions, binTools);
this.tmpDirRoot = tmpDirRoot;
}
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/BUILD b/src/main/java/com/google/devtools/build/lib/standalone/BUILD
index 8e5f7fd..76e0a48 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/standalone/BUILD
@@ -19,6 +19,7 @@
],
deps = [
"//src/main/java/com/google/devtools/build/lib:runtime",
+ "//src/main/java/com/google/devtools/build/lib:runtime/test_summary_options",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:actions/file_write_action_context",
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
index 916139f..6535cc9 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.ProcessWrapper;
+import com.google.devtools.build.lib.runtime.TestSummaryOptions;
import com.google.devtools.build.lib.vfs.Path;
/**
@@ -56,11 +57,13 @@
registryBuilder.register(CppIncludeScanningContext.class, new DummyCppIncludeScanningContext());
ExecutionOptions executionOptions = env.getOptions().getOptions(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = env.getOptions().getOptions(TestSummaryOptions.class);
Path testTmpRoot =
TestStrategy.getTmpRoot(env.getWorkspace(), env.getExecRoot(), executionOptions);
TestActionContext testStrategy =
new StandaloneTestStrategy(
executionOptions,
+ testSummaryOptions,
env.getBlazeWorkspace().getBinTools(),
testTmpRoot);
registryBuilder.register(TestActionContext.class, testStrategy, "standalone");
diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD
index 841dc6e..a6dc551 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD
@@ -22,6 +22,7 @@
"*.java",
]),
deps = [
+ "//src/main/java/com/google/devtools/build/lib:runtime/test_summary_options",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
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 9dca510..bc320b6 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
@@ -63,6 +63,7 @@
import com.google.devtools.build.lib.exec.StandaloneTestStrategy.StandaloneFailedAttemptResult;
import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache;
import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
+import com.google.devtools.build.lib.runtime.TestSummaryOptions;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
@@ -102,8 +103,11 @@
TestResult postedResult = null;
TestedStandaloneTestStrategy(
- ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) {
- super(executionOptions, binTools, tmpDirRoot);
+ ExecutionOptions executionOptions,
+ TestSummaryOptions testSummaryOptions,
+ BinTools binTools,
+ Path tmpDirRoot) {
+ super(executionOptions, testSummaryOptions, binTools, tmpDirRoot);
}
@Override
@@ -280,10 +284,12 @@
@Test
public void testRunTestOnce() throws Exception {
ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
+ TestSummaryOptions testSummaryOptions = TestSummaryOptions.DEFAULTS;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
@@ -341,13 +347,15 @@
@Test
public void testRunFlakyTest() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
// TODO(ulfjack): Update this test for split xml generation.
executionOptions.splitXmlGeneration = false;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
@@ -424,10 +432,12 @@
@Test
public void testRunTestRemotely() throws Exception {
ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
+ TestSummaryOptions testSummaryOptions = TestSummaryOptions.DEFAULTS;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
@@ -488,10 +498,12 @@
@Test
public void testRunRemotelyCachedTest() throws Exception {
ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
+ TestSummaryOptions testSummaryOptions = TestSummaryOptions.DEFAULTS;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
@@ -552,11 +564,13 @@
@Test
public void testThatTestLogAndOutputAreReturned() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
executionOptions.testOutput = ExecutionOptions.TestOutputFormat.ERRORS;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/failing_test.sh", "this does not get executed, it is mocked out");
@@ -637,12 +651,14 @@
@Test
public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
executionOptions.testOutput = ExecutionOptions.TestOutputFormat.ERRORS;
executionOptions.splitXmlGeneration = true;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/failing_test.sh", "this does not get executed, it is mocked out");
@@ -730,11 +746,13 @@
@Test
public void testEmptyOutputCreatesEmptyLogFile() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
executionOptions.testOutput = ExecutionOptions.TestOutputFormat.ALL;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
@@ -780,11 +798,13 @@
@Test
public void testAppendStdErrDoesNotBusyLoop() throws Exception {
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
executionOptions.testOutput = ExecutionOptions.TestOutputFormat.ALL;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
@@ -828,10 +848,12 @@
"--runs_per_test_detects_flakes",
"--experimental_cancel_concurrent_tests");
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
scratch.file(
@@ -909,10 +931,12 @@
"--runs_per_test_detects_flakes",
"--experimental_cancel_concurrent_tests");
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
scratch.file(
@@ -1015,10 +1039,12 @@
"--runs_per_test_detects_flakes",
"--experimental_cancel_concurrent_tests");
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+ TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
scratch.file(
@@ -1099,10 +1125,12 @@
@Test
public void missingTestLogSpawnTestResultIsIncomplete() throws Exception {
ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
+ TestSummaryOptions testSummaryOptions = TestSummaryOptions.DEFAULTS;
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
TestedStandaloneTestStrategy standaloneTestStrategy =
- new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+ new TestedStandaloneTestStrategy(
+ executionOptions, testSummaryOptions, binTools, tmpDirRoot);
// setup a test action
scratch.file("standalone/simple_test.sh", "this does not get executed, it is mocked out");
