diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index eda2fda21..d1977691b 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -71,6 +71,11 @@ // minRetries is the minimum number of retry attempts we make for a failed // test, regardless of perTestBudget. Override via TS_TESTWRAPPER_MIN_RETRIES. minRetries = envInt("TS_TESTWRAPPER_MIN_RETRIES", 2) + // maxRetryTime is the maximum wall-clock time we are willing to spend on the + // whole retry phase. It ensures a run with many flakes or real failures + // doesn't block for an unreasonable amount of time. Override via + // TS_TESTWRAPPER_MAX_RETRY_TIME (a time.Duration string). + maxRetryTime = envDuration("TS_TESTWRAPPER_MAX_RETRY_TIME", 10*time.Minute) ) func envDuration(key string, def time.Duration) time.Duration { @@ -572,7 +577,7 @@ func computePerAttemptTimeout(firstFail time.Duration) time.Duration { } // retryFailedTest runs the per-test retry loop for ft. It updates ft in place. -func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs []string) { +func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs []string, deadline time.Time) { perAttempt := computePerAttemptTimeout(ft.firstFailDuration) for { if ft.everPassed { @@ -584,6 +589,14 @@ func retryFailedTest(ctx context.Context, ft *failedTest, goTestArgs, testArgs [ if ft.attempts >= minRetries && ft.totalRetryElapsed >= perTestBudget { return } + if remaining := time.Until(deadline); remaining < perAttempt { + // perAttempt represents a reasonable guess of how long the test might take + // to run, so don't even attempt to retry a test that is likely to take us + // past our overall deadline. + log.Printf("testwrapper: not retrying %s.%s because its timeout (%.1fs) would exceed the remaining max retry time (%.1fs)", + ft.pkg, ft.testName, perAttempt.Seconds(), remaining.Seconds()) + return + } // FlakeAttemptEnv is 1-indexed counting the first pass as attempt 1. // Retry attempt N is FlakeAttemptEnv = 1 + N. @@ -845,9 +858,10 @@ func main() { // Second pass: retry each failed test serially with its per-test budget. if len(failed) > 0 { + deadline := time.Now().Add(maxRetryTime) fmt.Printf("\n\nRetrying %d failed test(s) to detect flakiness...\n\n", len(failed)) for _, ft := range failed { - retryFailedTest(ctx, ft, goTestArgs, testArgs) + retryFailedTest(ctx, ft, goTestArgs, testArgs, deadline) } } diff --git a/cmd/testwrapper/testwrapper_test.go b/cmd/testwrapper/testwrapper_test.go index 0d2ceeb87..7dbc4e57b 100644 --- a/cmd/testwrapper/testwrapper_test.go +++ b/cmd/testwrapper/testwrapper_test.go @@ -450,6 +450,55 @@ func TestCached(t *testing.T) {} } } +// TestMaxRetryTime verifies the suite-wide retry deadline +// (TS_TESTWRAPPER_MAX_RETRY_TIME) is a hard backstop: when the remaining time +// is smaller than a single attempt's timeout, testwrapper skips the retry +// entirely — even below minRetries — and reports the test as a permanent +// failure. Because computePerAttemptTimeout floors each attempt at 30s, any +// deadline under 30s deterministically cuts off the first retry with no timing +// race. +func TestMaxRetryTime(t *testing.T) { + t.Parallel() + + testfile := filepath.Join(t.TempDir(), "maxretrytime_test.go") + code := []byte(`package maxretrytime_test + +import "testing" + +func TestAlwaysFail(t *testing.T) { + t.Fatal("always fails") +} +`) + if err := os.WriteFile(testfile, code, 0o644); err != nil { + t.Fatalf("writing package: %s", err) + } + + cmd := cmdTestwrapper(t, "-v", testfile) + cmd.Env = append(cmd.Env, "TS_TESTWRAPPER_MAX_RETRY_TIME=1ms") + out, err := cmd.CombinedOutput() + if code, ok := errExitCode(err); !ok || code != 1 { + t.Fatalf("testwrapper %s: expected exit code 1, got %v; output:\n%s", testfile, err, out) + } + if !bytes.Contains(out, []byte("permanent test failures JSON:")) { + t.Errorf("missing permanent test failures JSON line in output:\n%s", out) + } + if !bytes.Contains(out, []byte("would exceed the remaining max retry time")) { + t.Errorf("missing retry-deadline skip message in output:\n%s", out) + } + // The deadline overrides minRetries, so no retry attempt should run: the + // test executes exactly once (the first pass). + if got := bytes.Count(out, []byte("[retry ")); got != 0 { + t.Errorf("expected no retry attempts, but %d were made; output:\n%s", got, out) + } + if runs := bytes.Count(out, []byte("=== RUN TestAlwaysFail")); runs != 1 { + t.Errorf("expected TestAlwaysFail to run exactly once, ran %d times; output:\n%s", runs, out) + } + + if testing.Verbose() { + t.Logf("success - output:\n%s", out) + } +} + func errExitCode(err error) (int, bool) { if exit, ok := errors.AsType[*exec.ExitError](err); ok { return exit.ExitCode(), true