From dbb91e4bb45f6f4c2da6a050d33ca75aa662261f Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 8 Oct 2022 12:36:04 +0200 Subject: [PATCH] Add `arguments` parameter to `RunEnvironmentInfo` Executable Starlark rules can use the new `arguments` parameter on `RunEnvironmentInfo` to specify the arguments that Bazel should pass on the command line with `test` or `run`. If set to a non-`None` value, this parameter overrides the value of the `args` attribute that is implicitly defined for all rules. This allows Starlark rules to implement their own version of this attribute which isn't bound to its proprietary processing (data label expansion and tokenization). Along the way, this commit adds test coverage and documentation for the interplay between `RunEnvironmentInfo`'s `environment` and `--test_env`. The value of the `arguments` field of `RunEnvironmentInfo` is intentionally not exposed to Starlark yet: It is not clear how these arguments should be represented and whether rules relying on the magic `args` attribute should also provide this field. RELNOTES: Executable starlark rules can use the `arguments` parameter of `RunEnvironmentInfo` to specify their command-line arguments with `bazel run` and `bazel test`. --- .../google/devtools/build/lib/analysis/BUILD | 1 + .../analysis/RuleConfiguredTargetBuilder.java | 1 + .../lib/analysis/RunEnvironmentInfo.java | 14 ++++- .../build/lib/analysis/RunfilesSupport.java | 14 +++++ .../StarlarkRuleConfiguredTargetUtil.java | 56 +++++++++++++------ .../lib/analysis/test/TestActionBuilder.java | 18 +++++- .../test/TestTargetExecutionSettings.java | 4 +- .../lib/rules/test/StarlarkTestingModule.java | 1 + .../lib/runtime/commands/RunCommand.java | 25 ++++++--- .../RunEnvironmentInfoApi.java | 29 ++++++++-- .../lib/starlark/StarlarkIntegrationTest.java | 37 ++++++++++++ src/test/shell/bazel/bazel_rules_test.sh | 49 +++++++++++++--- 12 files changed, 207 insertions(+), 42 deletions(-) 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 b630ac7daeac91..6f8804f361056f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -1025,6 +1025,7 @@ java_library( name = "run_environment_info", srcs = ["RunEnvironmentInfo.java"], deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index 8a6a9728894f70..c13d5c98bdcafe 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -474,6 +474,7 @@ private TestProvider initializeTestProvider(FilesToRunProvider filesToRunProvide if (environmentProvider != null) { testActionBuilder.addExtraEnv(environmentProvider.getEnvironment()); testActionBuilder.addExtraInheritedEnv(environmentProvider.getInheritedEnvironment()); + testActionBuilder.setStarlarkTargetArgs(environmentProvider.getArguments()); } TestParams testParams = diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java index 57b1c26f6c0b84..c688a50f69b48e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java @@ -17,15 +17,18 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuiltinProvider; import com.google.devtools.build.lib.packages.NativeInfo; import com.google.devtools.build.lib.starlarkbuildapi.RunEnvironmentInfoApi; import java.util.List; import java.util.Map; +import javax.annotation.Nullable; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkList; /** @@ -40,16 +43,19 @@ public final class RunEnvironmentInfo extends NativeInfo implements RunEnvironme private final ImmutableMap environment; private final ImmutableList inheritedEnvironment; + @Nullable private final CommandLine arguments; private final boolean shouldErrorOnNonExecutableRule; /** Constructs a new provider with the given fixed and inherited environment variables. */ public RunEnvironmentInfo( Map environment, List inheritedEnvironment, + @Nullable CommandLine arguments, boolean shouldErrorOnNonExecutableRule) { this.environment = ImmutableMap.copyOf(Preconditions.checkNotNull(environment)); this.inheritedEnvironment = ImmutableList.copyOf(Preconditions.checkNotNull(inheritedEnvironment)); + this.arguments = arguments; this.shouldErrorOnNonExecutableRule = shouldErrorOnNonExecutableRule; } @@ -76,6 +82,11 @@ public ImmutableList getInheritedEnvironment() { return inheritedEnvironment; } + @Nullable + public CommandLine getArguments() { + return arguments; + } + /** * Returns whether advertising this provider on a non-executable (and thus non-test) rule should * result in an error or a warning. The latter is required to not break testing.TestEnvironment, @@ -95,11 +106,12 @@ private RunEnvironmentInfoProvider() { @Override public RunEnvironmentInfoApi constructor( - Dict environment, Sequence inheritedEnvironment) throws EvalException { + Dict environment, Sequence inheritedEnvironment, Object arguments) throws EvalException { return new RunEnvironmentInfo( Dict.cast(environment, String.class, String.class, "environment"), StarlarkList.immutableCopyOf( Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")), + arguments == Starlark.NONE ? null : CommandLine.of(Sequence.cast(arguments, String.class, "arguments")), /* shouldErrorOnNonExecutableRule= */ true); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index ef668a7974a8be..00c3497c015861 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -420,6 +420,20 @@ public static RunfilesSupport withExecutable( computeActionEnvironment(ruleContext)); } + /** + * Creates and returns a {@link RunfilesSupport} object for the given rule and executable without + * computing arguments based on the value of the "args" attribute. + */ + public static RunfilesSupport withExecutableNoArgs( + RuleContext ruleContext, Runfiles runfiles, Artifact executable) { + return RunfilesSupport.create( + ruleContext, + executable, + runfiles, + CommandLine.EMPTY, + computeActionEnvironment(ruleContext)); + } + /** * Creates and returns a {@link RunfilesSupport} object for the given rule and executable. Note * that this method calls back into the passed in rule to obtain the runfiles. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java index 3a18f0afb86a14..7e4dabe06f40b0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java @@ -343,30 +343,40 @@ private static void addProviders( } } + DefaultInfo defaultInfo = null; boolean defaultProviderProvidedExplicitly = false; + boolean starlarkArgsProvided = false; for (Info declaredProvider : declaredProviders.values()) { if (getProviderKey(declaredProvider).equals(DefaultInfo.PROVIDER.getKey())) { - parseDefaultProviderFields((DefaultInfo) declaredProvider, context, builder); + defaultInfo = (DefaultInfo) declaredProvider; defaultProviderProvidedExplicitly = true; - } else if (getProviderKey(declaredProvider).equals(RunEnvironmentInfo.PROVIDER.getKey()) - && !(context.isExecutable() || context.getRuleContext().isTestTarget())) { - String message = - "Returning RunEnvironmentInfo from a non-executable, non-test target has no effect"; - RunEnvironmentInfo runEnvironmentInfo = (RunEnvironmentInfo) declaredProvider; - if (runEnvironmentInfo.shouldErrorOnNonExecutableRule()) { - context.getRuleContext().ruleError(message); - } else { - context.getRuleContext().ruleWarning(message); - builder.addStarlarkDeclaredProvider(declaredProvider); + } else if (getProviderKey(declaredProvider).equals(RunEnvironmentInfo.PROVIDER.getKey())) { + if (((RunEnvironmentInfo) declaredProvider).getArguments() != null) { + // Ensure that RunfilesSupport does not parse the "args" attribute, the rule handles it in + // a custom way. + starlarkArgsProvided = true; + } + if (!(context.isExecutable() || context.getRuleContext().isTestTarget())) { + String message = + "Returning RunEnvironmentInfo from a non-executable, non-test target has no effect"; + RunEnvironmentInfo runEnvironmentInfo = (RunEnvironmentInfo) declaredProvider; + if (runEnvironmentInfo.shouldErrorOnNonExecutableRule()) { + context.getRuleContext().ruleError(message); + } else { + context.getRuleContext().ruleWarning(message); + } } + builder.addStarlarkDeclaredProvider(declaredProvider); } else { builder.addStarlarkDeclaredProvider(declaredProvider); } } - if (!defaultProviderProvidedExplicitly) { - parseDefaultProviderFields(oldStyleProviders, context, builder); + if (defaultProviderProvidedExplicitly) { + parseDefaultProviderFields(defaultInfo, context, builder, starlarkArgsProvided); + } else { + parseDefaultProviderFields(oldStyleProviders, context, builder, starlarkArgsProvided); } for (String field : oldStyleProviders.getFieldNames()) { @@ -491,7 +501,10 @@ private static Provider.Key getProviderKey(Info info) throws EvalException { * throws an {@link EvalException} if there are unknown fields. */ private static void parseDefaultProviderFields( - StructImpl info, StarlarkRuleContext context, RuleConfiguredTargetBuilder builder) + StructImpl info, + StarlarkRuleContext context, + RuleConfiguredTargetBuilder builder, + boolean starlarkArgsProvided) throws EvalException { Depset files = null; Runfiles statelessRunfiles = null; @@ -591,7 +604,8 @@ private static void parseDefaultProviderFields( files, statelessRunfiles, dataRunfiles, - defaultRunfiles); + defaultRunfiles, + starlarkArgsProvided); } private static void addSimpleProviders( @@ -601,7 +615,8 @@ private static void addSimpleProviders( @Nullable Depset files, Runfiles statelessRunfiles, Runfiles dataRunfiles, - Runfiles defaultRunfiles) + Runfiles defaultRunfiles, + boolean starlarkArgsProvided) throws EvalException { // TODO(bazel-team) if both 'files' and 'executable' are provided, 'files' overrides @@ -643,8 +658,13 @@ private static void addSimpleProviders( RunfilesSupport runfilesSupport = null; if (!computedDefaultRunfiles.isEmpty()) { Preconditions.checkNotNull(executable, "executable must not be null"); - runfilesSupport = - RunfilesSupport.withExecutable(ruleContext, computedDefaultRunfiles, executable); + if (starlarkArgsProvided) { + runfilesSupport = RunfilesSupport.withExecutableNoArgs( + ruleContext, computedDefaultRunfiles, executable); + } else { + runfilesSupport = + RunfilesSupport.withExecutable(ruleContext, computedDefaultRunfiles, executable); + } assertExecutableSymlinkPresent(runfilesSupport.getRunfiles(), executable); } builder.setRunfilesSupport(runfilesSupport, executable); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java index 69539f7b5857fb..4dc81ba22602f4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.Allowlist; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; @@ -98,12 +99,14 @@ public NestedSet getPackageSpecifications() { private int explicitShardCount; private final Map extraEnv; private final Set extraInheritedEnv; + @Nullable private CommandLine starlarkTargetArgs; public TestActionBuilder(RuleContext ruleContext) { this.ruleContext = ruleContext; this.extraEnv = new TreeMap<>(); this.extraInheritedEnv = new TreeSet<>(); this.additionalTools = new ImmutableList.Builder<>(); + this.starlarkTargetArgs = null; } /** @@ -180,6 +183,12 @@ public TestActionBuilder addExtraInheritedEnv(List extraInheritedEnv) { return this; } + @CanIgnoreReturnValue + public TestActionBuilder setStarlarkTargetArgs(@Nullable CommandLine starlarkTargetArgs) { + this.starlarkTargetArgs = starlarkTargetArgs; + return this; + } + /** Set the explicit shard count. Note that this may be overridden by the sharding strategy. */ @CanIgnoreReturnValue public TestActionBuilder setShardCount(int explicitShardCount) { @@ -340,6 +349,7 @@ private TestParams createTestAction(int shards) ruleContext, runfilesSupport, executable, + starlarkTargetArgs, instrumentedFileManifest, shards, runsPerTest); @@ -352,7 +362,13 @@ private TestParams createTestAction(int shards) } else { executionSettings = new TestTargetExecutionSettings( - ruleContext, runfilesSupport, executable, null, shards, runsPerTest); + ruleContext, + runfilesSupport, + executable, + starlarkTargetArgs, + null, + shards, + runsPerTest); } extraTestEnv.putAll(extraEnv); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java index 595f3342cb194d..c4e28fb59a11e4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java @@ -53,6 +53,7 @@ public final class TestTargetExecutionSettings { RuleContext ruleContext, RunfilesSupport runfilesSupport, Artifact executable, + @Nullable CommandLine starlarkTargetArgs, Artifact instrumentedFileManifest, int shards, int runs) @@ -62,7 +63,8 @@ public final class TestTargetExecutionSettings { BuildConfigurationValue config = ruleContext.getConfiguration(); TestConfiguration testConfig = config.getFragment(TestConfiguration.class); - CommandLine targetArgs = runfilesSupport.getArgs(); + CommandLine targetArgs = + starlarkTargetArgs != null ? starlarkTargetArgs : runfilesSupport.getArgs(); testArguments = CommandLine.concat(targetArgs, ImmutableList.copyOf(testConfig.getTestArguments())); diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java index 20335fb6f35095..306282fdbf6e3e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java @@ -49,6 +49,7 @@ public RunEnvironmentInfo testEnvironment( Dict.cast(environment, String.class, String.class, "environment"), StarlarkList.immutableCopyOf( Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")), + /* arguments= */null, /* shouldErrorOnNonExecutableRule= */ false); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 99bab5bfea0c2b..979f2a440d33ed 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -176,19 +176,28 @@ protected BuildResult processRequest(final CommandEnvironment env, BuildRequest public void editOptions(OptionsParser optionsParser) { } /** - * Compute the arguments the binary should be run with by concatenating the arguments in its - * {@code args} attribute and the arguments on the Blaze command line. + * Compute the arguments the binary should be run with by concatenating the arguments provided by + * its {@link RunEnvironmentInfo} or, if not set, in its {@code args} attribute, and the arguments + * on the Blaze command line. */ - @Nullable private List computeArgs(ConfiguredTarget targetToRun, List commandLineArgs) throws InterruptedException, CommandLineExpansionException { List args = Lists.newArrayList(); - FilesToRunProvider provider = targetToRun.getProvider(FilesToRunProvider.class); - RunfilesSupport runfilesSupport = provider == null ? null : provider.getRunfilesSupport(); - if (runfilesSupport != null && runfilesSupport.getArgs() != null) { - CommandLine targetArgs = runfilesSupport.getArgs(); - Iterables.addAll(args, targetArgs.arguments()); + // If a Starlark rule explicitly sets arguments on RunEnvironmentInfo, these arguments take + // precedence over those obtained from the implicit "args" attribute on all rules so that + // Starlark rules can implement their own logic for this attribute. + RunEnvironmentInfo runEnvironmentInfo = + (RunEnvironmentInfo) targetToRun.get(RunEnvironmentInfo.PROVIDER.getKey()); + if (runEnvironmentInfo != null && runEnvironmentInfo.getArguments() != null) { + Iterables.addAll(args, runEnvironmentInfo.getArguments().arguments()); + } else { + FilesToRunProvider provider = targetToRun.getProvider(FilesToRunProvider.class); + RunfilesSupport runfilesSupport = provider == null ? null : provider.getRunfilesSupport(); + if (runfilesSupport != null && runfilesSupport.getArgs() != null) { + CommandLine targetArgs = runfilesSupport.getArgs(); + Iterables.addAll(args, targetArgs.arguments()); + } } args.addAll(commandLineArgs); return args; diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java index 3de4fca081a126..f3628a19bcab9d 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java @@ -26,11 +26,12 @@ import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.NoneType; import net.starlark.java.eval.Sequence; /** - * Provider containing any additional environment variables for use when running executables, either - * in test actions or when executed via the run command. + * Provider containing any additional environment variables and arguments for use when running + * executables, either in test actions or when executed via the run command. */ @StarlarkBuiltin( name = "RunEnvironmentInfo", @@ -78,7 +79,9 @@ interface RunEnvironmentInfoApiProvider extends ProviderApi { doc = "A map of string keys and values that represent environment variables and their" + " values. These will be made available when the target that returns this" - + " provider is executed, either as a test or via the run command."), + + " provider is executed, either as a test or via the run command. In the" + + " case of a test, environment variables specified on the command line via" + + " --test_env take precedence over values specified here."), @Param( name = "inherited_environment", allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)}, @@ -91,13 +94,29 @@ interface RunEnvironmentInfoApiProvider extends ProviderApi { + " when the target that returns this provider is executed, either as a" + " test or via the run command. If a variable is contained in both " + "environment and inherited_environment, the value" - + " inherited from the shell environment will take precedence if set.") + + " inherited from the shell environment will take precedence if set."), + @Param( + name = "arguments", + allowedTypes = { + @ParamType(type = Sequence.class, generic1 = String.class), + @ParamType(type = NoneType.class) + }, + defaultValue = "None", + named = true, + positional = false, + doc = "A list of arguments. If set to a value other than None, this" + + " list will be appended to the command line when the target that returns" + + " this provider is executed, either as a test or via the run command. In" + + " this case, the arguments specified in the args attribute" + + " implicitly defined for all rules are not appended" + + " automatically.") }, selfCall = true) @StarlarkConstructor RunEnvironmentInfoApi constructor( Dict environment, // expected - Sequence inheritedEnvironment /* expected */) + Sequence inheritedEnvironment, /* expected */ + Object arguments /* Sequence or NoneType expected */) throws EvalException; } } diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java index 21526932b03694..34eebe622bef3f 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java @@ -3602,6 +3602,43 @@ public void testStarlarkRulePropagatesRunEnvironmentProvider() throws Exception assertThat(provider.getEnvironment()).containsExactly("FIXED", "fixed"); assertThat(provider.getInheritedEnvironment()).containsExactly("INHERITED"); + assertThat(provider.getArguments()).isNull(); + } + + @Test + public void testStarlarkRuleCanOverrideArgs() throws Exception { + scratch.file( + "examples/rules.bzl", + "def my_rule_impl(ctx):", + " script = ctx.actions.declare_file(ctx.attr.name)", + " ctx.actions.write(script, '', is_executable = True)", + " run_env = RunEnvironmentInfo(", + " arguments = ['from', '\\'star lark'],", + " )", + " return [", + " DefaultInfo(executable = script),", + " run_env,", + " ]", + "my_rule = rule(", + " implementation = my_rule_impl,", + " attrs = {},", + " executable = True,", + ")"); + scratch.file( + "examples/BUILD", + "load(':rules.bzl', 'my_rule')", + "my_rule(", + " name = 'my_target',", + " args = ['magic', '$(location //:invalid_should_not_be_evaluated)'],", + ")"); + + ConfiguredTarget starlarkTarget = getConfiguredTarget("//examples:my_target"); + RunEnvironmentInfo provider = starlarkTarget.get(RunEnvironmentInfo.PROVIDER); + + assertThat(provider.getEnvironment()).isEmpty(); + assertThat(provider.getInheritedEnvironment()).isEmpty(); + assertThat(provider.getArguments()).isNotNull(); + assertThat(provider.getArguments().arguments()).containsExactly("from", "'star lark"); } @Test diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh index f4d69224e261e4..10cb6be2a0abee 100755 --- a/src/test/shell/bazel/bazel_rules_test.sh +++ b/src/test/shell/bazel/bazel_rules_test.sh @@ -593,12 +593,13 @@ EOF assert_contains "hello world" bazel-bin/pkg/hello_gen.txt } -function test_starlark_test_with_test_environment() { +function test_starlark_test_with_run_environment() { mkdir pkg cat >pkg/BUILD <<'EOF' load(":rules.bzl", "my_test") my_test( name = "my_test", + args = ["magic1", "magic2"], ) EOF @@ -608,22 +609,42 @@ EOF cat >pkg/rules.bzl <<'EOF' _SCRIPT_EXT = ".bat" _SCRIPT_CONTENT = """@ECHO OFF +set +echo "Arguments: %*" if not "%FIXED_ONLY%" == "fixed" exit /B 1 if not "%FIXED_AND_INHERITED%" == "inherited" exit /B 1 if not "%FIXED_AND_INHERITED_BUT_NOT_SET%" == "fixed" exit /B 1 if not "%INHERITED_ONLY%" == "inherited" exit /B 1 +if not "%TEST_ENV_FIXED%" == "fixed" exit /B 1 +if not "%TEST_ENV_INHERITED%" == "inherited" exit /B 1 +if not "%TEST_ENV_OVERRIDDEN%" == "overridden" exit /B 1 if defined INHERITED_BUT_UNSET exit /B 1 +if not "%1" == "starlark1" exit /B 1 +if not "%2" == "starlark2" exit /B 1 +if not "%3" == "test_arg1" exit /B 1 +if not "%4" == "test_arg2" exit /B 1 +if not "%~5" == "" exit /B 1 """ EOF else cat >pkg/rules.bzl <<'EOF' _SCRIPT_EXT = ".sh" _SCRIPT_CONTENT = """#!/bin/bash +env +echo "Arguments: $@" [[ "$FIXED_ONLY" == "fixed" \ && "$FIXED_AND_INHERITED" == "inherited" \ && "$FIXED_AND_INHERITED_BUT_NOT_SET" == "fixed" \ && "$INHERITED_ONLY" == "inherited" \ - && -z "${INHERITED_BUT_UNSET+default}" ]] + && "$TEST_ENV_FIXED" == "fixed" \ + && "$TEST_ENV_INHERITED" == "inherited" \ + && "$TEST_ENV_OVERRIDDEN" == "overridden" \ + && -z "${INHERITED_BUT_UNSET+default}" \ + && "$1" == "starlark1" \ + && "$2" == "starlark2" \ + && "$3" == "test_arg1" \ + && "$4" == "test_arg2" \ + && "$#" -eq 4 ]] """ EOF fi @@ -636,18 +657,20 @@ def _my_test_impl(ctx): content = _SCRIPT_CONTENT, is_executable = True, ) - test_env = testing.TestEnvironment( + test_env = RunEnvironmentInfo( { "FIXED_AND_INHERITED": "fixed", "FIXED_AND_INHERITED_BUT_NOT_SET": "fixed", "FIXED_ONLY": "fixed", + "TEST_ENV_OVERRIDDEN": "not_overridden", }, [ "FIXED_AND_INHERITED", "FIXED_AND_INHERITED_BUT_NOT_SET", "INHERITED_ONLY", "INHERITED_BUT_UNSET", - ] + ], + arguments = ["starlark1", "starlark2"], ) return [ DefaultInfo( @@ -663,8 +686,10 @@ my_test = rule( ) EOF - FIXED_AND_INHERITED=inherited INHERITED_ONLY=inherited \ - bazel test //pkg:my_test >$TEST_log 2>&1 || fail "Test should pass" + FIXED_AND_INHERITED=inherited INHERITED_ONLY=inherited TEST_ENV_INHERITED=inherited \ + bazel test --test_env=TEST_ENV_FIXED=fixed --test_env=TEST_ENV_INHERITED --test_env=TEST_ENV_OVERRIDDEN=overridden \ + --test_arg=test_arg1 --test_arg=test_arg2 \ + //pkg:my_test --test_output=errors >$TEST_log 2>&1 || fail "Test should pass" } function test_starlark_rule_with_run_environment() { @@ -673,6 +698,7 @@ function test_starlark_rule_with_run_environment() { load(":rules.bzl", "my_executable") my_executable( name = "my_executable", + args = ["magic1", "magic2"], ) EOF @@ -687,6 +713,9 @@ if not "%FIXED_AND_INHERITED%" == "inherited" exit /B 1 if not "%FIXED_AND_INHERITED_BUT_NOT_SET%" == "fixed" exit /B 1 if not "%INHERITED_ONLY%" == "inherited" exit /B 1 if defined INHERITED_BUT_UNSET exit /B 1 +if not "%1" == "starlark1" exit /B 1 +if not "%2" == "starlark2" exit /B 1 +if not "%~3" == "" exit /B 1 """ EOF else @@ -699,7 +728,10 @@ env && "$FIXED_AND_INHERITED" == "inherited" \ && "$FIXED_AND_INHERITED_BUT_NOT_SET" == "fixed" \ && "$INHERITED_ONLY" == "inherited" \ - && -z "${INHERITED_BUT_UNSET+default}" ]] + && -z "${INHERITED_BUT_UNSET+default}" \ + && "$1" == "starlark1" \ + && "$2" == "starlark2" \ + && "$#" -eq 2 ]] """ EOF fi @@ -723,7 +755,8 @@ def _my_executable_impl(ctx): "FIXED_AND_INHERITED_BUT_NOT_SET", "INHERITED_ONLY", "INHERITED_BUT_UNSET", - ] + ], + arguments = ["starlark1", "starlark2"], ) return [ DefaultInfo(