Skip to content

Commit

Permalink
Report error instead of crash if test uses reserved env variable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
blindpirate committed Aug 15, 2022
1 parent 61c433e commit 77fc085
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
/** Runs TestRunnerAction actions. */
// TODO(bazel-team): add tests for this strategy.
public class StandaloneTestStrategy extends TestStrategy {
private static final String TEST_NAME_ENV = "TEST_NAME";
private static final ImmutableMap<String, String> ENV_VARS =
ImmutableMap.<String, String>builder()
.put("TZ", "UTC")
Expand All @@ -105,24 +106,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_ENV)) {
throw createTestExecException(String.format("cannot set env variable TEST_NAME=%s because TEST_NAME is reserved", testEnvironment.get(TEST_NAME_ENV)));
}

Map<String, String> executionInfo =
new TreeMap<>(action.getTestProperties().getExecutionInfo());
if (!action.shouldCacheResult()) {
Expand Down Expand Up @@ -582,7 +591,7 @@ private static Spawn createXmlGeneratingSpawn(
// "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());
envBuilder.putAll(testEnv).put(TEST_NAME_ENV, 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()) {
Expand Down Expand Up @@ -630,7 +639,7 @@ private static Spawn createCoveragePostProcessingSpawn(
testEnvironment.put("TEST_SHARD_INDEX", Integer.toString(action.getShardNum()));
testEnvironment.put(
"TEST_TOTAL_SHARDS", Integer.toString(action.getExecutionSettings().getTotalShards()));
testEnvironment.put("TEST_NAME", action.getTestName());
testEnvironment.put(TEST_NAME_ENV, action.getTestName());
testEnvironment.put("IS_COVERAGE_SPAWN", "1");
return new SimpleSpawn(
action,
Expand Down
18 changes: 18 additions & 0 deletions src/test/shell/bazel/bazel_test_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 TEST_NAME is reserved"
}

function test_run_from_external_repo_sibling_repository_layout() {
cat <<EOF > WORKSPACE
local_repository(
Expand Down

0 comments on commit 77fc085

Please sign in to comment.