Move build smoke tooling to Go - #86
Merged
Merged
Conversation
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
Complex PR? Review this PR in Change Stack to move by importance, not file order. WalkthroughThis PR migrates build and smoke-check functionality from TypeScript/Bun scripts to Go CLI commands within the zero-release tool. New ChangesBuild and smoke command migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (3)
cmd/zero-release/main_test.go (2)
53-64: ⚡ Quick winSmoke parser coverage is missing the
--pathalias and default-source behavior.Lines 53-64 only validate
--binarywith explicit--goos/--version. Add table cases for--path=...and for omitted flags to validate fallback/default resolution expected by the smoke CLI contract.Suggested additional cases
func TestParseSmokeArgsAcceptsPathAlias(t *testing.T) { - options, help, err := parseSmokeArgs([]string{"--binary=dist/zero", "--goos", "linux", "--version", "0.1.0"}) + options, help, err := parseSmokeArgs([]string{"--binary=dist/zero", "--goos", "linux", "--version", "0.1.0"}) ... } + +func TestParseSmokeArgsAcceptsPathFlagAlias(t *testing.T) { + options, help, err := parseSmokeArgs([]string{"--path=dist/zero", "--goos", "linux", "--version", "0.1.0"}) + if err != nil || help { + t.Fatalf("parseSmokeArgs --path failed: help=%v err=%v", help, err) + } + if options.BinaryPath != "dist/zero" { + t.Fatalf("BinaryPath = %q", options.BinaryPath) + } +}🤖 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 `@cmd/zero-release/main_test.go` around lines 53 - 64, Update the test coverage for parseSmokeArgs by expanding TestParseSmokeArgsAcceptsPathAlias into table-driven cases that include: (1) an input using the alias flag `--path=...` (verify resulting options.BinaryPath matches the `--path` value), and (2) a case where flags are omitted to assert default/fallback resolution for BinaryPath/GOOS/Version (verify defaults match the smoke CLI contract). Locate parseSmokeArgs and the test function TestParseSmokeArgsAcceptsPathAlias and add table entries with expected outputs, looping over them and asserting err==nil, help==false (or expected), and that options fields equal the expected values for each case.
29-38: ⚡ Quick winBuild override test does not actually verify env override precedence.
Line 29 switches to
emptyEnv, so this only tests CLI parsing, not “CLI overrides env” behavior under conflict. Add a case where env is set (e.g.,linux/arm64) and CLI provides different values, then assert CLI wins.Proposed test tightening
- options, help, err = parseBuildArgs([]string{"--goos=windows", "--goarch", "amd64", "--output", "dist/zero.exe"}, emptyEnv) + options, help, err = parseBuildArgs( + []string{"--goos=windows", "--goarch", "amd64", "--output", "dist/zero.exe"}, + env, // keep conflicting env set to verify precedence + )🤖 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 `@cmd/zero-release/main_test.go` around lines 29 - 38, The test currently uses emptyEnv so it only verifies CLI parsing; add a second subcase that supplies a non-empty env map (e.g., set GOOS=linux, GOARCH=arm64, OUTPUT=dist/zero) and then call parseBuildArgs with the same CLI args used here ("--goos=windows", "--goarch", "amd64", "--output", "dist/zero.exe") to ensure CLI wins; after calling parseBuildArgs(assert no error and help==false) assert that the returned options (options.GOOS, options.GOARCH, options.Output) equal the CLI values ("windows", "amd64", "dist/zero.exe") not the env values so the test covers env vs CLI precedence for parseBuildArgs.internal/release/release.go (1)
114-122: 💤 Low valueConsider resolving relative output paths to absolute.
When
options.Outputis a non-empty relative path, it's passed through as-is and returned inBuildResult.OutputPath. WhilebuildZerohandles this correctly (sincecommand.Dir = rootDir), the returnedOutputPathremains relative. This differs fromSmokewhich explicitly resolves relativeBinaryPathto absolute (lines 148-150).For API consistency and caller convenience, consider resolving relative paths:
♻️ Optional: resolve relative output to absolute
output := strings.TrimSpace(options.Output) if output == "" { output = DefaultBuildOutput(rootDir, goos) +} else if !filepath.IsAbs(output) { + output = filepath.Join(rootDir, output) }🤖 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 `@internal/release/release.go` around lines 114 - 122, The returned BuildResult currently returns output as provided (variable output from options.Output or DefaultBuildOutput) which may be a relative path; update the code that sets output before calling buildZero so relative paths are resolved to absolute paths for consistency with Smoke.BinaryPath: if output is not filepath.IsAbs(output) then join it with rootDir (filepath.Join(rootDir, output)) and call filepath.Abs or filepath.Clean+filepath.Abs to produce an absolute path, then pass that absolute output into buildZero and set BuildResult.OutputPath to that absolute value.
🤖 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 `@cmd/zero-release/main_test.go`:
- Around line 53-64: Update the test coverage for parseSmokeArgs by expanding
TestParseSmokeArgsAcceptsPathAlias into table-driven cases that include: (1) an
input using the alias flag `--path=...` (verify resulting options.BinaryPath
matches the `--path` value), and (2) a case where flags are omitted to assert
default/fallback resolution for BinaryPath/GOOS/Version (verify defaults match
the smoke CLI contract). Locate parseSmokeArgs and the test function
TestParseSmokeArgsAcceptsPathAlias and add table entries with expected outputs,
looping over them and asserting err==nil, help==false (or expected), and that
options fields equal the expected values for each case.
- Around line 29-38: The test currently uses emptyEnv so it only verifies CLI
parsing; add a second subcase that supplies a non-empty env map (e.g., set
GOOS=linux, GOARCH=arm64, OUTPUT=dist/zero) and then call parseBuildArgs with
the same CLI args used here ("--goos=windows", "--goarch", "amd64", "--output",
"dist/zero.exe") to ensure CLI wins; after calling parseBuildArgs(assert no
error and help==false) assert that the returned options (options.GOOS,
options.GOARCH, options.Output) equal the CLI values ("windows", "amd64",
"dist/zero.exe") not the env values so the test covers env vs CLI precedence for
parseBuildArgs.
In `@internal/release/release.go`:
- Around line 114-122: The returned BuildResult currently returns output as
provided (variable output from options.Output or DefaultBuildOutput) which may
be a relative path; update the code that sets output before calling buildZero so
relative paths are resolved to absolute paths for consistency with
Smoke.BinaryPath: if output is not filepath.IsAbs(output) then join it with
rootDir (filepath.Join(rootDir, output)) and call filepath.Abs or
filepath.Clean+filepath.Abs to produce an absolute path, then pass that absolute
output into buildZero and set BuildResult.OutputPath to that absolute value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cf17e6db-cd3d-4264-b362-d850013c059d
📒 Files selected for processing (12)
README.mdcmd/zero-release/main.gocmd/zero-release/main_test.gointernal/release/release.gointernal/release/release_test.gointernal/testrunner/testrunner_test.gointernal/verify/verify_test.gopackage.jsonscripts/build.tsscripts/smoke-build.tsscripts/smoke-go.tstests/build-scripts.test.ts
💤 Files with no reviewable changes (4)
- scripts/smoke-go.ts
- scripts/build.ts
- tests/build-scripts.test.ts
- scripts/smoke-build.ts
Collaborator
BlockersNone found. Non-Blocking
Looks Good
Verdict: Approve — Clean migration of build/smoke tooling to the Go release helper. |
Vasanthdev2004
approved these changes
Jun 6, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Blockers
None found.
Non-Blocking
- None.
Looks Good
- The Go
zero-release build/smokecommands preserve the release-facing script contracts while removing the replaced TS build/smoke scripts. - CLI parsing covers env overrides, missing/flag-shaped option values, help output, custom output paths, and smoke path aliases; package destructive-path guards from #85 remain intact.
- Validation passed locally:
go test -count=1 ./internal/release ./cmd/zero-release,go test -count=1 ./..., CLI help/parser edge smokes,bun install --frozen-lockfile,bun test ./tests --timeout 15000,bun run typecheck,bun run build,bun run smoke:build,bun run build:go,bun run smoke:go,bun run package:release,bun run verify:release,bun run perf:smoke, archive-content inspection, native checksum verification, README cross-build command smoke, andgit diff --check origin/main...HEAD. GitHub checks are green.
Verdict: Approve — Clean migration of build/smoke tooling to the Go release helper.
This was referenced Jun 6, 2026
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.
Summary:
Tests:
Summary by CodeRabbit
New Features
buildandsmokecommands to thezero-releaseCLI tool for building and validating releases.Refactor
zero-release buildandzero-release smoke).