--experimental_add_exec_constraints_to_targets no longer works with test targets

[keith]

Description of the bug:

Previously I was using something like --experimental_add_exec_constraints_to_targets='\.test'=//:has_gpu to force the matching test targets to exec on a specific remote platform in some cases. I bisected to 38ee78c

cc @fmeum

I might be able to solve this use case a different way now, but still worth noting that this changed.

In my case I want some subset of targets to support 2 different modes, 1 with this constraint and 1 without it, so I add this only in specific CI workflows to make sure it gets tested on multiple exec platforms. The most obvious alternative is to use a macro to create multiple test targets that are defined with this constraint, and a negation of this constraint, but there are UX downsides to that.

Which category does this issue belong to?

No response

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

No response

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

95bbed0

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 HEAD ?

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

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]

The additional constraints previously also applied to the default exec group, e.g., the compilation action for your cc_test. I assume that wasn't intentional, but happened to be fine?

I can make it so that you can specify --experimental_add_exec_constraints_to_targets='\.test'=test.//:has_gpu to apply the constraint to the test exec group.

[fmeum]

The problem with this is that automatic exec group haves names that are themselves labels and may contain .s, so we can't just split on those.

@katre Do you have an idea for reasonable syntax for this flag?

[katre]

I would suggest using the macro to create two different test targets: this fits with our overall goal to reduce the use of flags and make more things explicit in BUILD files.

In general, we don't support the use of --experimental_add_exec_constraints_to_targets, and I would like to remove it before Bazel 9.0 is cut. I'm not sure it's worth adding new modes to that flag.

[keith]

The additional constraints previously also applied to the default exec group, e.g., the compilation action for your cc_test. I assume that wasn't intentional, but happened to be fine?

correct

I would suggest using the macro to create two different test targets: this fits with our overall goal to reduce the use of flags and make more things explicit in BUILD files.

yea like i said the annoying part of this is just that you have to then have a new set of constraints to make sure you don't test all tests twice

Interestingly I guess --use_target_platform_for_tests also potentially solves this case, but I probably don't want to rely on that either