ci: schedule Rust workspace tests on macOS or Linux self-hosted runners - #4287
Open
QuantumExplorer wants to merge 1 commit into
Open
ci: schedule Rust workspace tests on macOS or Linux self-hosted runners#4287QuantumExplorer wants to merge 1 commit into
QuantumExplorer wants to merge 1 commit into
Conversation
`runs-on` label arrays are AND-ed, so `[self-hosted, macOS, ARM64]` could never also match the new Linux runner. Select on a shared custom label instead: `[self-hosted, rust-ci]`, applied to both mac runners and the Linux one. Keeping `self-hosted` in the list means GitHub-hosted runners still can't pick the job up. Making the job portable: - Homebrew setup is gated to macOS; Linux gets an apt equivalent for what ./.github/actions/rust doesn't already install (it handles clang, llvm and libsnappy), plus gh for the immutable-structure check and rustup for dtolnay/rust-toolchain. Every branch is a no-op after the first run. - The disk prune clamps its absolute caps to a share of the volume, so a smaller runner doesn't purge target/ on every run and cold-rebuild. - The GPG keyboxd reset is a macOS runner workaround, so it's gated too. Linux builds rocksdb from source like macOS does rather than linking the prebuilt static lib the old Ubuntu jobs used. The persistent target/ makes that a one-time cost, and it avoids their sed pass over Cargo.toml crate types, which static rocksdb forced. The three Ubuntu backup jobs and their poll-the-Mac-job dance are removed. UBUNTU_BACKUP_ENABLED has been false, so they were dead code, and a second self-hosted runner is the redundancy they were standing in for. That empties tests-rs-doctests.yml; doctests already run inside both Rust jobs under `doctests-changed`. The job is renamed Tests (macOS) -> Tests. No branch protection rule references it. Requires the `rust-ci` label on all three runners. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughRust CI now runs through self-hosted ChangesRust CI workflow migration
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
|
🕓 Ready for review — 4 ahead in queue (commit 0444662) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 157-162: Update the workflow scope comment near the fork pull
request handling to say that untrusted fork PRs must use the full workspace
path, while acknowledging the trusted thepastaclaw fork exception that may use
the wallet scope and reach tests-rs-wallet.yml. Preserve the existing
distinctions for same-repository pull requests, push runs, and scheduled runs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44d48aaf-5c35-4c0a-8958-6b21dcabd387
📒 Files selected for processing (5)
.github/actionlint.yaml.github/workflows/tests-rs-doctests.yml.github/workflows/tests-rs-wallet.yml.github/workflows/tests-rs-workspace.yml.github/workflows/tests.yml
💤 Files with no reviewable changes (1)
- .github/workflows/tests-rs-doctests.yml
Comment on lines
157
to
+162
| # The fast path applies only to same-repo pull requests. Fork PRs | ||
| # must take the full workspace path (whose Ubuntu backup jobs cover | ||
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | ||
| # workflow's only job skips fork PRs). Push and schedule runs have | ||
| # no reliable base SHA — scope computed from the last commit alone | ||
| # could silently downgrade the nightly / post-merge full runs. | ||
| # must take the full workspace path (both Rust workflows skip fork | ||
| # PRs outright, since neither may run untrusted code on a persistent | ||
| # runner). Push and schedule runs have no reliable base SHA — scope | ||
| # computed from the last commit alone could silently downgrade the | ||
| # nightly / post-merge full runs. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the trusted fork exception in the scope comment.
The comment says that fork pull requests use the full workspace path. Lines [168]-[170] treat a fork owned by thepastaclaw as trusted, so that fork may use scope=wallet and reach tests-rs-wallet.yml. Change the comment to say Untrusted fork PRs.
Proposed wording
- # The fast path applies only to same-repo pull requests. Fork PRs
- # must take the full workspace path (both Rust workflows skip fork
- # PRs outright, since neither may run untrusted code on a persistent
- # runner). Push and schedule runs have no reliable base SHA — scope
+ # The fast path applies only to same-repo pull requests. Untrusted
+ # fork PRs are classified as full-scope changes. Both Rust workflows
+ # skip those PRs, since neither may run untrusted code on a
+ # persistent runner. Push and schedule runs have no reliable base SHA📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # The fast path applies only to same-repo pull requests. Fork PRs | |
| # must take the full workspace path (whose Ubuntu backup jobs cover | |
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | |
| # workflow's only job skips fork PRs). Push and schedule runs have | |
| # no reliable base SHA — scope computed from the last commit alone | |
| # could silently downgrade the nightly / post-merge full runs. | |
| # must take the full workspace path (both Rust workflows skip fork | |
| # PRs outright, since neither may run untrusted code on a persistent | |
| # runner). Push and schedule runs have no reliable base SHA — scope | |
| # computed from the last commit alone could silently downgrade the | |
| # nightly / post-merge full runs. | |
| # The fast path applies only to same-repo pull requests. Untrusted | |
| # fork PRs are classified as full-scope changes. Both Rust workflows | |
| # skip those PRs, since neither may run untrusted code on a | |
| # persistent runner. Push and schedule runs have no reliable base SHA | |
| # computed from the last commit alone could silently downgrade the | |
| # nightly / post-merge full runs. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/tests.yml around lines 157 - 162, Update the workflow
scope comment near the fork pull request handling to say that untrusted fork PRs
must use the full workspace path, while acknowledging the trusted thepastaclaw
fork exception that may use the wallet scope and reach tests-rs-wallet.yml.
Preserve the existing distinctions for same-repository pull requests, push runs,
and scheduled runs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
We added a self-hosted Linux runner, but
Rust workspace tests / Tests (macOS)was pinned toruns-on: [self-hosted, macOS, ARM64]and could never schedule onto it. Labels in aruns-onarray are AND-ed — there is no OR syntax — so no edit to that list alone can express "macOS or Linux".Current runners on this repo:
self-hosted, macOS, ARM64self-hosted, Linux, X64, ubuntu, ms-a2What was done?
Select on a shared custom label instead:
Whichever of the three runners is idle takes the job. Keeping
self-hostedin the list means a GitHub-hosted Linux VM can never match it, so Linux stays self-hosted-only.Making the job body portable:
./.github/actions/rustdoesn't already install (it handles clang, llvm and libsnappy on Linux), plusghfor the immutable-structure check andrustupfordtolnay/rust-toolchain. Each branch is guarded, so it's a no-op once the persistent runner is provisioned by the first run.TOTAL/3andTOTAL/5). On the large macOS volumes this is exactly today's behavior; it stops a smaller volume from tripping the floor on every run and forcing a cold rebuild each time.Linux builds rocksdb from source the same way macOS already does, rather than linking the prebuilt static lib the old Ubuntu jobs used. The persistent
target/makes that a one-time cost and it avoids theirsedpass rewriting cdylib crate-types inCargo.toml, which static rocksdb forced.Removed the three Ubuntu backup jobs (
test-ubuntu,lint-ubuntu, and the whole oftests-rs-doctests.yml) along with theirsleep 15+ poll-the-Mac-job-status dance.UBUNTU_BACKUP_ENABLEDisfalseon this repo, so they have been dead code; a second self-hosted runner is the redundancy they were standing in for. Doctests already run inside both Rust jobs underdoctests-changed, so dropping the standaloners-doctestsjob loses no coverage.Also added
.github/actionlint.yamldeclaringrust-ci, so localactionlintruns don't flag it as an unknown label.Required before merge
The
rust-cilabel must be added to all three runners (Settings → Actions → Runners → runner → Labels). Until then the job has nothing to schedule onto and will sit queued.How Has This Been Tested?
actionlint(with shellcheck) clean across the three touched workflows; the remaining shellcheck notes are pre-existing and unrelated — the sameSC2086fires on the untouched Homebrew line intests-rs-wallet.yml.Tests (macOS)job name, andUBUNTU_BACKUP_ENABLED; the three comment mentions left behind were updated.v4.2-devhas no required status checks configured, so the job rename breaks no gate.Breaking Changes
None.
Notes for reviewers
tests-rs-wallet.ymlstays pinned to the macOS runners. That fast path is short and mac-only by design; floating it too is a reasonable follow-up but I kept this PR to the workspace job. Its header comment is updated to say so.UBUNTU_BACKUP_ENABLEDis now unreferenced and can be deleted from repo variables.if:guard blocked them from the mac runners before and now guards both platforms; the Ubuntu backup that could theoretically have covered them was disabled.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
rust-ci.