From 08c144587c785038d3a9768759b7e9cf9a03c3d9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:16:32 +0300 Subject: [PATCH 1/4] 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 b10aeef6..184a6252 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 73381052..e9f699ba 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") +} From 85ceb01cb4f92e42e163b7dd00770b47a3161104 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 14:46:31 +0300 Subject: [PATCH 2/4] test(internal/harness): pin the column sizing and the input copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/harness/harness_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index e9f699ba..ecde0432 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -53,6 +53,8 @@ func TestReport_IsStableAndSorted(t *testing.T) { assert.Contains(t, got, "b") assert.Less(t, strings.Index(got, "a"), strings.Index(got, "b"), "results sorted by spec name") + assert.Equal(t, "b", results[0].Spec, + "Report sorts a copy: the caller's slice keeps the order it was passed in") } // reportLines splits a Report into its lines, dropping the trailing newline the @@ -95,6 +97,24 @@ func columnStart(t *testing.T, line, column string) int { return i } +// TestReport_OutcomeColumnIsSizedOnlyByLinesThatUseIt pins what the second +// column is measured against. A result with no Detail has nothing to the right +// of its outcome, so sizing the column to it pads every line that does carry a +// Detail out to a column standing empty on the line that set its width — the +// padding this report exists to stop emitting. The longest outcome the harness +// has goes on the line showing no detail, so widening the column is the only way +// the assertion below can fail. +func TestReport_OutcomeColumnIsSizedOnlyByLinesThatUseIt(t *testing.T) { + t.Parallel() + lines := reportLines(t, harness.Report([]harness.Result{ + {Spec: "a.yaml", Outcome: harness.OutcomeNondeterministic}, + {Spec: "b.yaml", Outcome: harness.OutcomeError, Detail: "boom"}, + })) + require.Len(t, lines, 2, "one line per result") + assert.Equal(t, "b.yaml "+string(harness.OutcomeError)+" boom", lines[1], + "a detail follows its own outcome, not a column sized by a line that has none") +} + func TestReport_LinesAreNotPaddedPastTheirLastColumn(t *testing.T) { t.Parallel() lines := reportLines(t, harness.Report([]harness.Result{ From 5602b055b36d59e8eaf95711cbfcb872865f9eb0 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 15:14:06 +0300 Subject: [PATCH 3/4] fix(internal/harness): sort the report stably and say what it renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/harness/harness.go | 16 +++++++++----- internal/harness/harness_test.go | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 184a6252..b36ce7b5 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -151,14 +151,20 @@ func deterministic(ctx context.Context, spec string, data []byte, doc *ir.Docume return "", true } -// Report renders results sorted by spec name into a stable multi-line summary, -// 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. +// Report renders results sorted by spec name into a stable multi-line summary: +// one line per spec, plus one more for every newline a Detail carries, as the +// round-trip oracle's does. Column widths are measured from the results being +// rendered, so a spec path never runs into its outcome, and they line up on the +// line each result begins. +// +// It copies its input, so the caller's slice order is preserved. The sort is +// stable for the same reason irverify's is: nothing orders two results named +// alike, and an unstable sort would render one sweep's findings differently from +// one run to the next. 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 }) + sort.SliceStable(sorted, func(i, j int) bool { return sorted[i].Spec < sorted[j].Spec }) specWidth, outcomeWidth := columnWidths(sorted) diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index ecde0432..27e7a387 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -2,6 +2,7 @@ package harness_test import ( "context" + "fmt" "strings" "testing" "unicode/utf8" @@ -148,3 +149,40 @@ func TestReport_NoResultsRenderNothing(t *testing.T) { t.Parallel() assert.Empty(t, harness.Report(nil), "an empty sweep has no lines to render") } + +// TestReport_ResultsNamedAlikeKeepTheirGivenOrder pins the stable sort. Nothing +// orders two results carrying one spec name — Check names a spec whatever its +// caller passes it — so under an unstable sort the same sweep can print its +// findings in a different order from one run to the next. +// +// The shape is what makes the assertion able to fail: 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 passes either way. These duplicates are scattered +// through enough distinct keys that the sort has to partition around them. +func TestReport_ResultsNamedAlikeKeepTheirGivenOrder(t *testing.T) { + t.Parallel() + const dup = "dup.yaml" + var results []harness.Result + var want []string + for i := range 30 { + if i%3 != 0 { + results = append(results, harness.Result{ + Spec: fmt.Sprintf("k%03d.yaml", 30-i), Outcome: harness.OutcomeOK}) + continue + } + detail := fmt.Sprintf("detail %02d", i) + results = append(results, harness.Result{ + Spec: dup, Outcome: harness.OutcomeError, Detail: detail}) + want = append(want, detail) + } + + // Only the duplicates carry a Detail, so the outcome they share is what picks + // their lines out of the report. + var got []string + for _, line := range reportLines(t, harness.Report(results)) { + if _, detail, found := strings.Cut(line, string(harness.OutcomeError)+" "); found { + got = append(got, detail) + } + } + assert.Equal(t, want, got, "results sharing a spec render in the order they were given") +} From f5519eddc802be7b55a31cef79989d3e98a2caf0 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 15:25:13 +0300 Subject: [PATCH 4/4] docs(internal/harness): say what an unstable sort actually costs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/harness/harness.go | 6 ++++-- internal/harness/harness_test.go | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/harness/harness.go b/internal/harness/harness.go index b36ce7b5..f3e1aa56 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -159,8 +159,10 @@ func deterministic(ctx context.Context, spec string, data []byte, doc *ir.Docume // // It copies its input, so the caller's slice order is preserved. The sort is // stable for the same reason irverify's is: nothing orders two results named -// alike, and an unstable sort would render one sweep's findings differently from -// one run to the next. +// alike, so an unstable sort leaves them in an order the API does not specify +// rather than the one the caller gave. Not a flaky one — sort.Slice is +// deterministic for a given input — but one no caller can rely on, which is the +// same thing a report promising a stable summary must not do. func Report(results []Result) string { sorted := make([]Result, len(results)) copy(sorted, results) diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 27e7a387..4d0d3bec 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -152,8 +152,11 @@ func TestReport_NoResultsRenderNothing(t *testing.T) { // TestReport_ResultsNamedAlikeKeepTheirGivenOrder pins the stable sort. Nothing // orders two results carrying one spec name — Check names a spec whatever its -// caller passes it — so under an unstable sort the same sweep can print its -// findings in a different order from one run to the next. +// caller passes it — so an unstable sort renders them in an order the API does +// not specify rather than the one they were given. The order below is what +// sort.Slice produces today and would keep producing, since it is deterministic +// for a given input; what it is not is the caller's, or anything a caller can +// rely on across a Go release. // // The shape is what makes the assertion able to fail: an unstable sort leaves a // short slice to an insertion pass and short-circuits one whose keys are all