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
71 changes: 67 additions & 4 deletions pkg/behaviourtest/steps/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ const forkReadyMaxAttempts = 30
// forkReadyPoll is the delay between GetBranchRef polls.
const forkReadyPoll = 2 * time.Second

// createBranchMaxAttempts is how many times createForkBranch
// retries CreateBranch when it fails with a replication error
// (409/422). Even after awaitForkReady passes, GitHub's
// eventually-consistent fork replication can cause transient
// failures when creating a branch.
const createBranchMaxAttempts = 5

// createBranchPoll is the delay between CreateBranch retries.
const createBranchPoll = 2 * time.Second

// givenFork creates a fork of the enrolled test repository if absent, or
// reuses it if it already exists. The fork is created within the same
// organization as the source repository.
Expand Down Expand Up @@ -162,6 +172,57 @@ func resolveForkName(w *world.World, logicalName string) string {
return w.RepoName + suffix
}

// isReplicationError reports whether err looks like a GitHub fork
// replication race — a 409 "Git Repository is empty" or a 422
// "Object does not exist" / "Tree SHA does not exist". These are
// transient: the underlying Git objects have not been replicated
// to the fork yet, but they will be shortly.
func isReplicationError(err error) bool {
if err == nil {
return false
}
msg := err.Error()
return strings.Contains(msg, "409") ||
(strings.Contains(msg, "422") &&
(strings.Contains(msg, "does not exist") ||
strings.Contains(msg, "empty")))
}

// createForkBranch wraps CreateBranch with retry logic for transient
// replication errors (409/422). Even after awaitForkReady confirms the
// default-branch ref is readable, GitHub's eventually-consistent fork
// replication can cause the ref to become temporarily unavailable again
// when CreateBranch re-fetches it. This retry closes that gap.
//
// maxAttempts and poll are explicit parameters so that unit tests can
// pass small values to avoid real sleeps.
func createForkBranch(ctx context.Context, w *world.World, owner, repo, branch string, maxAttempts int, poll time.Duration) error {
var lastErr error
for attempt := 1; attempt <= maxAttempts; attempt++ {
lastErr = w.SCM.CreateBranch(ctx, owner, repo, branch)
if lastErr == nil {
return nil
}
if !isReplicationError(lastErr) {
return lastErr
}
if attempt < maxAttempts {
select {
case <-ctx.Done():
return fmt.Errorf(
"context cancelled retrying CreateBranch on %s/%s: %w",
owner, repo, ctx.Err(),
)
case <-time.After(poll):
}
}
}
return fmt.Errorf(
"fork %s/%s branch creation failed after %d attempts: %w",
owner, repo, maxAttempts, lastErr,
)
}

