Skip to content

Commit

Permalink
fix(clone): update IsPushable to use full repo name
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sledigabel committed Oct 24, 2024
1 parent 3822206 commit e957b15
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 18 deletions.
2 changes: 1 addition & 1 deletion cmd/clone/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 12 additions & 12 deletions cmd/clone/clone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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"},
})

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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"},
})
Expand Down
4 changes: 2 additions & 2 deletions internal/github/fake_github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
13 changes: 10 additions & 3 deletions internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"encoding/json"
"fmt"
"io"
"os"
"strings"

"github.com/skyscanner/turbolift/internal/executor"
Expand All @@ -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{}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit e957b15

Please sign in to comment.