Add more tests for run_shell and actually run them (#61)

Also clean up the overall CI config.
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 3e84318..6bec873 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -1,49 +1,64 @@
 ---
-buildifier: latest
+# Avoid https://github.com/bazelbuild/buildtools/commit/4006b543a694f6cf77c2d8cc188c5f53f3bac1d9
+buildifier: 8.2.1
 
 matrix:
   platform:
     - ubuntu2004
     - macos
     - windows
-  bazel:
-    - 6.x
-    - 7.x
-    - 8.x
+  legacy_bazel:
+    - 7.*
+    - 8.*
+  modern_bazel:
+    - 9.*
+    - last_green
 
 tasks:
-  test_module_bzlmod:
-    name: "Test module (Bzlmod)"
+  test_module_bzlmod_legacy:
+    name: "Test module (${{legacy_bazel}}, Bzlmod)"
     working_directory: "tests/bcr"
-    bazel: ${{ bazel }}
+    bazel: ${{ legacy_bazel }}
     platform: ${{ platform }}
     build_flags:
       - "--enable_bzlmod"
+      - "--noenable_workspace"
     build_targets:
       - "//..."
     test_flags:
       - "--enable_bzlmod"
+      - "--noenable_workspace"
     test_targets:
       - "//..."
   test_module_workspace:
-    name: "Test module (WORKSPACE)"
+    name: "Test module (${{legacy_bazel}}, WORKSPACE)"
     working_directory: "tests/bcr"
-    bazel: ${{ bazel }}
+    bazel: ${{ legacy_bazel }}
     platform: ${{ platform }}
     build_flags:
-      - "--enable_bzlmod"
+      - "--noenable_bzlmod"
+      - "--enable_workspace"
     build_targets:
       - "//..."
     test_flags:
-      - "--enable_bzlmod"
+      - "--noenable_bzlmod"
+      - "--enable_workspace"
     test_targets:
       - "//..."
-  test_module_head:
-    name: "Test module (Bazel@HEAD, Bzlmod)"
+  test_module_modern:
+    name: "Test module (${{modern_bazel}}, Bzlmod)"
     working_directory: "tests/bcr"
-    bazel: last_green
+    bazel: ${{ modern_bazel }}
     platform: ${{ platform }}
     build_targets:
       - "//..."
     test_targets:
       - "//..."
+  tests:
+    name: "Tests (${{modern_bazel}})"
+    bazel: ${{ modern_bazel }}
+    platform: ${{ platform }}
+    build_targets:
+      - "//tests/..."
+    test_targets:
+      - "//tests/..."
diff --git a/.bazelrc b/.bazelrc
new file mode 100644
index 0000000..a535e14
--- /dev/null
+++ b/.bazelrc
@@ -0,0 +1,2 @@
+common --check_direct_dependencies=off
+common --keep_going
diff --git a/.bazelversion b/.bazelversion
index 2b0aa21..44931da 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-8.2.1
+9.1.1
diff --git a/.gitignore b/.gitignore
index 0d4fed2..c0c6e52 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,3 @@
 bazel-*
 MODULE.bazel.lock
+.idea
diff --git a/MODULE.bazel b/MODULE.bazel
index 1dde16c..c264ce0 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -12,6 +12,6 @@
 
 register_toolchains("@local_config_shell//:all")
 
-bazel_dep(name = "rules_shellcheck", version = "0.3.3", dev_dependency = True)
+bazel_dep(name = "rules_shellcheck", version = "0.6.2", dev_dependency = True)
 bazel_dep(name = "rules_testing", version = "0.9.0", dev_dependency = True)
 bazel_dep(name = "with_cfg.bzl", version = "0.14.1", dev_dependency = True)
diff --git a/tests/bcr/WORKSPACE b/tests/bcr/WORKSPACE
index 859c4a7..77a42f9 100644
--- a/tests/bcr/WORKSPACE
+++ b/tests/bcr/WORKSPACE
@@ -10,3 +10,16 @@
 rules_shell_dependencies()
 
 rules_shell_toolchains()
+
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+    name = "bazel_features",
+    sha256 = "89eca73d4c334cf664f84920365d2ce04e2c98099b89f7c5b676b5f377c8e754",
+    strip_prefix = "bazel_features-1.48.1",
+    url = "https://github.com/bazel-contrib/bazel_features/releases/download/v1.48.1/bazel_features-v1.48.1.tar.gz",
+)
+
+load("@bazel_features//:deps.bzl", "bazel_features_deps")
+
+bazel_features_deps()
diff --git a/tests/run_shell/BUILD.bazel b/tests/run_shell/BUILD.bazel
index c6ad417..3fb5064 100644
--- a/tests/run_shell/BUILD.bazel
+++ b/tests/run_shell/BUILD.bazel
@@ -9,6 +9,16 @@
 
 run_shell_test_suite(name = "run_shell_tests")
 
