Report error instead of crash if test uses reserved env variable

This closes bazelbuild#16094 Previously Bazel will crash if test uses reserved env variable (like `TEST_NAME`). Let's report an error in this case.
blindpirate · Aug 15, 2022 · d904397 · d904397
1 parent 61c433e
commit d904397
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 8 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
@@ -105,24 +105,32 @@ public StandaloneTestStrategy(
     this.tmpDirRoot = tmpDirRoot;
   }
 
+  private static TestExecException createTestExecException(String errorMessage) {
+    return new TestExecException(
+        errorMessage,
+        FailureDetail.newBuilder()
+            .setTestAction(
+                TestAction.newBuilder().setCode(TestAction.Code.LOCAL_TEST_PREREQ_UNMET))
+            .setMessage(errorMessage)
+            .build()
+    );
+  }
+
   @Override
   public TestRunnerSpawn createTestRunnerSpawn(
       TestRunnerAction action, ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
     if (action.getExecutionSettings().getInputManifest() == null) {
-      String errorMessage = "cannot run local tests with --nobuild_runfile_manifests";
-      throw new TestExecException(
-          errorMessage,
-          FailureDetail.newBuilder()
-              .setTestAction(
-                  TestAction.newBuilder().setCode(TestAction.Code.LOCAL_TEST_PREREQ_UNMET))
-              .setMessage(errorMessage)
-              .build());
+      throw createTestExecException("cannot run local tests with --nobuild_runfile_manifests");
     }
     Map<String, String> testEnvironment =
         createEnvironment(
             actionExecutionContext, action, tmpDirRoot, executionOptions.splitXmlGeneration);
 
+    if (testEnvironment.containsKey("TEST_NAME")) {
+      throw createTestExecException(String.format("cannot set env variable TEST_NAME=%s because TEST_NAME is reserved", testEnvironment.get("TEST_NAME")));
+    }
+
     Map<String, String> executionInfo =
         new TreeMap<>(action.getTestProperties().getExecutionInfo());
     if (!action.shouldCacheResult()) {

diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh
@@ -953,6 +953,24 @@ EOF
   expect_log "cannot run local tests with --nobuild_runfile_manifests"
 }
 
+function test_test_with_reserved_env_variable() {
+  mkdir -p dir
+
+  touch dir/test.sh
+  chmod u+x dir/test.sh
+  cat <<'EOF' > dir/BUILD
+sh_test(
+    name = 'test',
+    srcs = ['test.sh'],
+    env = {
+      "TEST_NAME": "foo"
+    },
+)
+EOF
+  bazel test //dir:test >& $TEST_log && fail "should have failed"
+  expect_log "cannot set env variable TEST_NAME=foo because the key is reserved"
+}
+
 function test_run_from_external_repo_sibling_repository_layout() {
   cat <<EOF > WORKSPACE
 local_repository(