// whenForkPullRequestOpened commits a file to a new branch on the fork
// and opens a cross-fork pull request against the base repository.
func whenForkPullRequestOpened(w *world.World) error {
Expand All @@ -174,10 +235,12 @@ func whenForkPullRequestOpened(w *world.World) error {

ctx := context.Background()

// Create the branch on the fork first — GitHub's Contents API
// (used by CommitFileToFork → CreateOrUpdateFileOnBranch) requires
// the target branch to already exist.
if err := w.SCM.CreateBranch(ctx, w.ForkOwner, w.ForkRepo, branch); err != nil {
// Create the branch on the fork with retry for replication
// errors. awaitForkReady confirmed the default-branch ref was
// readable, but GitHub's eventually-consistent replication can
// cause transient 409/422 failures when CreateBranch re-fetches
// the ref moments later.
if err := createForkBranch(ctx, w, w.ForkOwner, w.ForkRepo, branch, createBranchMaxAttempts, createBranchPoll); err != nil {
return fmt.Errorf("creating fork branch: %w", err)
}
// Record the branch immediately so CleanupScenario can delete it
Expand Down
111 changes: 111 additions & 0 deletions pkg/behaviourtest/steps/fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,98 @@ func TestAwaitForkReady_ContextCancelledDuringBranchNamePoll(t *testing.T) {
assert.Contains(t, err.Error(), "context cancelled")
}

// --- isReplicationError unit tests ---

func TestIsReplicationError_409(t *testing.T) {
err := fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty")
assert.True(t, isReplicationError(err))
}

func TestIsReplicationError_422ObjectDoesNotExist(t *testing.T) {
err := fmt.Errorf("create branch: github api: 422 Object does not exist")
assert.True(t, isReplicationError(err))
}

func TestIsReplicationError_422TreeSHADoesNotExist(t *testing.T) {
err := fmt.Errorf("create tree: github api: 422 Tree SHA does not exist")
assert.True(t, isReplicationError(err))
}

func TestIsReplicationError_NilError(t *testing.T) {
assert.False(t, isReplicationError(nil))
}

func TestIsReplicationError_NonReplicationError(t *testing.T) {
err := fmt.Errorf("permission denied")
assert.False(t, isReplicationError(err))
}

func TestIsReplicationError_422WithoutObjectMessage(t *testing.T) {
// 422 without "does not exist" or "empty" is not replication.
err := fmt.Errorf("github api: 422 Validation Failed (field=sha, code=invalid)")
assert.False(t, isReplicationError(err))
}

// --- createForkBranch unit tests ---

func TestCreateForkBranch_ImmediateSuccess(t *testing.T) {
scmDriver := &fakeForkSCM{}
w := &world.World{SCM: scmDriver}
err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0)
require.NoError(t, err)
assert.Equal(t, 1, scmDriver.createBranchCalls)
}

func TestCreateForkBranch_RetriesThenSucceeds(t *testing.T) {
scmDriver := &fakeForkSCM{
createBranchFailures: 2,
createBranchReplicaErr: fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty"),
}
w := &world.World{SCM: scmDriver}
err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0)
require.NoError(t, err)
assert.Equal(t, 3, scmDriver.createBranchCalls,
"CreateBranch should be called 2 failures + 1 success = 3 times")
}

func TestCreateForkBranch_ExhaustsRetries(t *testing.T) {
scmDriver := &fakeForkSCM{
createBranchFailures: -1,
createBranchReplicaErr: fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty"),
}
w := &world.World{SCM: scmDriver}
err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 3, 0)
require.Error(t, err)
assert.Contains(t, err.Error(), "branch creation failed after 3 attempts")
assert.Contains(t, err.Error(), "409")
assert.Equal(t, 3, scmDriver.createBranchCalls)
}

func TestCreateForkBranch_NonReplicationErrorNotRetried(t *testing.T) {
scmDriver := &fakeForkSCM{
createBranchErr: fmt.Errorf("permission denied"),
}
w := &world.World{SCM: scmDriver}
err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0)
require.Error(t, err)
assert.Contains(t, err.Error(), "permission denied")
assert.Equal(t, 1, scmDriver.createBranchCalls,
"non-replication errors should not be retried")
}

func TestCreateForkBranch_ContextCancelled(t *testing.T) {
scmDriver := &fakeForkSCM{
createBranchFailures: -1,
createBranchReplicaErr: fmt.Errorf("github api: 409 Git Repository is empty"),
}
w := &world.World{SCM: scmDriver}
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := createForkBranch(ctx, w, "org", "repo-fork", "test-branch", 30, 2*time.Second)
require.Error(t, err)
assert.Contains(t, err.Error(), "context cancelled")
}

// fakeForkSCM implements scm.Driver for fork step unit tests.
type fakeForkSCM struct {
forkRepo string
Expand Down Expand Up @@ -420,6 +512,14 @@ type fakeForkSCM struct {
// means GetBranchRef always fails.
getBranchRefFailures int
getBranchRefCalls int

// createBranchFailures controls how many times CreateBranch
// returns a replication error before succeeding. Each call
// decrements the counter; when it reaches 0, CreateBranch
// returns success. A value of -1 means CreateBranch always fails.
createBranchFailures int
createBranchCalls int
createBranchReplicaErr error // error to return on replication failures
}

type addedLabelRecord struct {
Expand Down Expand Up @@ -491,9 +591,20 @@ func (f *fakeForkSCM) CommitFile(context.Context, string, string, string, string

func (f *fakeForkSCM) CreateBranch(_ context.Context, _, _, _ string) error {
f.createBranchCalled = true
f.createBranchCalls++
if f.createBranchErr != nil {
return f.createBranchErr
}
// Support counted replication failures for retry tests.
if f.createBranchReplicaErr != nil {
if f.createBranchFailures == -1 {
return f.createBranchReplicaErr
}
if f.createBranchFailures > 0 {
f.createBranchFailures--
return f.createBranchReplicaErr
}
}
return nil
}

Expand Down
Loading