[go-fan] Go Module Review: charmbracelet/x/exp/golden

[github-actions]

Go Fan Report: charmbracelet/x/exp/golden 🐹

Daily round-robin review picked the most-recently-pushed unreviewed direct dependency: github.com/charmbracelet/x/exp/golden (umbrella repo pushed 2026-05-24). It's a tiny test helper — ~80 lines of code — but it's load-bearing for both the console rendering tests and the entire compiler-output snapshot suite, so it's worth a careful look.

Module Overview

A single-function golden-file assertion library. RequireEqual compares output against a checked-in testdata/<TestName>.golden file, prints a go-udiff unified diff on mismatch, and regenerates files when tests are invoked with -update. Control codes are escaped before comparison so ANSI sequences round-trip cleanly through golden files.

Current Usage in gh-aw

• Files: 3 (pkg/console/golden_test.go, pkg/workflow/wasm_golden_test.go, scripts/test-wasm-golden.mjs mirrors the snapshots from JS)
• API used: golden.RequireEqual(t, output) only — both string and []byte callers, leveraging the generic API added 2025-06-02
• Test coverage (.golden files): 29 in pkg/console/testdata/, 4 in pkg/workflow/testdata/wasm_golden/WasmBinary/, plus TestWasmGolden_CompileFixtures fixtures

Research Findings

Recent Updates

• 2026-03-09 — bumped aymanbagabas/go-udiff (deps only)
• 2025-09-08 — min Go version raised to 1.24
• 2025-06-02 — feat(exp/golden): accept strings on RequireEqual — the generic RequireEqual[T []byte | string] form gh-aw now uses
• 2025-06-02 — testdata directories are no longer ignored by the package's own .gitignore (gh-aw already commits its golden files ✅)
• RequireEqualEscape(tb, out, escapes bool) is marked deprecated in favor of RequireEqual — gh-aw does not use it (good)

Notable internals

• Registers a global flag.Bool("update", ...) at package init — importing exposes -update to the whole test binary (standard idiom, no concern)
• On mismatch, prints full expected + full got + unified diff. For multi-KB compiler YAML snapshots in TestWasmGolden_* this can flood CI logs
• Normalizes Windows \r\n to \n only on Windows

Improvement Opportunities

🏃 Quick Wins

1. Dead os.Stdout swap in TestGolden_TableRendering — pkg/console/golden_test.go:77-79 saves/restores os.Stdout, but RenderTable() returns a string and never touches stdout. The block has no effect today and would race if the test ever called t.Parallel(). Delete it.
2. Stale example in scratchpad/visual-regression-testing.md:137 — example shows golden.RequireEqual(t, []byte(output)); with the generic API the cast is redundant and inconsistent with the rest of the codebase. Drop the []byte().
3. Add the path-safety comment to pkg/workflow/wasm_golden_test.go — only pkg/console/golden_test.go warns that subtest names must avoid /. Same constraint applies where fixture filenames feed t.Run(...).

✨ Feature Opportunities

1. Diff-only wrapper for compiler snapshots — RequireEqual prints the full expected + got payload on failure. For YAML compiler snapshots that's multi-KB of noise on every failed CI run. A thin local helper that uses udiff.Unified directly and skips the dumps would make logs scannable. Source is ~30 lines, easy to copy locally.
2. Orphan-golden linter — when subtests are renamed/removed, stale .golden files survive -update. A CI check (find testdata -name '*.golden' cross-referenced with go test -list) would prevent accretion.

📐 Best Practice Alignment

1. Centralize normalization — normalizeOutput() in pkg/workflow/wasm_golden_test.go and normalize() in scripts/test-wasm-golden.mjs are mirror images maintained by hand (heredoc tokens, container pin digests, default-model strings). One drift between them would silently mask a wasm-vs-native regression. Consider extracting a single declarative manifest of regex pairs consumed by both.
2. Document -update once, in code — currently only the Makefile and the scratchpad doc mention it. A package-level doc comment in each golden_test.go would let go doc users find it.

🔧 General Improvements

• The codex engineYAML test patches output post-compilation to strip --exclude-env CODEX_API_KEY. Adding a short comment pointing at the underlying compiler conditional would help future readers locate the source-of-truth.

Recommendations

Prioritized:

1. (Quick win) Delete the no-op os.Stdout swap and update the doc example — 2-line PR.
2. (Best practice) Unify the Go and JS normalization rules into one source — prevents subtle wasm-vs-native drift.
3. (Feature) Add an orphan-golden CI check — protects against testdata bloat as the suite grows.
4. (Feature) Wrap RequireEqual for compiler snapshots so failure output stays scannable.

Next Steps

• Open small PRs for the two quick wins above.
• Discuss whether the diff-only wrapper is worth the local maintenance vs. accepting verbose CI logs on rare snapshot failures.

---

Module summary saved to scratchpad/mods/charmbracelet-x-exp-golden.md.
References: §26392689464

Generated by 🐹 Go Fan · opus47 8.4M · ◷

• expires on May 26, 2026, 9:17 AM UTC

[github-actions]

This issue was automatically closed because it expired on 2026-05-26T09:17:01.871Z.

Closed by Workflow