From 08c144587c785038d3a9768759b7e9cf9a03c3d9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:16:32 +0300 Subject: [PATCH] fix(internal/harness): size the report columns to the results 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. --- internal/harness/harness.go | 32 ++++++++++++-- internal/harness/harness_test.go | 76 ++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/internal/harness/harness.go b/internal/harness/harness.go index b10aeef..184a625 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -7,6 +7,7 @@ import ( "fmt" "sort" "strings" + "unicode/utf8" "github.com/dexpace/morphic/compilers" "github.com/dexpace/morphic/compilers/openapi" @@ -151,18 +152,43 @@ func deterministic(ctx context.Context, spec string, data []byte, doc *ir.Docume } // Report renders results sorted by spec name into a stable multi-line summary, -// one aligned line per spec. It copies its input, so the caller's slice order is -// preserved. +// one aligned line per spec. Column widths are measured from the results being +// rendered, so a spec path never runs into its outcome. It copies its input, so +// the caller's slice order is preserved. func Report(results []Result) string { sorted := make([]Result, len(results)) copy(sorted, results) sort.Slice(sorted, func(i, j int) bool { return sorted[i].Spec < sorted[j].Spec }) + specWidth, outcomeWidth := columnWidths(sorted) + var b strings.Builder for _, r := range sorted { // strings.Builder.Write never returns an error; the discard is explicit // so no write in this codebase is dropped silently. - _, _ = fmt.Fprintf(&b, "%-40s %-20s %s\n", r.Spec, r.Outcome, r.Detail) + if r.Detail == "" { + _, _ = fmt.Fprintf(&b, "%-*s %s\n", specWidth, r.Spec, r.Outcome) + continue + } + _, _ = fmt.Fprintf(&b, "%-*s %-*s %s\n", specWidth, r.Spec, outcomeWidth, r.Outcome, r.Detail) } return b.String() } + +// columnWidths returns the widths Report pads its first two columns to: the +// longest spec, and the longest outcome among the results that carry a Detail. +// A result with no Detail has nothing to the right of its outcome to line up, so +// it is not what the outcome column is sized against and its own line stops at +// the outcome rather than padding out to one. +// +// Widths are counted in runes because that is the unit fmt's %-*s pads in. +func columnWidths(results []Result) (spec, outcome int) { + for _, r := range results { + spec = max(spec, utf8.RuneCountInString(r.Spec)) + if r.Detail == "" { + continue + } + outcome = max(outcome, utf8.RuneCountInString(string(r.Outcome))) + } + return spec, outcome +} diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 7338105..e9f699b 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -4,8 +4,10 @@ import ( "context" "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/dexpace/morphic/internal/harness" "github.com/dexpace/morphic/internal/testspec" @@ -52,3 +54,77 @@ func TestReport_IsStableAndSorted(t *testing.T) { assert.Less(t, strings.Index(got, "a"), strings.Index(got, "b"), "results sorted by spec name") } + +// reportLines splits a Report into its lines, dropping the trailing newline the +// last line ends with so an empty final element is never mistaken for a row. +func reportLines(t *testing.T, report string) []string { + t.Helper() + require.NotEmpty(t, report, "a non-empty result set renders at least one line") + return strings.Split(strings.TrimSuffix(report, "\n"), "\n") +} + +func TestReport_ColumnsAreSizedToTheResults(t *testing.T) { + t.Parallel() + // Longer than the 40-column spec field the report used to pad to, which is + // true of nearly every path in testdata. + const longSpec = "testdata/conformance/openapi/allof-boolean-branch.yaml" + lines := reportLines(t, harness.Report([]harness.Result{ + {Spec: longSpec, Outcome: harness.OutcomeRoundtrip, Detail: "IR JSON differs"}, + {Spec: "a.yaml", Outcome: harness.OutcomeError, Detail: "boom"}, + })) + require.Len(t, lines, 2, "one line per result") + + short, long := lines[0], lines[1] // sorted by spec: a.yaml, then testdata/... + shortOutcome := columnStart(t, short, string(harness.OutcomeError)) + longOutcome := columnStart(t, long, string(harness.OutcomeRoundtrip)) + assert.Equal(t, shortOutcome, longOutcome, + "the outcome column starts at the same offset on both lines") + assert.Equal(t, columnStart(t, short, "boom"), columnStart(t, long, "IR JSON differs"), + "the detail column starts at the same offset on both lines") + assert.Equal(t, len(longSpec)+1, longOutcome, + "the spec column is exactly as wide as the longest spec, plus one separator") +} + +// columnStart returns the offset at which column begins in line, failing when +// the line does not carry it — an absent column would otherwise compare equal to +// another absent one and assert nothing. +func columnStart(t *testing.T, line, column string) int { + t.Helper() + i := strings.Index(line, column) + require.GreaterOrEqual(t, i, 0, "line %q carries column %q", line, column) + return i +} + +func TestReport_LinesAreNotPaddedPastTheirLastColumn(t *testing.T) { + t.Parallel() + lines := reportLines(t, harness.Report([]harness.Result{ + {Spec: "a.yaml", Outcome: harness.OutcomeOK}, + {Spec: "b.yaml", Outcome: harness.OutcomeError, Detail: "boom"}, + })) + require.Len(t, lines, 2, "one line per result") + for _, line := range lines { + assert.Equal(t, strings.TrimRight(line, " "), line, + "no line carries padding after its last column") + } +} + +func TestReport_WidthsAreCountedInRunes(t *testing.T) { + t.Parallel() + // Eight runes, eleven bytes: a byte-counted width would pad the spec column + // three spaces past where the spec ends, since fmt's %-*s pads in runes. + const spec = "ééé.yaml" + lines := reportLines(t, harness.Report([]harness.Result{ + {Spec: spec, Outcome: harness.OutcomeError, Detail: "boom"}, + })) + require.Len(t, lines, 1, "one line per result") + + upToOutcome, _, found := strings.Cut(lines[0], string(harness.OutcomeError)) + require.True(t, found, "the line names its outcome") + assert.Equal(t, utf8.RuneCountInString(spec)+1, utf8.RuneCountInString(upToOutcome), + "the spec column is padded to the spec's rune count, not its byte count") +} + +func TestReport_NoResultsRenderNothing(t *testing.T) { + t.Parallel() + assert.Empty(t, harness.Report(nil), "an empty sweep has no lines to render") +}