Skip to content

ci(standalone): run the crate's tests, and compile the platform-gated Rust - #428

Merged
nedtwigg merged 2 commits into
mainfrom
ci/platform-cargo-check
Aug 21, 2026
Merged

ci(standalone): run the crate's tests, and compile the platform-gated Rust#428
nedtwigg merged 2 commits into
mainfrom
ci/platform-cargo-check

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

CI's only Rust step is cargo check on ubuntu-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 a v* tag.

Surfaced by the nightly survey, which drew standalone/src-tauri/src/main.rs today and could not verify a one-line change to its Windows-only sibling.

What is currently invisible, precisely

The tests never run. cargo check type-checks #[cfg(test)] bodies only incidentally and executes nothing. 19 tests in lib.rs (sidecar resolution, dor CLI resolution, verbatim-path stripping, per-window session round-trips) and 4 in clipboard_win.rs (BITMAPFILEHEADER construction) 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) and pe_subsystem.rs, both #[cfg(windows)] mod
  • ~25 #[cfg(windows)] / #[cfg(target_os = "macos")] / #[cfg(unix)] arms inside lib.rs
  • the target.contains("windows") branch of build.rs, which is what stamps the bundled Node binary's PE subsystem

release.yml does build all three platforms — but only on: 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 action release.yml already uses) keeps the steady-state to an incremental compile, the TypeScript check is not repeated on the new runners since it is platform-independent, and fail-fast: false means a Windows failure still lets macOS report. If the trade still isn't worth it, dropping macos-latest from the matrix keeps most of the value — clipboard_win.rs and pe_subsystem.rs are the two whole modules at stake, and both are Windows.

The new job is deliberately separate from standalone-smoketest rather 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.rs refuses before cargo gets anywhere. The YAML parses and the job graph is build-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's workflow-audit run 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 to clipboard_win.rs that no job on that PR can compile.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 21, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: ce8a2e2
Status: ✅  Deploy successful!
Preview URL: https://37fcafbe.mouseterm.pages.dev
Branch Preview URL: https://ci-platform-cargo-check.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_binarynode -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 check to a full cargo test — 1:46 uncached on this run — but only the new job got swatinem/rust-cache. The same step on the Linux job would recover most of that, though restoring and saving a Tauri target/ isn't free either, so it's worth measuring rather than assuming.
  • ci.yml has no concurrency block, so stacked pushes to a PR now start two premium jobs each with nothing superseded. A cancel-in-progress scoped to pull_request is 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.

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml
…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.
@dormouse-bot

Copy link
Copy Markdown
Collaborator Author

All three inline suggestions are on the branch as ce8a2e2pnpm install and pnpm/action-setup dropped from standalone-platform-check, timeout-minutes: 20 added — and the run on that head is green on every gating check, including both premium legs: Windows 1:39, macOS 0:40. That answers the only thing worth verifying about the removal, which is that neither leg needs node_modules; the wall-clock numbers themselves aren't a like-for-like against the 2:13 / 5:14 above, since that run was cold-cache and this one restored swatinem/rust-cache.

The two follow-ups outside the diff are deliberately not in that commit, since both are judgment calls rather than corrections: adding swatinem/rust-cache to the Linux smoketest trades a target/ restore+save against the 1:46 cargo test, which is worth measuring rather than assuming, and a cancel-in-progress concurrency block on ci.yml caps the cost this PR adds but leaves superseded red check entries on the commit that never clear. Both are one-liners whenever you want either.

@nedtwigg
nedtwigg merged commit 99d908e into main Aug 21, 2026
7 checks passed
@nedtwigg
nedtwigg deleted the ci/platform-cargo-check branch August 21, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants