Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions internal/cli/parity_matrix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Comment thread
Copilot marked this conversation as resolved.

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)...)
Expand Down Expand Up @@ -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
}
Comment on lines +1258 to +1273

type paritySideStatus struct {
Skip bool
Timeout bool
Expand Down
134 changes: 134 additions & 0 deletions internal/cli/parity_retry_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
Loading