Merge pull request #194 from levineuwirth/silent-skip-arming
test: arm the silent skips, so external-tool tests stop passing vacuously
This commit is contained in:
GitHub
commit
b7bf2c6644
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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<PathBuf> {
|
|||
}
|
||||
}
|
||||
}
|
||||
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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)"
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue