From 5087ec5c15a549276bf56ef1462db3e9d323caf5 Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 12:00:16 -0500 Subject: [PATCH 01/18] restore pr create with new context/client --- api/queries.go | 71 +++++++++++++++++++++++++++++++++++++++++++++++++ command/pr.go | 1 + command/root.go | 1 + git/git.go | 23 ++++++++++++++++ 4 files changed, 96 insertions(+) diff --git a/api/queries.go b/api/queries.go index e1e078138..0d9fabaf8 100644 --- a/api/queries.go +++ b/api/queries.go @@ -22,6 +22,34 @@ type Repo interface { RepoOwner() string } +func GitHubRepoId(client *Client, ghRepo Repo) (string, error) { + owner := ghRepo.RepoOwner() + repo := ghRepo.RepoName() + + query := ` + query FindRepoID($owner:String!, $name:String!) { + repository(owner:$owner, name:$name) { + id + } + }` + variables := map[string]interface{}{ + "owner": owner, + "name": repo, + } + + result := struct { + Repository struct { + Id string + } + }{} + err := client.GraphQL(query, variables, &result) + if err != nil { + return "", fmt.Errorf("failed to determine GH repo ID: %s", err) + } + + return result.Repository.Id, nil +} + func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername string) (*PullRequestsPayload, error) { type edges struct { Edges []struct { @@ -171,3 +199,46 @@ func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRe return prs, nil } + +func CreatePullRequest(client *Client, ghRepo Repo, title string, body string, draft bool, base string, head string) (string, error) { + repoId, err := GitHubRepoId(client, ghRepo) + if err != nil { + return "", err + } + if repoId == "" { + return "", fmt.Errorf("could not determine GH repo ID") + } + + query := ` + mutation CreatePullRequest($input: CreatePullRequestInput!) { + createPullRequest(input: $input) { + pullRequest { + url + } + } + }` + + variables := map[string]interface{}{ + "input": map[string]interface{}{ + "repositoryId": repoId, + "baseRefName": base, + "headRefName": head, + "title": title, + "body": body, + "draft": draft, + }, + } + + result := struct { + CreatePullRequest struct { + PullRequest PullRequest + } + }{} + + err = client.GraphQL(query, variables, &result) + if err != nil { + return "", err + } + + return result.CreatePullRequest.PullRequest.URL, nil +} diff --git a/command/pr.go b/command/pr.go index f1dfe8fb0..bcd042315 100644 --- a/command/pr.go +++ b/command/pr.go @@ -22,6 +22,7 @@ func init() { Short: "Open a pull request in the browser", RunE: prView, }, + prCreateCmd, ) } diff --git a/command/root.go b/command/root.go index c0b676c86..12696397d 100644 --- a/command/root.go +++ b/command/root.go @@ -58,6 +58,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { opts := []api.ClientOption{ api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", version.Version)), + api.AddHeader("Accept", "application/vnd.github.shadow-cat-preview+json"), } if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, api.VerboseLog(os.Stderr)) diff --git a/git/git.go b/git/git.go index b3b63388e..759f71ebb 100644 --- a/git/git.go +++ b/git/git.go @@ -216,6 +216,29 @@ func LocalBranches() ([]string, error) { return branches, nil } +func UncommittedChangeCount() (int, error) { + statusCmd := exec.Command("git", "status", "--porcelain") + output, err := statusCmd.Output() + if err != nil { + return 0, fmt.Errorf("failed to run git status: %s", err) + } + lines := strings.Split(string(output), "\n") + + count := 0 + + for _, l := range lines { + if l != "" { + count++ + } + } + + return count, nil +} + +var Push = func(remote string, ref string) error { + return Run("push", "--set-upstream", remote, ref) +} + func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") if lines == "" { From dfd567bf7fd7df80692a966a8b518ed2847cd054 Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 12:00:35 -0500 Subject: [PATCH 02/18] blindly committing the state of these files --- go.mod | 1 + go.sum | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/go.mod b/go.mod index b80ee1d9a..2fb24c3ca 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/github/gh-cli go 1.13 require ( + github.com/AlecAivazis/survey/v2 v2.0.4 github.com/gookit/color v1.2.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 diff --git a/go.sum b/go.sum index 1a7c223fc..71ee182e2 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,9 @@ +github.com/AlecAivazis/survey v1.8.7 h1:QIBq36/0wfYpXxdBqDXNAjKHx1bKnRGu/EDnva27k84= +github.com/AlecAivazis/survey/v2 v2.0.4 h1:qzXnJSzXEvmUllWqMBWpZndvT2YfoAUzAMvZUax3L2M= +github.com/AlecAivazis/survey/v2 v2.0.4/go.mod h1:WYBhg6f0y/fNYUuesWQc0PKbJcEliGcYHB9sNT3Bg74= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61PbNXchhh0oikYAH+4Pcfw5LKv21+Jnpr6r6Pc= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk= @@ -12,16 +16,20 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/gookit/color v1.2.0 h1:lHA77Kuyi5JpBnA9ESvwkY+nanLjRZ0mHbWQXRYk2Lk= github.com/gookit/color v1.2.0/go.mod h1:AhIE+pS6D4Ql0SQWbBeXPHw7gY0/sjHoA4s/n1KB7xg= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= +github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= +github.com/kr/pty v1.1.4/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mattn/go-colorable v0.1.2 h1:/bC9yWikZXAL9uJdulbSfyVNIR3n3trXl+v8+1sx8mU= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.9 h1:d5US/mDsogSGW37IV293h//ZFaeajb69h+EHFsv2xGg= github.com/mattn/go-isatty v0.0.9/go.mod h1:YNRxwqDuOph6SZLI9vUUz6OYw3QyUt7WiY2yME+cCiQ= +github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1fWh90gTKwiN4QCGoY9TWyyO4= +github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= @@ -38,14 +46,21 @@ github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.2.1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190530182044-ad28b68e88f1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a h1:aYOabOQFp6Vj6W1F80affTUvO9UxmJRx8K0gsfABByQ= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= From 9efe96575ea18025cc8adff3d6cc8e247695e0ea Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 12:01:54 -0500 Subject: [PATCH 03/18] actually add command --- command/pr_create.go | 207 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 command/pr_create.go diff --git a/command/pr_create.go b/command/pr_create.go new file mode 100644 index 000000000..a3e3581fc --- /dev/null +++ b/command/pr_create.go @@ -0,0 +1,207 @@ +package command + +import ( + "fmt" + "github.com/AlecAivazis/survey/v2" + "github.com/github/gh-cli/api" + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/git" + "github.com/github/gh-cli/utils" + "github.com/spf13/cobra" + "os" + "runtime" +) + +var ( + _draftF bool + _titleF string + _bodyF string + _baseF string +) + +func prCreate(ctx context.Context) error { + ucc, err := git.UncommittedChangeCount() + if err != nil { + return fmt.Errorf( + "could not determine state of working directory: %v", err) + } + if ucc > 0 { + noun := "change" + if ucc > 1 { + noun = noun + "s" + } + + fmt.Printf("%s %d uncommitted %s %s", utils.Red("!!!"), + ucc, noun, utils.Red("!!!")) + } + + head, err := ctx.Branch() + if err != nil { + return fmt.Errorf("could not determine current branch: %s", err) + } + + remote, err := guessRemote(ctx) + if err != nil { + return err + } + + if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { + return fmt.Errorf("was not able to push to remote '%s': %s", remote, err) + } + + interactive := _titleF == "" || _bodyF == "" + + inProgress := struct { + Body string + Title string + }{} + + if interactive { + confirmed := false + editor := determineEditor() + + for !confirmed { + titleQuestion := &survey.Question{ + Name: "title", + Prompt: &survey.Input{ + Message: "PR Title", + Default: inProgress.Title, + }, + } + bodyQuestion := &survey.Question{ + Name: "body", + Prompt: &survey.Editor{ + Message: fmt.Sprintf("PR Body (%s)", editor), + FileName: "*.md", + Default: inProgress.Body, + AppendDefault: true, + Editor: editor, + }, + } + + qs := []*survey.Question{} + if _titleF == "" { + qs = append(qs, titleQuestion) + } + if _bodyF == "" { + qs = append(qs, bodyQuestion) + } + + err := survey.Ask(qs, &inProgress) + if err != nil { + return fmt.Errorf("could not prompt: %s", err) + } + confirmAnswers := struct { + Confirmation string + }{} + confirmQs := []*survey.Question{ + { + Name: "confirmation", + Prompt: &survey.Select{ + Message: "Submit?", + Options: []string{ + "Yes", + "Edit", + "Cancel", + }, + }, + }, + } + + err = survey.Ask(confirmQs, &confirmAnswers) + if err != nil { + return fmt.Errorf("could not prompt: %s", err) + } + + switch confirmAnswers.Confirmation { + case "Yes": + confirmed = true + case "Edit": + continue + case "Cancel": + fmt.Println(utils.Red("Discarding PR.")) + return nil + } + } + } + + title := _titleF + if title == "" { + title = inProgress.Title + } + body := _bodyF + if body == "" { + body = inProgress.Body + } + base := _baseF + if base == "" { + base = "master" + } + + client, err := apiClientForContext(ctx) + if err != nil { + return fmt.Errorf("could not initialize api client: %s", err) + } + + repo, err := ctx.BaseRepo() + if err != nil { + return fmt.Errorf("could not determine GitHub repo: %s", err) + } + + payload, err := api.CreatePullRequest(client, repo, title, body, _draftF, base, head) + if err != nil { + return fmt.Errorf("failed to create PR: %s", err) + } + + fmt.Println(payload) + + return nil +} + +func guessRemote(ctx context.Context) (string, error) { + remotes, err := ctx.Remotes() + if err != nil { + return "", fmt.Errorf("could not determine suitable remote: %s", err) + } + + remote, err := remotes.FindByName("origin", "github") + if err != nil { + return "", fmt.Errorf("could not determine suitable remote: %s", err) + } + + return remote.Name, nil +} + +func determineEditor() string { + if runtime.GOOS == "windows" { + return "notepad" + } + if v := os.Getenv("VISUAL"); v != "" { + return v + } + if e := os.Getenv("EDITOR"); e != "" { + return e + } + return "nano" +} + +var prCreateCmd = &cobra.Command{ + Use: "create", + Short: "Create a pull request", + RunE: func(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + return prCreate(ctx) + }, +} + +func init() { + prCreateCmd.Flags().BoolVarP(&_draftF, "draft", "d", false, + "Mark PR as a draft") + prCreateCmd.Flags().StringVarP(&_titleF, "title", "t", "", + "Supply a title. Will prompt for one otherwise.") + prCreateCmd.Flags().StringVarP(&_bodyF, "body", "b", "", + "Supply a body. Will prompt for one otherwise.") + prCreateCmd.Flags().StringVarP(&_baseF, "base", "T", "", + "The branch into which you want your code merged") + +} From f6afe1b576b02a5c946378952febab8a6167d991 Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 16:34:23 -0500 Subject: [PATCH 04/18] use testing hack to mock git call --- git/git.go | 10 +++++++- git/git_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 git/git_test.go diff --git a/git/git.go b/git/git.go index 759f71ebb..02366c75d 100644 --- a/git/git.go +++ b/git/git.go @@ -216,9 +216,17 @@ func LocalBranches() ([]string, error) { return branches, nil } +var GitCommand = func(args ...string) *exec.Cmd { + return exec.Command("git", args...) +} + func UncommittedChangeCount() (int, error) { - statusCmd := exec.Command("git", "status", "--porcelain") + statusCmd := GitCommand("status", "--porcelain") + fmt.Println("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!") + fmt.Printf("%+v", statusCmd) output, err := statusCmd.Output() + fmt.Println(string(output)) + fmt.Println("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!") if err != nil { return 0, fmt.Errorf("failed to run git status: %s", err) } diff --git a/git/git_test.go b/git/git_test.go new file mode 100644 index 000000000..f1b9bf661 --- /dev/null +++ b/git/git_test.go @@ -0,0 +1,62 @@ +package git + +import ( + "fmt" + "os" + "os/exec" + "testing" +) + +func StubbedGit(args ...string) *exec.Cmd { + cs := []string{"-test.run=TestHelperProcess", "--", "git"} + cs = append(cs, args...) + env := []string{ + "GO_WANT_HELPER_PROCESS=1", + } + + cmd := exec.Command(os.Args[0], cs...) + cmd.Env = append(env, os.Environ()...) + fmt.Printf("%+v", cmd.Env) + return cmd +} + +func TestHelperProcess(*testing.T) { + if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { + return + } + defer os.Exit(0) + args := os.Args + for len(args) > 0 { + if args[0] == "--" { + args = args[1:] + break + } + args = args[1:] + } + cmd, args := args[0], args[1:] + switch cmd { + case "git": + fmt.Println("fart") + fmt.Println("town") + } + +} + +func Test_UncommittedChangeCount(t *testing.T) { + origGitCommand := GitCommand + defer func() { + GitCommand = origGitCommand + }() + + GitCommand = StubbedGit + + ucc, err := UncommittedChangeCount() + if err != nil { + t.Fatalf("failed to run UncommittedChangeCount: %s", err) + } + + if ucc != 10 { + t.Errorf("got unexpected ucc value: %d", ucc) + } + +} From fdbf85e9abf50933a1b4a6007ed914d6d9398cc0 Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 16:45:11 -0500 Subject: [PATCH 05/18] sigh worthy mechanism for selecting arbitrary git outputs --- git/git.go | 4 ---- git/git_test.go | 33 +++++++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/git/git.go b/git/git.go index 02366c75d..dcfe56190 100644 --- a/git/git.go +++ b/git/git.go @@ -222,11 +222,7 @@ var GitCommand = func(args ...string) *exec.Cmd { func UncommittedChangeCount() (int, error) { statusCmd := GitCommand("status", "--porcelain") - fmt.Println("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!") - fmt.Printf("%+v", statusCmd) output, err := statusCmd.Output() - fmt.Println(string(output)) - fmt.Println("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!") if err != nil { return 0, fmt.Errorf("failed to run git status: %s", err) } diff --git a/git/git_test.go b/git/git_test.go index f1b9bf661..83fbeddf9 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -7,6 +7,16 @@ import ( "testing" ) +var _cases map[string]string + +func init() { + _cases = map[string]string{ + "foobar": `fart + bar + town`, + } +} + func StubbedGit(args ...string) *exec.Cmd { cs := []string{"-test.run=TestHelperProcess", "--", "git"} cs = append(cs, args...) @@ -33,11 +43,22 @@ func TestHelperProcess(*testing.T) { } args = args[1:] } - cmd, args := args[0], args[1:] - switch cmd { - case "git": - fmt.Println("fart") - fmt.Println("town") + c, args := args[0], args[1:] + fmt.Println(_cases[c]) +} + +func StubGit(c string) func(...string) *exec.Cmd { + return func(args ...string) *exec.Cmd { + cs := []string{"-test.run=TestHelperProcess", "--", c} + cs = append(cs, args...) + env := []string{ + "GO_WANT_HELPER_PROCESS=1", + } + + cmd := exec.Command(os.Args[0], cs...) + cmd.Env = append(env, os.Environ()...) + fmt.Printf("%+v", cmd.Env) + return cmd } } @@ -48,7 +69,7 @@ func Test_UncommittedChangeCount(t *testing.T) { GitCommand = origGitCommand }() - GitCommand = StubbedGit + GitCommand = StubGit("foobar") ucc, err := UncommittedChangeCount() if err != nil { From ab115efd893c07408aa41295adfc974f8d50db46 Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 17:05:41 -0500 Subject: [PATCH 06/18] add cases to test for --- git/git_test.go | 53 +++++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/git/git_test.go b/git/git_test.go index 83fbeddf9..19d1c3f90 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -7,29 +7,19 @@ import ( "testing" ) -var _cases map[string]string +var _outputs map[string]string func init() { - _cases = map[string]string{ - "foobar": `fart - bar - town`, + _outputs = map[string]string{ + "no changes": "", + "one change": ` M poem.txt +`, + "untracked file": ` M poem.txt +?? new.txt +`, } } -func StubbedGit(args ...string) *exec.Cmd { - cs := []string{"-test.run=TestHelperProcess", "--", "git"} - cs = append(cs, args...) - env := []string{ - "GO_WANT_HELPER_PROCESS=1", - } - - cmd := exec.Command(os.Args[0], cs...) - cmd.Env = append(env, os.Environ()...) - fmt.Printf("%+v", cmd.Env) - return cmd -} - func TestHelperProcess(*testing.T) { if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { return @@ -43,13 +33,12 @@ func TestHelperProcess(*testing.T) { } args = args[1:] } - c, args := args[0], args[1:] - fmt.Println(_cases[c]) + fmt.Println(_outputs[args[0]]) } -func StubGit(c string) func(...string) *exec.Cmd { +func StubGit(desiredOutput string) func(...string) *exec.Cmd { return func(args ...string) *exec.Cmd { - cs := []string{"-test.run=TestHelperProcess", "--", c} + cs := []string{"-test.run=TestHelperProcess", "--", desiredOutput} cs = append(cs, args...) env := []string{ "GO_WANT_HELPER_PROCESS=1", @@ -57,7 +46,6 @@ func StubGit(c string) func(...string) *exec.Cmd { cmd := exec.Command(os.Args[0], cs...) cmd.Env = append(env, os.Environ()...) - fmt.Printf("%+v", cmd.Env) return cmd } @@ -69,15 +57,20 @@ func Test_UncommittedChangeCount(t *testing.T) { GitCommand = origGitCommand }() - GitCommand = StubGit("foobar") + // TODO handle git status exiting poorly - ucc, err := UncommittedChangeCount() - if err != nil { - t.Fatalf("failed to run UncommittedChangeCount: %s", err) + cases := map[string]int{ + "no changes": 0, + "one change": 1, + "untracked file": 2, } - if ucc != 10 { - t.Errorf("got unexpected ucc value: %d", ucc) - } + for k, v := range cases { + GitCommand = StubGit(k) + ucc, _ := UncommittedChangeCount() + if ucc != v { + t.Errorf("got unexpected ucc value: %d for case %s", ucc, k) + } + } } From ee0fe61b041cf0a37025e7b8e5bb059ba7bf5d6b Mon Sep 17 00:00:00 2001 From: nate smith Date: Wed, 30 Oct 2019 17:46:22 -0500 Subject: [PATCH 07/18] test nonzero exit code --- git/git_test.go | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/git/git_test.go b/git/git_test.go index 19d1c3f90..5402d51b4 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -7,16 +7,22 @@ import ( "testing" ) -var _outputs map[string]string +type outputSpec struct { + Stdout string + ExitCode int +} + +var _outputs map[string]outputSpec func init() { - _outputs = map[string]string{ - "no changes": "", - "one change": ` M poem.txt -`, - "untracked file": ` M poem.txt + _outputs = map[string]outputSpec{ + "no changes": outputSpec{"", 0}, + "one change": outputSpec{` M poem.txt +`, 0}, + "untracked file": outputSpec{` M poem.txt ?? new.txt -`, +`, 0}, + "boom": outputSpec{"", 1}, } } @@ -24,7 +30,6 @@ func TestHelperProcess(*testing.T) { if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { return } - defer os.Exit(0) args := os.Args for len(args) > 0 { if args[0] == "--" { @@ -33,7 +38,9 @@ func TestHelperProcess(*testing.T) { } args = args[1:] } - fmt.Println(_outputs[args[0]]) + output := _outputs[args[0]] + defer os.Exit(output.ExitCode) + fmt.Println(output.Stdout) } func StubGit(desiredOutput string) func(...string) *exec.Cmd { @@ -57,8 +64,6 @@ func Test_UncommittedChangeCount(t *testing.T) { GitCommand = origGitCommand }() - // TODO handle git status exiting poorly - cases := map[string]int{ "no changes": 0, "one change": 1, @@ -73,4 +78,10 @@ func Test_UncommittedChangeCount(t *testing.T) { t.Errorf("got unexpected ucc value: %d for case %s", ucc, k) } } + + GitCommand = StubGit("boom") + _, err := UncommittedChangeCount() + if err.Error() != "failed to run git status: exit status 1" { + t.Errorf("got unexpected error message: %s", err) + } } From 7555aa9be33ea9e1adbc44afdeaba2ac7455f526 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 31 Oct 2019 11:10:57 -0500 Subject: [PATCH 08/18] first pass at generalizing process stubbing --- git/git_test.go | 61 ++++++++++++------------------------------------- test/helpers.go | 37 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 47 deletions(-) diff --git a/git/git_test.go b/git/git_test.go index 5402d51b4..ad60ef256 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -2,62 +2,29 @@ package git import ( "fmt" + "github.com/github/gh-cli/test" "os" - "os/exec" "testing" ) -type outputSpec struct { - Stdout string - ExitCode int -} - -var _outputs map[string]outputSpec - -func init() { - _outputs = map[string]outputSpec{ - "no changes": outputSpec{"", 0}, - "one change": outputSpec{` M poem.txt -`, 0}, - "untracked file": outputSpec{` M poem.txt -?? new.txt -`, 0}, - "boom": outputSpec{"", 1}, - } -} - -func TestHelperProcess(*testing.T) { - if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { +func TestGitStatusHelperProcess(*testing.T) { + if test.SkipTestHelperProcess() { return } - args := os.Args - for len(args) > 0 { - if args[0] == "--" { - args = args[1:] - break - } - args = args[1:] + outputs := map[string]test.ExecStub{ + "no changes": test.ExecStub{"", 0}, + "one change": test.ExecStub{` M poem.txt +`, 0}, + "untracked file": test.ExecStub{` M poem.txt +?? new.txt +`, 0}, + "boom": test.ExecStub{"", 1}, } - output := _outputs[args[0]] + output := test.GetExecStub(outputs) defer os.Exit(output.ExitCode) fmt.Println(output.Stdout) } -func StubGit(desiredOutput string) func(...string) *exec.Cmd { - return func(args ...string) *exec.Cmd { - cs := []string{"-test.run=TestHelperProcess", "--", desiredOutput} - cs = append(cs, args...) - env := []string{ - "GO_WANT_HELPER_PROCESS=1", - } - - cmd := exec.Command(os.Args[0], cs...) - cmd.Env = append(env, os.Environ()...) - return cmd - } - -} - func Test_UncommittedChangeCount(t *testing.T) { origGitCommand := GitCommand defer func() { @@ -71,7 +38,7 @@ func Test_UncommittedChangeCount(t *testing.T) { } for k, v := range cases { - GitCommand = StubGit(k) + GitCommand = test.StubExecCommand("TestGitStatusHelperProcess", k) ucc, _ := UncommittedChangeCount() if ucc != v { @@ -79,7 +46,7 @@ func Test_UncommittedChangeCount(t *testing.T) { } } - GitCommand = StubGit("boom") + GitCommand = test.StubExecCommand("TestGitStatusHelperProcess", "boom") _, err := UncommittedChangeCount() if err.Error() != "failed to run git status: exit status 1" { t.Errorf("got unexpected error message: %s", err) diff --git a/test/helpers.go b/test/helpers.go index d9276565c..e2f059175 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -11,6 +11,43 @@ import ( "github.com/spf13/cobra" ) +type ExecStub struct { + Stdout string + ExitCode int +} + +func GetExecStub(outputs map[string]ExecStub) ExecStub { + args := os.Args + for len(args) > 0 { + if args[0] == "--" { + args = args[1:] + break + } + args = args[1:] + } + return outputs[args[0]] +} + +func SkipTestHelperProcess() bool { + return os.Getenv("GO_WANT_HELPER_PROCESS") != "1" +} + +func StubExecCommand(testHelper string, desiredOutput string) func(...string) *exec.Cmd { + return func(args ...string) *exec.Cmd { + cs := []string{ + fmt.Sprintf("-test.run=%s", testHelper), + "--", desiredOutput} + cs = append(cs, args...) + env := []string{ + "GO_WANT_HELPER_PROCESS=1", + } + + cmd := exec.Command(os.Args[0], cs...) + cmd.Env = append(env, os.Environ()...) + return cmd + } +} + type TempGitRepo struct { Remote string TearDown func() From c12bdc2731f21555e595cc543ee80b0e6eb37417 Mon Sep 17 00:00:00 2001 From: nate smith Date: Fri, 1 Nov 2019 17:19:01 -0500 Subject: [PATCH 09/18] WIP resuming pr create test work --- command/pr_create_test.go | 60 +++++++++++++++++++++++++++++++++++++ git/git.go | 5 ++++ test/fixtures/createPr.json | 9 ++++++ test/fixtures/repoId.json | 7 +++++ test/helpers.go | 7 ++++- 5 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 command/pr_create_test.go create mode 100644 test/fixtures/createPr.json create mode 100644 test/fixtures/repoId.json diff --git a/command/pr_create_test.go b/command/pr_create_test.go new file mode 100644 index 000000000..8adec0a22 --- /dev/null +++ b/command/pr_create_test.go @@ -0,0 +1,60 @@ +package command + +import ( + //"regexp" + "testing" + + //"github.com/github/gh-cli/context" + "github.com/github/gh-cli/test" + //"github.com/github/gh-cli/utils" +) + +func TestPrCreateHelperProcess(*testing.T) { + if test.SkipTestHelperProcess() { + return + } + + statusOutputs := map[string]string{ + "clean": "", + "dirty": " M git/git.go", + } + + args := GetTestHelperProcessArgs() + switch args[1] { + case "status": + fmt.Println(statusOutputs(args[0])) + case "push": + fmt.Println() + } + + defer os.Exit(0) +} + +func TestReportsUncommittedChanges(t *testing.T) { + repoIdFix, _ := os.Open("test/fixtures/repoId.json") + defer repoIdFix.Close() + createPrFix, _ := os.Open("test/fixtures/createPr.json") + defer createPrFix.Close() + + http := initFakeHTTP() + http.StubResponse(200, repoIdFix) + http.StubResponse(200, createPrFix) + + origGitCommand := git.GitCommand + defer func() { + git.GitCommand = origGitCommand + }() + + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "dirty") + + // init blank command and just run prCreate? Or try to use RunCommand? + + // output, err := test.RunCommand(RootCmd, "pr create -tfoo -bbar") + // if err != nil { + // t.Errorf("error running command `pr create`: %v", err) + // } + + // if len(output) == 0 { + // panic("lol") + // } +} diff --git a/git/git.go b/git/git.go index dcfe56190..6c0c31528 100644 --- a/git/git.go +++ b/git/git.go @@ -239,6 +239,11 @@ func UncommittedChangeCount() (int, error) { return count, nil } +func Push(remote string, ref string) error { + cmd := GitCommand("push", "--set-upstream", remote, ref) + return cmd.Run() +} + var Push = func(remote string, ref string) error { return Run("push", "--set-upstream", remote, ref) } diff --git a/test/fixtures/createPr.json b/test/fixtures/createPr.json new file mode 100644 index 000000000..47be76948 --- /dev/null +++ b/test/fixtures/createPr.json @@ -0,0 +1,9 @@ +{ + "data": { + "createPullRequest": { + "pullRequest": { + "url": "https://github.com/vilmibm/testing/pull/14" + } + } + } +} diff --git a/test/fixtures/repoId.json b/test/fixtures/repoId.json new file mode 100644 index 000000000..965b39f3d --- /dev/null +++ b/test/fixtures/repoId.json @@ -0,0 +1,7 @@ +{ + "data": { + "repository": { + "id": "a repo id" + } + } +} diff --git a/test/helpers.go b/test/helpers.go index e2f059175..bbc493ae3 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -16,7 +16,7 @@ type ExecStub struct { ExitCode int } -func GetExecStub(outputs map[string]ExecStub) ExecStub { +func GetTestHelperProcessArgs() []string { args := os.Args for len(args) > 0 { if args[0] == "--" { @@ -25,6 +25,11 @@ func GetExecStub(outputs map[string]ExecStub) ExecStub { } args = args[1:] } + return args +} + +func GetExecStub(outputs map[string]ExecStub) ExecStub { + args := GetTestHelperProcessArgs() return outputs[args[0]] } From 18f104ad3825105c12bc110d5fc464b5b055114a Mon Sep 17 00:00:00 2001 From: nate smith Date: Sat, 2 Nov 2019 12:53:33 -0500 Subject: [PATCH 10/18] oops --- git/git.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/git/git.go b/git/git.go index 6c0c31528..c2b392490 100644 --- a/git/git.go +++ b/git/git.go @@ -244,10 +244,6 @@ func Push(remote string, ref string) error { return cmd.Run() } -var Push = func(remote string, ref string) error { - return Run("push", "--set-upstream", remote, ref) -} - func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") if lines == "" { From df3e3bb4af29f8ef6871ec8bad6cf4501594b36c Mon Sep 17 00:00:00 2001 From: nate smith Date: Sat, 2 Nov 2019 12:55:17 -0500 Subject: [PATCH 11/18] minor fix --- command/pr_create_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 8adec0a22..945acb058 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -2,9 +2,12 @@ package command import ( //"regexp" + "fmt" + "os" "testing" //"github.com/github/gh-cli/context" + "github.com/github/gh-cli/git" "github.com/github/gh-cli/test" //"github.com/github/gh-cli/utils" ) @@ -19,10 +22,10 @@ func TestPrCreateHelperProcess(*testing.T) { "dirty": " M git/git.go", } - args := GetTestHelperProcessArgs() + args := test.GetTestHelperProcessArgs() switch args[1] { case "status": - fmt.Println(statusOutputs(args[0])) + fmt.Println(statusOutputs[args[0]]) case "push": fmt.Println() } From ef4cadd8c98aaddfb22495424dda7b4dc7501a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 11 Nov 2019 14:09:24 +0100 Subject: [PATCH 12/18] Pass arbitrary params to CreatePullRequest --- api/queries.go | 28 ++++++++++++---------------- command/pr_create.go | 13 ++++++++++--- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/api/queries.go b/api/queries.go index 5a8dfec9e..335bb6f20 100644 --- a/api/queries.go +++ b/api/queries.go @@ -277,13 +277,10 @@ func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRe return prs, nil } -func CreatePullRequest(client *Client, ghRepo Repo, title string, body string, draft bool, base string, head string) (string, error) { - repoId, err := GitHubRepoId(client, ghRepo) +func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{}) (*PullRequest, error) { + repoID, err := GitHubRepoId(client, ghRepo) if err != nil { - return "", err - } - if repoId == "" { - return "", fmt.Errorf("could not determine GH repo ID") + return nil, err } query := ` @@ -295,15 +292,14 @@ func CreatePullRequest(client *Client, ghRepo Repo, title string, body string, d } }` + inputParams := map[string]interface{}{ + "repositoryId": repoID, + } + for key, val := range params { + inputParams[key] = val + } variables := map[string]interface{}{ - "input": map[string]interface{}{ - "repositoryId": repoId, - "baseRefName": base, - "headRefName": head, - "title": title, - "body": body, - "draft": draft, - }, + "input": inputParams, } result := struct { @@ -314,10 +310,10 @@ func CreatePullRequest(client *Client, ghRepo Repo, title string, body string, d err = client.GraphQL(query, variables, &result) if err != nil { - return "", err + return nil, err } - return result.CreatePullRequest.PullRequest.URL, nil + return &result.CreatePullRequest.PullRequest, nil } func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([]PullRequest, error) { diff --git a/command/pr_create.go b/command/pr_create.go index a3e3581fc..c52571123 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -148,13 +148,20 @@ func prCreate(ctx context.Context) error { return fmt.Errorf("could not determine GitHub repo: %s", err) } - payload, err := api.CreatePullRequest(client, repo, title, body, _draftF, base, head) + params := map[string]interface{}{ + "title": title, + "body": body, + "draft": _draftF, + "baseRefName": base, + "headRefName": head, + } + + pr, err := api.CreatePullRequest(client, repo, params) if err != nil { return fmt.Errorf("failed to create PR: %s", err) } - fmt.Println(payload) - + fmt.Fprintln(cmd.OutOrStdout(), pr.URL) return nil } From 65054fdc6e5bc6867102660472e72679cc15686a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 11 Nov 2019 15:01:41 +0100 Subject: [PATCH 13/18] Complete `pr create` test --- command/pr_create.go | 28 ++++++++-------- command/pr_create_test.go | 69 +++++++++++++++++++++++++++++---------- context/blank_context.go | 35 +++++++++++++++++--- 3 files changed, 95 insertions(+), 37 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index c52571123..a448045f2 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -2,14 +2,14 @@ package command import ( "fmt" + "os" + "runtime" + "github.com/AlecAivazis/survey/v2" "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" - "github.com/github/gh-cli/utils" "github.com/spf13/cobra" - "os" - "runtime" ) var ( @@ -19,20 +19,21 @@ var ( _baseF string ) -func prCreate(ctx context.Context) error { +func prCreate(cmd *cobra.Command, _ []string) error { + ctx := contextForCommand(cmd) + ucc, err := git.UncommittedChangeCount() if err != nil { - return fmt.Errorf( - "could not determine state of working directory: %v", err) + return err } if ucc > 0 { noun := "change" if ucc > 1 { + // TODO: use pluralize helper noun = noun + "s" } - fmt.Printf("%s %d uncommitted %s %s", utils.Red("!!!"), - ucc, noun, utils.Red("!!!")) + cmd.Printf("Warning: %d uncommitted %s\n", ucc, noun) } head, err := ctx.Branch() @@ -119,7 +120,7 @@ func prCreate(ctx context.Context) error { case "Edit": continue case "Cancel": - fmt.Println(utils.Red("Discarding PR.")) + cmd.Println("Discarding pull request") return nil } } @@ -171,7 +172,9 @@ func guessRemote(ctx context.Context) (string, error) { return "", fmt.Errorf("could not determine suitable remote: %s", err) } - remote, err := remotes.FindByName("origin", "github") + // TODO: consolidate logic with fsContext.BaseRepo + // TODO: check if the GH repo that the remote points to is writeable + remote, err := remotes.FindByName("upstream", "github", "origin", "*") if err != nil { return "", fmt.Errorf("could not determine suitable remote: %s", err) } @@ -195,10 +198,7 @@ func determineEditor() string { var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", - RunE: func(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - return prCreate(ctx) - }, + RunE: prCreate, } func init() { diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 945acb058..e25e3c582 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -1,15 +1,16 @@ package command import ( - //"regexp" + "bytes" + "encoding/json" "fmt" + "io/ioutil" "os" "testing" - //"github.com/github/gh-cli/context" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" "github.com/github/gh-cli/test" - //"github.com/github/gh-cli/utils" ) func TestPrCreateHelperProcess(*testing.T) { @@ -34,14 +35,26 @@ func TestPrCreateHelperProcess(*testing.T) { } func TestReportsUncommittedChanges(t *testing.T) { - repoIdFix, _ := os.Open("test/fixtures/repoId.json") - defer repoIdFix.Close() - createPrFix, _ := os.Open("test/fixtures/createPr.json") - defer createPrFix.Close() - + ctx := context.NewBlank() + ctx.SetBranch("feature") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } http := initFakeHTTP() - http.StubResponse(200, repoIdFix) - http.StubResponse(200, createPrFix) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `)) origGitCommand := git.GitCommand defer func() { @@ -50,14 +63,34 @@ func TestReportsUncommittedChanges(t *testing.T) { git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "dirty") - // init blank command and just run prCreate? Or try to use RunCommand? + out := bytes.Buffer{} + prCreateCmd.SetOut(&out) - // output, err := test.RunCommand(RootCmd, "pr create -tfoo -bbar") - // if err != nil { - // t.Errorf("error running command `pr create`: %v", err) - // } + RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) + _, err := prCreateCmd.ExecuteC() + eq(t, err, nil) - // if len(output) == 0 { - // panic("lol") - // } + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + reqBody := struct { + Variables struct { + Input struct { + RepositoryID string + Title string + Body string + BaseRefName string + HeadRefName string + } + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") + eq(t, reqBody.Variables.Input.Title, "mytitle") + eq(t, reqBody.Variables.Input.Body, "mybody") + eq(t, reqBody.Variables.Input.BaseRefName, "master") + eq(t, reqBody.Variables.Input.HeadRefName, "feature") + + eq(t, out.String(), `Warning: 1 uncommitted change +https://github.com/OWNER/REPO/pull/12 +`) } diff --git a/context/blank_context.go b/context/blank_context.go index d5ad2cafe..733ef38a8 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -3,10 +3,12 @@ package context import ( "fmt" "strings" + + "github.com/github/gh-cli/git" ) // NewBlank initializes a blank Context suitable for testing -func NewBlank() Context { +func NewBlank() *blankContext { return &blankContext{} } @@ -16,6 +18,7 @@ type blankContext struct { authLogin string branch string baseRepo GitHubRepository + remotes Remotes } type ghRepo struct { @@ -54,14 +57,36 @@ func (c *blankContext) SetBranch(b string) { } func (c *blankContext) Remotes() (Remotes, error) { - return Remotes{}, nil + if c.remotes == nil { + return nil, fmt.Errorf("remotes were not initialized") + } + return c.remotes, nil +} + +func (c *blankContext) SetRemotes(stubs map[string]string) { + c.remotes = Remotes{} + for remoteName, repo := range stubs { + ownerWithName := strings.SplitN(repo, "/", 2) + c.remotes = append(c.remotes, &Remote{ + Remote: &git.Remote{Name: remoteName}, + Owner: ownerWithName[0], + Repo: ownerWithName[1], + }) + } } func (c *blankContext) BaseRepo() (GitHubRepository, error) { - if c.baseRepo == nil { - return nil, fmt.Errorf("base repo was not initialized") + if c.baseRepo != nil { + return c.baseRepo, nil } - return c.baseRepo, nil + remotes, err := c.Remotes() + if err != nil { + return nil, err + } + if len(remotes) < 1 { + return nil, fmt.Errorf("remotes are empty") + } + return remotes[0], nil } func (c *blankContext) SetBaseRepo(nwo string) { From 6aa0c071d6fd2e60e2467bfb273286b402f73896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 11 Nov 2019 17:04:26 +0100 Subject: [PATCH 14/18] Simplify tests that use StubExecCommand --- command/pr_create_test.go | 21 ++++++++++++--------- git/git.go | 4 ++-- git/git_test.go | 29 ++++++++++++++++------------- test/helpers.go | 10 ---------- 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index e25e3c582..7453a0e1f 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -18,20 +18,23 @@ func TestPrCreateHelperProcess(*testing.T) { return } - statusOutputs := map[string]string{ - "clean": "", - "dirty": " M git/git.go", - } - args := test.GetTestHelperProcessArgs() switch args[1] { case "status": - fmt.Println(statusOutputs[args[0]]) + switch args[0] { + case "clean": + case "dirty": + fmt.Println(" M git/git.go") + default: + fmt.Fprintf(os.Stderr, "unknown scenario: %q", args[0]) + os.Exit(1) + } case "push": - fmt.Println() + default: + fmt.Fprintf(os.Stderr, "unknown command: %q", args[1]) + os.Exit(1) } - - defer os.Exit(0) + os.Exit(0) } func TestReportsUncommittedChanges(t *testing.T) { diff --git a/git/git.go b/git/git.go index 24297a7a8..411863b31 100644 --- a/git/git.go +++ b/git/git.go @@ -212,9 +212,9 @@ var GitCommand = func(args ...string) *exec.Cmd { func UncommittedChangeCount() (int, error) { statusCmd := GitCommand("status", "--porcelain") - output, err := statusCmd.Output() + output, err := utils.PrepareCmd(statusCmd).Output() if err != nil { - return 0, fmt.Errorf("failed to run git status: %s", err) + return 0, err } lines := strings.Split(string(output), "\n") diff --git a/git/git_test.go b/git/git_test.go index ad60ef256..dc1446c33 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -2,27 +2,30 @@ package git import ( "fmt" - "github.com/github/gh-cli/test" "os" + "strings" "testing" + + "github.com/github/gh-cli/test" ) func TestGitStatusHelperProcess(*testing.T) { if test.SkipTestHelperProcess() { return } - outputs := map[string]test.ExecStub{ - "no changes": test.ExecStub{"", 0}, - "one change": test.ExecStub{` M poem.txt -`, 0}, - "untracked file": test.ExecStub{` M poem.txt -?? new.txt -`, 0}, - "boom": test.ExecStub{"", 1}, + + args := test.GetTestHelperProcessArgs() + switch args[0] { + case "no changes": + case "one change": + fmt.Println(" M poem.txt") + case "untracked file": + fmt.Println(" M poem.txt") + fmt.Println("?? new.txt") + case "boom": + os.Exit(1) } - output := test.GetExecStub(outputs) - defer os.Exit(output.ExitCode) - fmt.Println(output.Stdout) + os.Exit(0) } func Test_UncommittedChangeCount(t *testing.T) { @@ -48,7 +51,7 @@ func Test_UncommittedChangeCount(t *testing.T) { GitCommand = test.StubExecCommand("TestGitStatusHelperProcess", "boom") _, err := UncommittedChangeCount() - if err.Error() != "failed to run git status: exit status 1" { + if !strings.HasSuffix(err.Error(), "git.test: exit status 1") { t.Errorf("got unexpected error message: %s", err) } } diff --git a/test/helpers.go b/test/helpers.go index 8c2b5f416..dca930be5 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -11,11 +11,6 @@ import ( "github.com/spf13/cobra" ) -type ExecStub struct { - Stdout string - ExitCode int -} - func GetTestHelperProcessArgs() []string { args := os.Args for len(args) > 0 { @@ -28,11 +23,6 @@ func GetTestHelperProcessArgs() []string { return args } -func GetExecStub(outputs map[string]ExecStub) ExecStub { - args := GetTestHelperProcessArgs() - return outputs[args[0]] -} - func SkipTestHelperProcess() bool { return os.Getenv("GO_WANT_HELPER_PROCESS") != "1" } From e3e8647760fbf6489a786d03f3ad22a0560aec8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 13 Nov 2019 17:57:01 +0100 Subject: [PATCH 15/18] Eliminate package-level flags --- command/pr_create.go | 46 ++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index a448045f2..b9d71df8c 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -12,13 +12,6 @@ import ( "github.com/spf13/cobra" ) -var ( - _draftF bool - _titleF string - _bodyF string - _baseF string -) - func prCreate(cmd *cobra.Command, _ []string) error { ctx := contextForCommand(cmd) @@ -50,7 +43,16 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("was not able to push to remote '%s': %s", remote, err) } - interactive := _titleF == "" || _bodyF == "" + title, err := cmd.Flags().GetString("title") + if err != nil { + return err + } + body, err := cmd.Flags().GetString("body") + if err != nil { + return err + } + + interactive := title == "" || body == "" inProgress := struct { Body string @@ -81,10 +83,10 @@ func prCreate(cmd *cobra.Command, _ []string) error { } qs := []*survey.Question{} - if _titleF == "" { + if title == "" { qs = append(qs, titleQuestion) } - if _bodyF == "" { + if body == "" { qs = append(qs, bodyQuestion) } @@ -126,16 +128,18 @@ func prCreate(cmd *cobra.Command, _ []string) error { } } - title := _titleF if title == "" { title = inProgress.Title } - body := _bodyF if body == "" { body = inProgress.Body } - base := _baseF + base, err := cmd.Flags().GetString("base") + if err != nil { + return err + } if base == "" { + // TODO: use default branch for the repo base = "master" } @@ -149,10 +153,15 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("could not determine GitHub repo: %s", err) } + isDraft, err := cmd.Flags().GetBool("draft") + if err != nil { + return err + } + params := map[string]interface{}{ "title": title, "body": body, - "draft": _draftF, + "draft": isDraft, "baseRefName": base, "headRefName": head, } @@ -202,13 +211,12 @@ var prCreateCmd = &cobra.Command{ } func init() { - prCreateCmd.Flags().BoolVarP(&_draftF, "draft", "d", false, + prCreateCmd.Flags().BoolP("draft", "d", false, "Mark PR as a draft") - prCreateCmd.Flags().StringVarP(&_titleF, "title", "t", "", + prCreateCmd.Flags().StringP("title", "t", "", "Supply a title. Will prompt for one otherwise.") - prCreateCmd.Flags().StringVarP(&_bodyF, "body", "b", "", + prCreateCmd.Flags().StringP("body", "b", "", "Supply a body. Will prompt for one otherwise.") - prCreateCmd.Flags().StringVarP(&_baseF, "base", "T", "", + prCreateCmd.Flags().StringP("base", "T", "", "The branch into which you want your code merged") - } From bb87f4a2fbe56de4f299cc070de9ef550009b90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 13 Nov 2019 19:25:02 +0100 Subject: [PATCH 16/18] Add `pr create` test for git clean state --- command/pr_create_test.go | 43 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 7453a0e1f..191043be3 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -37,7 +37,7 @@ func TestPrCreateHelperProcess(*testing.T) { os.Exit(0) } -func TestReportsUncommittedChanges(t *testing.T) { +func TestPRCreate(t *testing.T) { ctx := context.NewBlank() ctx.SetBranch("feature") ctx.SetRemotes(map[string]string{ @@ -60,12 +60,11 @@ func TestReportsUncommittedChanges(t *testing.T) { `)) origGitCommand := git.GitCommand + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "clean") defer func() { git.GitCommand = origGitCommand }() - git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "dirty") - out := bytes.Buffer{} prCreateCmd.SetOut(&out) @@ -93,6 +92,44 @@ func TestReportsUncommittedChanges(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") + eq(t, out.String(), "https://github.com/OWNER/REPO/pull/12\n") +} + +func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("feature") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `)) + + origGitCommand := git.GitCommand + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "dirty") + defer func() { + git.GitCommand = origGitCommand + }() + + out := bytes.Buffer{} + prCreateCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"pr", "create", "-t", "mytitle", "-b", "mybody"}) + _, err := prCreateCmd.ExecuteC() + eq(t, err, nil) + eq(t, out.String(), `Warning: 1 uncommitted change https://github.com/OWNER/REPO/pull/12 `) From 237fd04ad05ba07bf82ba95e742da801258f5221 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 13 Nov 2019 15:00:57 -0600 Subject: [PATCH 17/18] use errors.Wrap --- command/pr_create.go | 27 ++++++++++++++------------- go.mod | 1 + go.sum | 2 ++ 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index b9d71df8c..7f1c4fcac 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -31,12 +32,12 @@ func prCreate(cmd *cobra.Command, _ []string) error { head, err := ctx.Branch() if err != nil { - return fmt.Errorf("could not determine current branch: %s", err) + return errors.Wrap(err, "could not determine current branch") } remote, err := guessRemote(ctx) if err != nil { - return err + return errors.Wrap(err, "could not determine suitable remote") } if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { @@ -45,11 +46,11 @@ func prCreate(cmd *cobra.Command, _ []string) error { title, err := cmd.Flags().GetString("title") if err != nil { - return err + return errors.Wrap(err, "could not parse title") } body, err := cmd.Flags().GetString("body") if err != nil { - return err + return errors.Wrap(err, "could not parse body") } interactive := title == "" || body == "" @@ -92,7 +93,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { err := survey.Ask(qs, &inProgress) if err != nil { - return fmt.Errorf("could not prompt: %s", err) + return errors.Wrap(err, "could not prompt") } confirmAnswers := struct { Confirmation string @@ -113,7 +114,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { err = survey.Ask(confirmQs, &confirmAnswers) if err != nil { - return fmt.Errorf("could not prompt: %s", err) + return errors.Wrap(err, "could not prompt") } switch confirmAnswers.Confirmation { @@ -136,7 +137,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { } base, err := cmd.Flags().GetString("base") if err != nil { - return err + return errors.Wrap(err, "could not parse base") } if base == "" { // TODO: use default branch for the repo @@ -145,17 +146,17 @@ func prCreate(cmd *cobra.Command, _ []string) error { client, err := apiClientForContext(ctx) if err != nil { - return fmt.Errorf("could not initialize api client: %s", err) + return errors.Wrap(err, "could not initialize api client") } repo, err := ctx.BaseRepo() if err != nil { - return fmt.Errorf("could not determine GitHub repo: %s", err) + return errors.Wrap(err, "could not determine GitHub repo") } isDraft, err := cmd.Flags().GetBool("draft") if err != nil { - return err + return errors.Wrap(err, "could not parse draft") } params := map[string]interface{}{ @@ -168,7 +169,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { pr, err := api.CreatePullRequest(client, repo, params) if err != nil { - return fmt.Errorf("failed to create PR: %s", err) + return errors.Wrap(err, "failed to create PR") } fmt.Fprintln(cmd.OutOrStdout(), pr.URL) @@ -178,14 +179,14 @@ func prCreate(cmd *cobra.Command, _ []string) error { func guessRemote(ctx context.Context) (string, error) { remotes, err := ctx.Remotes() if err != nil { - return "", fmt.Errorf("could not determine suitable remote: %s", err) + return "", errors.Wrap(err, "could not determine suitable remote") } // TODO: consolidate logic with fsContext.BaseRepo // TODO: check if the GH repo that the remote points to is writeable remote, err := remotes.FindByName("upstream", "github", "origin", "*") if err != nil { - return "", fmt.Errorf("could not determine suitable remote: %s", err) + return "", errors.Wrap(err, "could not determine suitable remote") } return remote.Name, nil diff --git a/go.mod b/go.mod index 372e1e07d..f4a0ea38b 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/mattn/go-isatty v0.0.9 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.8.1 github.com/spf13/cobra v0.0.5 github.com/stretchr/testify v1.3.0 // indirect golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 diff --git a/go.sum b/go.sum index 5e670565a..b2f2e063e 100644 --- a/go.sum +++ b/go.sum @@ -34,6 +34,8 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= +github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= From 135efda6d325a1375d00a77b677eb56abfe5d69c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 14 Nov 2019 19:31:33 +0100 Subject: [PATCH 18/18] Avoid re-wrapping of "could not determine suitable remote" error --- command/pr_create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index 7f1c4fcac..c8d86f53a 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -37,7 +37,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { remote, err := guessRemote(ctx) if err != nil { - return errors.Wrap(err, "could not determine suitable remote") + return err } if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { @@ -179,7 +179,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { func guessRemote(ctx context.Context) (string, error) { remotes, err := ctx.Remotes() if err != nil { - return "", errors.Wrap(err, "could not determine suitable remote") + return "", errors.Wrap(err, "could not read git remotes") } // TODO: consolidate logic with fsContext.BaseRepo