Skip to content

output: rendering has no golden tests — the suite only asserts that something was written #116

Description

@Ilyes512

The observation

internal/util/output/log_test.go
has six cases of this shape:

func TestHumanWriter_Info_NonEmpty(t *testing.T) {
	var buf bytes.Buffer

	w := output.NewHumanWriter(&buf, &bytes.Buffer{})
	w.Info("hello %s", "world")

	if buf.Len() == 0 {
		t.Error("HumanWriter.Info produced no output")
	}
}

That asserts something was written. It does not assert what, how it was styled, or — since both
buffers are passed and only one is checked — which stream it went to.
table_test.go
is in the same position for the table renderer.

Why it matters

This is the reason several output defects sat undetected long enough to be found by reading rather
than by CI. Each of the following passes the current suite unchanged:

Rendering is exactly the kind of code golden files are for: the output is long, mostly whitespace,
and tedious to assert field by field, but a diff of it reads instantly.

Proposal

A golden-file helper in the output package:

var update = flag.Bool("update", false, "rewrite the .golden files from the current output")

// assertGolden compares got against testdata/<name>.golden, rewriting the file
// instead when -update is passed.
func assertGolden(t *testing.T, name, got string)

with testdata/*.golden checked in and a task test:update target in Taskfile.dist.yml wrapping
go test ./... -update, so regenerating is one command and the diff is what gets reviewed.

Pin the colour environment. lipgloss derives a colour profile from the process, so a suite run
under a terminal and a suite run in CI can render the same call differently. Two fixed environments
make the golden files reproducible regardless of who runs them:

// The CI-log rendering: nothing in the environment for colour detection to enable.
var goldenPlain = []string{}

// Colour forced on regardless of where the test runs. CLICOLOR_FORCE overrides
// the not-a-terminal answer; TERM picks the depth.
var goldenColor = []string{"CLICOLOR_FORCE=1", "TERM=xterm-256color"}

This requires the writers to accept an environment rather than reading the process one — which is the
seam proposed in the colour-detection issue. The two issues are worth doing together, or this one
first with plain rendering only and the coloured cases added after.

Cases worth covering: each writer method's stream and exact bytes; RenderTable with ragged rows,
an empty row set, and a multibyte cell; JSONWriter line framing; WriteErr with and without a known
sentinel.

Scope

  • internal/util/output/golden_test.go (new) — the helper, the -update flag, the two environments
  • internal/util/output/testdata/*.golden — checked in
  • internal/util/output/log_test.go, table_test.go — replace the Len() == 0 assertions
  • Taskfile.dist.yml — a test:update target
  • AGENTS.md — mention task test:update alongside task test
  • While here: internal/util/exit has no test file at all

Done when

  • No test in internal/util/output asserts only that output is non-empty
  • Every Writer method has a test asserting which stream it wrote to
  • task test:update regenerates the golden files and task test then passes
  • The suite gives the same result under a terminal and in CI

Notes

Worth landing before #113, the colour-detection issue, and the column-width issue — each of those
changes rendering or stream routing, and this is what gives them something to diff against.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions