From c4f5d11d73fe86b657592a5f25af89ba2267f520 Mon Sep 17 00:00:00 2001 From: Adam Hamrick Date: Wed, 17 Jun 2026 20:26:47 -0400 Subject: [PATCH 1/2] show failing tests during diagnose progress --- internal/runner/analyze.go | 42 +++++++++---- internal/runner/analyze_test.go | 17 +++++- internal/runner/diagnose_table.go | 84 +++++++++++++++++++++++++ internal/runner/diagnose_table_test.go | 85 ++++++++++++++++++++++++++ internal/runner/runner.go | 9 ++- 5 files changed, 222 insertions(+), 15 deletions(-) diff --git a/internal/runner/analyze.go b/internal/runner/analyze.go index 336eb10..a3414b0 100644 --- a/internal/runner/analyze.go +++ b/internal/runner/analyze.go @@ -653,13 +653,15 @@ func buildReportSummary(rep *Report, aggs map[testKey]*aggregate, slowThreshold // IterationDigest summarizes one iteration JSONL log for per-iteration CLI output. // Counts match a single-iteration Analyze (same rules as the final report). type IterationDigest struct { - Result string // pass, fail, timeout - RanTests int // distinct named tests (package.test) that executed (pass/fail/timeout), excluding skip-only - FailTests int // len(IterationSummaries[0].FailingTests) - TimeoutTests int // len(Timeouts) for this iteration - SkipTests int // distinct named tests skipped in this iteration - SlowTests int // tests over slow threshold - BuildFailure bool // compile/build failed or heuristic package-level fail with no named tests run + Result string // pass, fail, timeout + RanTests int // distinct named tests (package.test) that executed (pass/fail/timeout), excluding skip-only + FailTests int // len(IterationSummaries[0].FailingTests) + TimeoutTests int // len(Timeouts) for this iteration + SkipTests int // distinct named tests skipped in this iteration + SlowTests int // tests over slow threshold + BuildFailure bool // compile/build failed or heuristic package-level fail with no named tests run + FailingTests []string // named tests / packages that failed this iteration + TimedOutTests []string // named tests that timed out this iteration } // countNamedTestsRanInAggs counts distinct non-empty test keys that recorded @@ -726,13 +728,31 @@ func iterationDigestFromReport(rep *Report) IterationDigest { slowTests = rep.Summary.SlowCount } return IterationDigest{ - Result: s.Result, - FailTests: len(s.FailingTests), - SlowTests: slowTests, - TimeoutTests: len(rep.Timeouts), + Result: s.Result, + FailTests: len(s.FailingTests), + SlowTests: slowTests, + TimeoutTests: len(rep.Timeouts), + FailingTests: append([]string(nil), s.FailingTests...), + TimedOutTests: timedOutTestNamesFromReport(rep), } } +func timedOutTestNamesFromReport(rep *Report) []string { + if len(rep.Timeouts) == 0 { + return nil + } + names := make([]string, 0, len(rep.Timeouts)) + for _, e := range rep.Timeouts { + name := e.Test + if name == "" { + name = e.Package + } + names = append(names, name) + } + sort.Strings(names) + return names +} + // AnalyzeResults opens every `iteration-*.log.jsonl` file in resultsDir, in // numeric-iteration order, and delegates to Analyze. func AnalyzeResults(resultsDir string, slowThreshold time.Duration) (*Report, LogMap, error) { diff --git a/internal/runner/analyze_test.go b/internal/runner/analyze_test.go index 4b7eb13..e896dd2 100644 --- a/internal/runner/analyze_test.go +++ b/internal/runner/analyze_test.go @@ -202,9 +202,19 @@ func TestDigestIterationJSONL(t *testing.T) { assert.Equal(t, "fail", d.Result) assert.Equal(t, 0, d.RanTests) assert.Equal(t, 1, d.FailTests) + assert.Equal(t, []string{"pkg/build"}, d.FailingTests) assert.True(t, d.BuildFailure) }) + t.Run("named test fail", func(t *testing.T) { + t.Parallel() + failJSON := `{"Action":"fail","Package":"pkg/a","Test":"TestX","Elapsed":0.1}` + "\n" + d, err := DigestIterationJSONL(strings.NewReader(failJSON), 30*time.Second) + require.NoError(t, err) + assert.Equal(t, "fail", d.Result) + assert.Equal(t, []string{"TestX"}, d.FailingTests) + }) + t.Run("failed_build_field", func(t *testing.T) { t.Parallel() jsonl := `{"Action":"fail","Package":"example.com/badpkg","Elapsed":0,"FailedBuild":"example.com/badpkg.test"}` + "\n" @@ -216,14 +226,15 @@ func TestDigestIterationJSONL(t *testing.T) { t.Run("timeout", func(t *testing.T) { t.Parallel() - toJSON := `{"Action":"output","Package":"pkg/hang","Output":"panic: test timed out after 2m0s\n"} -{"Action":"fail","Package":"pkg/hang","Elapsed":120.0} + toJSON := `{"Action":"output","Package":"pkg/hang","Test":"TestHang","Output":"panic: test timed out after 2m0s\n"} +{"Action":"fail","Package":"pkg/hang","Test":"TestHang","Elapsed":120.0} ` d, err := DigestIterationJSONL(strings.NewReader(toJSON), 30*time.Second) require.NoError(t, err) assert.Equal(t, "timeout", d.Result) - assert.Equal(t, 0, d.RanTests) + assert.Equal(t, 1, d.RanTests) assert.GreaterOrEqual(t, d.TimeoutTests, 1) + assert.Equal(t, []string{"TestHang"}, d.TimedOutTests) }) t.Run("two named tests", func(t *testing.T) { diff --git a/internal/runner/diagnose_table.go b/internal/runner/diagnose_table.go index 8b1498e..2a99ed8 100644 --- a/internal/runner/diagnose_table.go +++ b/internal/runner/diagnose_table.go @@ -7,6 +7,7 @@ import ( "time" "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" "github.com/smartcontractkit/testrig/internal/output" "github.com/smartcontractkit/testrig/internal/termstyle" @@ -76,3 +77,86 @@ func diagnoseTableCountStyled(n int, kind string) string { return termstyle.Muted.Render(s) } } + +// formatDiagnoseProblemTestsSuffix appends " (TestA, TestB, +N)" after a table row, +// fitting within cols when combined with baseRow. renderName styles each test name (e.g. red). +func formatDiagnoseProblemTestsSuffix( + names []string, + cols int, + baseRow string, + renderName func(...string) string, +) string { + if len(names) == 0 { + return "" + } + if cols < 1 { + cols = 80 + } + budget := cols - ansi.StringWidth(baseRow) + if budget <= 0 { + return "" + } + + for shown := len(names); shown >= 0; shown-- { + hidden := len(names) - shown + suffix := buildDiagnoseProblemTestsSuffix(names[:shown], hidden, renderName) + if suffix == "" { + continue + } + if ansi.StringWidth(suffix) <= budget { + return suffix + } + if shown == 1 { + if truncated := truncateDiagnoseProblemTestsSuffix(names[0], hidden, budget, renderName); truncated != "" { + return truncated + } + } + } + return "" +} + +func buildDiagnoseProblemTestsSuffix(shown []string, hidden int, renderName func(...string) string) string { + if len(shown) == 0 && hidden == 0 { + return "" + } + var sb strings.Builder + sb.WriteString(termstyle.Muted.Render(" (")) + for i, name := range shown { + if i > 0 { + sb.WriteString(termstyle.Muted.Render(", ")) + } + sb.WriteString(renderName(name)) + } + if hidden > 0 { + if len(shown) > 0 { + sb.WriteString(termstyle.Muted.Render(", ")) + } + sb.WriteString(termstyle.Muted.Render(fmt.Sprintf("+%d", hidden))) + } + sb.WriteString(termstyle.Muted.Render(")")) + return sb.String() +} + +func truncateDiagnoseProblemTestsSuffix( + name string, + hidden int, + budget int, + renderName func(...string) string, +) string { + open := termstyle.Muted.Render(" (") + closeParen := termstyle.Muted.Render(")") + others := "" + if hidden > 0 { + others = termstyle.Muted.Render(", ") + termstyle.Muted.Render(fmt.Sprintf("+%d", hidden)) + } + fixed := ansi.StringWidth(open) + ansi.StringWidth(others) + ansi.StringWidth(closeParen) + avail := budget - fixed + if avail <= 0 { + return "" + } + truncated := ansi.Truncate(renderName(name), avail, "…") + if ansi.StringWidth(truncated) == 0 { + return "" + } + return open + truncated + others + closeParen +} diff --git a/internal/runner/diagnose_table_test.go b/internal/runner/diagnose_table_test.go index 3285d0e..d7bc79c 100644 --- a/internal/runner/diagnose_table_test.go +++ b/internal/runner/diagnose_table_test.go @@ -2,15 +2,18 @@ package runner import ( "fmt" + "io" "regexp" "strings" "testing" "time" + "github.com/charmbracelet/x/ansi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/smartcontractkit/testrig/internal/output" + "github.com/smartcontractkit/testrig/internal/termstyle" ) var ansiEscapeSeq = regexp.MustCompile(`\x1b\[[0-9;]*m`) @@ -85,3 +88,85 @@ func TestFormatDiagnoseIterationTableRow(t *testing.T) { }) } } + +func TestFormatDiagnoseProblemTestsSuffix(t *testing.T) { + t.Parallel() + baseRowANSI := formatDiagnoseIterationTableRow(19, IterationDigest{ + Result: "fail", RanTests: 8657, SkipTests: 103, FailTests: 4, TimeoutTests: 0, SlowTests: 14, + }, 3*time.Minute) + baseRow := stripANSI(baseRowANSI) + cases := []struct { + name string + names []string + cols int + wantPlain string + }{ + { + name: "empty", + names: nil, + cols: 120, + wantPlain: "", + }, + { + name: "two_names_wide", + names: []string{"Testxxx", "Testyyy"}, + cols: 120, + wantPlain: " (Testxxx, Testyyy)", + }, + { + name: "four_names_narrow", + names: []string{"TestA", "TestB", "TestC", "TestD"}, + cols: ansi.StringWidth(baseRowANSI) + 24, + wantPlain: " (TestA, TestB, +2)", + }, + { + name: "long_name_truncated", + names: []string{"TestVeryLongNameThatDoesNotFit"}, + cols: len(baseRow) + 20, + wantPlain: " (TestVeryLongName…)", + }, + { + name: "others_only_when_tight", + names: []string{"TestA", "TestB", "TestC"}, + cols: len(baseRow) + len(" (+3)"), + wantPlain: " (+3)", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := formatDiagnoseProblemTestsSuffix(tc.names, tc.cols, baseRowANSI, termstyle.Bad.Render) + if tc.wantPlain == "" { + assert.Empty(t, got) + return + } + assert.Equal(t, tc.wantPlain, stripANSI(got)) + for _, name := range tc.names { + if strings.Contains(tc.wantPlain, name) { + assert.Contains(t, got, termstyle.Bad.Render(name)) + } + } + require.LessOrEqual(t, ansi.StringWidth(baseRowANSI+got), tc.cols) + }) + } +} + +func TestPrintIterationDigestHuman_withFailingTests(t *testing.T) { + t.Parallel() + var stderr strings.Builder + out := output.NewForTest(false, io.Discard, &stderr, false) + out.SetTermColumnsForTest(120) + + d := IterationDigest{ + Result: "fail", + RanTests: 10, + FailTests: 2, + FailingTests: []string{"Testxxx", "Testyyy"}, + } + printIterationDigestHuman(out, 19, d, 3*time.Minute) + + got := stderr.String() + require.Contains(t, got, termstyle.Bad.Render("Testxxx")) + require.Contains(t, got, termstyle.Bad.Render("Testyyy")) + require.Contains(t, stripANSI(got), stripANSI(formatDiagnoseIterationTableRow(19, d, 3*time.Minute))) +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index c88ad64..6c4a4d1 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -1256,7 +1256,14 @@ func formatIterationDigestAI(iter, total int, d IterationDigest, dur time.Durati } func printIterationDigestHuman(out *output.Printer, iter int, d IterationDigest, dur time.Duration) { - out.HumanStderr(formatDiagnoseIterationTableRow(iter, d, dur)) + row := formatDiagnoseIterationTableRow(iter, d, dur) + switch d.Result { + case "fail": + row += formatDiagnoseProblemTestsSuffix(d.FailingTests, out.TermColumns(), row, termstyle.Bad.Render) + case "timeout": + row += formatDiagnoseProblemTestsSuffix(d.TimedOutTests, out.TermColumns(), row, termstyle.Accent.Render) + } + out.HumanStderr(row) } func renderIterationResultHuman(r string) string { From a0aa9309fac91968c00bc81632062afaecea44ff Mon Sep 17 00:00:00 2001 From: Adam Hamrick Date: Wed, 17 Jun 2026 20:48:35 -0400 Subject: [PATCH 2/2] fix bug with pre-cancel --- internal/output/inline.go | 56 ++++++++++++++++++ internal/output/inline_test.go | 15 +++++ internal/output/output.go | 24 +++++--- internal/runner/diagnose_interrupt_other.go | 7 +++ internal/runner/diagnose_interrupt_unix.go | 32 +++++++++++ internal/runner/diagnose_output.go | 9 ++- internal/runner/diagnose_process_other.go | 7 +++ internal/runner/diagnose_process_unix.go | 18 ++++++ internal/runner/runner.go | 63 +++++++++++++++------ internal/runner/runner_test.go | 33 +++++++---- 10 files changed, 226 insertions(+), 38 deletions(-) create mode 100644 internal/runner/diagnose_interrupt_other.go create mode 100644 internal/runner/diagnose_interrupt_unix.go create mode 100644 internal/runner/diagnose_process_other.go create mode 100644 internal/runner/diagnose_process_unix.go diff --git a/internal/output/inline.go b/internal/output/inline.go index 86df515..0d02133 100644 --- a/internal/output/inline.go +++ b/internal/output/inline.go @@ -3,6 +3,7 @@ package output import ( "fmt" "io" + "strings" "github.com/charmbracelet/x/ansi" "github.com/charmbracelet/x/term" @@ -74,6 +75,61 @@ func (p *Printer) clearInlineBeforeDraw() { eraseInlineLines(p.stderr, p.inlineEraseLineCount()) } +// ShowTransientNotice prints a short multi-line block that ClearTransientNotice can +// erase on a TTY. Use for Ctrl+C hints that should not remain in the iteration table. +func (p *Printer) ShowTransientNotice(text string) { + if p.aiOutput { + return + } + text = strings.TrimRight(text, "\n") + if text == "" { + return + } + p.ClearTransientNotice() + p.ClearInline() + if !p.liveInline { + for part := range strings.SplitSeq(text, "\n") { + _, _ = fmt.Fprintln(p.stderr, part) + } + return + } + cols := p.termColumns() + parts := strings.Split(text, "\n") + lines := 0 + for i, part := range parts { + if i > 0 { + _, _ = fmt.Fprint(p.stderr, "\n") + } + _, _ = fmt.Fprint(p.stderr, part) + w := inlineVisualLines(part, cols) + if w < 1 && part != "" { + w = 1 + } + lines += w + } + if lines < 1 { + lines = len(parts) + } + p.transientNoticeLines = lines +} + +// ClearTransientNotice removes the last ShowTransientNotice block from a TTY. +func (p *Printer) ClearTransientNotice() { + lines := p.transientNoticeLines + if lines <= 0 { + return + } + if p.liveInline { + eraseInlineLines(p.stderr, lines) + if lines > 1 { + // eraseInlineLines leaves the cursor on the last cleared row; move back to + // the first so the next stderr write replaces the block without a gap. + _, _ = fmt.Fprintf(p.stderr, "\033[%dA", lines-1) + } + } + p.transientNoticeLines = 0 +} + // TermColumns returns stderr width for progress fitting (defaults to 80 when unknown). func (p *Printer) TermColumns() int { return p.termColumns() diff --git a/internal/output/inline_test.go b/internal/output/inline_test.go index 93f11dc..148e290 100644 --- a/internal/output/inline_test.go +++ b/internal/output/inline_test.go @@ -127,6 +127,21 @@ func TestInlineEraseLineCount_usesCurrentWidthAfterShrink(t *testing.T) { require.GreaterOrEqual(t, p.inlineEraseLineCount(), 3) } +func TestClearTransientNotice_beforeTableRow(t *testing.T) { + t.Parallel() + var stderr strings.Builder + out := NewForTest(false, io.Discard, &stderr, true) + out.ShowTransientNotice("Stopping diagnose run\nPress Ctrl+C again") + require.Equal(t, 2, out.TransientNoticeLinesForTest()) + out.ClearTransientNotice() + out.HumanStderr(" 2 pass") + got := stderr.String() + require.NotContains(t, got, "Press Ctrl+C again 2") + require.NotContains(t, got, "Stopping diagnose run\n\n 2") + require.Contains(t, got, " 2 pass\n") + require.Contains(t, got, "\x1b[1A") +} + func TestClearInline_afterShrink_clearsReflowRows(t *testing.T) { t.Parallel() var stderr strings.Builder diff --git a/internal/output/output.go b/internal/output/output.go index 618bc35..bed0e26 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -15,15 +15,16 @@ import ( // Printer writes CLI messages for tools/test. Child processes (go test) still // attach os.Stdout/os.Stderr directly where passthrough is intended. type Printer struct { - aiOutput bool - stdout io.Writer - stderr io.Writer - stderrFD uintptr - liveInline bool // human mode and stderr is a TTY (safe for \r progress) - inlineLastLines int - inlineLastCols int // terminal width when inlineLastLine was drawn - inlineLastLine string // last RedrawInline payload (for resize-aware erase) - testTermColumns int // when >0, overrides termColumns (tests only) + aiOutput bool + stdout io.Writer + stderr io.Writer + stderrFD uintptr + liveInline bool // human mode and stderr is a TTY (safe for \r progress) + inlineLastLines int + inlineLastCols int // terminal width when inlineLastLine was drawn + inlineLastLine string // last RedrawInline payload (for resize-aware erase) + transientNoticeLines int // erasable block from ShowTransientNotice (TTY only) + testTermColumns int // when >0, overrides termColumns (tests only) } // New builds a production Printer. liveInline is enabled when stderrFD points @@ -134,3 +135,8 @@ func (p *Printer) ClearInline() { p.clearInlineBeforeDraw() p.resetInlineState() } + +// TransientNoticeLinesForTest returns erasable notice line count after ShowTransientNotice. +func (p *Printer) TransientNoticeLinesForTest() int { + return p.transientNoticeLines +} diff --git a/internal/runner/diagnose_interrupt_other.go b/internal/runner/diagnose_interrupt_other.go new file mode 100644 index 0000000..aa55d56 --- /dev/null +++ b/internal/runner/diagnose_interrupt_other.go @@ -0,0 +1,7 @@ +//go:build !unix + +package runner + +func isInterruptedIteration(iterErr error) bool { + return false +} diff --git a/internal/runner/diagnose_interrupt_unix.go b/internal/runner/diagnose_interrupt_unix.go new file mode 100644 index 0000000..5881af4 --- /dev/null +++ b/internal/runner/diagnose_interrupt_unix.go @@ -0,0 +1,32 @@ +//go:build unix + +package runner + +import ( + "context" + "errors" + "os/exec" + "syscall" +) + +// isInterruptedIteration reports whether go test exited due to SIGINT/SIGTERM +// rather than a compile/build failure. Interrupt output is often misclassified +// as a build failure when RanTests is zero. +func isInterruptedIteration(iterErr error) bool { + if iterErr == nil { + return false + } + if errors.Is(iterErr, context.Canceled) { + return true + } + var exitErr *exec.ExitError + if !errors.As(iterErr, &exitErr) { + return false + } + st, ok := exitErr.Sys().(syscall.WaitStatus) + if !ok || !st.Signaled() { + return false + } + sig := st.Signal() + return sig == syscall.SIGINT || sig == syscall.SIGTERM +} diff --git a/internal/runner/diagnose_output.go b/internal/runner/diagnose_output.go index 70e166b..3b64264 100644 --- a/internal/runner/diagnose_output.go +++ b/internal/runner/diagnose_output.go @@ -173,7 +173,7 @@ func printDiagnoseGracefulStopNotice(out *output.Printer, completed, total int) } out.ClearInline() hint := diagnoseInterruptKeyHint() - out.HumanStderr( + out.ShowTransientNotice( termstyle.Accent.Render( fmt.Sprintf("Stopping diagnose run after current iteration — %d/%d completed.", completed, total), ) + "\n" + @@ -182,3 +182,10 @@ func printDiagnoseGracefulStopNotice(out *output.Printer, completed, total int) ), ) } + +func clearDiagnoseGracefulStopNotice(out *output.Printer) { + if out == nil { + return + } + out.ClearTransientNotice() +} diff --git a/internal/runner/diagnose_process_other.go b/internal/runner/diagnose_process_other.go new file mode 100644 index 0000000..8688f26 --- /dev/null +++ b/internal/runner/diagnose_process_other.go @@ -0,0 +1,7 @@ +//go:build !unix + +package runner + +import "os/exec" + +func isolateDiagnoseChildProcessGroup(cmd *exec.Cmd) {} diff --git a/internal/runner/diagnose_process_unix.go b/internal/runner/diagnose_process_unix.go new file mode 100644 index 0000000..3fe99cd --- /dev/null +++ b/internal/runner/diagnose_process_unix.go @@ -0,0 +1,18 @@ +//go:build unix + +package runner + +import ( + "os/exec" + "syscall" +) + +// isolateDiagnoseChildProcessGroup puts each go test child in its own process +// group so a terminal SIGINT (first Ctrl+C) stops testrig only, not in-flight +// test runs. Hard cancel still signals the child explicitly via cmd.Cancel. +func isolateDiagnoseChildProcessGroup(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 6c4a4d1..e122a98 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -58,7 +58,7 @@ type diagnoseRunHooks struct { // DiagnoseRunState captures the runtime progress and limits of a diagnose session. type DiagnoseRunState struct { - completed int + completed atomic.Int32 failedFast bool failedFastReason string failedFastIteration int // 0-based diagnose iteration index; -1 if unset @@ -68,6 +68,14 @@ type DiagnoseRunState struct { liveProgress bool } +// Completed returns the number of diagnose iterations finished so far. +func (s *DiagnoseRunState) Completed() int { + if s == nil { + return 0 + } + return int(s.completed.Load()) +} + func runCommand(ctx context.Context, conf *config.App, binary string, args []string, env []string) error { dir, args, err := modresolve.ResolveArgs(conf.RepoRoot, args) if err != nil { @@ -148,7 +156,8 @@ func RunIterations( } } - if state.failedFast && state.failedFastReason == failFastReasonBuildFailure && state.failedFastIteration >= 0 { + if state.failedFast && state.failedFastReason == failFastReasonBuildFailure && state.failedFastIteration >= 0 && + !state.GracefulStop { if !out.AIOutput() { printBuildError(out, resultsDir, state.failedFastIteration) } else { @@ -216,7 +225,8 @@ func FinishDiagnoseAnalysis( start time.Time, resultsDir string, ) error { - if state.failedFast && state.failedFastReason == failFastReasonBuildFailure && state.failedFastIteration >= 0 { + if state.failedFast && state.failedFastReason == failFastReasonBuildFailure && state.failedFastIteration >= 0 && + !state.GracefulStop { return nil } @@ -256,11 +266,12 @@ func FinishDiagnoseAnalysis( } func printDiagnoseInterruptions(ctx context.Context, conf *config.App, out *output.Printer, state *DiagnoseRunState) { + clearDiagnoseGracefulStopNotice(out) if state.GracefulStop { if !out.AIOutput() { out.HumanStderr( termstyle.Accent.Render( - fmt.Sprintf("stopped early after %d/%d iterations", state.completed, conf.Iterations), + fmt.Sprintf("stopped early after %d/%d iterations", state.Completed(), conf.Iterations), ) + termstyle.Muted.Render( " — analyzing partial results…", @@ -269,11 +280,11 @@ func printDiagnoseInterruptions(ctx context.Context, conf *config.App, out *outp } } else if interrupted := ctx.Err() != nil; interrupted { if out.AIOutput() { - out.Stderrf("interrupted completed=%d total=%d\n", state.completed, conf.Iterations) + out.Stderrf("interrupted completed=%d total=%d\n", state.Completed(), conf.Iterations) } else { out.HumanStderr( termstyle.Accent.Render( - fmt.Sprintf("interrupted after %d/%d iterations", state.completed, conf.Iterations), + fmt.Sprintf("interrupted after %d/%d iterations", state.Completed(), conf.Iterations), ) + termstyle.Muted.Render( " — analyzing partial results…", @@ -464,12 +475,13 @@ func recordDiagnoseIterationResult( parallelProgress *parallelDiagnoseProgress, serialProgressMu *sync.Mutex, ) { - state.completed++ + state.completed.Add(1) state.iterDurations[result.iteration] = result.duration if state.shuffleSeeds != nil { state.shuffleSeeds[result.iteration] = result.shuffle } diagnoseWithRenderLock(parallelProgress, serialProgressMu, func() { + clearDiagnoseGracefulStopNotice(out) if parallelProgress != nil || serialProgressMu != nil { out.ClearInline() } @@ -555,8 +567,10 @@ func runDiagnoseIterations( }() var serialProgressMu *sync.Mutex - if parallel == 1 && state.liveProgress { - serialProgressMu = new(sync.Mutex) + gracefulStopCh := DiagnoseGracefulStopChan(ctx) + stderrRenderMu := &sync.Mutex{} + if parallelProgress == nil && (state.liveProgress || gracefulStopCh != nil) { + serialProgressMu = stderrRenderMu } var inFlight atomic.Int32 @@ -585,7 +599,6 @@ func runDiagnoseIterations( wg.Go(func() { defer close(jobs) - gracefulStopCh := DiagnoseGracefulStopChan(ctx) for i := range conf.Iterations { if DiagnoseGracefulStopRequested(ctx) { return @@ -615,15 +628,23 @@ func runDiagnoseIterations( var firstErr error var gracefulStopNotice sync.Once - maybePrintGracefulStopNotice := func() { - if !DiagnoseGracefulStopRequested(ctx) || inFlight.Load() > 0 { - return - } + markGracefulStop := func() { gracefulStopNotice.Do(func() { state.GracefulStop = true - printDiagnoseGracefulStopNotice(out, state.completed, conf.Iterations) + diagnoseWithRenderLock(parallelProgress, serialProgressMu, func() { + printDiagnoseGracefulStopNotice(out, state.Completed(), conf.Iterations) + }) }) } + if gracefulStopCh != nil { + go func() { + select { + case <-gracefulStopCh: + markGracefulStop() + case <-runCtx.Done(): + } + }() + } for result := range results { if result.fatalErr != nil { @@ -634,9 +655,13 @@ func runDiagnoseIterations( continue } recordDiagnoseIterationResult(state, result, conf, out, parallelProgress, serialProgressMu) - maybePrintGracefulStopNotice() } - maybePrintGracefulStopNotice() + diagnoseWithRenderLock(parallelProgress, serialProgressMu, func() { + clearDiagnoseGracefulStopNotice(out) + }) + if DiagnoseGracefulStopRequested(ctx) { + markGracefulStop() + } return state, firstErr } @@ -800,6 +825,9 @@ func shouldFailFastIteration(conf *config.App, iterErr error, d IterationDigest, return false, "" } if d.BuildFailure { + if isInterruptedIteration(iterErr) { + return false, "" + } return true, failFastReasonBuildFailure } if iterErr != nil && conf.FailFast { @@ -1326,6 +1354,7 @@ func diagnoseIteration(ctx context.Context, p diagnoseIterationParams) error { cmd.Dir = moduleDir cmd.Stdin = os.Stdin cmd.Env = append(os.Environ(), env...) + isolateDiagnoseChildProcessGroup(cmd) // Soft-cancel on ctx cancellation so `go test -json` gets a chance to flush // its final events before we escalate to SIGKILL after WaitDelay. cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 9a9b5f4..a2b637c 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -829,7 +829,7 @@ func TestRunDiagnoseIterationsRunsInParallelWithWorkerIsolation(t *testing.T) { hooks, ) require.NoError(t, err) - assert.Equal(t, 4, state.completed) + assert.Equal(t, 4, state.Completed()) assert.Equal(t, int32(2), maxActive.Load()) assert.Equal(t, 2, resets, "each worker should reset before being reused") assert.Equal(t, 4, dumps) @@ -943,7 +943,7 @@ func TestRunDiagnoseIterationsStopsOnBuildFailure(t *testing.T) { hooks, ) require.NoError(t, err) - assert.Equal(t, 1, state.completed) + assert.Equal(t, 1, state.Completed()) assert.True(t, state.failedFast) assert.Equal(t, failFastReasonBuildFailure, state.failedFastReason) assert.Equal(t, 0, state.failedFastIteration) @@ -1021,7 +1021,7 @@ func TestRunDiagnoseIterations_serialLiveProgressMutex_noMergedProgressAndTableL hooks, ) require.NoError(t, err) - require.Equal(t, conf.Iterations, state.completed) + require.Equal(t, conf.Iterations, state.Completed()) for line := range strings.SplitSeq(stderr.String(), "\n") { plain := stripANSI(ttySegmentAfterLastCR(line)) @@ -1143,7 +1143,7 @@ func TestRunDiagnoseIterationsFailFastOnCategories(t *testing.T) { hooks, ) require.NoError(t, err) - assert.Equal(t, tc.wantCompleted, state.completed) + assert.Equal(t, tc.wantCompleted, state.Completed()) assert.Equal(t, tc.wantFailed, state.failedFast) }) } @@ -1174,6 +1174,16 @@ func TestShouldFailFastIterationOptimization(t *testing.T) { assert.Empty(t, reason) } +func TestShouldFailFastIteration_interruptNotBuildFailure(t *testing.T) { + t.Parallel() + conf := &config.App{FailFast: false} + + digest := IterationDigest{Result: "fail", RanTests: 0, FailTests: 3, BuildFailure: true} + failed, reason := shouldFailFastIteration(conf, context.Canceled, digest, nil) + assert.False(t, failed) + assert.Empty(t, reason) +} + func TestExtractBuildOutput(t *testing.T) { t.Parallel() @@ -1322,7 +1332,7 @@ func TestRunDiagnoseIterationsCallsIterationHooks(t *testing.T) { state, err := runDiagnoseIterations(context.Background(), conf, out, resultsDir, []string{"./pkg"}, nil, hooks) require.NoError(t, err) - assert.Equal(t, 3, state.completed) + assert.Equal(t, 3, state.Completed()) assert.Equal(t, int32(3), setupCalls.Load(), "setup called once per iteration") assert.Equal(t, int32(3), teardownCalls.Load(), "teardown called once per iteration") assert.True(t, setupBeforeTeardown.Load(), "setup must be called before teardown") @@ -1380,13 +1390,13 @@ func TestRunDiagnoseIterations_gracefulStop_finishesInFlight(t *testing.T) { hooks, ) require.NoError(t, err) - assert.Equal(t, 1, state.completed) + assert.Equal(t, 1, state.Completed()) assert.True(t, state.GracefulStop) assert.Len(t, started, 1) }) } -func TestRunDiagnoseIterations_gracefulStop_printsNoticeAfterDigest(t *testing.T) { +func TestRunDiagnoseIterations_gracefulStop_printsNoticeImmediately(t *testing.T) { t.Parallel() synctest.Test(t, func(t *testing.T) { ctx, stop := NewDiagnoseRunContextForTest(context.Background()) @@ -1405,6 +1415,7 @@ func TestRunDiagnoseIterations_gracefulStop_printsNoticeAfterDigest(t *testing.T runIteration: func(runCtx context.Context, p diagnoseIterationParams) error { if p.Iteration == 0 { RequestDiagnoseGracefulStop(runCtx) + time.Sleep(1 * time.Second) } writePassIterationJSONL(t, p.ResultsDir, p.Iteration) return nil @@ -1423,7 +1434,7 @@ func TestRunDiagnoseIterations_gracefulStop_printsNoticeAfterDigest(t *testing.T require.NoError(t, err) assert.True(t, state.GracefulStop) assert.Contains(t, stderr.String(), "Stopping diagnose run after current iteration") - assert.Contains(t, stderr.String(), "again to cancel immediately") + require.Equal(t, 0, out.TransientNoticeLinesForTest(), "notice should be cleared before run ends") }) } @@ -1477,7 +1488,7 @@ func TestRunDiagnoseIterations_gracefulStop_idleBetweenIterations(t *testing.T) require.NoError(t, runErr) require.NotNil(t, state) - assert.Equal(t, 1, state.completed) + assert.Equal(t, 1, state.Completed()) assert.True(t, state.GracefulStop) }) } @@ -1529,7 +1540,7 @@ func TestRunDiagnoseIterations_hardCancel_abortsInFlight(t *testing.T) { require.NoError(t, runErr) require.NotNil(t, state) - assert.Less(t, state.completed, conf.Iterations) + assert.Less(t, state.Completed(), conf.Iterations) assert.False(t, state.GracefulStop) require.Error(t, ctx.Err()) }) @@ -1571,7 +1582,7 @@ func TestDiagnoseGracefulStop_writesPartialReport(t *testing.T) { ) require.NoError(t, err) require.True(t, state.GracefulStop) - require.Equal(t, 2, state.completed) + require.Equal(t, 2, state.Completed()) require.NoError(t, writeRunState(resultsDir, conf, []string{"./pkg"}, state, start)) require.NoError(t, FinishDiagnoseAnalysis(ctx, conf, out, []string{"./pkg"}, state, start, resultsDir))