diff --git a/.claude/skills/running-tend/SKILL.md b/.claude/skills/running-tend/SKILL.md index d02c8e113ea2..dbd6ac3c918f 100644 --- a/.claude/skills/running-tend/SKILL.md +++ b/.claude/skills/running-tend/SKILL.md @@ -2,9 +2,10 @@ name: running-tend description: PRQL-specific guidance for tend CI workflows. Adds a standing exception for - filing issues in other repos, PR title conventions, CI structure, - Dependabot-batch polling, weekly maintenance tasks, and issue-closing policy - on top of the generic tend-* skills. Use when operating in CI. + filing issues in other repos, PR title conventions, CI structure, which test + commands actually run inside the sandbox, Dependabot-batch polling, weekly + maintenance tasks, and issue-closing policy on top of the generic tend-* + skills. Use when operating in CI. --- # Running Tend in PRQL @@ -42,6 +43,84 @@ permission first) still applies when the target shows no agent signals. in #5753, so bot PRs must be merged manually by a maintainer (or via repo branch-protection auto-merge if a maintainer enables it on the PR). +## Running tests from a tend session + +**`cargo-insta` and `cargo-nextest` are not on the sandbox PATH by default**, so +`task prqlc:test` and `task prqlc:pull-request` — both of which route through +`cargo insta test --accept … --test-runner=nextest` — answer `no such command` +out of the box. `.github/actions/tend-setup` installs both, but +`baptiste0928/cargo-install` puts them under the runner user's home +(`/home/runner/.cargo-install//bin`), and tend derives the sandbox PATH +without any runner-home directory. + +The binaries are world-executable and `/home/runner` is `drwxr-x--x`, so +`tend-sandbox` can traverse to them without being able to enumerate that home. +Prepend them in the same invocation as the command — shell state doesn't persist +between tool calls: + +```sh +export PATH="/home/runner/.cargo-install/cargo-insta/bin:/home/runner/.cargo-install/cargo-nextest/bin:$PATH" +task prqlc:test +``` + +`task` is already on the sandbox PATH, so `task prqlc:test` — the inner loop +`CLAUDE.md` documents — then runs unchanged. Verified in a session on +2026-08-26: it exits 0 in ~3 minutes on a warm `target/`, and +`cargo insta test --accept` rewrote an `assert_snapshot!(ident, @"")` literal in +place, so the initialize-empty-then-accept flow works here too. +`task prqlc:pull-request` does not: through `test-all` it also needs +`cargo-llvm-cov` for the coverage step and `pre-commit` for `:lint`, and +`tend-setup` installs neither. Those two steps are all that's missing, so when +`CLAUDE.md` calls for the pre-return check, run the rest by hand: `test-all`'s +first two commands — the `--features=default,test-dbs` insta/nextest run and the +matching `cargo test --doc`, both over `packages_core` plus `packages_addon` and +`packages_bindings` — and then `:lint`'s +`cargo clippy --all-targets --all-features`, budgeting for the cold-cache cost +the `rust-toolchain.toml` section below describes. Don't substitute +`prqlc:test`: it covers `-p prqlc-parser -p prqlc` only, so a change that breaks +`prqlc-js` or `prqlc-c` passes it. + +Notes on running tests here: + +- **Never verify with `INSTA_UPDATE=always cargo test`.** `always` selects + insta's in-place update, so a `.snap` file is rewritten to whatever the code + produced and the assertion passes unconditionally — a green run that checked + nothing. Use it only to regenerate file snapshots deliberately, then re-run + plain `cargo test` to verify. Plain `cargo test` is a real check: insta's + default `auto` behaviour writes nothing when `CI` is set. +- **A green `task prqlc:test` is weaker than it looks.** The task runs + `cargo insta test --accept` and then + `cargo clippy --fix --allow-dirty --allow-staged`, so exit 0 can mean + snapshots and source were rewritten to match what the code now produces rather + than that the assertions held. Check `git status` and read the diff before + reporting the run as verification. +- Without the export, plain `cargo test` is the fallback — scoped the same way + the `CLAUDE.md` examples are, e.g. + `cargo test -p prqlc --test integration -- date`. Inline snapshots then have + to be transcribed by hand: insta never rewrites a source file itself, it + records the value in a pending-snapshot file and leaves applying it to + `cargo-insta`. Take the expected value from the failure's diff, write it into + the `@"…"` literal, and re-run to confirm. +- **Don't `cargo install` either crate.** Building from source costs several + minutes of the session budget, and the binaries `tend-setup` already built are + one `export` away. +- Scope the claim to the command that actually ran. `cargo test -p prqlc` is not + `task prqlc:pull-request`, and saying so is the difference between a useful + caveat and a false green. + +**On making the export unnecessary — a maintainer's call.** #6144 (a workflow +step symlinking both binaries into `/usr/local/bin`) sat open for 20 days and +was closed unmerged on 2026-08-25; don't re-propose it. `sandbox_path:` is not +the alternative either: tend's `proxy/setup-sandbox.sh` rejects any entry under +the runner's home outside the checkout and fails the job with "Install the tool +into the sandbox with `sandbox_setup:` instead" — deliberately, since that home +can hold credentials unrelated to the tool being reached for. `sandbox_setup:` +is the supported lever, and it needn't mean a rebuild: `tend-setup` runs before +the tend action, so copying the two existing binaries into +`/home/tend-sandbox/.cargo/bin` (already on the sandbox PATH, and writable by +the sandbox user) would cost a `cp`. Leave that to a maintainer rather than +re-litigating the area from a session. + ## Verifying a `rust-toolchain.toml` bump The `update-rust-toolchain` action opens `build: Update rust toolchain version`