diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fbad57..3c1bc59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,12 +85,69 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 + # Arm the external-tool-gated tests (see `TEST_IMPROVEMENT.md` + # §1.2). Before this step nothing installed these tools, so every + # test guarded on them returned early and reported GREEN without + # executing its body — a whole block of real-language-server and + # multi-shell coverage that had never once run in CI. Installing + # them is only half the fix; the `PMACS_REQUIRE_*` variables below + # are what turn a future missing tool back into a failure instead + # of silently restoring the vacuum. + # + # Linux only for now, deliberately. macOS would need the brew + # equivalents and roughly doubles the install cost on the slowest + # leg of the matrix; arming one platform already converts these + # from never-executed to executed, and the second is incremental. + # The tests still skip cleanly on macOS because the variables are + # unset there. + - name: Install external tools that gate acceptance tests (Linux) + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install -y clangd zsh fish lua5.4 + # `locate_lua` looks for `lua` or `luajit` by name; the + # distro package installs `lua5.4` only. + sudo ln -sf "$(command -v lua5.4)" /usr/local/bin/lua + # rust-analyzer belongs HERE, not on the shared toolchain + # step. `components:` there applies to every matrix leg, so it + # would install the binary on macOS too — and *presence*, not + # PMACS_REQUIRE_LSP, is what decides whether a gated test body + # runs. That would have executed the rust-analyzer tests on + # macOS for the first time ever, on the legs that are both the + # CI critical path and the documented flake surface, while + # this lane's text claimed Linux only. + rustup component add rust-analyzer + # Versions are PINNED. `@latest` and bare `npm install -g` + # make CI behaviour drift with upstream releases: a bad gopls + # or yaml-language-server publish then breaks CI with no + # commit in this repository to bisect against. + go install golang.org/x/tools/gopls@v0.16.2 + echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" + npm install -g vscode-langservers-extracted@4.10.0 \ + yaml-language-server@1.15.0 - run: cargo build --all-targets --no-default-features --features ${{ matrix.lua }} # Several acceptance binaries spawn real daemon / PTY child # processes. Keep the harness serial so macOS runners do not # expose cross-test process lifecycle races that are unrelated # to the behavior under test. + # + # PMACS_REQUIRE_* make a missing tool fatal rather than a silent + # skip, exactly as PMACS_REQUIRE_GPU already does for the headless + # render job. Set only where the install step ran. + # + # PMACS_REQUIRE_PYRIGHT is deliberately NOT set and basedpyright + # is deliberately NOT installed: that test has no timeout and + # hangs forever (root cause is the non-interruptible reader-thread + # join in `RuntimeHandles::drop`, already a named deferral in + # `src/process.rs`). This job has no `timeout-minutes`, so arming + # it today would trade a vacuous green for a six-hour hang on four + # legs. It gets armed after the hang fix and the CI timeouts land, + # and its own variable exists so that flip is one line. - run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1 + env: + PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }} + PMACS_REQUIRE_SHELLS: ${{ runner.os == 'Linux' && '1' || '' }} + PMACS_REQUIRE_LUA: ${{ runner.os == 'Linux' && '1' || '' }} - run: cargo test --doc --no-default-features --features ${{ matrix.lua }} # The workspace default member is only the root `pmacs` package, so # the runs above never execute pmacs-protocol's own tests — the diff --git a/docs/active-work.md b/docs/active-work.md index 7114083..dd0016a 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -717,6 +717,109 @@ has **no branch and no framing yet**. warns against quoting a stale figure; it does not replace that section's per-target census, which was not re-derived. +## Test-improvement arc, lane 2 — silent-skip arming + +- Portable branch: `githubsucks/silent-skip-arming`, worktree + `../pmacs-skiparm`. Implements `TEST_IMPROVEMENT.md` §1.2 and §5.4. +- **Base, measured at write time rather than quoted:** + + ``` + $ git log --oneline -1 githubsucks/main + 5e186c7 Merge pull request #193 from levineuwirth/test-improvement-audit + ``` + + The previous revision of this entry said "base measured at write + time, pasted below" and then pasted nothing: the script meant to + substitute it reported success and silently matched no text, and the + claim was not re-read. Recorded because it is the same defect this + ledger keeps catching one level up — **asserting a measurement is not + making one, and a tool reporting success is not the measurement + either.** +- Recovery from a clean checkout: + `git fetch githubsucks && git worktree add ../pmacs-skiparm + -b silent-skip-arming githubsucks/silent-skip-arming`. +- **The defect:** `let Ok(_) = which_binary(x) else { eprintln!(..); + return; }` reports GREEN when the tool is absent, and CI installed + none of the tools. A block of real-language-server and multi-shell + tests had therefore **never once executed their bodies** in CI while + reporting success. A suite that cannot distinguish "passed" from + "never ran" is worse than a missing one, because it reads as + coverage. +- **The fix is the project's own pattern.** `PMACS_REQUIRE_*` already + makes a missing GPU fatal for `vterm_stage3_acceptance`; this adds + `PMACS_REQUIRE_LSP`, `PMACS_REQUIRE_SHELLS` and `PMACS_REQUIRE_LUA`, + plus the CI step that installs the tools. Per-tool variables, not one + blanket flag, so a tool that must stay unarmed keeps its decision + visible at the call site. +- **`basedpyright` is deliberately NOT installed and NOT armed.** Its + test has no timeout and hangs forever — root cause is the + non-interruptible reader-thread join in `RuntimeHandles::drop`, + already a named deferral in `src/process.rs`. The `test` job has no + `timeout-minutes` either. Arming it today would trade a vacuous green + for a six-hour hang across four legs. `PMACS_REQUIRE_PYRIGHT` exists + and is never set, so the flip is one line once lane 4 (the hang) and + lane 3 (timeouts) land. **Do not arm it before both.** +- **A trap found while writing the workflow, not after:** the natural + Actions idiom `${{ runner.os == 'Linux' && '1' || '' }}` sets the + variable to the EMPTY STRING elsewhere, and `var_os().is_some()` is + true for `Some("")`. That would have armed the guard on exactly the + runners with no tools installed. The helper therefore treats empty as + unset. `PMACS_REQUIRE_GPU` has the same latent shape and is safe only + because it is set literally. +- **Verified by execution in all three states**, on a tool genuinely + absent from this machine (`vscode-json-language-server`): unset -> + skips green; armed -> hard failure naming the CI step; empty string + -> skips green. The armed failure is the bite, and on `main` it + cannot occur because no guard exists. +- **The tests pass when they actually run** — which was the open + question, since none of them had. Armed locally: 11 `m6_5` + 8 `m6_8` + REPL tests green, and all six real-LSP tests (clangd x2, gopls x2, + rust-analyzer x2) green individually. +- **rust-analyzer is installed in the Linux-gated step, not via the + toolchain action's `components:`.** The first revision put it there, + which applies to *every* matrix leg — and **presence, not + `PMACS_REQUIRE_LSP`, is what decides whether a gated test body + runs**. That would have executed the two rust-analyzer tests on macOS + for the first time ever, on the legs that are simultaneously the CI + critical path and the documented flake surface, while this entry + claimed Linux only. The variables not being set there would only have + meant absence was tolerated; it would not have kept the tests + skipped. Text and workflow now agree. +- **Tool versions are pinned** (`gopls@v0.16.2`, + `vscode-langservers-extracted@4.10.0`, + `yaml-language-server@1.15.0`). `@latest` and bare `npm install -g` + make CI drift with upstream releases, so a bad publish breaks CI with + no commit here to bisect against. Caching the built `gopls` on the + pinned version is a follow-up, not done here. +- **§1.2 is NOT fully closed by this lane.** The guards arm the + *entry* skip only. `tests/m4_acceptance.rs`'s mid-test rust-analyzer + bail ("workspace likely still indexing; skipping") survives, so even + armed, that test's only assertion can still vanish under load — + precisely when a regression would show. Mid-test skips are their own + shape and want their own pass. +- **Not this lane's to fix, recorded so it is not mistaken for + oversight:** the generated-buffer immutability lane above still reads + "PR #188 OPEN, PROPOSED" and #188 has merged. Rule 4 forbids + relabelling it and permits removal only once its durable facts reach + `docs/agent-handoff.md`, which #188 did not touch — it changed the + framing and this ledger only. So the absorption is genuinely owed, + and the natural carrier is the arc's own next PR (#191, Stage 1), + not a testing lane reaching across into someone else's arc. +- **Follow-up owed after this merges:** delete + `githubsucks/handoff-2026-07-20`. Removing the documentation lane + removes the only pointer to that branch, so nothing will otherwise + remind anyone it still exists on the remote. +- Linux only for now, deliberately: macOS needs the brew equivalents + and roughly doubles install cost on the slowest matrix leg. The + variables stay unset there, so those tests skip cleanly. +- Also removes the **documentation lane**, whose disposition the ledger + left undecided pending confirmation that its branch carried nothing + unique. Confirmed by measurement: `githubsucks/handoff-2026-07-20` is + **1 ahead, 365 behind**, and its entire unique diff is four doc files + at 42 insertions against 88 deletions — merging it would *revert* + current documentation. The section said "whoever confirms the branch + carries nothing unique removes the section"; this is that. + ## Parked lane: kill-ring browser + persistence - Portable branch: `githubsucks/kill-ring-browser` @@ -739,32 +842,6 @@ git worktree add --track \ githubsucks/kill-ring-browser ``` -## Documentation lane — STALE, AND ITS DISPOSITION IS UNDECIDED - -> **Measured 2026-07-28, not inferred:** `githubsucks/handoff-2026-07-20` -> is at `c11d7e7`, **1 commit ahead of `main` and 320 behind**. Its -> whole diff against `main` is four documentation files -> (`docs/active-work.md`, `docs/agent-handoff.md`, -> `docs/roadmap-2026-07.md`, `docs/vterm-framing.md`), every one of -> which has been rewritten repeatedly since by the landed-doc PRs -> #156/#168/#169/#172/#180. Rule 4 removes a lane on merge *or -> abandonment*, and this one looks abandoned in substance — but "looks -> abandoned" is not the same as a decision, and no PR was ever opened -> for it. **This snapshot deliberately annotates rather than deletes: -> whoever confirms the branch carries nothing unique removes the -> section.** The bullets below are its original claims, preserved as -> written and now unverified. - -- Portable branch: `githubsucks/handoff-2026-07-20` -- Carries synchronized `AGENTS.md` / `CLAUDE.md`, this ledger, the - durable handoff refresh, and the keybinding reference correction. -- It changes no runtime code. -- Review and merge this documentation branch separately; it must not be - folded into a feature framing branch. -- Now also absorbs both landed arcs: Vterm Stage 1 (#126) and the config - registry (#127). Canonical `main` is merged into it up to `2e37c04`, - so its diff against `main` is documentation only. - ## Closed since the last snapshot - **Terminal configuration + copy mode arc — BOTH STAGES MERGED, lane diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 5307799..2c5ca22 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -1443,6 +1443,22 @@ round-trip cannot detect a discriminant shift. ## 5. Hard-won ops lessons +- **A gate summary assembled through a pipe can report success over a + failure.** `cmd | tail -2` returns **`tail`'s** exit status, not + `cmd`'s — in `fish` and `bash` alike — so a chain of + `cargo test ... | tail -2 && cargo test ... | tail -2 && echo "ALL + GATES CLEAN"` prints the clean line even when a suite failed. This + is not carelessness that closer reading would catch: the failure is + **structurally invisible** in the summary the PR then cites. It + happened while gating the silent-skip lane, and a `pmacs-gpu` + failure was reported as clean. + + Either check `$pipestatus[1]` in fish (`${PIPESTATUS[0]}` in bash), + or — better — redirect each gate to a file and read the file + afterwards, which also preserves the full log this section already + asks you to keep. Same family as the skip-reports-`ok` lesson below + and the double-invocation traps: **the thing that summarizes a gate + must not be able to lose the gate's verdict.** - **A test that skips on a missing precondition reports `ok`, and a gate log cannot tell that apart from a pass.** `vterm_stage3_acceptance::a37` — the only acceptance driving a real daemon, a real PTY and a real wgpu render diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 691d746..f56dfc7 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -1298,7 +1298,7 @@ fn fake_spec(label: &str) -> LspServerSpec { #[test] fn m4_5_rust_analyzer_initializes() { let Ok(_) = which_binary("rust-analyzer") else { - eprintln!("rust-analyzer not on PATH; skipping"); + support::skip_or_fail("rust-analyzer", "PMACS_REQUIRE_LSP"); return; }; let (sup, mgr) = make_lsp_test_manager(); @@ -1338,7 +1338,7 @@ fn m4_5_rust_analyzer_initializes() { #[test] fn m4_5_basedpyright_initializes_and_negotiates_encoding() { let Ok(_) = which_binary("basedpyright-langserver") else { - eprintln!("basedpyright-langserver not on PATH; skipping"); + support::skip_or_fail("basedpyright-langserver", "PMACS_REQUIRE_PYRIGHT"); return; }; let (sup, mgr) = make_lsp_test_manager(); @@ -1419,7 +1419,7 @@ fn assert_lsp_initializes_and_negotiates( #[test] fn m4_5_clangd_initializes_and_negotiates_encoding() { let Ok(_) = which_binary("clangd") else { - eprintln!("clangd not on PATH; skipping"); + support::skip_or_fail("clangd", "PMACS_REQUIRE_LSP"); return; }; assert_lsp_initializes_and_negotiates("clangd", "cpp", "clangd", &["--background-index"]); @@ -1431,7 +1431,7 @@ fn m4_5_clangd_initializes_and_negotiates_encoding() { #[test] fn m4_5_gopls_initializes_and_negotiates_encoding() { let Ok(_) = which_binary("gopls") else { - eprintln!("gopls not on PATH; skipping"); + support::skip_or_fail("gopls", "PMACS_REQUIRE_LSP"); return; }; assert_lsp_initializes_and_negotiates("gopls", "go", "gopls", &[]); @@ -3428,6 +3428,9 @@ fn m4_11_snippets_surface_through_completion() { use pmacs::definition::DefinitionKey; use pmacs::formatting::FormattingKey; +#[path = "support/mod.rs"] +mod support; + /// Acceptance (1/3): a `textDocument/definition` request round-trips /// through the manager and lands in the definition store as a parsed /// `Location` list. @@ -5536,7 +5539,7 @@ fn m4_27_real_gopls_analyzes_module_via_auto_attach() { use pmacs::editor::EditorState; let Ok(gopls) = which_binary("gopls") else { - eprintln!("gopls not on PATH; skipping"); + support::skip_or_fail("gopls", "PMACS_REQUIRE_LSP"); return; }; let gopls = gopls.display().to_string(); @@ -5633,7 +5636,7 @@ fn m4_28_real_clangd_diagnostics_and_semantic_tokens_via_auto_attach() { use pmacs::editor::EditorState; let Ok(clangd) = which_binary("clangd") else { - eprintln!("clangd not on PATH; skipping"); + support::skip_or_fail("clangd", "PMACS_REQUIRE_LSP"); return; }; let clangd = clangd.display().to_string(); @@ -5731,7 +5734,7 @@ fn m4_29_real_rust_analyzer_inlay_hints_via_auto_attach() { use pmacs::editor::EditorState; let Ok(rust_analyzer) = which_binary("rust-analyzer") else { - eprintln!("rust-analyzer not on PATH; skipping"); + support::skip_or_fail("rust-analyzer", "PMACS_REQUIRE_LSP"); return; }; let rust_analyzer = rust_analyzer.display().to_string(); @@ -6880,7 +6883,7 @@ fn m4_real_json_provider_receives_config_and_reports_diagnostics() { use pmacs::editor::EditorState; let Ok(command) = which_binary("vscode-json-language-server") else { - eprintln!("vscode-json-language-server not on PATH; skipping"); + support::skip_or_fail("vscode-json-language-server", "PMACS_REQUIRE_LSP"); return; }; let command = command.display().to_string(); @@ -6949,7 +6952,7 @@ fn m4_real_yaml_provider_pulls_config_and_reports_diagnostics() { use pmacs::editor::EditorState; let Ok(command) = which_binary("yaml-language-server") else { - eprintln!("yaml-language-server not on PATH; skipping"); + support::skip_or_fail("yaml-language-server", "PMACS_REQUIRE_LSP"); return; }; let command = command.display().to_string(); diff --git a/tests/m6_5_repl_acceptance.rs b/tests/m6_5_repl_acceptance.rs index fcf9bf4..44d6b95 100644 --- a/tests/m6_5_repl_acceptance.rs +++ b/tests/m6_5_repl_acceptance.rs @@ -35,6 +35,9 @@ use std::path::PathBuf; use std::sync::{Mutex, MutexGuard}; use std::time::{Duration, Instant}; +#[path = "support/mod.rs"] +mod support; + static PUMP_TEST_LOCK: Mutex<()> = Mutex::new(()); fn pump_test_guard() -> MutexGuard<'static, ()> { @@ -196,7 +199,7 @@ fn m6_5_ret_submits_input_to_process() { #[test] fn m6_5_ctrl_d_on_empty_prompt_closes_stdin() { let Some(bash) = locate_shell("bash") else { - eprintln!("skipping: bash not on PATH (set PMACS_TEST_BASH to override)"); + support::skip_or_fail_overridable("bash", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_BASH"); return; }; let setup = format!( @@ -290,7 +293,7 @@ fn m6_5_ctrl_d_on_nonempty_input_deletes_char_forward() { )] fn m6_5_ctrl_c_sends_sigint() { let Some(sleep) = locate_shell("sleep") else { - eprintln!("skipping: sleep not on PATH (set PMACS_TEST_SLEEP to override)"); + support::skip_or_fail_overridable("sleep", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_SLEEP"); return; }; let setup = format!( @@ -344,7 +347,7 @@ fn m6_5_ctrl_c_sends_sigint() { )] fn m6_5_exit_marker_uses_basename_with_leading_newline() { let Some(false_bin) = locate_shell("false") else { - eprintln!("skipping: false not on PATH (set PMACS_TEST_FALSE to override)"); + support::skip_or_fail_overridable("false", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_FALSE"); return; }; let setup = format!( @@ -425,7 +428,7 @@ fn run_shell_smoke_test(shell_path: &std::path::Path, argv_extra: &[&str]) { #[test] fn m6_5_repl_spawns_bash() { let Some(bash) = locate_shell("bash") else { - eprintln!("skipping: bash not on PATH (set PMACS_TEST_BASH to override)"); + support::skip_or_fail_overridable("bash", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_BASH"); return; }; run_shell_smoke_test(&bash, &["-i"]); @@ -435,7 +438,7 @@ fn m6_5_repl_spawns_bash() { #[test] fn m6_5_repl_spawns_zsh() { let Some(zsh) = locate_shell("zsh") else { - eprintln!("skipping: zsh not on PATH (set PMACS_TEST_ZSH to override)"); + support::skip_or_fail_overridable("zsh", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_ZSH"); return; }; run_shell_smoke_test(&zsh, &["-i"]); @@ -446,7 +449,7 @@ fn m6_5_repl_spawns_zsh() { #[test] fn m6_5_repl_spawns_fish() { let Some(fish) = locate_shell("fish") else { - eprintln!("skipping: fish not on PATH (set PMACS_TEST_FISH to override)"); + support::skip_or_fail_overridable("fish", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_FISH"); return; }; run_shell_smoke_test(&fish, &["-i"]); @@ -458,8 +461,10 @@ fn m6_5_repl_spawns_fish() { #[test] fn m6_5_repl_spawns_lua() { let Some(lua) = locate_shell("lua").or_else(|| locate_shell("luajit")) else { - eprintln!( - "skipping: lua/luajit not on PATH (set PMACS_TEST_LUA or PMACS_TEST_LUAJIT to override)" + support::skip_or_fail_overridable( + "lua/luajit", + "PMACS_REQUIRE_LUA", + "PMACS_TEST_LUA or PMACS_TEST_LUAJIT", ); return; }; diff --git a/tests/m6_8_multi_repl_acceptance.rs b/tests/m6_8_multi_repl_acceptance.rs index 673daa4..4bc482f 100644 --- a/tests/m6_8_multi_repl_acceptance.rs +++ b/tests/m6_8_multi_repl_acceptance.rs @@ -68,6 +68,9 @@ use pmacs::editor::EditorState; use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; +#[path = "support/mod.rs"] +mod support; + // --------------------------------------------------------------------------- // Test harness // --------------------------------------------------------------------------- @@ -94,8 +97,10 @@ fn locate_lua() -> Option { } } } - eprintln!( - "skipping: lua/luajit not on PATH (set PMACS_TEST_LUA or PMACS_TEST_LUAJIT to override)" + support::skip_or_fail_overridable( + "lua/luajit", + "PMACS_REQUIRE_LUA", + "PMACS_TEST_LUA or PMACS_TEST_LUAJIT", ); None } diff --git a/tests/support/mod.rs b/tests/support/mod.rs new file mode 100644 index 0000000..14fbeec --- /dev/null +++ b/tests/support/mod.rs @@ -0,0 +1,104 @@ +//! Shared test-support helpers. +//! +//! Included by `#[path = "support/mod.rs"] mod support;` rather than +//! copied. Files under `tests/` subdirectories are not compiled as +//! their own test binaries, so this costs nothing — and +//! `m6_8_multi_repl_acceptance.rs` previously carried a comment saying +//! cross-test-binary sharing "would need a fixture crate", which is not +//! so. A correct helper in one file and a degraded copy in another is +//! this suite's most repeated defect shape; sharing removes the way it +//! happens. +//! +//! **Why this is separate from `tests/common/`, which also exists.** +//! `tests/common/mod.rs` re-exports `daemon` and `pty` — real daemon +//! spawning and PTY plumbing. Including it to reach a six-line +//! environment check would compile that machinery into three test +//! binaries that spawn neither, for no benefit. `support` is the +//! dependency-free half: helpers any test binary can take without +//! taking a subsystem with them. Two directories is a cost worth +//! naming rather than leaving to be rediscovered; if a third appears, +//! consolidate instead of continuing the pattern. + +#![allow(dead_code)] + +/// Report a missing external tool, and turn the skip into a HARD +/// FAILURE when the environment has promised the tool is present. +/// +/// The bare shape this replaces — +/// +/// ```ignore +/// let Ok(_) = which_binary("gopls") else { +/// eprintln!("gopls not on PATH; skipping"); +/// return; +/// }; +/// ``` +/// +/// passes GREEN when the tool is absent, and is why a large block of +/// external-tool-gated tests had never once executed their bodies in +/// CI: nothing installed the tools, so every one of them reported +/// success without running. A suite that cannot tell "passed" from +/// "never ran" is worse than a missing suite, because it reads as +/// coverage. +/// +/// `PMACS_REQUIRE_*` is the project's own fix, already load-bearing for +/// `PMACS_REQUIRE_GPU` in `vterm_stage3_acceptance`: CI installs the +/// tool, sets the variable, and absence becomes a failure that names +/// the step that should have provided it. Locally the variable is +/// unset, so the skip still works and nobody needs the whole toolchain +/// to run the suite. +/// +/// Deliberately per-tool rather than one blanket variable: a tool that +/// must stay unarmed (because arming it would hang, or because CI does +/// not install it yet) keeps its own variable that CI never sets, and +/// that decision is then visible at the call site instead of buried in +/// a workflow file. +/// True when `var` is set to a non-empty value. +/// +/// Emptiness matters, and the reason is a trap rather than a nicety. +/// The natural GitHub Actions idiom for a conditional environment +/// variable — +/// +/// ```yaml +/// PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }} +/// ``` +/// +/// sets the variable to the EMPTY STRING on every other platform, not +/// to nothing. A bare `var_os(..).is_some()` is therefore true there, +/// which would arm the guard on exactly the runners that have none of +/// the tools installed and fail every one of them. Treating empty as +/// unset makes the common workflow spelling safe instead of subtly +/// wrong. +fn armed(var: &str) -> bool { + std::env::var_os(var).is_some_and(|v| !v.is_empty()) +} + +#[track_caller] +pub fn skip_or_fail(tool: &str, require_var: &str) { + assert!( + !armed(require_var), + "{require_var} is set, but `{tool}` is not on PATH. \ + The CI step that installs it did not run, or installed it \ + somewhere not on PATH. This is a hard failure precisely so \ + the test cannot report green without executing." + ); + eprintln!("{tool} not on PATH; skipping (set {require_var} to make this fatal)"); +} + +/// As [`skip_or_fail`], for tools whose PATH lookup can be overridden +/// by a `PMACS_TEST_*` variable. The skip notice keeps naming that +/// override, because losing it would make the local escape hatch +/// undiscoverable — the REPL suites are routinely run on machines +/// without zsh or fish. +#[track_caller] +pub fn skip_or_fail_overridable(tool: &str, require_var: &str, override_var: &str) { + assert!( + !armed(require_var), + "{require_var} is set, but `{tool}` is not on PATH and {override_var} \ + is unset or points at nothing. The CI step that installs it did not \ + run, or installed it somewhere not on PATH." + ); + eprintln!( + "skipping: {tool} not on PATH (set {override_var} to override, \ + or {require_var} to make this fatal)" + ); +}