diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 000000000..28d17464b --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,22 @@ +name: Code Scanning + +on: + push: + schedule: + - cron: "0 0 * * 0" + +jobs: + CodeQL-Build: + runs-on: ubuntu-latest + + steps: + - name: Check out code + uses: actions/checkout@v2 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: go + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 diff --git a/api/client_test.go b/api/client_test.go index 492609e5c..4c81bf315 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "reflect" "testing" + + "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -15,7 +17,7 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } func TestGraphQL(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient( ReplaceTripper(http), AddHeader("Authorization", "token OTOKEN"), @@ -40,7 +42,7 @@ func TestGraphQL(t *testing.T) { } func TestGraphQLError(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) response := struct{}{} @@ -52,7 +54,7 @@ func TestGraphQLError(t *testing.T) { } func TestRESTGetDelete(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient( ReplaceTripper(http), diff --git a/api/fake_http.go b/api/fake_http.go deleted file mode 100644 index a88c9df17..000000000 --- a/api/fake_http.go +++ /dev/null @@ -1,101 +0,0 @@ -package api - -import ( - "bytes" - "fmt" - "io" - "io/ioutil" - "net/http" - "os" - "path" - "strings" -) - -// FakeHTTP provides a mechanism by which to stub HTTP responses through -type FakeHTTP struct { - // Requests stores references to sequential requests that RoundTrip has received - Requests []*http.Request - count int - responseStubs []*http.Response -} - -// StubResponse pre-records an HTTP response -func (f *FakeHTTP) StubResponse(status int, body io.Reader) { - resp := &http.Response{ - StatusCode: status, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} - -// RoundTrip satisfies http.RoundTripper -func (f *FakeHTTP) RoundTrip(req *http.Request) (*http.Response, error) { - if len(f.responseStubs) <= f.count { - return nil, fmt.Errorf("FakeHTTP: missing response stub for request %d", f.count) - } - resp := f.responseStubs[f.count] - f.count++ - resp.Request = req - f.Requests = append(f.Requests, req) - return resp, nil -} - -func (f *FakeHTTP) StubWithFixture(status int, fixtureFileName string) func() { - fixturePath := path.Join("../test/fixtures/", fixtureFileName) - fixtureFile, _ := os.Open(fixturePath) - f.StubResponse(status, fixtureFile) - return func() { fixtureFile.Close() } -} - -func (f *FakeHTTP) StubRepoResponse(owner, repo string) { - f.StubRepoResponseWithPermission(owner, repo, "WRITE") -} - -func (f *FakeHTTP) StubRepoResponseWithPermission(owner, repo, permission string) { - body := bytes.NewBufferString(fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "%s" - } } } - `, repo, owner, permission)) - resp := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} - -func (f *FakeHTTP) StubForkedRepoResponse(forkFullName, parentFullName string) { - forkRepo := strings.SplitN(forkFullName, "/", 2) - parentRepo := strings.SplitN(parentFullName, "/", 2) - body := bytes.NewBufferString(fmt.Sprintf(` - { "data": { "repo_000": { - "id": "REPOID2", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "ADMIN", - "parent": { - "id": "REPOID1", - "name": "%s", - "owner": {"login": "%s"}, - "defaultBranchRef": { - "name": "master" - }, - "viewerPermission": "READ" - } - } } } - `, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0])) - resp := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(body), - } - f.responseStubs = append(f.responseStubs, resp) -} diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index b2dc7c819..c8ee505cf 100644 --- a/api/queries_issue_test.go +++ b/api/queries_issue_test.go @@ -7,10 +7,11 @@ import ( "testing" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" ) func TestIssueList(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` diff --git a/api/queries_pr.go b/api/queries_pr.go index 616b453fd..a88ec1bbb 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -1,13 +1,29 @@ package api import ( + "context" "fmt" "strings" "time" + "github.com/shurcooL/githubv4" + "github.com/cli/cli/internal/ghrepo" ) +type PullRequestReviewState int + +const ( + ReviewApprove PullRequestReviewState = iota + ReviewRequestChanges + ReviewComment +) + +type PullRequestReviewInput struct { + Body string + State PullRequestReviewState +} + type PullRequestsPayload struct { ViewerCreated PullRequestAndTotalCount ReviewRequested PullRequestAndTotalCount @@ -24,6 +40,7 @@ type PullRequest struct { Number int Title string State string + Closed bool URL string BaseRefName string HeadRefName string @@ -345,10 +362,12 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu query($owner: String!, $repo: String!, $pr_number: Int!) { repository(owner: $owner, name: $repo) { pullRequest(number: $pr_number) { + id url number title state + closed body author { login @@ -451,6 +470,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea repository(owner: $owner, name: $repo) { pullRequests(headRefName: $headRefName, states: OPEN, first: 30) { nodes { + id number title state @@ -654,6 +674,34 @@ func isBlank(v interface{}) bool { } } +func AddReview(client *Client, pr *PullRequest, input *PullRequestReviewInput) error { + var mutation struct { + AddPullRequestReview struct { + ClientMutationID string + } `graphql:"addPullRequestReview(input:$input)"` + } + + state := githubv4.PullRequestReviewEventComment + switch input.State { + case ReviewApprove: + state = githubv4.PullRequestReviewEventApprove + case ReviewRequestChanges: + state = githubv4.PullRequestReviewEventRequestChanges + } + + body := githubv4.String(input.Body) + + gqlInput := githubv4.AddPullRequestReviewInput{ + PullRequestID: pr.ID, + Event: &state, + Body: &body, + } + + v4 := githubv4.NewClient(client.http) + + return v4.Mutate(context.Background(), &mutation, gqlInput, nil) +} + func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { type prBlock struct { Edges []struct { @@ -822,6 +870,44 @@ loop: return &res, nil } +func PullRequestClose(client *Client, repo ghrepo.Interface, pr *PullRequest) error { + var mutation struct { + ClosePullRequest struct { + PullRequest struct { + ID githubv4.ID + } + } `graphql:"closePullRequest(input: $input)"` + } + + input := githubv4.ClosePullRequestInput{ + PullRequestID: pr.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + return err +} + +func PullRequestReopen(client *Client, repo ghrepo.Interface, pr *PullRequest) error { + var mutation struct { + ReopenPullRequest struct { + PullRequest struct { + ID githubv4.ID + } + } `graphql:"reopenPullRequest(input: $input)"` + } + + input := githubv4.ReopenPullRequestInput{ + PullRequestID: pr.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + return err +} + func min(a, b int) int { if a < b { return a diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 49202f628..d8a75f4ff 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -5,10 +5,12 @@ import ( "encoding/json" "io/ioutil" "testing" + + "github.com/cli/cli/pkg/httpmock" ) func Test_RepoCreate(t *testing.T) { - http := &FakeHTTP{} + http := &httpmock.Registry{} client := NewClient(ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(`{}`)) diff --git a/command/completion_test.go b/command/completion_test.go index 3db5ce071..029330a16 100644 --- a/command/completion_test.go +++ b/command/completion_test.go @@ -6,7 +6,7 @@ import ( ) func TestCompletion_bash(t *testing.T) { - output, err := RunCommand(completionCmd, `completion`) + output, err := RunCommand(`completion`) if err != nil { t.Fatal(err) } @@ -17,7 +17,7 @@ func TestCompletion_bash(t *testing.T) { } func TestCompletion_zsh(t *testing.T) { - output, err := RunCommand(completionCmd, `completion -s zsh`) + output, err := RunCommand(`completion -s zsh`) if err != nil { t.Fatal(err) } @@ -28,7 +28,7 @@ func TestCompletion_zsh(t *testing.T) { } func TestCompletion_fish(t *testing.T) { - output, err := RunCommand(completionCmd, `completion -s fish`) + output, err := RunCommand(`completion -s fish`) if err != nil { t.Fatal(err) } @@ -39,7 +39,7 @@ func TestCompletion_fish(t *testing.T) { } func TestCompletion_powerShell(t *testing.T) { - output, err := RunCommand(completionCmd, `completion -s powershell`) + output, err := RunCommand(`completion -s powershell`) if err != nil { t.Fatal(err) } @@ -50,7 +50,7 @@ func TestCompletion_powerShell(t *testing.T) { } func TestCompletion_unsupported(t *testing.T) { - _, err := RunCommand(completionCmd, `completion -s csh`) + _, err := RunCommand(`completion -s csh`) if err == nil || err.Error() != `unsupported shell type "csh"` { t.Fatal(err) } diff --git a/command/config_test.go b/command/config_test.go index cdfeb743a..00148f3b4 100644 --- a/command/config_test.go +++ b/command/config_test.go @@ -17,7 +17,7 @@ editor: ed ` initBlankContext(cfg, "OWNER/REPO", "master") - output, err := RunCommand(configGetCmd, "config get editor") + output, err := RunCommand("config get editor") if err != nil { t.Fatalf("error running command `config get editor`: %v", err) } @@ -27,7 +27,7 @@ editor: ed func TestConfigGet_default(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") - output, err := RunCommand(configGetCmd, "config get git_protocol") + output, err := RunCommand("config get git_protocol") if err != nil { t.Fatalf("error running command `config get git_protocol`: %v", err) } @@ -38,7 +38,7 @@ func TestConfigGet_default(t *testing.T) { func TestConfigGet_not_found(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") - output, err := RunCommand(configGetCmd, "config get missing") + output, err := RunCommand("config get missing") if err != nil { t.Fatalf("error running command `config get missing`: %v", err) } @@ -51,7 +51,7 @@ func TestConfigSet(t *testing.T) { buf := bytes.NewBufferString("") defer config.StubWriteConfig(buf)() - output, err := RunCommand(configSetCmd, "config set editor ed") + output, err := RunCommand("config set editor ed") if err != nil { t.Fatalf("error running command `config set editor ed`: %v", err) } @@ -82,7 +82,7 @@ editor: ed buf := bytes.NewBufferString("") defer config.StubWriteConfig(buf)() - output, err := RunCommand(configSetCmd, "config set editor vim") + output, err := RunCommand("config set editor vim") if err != nil { t.Fatalf("error running command `config get editor`: %v", err) } @@ -110,7 +110,7 @@ git_protocol: https ` initBlankContext(cfg, "OWNER/REPO", "master") - output, err := RunCommand(configGetCmd, "config get -hgithub.com git_protocol") + output, err := RunCommand("config get -hgithub.com git_protocol") if err != nil { t.Fatalf("error running command `config get editor`: %v", err) } @@ -130,7 +130,7 @@ git_protocol: ssh ` initBlankContext(cfg, "OWNER/REPO", "master") - output, err := RunCommand(configGetCmd, "config get -hgithub.com git_protocol") + output, err := RunCommand("config get -hgithub.com git_protocol") if err != nil { t.Fatalf("error running command `config get -hgithub.com git_protocol`: %v", err) } @@ -143,7 +143,7 @@ func TestConfigSetHost(t *testing.T) { buf := bytes.NewBufferString("") defer config.StubWriteConfig(buf)() - output, err := RunCommand(configSetCmd, "config set -hgithub.com git_protocol ssh") + output, err := RunCommand("config set -hgithub.com git_protocol ssh") if err != nil { t.Fatalf("error running command `config set editor ed`: %v", err) } @@ -174,7 +174,7 @@ hosts: buf := bytes.NewBufferString("") defer config.StubWriteConfig(buf)() - output, err := RunCommand(configSetCmd, "config set -hgithub.com git_protocol https") + output, err := RunCommand("config set -hgithub.com git_protocol https") if err != nil { t.Fatalf("error running command `config get editor`: %v", err) } diff --git a/command/issue.go b/command/issue.go index 33eab7a1c..163fc72ba 100644 --- a/command/issue.go +++ b/command/issue.go @@ -87,13 +87,13 @@ With '--web', open the issue in a web browser instead.`, RunE: issueView, } var issueCloseCmd = &cobra.Command{ - Use: "close ", + Use: "close { | }", Short: "close issue", Args: cobra.ExactArgs(1), RunE: issueClose, } var issueReopenCmd = &cobra.Command{ - Use: "reopen ", + Use: "reopen { | }", Short: "reopen issue", Args: cobra.ExactArgs(1), RunE: issueReopen, @@ -421,7 +421,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { action := SubmitAction - interactive := title == "" || body == "" + interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) if interactive { tb, err := titleBodySurvey(cmd, title, body, defaults{}, templateFiles, !hasMetadata) @@ -443,6 +443,10 @@ func issueCreate(cmd *cobra.Command, args []string) error { if body == "" { body = tb.Body } + } else { + if title == "" { + return fmt.Errorf("title can't be blank") + } } if action == PreviewAction { diff --git a/command/issue_test.go b/command/issue_test.go index e408ce3cc..f2985d428 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -24,7 +24,7 @@ func TestIssueStatus(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(issueStatusCmd, "issue status") + output, err := RunCommand("issue status") if err != nil { t.Errorf("error running command `issue status`: %v", err) } @@ -58,7 +58,7 @@ func TestIssueStatus_blankSlate(t *testing.T) { } } } `)) - output, err := RunCommand(issueStatusCmd, "issue status") + output, err := RunCommand("issue status") if err != nil { t.Errorf("error running command `issue status`: %v", err) } @@ -92,7 +92,7 @@ func TestIssueStatus_disabledIssues(t *testing.T) { } } } `)) - _, err := RunCommand(issueStatusCmd, "issue status") + _, err := RunCommand("issue status") if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Errorf("error running command `issue status`: %v", err) } @@ -107,7 +107,7 @@ func TestIssueList(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(issueListCmd, "issue list") + output, err := RunCommand("issue list") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -143,7 +143,7 @@ func TestIssueList_withFlags(t *testing.T) { } } } `)) - output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open -A foo") + output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -183,7 +183,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) { } } } `)) - _, err := RunCommand(issueListCmd, "issue list") + _, err := RunCommand("issue list") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -211,7 +211,7 @@ func TestIssueList_disabledIssues(t *testing.T) { } } } `)) - _, err := RunCommand(issueListCmd, "issue list") + _, err := RunCommand("issue list") if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Errorf("error running command `issue list`: %v", err) } @@ -236,7 +236,7 @@ func TestIssueView_web(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(issueViewCmd, "issue view -w 123") + output, err := RunCommand("issue view -w 123") if err != nil { t.Errorf("error running command `issue view`: %v", err) } @@ -270,7 +270,7 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(issueViewCmd, "issue view -w \"#123\"") + output, err := RunCommand("issue view -w \"#123\"") if err != nil { t.Errorf("error running command `issue view`: %v", err) } @@ -350,7 +350,7 @@ func TestIssueView_Preview(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(issueViewCmd, tc.command) + output, err := RunCommand(tc.command) if err != nil { t.Errorf("error running command `%v`: %v", tc.command, err) } @@ -379,7 +379,7 @@ func TestIssueView_web_notFound(t *testing.T) { }) defer restoreCmd() - _, err := RunCommand(issueViewCmd, "issue view -w 9999") + _, err := RunCommand("issue view -w 9999") if err == nil || err.Error() != "graphql error: 'Could not resolve to an Issue with the number of 9999.'" { t.Errorf("error running command `issue view`: %v", err) } @@ -401,7 +401,7 @@ func TestIssueView_disabledIssues(t *testing.T) { } } } `)) - _, err := RunCommand(issueViewCmd, `issue view 6666`) + _, err := RunCommand(`issue view 6666`) if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Errorf("error running command `issue view`: %v", err) } @@ -426,7 +426,7 @@ func TestIssueView_web_urlArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(issueViewCmd, "issue view -w https://github.com/OWNER/REPO/issues/123") + output, err := RunCommand("issue view -w https://github.com/OWNER/REPO/issues/123") if err != nil { t.Errorf("error running command `issue view`: %v", err) } @@ -457,7 +457,7 @@ func TestIssueCreate(t *testing.T) { } } } } `)) - output, err := RunCommand(issueCreateCmd, `issue create -t hello -b "cash rules everything around me"`) + output, err := RunCommand(`issue create -t hello -b "cash rules everything around me"`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -493,7 +493,7 @@ func TestIssueCreate_disabledIssues(t *testing.T) { } } } `)) - _, err := RunCommand(issueCreateCmd, `issue create -t heres -b johnny`) + _, err := RunCommand(`issue create -t heres -b johnny`) if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Errorf("error running command `issue create`: %v", err) } @@ -511,7 +511,7 @@ func TestIssueCreate_web(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(issueCreateCmd, `issue create --web`) + output, err := RunCommand(`issue create --web`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -537,7 +537,7 @@ func TestIssueCreate_webTitleBody(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(issueCreateCmd, `issue create -w -t mytitle -b mybody`) + output, err := RunCommand(`issue create -w -t mytitle -b mybody`) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -695,7 +695,7 @@ func TestIssueClose(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand(issueCloseCmd, "issue close 13") + output, err := RunCommand("issue close 13") if err != nil { t.Fatalf("error running command `issue close`: %v", err) } @@ -721,7 +721,7 @@ func TestIssueClose_alreadyClosed(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand(issueCloseCmd, "issue close 13") + output, err := RunCommand("issue close 13") if err != nil { t.Fatalf("error running command `issue close`: %v", err) } @@ -744,7 +744,7 @@ func TestIssueClose_issuesDisabled(t *testing.T) { } } } `)) - _, err := RunCommand(issueCloseCmd, "issue close 13") + _, err := RunCommand("issue close 13") if err == nil { t.Fatalf("expected error when issues are disabled") } @@ -768,7 +768,7 @@ func TestIssueReopen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand(issueReopenCmd, "issue reopen 2") + output, err := RunCommand("issue reopen 2") if err != nil { t.Fatalf("error running command `issue reopen`: %v", err) } @@ -794,7 +794,7 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand(issueReopenCmd, "issue reopen 2") + output, err := RunCommand("issue reopen 2") if err != nil { t.Fatalf("error running command `issue reopen`: %v", err) } @@ -817,7 +817,7 @@ func TestIssueReopen_issuesDisabled(t *testing.T) { } } } `)) - _, err := RunCommand(issueReopenCmd, "issue reopen 2") + _, err := RunCommand("issue reopen 2") if err == nil { t.Fatalf("expected error when issues are disabled") } diff --git a/command/pr.go b/command/pr.go index 009548061..069396714 100644 --- a/command/pr.go +++ b/command/pr.go @@ -21,16 +21,18 @@ func init() { RootCmd.AddCommand(prCmd) prCmd.AddCommand(prCheckoutCmd) prCmd.AddCommand(prCreateCmd) - prCmd.AddCommand(prListCmd) prCmd.AddCommand(prStatusCmd) - prCmd.AddCommand(prViewCmd) + prCmd.AddCommand(prCloseCmd) + prCmd.AddCommand(prReopenCmd) + prCmd.AddCommand(prListCmd) prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") prListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|merged|all}") prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") + prCmd.AddCommand(prViewCmd) prViewCmd.Flags().BoolP("web", "w", false, "Open a pull request in the browser") } @@ -65,6 +67,18 @@ is displayed. With '--web', open the pull request in a web browser instead.`, RunE: prView, } +var prCloseCmd = &cobra.Command{ + Use: "close { | | }", + Short: "Close a pull request", + Args: cobra.ExactArgs(1), + RunE: prClose, +} +var prReopenCmd = &cobra.Command{ + Use: "reopen { | | }", + Short: "Reopen a pull request", + Args: cobra.ExactArgs(1), + RunE: prReopen, +} func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -101,13 +115,19 @@ func prStatus(cmd *cobra.Command, args []string) error { fmt.Fprintln(out, "") printHeader(out, "Current branch") - if prPayload.CurrentPR != nil { - printPrs(out, 0, *prPayload.CurrentPR) + currentPR := prPayload.CurrentPR + currentBranch, _ := ctx.Branch() + noPRMessage := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]")) + if currentPR != nil { + if baseRepo.(*api.Repository).DefaultBranchRef.Name == currentBranch && currentPR.State != "OPEN" { + printMessage(out, noPRMessage) + } else { + printPrs(out, 0, *currentPR) + } } else if currentPRHeadRef == "" { printMessage(out, " There is no current branch") } else { - message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]")) - printMessage(out, message) + printMessage(out, noPRMessage) } fmt.Fprintln(out) @@ -328,6 +348,78 @@ func prView(cmd *cobra.Command, args []string) error { } } +func prClose(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + pr, err := prFromArg(apiClient, baseRepo, args[0]) + if err != nil { + return err + } + + if pr.State == "MERGED" { + err := fmt.Errorf("%s Pull request #%d can't be closed because it was already merged", utils.Red("!"), pr.Number) + return err + } else if pr.Closed { + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already closed\n", utils.Yellow("!"), pr.Number) + return nil + } + + err = api.PullRequestClose(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Closed pull request #%d\n", utils.Red("✔"), pr.Number) + + return nil +} + +func prReopen(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + pr, err := prFromArg(apiClient, baseRepo, args[0]) + if err != nil { + return err + } + + if pr.State == "MERGED" { + err := fmt.Errorf("%s Pull request #%d can't be reopened because it was already merged", utils.Red("!"), pr.Number) + return err + } + + if !pr.Closed { + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already open\n", utils.Yellow("!"), pr.Number) + return nil + } + + err = api.PullRequestReopen(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Reopened pull request #%d\n", utils.Green("✔"), pr.Number) + + return nil +} + func printPrPreview(out io.Writer, pr *api.PullRequest) error { // Header (Title and State) fmt.Fprintln(out, utils.Bold(pr.Title)) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index 77be4c15a..851d6ed22 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -55,7 +55,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -107,7 +107,7 @@ func TestPRCheckout_urlArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OWNER/REPO/pull/123/files`) + output, err := RunCommand(`pr checkout https://github.com/OWNER/REPO/pull/123/files`) eq(t, err, nil) eq(t, output.String(), "") @@ -156,7 +156,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OTHER/POE/pull/123/files`) + output, err := RunCommand(`pr checkout https://github.com/OTHER/POE/pull/123/files`) eq(t, err, nil) eq(t, output.String(), "") @@ -219,7 +219,7 @@ func TestPRCheckout_branchArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout hubot:feature`) + output, err := RunCommand(`pr checkout hubot:feature`) eq(t, err, nil) eq(t, output.String(), "") @@ -269,7 +269,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -322,7 +322,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -375,7 +375,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -428,7 +428,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -479,7 +479,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") @@ -530,7 +530,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) + output, err := RunCommand(`pr checkout 123`) eq(t, err, nil) eq(t, output.String(), "") diff --git a/command/pr_create_test.go b/command/pr_create_test.go index f3a0954c8..f69c5c0c6 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/context" "github.com/cli/cli/git" + "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" ) @@ -39,7 +40,7 @@ func TestPRCreate(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push - output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) + output, err := RunCommand(`pr create -t "my title" -b "my body"`) eq(t, err, nil) bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) @@ -101,7 +102,7 @@ func TestPRCreate_withForking(t *testing.T) { cs.Stub("") // git remote add cs.Stub("") // git push - output, err := RunCommand(prCreateCmd, `pr create -t title -b body`) + output, err := RunCommand(`pr create -t title -b body`) eq(t, err, nil) eq(t, http.Requests[3].URL.Path, "/repos/OWNER/REPO/forks") @@ -132,7 +133,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - _, err := RunCommand(prCreateCmd, `pr create`) + _, err := RunCommand(`pr create`) if err == nil { t.Fatal("error expected, got nil") } @@ -167,7 +168,7 @@ func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git rev-parse - _, err := RunCommand(prCreateCmd, `pr create -BanotherBase -t"cool" -b"nah"`) + _, err := RunCommand(`pr create -BanotherBase -t"cool" -b"nah"`) if err != nil { t.Errorf("got unexpected error %q", err) } @@ -192,7 +193,7 @@ func TestPRCreate_web(t *testing.T) { cs.Stub("") // git push cs.Stub("") // browser - output, err := RunCommand(prCreateCmd, `pr create --web`) + output, err := RunCommand(`pr create --web`) eq(t, err, nil) eq(t, output.String(), "") @@ -232,7 +233,7 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push - output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) + output, err := RunCommand(`pr create -t "my title" -b "my body"`) eq(t, err, nil) eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") @@ -301,7 +302,7 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push - output, err := RunCommand(prCreateCmd, `pr create -t "cross repo" -b "same branch"`) + output, err := RunCommand(`pr create -t "cross repo" -b "same branch"`) eq(t, err, nil) bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) @@ -377,7 +378,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { }, }) - output, err := RunCommand(prCreateCmd, `pr create`) + output, err := RunCommand(`pr create`) eq(t, err, nil) bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) @@ -408,20 +409,27 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { func TestPRCreate_survey_defaults_monocommit(t *testing.T) { initBlankContext("", "OWNER/REPO", "feature") http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } + defer http.Verify(t) + http.Register(httpmock.GraphQL(`\bviewerPermission\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) + http.Register(httpmock.GraphQL(`\bforks\(`), httpmock.StringResponse(` + { "data": { "repository": { "forks": { "nodes": [ + ] } } } } `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register(httpmock.GraphQL(`\bpullRequests\(`), httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register(httpmock.GraphQL(`\bcreatePullRequest\(`), httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(inputs map[string]interface{}) { + eq(t, inputs["repositoryId"], "REPOID") + eq(t, inputs["title"], "the sky above the port") + eq(t, inputs["body"], "was the color of a television, turned to a dead channel") + eq(t, inputs["baseRefName"], "master") + eq(t, inputs["headRefName"], "feature") + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -454,31 +462,8 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { }, }) - output, err := RunCommand(prCreateCmd, `pr create`) + output, err := RunCommand(`pr create`) eq(t, err, nil) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "the sky above the port") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } @@ -512,7 +497,7 @@ func TestPRCreate_survey_autofill(t *testing.T) { cs.Stub("") // git push cs.Stub("") // browser open - output, err := RunCommand(prCreateCmd, `pr create -f`) + output, err := RunCommand(`pr create -f`) eq(t, err, nil) bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) @@ -553,7 +538,7 @@ func TestPRCreate_defaults_error_autofill(t *testing.T) { cs.Stub("") // git status cs.Stub("") // git log - _, err := RunCommand(prCreateCmd, "pr create -f") + _, err := RunCommand("pr create -f") eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") } @@ -571,7 +556,7 @@ func TestPRCreate_defaults_error_web(t *testing.T) { cs.Stub("") // git status cs.Stub("") // git log - _, err := RunCommand(prCreateCmd, "pr create -w") + _, err := RunCommand("pr create -w") eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") } @@ -621,7 +606,7 @@ func TestPRCreate_defaults_error_interactive(t *testing.T) { }, }) - output, err := RunCommand(prCreateCmd, `pr create`) + output, err := RunCommand(`pr create`) eq(t, err, nil) stderr := string(output.Stderr()) diff --git a/command/pr_review.go b/command/pr_review.go new file mode 100644 index 000000000..bd0453784 --- /dev/null +++ b/command/pr_review.go @@ -0,0 +1,134 @@ +package command + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/cli/cli/api" + "github.com/spf13/cobra" +) + +func init() { + prCmd.AddCommand(prReviewCmd) + + prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request") + prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request") + prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request") + prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review") +} + +var prReviewCmd = &cobra.Command{ + Use: "review [{ | | ]", + Short: "Add a review to a pull request.", + Args: cobra.MaximumNArgs(1), + Long: `Add a review to either a specified pull request or the pull request associated with the current branch. + +Examples: + + gh pr review -a # mark the current branch's pull request as approved + gh pr review -c -b "interesting" # comment on the current branch's pull request + gh pr review 123 -r -b "needs more ascii art" # request changes on pull request 123 + `, + RunE: prReview, +} + +func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { + found := 0 + flag := "" + var state api.PullRequestReviewState + + if cmd.Flags().Changed("approve") { + found++ + flag = "approve" + state = api.ReviewApprove + } + if cmd.Flags().Changed("request-changes") { + found++ + flag = "request-changes" + state = api.ReviewRequestChanges + } + if cmd.Flags().Changed("comment") { + found++ + flag = "comment" + state = api.ReviewComment + } + + if found != 1 { + return nil, errors.New("need exactly one of --approve, --request-changes, or --comment") + } + + body, err := cmd.Flags().GetString("body") + if err != nil { + return nil, err + } + + if (flag == "request-changes" || flag == "comment") && body == "" { + return nil, fmt.Errorf("body cannot be blank for %s review", flag) + } + + return &api.PullRequestReviewInput{ + Body: body, + State: state, + }, nil +} + +func prReview(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return fmt.Errorf("could not determine base repo: %w", err) + } + + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + var prNum int + branchWithOwner := "" + + if len(args) == 0 { + prNum, branchWithOwner, err = prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + } else { + prArg, repo := prFromURL(args[0]) + if repo != nil { + baseRepo = repo + } else { + prArg = strings.TrimPrefix(args[0], "#") + } + prNum, err = strconv.Atoi(prArg) + if err != nil { + return errors.New("could not parse pull request argument") + } + } + + input, err := processReviewOpt(cmd) + if err != nil { + return fmt.Errorf("did not understand desired review action: %w", err) + } + + var pr *api.PullRequest + if prNum > 0 { + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNum) + if err != nil { + return fmt.Errorf("could not find pull request: %w", err) + } + } else { + pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) + if err != nil { + return fmt.Errorf("could not find pull request: %w", err) + } + } + + err = api.AddReview(apiClient, pr, input) + if err != nil { + return fmt.Errorf("failed to create review: %w", err) + } + + return nil +} diff --git a/command/pr_review_test.go b/command/pr_review_test.go new file mode 100644 index 000000000..96ceabd7e --- /dev/null +++ b/command/pr_review_test.go @@ -0,0 +1,213 @@ +package command + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "testing" +) + +func TestPRReview_validation(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + for _, cmd := range []string{ + `pr review 123`, + `pr review --approve --comment 123`, + `pr review --approve --comment -b"hey" 123`, + } { + http.StubRepoResponse("OWNER", "REPO") + _, err := RunCommand(cmd) + eq(t, err.Error(), "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment") + } +} + +func TestPRReview_url_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "id": "foobar123", + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123") + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "") +} + +func TestPRReview_number_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "id": "foobar123", + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand("pr review --approve 123") + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "") +} + +func TestPRReview_no_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } }`)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand(`pr review --comment -b "cool story"`) + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "COMMENT") + eq(t, reqBody.Variables.Input.Body, "cool story") +} + +func TestPRReview_blank_comment(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr review --comment 123`) + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review") +} + +func TestPRReview_blank_request_changes(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr review -r 123`) + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review") +} + +func TestPRReview(t *testing.T) { + type c struct { + Cmd string + ExpectedEvent string + ExpectedBody string + } + cases := []c{ + c{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, + c{`pr review --approve`, "APPROVE", ""}, + c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"}, + c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"}, + } + + for _, kase := range cases { + initBlankContext("", "OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand(kase.Cmd) + if err != nil { + t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.Event, kase.ExpectedEvent) + eq(t, reqBody.Variables.Input.Body, kase.ExpectedBody) + } +} diff --git a/command/pr_test.go b/command/pr_test.go index ba0989645..0aff5e14a 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -15,9 +15,6 @@ import ( "github.com/cli/cli/internal/run" "github.com/cli/cli/test" "github.com/google/go-cmp/cmp" - "github.com/google/shlex" - "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -27,52 +24,6 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -type cmdOut struct { - outBuf, errBuf *bytes.Buffer -} - -func (c cmdOut) String() string { - return c.outBuf.String() -} - -func (c cmdOut) Stderr() string { - return c.errBuf.String() -} - -func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { - rootCmd := cmd.Root() - argv, err := shlex.Split(args) - if err != nil { - return nil, err - } - rootCmd.SetArgs(argv) - - outBuf := bytes.Buffer{} - cmd.SetOut(&outBuf) - errBuf := bytes.Buffer{} - cmd.SetErr(&errBuf) - - // Reset flag values so they don't leak between tests - // FIXME: change how we initialize Cobra commands to render this hack unnecessary - cmd.Flags().VisitAll(func(f *pflag.Flag) { - switch v := f.Value.(type) { - case pflag.SliceValue: - _ = v.Replace([]string{}) - default: - switch v.Type() { - case "bool", "string", "int": - _ = v.Set(f.DefValue) - } - } - }) - - _, err = rootCmd.ExecuteC() - cmd.SetOut(nil) - cmd.SetErr(nil) - - return &cmdOut{&outBuf, &errBuf}, err -} - func TestPRStatus(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -82,7 +33,7 @@ func TestPRStatus(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -120,7 +71,7 @@ branch.blueberries.merge refs/heads/blueberries`)} } })() - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Fatalf("error running command `pr status`: %v", err) } @@ -140,7 +91,7 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -167,7 +118,7 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -190,6 +141,27 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { } } +func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") + + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand("pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + func TestPRStatus_currentBranch_Closed(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -199,7 +171,7 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -211,6 +183,27 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) { } } +func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") + + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosed.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand("pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + func TestPRStatus_currentBranch_Merged(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -220,7 +213,7 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -232,6 +225,27 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) { } } +func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") + + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMerged.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand("pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + func TestPRStatus_blankSlate(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -241,7 +255,7 @@ func TestPRStatus_blankSlate(t *testing.T) { { "data": {} } `)) - output, err := RunCommand(prStatusCmd, "pr status") + output, err := RunCommand("pr status") if err != nil { t.Errorf("error running command `pr status`: %v", err) } @@ -273,7 +287,7 @@ func TestPRList(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prListCmd, "pr list") + output, err := RunCommand("pr list") if err != nil { t.Fatal(err) } @@ -296,7 +310,7 @@ func TestPRList_filtering(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) + output, err := RunCommand(`pr list -s all -l one,two -l three`) if err != nil { t.Fatal(err) } @@ -329,7 +343,7 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prListCmd, "pr list -l one,two") + output, err := RunCommand("pr list -l one,two") if err != nil { t.Fatal(err) } @@ -348,7 +362,7 @@ func TestPRList_filteringClosed(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - _, err := RunCommand(prListCmd, `pr list -s closed`) + _, err := RunCommand(`pr list -s closed`) if err != nil { t.Fatal(err) } @@ -372,7 +386,7 @@ func TestPRList_filteringAssignee(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - _, err := RunCommand(prListCmd, `pr list -s merged -l "needs tests" -a hubot -B develop`) + _, err := RunCommand(`pr list -s merged -l "needs tests" -a hubot -B develop`) if err != nil { t.Fatal(err) } @@ -396,7 +410,7 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - _, err := RunCommand(prListCmd, `pr list -l one,two -a hubot`) + _, err := RunCommand(`pr list -l one,two -a hubot`) if err == nil && err.Error() != "multiple labels with --assignee are not supported" { t.Fatal(err) } @@ -527,7 +541,7 @@ func TestPRView_Preview(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(prViewCmd, tc.args) + output, err := RunCommand(tc.args) if err != nil { t.Errorf("error running command `%v`: %v", tc.args, err) } @@ -560,7 +574,7 @@ func TestPRView_web_currentBranch(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w") + output, err := RunCommand("pr view -w") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -598,7 +612,7 @@ func TestPRView_web_noResultsForBranch(t *testing.T) { }) defer restoreCmd() - _, err := RunCommand(prViewCmd, "pr view -w") + _, err := RunCommand("pr view -w") if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) } @@ -626,7 +640,7 @@ func TestPRView_web_numberArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w 23") + output, err := RunCommand("pr view -w 23") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -658,7 +672,7 @@ func TestPRView_web_numberArgWithHash(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w \"#23\"") + output, err := RunCommand("pr view -w \"#23\"") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -689,7 +703,7 @@ func TestPRView_web_urlArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w https://github.com/OWNER/REPO/pull/23/files") + output, err := RunCommand("pr view -w https://github.com/OWNER/REPO/pull/23/files") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -723,7 +737,7 @@ func TestPRView_web_branchArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w blueberries") + output, err := RunCommand("pr view -w blueberries") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -758,7 +772,7 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(prViewCmd, "pr view -w hubot:blueberries") + output, err := RunCommand("pr view -w hubot:blueberries") if err != nil { t.Errorf("error running command `pr view`: %v", err) } @@ -800,3 +814,129 @@ func TestPrStateTitleWithColor(t *testing.T) { }) } } + +func TestPrClose(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 96 } + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr close 96") + if err != nil { + t.Fatalf("error running command `pr close`: %v", err) + } + + r := regexp.MustCompile(`Closed pull request #96`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrClose_alreadyClosed(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 101, "closed": true } + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr close 101") + if err != nil { + t.Fatalf("error running command `pr close`: %v", err) + } + + r := regexp.MustCompile(`Pull request #101 is already closed`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReopen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "closed": true} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr reopen 666") + if err != nil { + t.Fatalf("error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Reopened pull request #666`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReopen_alreadyOpen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "closed": false} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr reopen 666") + if err != nil { + t.Fatalf("error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Pull request #666 is already open`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReopen_alreadyMerged(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "closed": true, "state": "MERGED"} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr reopen 666") + if err == nil { + t.Fatalf("expected an error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Pull request #666 can't be reopened because it was already merged`) + + if !r.MatchString(err.Error()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } + +} diff --git a/command/repo_test.go b/command/repo_test.go index 3e9fca954..760298231 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -42,7 +42,7 @@ func TestRepoFork_already_forked(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") defer http.StubWithFixture(200, "forkResult.json")() - output, err := RunCommand(repoForkCmd, "repo fork --remote=false") + output, err := RunCommand("repo fork --remote=false") if err != nil { t.Errorf("got unexpected error: %v", err) } @@ -69,7 +69,7 @@ func TestRepoFork_reuseRemote(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") defer http.StubWithFixture(200, "forkResult.json")() - output, err := RunCommand(repoForkCmd, "repo fork") + output, err := RunCommand("repo fork") if err != nil { t.Errorf("got unexpected error: %v", err) } @@ -97,7 +97,7 @@ func TestRepoFork_in_parent(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") defer http.StubWithFixture(200, "forkResult.json")() - output, err := RunCommand(repoForkCmd, "repo fork --remote=false") + output, err := RunCommand("repo fork --remote=false") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -132,7 +132,7 @@ func TestRepoFork_outside(t *testing.T) { http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() - output, err := RunCommand(repoForkCmd, tt.args) + output, err := RunCommand(tt.args) if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -162,7 +162,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { return &test.OutputStub{} })() - output, err := RunCommand(repoForkCmd, "repo fork --remote") + output, err := RunCommand("repo fork --remote") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -195,7 +195,7 @@ func TestRepoFork_outside_yes(t *testing.T) { cs.Stub("") // git clone cs.Stub("") // git remote add - output, err := RunCommand(repoForkCmd, "repo fork --clone OWNER/REPO") + output, err := RunCommand("repo fork --clone OWNER/REPO") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -229,7 +229,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } defer func() { Confirm = oldConfirm }() - output, err := RunCommand(repoForkCmd, "repo fork OWNER/REPO") + output, err := RunCommand("repo fork OWNER/REPO") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -263,7 +263,7 @@ func TestRepoFork_outside_survey_no(t *testing.T) { } defer func() { Confirm = oldConfirm }() - output, err := RunCommand(repoForkCmd, "repo fork OWNER/REPO") + output, err := RunCommand("repo fork OWNER/REPO") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -300,7 +300,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { } defer func() { Confirm = oldConfirm }() - output, err := RunCommand(repoForkCmd, "repo fork") + output, err := RunCommand("repo fork") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -343,7 +343,7 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) { } defer func() { Confirm = oldConfirm }() - output, err := RunCommand(repoForkCmd, "repo fork") + output, err := RunCommand("repo fork") if err != nil { t.Errorf("error running command `repo fork`: %v", err) } @@ -469,7 +469,7 @@ func TestRepoClone(t *testing.T) { cs.Stub("") // git clone - output, err := RunCommand(repoCloneCmd, tt.args) + output, err := RunCommand(tt.args) if err != nil { t.Fatalf("error running command `repo clone`: %v", err) } @@ -499,7 +499,7 @@ func TestRepoClone_hasParent(t *testing.T) { cs.Stub("") // git clone cs.Stub("") // git remote add - _, err := RunCommand(repoCloneCmd, "repo clone OWNER/REPO") + _, err := RunCommand("repo clone OWNER/REPO") if err != nil { t.Fatalf("error running command `repo clone`: %v", err) } @@ -536,7 +536,7 @@ func TestRepoCreate(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoCreateCmd, "repo create REPO") + output, err := RunCommand("repo create REPO") if err != nil { t.Errorf("error running command `repo create`: %v", err) } @@ -605,7 +605,7 @@ func TestRepoCreate_org(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoCreateCmd, "repo create ORG/REPO") + output, err := RunCommand("repo create ORG/REPO") if err != nil { t.Errorf("error running command `repo create`: %v", err) } @@ -674,7 +674,7 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoCreateCmd, "repo create ORG/REPO --team monkeys") + output, err := RunCommand("repo create ORG/REPO --team monkeys") if err != nil { t.Errorf("error running command `repo create`: %v", err) } @@ -725,7 +725,7 @@ func TestRepoView_web(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoViewCmd, "repo view -w") + output, err := RunCommand("repo view -w") if err != nil { t.Errorf("error running command `repo view`: %v", err) } @@ -758,7 +758,7 @@ func TestRepoView_web_ownerRepo(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoViewCmd, "repo view -w cli/cli") + output, err := RunCommand("repo view -w cli/cli") if err != nil { t.Errorf("error running command `repo view`: %v", err) } @@ -790,7 +790,7 @@ func TestRepoView_web_fullURL(t *testing.T) { }) defer restoreCmd() - output, err := RunCommand(repoViewCmd, "repo view -w https://github.com/cli/cli") + output, err := RunCommand("repo view -w https://github.com/cli/cli") if err != nil { t.Errorf("error running command `repo view`: %v", err) } @@ -820,7 +820,7 @@ func TestRepoView(t *testing.T) { "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="} `)) - output, err := RunCommand(repoViewCmd, "repo view") + output, err := RunCommand("repo view") if err != nil { t.Errorf("error running command `repo view`: %v", err) } @@ -848,7 +848,7 @@ func TestRepoView_nonmarkdown_readme(t *testing.T) { "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="} `)) - output, err := RunCommand(repoViewCmd, "repo view") + output, err := RunCommand("repo view") if err != nil { t.Errorf("error running command `repo view`: %v", err) } @@ -867,7 +867,7 @@ func TestRepoView_blanks(t *testing.T) { http.StubResponse(200, bytes.NewBufferString("{}")) http.StubResponse(200, bytes.NewBufferString("{}")) - output, err := RunCommand(repoViewCmd, "repo view") + output, err := RunCommand("repo view") if err != nil { t.Errorf("error running command `repo view`: %v", err) } diff --git a/command/testing.go b/command/testing.go index b9b0ba04a..784488fd8 100644 --- a/command/testing.go +++ b/command/testing.go @@ -1,16 +1,19 @@ package command import ( + "bytes" "errors" "fmt" "reflect" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/core" - "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/httpmock" + "github.com/google/shlex" + "github.com/spf13/pflag" ) const defaultTestConfig = `hosts: @@ -91,14 +94,67 @@ func initBlankContext(cfg, repo, branch string) { } } -func initFakeHTTP() *api.FakeHTTP { - http := &api.FakeHTTP{} +func initFakeHTTP() *httpmock.Registry { + http := &httpmock.Registry{} apiClientForContext = func(context.Context) (*api.Client, error) { return api.NewClient(api.ReplaceTripper(http)), nil } return http } +type cmdOut struct { + outBuf, errBuf *bytes.Buffer +} + +func (c cmdOut) String() string { + return c.outBuf.String() +} + +func (c cmdOut) Stderr() string { + return c.errBuf.String() +} + +func RunCommand(args string) (*cmdOut, error) { + rootCmd := RootCmd + rootArgv, err := shlex.Split(args) + if err != nil { + return nil, err + } + + cmd, _, err := rootCmd.Traverse(rootArgv) + if err != nil { + return nil, err + } + + rootCmd.SetArgs(rootArgv) + + outBuf := bytes.Buffer{} + cmd.SetOut(&outBuf) + errBuf := bytes.Buffer{} + cmd.SetErr(&errBuf) + + // Reset flag values so they don't leak between tests + // FIXME: change how we initialize Cobra commands to render this hack unnecessary + cmd.Flags().VisitAll(func(f *pflag.Flag) { + f.Changed = false + switch v := f.Value.(type) { + case pflag.SliceValue: + _ = v.Replace([]string{}) + default: + switch v.Type() { + case "bool", "string", "int": + _ = v.Set(f.DefValue) + } + } + }) + + _, err = rootCmd.ExecuteC() + cmd.SetOut(nil) + cmd.SetErr(nil) + + return &cmdOut{&outBuf, &errBuf}, err +} + type errorStub struct { message string } diff --git a/context/remote_test.go b/context/remote_test.go index 4b6838e23..9cf7a0adf 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -70,7 +71,7 @@ func Test_translateRemotes(t *testing.T) { } func Test_resolvedRemotes_triangularSetup(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} apiClient := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` @@ -137,7 +138,7 @@ func Test_resolvedRemotes_triangularSetup(t *testing.T) { } func Test_resolvedRemotes_forkLookup(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} apiClient := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(` diff --git a/pkg/githubtemplate/github_template.go b/pkg/githubtemplate/github_template.go index 3aac35554..89cceda4c 100644 --- a/pkg/githubtemplate/github_template.go +++ b/pkg/githubtemplate/github_template.go @@ -66,11 +66,12 @@ mainLoop: // ExtractName returns the name of the template from YAML front-matter func ExtractName(filePath string) string { contents, err := ioutil.ReadFile(filePath) - if err == nil && detectFrontmatter(contents)[0] == 0 { + frontmatterBoundaries := detectFrontmatter(contents) + if err == nil && frontmatterBoundaries[0] == 0 { templateData := struct { Name string }{} - if err := yaml.Unmarshal(contents, &templateData); err == nil && templateData.Name != "" { + if err := yaml.Unmarshal(contents[0:frontmatterBoundaries[1]], &templateData); err == nil && templateData.Name != "" { return templateData.Name } } diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index fa757873c..839e215b3 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -148,9 +148,7 @@ name: Bug Report about: This is how you report bugs --- -Template contents ---- -More of template +**Template contents** `, args: args{ filePath: tmpfile.Name(), diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go new file mode 100644 index 000000000..9474c3dd8 --- /dev/null +++ b/pkg/httpmock/legacy.go @@ -0,0 +1,89 @@ +package httpmock + +import ( + "fmt" + "io" + "net/http" + "os" + "path" + "strings" +) + +// TODO: clean up methods in this file when there are no more callers + +func (r *Registry) StubResponse(status int, body io.Reader) { + r.Register(MatchAny, func(*http.Request) (*http.Response, error) { + return httpResponse(status, body), nil + }) +} + +func (r *Registry) StubWithFixture(status int, fixtureFileName string) func() { + fixturePath := path.Join("../test/fixtures/", fixtureFileName) + fixtureFile, err := os.Open(fixturePath) + r.Register(MatchAny, func(*http.Request) (*http.Response, error) { + if err != nil { + return nil, err + } + return httpResponse(200, fixtureFile), nil + }) + return func() { + if err == nil { + fixtureFile.Close() + } + } +} + +func (r *Registry) StubRepoResponse(owner, repo string) { + r.StubRepoResponseWithPermission(owner, repo, "WRITE") +} + +func (r *Registry) StubRepoResponseWithPermission(owner, repo, permission string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, "master", permission))) +} + +func (r *Registry) StubRepoResponseWithDefaultBranch(owner, repo, defaultBranch string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, defaultBranch, "WRITE"))) +} + +func (r *Registry) StubForkedRepoResponse(ownRepo, parentRepo string) { + r.Register(MatchAny, StringResponse(RepoNetworkStubForkResponse(ownRepo, parentRepo))) +} + +func RepoNetworkStubResponse(owner, repo, defaultBranch, permission string) string { + return fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "%s" + }, + "viewerPermission": "%s" + } } } + `, repo, owner, defaultBranch, permission) +} + +func RepoNetworkStubForkResponse(forkFullName, parentFullName string) string { + forkRepo := strings.SplitN(forkFullName, "/", 2) + parentRepo := strings.SplitN(parentFullName, "/", 2) + return fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID2", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master" + }, + "viewerPermission": "ADMIN", + "parent": { + "id": "REPOID1", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master" + }, + "viewerPermission": "READ" + } + } } } + `, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0]) +} diff --git a/pkg/httpmock/registry.go b/pkg/httpmock/registry.go new file mode 100644 index 000000000..486d79a06 --- /dev/null +++ b/pkg/httpmock/registry.go @@ -0,0 +1,70 @@ +package httpmock + +import ( + "fmt" + "net/http" + "sync" +) + +type Registry struct { + mu sync.Mutex + stubs []*Stub + Requests []*http.Request +} + +func (r *Registry) Register(m Matcher, resp Responder) { + r.stubs = append(r.stubs, &Stub{ + Matcher: m, + Responder: resp, + }) +} + +type Testing interface { + Errorf(string, ...interface{}) +} + +func (r *Registry) Verify(t Testing) { + n := 0 + for _, s := range r.stubs { + if !s.matched { + n++ + } + } + if n > 0 { + // NOTE: stubs offer no useful reflection, so we can't print details + // about dead stubs and what they were trying to match + t.Errorf("%d unmatched HTTP stubs", n) + } +} + +// RoundTrip satisfies http.RoundTripper +func (r *Registry) RoundTrip(req *http.Request) (*http.Response, error) { + var stub *Stub + + r.mu.Lock() + for _, s := range r.stubs { + if s.matched || !s.Matcher(req) { + continue + } + // TODO: reinstate this check once the legacy layer has been cleaned up + // if stub != nil { + // r.mu.Unlock() + // return nil, fmt.Errorf("more than 1 stub matched %v", req) + // } + stub = s + break // TODO: remove + } + if stub != nil { + stub.matched = true + } + + if stub == nil { + r.mu.Unlock() + return nil, fmt.Errorf("no registered stubs matched %v", req) + } + + r.Requests = append(r.Requests, req) + r.mu.Unlock() + + return stub.Responder(req) +} diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go new file mode 100644 index 000000000..b27ceea6c --- /dev/null +++ b/pkg/httpmock/stub.go @@ -0,0 +1,96 @@ +package httpmock + +import ( + "bytes" + "encoding/json" + "io" + "io/ioutil" + "net/http" + "regexp" + "strings" +) + +type Matcher func(req *http.Request) bool +type Responder func(req *http.Request) (*http.Response, error) + +type Stub struct { + matched bool + Matcher Matcher + Responder Responder +} + +func MatchAny(*http.Request) bool { + return true +} + +func GraphQL(q string) Matcher { + re := regexp.MustCompile(q) + + return func(req *http.Request) bool { + if !strings.EqualFold(req.Method, "POST") { + return false + } + if req.URL.Path != "/graphql" { + return false + } + + var bodyData struct { + Query string + } + _ = decodeJSONBody(req, &bodyData) + + return re.MatchString(bodyData.Query) + } +} + +func readBody(req *http.Request) ([]byte, error) { + bodyCopy := &bytes.Buffer{} + r := io.TeeReader(req.Body, bodyCopy) + req.Body = ioutil.NopCloser(bodyCopy) + return ioutil.ReadAll(r) +} + +func decodeJSONBody(req *http.Request, dest interface{}) error { + b, err := readBody(req) + if err != nil { + return err + } + return json.Unmarshal(b, dest) +} + +func StringResponse(body string) Responder { + return func(*http.Request) (*http.Response, error) { + return httpResponse(200, bytes.NewBufferString(body)), nil + } +} + +func JSONResponse(body interface{}) Responder { + return func(*http.Request) (*http.Response, error) { + b, _ := json.Marshal(body) + return httpResponse(200, bytes.NewBuffer(b)), nil + } +} + +func GraphQLMutation(body string, cb func(map[string]interface{})) Responder { + return func(req *http.Request) (*http.Response, error) { + var bodyData struct { + Variables struct { + Input map[string]interface{} + } + } + err := decodeJSONBody(req, &bodyData) + if err != nil { + return nil, err + } + cb(bodyData.Variables.Input) + + return httpResponse(200, bytes.NewBufferString(body)), nil + } +} + +func httpResponse(status int, body io.Reader) *http.Response { + return &http.Response{ + StatusCode: status, + Body: ioutil.NopCloser(body), + } +} diff --git a/update/update_test.go b/update/update_test.go index bd919f530..2fcb2d6ab 100644 --- a/update/update_test.go +++ b/update/update_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/cli/cli/api" + "github.com/cli/cli/pkg/httpmock" ) func TestCheckForUpdate(t *testing.T) { @@ -51,7 +52,7 @@ func TestCheckForUpdate(t *testing.T) { for _, s := range scenarios { t.Run(s.Name, func(t *testing.T) { - http := &api.FakeHTTP{} + http := &httpmock.Registry{} client := api.NewClient(api.ReplaceTripper(http)) http.StubResponse(200, bytes.NewBufferString(fmt.Sprintf(`{ "tag_name": "%s",