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

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"
@@ -150,19 +151,52 @@ 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. 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, 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)
-	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)
 
 	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
+}

internal/harness/harness_test.go

@@ -2,10 +2,13 @@ package harness_test
 
 import (
 	"context"
+	"fmt"
 	"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"
@@ -51,4 +54,138 @@ 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
+// 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
+}
+
+// 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{
+		{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")
+}
+
+// TestReport_ResultsNamedAlikeKeepTheirGivenOrder pins the stable sort. Nothing
+// orders two results carrying one spec name — Check names a spec whatever its
+// 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
+// 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")
 }