ci(standalone): run the crate's tests, and compile the platform-gated Rust - #428
Conversation
Deploying mouseterm with
|
| Latest commit: |
ce8a2e2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://37fcafbe.mouseterm.pages.dev |
| Branch Preview URL: | https://ci-platform-cargo-check.mouseterm.pages.dev |
dormouse-bot
left a comment
There was a problem hiding this comment.
The macOS leg came back green in 2:13 from a cold cache and the Linux cargo test passed all 23, so the approach is answered — Windows is still compiling. Three efficiency notes, all against the premium-runner cost the PR body flags as the trade.
The pnpm install in standalone-platform-check (and the pnpm/action-setup that feeds it) do no work. The job runs no npm script — the TypeScript check is deliberately skipped — and the Rust build's only Node dependency is node on PATH, which build.rs resolves via resolve_node_binary → node -p process.execPath, plus the root package.json pin it reads off disk. Nothing under standalone/src-tauri/ references node_modules. On this run those two steps cost 16s on macOS and 48s on Windows.
Two follow-ups outside the diff, which I can push if you want them:
- The smoketest went from
cargo checkto a fullcargo test— 1:46 uncached on this run — but only the new job gotswatinem/rust-cache. The same step on the Linux job would recover most of that, though restoring and saving a Tauritarget/isn't free either, so it's worth measuring rather than assuming. ci.ymlhas noconcurrencyblock, so stacked pushes to a PR now start two premium jobs each with nothing superseded. Acancel-in-progressscoped topull_requestis the cheapest cap on the cost this PR adds; the tradeoff is that cancelled runs leave superseded red check entries on the commit that never clear.
…untime The platform-check job runs no npm script — the TypeScript check is deliberately left to the smoketest — and the Rust build never reads node_modules: build.rs resolves Node from PATH (`node -p process.execPath`) and reads the version pin straight out of the root package.json. So `pnpm install` and the pnpm setup it needs cost 16s (macOS) and 48s (Windows) per PR for nothing. Also cap the job at 20 minutes. Cold-cache runs are ~2min on macOS and ~5min on Windows, so a hung compile riding the 6-hour default is pure premium-runner burn.
|
All three inline suggestions are on the branch as ce8a2e2 — The two follow-ups outside the diff are deliberately not in that commit, since both are judgment calls rather than corrections: adding |
CI's only Rust step is
cargo checkonubuntu-latest, which means two things nobody would guess from a green PR: the crate's 23 unit tests have never been run anywhere, and none of its#[cfg(windows)]/#[cfg(target_os = "macos")]code has ever been compiled by a check. This makes the Linux job run the tests, and adds a Windows/macOS job so the platform-gated halves compile on a PR instead of for the first time on av*tag.Surfaced by the nightly survey, which drew
standalone/src-tauri/src/main.rstoday and could not verify a one-line change to its Windows-only sibling.What is currently invisible, precisely
The tests never run.
cargo checktype-checks#[cfg(test)]bodies only incidentally and executes nothing. 19 tests inlib.rs(sidecar resolution,dorCLI resolution, verbatim-path stripping, per-window session round-trips) and 4 inclipboard_win.rs(BITMAPFILEHEADERconstruction) are dead weight today — a broken assertion in any of them merges green. All 23 are pure path/filesystem tests with no platform assumptions beyond the two explicitly#[cfg(windows)]-gated ones, so they are safe to run on all three runners.The platform-gated code never compiles.
#[cfg]-excluded code is not type-checked at all — it is dropped before name resolution. On Linux that silently excludes:clipboard_win.rs(~330 lines) andpe_subsystem.rs, both#[cfg(windows)] mod#[cfg(windows)]/#[cfg(target_os = "macos")]/#[cfg(unix)]arms insidelib.rstarget.contains("windows")branch ofbuild.rs, which is what stamps the bundled Node binary's PE subsystemrelease.ymldoes build all three platforms — but onlyon: push: tags: v*. So the failure mode this closes is: a typo in Windows-only Rust merges green, and the next release tag fails its Windows leg mid-pipeline.Cost, stated plainly. Windows and macOS runners bill at a multiple of Linux, so this is not free — it is two new runners per PR that touches anything.
swatinem/rust-cache(the same pinned actionrelease.ymlalready uses) keeps the steady-state to an incremental compile, the TypeScript check is not repeated on the new runners since it is platform-independent, andfail-fast: falsemeans a Windows failure still lets macOS report. If the trade still isn't worth it, droppingmacos-latestfrom the matrix keeps most of the value —clipboard_win.rsandpe_subsystem.rsare the two whole modules at stake, and both are Windows.The new job is deliberately separate from
standalone-smoketestrather than a matrix over it, so that job keeps its registered check name and branch protection is unaffected.Verification. I could not run any of this locally: this checkout has no GTK/WebKit dev libraries and its Node is v22 against the crate's pinned build, so
build.rsrefuses beforecargogets anywhere. The YAML parses and the job graph isbuild-and-test,standalone-smoketest,standalone-platform-check; everything else is for CI on this PR to answer, and the two new legs are themselves the evidence. If they come back red for an environmental reason rather than a real compile error, that is worth knowing here rather than at a release.Two notes for review: this edits
.github/workflows/, so tonight'sworkflow-auditrun will report the commit as an unexplained workflow change — that is the audit working, not a second problem. And it is the check that would have verified #427, a one-line fix toclipboard_win.rsthat no job on that PR can compile.