diff --git a/internal/cli/parity_matrix_test.go b/internal/cli/parity_matrix_test.go index 8210aaa..7900c14 100644 --- a/internal/cli/parity_matrix_test.go +++ b/internal/cli/parity_matrix_test.go @@ -518,7 +518,42 @@ func runParitySide(t *testing.T, ctx context.Context, repoRoot, cli string, tc p return result } +// parityInfraRetries bounds how many times a side is re-run when it fails with a +// transient build/environment error. The runtime lane's docker builds flake on +// contended CI runners (BuildKit "failed to solve", a registry hiccup, or a +// container-setup failure surfaced as `Command failed: docker build`), which then +// shows up as a false TS-vs-Go divergence (one side's build flaked while the other +// succeeded). A single retry absorbs the flake. Retrying is safe: a deterministic +// product failure reproduces on every attempt and still fails the case. +const parityInfraRetries = 2 // total attempts, not extra retries + func runParityCLI(ctx context.Context, repoRoot, cli, cmdArgs string, env map[string]string) (stdout, stderr string, exitCode int) { + return runWithInfraRetry(ctx, parityInfraRetries, func() (string, string, int) { + return runParityCLIOnce(ctx, repoRoot, cli, cmdArgs, env) + }) +} + +// runWithInfraRetry re-runs run while it fails with a retryable transient error, +// up to maxAttempts total. It stops early on success, on a non-retryable failure, +// or once the context is done (so a retry never runs past the per-case deadline). +func runWithInfraRetry(ctx context.Context, maxAttempts int, run func() (stdout, stderr string, exitCode int)) (stdout, stderr string, exitCode int) { + if maxAttempts < 1 { + maxAttempts = 1 + } + for attempt := 1; ; attempt++ { + // Always allow the first attempt; after that, don't start new work once the + // per-case context is done. + if attempt > 1 && ctx.Err() != nil { + return + } + stdout, stderr, exitCode = run() + if exitCode == 0 || attempt >= maxAttempts || !isRetryableFailure(stdout, stderr) || ctx.Err() != nil { + return + } + } +} + +func runParityCLIOnce(ctx context.Context, repoRoot, cli, cmdArgs string, env map[string]string) (stdout, stderr string, exitCode int) { cmd := exec.CommandContext(ctx, "/bin/sh", "-lc", cli+" "+cmdArgs) cmd.Dir = repoRoot cmd.Env = append(os.Environ(), envList(env)...) @@ -1211,6 +1246,32 @@ func isInfraError(stdout, stderr string) bool { return false } +// isRetryableFailure reports whether a non-zero CLI result looks like a transient +// build/environment failure worth re-running, as opposed to a stable product +// divergence. It is deliberately BROADER than isInfraError: a retry is safe (a +// deterministic failure reproduces on the next attempt and still goes RED), so it +// also matches the container-setup / `docker build` wrapper a flaky BuildKit or +// registry hiccup surfaces without a recognizable low-level signal. Do NOT use it +// to mark a side an unusable oracle — classifyParitySide/isInfraError must stay +// narrow so a real divergence is never silently skipped; use this only to decide a +// retry. +func isRetryableFailure(stdout, stderr string) bool { + if isInfraError(stdout, stderr) { + return true + } + combined := strings.ToLower(stdout + stderr) + retryablePatterns := []string{ + "an error occurred setting up the container", + "command failed: docker build", + } + for _, p := range retryablePatterns { + if strings.Contains(combined, p) { + return true + } + } + return false +} + type paritySideStatus struct { Skip bool Timeout bool diff --git a/internal/cli/parity_retry_test.go b/internal/cli/parity_retry_test.go new file mode 100644 index 0000000..d173340 --- /dev/null +++ b/internal/cli/parity_retry_test.go @@ -0,0 +1,134 @@ +package cli + +import ( + "context" + "testing" +) + +// TestIsRetryableFailure guards the classifier that decides whether a non-zero +// parity side is a transient build/environment flake (retryable) or a stable +// product divergence (not). The two positive fixtures are the exact signatures +// that turned dependabot PRs #9/#10 red: a BuildKit "failed to solve" on one +// side and the reference's container-setup wrapper on the other, while the other +// side succeeded. The negative fixtures must stay non-retryable so a real CLI +// contract failure is never masked by a retry. +func TestIsRetryableFailure(t *testing.T) { + cases := []struct { + name string + stdout string + stderr string + wantRetryable bool + }{ + { + name: "buildkit failed to solve (isInfraError)", + stderr: "ERROR: failed to solve: rpc error: code = Unknown desc = failed to fetch", + wantRetryable: true, + }, + { + name: "docker daemon unavailable (isInfraError)", + stderr: "Cannot connect to the Docker daemon at unix:///var/run/docker.sock", + wantRetryable: true, + }, + { + // PR #10 / main 07-14 signature: TS reference build flaked and the + // low-level BuildKit error is hidden behind the generic wrapper. + name: "reference container-setup wrapper", + stdout: `{"description":"An error occurred setting up the container.","message":"Command failed: docker build -f /tmp/x -t vsc-parity-dotfiles --platform linux/amd64 /tmp/x","outcome":"error"}`, + wantRetryable: true, + }, + { + name: "config not found is a stable contract error", + stdout: `{"outcome":"error","message":"Dev container config (path/devcontainer.json) not found."}`, + wantRetryable: false, + }, + { + name: "flag validation is a stable contract error", + stderr: "Unknown argument: --bogus-flag", + wantRetryable: false, + }, + { + name: "empty output is not retryable", + wantRetryable: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isRetryableFailure(tc.stdout, tc.stderr); got != tc.wantRetryable { + t.Errorf("isRetryableFailure(%q, %q) = %v, want %v", tc.stdout, tc.stderr, got, tc.wantRetryable) + } + }) + } +} + +// TestRunWithInfraRetry asserts the retry loop absorbs a transient build flake +// (success on the second attempt) while never masking a stable failure: a +// deterministic contract error runs exactly once, and a persistent transient +// error is retried up to the cap and then surfaced as the final non-zero result. +func TestRunWithInfraRetry(t *testing.T) { + const transient = "ERROR: failed to solve: connection reset by peer" + const stable = `{"outcome":"error","message":"Dev container config not found."}` + + t.Run("transient failure then success is absorbed", func(t *testing.T) { + calls := 0 + _, _, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + if calls == 1 { + return "", transient, 1 + } + return "ok", "", 0 + }) + if exit != 0 { + t.Errorf("exit = %d, want 0 (flake should be absorbed)", exit) + } + if calls != 2 { + t.Errorf("calls = %d, want 2 (one retry)", calls) + } + }) + + t.Run("stable contract failure is not retried", func(t *testing.T) { + calls := 0 + _, _, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + return stable, "", 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1", exit) + } + if calls != 1 { + t.Errorf("calls = %d, want 1 (no retry for a stable contract error)", calls) + } + }) + + t.Run("persistent transient failure surfaces after the cap", func(t *testing.T) { + calls := 0 + _, stderr, exit := runWithInfraRetry(context.Background(), parityInfraRetries, func() (string, string, int) { + calls++ + return "", transient, 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1 (real failure must still go red)", exit) + } + if calls != parityInfraRetries { + t.Errorf("calls = %d, want %d (retry cap)", calls, parityInfraRetries) + } + if stderr != transient { + t.Errorf("stderr = %q, want the final attempt's output", stderr) + } + }) + + t.Run("cancelled context stops retries", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + calls := 0 + _, _, exit := runWithInfraRetry(ctx, parityInfraRetries, func() (string, string, int) { + calls++ + return "", transient, 1 + }) + if exit != 1 { + t.Errorf("exit = %d, want 1", exit) + } + if calls != 1 { + t.Errorf("calls = %d, want 1 (no retry once the deadline passed)", calls) + } + }) +}