diff --git a/cmd/clone/clone.go b/cmd/clone/clone.go index f439029..9919f03 100644 --- a/cmd/clone/clone.go +++ b/cmd/clone/clone.go @@ -113,6 +113,25 @@ func run(c *cobra.Command, _ []string) { continue } createBranchActivity.EndWithSuccess() + + if !nofork { + pullFromUpstreamActivity := logger.StartActivity("Pulling latest changes from %s", repo.FullRepoName) + var defaultBranch string + defaultBranch, err = gh.GetDefaultBranchName(pullFromUpstreamActivity.Writer(), repoDirPath, repo.FullRepoName) + if err != nil { + pullFromUpstreamActivity.EndWithFailure(err) + errorCount++ + continue + } + err = g.Pull(pullFromUpstreamActivity.Writer(), repoDirPath, "upstream", defaultBranch) + if err != nil { + pullFromUpstreamActivity.EndWithFailure(err) + errorCount++ + continue + } + pullFromUpstreamActivity.EndWithSuccess() + } + doneCount++ } diff --git a/cmd/clone/clone_test.go b/cmd/clone/clone_test.go index f328f74..02e3ad7 100644 --- a/cmd/clone/clone_test.go +++ b/cmd/clone/clone_test.go @@ -115,6 +115,112 @@ func TestItLogsCheckoutErrorsButContinuesToTryAll(t *testing.T) { }) } +func TestItPullsFromUpstreamWhenCloningWithFork(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + + testsupport.PrepareTempCampaign(false, "org1/repo1", "org2/repo2") + + out, err := runCloneCommandWithFork() + assert.NoError(t, err) + assert.Contains(t, out, "Pulling latest changes from org1/repo1") + assert.Contains(t, out, "Pulling latest changes from org2/repo2") + assert.Contains(t, out, "turbolift clone completed (2 repos cloned, 0 repos skipped)") + + fakeGitHub.AssertCalledWith(t, [][]string{ + {"work/org1", "org1/repo1"}, + {"work/org1/repo1", "org1/repo1"}, + {"work/org2", "org2/repo2"}, + {"work/org2/repo2", "org2/repo2"}, + }) + fakeGit.AssertCalledWith(t, [][]string{ + {"checkout", "work/org1/repo1", testsupport.Pwd()}, + {"pull", "--ff-only", "work/org1/repo1", "upstream", "main"}, + {"checkout", "work/org2/repo2", testsupport.Pwd()}, + {"pull", "--ff-only", "work/org2/repo2", "upstream", "main"}, + }) +} + +func TestItDoesNotPullFromUpstreamWhenCloningWithoutFork(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + + testsupport.PrepareTempCampaign(false, "org1/repo1", "org2/repo2") + + out, err := runCloneCommand() + assert.NoError(t, err) + assert.NotContains(t, out, "Pulling latest changes from org1/repo1") + assert.NotContains(t, out, "Pulling latest changes from org2/repo2") + assert.Contains(t, out, "turbolift clone completed (2 repos cloned, 0 repos skipped)") + + fakeGitHub.AssertCalledWith(t, [][]string{ + {"work/org1", "org1/repo1"}, + {"work/org2", "org2/repo2"}, + }) + fakeGit.AssertCalledWith(t, [][]string{ + {"checkout", "work/org1/repo1", testsupport.Pwd()}, + {"checkout", "work/org2/repo2", testsupport.Pwd()}, + }) +} + +func TestItLogsDefaultBranchErrorsButContinuesToTryAll(t *testing.T) { + fakeGitHub := github.NewAlwaysFailsOnGetDefaultBranchFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + + testsupport.PrepareTempCampaign(false, "org1/repo1", "org2/repo2") + out, err := runCloneCommandWithFork() + assert.NoError(t, err) + assert.Contains(t, out, "Pulling latest changes from org1/repo1") + assert.Contains(t, out, "Pulling latest changes from org2/repo2") + assert.Contains(t, out, "turbolift clone completed with errors") + assert.Contains(t, out, "2 repos errored") + + fakeGitHub.AssertCalledWith(t, [][]string{ + {"work/org1", "org1/repo1"}, + {"work/org1/repo1", "org1/repo1"}, + {"work/org2", "org2/repo2"}, + {"work/org2/repo2", "org2/repo2"}, + }) + fakeGit.AssertCalledWith(t, [][]string{ + {"checkout", "work/org1/repo1", testsupport.Pwd()}, + {"checkout", "work/org2/repo2", testsupport.Pwd()}, + }) +} + +func TestItLogsPullErrorsButContinuesToTryAll(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysFailsOnPullFakeGit() + g = fakeGit + + testsupport.PrepareTempCampaign(false, "org1/repo1", "org2/repo2") + out, err := runCloneCommandWithFork() + assert.NoError(t, err) + assert.Contains(t, out, "Pulling latest changes from org1/repo1") + assert.Contains(t, out, "Pulling latest changes from org2/repo2") + assert.Contains(t, out, "turbolift clone completed with errors") + assert.Contains(t, out, "2 repos errored") + + fakeGitHub.AssertCalledWith(t, [][]string{ + {"work/org1", "org1/repo1"}, + {"work/org1/repo1", "org1/repo1"}, + {"work/org2", "org2/repo2"}, + {"work/org2/repo2", "org2/repo2"}, + }) + fakeGit.AssertCalledWith(t, [][]string{ + {"checkout", "work/org1/repo1", testsupport.Pwd()}, + {"pull", "--ff-only", "work/org1/repo1", "upstream", "main"}, + {"checkout", "work/org2/repo2", testsupport.Pwd()}, + {"pull", "--ff-only", "work/org2/repo2", "upstream", "main"}, + }) +} + func TestItClonesReposFoundInReposFile(t *testing.T) { fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() gh = fakeGitHub @@ -195,9 +301,11 @@ func TestItSkipsCloningIfAWorkingCopyAlreadyExists(t *testing.T) { fakeGitHub.AssertCalledWith(t, [][]string{ {"work/org", "org/repo2"}, + {"work/org/repo2", "org/repo2"}, }) fakeGit.AssertCalledWith(t, [][]string{ {"checkout", "work/org/repo2", testsupport.Pwd()}, + {"pull", "--ff-only", "work/org/repo2", "upstream", "main"}, }) } diff --git a/internal/git/fake_git.go b/internal/git/fake_git.go index 7b14227..133b38f 100644 --- a/internal/git/fake_git.go +++ b/internal/git/fake_git.go @@ -55,6 +55,13 @@ func (f *FakeGit) Push(output io.Writer, workingDir string, _ string, branchName return err } +func (f *FakeGit) Pull(output io.Writer, workingDir string, remote string, branchName string) error { + call := []string{"pull", "--ff-only", workingDir, remote, branchName} + f.calls = append(f.calls, call) + _, err := f.handler(output, call) + return err +} + func (f *FakeGit) AssertCalledWith(t *testing.T, expected [][]string) { assert.Equal(t, expected, f.calls) } @@ -77,3 +84,12 @@ func NewAlwaysFailsFakeGit() *FakeGit { return false, errors.New("synthetic error") }) } + +func NewAlwaysFailsOnPullFakeGit() *FakeGit { + return NewFakeGit(func(_ io.Writer, args []string) (bool, error) { + if args[0] == "pull" { + return false, errors.New("synthetic error") + } + return true, nil + }) +} diff --git a/internal/git/git.go b/internal/git/git.go index 1e0b57e..f9f83b8 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -29,6 +29,7 @@ type Git interface { Push(stdout io.Writer, workingDir string, remote string, branchName string) error Commit(output io.Writer, workingDir string, message string) error IsRepoChanged(output io.Writer, workingDir string) (bool, error) + Pull(output io.Writer, workingDir string, remote string, branchName string) error } type RealGit struct { @@ -66,6 +67,10 @@ func (r *RealGit) IsRepoChanged(output io.Writer, workingDir string) (bool, erro return diffSize > 0, nil } +func (r *RealGit) Pull(output io.Writer, workingDir string, remote string, branchName string) error { + return execInstance.Execute(output, workingDir, "git", "pull", "--ff-only", remote, branchName) +} + func NewRealGit() *RealGit { return &RealGit{} } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 9952f93..28d3d2a 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -22,11 +22,11 @@ import ( "testing" ) -func TestItReturnsErrorOnFailure(t *testing.T) { +func TestItReturnsErrorOnFailedCheckout(t *testing.T) { fakeExecutor := executor.NewAlwaysFailsFakeExecutor() execInstance = fakeExecutor - _, err := runAndCaptureOutput() + _, err := runCheckoutAndCaptureOutput() assert.Error(t, err) fakeExecutor.AssertCalledWith(t, [][]string{ @@ -34,11 +34,11 @@ func TestItReturnsErrorOnFailure(t *testing.T) { }) } -func TestItReturnsNilErrorOnSuccess(t *testing.T) { +func TestItReturnsNilErrorOnSuccessfulCheckout(t *testing.T) { fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() execInstance = fakeExecutor - _, err := runAndCaptureOutput() + _, err := runCheckoutAndCaptureOutput() assert.NoError(t, err) fakeExecutor.AssertCalledWith(t, [][]string{ @@ -46,7 +46,31 @@ func TestItReturnsNilErrorOnSuccess(t *testing.T) { }) } -func runAndCaptureOutput() (string, error) { +func TestItReturnsErrorOnFailedPull(t *testing.T) { + fakeExecutor := executor.NewAlwaysFailsFakeExecutor() + execInstance = fakeExecutor + + _, err := runPullAndCaptureOutput() + assert.Error(t, err) + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org1/repo1", "git", "pull", "--ff-only", "upstream", "main"}, + }) +} + +func TestItReturnsNilErrorOnSuccessfulPull(t *testing.T) { + fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() + execInstance = fakeExecutor + + _, err := runPullAndCaptureOutput() + assert.NoError(t, err) + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org1/repo1", "git", "pull", "--ff-only", "upstream", "main"}, + }) +} + +func runCheckoutAndCaptureOutput() (string, error) { sb := strings.Builder{} err := NewRealGit().Checkout(&sb, "work/org/repo1", "some_branch") @@ -55,3 +79,10 @@ func runAndCaptureOutput() (string, error) { } return sb.String(), nil } + +func runPullAndCaptureOutput() (string, error) { + sb := strings.Builder{} + err := NewRealGit().Pull(&sb, "work/org1/repo1", "upstream", "main") + + return sb.String(), err +} diff --git a/internal/github/fake_github.go b/internal/github/fake_github.go index 4d95fda..c2bdd3d 100644 --- a/internal/github/fake_github.go +++ b/internal/github/fake_github.go @@ -24,25 +24,25 @@ import ( ) type FakeGitHub struct { - handler func(output io.Writer, workingDir string, fullRepoName string) (bool, error) + handler func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error) returningHandler func(output io.Writer, workingDir string) (interface{}, error) calls [][]string } func (f *FakeGitHub) CreatePullRequest(output io.Writer, workingDir string, metadata PullRequest) (didCreate bool, err error) { f.calls = append(f.calls, []string{workingDir, metadata.Title}) - return f.handler(output, workingDir, "") + return f.handler(output, CreatePullRequest, workingDir, "") } func (f *FakeGitHub) ForkAndClone(output io.Writer, workingDir string, fullRepoName string) error { f.calls = append(f.calls, []string{workingDir, fullRepoName}) - _, err := f.handler(output, workingDir, fullRepoName) + _, err := f.handler(output, ForkAndClone, workingDir, fullRepoName) return err } func (f *FakeGitHub) Clone(output io.Writer, workingDir string, fullRepoName string) error { f.calls = append(f.calls, []string{workingDir, fullRepoName}) - _, err := f.handler(output, workingDir, fullRepoName) + _, err := f.handler(output, Clone, workingDir, fullRepoName) return err } @@ -50,7 +50,7 @@ func (f *FakeGitHub) ClosePullRequest(output io.Writer, workingDir string, branc // TODO: handle this differently; branchName here is replacing fullRepoName // This is OK for now because fullRepoName is used nowhere in the github mocks f.calls = append(f.calls, []string{workingDir, branchName}) - _, err := f.handler(output, workingDir, branchName) + _, err := f.handler(output, ClosePullRequest, workingDir, branchName) return err } @@ -63,11 +63,17 @@ func (f *FakeGitHub) GetPR(output io.Writer, workingDir string, _ string) (*PrSt return result.(*PrStatus), err } +func (f *FakeGitHub) GetDefaultBranchName(output io.Writer, workingDir string, fullRepoName string) (string, error) { + f.calls = append(f.calls, []string{workingDir, fullRepoName}) + _, err := f.handler(output, GetDefaultBranchName, workingDir, fullRepoName) + return "main", err +} + func (f *FakeGitHub) AssertCalledWith(t *testing.T, expected [][]string) { assert.Equal(t, expected, f.calls) } -func NewFakeGitHub(h func(output io.Writer, workingDir string, fullRepoName string) (bool, error), r func(output io.Writer, workingDir string) (interface{}, error)) *FakeGitHub { +func NewFakeGitHub(h func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error), r func(output io.Writer, workingDir string) (interface{}, error)) *FakeGitHub { return &FakeGitHub{ handler: h, returningHandler: r, @@ -76,7 +82,7 @@ func NewFakeGitHub(h func(output io.Writer, workingDir string, fullRepoName stri } func NewAlwaysSucceedsFakeGitHub() *FakeGitHub { - return NewFakeGitHub(func(output io.Writer, workingDir string, fullRepoName string) (bool, error) { + return NewFakeGitHub(func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error) { return true, nil }, func(output io.Writer, workingDir string) (interface{}, error) { return PrStatus{}, nil @@ -84,7 +90,7 @@ func NewAlwaysSucceedsFakeGitHub() *FakeGitHub { } func NewAlwaysFailsFakeGitHub() *FakeGitHub { - return NewFakeGitHub(func(output io.Writer, workingDir string, fullRepoName string) (bool, error) { + return NewFakeGitHub(func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error) { return false, errors.New("synthetic error") }, func(output io.Writer, workingDir string) (interface{}, error) { return nil, errors.New("synthetic error") @@ -92,7 +98,7 @@ func NewAlwaysFailsFakeGitHub() *FakeGitHub { } func NewAlwaysThrowNoPRFound() *FakeGitHub { - return NewFakeGitHub(func(output io.Writer, workingDir string, branchName string) (bool, error) { + return NewFakeGitHub(func(output io.Writer, command Command, workingDir string, branchName string) (bool, error) { return false, &NoPRFoundError{Path: workingDir, BranchName: branchName} }, func(output io.Writer, workingDir string) (interface{}, error) { panic("should not be invoked") @@ -100,9 +106,30 @@ func NewAlwaysThrowNoPRFound() *FakeGitHub { } func NewAlwaysReturnsFalseFakeGitHub() *FakeGitHub { - return NewFakeGitHub(func(output io.Writer, workingDir string, fullRepoName string) (bool, error) { + return NewFakeGitHub(func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error) { return false, nil }, func(output io.Writer, workingDir string) (interface{}, error) { return PrStatus{}, nil }) } + +func NewAlwaysFailsOnGetDefaultBranchFakeGitHub() *FakeGitHub { + return NewFakeGitHub(func(output io.Writer, command Command, workingDir string, fullRepoName string) (bool, error) { + if command == GetDefaultBranchName { + return false, errors.New("synthetic error") + } + return true, nil + }, func(output io.Writer, workingDir string) (interface{}, error) { + return PrStatus{}, nil + }) +} + +type Command int + +const ( + ForkAndClone Command = iota + Clone + CreatePullRequest + ClosePullRequest + GetDefaultBranchName +) diff --git a/internal/github/github.go b/internal/github/github.go index 880bc1f..21ed884 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -40,6 +40,7 @@ type GitHub interface { CreatePullRequest(output io.Writer, workingDir string, metadata PullRequest) (didCreate bool, err error) ClosePullRequest(output io.Writer, workingDir string, branchName string) error GetPR(output io.Writer, workingDir string, branchName string) (*PrStatus, error) + GetDefaultBranchName(output io.Writer, workingDir string, fullRepoName string) (string, error) } type RealGitHub struct{} @@ -87,6 +88,11 @@ func (r *RealGitHub) ClosePullRequest(output io.Writer, workingDir string, branc return execInstance.Execute(output, workingDir, "gh", "pr", "close", fmt.Sprint(pr.Number)) } +func (r *RealGitHub) GetDefaultBranchName(output io.Writer, workingDir string, fullRepoName string) (string, error) { + defaultBranch, err := execInstance.ExecuteAndCapture(output, workingDir, "gh", "repo", "view", fullRepoName, "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name") + return strings.Trim(defaultBranch, "\n"), err +} + // the following is used internally to retrieve PRs from a given repository // using `gh pr status` diff --git a/internal/github/github_test.go b/internal/github/github_test.go index f5032d4..732d0d9 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -127,6 +127,30 @@ func TestItReturnsTrueAndNilErrorOnSuccessfulCreatePr(t *testing.T) { }) } +func TestItReturnsErrorOnFailedGetDefaultBranchName(t *testing.T) { + fakeExecutor := executor.NewAlwaysFailsFakeExecutor() + execInstance = fakeExecutor + + _, _, err := runGetDefaultBranchNameAndCaptureOutput() + assert.Error(t, err) + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org1/repo1", "gh", "repo", "view", "org1/repo1", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"}, + }) +} + +func TestItReturnsNilErrorOnSuccessfulGetDefaultBranchName(t *testing.T) { + fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() + execInstance = fakeExecutor + + _, _, err := runGetDefaultBranchNameAndCaptureOutput() + assert.NoError(t, err) + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org1/repo1", "gh", "repo", "view", "org1/repo1", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"}, + }) +} + func runForkAndCloneAndCaptureOutput() (string, error) { sb := strings.Builder{} err := NewRealGitHub().ForkAndClone(&sb, "work/org", "org/repo1") @@ -163,3 +187,9 @@ func runCreateDraftPrAndCaptureOutput() (bool, string, error) { return didCreatePr, sb.String(), err } + +func runGetDefaultBranchNameAndCaptureOutput() (string, string, error) { + sb := strings.Builder{} + defaultBranchName, err := NewRealGitHub().GetDefaultBranchName(&sb, "work/org1/repo1", "org1/repo1") + return defaultBranchName, sb.String(), err +}