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": [],
+ }),
)