Add arguments parameter to testing.TestEnvironment

[UebelAndre]

Description of the feature request:

I would like to write rules which contain an args attribute where users can define a test with unique command line arguments. I think it would make the most sense to expand testing.TestEnvironment to support some arguments field where users can provide a list of arguments (or, stretch goal, an Args object) to describe arguments to be passed to the test when it starts.

What underlying problem are you trying to solve with this feature?

To accomplish this I'm constantly needing to have my tests execute a platform specific bash/batch script that has the arguments hard coded into it but this has an unfortunate drawback that I can no longer use --run_under on the actual test binary. Over all, the complexity needed to solve for this feel unjustified and that instead, I should be able to describe in a provider some arguments to pass to my test.

Which operating system are you running Bazel on?

Linux, MacOS, Windows

What is the output of bazel info release?

release 5.2.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[fmeum]

This would be very useful. Just let me add that testing.TestEnvironment is simply a wrapper function around RunEnvironmentInfo, which is the actual provider. If anyone wants to pick this up, I'm happy to help it come along.

[UebelAndre]

Is RunEnvironmentInfo available to starlark rules somehow?

[fmeum]

Yes, other than testing.TestEnvironment, it's a regular Starlark-accessible provider available in Bazel 5.3.0.

[UebelAndre]

I've tried to implement this feature but it seems to require quite a bit of plumbing and I think (as someone who has zero familiarity with the Bazel code base) would require hand holding or would need to defer to someone more familiar. It's easy enough to locate and update RunEnvironmentInfoApi.java but from there, how to get it into test actions (and I'd assume run invocations) goes into quite a few different classes.

For maintainers and experienced contributors, I'd very much like to see this implemented.

[fmeum]

@UebelAndre I'll be busy next week, but feel free to ping me the week after and I can give you some pointers and/or pair.

[UebelAndre]

@fmeum sorry for the delay. Seems you've run into this issue too?

[fmeum]

Yes, resolving it is required to allow for generic transition wrappers around tests, which is something I'm interested in.

[UebelAndre]

So how should it be implemented? Do you have bandwidth to work on this?

[fmeum]

I think I could work on it, but we should first figure out how we want the new arguments field on RunEnvironmentInfo (the replacement for the deprecated testing.TestEnvironment) to behave in combination with the magic args attribute available on all (including Starlark) rules. I would find the following procedure reasonable:

1. If a rule doesn't advertise RunEnvironmentInfo or doesn't set the arguments field of RunEnvironmentInfo, process the arguments provided in the implicitly defined args attribute (i.e. tokenize them) and use the result to populate the field. As a result, the arguments would finally be available to downstream consumers of the rule without requiring any existing rules to change.

2. If a rule advertises RunEnvironmentInfo with arguments set to a non-None value, this list takes precedence over the arguments provided in args. This allows rules to implement their own logic for handling args or even their own such attribute not called args without forcing tokenization on them. This could help fix *_binary args get splitted #12313 for starlark rules.

@comius @UebelAndre I would be very interested in your opinions on this.

[UebelAndre]

re #16076 (comment)

I think this sounds good. This provider though wouldn't have any impact on actions though, right? RunEnvironmentInfo would only affect run and test invocations. If one wanted arguments to be passed to an action, they would have to read them from RunEnvironmentInfo and explicitly add them to the action?

[fmeum]

Yes, this is only about run and test. Implicitly passing arguments or environment variables to actions seems intrusive, but of course rulesets would be free to use the fields provided by the provider to form their command-lines.

[UebelAndre]

Any word from @comius on this? Do I understand correctly that progress is blocked on gaining consensus for the design?

[fmeum]

In my experience, it's usually easier to gain consensus for a PR than on issue comments. Given that these questions probably don't warrant a design doc yet, I will look into turning my comment from above into code.

[fmeum]

@UebelAndre I just submitted #16430, which implements arguments but does not yet expose it to Starlark. This should address your original request while evading potentially difficult questions around what the right Starlark API representation of such arguments would be and whether this could cause memory issues.

Note that the argument takes in a list of strings rather than an Args object as I believe the latter could lead to confusing situations: After all, it resolves artifacts to their exec paths, which don't really make much sense in the context of bazel test or bazel run. Did you have a particular use case in mind when you mentioned Args as a stretch goal?

[UebelAndre]

@UebelAndre I just submitted #16430, which implements arguments but does not yet expose it to Starlark.

I'm not sure what you mean by this. Looking at the PR it seems like you added the ability to set arguments in starlark rules. Are you saying I'm not able to read that parameter from existing rules? An example being I have foo_binary which returns RunEnvironmentInfo(arguments=["foo"]) as a dependency of bar_binary which tries to index the RunEnvironmentInfo provider and read arguments. This is what's not exposed?

[fmeum]

I'm not sure what you mean by this. Looking at the PR it seems like you added the ability to set arguments in starlark rules. Are you saying I'm not able to read that parameter from existing rules? An example being I have foo_binary which returns RunEnvironmentInfo(arguments=["foo"]) as a dependency of bar_binary which tries to index the RunEnvironmentInfo provider and read arguments. This is what's not exposed?

You can obtain the RunEnvironmentInfo provider instance and e.g. forward it, but you (with this PR) can't get the arguments out of it. Making that possible would require more thought - I don't want to propose an API that lends itself to memory regressions by e.g. encouraging recursive list building.

[UebelAndre]

I see, that sounds totally reasonable to me! Thank you so much for working on this!

[UebelAndre]

This continues to be a seemingly unnecessary pain point for my team. Being able to generate arguments for a test binary in a rule would save a ton of boilerplate code we need to explain to others all the time. I would very much appreciate if something could be done to support this.