+platform(
+    name = "linux_exec",
+    constraint_values = ["@platforms//os:linux"],
+)
+
+platform(
+    name = "windows_exec",
+    constraint_values = ["@platforms//os:windows"],
+)
+
 run_shell_output(
     name = "basic_output",
     command = "echo 'hello world' > $1",
diff --git a/tests/run_shell/exec_fixtures.bzl b/tests/run_shell/exec_fixtures.bzl
index 886705e..71943df 100644
--- a/tests/run_shell/exec_fixtures.bzl
+++ b/tests/run_shell/exec_fixtures.bzl
@@ -44,11 +44,12 @@
     out = ctx.actions.declare_file(ctx.label.name + ".out")
 
     # A command well above the 64,000 char (8,000 on Windows) threshold at which
-    # run_shell spills the command into a helper script. All but the last
-    # statement are no-ops, so the output is deterministic. The output path is
-    # embedded directly because positional arguments are not forwarded into the
-    # helper script.
-    command = "( %s ; echo done ) > %s" % (" ; ".join(["true"] * 20000), out.path)
+    # run_shell spills the command into a helper script. The length comes from a
+    # trailing comment rather than many statements: a deeply nested command list
+    # (e.g. tens of thousands of `;`-separated commands) overflows the stack of
+    # the smaller-stacked bash on Windows. The output path is embedded directly
+    # because positional arguments are not forwarded into the helper script.
+    command = "echo done > %s #%s" % (out.path, "x" * 70000)
     ctx.actions.run_shell(
         outputs = [out],
         command = command,
diff --git a/tests/run_shell/fixtures.bzl b/tests/run_shell/fixtures.bzl
index 8f7ca51..231851e 100644
--- a/tests/run_shell/fixtures.bzl
+++ b/tests/run_shell/fixtures.bzl
@@ -112,3 +112,52 @@
     implementation = _long_command_impl,
     attrs = {},
 )
+
+def _medium_command_impl(ctx):
+    out = ctx.actions.declare_file(ctx.label.name + ".out")
+
+    # A command between the Windows (8000) and non-Windows (64000) command
+    # length limits. It is spilled into a helper script only on Windows.
+    medium_command = "( %s ; ) > $1" % " ; ".join(
+        ["echo zzz%d" % i for i in range(0, 1000)],
+    )
+    ctx.actions.run_shell(
+        outputs = [out],
+        command = medium_command,
+        mnemonic = "MediumMnemonic",
+        arguments = [out.path],
+    )
+    return [DefaultInfo(files = depset([out]))]
+
+medium_command = rule(
+    implementation = _medium_command_impl,
+    attrs = {},
+)
+
+def _two_long_commands_impl(ctx):
+    out1 = ctx.actions.declare_file(ctx.label.name + "1.out")
+    out2 = ctx.actions.declare_file(ctx.label.name + "2.out")
+    command1 = "( %s ; ) > $1" % " ; ".join(
+        ["echo xxx%d" % i for i in range(0, 7000)],
+    )
+    command2 = "( %s ; ) > $1" % " ; ".join(
+        ["echo yyy%d" % i for i in range(0, 7000)],
+    )
+    ctx.actions.run_shell(
+        outputs = [out1],
+        command = command1,
+        mnemonic = "Mnemonic1",
+        arguments = [out1.path],
+    )
+    ctx.actions.run_shell(
+        outputs = [out2],
+        command = command2,
+        mnemonic = "Mnemonic2",
+        arguments = [out2.path],
+    )
+    return [DefaultInfo(files = depset([out1, out2]))]
+
+two_long_commands = rule(
+    implementation = _two_long_commands_impl,
+    attrs = {},
+)
diff --git a/tests/run_shell/run_shell_tests.bzl b/tests/run_shell/run_shell_tests.bzl
index 1130f13..7f08175 100644
--- a/tests/run_shell/run_shell_tests.bzl
+++ b/tests/run_shell/run_shell_tests.bzl
@@ -12,6 +12,8 @@
     "invalid_mnemonic",
     "lazy_args",
     "long_command",
+    "medium_command",
+    "two_long_commands",
 )
 
 def _test_command_string(name):
