Open UebelAndre opened 2 years ago
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.
Is RunEnvironmentInfo
available to starlark rules somehow?
Yes, other than testing.TestEnvironment
, it's a regular Starlark-accessible provider available in Bazel 5.3.0.
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.
@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.
@fmeum sorry for the delay. Seems you've run into this issue too?
Yes, resolving it is required to allow for generic transition wrappers around tests, which is something I'm interested in.
So how should it be implemented? Do you have bandwidth to work on this?
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:
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.
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 https://github.com/bazelbuild/bazel/issues/12313 for starlark rules.
@comius @UebelAndre I would be very interested in your opinions on this.
re https://github.com/bazelbuild/bazel/issues/16076#issuecomment-1242961552
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?
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.
Any word from @comius on this? Do I understand correctly that progress is blocked on gaining consensus for the design?
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.
@UebelAndre I just submitted https://github.com/bazelbuild/bazel/pull/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 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?
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 havefoo_binary
which returnsRunEnvironmentInfo(arguments=["foo"])
as a dependency ofbar_binary
which tries to index theRunEnvironmentInfo
provider and readarguments
. 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.
I see, that sounds totally reasonable to me! Thank you so much for working on this!
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.
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 somearguments
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
returnsdevelopment 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