fix(internal/harness): size the report columns to the results - #358
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix(internal/harness): size the report columns to the results#358OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
harness.Reportlaid its table out with printf field widths chosen by eye:r.Specis a spec path, and 168 of the 177 trackedtestdatapaths are longerthan 40 characters, so the outcome was not a column at all — it landed wherever
the path happened to end:
The trailing
%-20salso padded every line with spaces the report had no usefor, since most lines have no
Detailto 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%-*spads in — a byte count would over-pad a non-ASCII path.A line whose
Detailis empty stops at its outcome instead of padding out to acolumn with nothing to its right; that is also what sizes the outcome column,
since only rows carrying a
Detailhave a neighbour to line up against.After:
Test plan
Three new tests in
internal/harness, each confirmed to redden against thedefect it names rather than accepted because it went green:
TestReport_ColumnsAreSizedToTheResults— the outcome and detail columnsstart 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 %sfails it on both offsets.
TestReport_LinesAreNotPaddedPastTheirLastColumn— no line carries paddingafter its last column. Restoring the old format fails it.
TestReport_WidthsAreCountedInRunes— an eight-rune, eleven-byte path ispadded to eight. Swapping
utf8.RuneCountInStringforlenfails it withexpected: 9, actual: 12.columnStartfails when a column is absent, so two missing columns cannotcompare equal and assert nothing.
Full gate green:
gofmt,go vet ./...,golangci-lint run(0 issues),go build ./...,./scripts/check-coverage.sh(all 4952 statements covered).Closes #309