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 fd1214daf9cedb..cdc4b8de162cec 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 @@ -391,7 +391,8 @@ private static BuildEventStreamProtos.TestResult.ExecutionInfo extractExecutionI * A spawn to generate a test.xml file from the test log. This is only used if the test does not * generate a test.xml file itself. */ - private static Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult result) { + private static Spawn createXmlGeneratingSpawn( + TestRunnerAction action, ImmutableMap testEnv, SpawnResult result) { ImmutableList args = ImmutableList.of( action.getTestXmlGeneratorScript().getExecPath().getCallablePathString(), @@ -399,18 +400,21 @@ private static Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResu action.getXmlOutputPath().getPathString(), Long.toString(result.getWallTime().orElse(Duration.ZERO).getSeconds()), Integer.toString(result.exitCode())); - - String testBinaryName = - action.getExecutionSettings().getExecutable().getRootRelativePath().getCallablePathString(); + ImmutableMap.Builder envBuilder = ImmutableMap.builder(); + // "PATH" and "TEST_BINARY" are also required, they should always be set in testEnv. + Preconditions.checkArgument(testEnv.containsKey("PATH")); + Preconditions.checkArgument(testEnv.containsKey("TEST_BINARY")); + envBuilder.putAll(testEnv).put("TEST_NAME", action.getTestName()); + // testEnv only contains TEST_SHARD_INDEX and TEST_TOTAL_SHARDS if the test action is sharded, + // we need to set the default value when the action isn't sharded. + if (!action.isSharded()) { + envBuilder.put("TEST_SHARD_INDEX", "0"); + envBuilder.put("TEST_TOTAL_SHARDS", "0"); + } return new SimpleSpawn( action, args, - ImmutableMap.of( - "PATH", "/usr/bin:/bin", - "TEST_SHARD_INDEX", Integer.toString(action.getShardNum()), - "TEST_TOTAL_SHARDS", Integer.toString(action.getExecutionSettings().getTotalShards()), - "TEST_NAME", action.getTestName(), - "TEST_BINARY", testBinaryName), + envBuilder.build(), // Pass the execution info of the action which is identical to the supported tags set on the // test target. In particular, this does not set the test timeout on the spawn. ImmutableMap.copyOf(action.getExecutionInfo()), @@ -766,7 +770,8 @@ public TestAttemptContinuation execute() if (executionOptions.splitXmlGeneration && fileOutErr.getOutputPath().exists() && !xmlOutputPath.exists()) { - Spawn xmlGeneratingSpawn = createXmlGeneratingSpawn(testAction, spawnResults.get(0)); + Spawn xmlGeneratingSpawn = + createXmlGeneratingSpawn(testAction, spawn.getEnvironment(), spawnResults.get(0)); SpawnStrategyResolver spawnStrategyResolver = actionExecutionContext.getContext(SpawnStrategyResolver.class); // We treat all failures to generate the test.xml here as catastrophic, and won't rerun 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 be2f78cf9e6b27..8672f92b4d3877 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 @@ -143,7 +143,7 @@ public FakeActionExecutionContext( LostInputsCheck.NONE, fileOutErr, /*eventHandler=*/ null, - /*clientEnv=*/ ImmutableMap.of(), + /*clientEnv=*/ ImmutableMap.of("PATH", "/usr/bin:/bin"), /*topLevelFilesets=*/ ImmutableMap.of(), /*artifactExpander=*/ null, /*actionFileSystem=*/ null, diff --git a/tools/test/windows/tw.cc b/tools/test/windows/tw.cc index 70677bdca4e986..d0338a2f030572 100644 --- a/tools/test/windows/tw.cc +++ b/tools/test/windows/tw.cc @@ -189,6 +189,9 @@ struct Duration { bool FromString(const wchar_t* str); }; +enum class MainType { kTestWrapperMain, kXmlWriterMain }; +enum class DeleteAfterwards { kEnabled, kDisabled }; + void WriteStdout(const std::string& s) { DWORD written; WriteFile(GetStdHandle(STD_OUTPUT_HANDLE), s.c_str(), s.size(), &written, @@ -1603,9 +1606,16 @@ std::string CreateErrorTag(int exit_code) { } } -bool ShouldCreateXml(const Path& xml_log, bool* result) { +bool ShouldCreateXml(const Path& xml_log, const MainType main_type, + bool* result) { *result = true; + // If running from the xml generator binary, we should always create the xml + // file. + if (main_type == MainType::kXmlWriterMain) { + return true; + } + DWORD attr = GetFileAttributesW(AddUncPrefixMaybe(xml_log).c_str()); if (attr != INVALID_FILE_ATTRIBUTES) { // The XML file already exists, maybe the test framework wrote it. @@ -1630,9 +1640,10 @@ bool ShouldCreateXml(const Path& xml_log, bool* result) { bool CreateXmlLog(const Path& output, const Path& test_outerr, const Duration duration, const int exit_code, - const bool delete_afterwards) { + const DeleteAfterwards delete_afterwards, + const MainType main_type) { bool should_create_xml; - if (!ShouldCreateXml(output, &should_create_xml)) { + if (!ShouldCreateXml(output, main_type, &should_create_xml)) { LogErrorWithArg(__LINE__, "Failed to decide if XML log is needed", output.Get()); return false; @@ -1645,7 +1656,7 @@ bool CreateXmlLog(const Path& output, const Path& test_outerr, // Delete the test's outerr file after we have the XML file. // We don't care if this succeeds or not, because the outerr file is not a // declared output. - if (delete_afterwards) { + if (delete_afterwards == DeleteAfterwards::kEnabled) { DeleteFileW(test_outerr.Get().c_str()); } }); @@ -1904,7 +1915,8 @@ int TestWrapperMain(int argc, wchar_t** argv) { Duration test_duration; int result = RunSubprocess(test_path, args, test_outerr, &test_duration); - if (!CreateXmlLog(xml_log, test_outerr, test_duration, result, true) || + if (!CreateXmlLog(xml_log, test_outerr, test_duration, result, + DeleteAfterwards::kEnabled, MainType::kTestWrapperMain) || !ArchiveUndeclaredOutputs(undecl) || !CreateUndeclaredOutputsAnnotations(undecl.annotations_dir, undecl.annotations)) { @@ -1921,7 +1933,8 @@ int XmlWriterMain(int argc, wchar_t** argv) { if (!GetCwd(&cwd) || !ParseXmlWriterArgs(argc, argv, cwd, &test_outerr, &test_xml_log, &duration, &exit_code) || - !CreateXmlLog(test_xml_log, test_outerr, duration, exit_code, false)) { + !CreateXmlLog(test_xml_log, test_outerr, duration, exit_code, + DeleteAfterwards::kDisabled, MainType::kXmlWriterMain)) { return 1; }