diff --git a/internal/harness/harness.go b/internal/harness/harness.go index b10aeef..f3e1aa5 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" @@ -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 +} diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 7338105..4d0d3be 100644 --- a/internal/harness/harness_test.go +++ b/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") }