@@ -153,11 +155,18 @@
     util.helper_target(
         long_command,
         name = name + "_subject",
+        # Force a Unix execution platform so that the longer Unix command length limit applies.
+        exec_compatible_with = ["@platforms//os:linux"],
     )
     analysis_test(
         name = name,
         impl = _test_long_command_uses_helper_script_impl,
         target = name + "_subject",
+        config_settings = {
+            "//command_line_option:extra_execution_platforms": [
+                "//tests/run_shell:linux_exec",
+            ],
+        },
     )
 
 def _test_long_command_uses_helper_script_impl(env, target):
@@ -171,6 +180,81 @@
         matching.file_basename_contains(".run_shell_0.sh"),
     )
 
+def _test_windows_threshold(name):
+    util.helper_target(
+        medium_command,
+        name = name + "_subject",
+        # Force a Windows execution platform so that the shorter Windows command length limit applies.
+        exec_compatible_with = ["@platforms//os:windows"],
+    )
+    analysis_test(
+        name = name,
+        impl = _test_windows_threshold_impl,
+        target = name + "_subject",
+        config_settings = {
+            "//command_line_option:extra_execution_platforms": [
+                "//tests/run_shell:windows_exec",
+            ],
+        },
+    )
+
+def _test_windows_threshold_impl(env, target):
+    helper = env.expect.that_target(target).action_generating(
+        "{package}/{name}.run_shell_0.sh",
+    )
+    helper.content().contains("echo zzz999 ;")
+
+def _test_medium_command_does_not_use_helper_script(name):
+    util.helper_target(
+        medium_command,
+        name = name + "_subject",
+        # Force a Linux execution platform so that the larger non-Windows command length limit applies.
+        exec_compatible_with = ["@platforms//os:linux"],
+    )
+    analysis_test(
+        name = name,
+        impl = _test_medium_command_does_not_use_helper_script_impl,
+        target = name + "_subject",
+        config_settings = {
+            "//command_line_option:extra_execution_platforms": [
+                "//tests/run_shell:linux_exec",
+            ],
+        },
+    )
+
+def _test_medium_command_does_not_use_helper_script_impl(env, target):
+    # On Linux the command length limit is 64000, so a ~14000 char command is
+    # passed inline instead of being spilled into a helper script.
+    action = env.expect.that_target(target).action_named("MediumMnemonic")
+    action.argv().contains_predicate(matching.str_matches("*echo zzz999*"))
+    action.inputs().not_contains_predicate(
+        matching.file_basename_contains(".run_shell_0.sh"),
+    )
+
+def _test_two_run_shell_calls(name):
+    util.helper_target(
+        two_long_commands,
+        name = name + "_subject",
+    )
+    analysis_test(
+        name = name,
+        impl = _test_two_run_shell_calls_impl,
+        target = name + "_subject",
+    )
+
+def _test_two_run_shell_calls_impl(env, target):
+    # Each run_shell call in a rule gets its own helper script, numbered by an
+    # incrementing per-rule counter (run_shell_0.sh, run_shell_1.sh, ...).
+    helper0 = env.expect.that_target(target).action_generating(
+        "{package}/{name}.run_shell_0.sh",
+    )
+    helper0.content().contains("echo xxx6999 ;")
+
+    helper1 = env.expect.that_target(target).action_generating(
+        "{package}/{name}.run_shell_1.sh",
+    )
+    helper1.content().contains("echo yyy6999 ;")
+
 def run_shell_test_suite(name):
     test_suite(
         name = name,
@@ -183,5 +267,8 @@
             _test_invalid_mnemonic_rejected,
             _test_lazy_args,
             _test_long_command_uses_helper_script,
+            _test_windows_threshold,
+            _test_medium_command_does_not_use_helper_script,
+            _test_two_run_shell_calls,
         ],
     )
diff --git a/tests/runfiles/BUILD b/tests/runfiles/BUILD
index 52d9867..0e1d4c4 100644
--- a/tests/runfiles/BUILD
+++ b/tests/runfiles/BUILD
@@ -12,4 +12,10 @@
     data = ["//shell/runfiles:runfiles.bash"],
     format = "gcc",
     severity = "warning",
+    # Fails on Windows with:
+    # '..' is not recognized as an internal or external command, operable program or batch file.
+    target_compatible_with = select({
+        "@platforms//os:windows": ["@platforms//:incompatible"],
+        "//conditions:default": [],
+    }),
 )