Forward exec_properties to main test spawn's execution info

This makes `exec_properties` such as `no-remote-exec` effective for the main test spawn, where they previously only affected post-processing spawns such as test XML generation.

Pros:
* relatively simple (and more precise than `tags`) way to affect test execution using existing API
* provides consistency between test spawns
* provides consistency between test spawns and spawns produced by Starlark actions

Cons:
* similar to Starlark actions, execution info is "polluted" with `exec_properties` such as `OSFamily` and `container-image`, which shows up in `aquery` and may increase memory usage
* the fact that `exec_properties` end up in execution info appears to have been an unintentional side effect of bazelbuild@adb56cc rather than a conscious design decision
* provides functionality may be better addressed by [Execution Platform Scoped Spawn Strategies](https://github.com/bazelbuild/proposals/blob/main/designs/2023-06-04-exec-platform-scoped-spawn-strategies.md)

Closes bazelbuild#24979.

PiperOrigin-RevId: 721088264
Change-Id: I7ce5c899ed6a647831f156f6b34c1f776655425c

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -112,8 +112,7 @@ public TestRunnerSpawn createTestRunnerSpawn(
         createEnvironment(
             actionExecutionContext, action, tmpDirRoot, executionOptions.splitXmlGeneration);
 
-    Map<String, String> executionInfo =
-        new TreeMap<>(action.getTestProperties().getExecutionInfo());
+    Map<String, String> executionInfo = new TreeMap<>(action.getExecutionInfo());
     if (!action.shouldAcceptCachedResult()) {
       // TODO(tjgq): We want to reject a previously cached result, but not prevent the result of the
       // current execution from being uploaded. We should introduce a separate execution requirement

src/test/java/com/google/devtools/build/lib/analysis/StarlarkExecGroupTest.java

@@ -18,12 +18,14 @@
 import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
 import static com.google.devtools.build.lib.skyframe.BzlLoadValue.keyForBuild;
 
+import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Provider;
 import com.google.devtools.build.lib.packages.StarlarkProvider;
 import com.google.devtools.build.lib.packages.StructImpl;
+import com.google.devtools.build.lib.testutil.TestConstants;
 import java.io.IOException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -707,4 +709,167 @@ public void ruleOverridePlatformExecGroupExecProperty() throws Exception {
     assertThat(getGeneratingAction(target, "test/out.txt").getOwner().getExecProperties())
         .containsExactly("ripeness", "unknown");
   }
+
+  @Test
+  public void testRuleExecGroup() throws Exception {
+    scratch.file(
+        "rule/my_toolchain.bzl",
+        """
+        def _impl(ctx):
+            return [platform_common.ToolchainInfo(label = ctx.label)]
+
+        my_toolchain = rule(
+            implementation = _impl,
+        )
+        """);
+    scratch.file(
+        "rule/BUILD",
+        """
+        toolchain_type(name = "toolchain_type")
+        """);
+    scratch.file(
+        "toolchain/BUILD",
+        """
+        load("//rule:my_toolchain.bzl", "my_toolchain")
+
+        my_toolchain(
+            name = "target_target",
+        )
+
+        toolchain(
+            name = "target_target_toolchain",
+            exec_compatible_with = ["CONSTRAINTS_PACKAGE_ROOTos:linux"],
+            target_compatible_with = ["CONSTRAINTS_PACKAGE_ROOTos:linux"],
+            toolchain = ":target_target",
+            toolchain_type = "//rule:toolchain_type",
+        )
+
+        my_toolchain(
+            name = "exec_target",
+        )
+
+        toolchain(
+            name = "exec_target_toolchain",
+            exec_compatible_with = ["CONSTRAINTS_PACKAGE_ROOTos:macos"],
+            target_compatible_with = ["CONSTRAINTS_PACKAGE_ROOTos:linux"],
+            toolchain = ":exec_target",
+            toolchain_type = "//rule:toolchain_type",
+        )
+        """
+            .replace("CONSTRAINTS_PACKAGE_ROOT", TestConstants.CONSTRAINTS_PACKAGE_ROOT));
+
+    scratch.overwriteFile(
+        "platform/BUILD",
+        """
+        constraint_setting(
+            name = "fast_cpu",
+            default_constraint_value = ":no_fast_cpu",
+        )
+
+        constraint_value(
+            name = "no_fast_cpu",
+            constraint_setting = ":fast_cpu",
+        )
+
+        constraint_value(
+            name = "has_fast_cpu",
+            constraint_setting = ":fast_cpu",
+        )
+
+        platform(
+            name = "target_platform",
+            constraint_values = [
+                "CONSTRAINTS_PACKAGE_ROOTos:linux",
+            ],
+        )
+
+        platform(
+            name = "fast_cpu_platform",
+            constraint_values = [
+                "CONSTRAINTS_PACKAGE_ROOTos:macos",
+                ":has_fast_cpu",
+            ],
+            exec_properties = {
+                "require_fast_cpu": "true",
+            },
+        )
+        """
+            .replace("CONSTRAINTS_PACKAGE_ROOT", TestConstants.CONSTRAINTS_PACKAGE_ROOT));
+
+    scratch.file(
+        "test/defs.bzl",
+        """
+        MyInfo = provider(fields = ["toolchain_label"])
+
+        def _impl(ctx):
+            executable = ctx.actions.declare_file(ctx.label.name)
+            ctx.actions.run_shell(
+                outputs = [executable],
+                command = "touch $1",
+                arguments = [executable.path],
+            )
+            return [
+                DefaultInfo(
+                    executable = executable,
+                ),
+                MyInfo(
+                    toolchain_label = ctx.toolchains["//rule:toolchain_type"].label,
+                ),
+            ]
+
+        my_cc_test = rule(
+            implementation = _impl,
+            test = True,
+            toolchains = ["//rule:toolchain_type"],
+        )
+        """);
+
+    scratch.file(
+        "test/BUILD",
+        """
+        load("//test:defs.bzl", "my_cc_test")
+
+        my_cc_test(
+            name = "my_test",
+            exec_compatible_with = [
+                "//platform:has_fast_cpu",
+            ],
+            exec_properties = {
+                "test.require_gpu": "true",
+            },
+        )
+        """);
+
+    useConfiguration(
+        "--extra_toolchains=//toolchain:target_target_toolchain,//toolchain:exec_target_toolchain",
+        "--platforms=//platform:target_platform",
+        "--extra_execution_platforms=//platform:target_platform,//platform:fast_cpu_platform");
+
+    ConfiguredTarget target = getConfiguredTarget("//test:my_test");
+
+    Provider.Key key =
+        new StarlarkProvider.Key(keyForBuild(Label.parseCanonical("//test:defs.bzl")), "MyInfo");
+    Label toolchainLabel = (Label) ((StructImpl) target.get(key)).getValue("toolchain_label");
+    assertThat(toolchainLabel).isEqualTo(Label.parseCanonicalUnchecked("//toolchain:exec_target"));
+
+    Action compileAction = getGeneratingAction(target, "test/my_test");
+    assertThat(compileAction.getExecutionPlatform().label())
+        .isEqualTo(Label.parseCanonicalUnchecked("//platform:fast_cpu_platform"));
+    assertThat(compileAction.getExecProperties()).containsExactly("require_fast_cpu", "true");
+
+    Action testAction =
+        getActions("//test:my_test").stream()
+            .filter(action -> action.getMnemonic().equals("TestRunner"))
+            .findFirst()
+            .orElseThrow();
+    // This is NOT the desired behavior: the test action should not be affected by
+    // the exec_compatible_with constraint and thus select the target platform.
+    // TODO: Change this as soon as exec_group_compatible_with is available, which provides an
+    // explicit way to specify additional constraints for the test exec group.
+    // https://github.com/bazelbuild/bazel/issues/23802
+    assertThat(testAction.getExecutionPlatform().label())
+        .isEqualTo(Label.parseCanonicalUnchecked("//platform:fast_cpu_platform"));
+    assertThat(testAction.getExecProperties())
+        .containsExactly("require_fast_cpu", "true", "require_gpu", "true");
+  }
 }

src/test/shell/bazel/remote/BUILD

@@ -28,6 +28,7 @@ sh_test(
         "//src/test/shell/bazel:test-deps",
         "//src/tools/remote:worker",
         "@bazel_tools//tools/bash/runfiles",
+        "@local_jdk//:jdk",  # for remote_helpers setup_localjdk_javabase
     ],
     shard_count = 5,
     tags = [

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3449,7 +3449,29 @@ function test_platform_no_remote_exec_test_action() {
 exit 0
 EOF
   chmod 755 a/test.sh
+  cat > a/rule.bzl <<'EOF'
+def _my_test(ctx):
+  script = ctx.actions.declare_file(ctx.label.name)
+  ctx.actions.run_shell(
+    outputs = [script],
+    command = """
+cat > $1 <<'EOF2'
+#!/usr/bin/env sh
+exit 0
+EOF2
+chmod +x $1
+""",
+    arguments = [script.path],
+  )
+  return [DefaultInfo(executable = script)]
+my_test = rule(
+  implementation = _my_test,
+  test = True,
+)
+EOF
   cat > a/BUILD <<'EOF'
+load(":rule.bzl", "my_test")
+
 constraint_setting(name = "foo")
 
 constraint_value(
@@ -3462,7 +3484,7 @@ platform(
     name = "host",
     constraint_values = [":has_foo"],
     exec_properties = {
-        "no-remote-exec": "1",
+        "test.no-remote-exec": "1",
     },
     parents = ["@bazel_tools//tools:host_platform"],
     visibility = ["//visibility:public"],
@@ -3480,44 +3502,46 @@ platform(
     },
 )
 
-sh_test(
+my_test(
     name = "test",
-    srcs = ["test.sh"],
+    # TODO: This uses a hack by setting test.no-remote-exec on the exec platform
+    # forced by this constraint for both the build and the test action. Instead,
+    # use exec_group_compatible_with = {"test": [":has_foo"]} once it is
+    # implemented.
+    # https://github.com/bazelbuild/bazel/issues/23802
     exec_compatible_with = [":has_foo"],
 )
 
-sh_test(
+my_test(
     name = "test2",
-    srcs = ["test.sh"],
-    exec_compatible_with = [":has_foo"],
-    target_compatible_with = [":has_foo"],
+    exec_properties = {
+        "test.no-remote-exec": "1",
+    },
 )
 EOF
 
-  # A test target includes 2 actions: 1 build action (a) and 1 test action (b)
-  # This test currently demonstrates that:
-  #  - (b) would always be executed on Bazel's target platform, set by "--platforms=" flag.
-  #  - Regardless of 'no-remote-exec' set on (b)'s platform, (b) would still be executed remotely.
-  #    The remote test action will be sent with `"no-remote-exec": "1"` in it's platform.
-  #
-  # TODO: Make this test's result consistent with 'test_platform_no_remote_exec'.
-  # Test action (b) should be executed locally instead of remotely in this setup.
-
+  # A my_test target includes 2 actions: 1 build action (a) and 1 test action (b),
+  # with (b) running two spawns (test execution, test XML generation).
+  # The genrule spawn runs remotely, both test spawns run locally.
   bazel test \
     --extra_execution_platforms=//a:remote,//a:host \
     --platforms=//a:remote \
     --spawn_strategy=remote,local \
     --remote_executor=grpc://localhost:${worker_port} \
     //a:test >& $TEST_log || fail "Failed to test //a:test"
-  expect_log "1 local, 1 remote"
+  expect_log "2 local, 1 remote"
 
+  # Note: While the test spawns are executed locally, they still select the
+  # remote platform as it is the first registered execution platform and there
+  # are no constraints to force a different one. This is not desired behavior,
+  # but it isn't covered by this test.
   bazel test \
     --extra_execution_platforms=//a:remote,//a:host \
     --platforms=//a:host \
     --spawn_strategy=remote,local \
     --remote_executor=grpc://localhost:${worker_port} \
     //a:test2 >& $TEST_log || fail "Failed to test //a:test2"
-  expect_log "1 local, 1 remote"
+  expect_log "2 local, 1 remote"
 
   bazel clean
 
@@ -3527,15 +3551,15 @@ EOF
     --spawn_strategy=remote,local \
     --remote_executor=grpc://localhost:${worker_port} \
     //a:test >& $TEST_log || fail "Failed to test //a:test"
-  expect_log "2 remote cache hit"
+  expect_log "3 remote cache hit"
 
   bazel test \
     --extra_execution_platforms=//a:remote,//a:host \
     --platforms=//a:host \
     --spawn_strategy=remote,local \
     --remote_executor=grpc://localhost:${worker_port} \
     //a:test2 >& $TEST_log || fail "Failed to test //a:test2"
-  expect_log "2 remote cache hit"
+  expect_log "3 remote cache hit"
 }
 
 function setup_inlined_outputs() {