From a09d1619031ca8a7dd099690e9e6c53cda270e97 Mon Sep 17 00:00:00 2001 From: Benjamin Smidt Date: Sun, 17 May 2026 19:58:32 -0700 Subject: [PATCH 1/2] fix(covgate,covratchet): align live progress lines with table columns Stream live progress rows in the same column layout as the final table, with a new leading COUNT column ([N/total]) above STATUS, COVERAGE, REQUIRED, TIME, PACKAGE. The previous compact format ("[i/N] STATUS pkg time") didn't share column widths with the sorted recap, making the two blocks visually inconsistent. The final table still prints unchanged after the progress block: in completion order live, then alphabetical recap once everything finishes. The leading announcement is also shortened to "Running N packages with parallelism=P; progress:" since the COUNT header now self-documents what follows. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/services/covgate/covgate.go | 56 ++++++++++--- internal/services/covgate/covgate_test.go | 55 +++++++++++-- internal/services/covratchet/covratchet.go | 82 ++++++++++++++----- .../services/covratchet/covratchet_test.go | 53 ++++++++++-- 4 files changed, 196 insertions(+), 50 deletions(-) diff --git a/internal/services/covgate/covgate.go b/internal/services/covgate/covgate.go index 444cbcf..bb43528 100644 --- a/internal/services/covgate/covgate.go +++ b/internal/services/covgate/covgate.go @@ -5,6 +5,7 @@ import ( "io" "os" "runtime" + "strconv" "strings" "sync" "time" @@ -91,14 +92,14 @@ func (r *runner) run(opts Opts) error { if r.emitProgress { _, _ = fmt.Fprintf( - w, "Running %d packages with parallelism=%d; "+ - "progress will appear as packages finish:\n", + w, "Running %d packages with parallelism=%d; progress:\n", len(pkgs), parallelism, ) + printProgressHeader(w, len(pkgs)) + } else { + printHeader(w) } - printHeader(w) - ctx := checkPackageCtx{ module: module, srcPrefix: opts.SrcPrefix, @@ -111,6 +112,11 @@ func (r *runner) run(opts Opts) error { start := time.Now() results := r.runPackages(pkgs, ctx, parallelism, w) wallTime := time.Since(start) + + if r.emitProgress { + _, _ = fmt.Fprintln(w) + printHeader(w) + } return r.printResults(w, results, excluded, module, wallTime) } @@ -169,6 +175,8 @@ func (r *runner) runPackages( sem := make(chan struct{}, parallelism) var wg sync.WaitGroup var progressMu sync.Mutex + countWidth := len(strconv.Itoa(total)) + colWidth := progressColWidth(total) for i, pkg := range pkgs { wg.Add(1) @@ -178,13 +186,11 @@ func (r *runner) runPackages( defer func() { <-sem }() results[idx] = r.checkPackage(p, ctx) if r.emitProgress { + label := fmt.Sprintf("[%*d/%d]", countWidth, idx+1, total) progressMu.Lock() _, _ = fmt.Fprintf( - w, "[%d/%d] %s %s %s\n", - idx+1, total, - progressStatus(results[idx].output), - gocover.RelPkg(p, ctx.module), - fmtDuration(results[idx].duration), + w, "%-*s %s", + colWidth, label, firstLine(results[idx].output), ) progressMu.Unlock() } @@ -194,10 +200,15 @@ func (r *runner) runPackages( return results } -// progressStatus extracts the first whitespace-delimited token of -// the result's first output line, which is the status keyword -// (PASS, FAIL, LOOSE) printed by checkPackage. -func progressStatus(output string) string { return strings.Fields(output)[0] } +// progressColWidth returns the width of the COUNT column for total +// packages. The value 3 + 2*ndigits is always >= 5 ("COUNT") for +// total >= 0, so no clamp is needed. +func progressColWidth(total int) int { return 3 + 2*len(strconv.Itoa(total)) } + +// firstLine returns the first line of s, including the trailing +// newline. s must contain at least one '\n' — every checkResult's +// output is built from a "%...\n" format string, so this holds. +func firstLine(s string) string { return s[:strings.IndexByte(s, '\n')+1] } func (r *runner) printResults( w io.Writer, @@ -243,6 +254,25 @@ func printHeader(w io.Writer) { ) } +// printProgressHeader prints the table header used during the live +// progress stream. It adds a leading COUNT column ahead of the +// standard columns so each progress line carries an [N/total] tag +// aligned under "COUNT". +func printProgressHeader(w io.Writer, total int) { + cw := progressColWidth(total) + dashes := strings.Repeat("-", cw) + _, _ = fmt.Fprintf( + w, "%-*s %-7s %8s %8s %8s %s\n", + cw, "COUNT", + "STATUS", "COVERAGE", "REQUIRED", "TIME", "PACKAGE", + ) + _, _ = fmt.Fprintf( + w, "%-*s %-7s %8s %8s %8s %s\n", + cw, dashes, + "-------", "--------", "--------", "--------", "-------", + ) +} + // checkResult holds the output and pass/fail status for a single package check. type checkResult struct { output string // formatted line(s) to print diff --git a/internal/services/covgate/covgate_test.go b/internal/services/covgate/covgate_test.go index 864e9ec..c845067 100644 --- a/internal/services/covgate/covgate_test.go +++ b/internal/services/covgate/covgate_test.go @@ -497,18 +497,35 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { t.Fatalf("unexpected error: %v", err) } out := buf.String() - if !strings.Contains(out, "[1/2]") { - t.Errorf("output missing [1/2] progress line:\n%s", out) + if !hasProgressLineWith(out, "[1/2]", "90.0%") { + t.Errorf( + "expected [1/2] progress line to include coverage "+ + "data (90.0%%):\n%s", out, + ) + } + if !hasProgressLineWith(out, "[2/2]", "90.0%") { + t.Errorf( + "expected [2/2] progress line to include coverage "+ + "data (90.0%%):\n%s", out, + ) + } + if !strings.Contains(out, "COUNT") { + t.Errorf("expected progress header to include COUNT column:\n%s", out) } - if !strings.Contains(out, "[2/2]") { - t.Errorf("output missing [2/2] progress line:\n%s", out) + // STATUS header should appear twice: once above the progress + // stream and once above the final alphabetical table. + if got := strings.Count(out, "STATUS"); got != 2 { + t.Errorf( + "expected STATUS header to appear twice (progress + "+ + "final); got %d:\n%s", got, out, + ) } - // Leading announcement should appear before the STATUS header. + // Leading announcement should appear before the COUNT header. idxAnnounce := strings.Index(out, "Running 2 packages with parallelism=") - idxHeader := strings.Index(out, "STATUS") + idxHeader := strings.Index(out, "COUNT") if idxAnnounce < 0 || idxHeader < 0 || idxAnnounce >= idxHeader { t.Errorf( - "expected leading announcement before STATUS header "+ + "expected leading announcement before COUNT header "+ "(announce=%d, header=%d):\n%s", idxAnnounce, idxHeader, out, ) @@ -527,6 +544,19 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { } } +// hasProgressLineWith reports whether out contains a single line +// that matches both the count marker and an additional substring +// (used to verify progress lines carry full row data, not just the +// counter). +func hasProgressLineWith(out, marker, contains string) bool { + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, marker) && strings.Contains(line, contains) { + return true + } + } + return false +} + func TestRun_SuppressesProgress_WhenExplicitParallelism(t *testing.T) { tmp := t.TempDir() t.Chdir(tmp) @@ -557,9 +587,18 @@ func TestRun_SuppressesProgress_WhenExplicitParallelism(t *testing.T) { if strings.Contains(out, "[1/2]") || strings.Contains(out, "[2/2]") { t.Errorf("output unexpectedly contains progress prefix:\n%s", out) } - if strings.Contains(out, "progress will appear") { + if strings.Contains(out, "Running 2 packages with parallelism=") { t.Errorf("output unexpectedly contains leading announcement:\n%s", out) } + if strings.Contains(out, "COUNT") { + t.Errorf("output unexpectedly contains COUNT progress header:\n%s", out) + } + if got := strings.Count(out, "STATUS"); got != 1 { + t.Errorf( + "expected exactly one STATUS header (no progress "+ + "section); got %d:\n%s", got, out, + ) + } } func TestMeasure_Pass(t *testing.T) { diff --git a/internal/services/covratchet/covratchet.go b/internal/services/covratchet/covratchet.go index 5e2e73a..ab0b3c4 100644 --- a/internal/services/covratchet/covratchet.go +++ b/internal/services/covratchet/covratchet.go @@ -74,26 +74,16 @@ func (r *runner) run(opts Opts) error { return err } - if r.emitProgress { - _, _ = fmt.Fprintf( - w, "Running %d packages with parallelism=%d; "+ - "progress will appear as packages finish:\n", - len(pkgs), parallelism, - ) - } - - printHeader(w) - + r.writeProgressBegin(w, len(pkgs), parallelism) ctx := ratchetCtx{ module: module, srcPrefix: opts.SrcPrefix, testDir: opts.TestDir, } results := r.runPackages(pkgs, ctx, parallelism, w) + r.writeProgressEnd(w) - updated := 0 - unchanged := 0 - failed := 0 + updated, unchanged, failed := 0, 0, 0 for _, res := range results { _, _ = fmt.Fprint(w, res.output) updated += res.updated @@ -112,6 +102,31 @@ func (r *runner) run(opts Opts) error { return nil } +// writeProgressBegin writes the announce line and the progress +// table header when progress is enabled; otherwise it writes the +// standard table header. +func (r *runner) writeProgressBegin(w io.Writer, total, parallelism int) { + if !r.emitProgress { + printHeader(w) + return + } + _, _ = fmt.Fprintf( + w, "Running %d packages with parallelism=%d; progress:\n", + total, parallelism, + ) + printProgressHeader(w, total) +} + +// writeProgressEnd writes the separator and the final table header +// when progress was enabled; otherwise it is a no-op. +func (r *runner) writeProgressEnd(w io.Writer) { + if !r.emitProgress { + return + } + _, _ = fmt.Fprintln(w) + printHeader(w) +} + func printHeader(w io.Writer) { _, _ = fmt.Fprintf( w, "%-6s %8s %8s %s\n", @@ -123,6 +138,33 @@ func printHeader(w io.Writer) { ) } +// printProgressHeader prints the table header used during the live +// progress stream. It adds a leading COUNT column ahead of the +// standard columns so each progress line carries an [N/total] tag +// aligned under "COUNT". +func printProgressHeader(w io.Writer, total int) { + cw := progressColWidth(total) + dashes := strings.Repeat("-", cw) + _, _ = fmt.Fprintf( + w, "%-*s %-6s %8s %8s %s\n", + cw, "COUNT", "STATUS", "PREVIOUS", "CURRENT", "PACKAGE", + ) + _, _ = fmt.Fprintf( + w, "%-*s %-6s %8s %8s %s\n", + cw, dashes, "------", "--------", "-------", "-------", + ) +} + +// progressColWidth returns the width of the COUNT column for total +// packages. The value 3 + 2*ndigits is always >= 5 ("COUNT") for +// total >= 0, so no clamp is needed. +func progressColWidth(total int) int { return 3 + 2*len(strconv.Itoa(total)) } + +// firstLine returns the first line of s, including the trailing +// newline. s must contain at least one '\n' — every ratchetResult's +// output is built from a "%...\n" format string, so this holds. +func firstLine(s string) string { return s[:strings.IndexByte(s, '\n')+1] } + // ratchetCtx holds the per-run constants threaded into ratchetPackage. type ratchetCtx struct { module string @@ -138,6 +180,8 @@ func (r *runner) runPackages( sem := make(chan struct{}, parallelism) var wg sync.WaitGroup var progressMu sync.Mutex + countWidth := len(strconv.Itoa(total)) + colWidth := progressColWidth(total) for i, pkg := range pkgs { wg.Add(1) @@ -147,12 +191,11 @@ func (r *runner) runPackages( defer func() { <-sem }() results[idx] = r.ratchetPackage(p, ctx.module, ctx.srcPrefix, ctx.testDir) if r.emitProgress { + label := fmt.Sprintf("[%*d/%d]", countWidth, idx+1, total) progressMu.Lock() _, _ = fmt.Fprintf( - w, "[%d/%d] %s %s\n", - idx+1, total, - progressStatus(results[idx].output), - gocover.RelPkg(p, ctx.module), + w, "%-*s %s", + colWidth, label, firstLine(results[idx].output), ) progressMu.Unlock() } @@ -162,11 +205,6 @@ func (r *runner) runPackages( return results } -// progressStatus extracts the first whitespace-delimited token of -// the result's first output line, which is the status keyword -// (NEW, UP, OK, FAIL) printed by ratchetPackage. -func progressStatus(output string) string { return strings.Fields(output)[0] } - // ratchetResult holds the output and counts for a single package ratchet. type ratchetResult struct { output string diff --git a/internal/services/covratchet/covratchet_test.go b/internal/services/covratchet/covratchet_test.go index ee8b77b..61283c4 100644 --- a/internal/services/covratchet/covratchet_test.go +++ b/internal/services/covratchet/covratchet_test.go @@ -446,17 +446,34 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { t.Fatalf("unexpected error: %v", err) } out := buf.String() - if !strings.Contains(out, "[1/2]") { - t.Errorf("output missing [1/2] progress line:\n%s", out) + // Each progress line should carry the per-package package + // path (the row's last column), not just the count marker. + if !hasProgressLineWith(out, "[1/2]", "pkg/") { + t.Errorf( + "expected [1/2] progress line to include the package "+ + "path:\n%s", out, + ) + } + if !hasProgressLineWith(out, "[2/2]", "pkg/") { + t.Errorf( + "expected [2/2] progress line to include the package "+ + "path:\n%s", out, + ) + } + if !strings.Contains(out, "COUNT") { + t.Errorf("expected progress header to include COUNT column:\n%s", out) } - if !strings.Contains(out, "[2/2]") { - t.Errorf("output missing [2/2] progress line:\n%s", out) + if got := strings.Count(out, "STATUS"); got != 2 { + t.Errorf( + "expected STATUS header to appear twice (progress + "+ + "final); got %d:\n%s", got, out, + ) } idxAnnounce := strings.Index(out, "Running 2 packages with parallelism=") - idxHeader := strings.Index(out, "STATUS") + idxHeader := strings.Index(out, "COUNT") if idxAnnounce < 0 || idxHeader < 0 || idxAnnounce >= idxHeader { t.Errorf( - "expected leading announcement before STATUS header "+ + "expected leading announcement before COUNT header "+ "(announce=%d, header=%d):\n%s", idxAnnounce, idxHeader, out, ) @@ -473,6 +490,19 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { } } +// hasProgressLineWith reports whether out contains a single line +// that matches both the count marker and an additional substring +// (used to verify progress lines carry full row data, not just the +// counter). +func hasProgressLineWith(out, marker, contains string) bool { + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, marker) && strings.Contains(line, contains) { + return true + } + } + return false +} + func TestRun_SuppressesProgress_WhenExplicitParallelism(t *testing.T) { tmp := t.TempDir() t.Chdir(tmp) @@ -503,9 +533,18 @@ func TestRun_SuppressesProgress_WhenExplicitParallelism(t *testing.T) { if strings.Contains(out, "[1/2]") || strings.Contains(out, "[2/2]") { t.Errorf("output unexpectedly contains progress prefix:\n%s", out) } - if strings.Contains(out, "progress will appear") { + if strings.Contains(out, "Running 2 packages with parallelism=") { t.Errorf("output unexpectedly contains leading announcement:\n%s", out) } + if strings.Contains(out, "COUNT") { + t.Errorf("output unexpectedly contains COUNT progress header:\n%s", out) + } + if got := strings.Count(out, "STATUS"); got != 1 { + t.Errorf( + "expected exactly one STATUS header (no progress "+ + "section); got %d:\n%s", got, out, + ) + } } func TestRun_PublicWrapper_HappyPath(t *testing.T) { From 7c2e2715e36dd99f189831e11b4bd977593db68d Mon Sep 17 00:00:00 2001 From: Benjamin Smidt Date: Sun, 17 May 2026 20:07:32 -0700 Subject: [PATCH 2/2] fix(covgate,covratchet): drop alphabetical recap; progress is the table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The streamed progress block and the alphabetical recap carried the exact same per-package rows, doubling the output without adding information. Now there is one table: the live progress stream. Behavior changes when --parallelism=0: - Per-package rows print only once (in completion order, as part of the progress stream). - The second STATUS header and reprinted rows are gone. - FAIL detail (multi-line raw test output for testErr cases) still prints after the stream — that's not duplicated data. - --exclude SKIPPED rows still print, indented under the COUNT column so they align with the progress table's STATUS column. Non-progress mode (--parallelism>0) is unchanged: a single alphabetical table, no progress stream. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/services/covgate/covgate.go | 49 +++++++++++++------ internal/services/covgate/covgate_test.go | 10 ++-- internal/services/covratchet/covratchet.go | 23 +++------ .../services/covratchet/covratchet_test.go | 8 +-- 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/internal/services/covgate/covgate.go b/internal/services/covgate/covgate.go index bb43528..455f14b 100644 --- a/internal/services/covgate/covgate.go +++ b/internal/services/covgate/covgate.go @@ -90,15 +90,7 @@ func (r *runner) run(opts Opts) error { return err } - if r.emitProgress { - _, _ = fmt.Fprintf( - w, "Running %d packages with parallelism=%d; progress:\n", - len(pkgs), parallelism, - ) - printProgressHeader(w, len(pkgs)) - } else { - printHeader(w) - } + r.writeRunHeader(w, len(pkgs), parallelism) ctx := checkPackageCtx{ module: module, @@ -112,11 +104,6 @@ func (r *runner) run(opts Opts) error { start := time.Now() results := r.runPackages(pkgs, ctx, parallelism, w) wallTime := time.Since(start) - - if r.emitProgress { - _, _ = fmt.Fprintln(w) - printHeader(w) - } return r.printResults(w, results, excluded, module, wallTime) } @@ -200,6 +187,21 @@ func (r *runner) runPackages( return results } +// writeRunHeader writes the announce line and the progress table +// header when progress is enabled; otherwise it writes the +// standard table header. +func (r *runner) writeRunHeader(w io.Writer, total, parallelism int) { + if !r.emitProgress { + printHeader(w) + return + } + _, _ = fmt.Fprintf( + w, "Running %d packages with parallelism=%d; progress:\n", + total, parallelism, + ) + printProgressHeader(w, total) +} + // progressColWidth returns the width of the COUNT column for total // packages. The value 3 + 2*ndigits is always >= 5 ("COUNT") for // total >= 0, so no clamp is needed. @@ -210,6 +212,12 @@ func progressColWidth(total int) int { return 3 + 2*len(strconv.Itoa(total)) } // output is built from a "%...\n" format string, so this holds. func firstLine(s string) string { return s[:strings.IndexByte(s, '\n')+1] } +// restOfOutput returns everything after the first '\n' in s. Used +// in progress mode to preserve trailing FAIL detail (multi-line +// raw test output) without reprinting the row that was already +// streamed. Same '\n' precondition as firstLine. +func restOfOutput(s string) string { return s[strings.IndexByte(s, '\n')+1:] } + func (r *runner) printResults( w io.Writer, results []checkResult, @@ -219,14 +227,23 @@ func (r *runner) printResults( ) error { hasFailures := false for _, res := range results { - _, _ = fmt.Fprint(w, res.output) + if r.emitProgress { + _, _ = fmt.Fprint(w, restOfOutput(res.output)) + } else { + _, _ = fmt.Fprint(w, res.output) + } if !res.passed { hasFailures = true } } + indent := "" + if r.emitProgress { + indent = strings.Repeat(" ", progressColWidth(len(results))) + " " + } for _, pkg := range excluded { - _, _ = fmt.Fprint(w, skippedRow(gocover.RelPkg(pkg, module))) + row := skippedRow(gocover.RelPkg(pkg, module)) + _, _ = fmt.Fprintf(w, "%s%s", indent, row) } _, _ = fmt.Fprintf(w, "\nTotal time: %s\n", fmtDuration(totalTime)) diff --git a/internal/services/covgate/covgate_test.go b/internal/services/covgate/covgate_test.go index c845067..80409eb 100644 --- a/internal/services/covgate/covgate_test.go +++ b/internal/services/covgate/covgate_test.go @@ -512,12 +512,12 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { if !strings.Contains(out, "COUNT") { t.Errorf("expected progress header to include COUNT column:\n%s", out) } - // STATUS header should appear twice: once above the progress - // stream and once above the final alphabetical table. - if got := strings.Count(out, "STATUS"); got != 2 { + // STATUS header should appear exactly once — the progress + // stream is the single canonical table. + if got := strings.Count(out, "STATUS"); got != 1 { t.Errorf( - "expected STATUS header to appear twice (progress + "+ - "final); got %d:\n%s", got, out, + "expected exactly one STATUS header (progress IS "+ + "the table); got %d:\n%s", got, out, ) } // Leading announcement should appear before the COUNT header. diff --git a/internal/services/covratchet/covratchet.go b/internal/services/covratchet/covratchet.go index ab0b3c4..7f9572b 100644 --- a/internal/services/covratchet/covratchet.go +++ b/internal/services/covratchet/covratchet.go @@ -74,18 +74,19 @@ func (r *runner) run(opts Opts) error { return err } - r.writeProgressBegin(w, len(pkgs), parallelism) + r.writeRunHeader(w, len(pkgs), parallelism) ctx := ratchetCtx{ module: module, srcPrefix: opts.SrcPrefix, testDir: opts.TestDir, } results := r.runPackages(pkgs, ctx, parallelism, w) - r.writeProgressEnd(w) updated, unchanged, failed := 0, 0, 0 for _, res := range results { - _, _ = fmt.Fprint(w, res.output) + if !r.emitProgress { + _, _ = fmt.Fprint(w, res.output) + } updated += res.updated unchanged += res.unchanged failed += res.failed @@ -102,10 +103,10 @@ func (r *runner) run(opts Opts) error { return nil } -// writeProgressBegin writes the announce line and the progress -// table header when progress is enabled; otherwise it writes the +// writeRunHeader writes the announce line and the progress table +// header when progress is enabled; otherwise it writes the // standard table header. -func (r *runner) writeProgressBegin(w io.Writer, total, parallelism int) { +func (r *runner) writeRunHeader(w io.Writer, total, parallelism int) { if !r.emitProgress { printHeader(w) return @@ -117,16 +118,6 @@ func (r *runner) writeProgressBegin(w io.Writer, total, parallelism int) { printProgressHeader(w, total) } -// writeProgressEnd writes the separator and the final table header -// when progress was enabled; otherwise it is a no-op. -func (r *runner) writeProgressEnd(w io.Writer) { - if !r.emitProgress { - return - } - _, _ = fmt.Fprintln(w) - printHeader(w) -} - func printHeader(w io.Writer) { _, _ = fmt.Fprintf( w, "%-6s %8s %8s %s\n", diff --git a/internal/services/covratchet/covratchet_test.go b/internal/services/covratchet/covratchet_test.go index 61283c4..ba71b4f 100644 --- a/internal/services/covratchet/covratchet_test.go +++ b/internal/services/covratchet/covratchet_test.go @@ -463,10 +463,12 @@ func TestRun_EmitsProgress_WhenAutoParallelism(t *testing.T) { if !strings.Contains(out, "COUNT") { t.Errorf("expected progress header to include COUNT column:\n%s", out) } - if got := strings.Count(out, "STATUS"); got != 2 { + // STATUS header should appear exactly once — the progress + // stream is the single canonical table. + if got := strings.Count(out, "STATUS"); got != 1 { t.Errorf( - "expected STATUS header to appear twice (progress + "+ - "final); got %d:\n%s", got, out, + "expected exactly one STATUS header (progress IS "+ + "the table); got %d:\n%s", got, out, ) } idxAnnounce := strings.Index(out, "Running 2 packages with parallelism=")