From b33d51f250499b6e51ff9d82371d053fff8e3f55 Mon Sep 17 00:00:00 2001 From: Yun Peng Date: Thu, 12 Dec 2024 07:23:58 -0800 Subject: [PATCH] Fix checking TEST_PREMATURE_EXIT_FILE `TEST_PREMATURE_EXIT_FILE` was not cleaned up between test attempts (--flaky_test_attempts). Fixes https://github.com/bazelbuild/bazel/issues/24655 Closes #24657. PiperOrigin-RevId: 705495967 Change-Id: Ic4d4be1e72230c3a6bc45c9f372d1188fefad201 --- .../lib/exec/StandaloneTestStrategy.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) 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 b5f4ba2b4bb73f..f412cbee769e91 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 @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.exec; +import static java.nio.charset.StandardCharsets.UTF_8; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Verify; @@ -65,7 +67,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -655,25 +656,7 @@ private TestAttemptResult runTestAttempt( try { spawnResults = resolver.exec(spawn, actionExecutionContext.withFileOutErr(fileOutErr)); testResultDataBuilder = TestResultData.newBuilder(); - if (actionExecutionContext - .getPathResolver() - .convertPath(resolvedPaths.getExitSafeFile()) - .exists()) { - testResultDataBuilder - .setCachable(false) - .setTestPassed(false) - .setStatus(BlazeTestStatus.FAILED); - fileOutErr - .getErrorStream() - .write( - "-- Test exited prematurely (TEST_PREMATURE_EXIT_FILE exists) --\n" - .getBytes(StandardCharsets.UTF_8)); - } else { - testResultDataBuilder - .setCachable(true) - .setTestPassed(true) - .setStatus(BlazeTestStatus.PASSED); - } + testResultDataBuilder.setCachable(true).setTestPassed(true).setStatus(BlazeTestStatus.PASSED); } catch (SpawnExecException e) { if (e.isCatastrophic()) { closeSuppressed(e, streamed); @@ -699,6 +682,22 @@ private TestAttemptResult runTestAttempt( } long endTimeMillis = actionExecutionContext.getClock().currentTimeMillis(); + // Check TEST_PREMATURE_EXIT_FILE file (and always delete it) + if (actionExecutionContext + .getPathResolver() + .convertPath(resolvedPaths.getExitSafeFile()) + .delete() + && testResultDataBuilder.getTestPassed()) { + testResultDataBuilder + .setCachable(false) + .setTestPassed(false) + .setStatus(BlazeTestStatus.FAILED); + fileOutErr + .getErrorStream() + .write( + "-- Test exited prematurely (TEST_PREMATURE_EXIT_FILE exists) --\n".getBytes(UTF_8)); + } + // Do not override a more informative test failure with a generic failure due to the missing // shard file, which may have been caused by the test failing before the runner had a chance to // touch the file