Repository navigation
Migrate output.Stdout* helpers to accept an io.Writer #22
Description
BLUF
Every structured-output helper in internal/output writes to os.Stdout directly. That makes command output untestable without mutating a process global, and it's the reason the SIP credential write-failure tests had to swap os.Stdout to simulate a broken pipe (see #20). Migrating to io.Writer + cmd.OutOrStdout() is the right long-term shape. Zero user-visible change, so this is pure hygiene — but it's 91 call sites, so it wants its own PR series.
Why now
Surfaced while fixing a Windows CI failure in #20. The test there injected a failing stdout by pointing os.Stdout at an O_RDONLY file — which fails on Unix but succeeds on Windows, so the test broke on one platform only. We fixed it by using an already-closed *os.File (ErrClosed is returned by Go's portable internal/poll layer, so it's platform-independent), which kept the change to one test file and zero production edits.
That's a fine fix, but the underlying friction remains: testing output requires global mutation, and cmd.OutOrStdout() is the convention cobra exists to provide. The repo already half-uses it — cmd/vcp, cmd/recording, cmd/message, and cmd/bxml all pass cmd.OutOrStdout() to fmt.Fprintf, while every output.* call bypasses it.
Scope
| Count | |
|---|---|
Direct output.Stdout* call sites |
91 |
| Files | 63 |
| Packages | 18 |
cmd-less local wrappers needing signature changes |
3 |
The wrappers that don't currently take a *cobra.Command:
cmd/sip/emit(format, plain, data)— 15 callerscmd/quickstart/printResult(r)— 3 callerscmd/auth/emitJSON(v)— 2 callers
Most direct call sites are tail-position return output.StdoutAuto(format, plain, result) inside a RunE, so cmd is already in scope. Those are mechanical.
The actual risk
Not the edit count — it's that the three helpers have subtly different semantics:
StdoutAuto— flattens whenplain, flattens fortableStdoutPlain— flattens, always JSONStdoutPlainList— flattens and normalizes single items to arrays, specifically to prevent single-item ambiguity in list commands
A wrong-helper swap silently changes a command's JSON shape. Auditing golden-test coverage across all 18 packages is the real work here, not the mechanical edits.
Proposed sequencing
Each step provably behavior-preserving, rather than big-bang:
- Add
Print*(w io.Writer, ...)variants and reimplement the existingStdout*helpers as one-line delegates passingos.Stdout. Pure addition, no call sites touched, trivially reviewable. - Migrate package-by-package, one PR per package or small group, golden tests as the guard.
- Add a lint rule banning new
output.Stdout*outsideinternal/output, so the migration can't regress while it's in flight.
Out of scope
Changing any command's output shape. This is a plumbing change — if a golden test needs updating, that's a bug in the migration, not an expected diff.
Activity
fix(sip): make the stdout-failure tests pass on Windows