Skip to content

feat(cli): add smoke tests across multiple distros

39dd7b7
Select commit
Loading
Failed to load commit list.
Open

feat(cli): add smoke tests across multiple distros #239

feat(cli): add smoke tests across multiple distros
39dd7b7
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 19, 2026 in 11m 36s

Code review found 1 important issue

Found 10 candidates, confirmed 3. See review comments for details.

Details

Severity Count
πŸ”΄ Important 1
🟑 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
πŸ”΄ Important .github/workflows/run-cli-smoke-tests.yml:110-118 Smoke-test jobs reference nonexistent artifact paths
🟑 Nit .github/workflows/run-cli-smoke-tests.yml:35-37 Pin nfpm install to a specific version
🟑 Nit smoke-tests/smoke.sh:55-64 Shared-library check is a no-op for the static binary it tests

Annotations

Check failure on line 118 in .github/workflows/run-cli-smoke-tests.yml

See this annotation in the file changed.

@claude claude / Claude Code Review

Smoke-test jobs reference nonexistent artifact paths

All matrix jobs will fail at the `chmod` step: `actions/upload-artifact@v4` strips the least common ancestor of the supplied paths, so the artifact contains `dist/...` and `smoke.sh` at the root (not under a `smoke-tests/` prefix). After download to `artifacts/`, the files live at `artifacts/dist/` and `artifacts/smoke.sh`, but the next step references `artifacts/smoke-tests/...`. Fix by referencing the correct paths, e.g. `chmod +x artifacts/smoke.sh` and bind-mount `artifacts/dist` and `artifa

Check warning on line 37 in .github/workflows/run-cli-smoke-tests.yml

See this annotation in the file changed.

@claude claude / Claude Code Review

Pin nfpm install to a specific version

Line 37 installs nfpm with `@latest`, which is inconsistent with the rest of this workflow (every `uses:` is SHA-pinned with a version comment) and with other workflows that pin tool versions (e.g. `build-rdp-bridge.yml` uses `cargo install cross --locked --version 0.2.5`). Pin to a tagged release such as `go install github.com/goreleaser/nfpm/v2/cmd/nfpm@v2.40.0` so the smoke-test artifacts are reproducible and a future upstream nfpm change cannot silently alter package layout for unrelated PRs

Check warning on line 64 in smoke-tests/smoke.sh

See this annotation in the file changed.

@claude claude / Claude Code Review

Shared-library check is a no-op for the static binary it tests

The shared-library check at smoke-tests/smoke.sh:55-63 is a no-op for the static binary it tests: with `CGO_ENABLED=0` (run-cli-smoke-tests.yml:31-34) the binary has no dynamic deps, so glibc `ldd` prints `not a dynamic executable` and musl/Alpine prints similar β€” neither output ever contains the substring `not found`, so `PASS: no missing shared libraries` is printed regardless of binary content. Either drop the check or special-case the `not a dynamic executable` output with a clearer "skipped