From e957b156d1c729930a80c112df97e1e9e3514214 Mon Sep 17 00:00:00 2001 From: Sebastien Le Digabel Date: Thu, 24 Oct 2024 14:53:56 +0100 Subject: [PATCH] fix(clone): update IsPushable to use full repo name Updated the IsPushable function to use the full repository name instead of the repository directory path. The gh command now uses a repository as a parameter, since the clone has not taken place yet. --- cmd/clone/clone.go | 2 +- cmd/clone/clone_test.go | 24 ++++++++++++------------ internal/github/fake_github.go | 4 ++-- internal/github/github.go | 13 ++++++++++--- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cmd/clone/clone.go b/cmd/clone/clone.go index 4b91c4c..d01e4d7 100644 --- a/cmd/clone/clone.go +++ b/cmd/clone/clone.go @@ -77,7 +77,7 @@ func run(c *cobra.Command, _ []string) { if forceFork { fork = true } else { - res, err := gh.IsPushable(logger.Writer(), repoDirPath) + res, err := gh.IsPushable(logger.Writer(), repo.FullRepoName) if err != nil { logger.Warnf("Unable to determine if we can push to %s: %s", orgDirPath, err) fork = true diff --git a/cmd/clone/clone_test.go b/cmd/clone/clone_test.go index 5a32665..c4d4196 100644 --- a/cmd/clone/clone_test.go +++ b/cmd/clone/clone_test.go @@ -79,9 +79,9 @@ func TestItLogsCloneErrorsButContinuesToTryAll(t *testing.T) { assert.Contains(t, out, "2 repos errored") fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/org/repo1"}, + {"user_can_push", "org/repo1"}, {"clone", "work/org", "org/repo1"}, - {"user_can_push", "work/org/repo2"}, + {"user_can_push", "org/repo2"}, {"clone", "work/org", "org/repo2"}, }) fakeGit.AssertCalledWith(t, [][]string{}) @@ -187,9 +187,9 @@ func TestItDoesNotPullFromUpstreamWhenCloningWithoutFork(t *testing.T) { assert.Contains(t, out, "turbolift clone completed (2 repos cloned, 0 repos skipped)") fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/org1/repo1"}, + {"user_can_push", "org1/repo1"}, {"clone", "work/org1", "org1/repo1"}, - {"user_can_push", "work/org2/repo2"}, + {"user_can_push", "org2/repo2"}, {"clone", "work/org2", "org2/repo2"}, }) @@ -269,9 +269,9 @@ func TestItClonesReposFoundInReposFile(t *testing.T) { assert.Contains(t, out, "turbolift clone completed (2 repos cloned, 0 repos skipped)") fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/org/repo1"}, + {"user_can_push", "org/repo1"}, {"clone", "work/org", "org/repo1"}, - {"user_can_push", "work/org/repo2"}, + {"user_can_push", "org/repo2"}, {"clone", "work/org", "org/repo2"}, }) fakeGit.AssertCalledWith(t, [][]string{ @@ -292,9 +292,9 @@ func TestItClonesReposInMultipleOrgs(t *testing.T) { assert.NoError(t, err) fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/orgA/repo1"}, + {"user_can_push", "orgA/repo1"}, {"clone", "work/orgA", "orgA/repo1"}, - {"user_can_push", "work/orgB/repo2"}, + {"user_can_push", "orgB/repo2"}, {"clone", "work/orgB", "orgB/repo2"}, }) fakeGit.AssertCalledWith(t, [][]string{ @@ -315,9 +315,9 @@ func TestItClonesReposFromOtherHosts(t *testing.T) { assert.NoError(t, err) fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/orgA/repo1"}, + {"user_can_push", "mygitserver.com/orgA/repo1"}, {"clone", "work/orgA", "mygitserver.com/orgA/repo1"}, - {"user_can_push", "work/orgB/repo2"}, + {"user_can_push", "orgB/repo2"}, {"clone", "work/orgB", "orgB/repo2"}, }) fakeGit.AssertCalledWith(t, [][]string{ @@ -379,10 +379,10 @@ func TestItForksIfUserHasNoPushPermission(t *testing.T) { assert.Contains(t, out, "turbolift clone completed (2 repos cloned, 0 repos skipped)") fakeGitHub.AssertCalledWith(t, [][]string{ - {"user_can_push", "work/org/repo1"}, + {"user_can_push", "org/repo1"}, {"fork_and_clone", "work/org", "org/repo1"}, {"get_default_branch", "work/org/repo1", "org/repo1"}, - {"user_can_push", "work/org/repo2"}, + {"user_can_push", "org/repo2"}, {"fork_and_clone", "work/org", "org/repo2"}, {"get_default_branch", "work/org/repo2", "org/repo2"}, }) diff --git a/internal/github/fake_github.go b/internal/github/fake_github.go index 30f1b2f..a0c7925 100644 --- a/internal/github/fake_github.go +++ b/internal/github/fake_github.go @@ -61,8 +61,8 @@ func (f *FakeGitHub) Clone(_ io.Writer, workingDir string, fullRepoName string) return err } -func (f *FakeGitHub) IsPushable(_ io.Writer, repoDir string) (bool, error) { - args := []string{"user_can_push", repoDir} +func (f *FakeGitHub) IsPushable(_ io.Writer, repo string) (bool, error) { + args := []string{"user_can_push", repo} f.calls = append(f.calls, args) return f.handler(IsPushable, args) } diff --git a/internal/github/github.go b/internal/github/github.go index 41809ee..898c390 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "io" + "os" "strings" "github.com/skyscanner/turbolift/internal/executor" @@ -42,7 +43,7 @@ type GitHub interface { UpdatePRDescription(output io.Writer, workingDir string, title string, body string) error GetPR(output io.Writer, workingDir string, branchName string) (*PrStatus, error) GetDefaultBranchName(output io.Writer, workingDir string, fullRepoName string) (string, error) - IsPushable(output io.Writer, repoDir string) (bool, error) + IsPushable(output io.Writer, repo string) (bool, error) } type RealGitHub struct{} @@ -171,8 +172,14 @@ func (r *RealGitHub) GetPR(output io.Writer, workingDir string, branchName strin return nil, &NoPRFoundError{Path: workingDir, BranchName: branchName} } -func (r *RealGitHub) IsPushable(output io.Writer, repoDir string) (bool, error) { - s, err := execInstance.ExecuteAndCapture(output, repoDir, "gh", "repo", "view", "--json", "viewerPermission") +func (r *RealGitHub) IsPushable(output io.Writer, repo string) (bool, error) { + // The command can be run from any repo + // so we use the current repository. + currentDir, err := os.Getwd() + if err != nil { + return false, err + } + s, err := execInstance.ExecuteAndCapture(output, currentDir, "gh", "repo", "view", repo, "--json", "viewerPermission") if err != nil { return false, err }