diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce7e0a0..3c1bc59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,8 +84,6 @@ jobs: steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - with: - components: rust-analyzer - 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 @@ -110,9 +108,23 @@ jobs: # `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 - go install golang.org/x/tools/gopls@latest + # 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 yaml-language-server + 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 diff --git a/docs/active-work.md b/docs/active-work.md index 8e216b3..7988729 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -524,8 +524,24 @@ has **no branch and no framing yet**. ## Test-improvement arc, lane 2 — silent-skip arming - Portable branch: `githubsucks/silent-skip-arming`, worktree - `../pmacs-skiparm`. Base measured at write time, pasted below. - Implements `TEST_IMPROVEMENT.md` §1.2 and §5.4. + `../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 @@ -563,6 +579,32 @@ has **no branch and no framing yet**. 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. +- **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. @@ -574,7 +616,6 @@ has **no branch and no framing yet**. 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` diff --git a/tests/m6_5_repl_acceptance.rs b/tests/m6_5_repl_acceptance.rs index fc453ba..44d6b95 100644 --- a/tests/m6_5_repl_acceptance.rs +++ b/tests/m6_5_repl_acceptance.rs @@ -461,7 +461,11 @@ 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 { - support::skip_or_fail_overridable("lua/luajit", "PMACS_REQUIRE_LUA", "PMACS_TEST_LUA"); + support::skip_or_fail_overridable( + "lua/luajit", + "PMACS_REQUIRE_LUA", + "PMACS_TEST_LUA or PMACS_TEST_LUAJIT", + ); return; }; let setup = format!( diff --git a/tests/m6_8_multi_repl_acceptance.rs b/tests/m6_8_multi_repl_acceptance.rs index c1bd071..4bc482f 100644 --- a/tests/m6_8_multi_repl_acceptance.rs +++ b/tests/m6_8_multi_repl_acceptance.rs @@ -97,7 +97,11 @@ fn locate_lua() -> Option { } } } - support::skip_or_fail_overridable("lua/luajit", "PMACS_REQUIRE_LUA", "PMACS_TEST_LUA"); + 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 index 5b6ca97..f661b3d 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -1,7 +1,17 @@ //! Shared test-support helpers. //! //! Included by `#[path = "support/mod.rs"] mod support;` rather than -//! copied. Files under `tests/` subdirectories are not compiled as +//! copied. +//! +//! **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. 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