scriptability improvements for repo commands

This commit is contained in:
vilmibm 2020-07-17 16:10:49 -05:00
parent 71bebd3f54
commit eb204c0dee
2 changed files with 218 additions and 53 deletions

View file

@ -434,21 +434,36 @@ func repoFork(cmd *cobra.Command, args []string) error {
}
}
if !connectedToTerminal(cmd) {
if (inParent && remotePref == "prompt") || (!inParent && clonePref == "prompt") {
return errors.New("--remote or --clone must be explicitly set when not attached to tty")
}
}
greenCheck := utils.Green("✓")
out := colorableOut(cmd)
s := utils.Spinner(out)
loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...")
s.Suffix = " " + loading
s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading))
utils.StartSpinner(s)
stderr := colorableErr(cmd)
s := utils.Spinner(stderr)
stopSpinner := func() {}
if connectedToTerminal(cmd) {
loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...")
s.Suffix = " " + loading
s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading))
utils.StartSpinner(s)
stopSpinner = func() {
utils.StopSpinner(s)
}
}
forkedRepo, err := api.ForkRepo(apiClient, repoToFork)
if err != nil {
utils.StopSpinner(s)
stopSpinner()
return fmt.Errorf("failed to fork: %w", err)
}
s.Stop()
stopSpinner()
// This is weird. There is not an efficient way to determine via the GitHub API whether or not a
// given user has forked a given repo. We noticed, also, that the create fork API endpoint just
// returns the fork repo data even if it already exists -- with no change in status code or
@ -456,12 +471,19 @@ func repoFork(cmd *cobra.Command, args []string) error {
// we assume the fork already existed and report an error.
createdAgo := Since(forkedRepo.CreatedAt)
if createdAgo > time.Minute {
fmt.Fprintf(out, "%s %s %s\n",
utils.Yellow("!"),
utils.Bold(ghrepo.FullName(forkedRepo)),
"already exists")
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s %s %s\n",
utils.Yellow("!"),
utils.Bold(ghrepo.FullName(forkedRepo)),
"already exists")
} else {
fmt.Fprintf(stderr, "%s already exists", ghrepo.FullName(forkedRepo))
return nil
}
} else {
fmt.Fprintf(out, "%s Created fork %s\n", greenCheck, utils.Bold(ghrepo.FullName(forkedRepo)))
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s Created fork %s\n", greenCheck, utils.Bold(ghrepo.FullName(forkedRepo)))
}
}
if (inParent && remotePref == "false") || (!inParent && clonePref == "false") {
@ -474,7 +496,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
return err
}
if remote, err := remotes.FindByRepo(forkedRepo.RepoOwner(), forkedRepo.RepoName()); err == nil {
fmt.Fprintf(out, "%s Using existing remote %s\n", greenCheck, utils.Bold(remote.Name))
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s Using existing remote %s\n", greenCheck, utils.Bold(remote.Name))
}
return nil
}
@ -499,7 +523,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
fmt.Fprintf(out, "%s Renamed %s remote to %s\n", greenCheck, utils.Bold(remoteName), utils.Bold(renameTarget))
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s Renamed %s remote to %s\n", greenCheck, utils.Bold(remoteName), utils.Bold(renameTarget))
}
}
forkedRepoCloneURL := formatRemoteURL(cmd, forkedRepo)
@ -509,7 +535,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
return fmt.Errorf("failed to add remote: %w", err)
}
fmt.Fprintf(out, "%s Added remote %s\n", greenCheck, utils.Bold(remoteName))
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s Added remote %s\n", greenCheck, utils.Bold(remoteName))
}
}
} else {
cloneDesired := clonePref == "true"
@ -531,7 +559,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
return err
}
fmt.Fprintf(out, "%s Cloned fork\n", greenCheck)
if connectedToTerminal(cmd) {
fmt.Fprintf(stderr, "%s Cloned fork\n", greenCheck)
}
}
}
@ -591,14 +621,31 @@ func repoView(cmd *cobra.Command, args []string) error {
return err
}
fullName := ghrepo.FullName(toView)
openURL := generateRepoURL(toView, "")
if web {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL))
if connectedToTerminal(cmd) {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL))
}
return utils.OpenInBrowser(openURL)
}
fullName := ghrepo.FullName(toView)
if !connectedToTerminal(cmd) {
readme, err := api.RepositoryReadme(apiClient, toView)
if err != nil {
return err
}
out := cmd.OutOrStdout()
fmt.Fprintf(out, "name:\t%s\n", fullName)
fmt.Fprintf(out, "description:\t%s\n", repo.Description)
fmt.Fprintln(out, "--")
fmt.Fprintf(out, readme.Content)
return nil
}
repoTmpl := `
{{.FullName}}
{{.Description}}

View file

@ -17,6 +17,7 @@ import (
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/test"
"github.com/cli/cli/utils"
"github.com/stretchr/testify/assert"
)
func stubSpinner() {
@ -27,6 +28,72 @@ func stubSpinner() {
}
}
func TestRepoFork_nontty_insufficient_flags(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer stubTerminal(false)()
_, err := RunCommand("repo fork")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "--remote or --clone must be explicitly set when not attached to tty", err.Error())
}
func TestRepoFork_in_parent_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(false)()
cs, restore := test.InitCmdStubber()
defer restore()
cs.Stub("") // git remote rename
cs.Stub("") // git remote add
output, err := RunCommand("repo fork --remote")
if err != nil {
t.Fatalf("error running command `repo fork`: %v", err)
}
eq(t, strings.Join(cs.Calls[0].Args, " "), "git remote rename origin upstream")
eq(t, strings.Join(cs.Calls[1].Args, " "), "git remote add -f origin https://github.com/someone/REPO.git")
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
}
func TestRepoFork_outside_parent_nontty(t *testing.T) {
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(false)()
cs, restore := test.InitCmdStubber()
defer restore()
cs.Stub("") // git clone
cs.Stub("") // git remote add
output, err := RunCommand("repo fork --clone OWNER/REPO")
if err != nil {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.String(), "")
eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/REPO.git")
eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git")
assert.Equal(t, output.Stderr(), "")
}
func TestRepoFork_already_forked(t *testing.T) {
stubSpinner()
initContext = func() context.Context {
@ -41,14 +108,15 @@ func TestRepoFork_already_forked(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
output, err := RunCommand("repo fork --remote=false")
if err != nil {
t.Errorf("got unexpected error: %v", err)
}
r := regexp.MustCompile(`someone/REPO already exists`)
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
r := regexp.MustCompile(`someone/REPO.*already exists`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output.Stderr())
return
}
}
@ -68,13 +136,15 @@ func TestRepoFork_reuseRemote(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
output, err := RunCommand("repo fork")
if err != nil {
t.Errorf("got unexpected error: %v", err)
}
if !strings.Contains(output.String(), "Using existing remote origin") {
t.Errorf("output did not match: %q", output)
r := regexp.MustCompile(`Using existing remote.*origin`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match: %q", output.Stderr())
return
}
}
@ -96,16 +166,17 @@ func TestRepoFork_in_parent(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
output, err := RunCommand("repo fork --remote=false")
if err != nil {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
r := regexp.MustCompile(`Created fork someone/REPO`)
if !r.MatchString(output.String()) {
r := regexp.MustCompile(`Created fork.*someone/REPO`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -131,16 +202,17 @@ func TestRepoFork_outside(t *testing.T) {
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
output, err := RunCommand(tt.args)
if err != nil {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
r := regexp.MustCompile(`Created fork someone/REPO`)
if !r.MatchString(output.String()) {
r := regexp.MustCompile(`Created fork.*someone/REPO`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -155,6 +227,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
var seenCmds []*exec.Cmd
defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@ -176,11 +249,11 @@ func TestRepoFork_in_parent_yes(t *testing.T) {
eq(t, strings.Join(cmd.Args, " "), expectedCmds[x])
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
test.ExpectLines(t, output.String(),
"Created fork someone/REPO",
"Added remote origin")
test.ExpectLines(t, output.Stderr(),
"Created fork.*someone/REPO",
"Added remote.*origin")
}
func TestRepoFork_outside_yes(t *testing.T) {
@ -188,6 +261,7 @@ func TestRepoFork_outside_yes(t *testing.T) {
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
cs, restore := test.InitCmdStubber()
defer restore()
@ -200,13 +274,13 @@ func TestRepoFork_outside_yes(t *testing.T) {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/REPO.git")
eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git")
test.ExpectLines(t, output.String(),
"Created fork someone/REPO",
test.ExpectLines(t, output.Stderr(),
"Created fork.*someone/REPO",
"Cloned fork")
}
@ -215,6 +289,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) {
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
cs, restore := test.InitCmdStubber()
defer restore()
@ -234,13 +309,13 @@ func TestRepoFork_outside_survey_yes(t *testing.T) {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/REPO.git")
eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git")
test.ExpectLines(t, output.String(),
"Created fork someone/REPO",
test.ExpectLines(t, output.Stderr(),
"Created fork.*someone/REPO",
"Cloned fork")
}
@ -249,6 +324,7 @@ func TestRepoFork_outside_survey_no(t *testing.T) {
defer stubSince(2 * time.Second)()
http := initFakeHTTP()
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
cmdRun := false
defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@ -268,12 +344,12 @@ func TestRepoFork_outside_survey_no(t *testing.T) {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
eq(t, cmdRun, false)
r := regexp.MustCompile(`Created fork someone/REPO`)
if !r.MatchString(output.String()) {
r := regexp.MustCompile(`Created fork.*someone/REPO`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -286,6 +362,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
var seenCmds []*exec.Cmd
defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@ -314,12 +391,12 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) {
eq(t, strings.Join(cmd.Args, " "), expectedCmds[x])
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
test.ExpectLines(t, output.String(),
"Created fork someone/REPO",
"Renamed origin remote to upstream",
"Added remote origin")
test.ExpectLines(t, output.Stderr(),
"Created fork.*someone/REPO",
"Renamed.*origin.*remote to.*upstream",
"Added remote.*origin")
}
func TestRepoFork_in_parent_survey_no(t *testing.T) {
@ -329,6 +406,7 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) {
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
defer http.StubWithFixture(200, "forkResult.json")()
defer stubTerminal(true)()
cmdRun := false
defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@ -348,12 +426,12 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) {
t.Errorf("error running command `repo fork`: %v", err)
}
eq(t, output.Stderr(), "")
eq(t, output.String(), "")
eq(t, cmdRun, false)
r := regexp.MustCompile(`Created fork someone/REPO`)
if !r.MatchString(output.String()) {
r := regexp.MustCompile(`Created fork.*someone/REPO`)
if !r.MatchString(output.Stderr()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
@ -766,6 +844,8 @@ func TestRepoView_web(t *testing.T) {
})
defer restoreCmd()
defer stubTerminal(true)()
output, err := RunCommand("repo view -w")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
@ -799,6 +879,7 @@ func TestRepoView_web_ownerRepo(t *testing.T) {
return &test.OutputStub{}
})
defer restoreCmd()
defer stubTerminal(true)()
output, err := RunCommand("repo view -w cli/cli")
if err != nil {
@ -833,6 +914,7 @@ func TestRepoView_web_fullURL(t *testing.T) {
})
defer restoreCmd()
defer stubTerminal(true)()
output, err := RunCommand("repo view -w https://github.com/cli/cli")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
@ -865,6 +947,8 @@ func TestRepoView(t *testing.T) {
{ "name": "readme.md",
"content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`))
defer stubTerminal(true)()
output, err := RunCommand("repo view")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
@ -875,7 +959,6 @@ func TestRepoView(t *testing.T) {
"social distancing",
"truly cool readme",
"View this repository on GitHub: https://github.com/OWNER/REPO")
}
func TestRepoView_nonmarkdown_readme(t *testing.T) {
@ -895,6 +978,8 @@ func TestRepoView_nonmarkdown_readme(t *testing.T) {
{ "name": "readme.org",
"content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`))
defer stubTerminal(true)()
output, err := RunCommand("repo view")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
@ -916,6 +1001,8 @@ func TestRepoView_blanks(t *testing.T) {
httpmock.REST("GET", "repos/OWNER/REPO/readme"),
httpmock.StatusStringResponse(404, `{}`))
defer stubTerminal(true)()
output, err := RunCommand("repo view")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
@ -927,3 +1014,34 @@ func TestRepoView_blanks(t *testing.T) {
"This repository does not have a README",
"View this repository on GitHub: https://github.com/OWNER/REPO")
}
func TestRepoView_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": {
"repository": {
"description": "social distancing"
} } }`))
http.Register(
httpmock.REST("GET", "repos/OWNER/REPO/readme"),
httpmock.StringResponse(`
{ "name": "readme.md",
"content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`))
defer stubTerminal(false)()
output, err := RunCommand("repo view")
if err != nil {
t.Errorf("error running command `repo view`: %v", err)
}
test.ExpectLines(t, output.String(),
"OWNER/REPO",
"social distancing",
"# truly cool readme check it out",
)
}