Skip to content

fix(test): run subprocess before reading stdout/stderr buffers#40

Merged
fullstackjam merged 2 commits intomainfrom
claude/peaceful-ramanujan-166275
Apr 21, 2026
Merged

fix(test): run subprocess before reading stdout/stderr buffers#40
fullstackjam merged 2 commits intomainfrom
claude/peaceful-ramanujan-166275

Conversation

@fullstackjam
Copy link
Copy Markdown
Collaborator

Summary

Fixes the persistent TestE2E_Publish_UpdateViaSyncSource failure that has blocked every release since v0.55.1.

Root cause: runBinary had a subtle Go evaluation-order bug:

// BEFORE — wrong
return outBuf.String(), errBuf.String(), cmd.Run()

Go evaluates return expressions left-to-right, so outBuf.String() and errBuf.String() were captured before cmd.Run() executed the subprocess. Both always returned "".

// AFTER — correct
err = cmd.Run()
return outBuf.String(), errBuf.String(), err

Why the test failed: TestE2E_Publish_UpdateViaSyncSource asserts assert.Contains(t, stderr, "bob/my-env") — but stderr was always "" regardless of what the binary printed. The companion fix in the prior commit (routing the banner to os.Stderr) is correct and still needed; this commit makes the test actually capable of observing it.

Test plan

  • CI gate-tests → Destructive tests passes on macos-latest
  • TestE2E_Publish_UpdateViaSyncSource passes

TestE2E_Publish_UpdateViaSyncSource asserts the "Publishing to
@user/slug (updating)" message lands on stderr, but ui.Info prints
via fmt.Println to stdout. Every other line in publishSnapshot
(surrounding blank lines, the final success marker) already goes
through os.Stderr, so the ui.Info calls were the lone stream
mismatch and blocked the destructive-test gate in the release
workflow since v0.55.1.

Write both publish-status lines directly to os.Stderr, matching
the rest of the publish output stream and unblocking releases.
runBinary captured outBuf.String() and errBuf.String() in the same
return statement as cmd.Run(), causing Go to evaluate the buffer
reads left-to-right *before* the subprocess executed. Both captures
returned empty strings regardless of what the binary printed.

Split into two statements: run first, then read the buffers.

Also keep the companion fix from the prior commit: route the
"Publishing to @user/slug" banner to os.Stderr so it appears on the
stream the test asserts against.
@github-actions github-actions Bot added the tests Tests only label Apr 21, 2026
@github-actions
Copy link
Copy Markdown

👋 Thanks for opening this pull request!

Before merging:

  • Code follows existing patterns in the codebase
  • go build ./... and go vet ./... pass
  • Commit message is clear and descriptive

@fullstackjam will review this soon. Thanks for contributing! 🚀

@fullstackjam fullstackjam merged commit aa91e74 into main Apr 21, 2026
9 checks passed
@fullstackjam fullstackjam deleted the claude/peaceful-ramanujan-166275 branch April 21, 2026 17:02
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/snapshot_publish.go 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant