Skip to content

fix(internal/harness): size the report columns to the results - #358

Merged
OmarAlJarrah merged 5 commits into
mainfrom
fix/harness-report-column-widths
Aug 11, 2026
Merged

fix(internal/harness): size the report columns to the results#358
OmarAlJarrah merged 5 commits into
mainfrom
fix/harness-report-column-widths

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

harness.Report laid its table out with printf field widths chosen by eye:

_, _ = fmt.Fprintf(&b, "%-40s %-20s %s\n", r.Spec, r.Outcome, r.Detail)

r.Spec is a spec path, and nearly every spec the sweep renders is longer than
40 characters, so the outcome was not a column at all — it landed wherever the
path happened to end:

git ls-files testdata | grep -E '\.(ya?ml|json)$' | grep -v '\.golden\.' |
  awk 'length($0) > 40' | wc -l   # against the same list without the awk
testdata/conformance/openapi/allof-boolean-branch.yaml ok
testdata/conformance/openapi/allof-inheritance.yaml ok

The trailing %-20s also padded every line with spaces the report had no use
for, since most lines have no Detail to follow them.

Both widths are now measured from the results being rendered, the way the CLI's
command column is derived rather than declared. Counting is in runes, which is
the unit fmt's %-*s pads in — a byte count would over-pad a non-ASCII path.
A line whose Detail is empty stops at its outcome instead of padding out to a
column with nothing to its right; that is also what sizes the outcome column,
since only rows carrying a Detail have a neighbour to line up against.

After:

testdata/conformance/openapi/allof-boolean-branch.yaml          ok
testdata/conformance/openapi/allof-inheritance.yaml             ok

Test plan

Three new tests in internal/harness, each confirmed to redden against the
defect it names rather than accepted because it went green:

  • TestReport_ColumnsAreSizedToTheResults — the outcome and detail columns
    start at the same offset on a long-path line and a short-path line, and the
    spec column is exactly the longest spec wide. Restoring %-40s %-20s %s
    fails it on both offsets.
  • TestReport_LinesAreNotPaddedPastTheirLastColumn — no line carries padding
    after its last column. Restoring the old format fails it.
  • TestReport_WidthsAreCountedInRunes — an eight-rune, eleven-byte path is
    padded to eight. Swapping utf8.RuneCountInString for len fails it with
    expected: 9, actual: 12.
  • TestReport_OutcomeColumnIsSizedOnlyByLinesThatUseIt — the outcome column is
    measured against the results that carry a Detail and no others. Sizing it
    from every result passed the rest of the suite while padding each detail out
    past a column standing empty on the line that set its width.
  • TestReport_IsStableAndSorted now also checks the caller's slice, which the
    copy exists to protect; reading only the rendered string let an in-place sort
    pass.
  • TestReport_ResultsNamedAlikeKeepTheirGivenOrder — two results carrying one
    spec name render in the order they were given. Report documented a "stable"
    summary and sorted with sort.Slice, which is not; it uses sort.SliceStable
    now, for the reason irverify already does. The case scatters its duplicates
    through enough distinct keys that the sort must partition around them, since an
    unstable sort leaves a short slice to an insertion pass and short-circuits one
    whose keys are all equal — a two-result case would pass either way.

Report's doc comment also claimed "one aligned line per spec", which is not
what a Detail holding newlines renders, and the round-trip oracle's holds
several: it prints both encodings. It now says one line per spec plus one for
each newline a Detail carries, and that the columns line up on the line each
result begins.

columnStart fails when a column is absent, so two missing columns cannot
compare equal and assert nothing.

main is merged in, and the full gate is green on the merged branch: gofmt,
go vet ./..., golangci-lint run (0 issues), go build ./...,
./scripts/check-coverage.sh.

Closes #309

Report laid its table out with printf field widths chosen by eye -- %-40s
for the spec and %-20s for the outcome. Nearly every path in testdata is
longer than 40 characters (168 of 177 tracked paths), so the outcome was
not a column at all: it landed wherever the path happened to end. The
trailing %-20s also padded every line with spaces nothing followed.

Both widths are now measured from the results being rendered, in runes,
which is the unit fmt's %-*s pads in. A line whose Detail is empty stops
at its outcome rather than padding out to a column with nothing to its
right, which is also what sizes the outcome column: only rows carrying a
Detail have a neighbour to line up against.
Two claims Report's own documentation makes that no test held it to, each
confirmed by planting the defect and watching the new assertion redden.

Sizing the outcome column from every result rather than only the ones
carrying a Detail passed the whole suite. It is an observable difference —
a line showing no detail is the longest outcome the harness has, and
widening the column for it pads every detail out past an empty column, the
padding this report exists to stop emitting.

Report copies its input so the caller's slice order survives, and
TestReport_IsStableAndSorted read only the rendered string, so sorting in
place passed it. It now checks the caller's slice as well.
Two claims in Report's own documentation that the code did not keep.

"A stable multi-line summary" was sorted with sort.Slice, which is not
stable, so nothing ordered two results carrying one spec name and the same
sweep could print its findings differently from one run to the next. It
sorts stably now, for the reason irverify already does. The test that pins
it scatters the duplicates through enough distinct keys to make the sort
partition around them: an unstable sort leaves a short slice to an
insertion pass and short-circuits one whose keys are all equal, so a
two-result case would have passed either way.

"One aligned line per spec" is not what a Detail carrying newlines
renders, and the round-trip oracle's carries several — it prints both
encodings. The comment now says one line per spec plus one for each
newline a Detail holds, and that the columns line up on the line each
result begins.
The comment justifying the stable sort claimed an unstable one would render
a sweep differently from one run to the next. It would not: sort.Slice is
deterministic for a given input, identical across repeated sorts and across
processes. What it gives is an order the API does not specify and the caller
did not choose — no less a reason for a report that promises a stable
summary, but not the reason that was written down.
@OmarAlJarrah
OmarAlJarrah merged commit a0196d6 into main Aug 11, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/harness-report-column-widths branch August 11, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

internal/harness: Report pads to hardcoded widths nearly every spec path exceeds

1 participant