ci: smoke test built binaries to make sure they run before release#10296
ci: smoke test built binaries to make sure they run before release#10296dyc3 wants to merge 1 commit into
Conversation
|
WalkthroughThe pull request adds smoke testing for compiled CLI binaries in the pull request workflow. A new 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/release_cli.yml:
- Around line 251-252: The job-level if that references needs.check.outputs
(e.g., the line "if: needs.check.outputs.version_changed == 'true' ||
needs.check.outputs.nightly == 'true'") should be removed from the
smoke-test-win32-arm64 job so the job is gated only by its existing "needs:
build" dependency; update the smoke-test-win32-arm64 job (and the other job
noted around line 269, e.g., the publish-related job that depends on it) to drop
that extra if condition so they no longer reference out-of-scope
needs.check.outputs and instead rely solely on their declared needs.
🪄 Autofix (Beta)
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
Run ID: b9d0b436-f81f-45b9-8de9-ce0bd863fd06
📒 Files selected for processing (1)
.github/workflows/release_cli.yml
dyc3
changed the title
dyc3
force-pushed
the
dyc3/smoke-test-releases
branch
from
fdfef77 to
01ecac1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
181-190: ⚡ Quick winAdd tight timeouts to smoke-test commands.
Great addition overall. One quick hardening: if
--versionhangs, the release can be blocked for a long time.Suggested patch
- name: Smoke test binary (Windows) if: matrix.target == 'x86_64-pc-windows-msvc' + timeout-minutes: 1 shell: pwsh run: .\dist\biome-${{ matrix.code-target }}.exe --version - name: Smoke test binary (Unix) if: matrix.target != 'x86_64-pc-windows-msvc' && matrix.target != 'aarch64-pc-windows-msvc' + timeout-minutes: 1 run: | chmod +x ./dist/biome-${{ matrix.code-target }} ./dist/biome-${{ matrix.code-target }} --version- name: Run --version + timeout-minutes: 1 shell: pwsh run: .\biome-win32-arm64.exe --version- name: Smoke test binary + timeout-minutes: 1 run: | chmod +x ./dist/biome-${{ matrix.code-target }} ./dist/biome-${{ matrix.code-target }} --version🤖 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/release.yml around lines 181 - 190, Add a step-level timeout to the smoke-test steps to avoid long hangs: for the "Smoke test binary (Windows)" step that runs ".\\dist\\biome-${{ matrix.code-target }}.exe --version" and the "Smoke test binary (Unix)" step that chmods and runs "./dist/biome-${{ matrix.code-target }} --version", add a timeout-minutes (e.g. timeout-minutes: 5) field to each step so the workflow will fail fast if the --version command hangs.
🤖 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.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 181-190: Add a step-level timeout to the smoke-test steps to avoid
long hangs: for the "Smoke test binary (Windows)" step that runs
".\\dist\\biome-${{ matrix.code-target }}.exe --version" and the "Smoke test
binary (Unix)" step that chmods and runs "./dist/biome-${{ matrix.code-target }}
--version", add a timeout-minutes (e.g. timeout-minutes: 5) field to each step
so the workflow will fail fast if the --version command hangs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0cd4d81d-a7be-415c-96bf-3632c8e52715
📒 Files selected for processing (1)
.github/workflows/release.yml
There was a problem hiding this comment.
Such jobs that could block the release shouldn't be here. If it fails, we need to revert many things, which is something we shouldn't do unless something we can't control happens. In the past we messed up the changesets, or forced pushed in main which only few people can do.
Maybe we should have a cron job for these smoke tests, or add that job to the PR CI
dyc3
force-pushed
the
dyc3/smoke-test-releases
branch
from
01ecac1 to
843dfcf
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/pull_request.yml (2)
127-153: 💤 Low valueConsider
fail-fast: falseto surface all platform failures.With the default
fail-fast: true, a failure on one platform cancels the others. For a smoke test, you might prefer seeing all failures at once.strategy: + fail-fast: false matrix:🤖 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/pull_request.yml around lines 127 - 153, The workflow's strategy currently uses the matrix defined under strategy.matrix.include but doesn't set fail-fast, so a single platform failure cancels others; to surface all platform failures, add fail-fast: false under the same strategy block (i.e., alongside strategy.matrix) so the matrix build runs all entries (the include list of targets like x86_64-pc-windows-msvc, aarch64-unknown-linux-musl, x86_64-unknown-linux-gnu, etc.) without short-circuiting on first failure.
157-158: 💤 Low valueRelies on pre-installed Rust rather than
moonrepo/setup-rust.Other jobs in this workflow use
moonrepo/setup-rustfor consistent toolchain versions and caching. This job uses whatever Rust is pre-installed on the runner. Acceptable for smoke testing, but worth noting if version consistency matters.🤖 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/pull_request.yml around lines 157 - 158, The "Install Rust target" step currently runs rustup directly and relies on a preinstalled Rust; replace that step to use the moonrepo/setup-rust action (the same action used in other jobs) so the workflow installs/configures a consistent toolchain and caching; update the step named "Install Rust target" to call the moonrepo/setup-rust action with the appropriate toolchain/matrix.target inputs and ensure the rustup target add invocation runs after the action provides the configured rustup (so reference the existing step name "Install Rust target" and align its inputs with other jobs that use moonrepo/setup-rust).
🤖 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.
Nitpick comments:
In @.github/workflows/pull_request.yml:
- Around line 127-153: The workflow's strategy currently uses the matrix defined
under strategy.matrix.include but doesn't set fail-fast, so a single platform
failure cancels others; to surface all platform failures, add fail-fast: false
under the same strategy block (i.e., alongside strategy.matrix) so the matrix
build runs all entries (the include list of targets like x86_64-pc-windows-msvc,
aarch64-unknown-linux-musl, x86_64-unknown-linux-gnu, etc.) without
short-circuiting on first failure.
- Around line 157-158: The "Install Rust target" step currently runs rustup
directly and relies on a preinstalled Rust; replace that step to use the
moonrepo/setup-rust action (the same action used in other jobs) so the workflow
installs/configures a consistent toolchain and caching; update the step named
"Install Rust target" to call the moonrepo/setup-rust action with the
appropriate toolchain/matrix.target inputs and ensure the rustup target add
invocation runs after the action provides the configured rustup (so reference
the existing step name "Install Rust target" and align its inputs with other
jobs that use moonrepo/setup-rust).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1fa2813-2d33-4f9a-a145-703eb4197e96
📒 Files selected for processing (1)
.github/workflows/pull_request.yml
dyc3
force-pushed
the
dyc3/smoke-test-releases
branch
from
843dfcf to
536ed2c
Compare
dyc3
force-pushed
the
dyc3/smoke-test-releases
branch
from
536ed2c to
ff407e7
Compare
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/pull_request.yml:
- Around line 165-180: The workflow currently builds and copies the debug
artifacts (the "Build binary" step uses cargo build and the copy steps reference
target/.../debug/.../biome), so update the job to build the release artifact and
copy the same paths used in your release flow: change the build command in the
"Build binary" step to use the release profile (cargo build --release -p
biome_cli --target ${{ matrix.target }}), adjust RUSTFLAGS handling if needed
for release, and update the "Copy CLI binary" steps to pull from target/${{
matrix.target }}/release/... (both Windows and non-Windows copy branches) so the
smoke test exercises the exact binary you ship.
🪄 Autofix (Beta)
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
Run ID: df7452fa-c404-42a4-b0bc-27e2524451ea
📒 Files selected for processing (1)
.github/workflows/pull_request.yml
| - name: Build binary | ||
| run: cargo build -p biome_cli --target ${{ matrix.target }} | ||
| env: | ||
| # Windows binaries can fail to build with a stack overflow error. This increases the stack size to 8MB to mitigate that. | ||
| # We do this because the alternative is to build a release binary, which takes significantly longer to build and is not necessary for a smoke test. | ||
| RUSTFLAGS: ${{ runner.os == 'Windows' && '-C link-arg=/STACK:8388608' || '' }} | ||
| - name: Copy CLI binary | ||
| if: runner.os == 'Windows' | ||
| run: | | ||
| mkdir dist | ||
| cp target/${{ matrix.target }}/debug/biome.exe ./dist/biome-${{ matrix.code-target }}.exe | ||
| - name: Copy CLI binary | ||
| if: runner.os != 'Windows' | ||
| run: | | ||
| mkdir dist | ||
| cp target/${{ matrix.target }}/debug/biome ./dist/biome-${{ matrix.code-target }} |
There was a problem hiding this comment.
Smoke-test the artefact you actually ship.
This job only builds and runs debug/ outputs, so a release-profile break can still stroll into a tag with a green PR. If this check is meant to protect releases, it needs to exercise the same build profile/path as the release flow.
🛠️ Minimal fix
- - name: Build binary
- run: cargo build -p biome_cli --target ${{ matrix.target }}
+ - name: Build binary
+ run: cargo build -p biome_cli --release --target ${{ matrix.target }}
env:
# Windows binaries can fail to build with a stack overflow error. This increases the stack size to 8MB to mitigate that.
# We do this because the alternative is to build a release binary, which takes significantly longer to build and is not necessary for a smoke test.
RUSTFLAGS: ${{ runner.os == 'Windows' && '-C link-arg=/STACK:8388608' || '' }}
@@
- cp target/${{ matrix.target }}/debug/biome.exe ./dist/biome-${{ matrix.code-target }}.exe
+ cp target/${{ matrix.target }}/release/biome.exe ./dist/biome-${{ matrix.code-target }}.exe
@@
- cp target/${{ matrix.target }}/debug/biome ./dist/biome-${{ matrix.code-target }}
+ cp target/${{ matrix.target }}/release/biome ./dist/biome-${{ matrix.code-target }}🤖 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/pull_request.yml around lines 165 - 180, The workflow
currently builds and copies the debug artifacts (the "Build binary" step uses
cargo build and the copy steps reference target/.../debug/.../biome), so update
the job to build the release artifact and copy the same paths used in your
release flow: change the build command in the "Build binary" step to use the
release profile (cargo build --release -p biome_cli --target ${{ matrix.target
}}), adjust RUSTFLAGS handling if needed for release, and update the "Copy CLI
binary" steps to pull from target/${{ matrix.target }}/release/... (both Windows
and non-Windows copy branches) so the smoke test exercises the exact binary you
ship.
2 participants
Summary
to prevent repeated occurances of #10270
Test Plan
Docs