diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index aff0ce343..39f048881 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -29,7 +29,7 @@ jobs: go mod verify go mod download - LINT_VERSION=1.26.0 + LINT_VERSION=1.27.0 curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \ tar xz --strip-components 1 --wildcards \*/golangci-lint mkdir -p bin && mv golangci-lint bin/ diff --git a/.goreleaser.yml b/.goreleaser.yml index 0909fa8f7..fa6883834 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -61,7 +61,7 @@ brews: folder: Formula custom_block: | head do - url "https://github.com/cli/cli.git" + url "https://github.com/cli/cli.git", :branch => "trunk" depends_on "go" end install: | diff --git a/Makefile b/Makefile index b925c11fd..504564dee 100644 --- a/Makefile +++ b/Makefile @@ -8,15 +8,26 @@ ifdef SOURCE_DATE_EPOCH else BUILD_DATE ?= $(shell date "$(DATE_FMT)") endif -LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) $(LDFLAGS) -LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(LDFLAGS) + +ifndef CGO_CPPFLAGS + export CGO_CPPFLAGS := $(CPPFLAGS) +endif +ifndef CGO_CFLAGS + export CGO_CFLAGS := $(CFLAGS) +endif +ifndef CGO_LDFLAGS + export CGO_LDFLAGS := $(LDFLAGS) +endif + +GO_LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) +GO_LDFLAGS += -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) ifdef GH_OAUTH_CLIENT_SECRET - LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(LDFLAGS) - LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(LDFLAGS) + GO_LDFLAGS += -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID) + GO_LDFLAGS += -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) endif bin/gh: $(BUILD_FILES) - @go build -trimpath -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh + @go build -trimpath -ldflags "$(GO_LDFLAGS)" -o "$@" ./cmd/gh test: go test ./... diff --git a/api/client.go b/api/client.go index 1c8c1eaa2..f307b2bf5 100644 --- a/api/client.go +++ b/api/client.go @@ -7,10 +7,12 @@ import ( "io" "io/ioutil" "net/http" + "net/url" "regexp" "strings" "github.com/henvic/httpretty" + "github.com/shurcooL/graphql" ) // ClientOption represents an argument to NewClient @@ -154,7 +156,21 @@ func (gr GraphQLErrorResponse) Error() string { for _, e := range gr.Errors { errorMessages = append(errorMessages, e.Message) } - return fmt.Sprintf("graphql error: '%s'", strings.Join(errorMessages, ", ")) + return fmt.Sprintf("GraphQL error: %s", strings.Join(errorMessages, "\n")) +} + +// HTTPError is an error returned by a failed API call +type HTTPError struct { + StatusCode int + RequestURL *url.URL + Message string +} + +func (err HTTPError) Error() string { + if err.Message != "" { + return fmt.Sprintf("HTTP %d: %s (%s)", err.StatusCode, err.Message, err.RequestURL) + } + return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL) } // Returns whether or not scopes are present, appID, and error @@ -220,6 +236,10 @@ func (c Client) GraphQL(query string, variables map[string]interface{}, data int return handleResponse(resp, data) } +func graphQLClient(h *http.Client) *graphql.Client { + return graphql.NewClient("https://api.github.com/graphql", h) +} + // REST performs a REST request and parses the response. func (c Client) REST(method string, p string, body io.Reader, data interface{}) error { url := "https://api.github.com/" + p @@ -283,22 +303,25 @@ func handleResponse(resp *http.Response, data interface{}) error { } func handleHTTPError(resp *http.Response) error { - var message string + httpError := HTTPError{ + StatusCode: resp.StatusCode, + RequestURL: resp.Request.URL, + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + httpError.Message = err.Error() + return httpError + } + var parsedBody struct { Message string `json:"message"` } - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return err - } - err = json.Unmarshal(body, &parsedBody) - if err != nil { - message = string(body) - } else { - message = parsedBody.Message + if err := json.Unmarshal(body, &parsedBody); err == nil { + httpError.Message = parsedBody.Message } - return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message) + return httpError } var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`) diff --git a/api/client_test.go b/api/client_test.go index 4c81bf315..b7c226c8f 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "errors" "io/ioutil" "reflect" "testing" @@ -46,9 +47,15 @@ func TestGraphQLError(t *testing.T) { client := NewClient(ReplaceTripper(http)) response := struct{}{} - http.StubResponse(200, bytes.NewBufferString(`{"errors":[{"message":"OH NO"}]}`)) + http.StubResponse(200, bytes.NewBufferString(` + { "errors": [ + {"message":"OH NO"}, + {"message":"this is fine"} + ] + }`)) + err := client.GraphQL("", nil, &response) - if err == nil || err.Error() != "graphql error: 'OH NO'" { + if err == nil || err.Error() != "GraphQL error: OH NO\nthis is fine" { t.Fatalf("got %q", err.Error()) } } @@ -66,3 +73,23 @@ func TestRESTGetDelete(t *testing.T) { err := client.REST("DELETE", "applications/CLIENTID/grant", r, nil) eq(t, err, nil) } + +func TestRESTError(t *testing.T) { + http := &httpmock.Registry{} + client := NewClient(ReplaceTripper(http)) + + http.StubResponse(422, bytes.NewBufferString(`{"message": "OH NO"}`)) + + var httpErr HTTPError + err := client.REST("DELETE", "repos/branch", nil, nil) + if err == nil || !errors.As(err, &httpErr) { + t.Fatalf("got %v", err) + } + + if httpErr.StatusCode != 422 { + t.Errorf("expected status code 422, got %d", httpErr.StatusCode) + } + if httpErr.Error() != "HTTP 422: OH NO (https://api.github.com/repos/branch)" { + t.Errorf("got %q", httpErr.Error()) + } +} diff --git a/api/queries_issue.go b/api/queries_issue.go index 56a04edd9..1b3bddd64 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -88,7 +88,7 @@ const fragments = ` // IssueCreate creates an issue in a GitHub repository func IssueCreate(client *Client, repo *Repository, params map[string]interface{}) (*Issue, error) { query := ` - mutation CreateIssue($input: CreateIssueInput!) { + mutation IssueCreate($input: CreateIssueInput!) { createIssue(input: $input) { issue { url @@ -140,7 +140,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string) } query := fragments + ` - query($owner: String!, $repo: String!, $viewer: String!, $per_page: Int = 10) { + query IssueStatus($owner: String!, $repo: String!, $viewer: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { hasIssuesEnabled assigned: issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: UPDATED_AT, direction: DESC}) { @@ -198,7 +198,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string) return &payload, nil } -func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string) (*IssuesAndTotalCount, error) { +func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) { var states []string switch state { case "open", "": @@ -212,10 +212,10 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str } query := fragments + ` - query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String) { + query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mention: String, $milestone: String) { repository(owner: $owner, name: $repo) { hasIssuesEnabled - issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author}) { + issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) { totalCount nodes { ...issue @@ -243,6 +243,12 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str if authorString != "" { variables["author"] = authorString } + if mentionString != "" { + variables["mention"] = mentionString + } + if milestoneString != "" { + variables["milestone"] = milestoneString + } var response struct { Repository struct { @@ -300,7 +306,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } query := ` - query($owner: String!, $repo: String!, $issue_number: Int!) { + query IssueByNumber($owner: String!, $repo: String!, $issue_number: Int!) { repository(owner: $owner, name: $repo) { hasIssuesEnabled issue(number: $issue_number) { @@ -377,12 +383,14 @@ func IssueClose(client *Client, repo ghrepo.Interface, issue Issue) error { } `graphql:"closeIssue(input: $input)"` } - input := githubv4.CloseIssueInput{ - IssueID: issue.ID, + variables := map[string]interface{}{ + "input": githubv4.CloseIssueInput{ + IssueID: issue.ID, + }, } - v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + err := gql.MutateNamed(context.Background(), "IssueClose", &mutation, variables) if err != nil { return err @@ -400,12 +408,14 @@ func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error { } `graphql:"reopenIssue(input: $input)"` } - input := githubv4.ReopenIssueInput{ - IssueID: issue.ID, + variables := map[string]interface{}{ + "input": githubv4.ReopenIssueInput{ + IssueID: issue.ID, + }, } - v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + err := gql.MutateNamed(context.Background(), "IssueReopen", &mutation, variables) return err } diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index ffe4aaad1..2ea64f0c6 100644 --- a/api/queries_issue_test.go +++ b/api/queries_issue_test.go @@ -40,7 +40,7 @@ func TestIssueList(t *testing.T) { `)) repo, _ := ghrepo.FromFullName("OWNER/REPO") - _, err := IssueList(client, repo, "open", []string{}, "", 251, "") + _, err := IssueList(client, repo, "open", []string{}, "", 251, "", "", "") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/api/queries_org.go b/api/queries_org.go index eb7ec0789..b91c2da78 100644 --- a/api/queries_org.go +++ b/api/queries_org.go @@ -47,11 +47,11 @@ func OrganizationProjects(client *Client, owner string) ([]RepoProject, error) { "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var projects []RepoProject for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "OrganizationProjectList", &query, variables) if err != nil { return nil, err } @@ -90,11 +90,11 @@ func OrganizationTeams(client *Client, owner string) ([]OrgTeam, error) { "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var teams []OrgTeam for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "OrganizationTeamList", &query, variables) if err != nil { return nil, err } diff --git a/api/queries_pr.go b/api/queries_pr.go index 00d01661a..999d67126 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -296,7 +296,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu ` queryPrefix := ` - query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { + query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { defaultBranchRef { name } pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) { @@ -311,7 +311,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu ` if currentPRNumber > 0 { queryPrefix = ` - query($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { + query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { defaultBranchRef { name } pullRequest(number: $number) { @@ -408,7 +408,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu } query := ` - query($owner: String!, $repo: String!, $pr_number: Int!) { + query PullRequestByNumber($owner: String!, $repo: String!, $pr_number: Int!) { repository(owner: $owner, name: $repo) { pullRequest(number: $pr_number) { id @@ -518,7 +518,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea } query := ` - query($owner: String!, $repo: String!, $headRefName: String!) { + query PullRequestForBranch($owner: String!, $repo: String!, $headRefName: String!) { repository(owner: $owner, name: $repo) { pullRequests(headRefName: $headRefName, states: OPEN, first: 30) { nodes { @@ -634,7 +634,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea // CreatePullRequest creates a pull request in a GitHub repository func CreatePullRequest(client *Client, repo *Repository, params map[string]interface{}) (*PullRequest, error) { query := ` - mutation CreatePullRequest($input: CreatePullRequestInput!) { + mutation PullRequestCreate($input: CreatePullRequestInput!) { createPullRequest(input: $input) { pullRequest { id @@ -681,7 +681,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter } if len(updateParams) > 0 { updateQuery := ` - mutation UpdatePullRequest($input: UpdatePullRequestInput!) { + mutation PullRequestCreateMetadata($input: UpdatePullRequestInput!) { updatePullRequest(input: $input) { clientMutationId } }` updateParams["pullRequestId"] = pr.ID @@ -705,7 +705,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter if len(reviewParams) > 0 { reviewQuery := ` - mutation RequestReviews($input: RequestReviewsInput!) { + mutation PullRequestCreateRequestReviews($input: RequestReviewsInput!) { requestReviews(input: $input) { clientMutationId } }` reviewParams["pullRequestId"] = pr.ID @@ -749,16 +749,16 @@ func AddReview(client *Client, pr *PullRequest, input *PullRequestReviewInput) e } body := githubv4.String(input.Body) - - gqlInput := githubv4.AddPullRequestReviewInput{ - PullRequestID: pr.ID, - Event: &state, - Body: &body, + variables := map[string]interface{}{ + "input": githubv4.AddPullRequestReviewInput{ + PullRequestID: pr.ID, + Event: &state, + Body: &body, + }, } - v4 := githubv4.NewClient(client.http) - - return v4.Mutate(context.Background(), &mutation, gqlInput, nil) + gql := graphQLClient(client.http) + return gql.MutateNamed(context.Background(), "PullRequestReviewAdd", &mutation, variables) } func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { @@ -798,7 +798,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*P // If assignee wasn't specified, use `Repository.pullRequest` for ability to // query by multiple labels query := fragment + ` - query( + query PullRequestList( $owner: String!, $repo: String!, $limit: Int!, @@ -840,7 +840,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*P // `Repository.pullRequests`, but this mode doesn't support multiple labels if assignee, ok := vars["assignee"].(string); ok { query = fragment + ` - query( + query PullRequestList( $q: String!, $limit: Int!, $endCursor: String, @@ -938,12 +938,14 @@ func PullRequestClose(client *Client, repo ghrepo.Interface, pr *PullRequest) er } `graphql:"closePullRequest(input: $input)"` } - input := githubv4.ClosePullRequestInput{ - PullRequestID: pr.ID, + variables := map[string]interface{}{ + "input": githubv4.ClosePullRequestInput{ + PullRequestID: pr.ID, + }, } - v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + err := gql.MutateNamed(context.Background(), "PullRequestClose", &mutation, variables) return err } @@ -957,12 +959,14 @@ func PullRequestReopen(client *Client, repo ghrepo.Interface, pr *PullRequest) e } `graphql:"reopenPullRequest(input: $input)"` } - input := githubv4.ReopenPullRequestInput{ - PullRequestID: pr.ID, + variables := map[string]interface{}{ + "input": githubv4.ReopenPullRequestInput{ + PullRequestID: pr.ID, + }, } - v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + err := gql.MutateNamed(context.Background(), "PullRequestReopen", &mutation, variables) return err } @@ -984,13 +988,15 @@ func PullRequestMerge(client *Client, repo ghrepo.Interface, pr *PullRequest, m } `graphql:"mergePullRequest(input: $input)"` } - input := githubv4.MergePullRequestInput{ - PullRequestID: pr.ID, - MergeMethod: &mergeMethod, + variables := map[string]interface{}{ + "input": githubv4.MergePullRequestInput{ + PullRequestID: pr.ID, + MergeMethod: &mergeMethod, + }, } - v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + err := gql.MutateNamed(context.Background(), "PullRequestMerge", &mutation, variables) return err } @@ -1004,18 +1010,19 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er } `graphql:"markPullRequestReadyForReview(input: $input)"` } - input := githubv4.MarkPullRequestReadyForReviewInput{PullRequestID: pr.ID} + variables := map[string]interface{}{ + "input": githubv4.MarkPullRequestReadyForReviewInput{ + PullRequestID: pr.ID, + }, + } - v4 := githubv4.NewClient(client.http) - return v4.Mutate(context.Background(), &mutation, input, nil) + gql := graphQLClient(client.http) + return gql.MutateNamed(context.Background(), "PullRequestReadyForReview", &mutation, variables) } func BranchDeleteRemote(client *Client, repo ghrepo.Interface, branch string) error { - var response struct { - NodeID string `json:"node_id"` - } path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), branch) - return client.REST("DELETE", path, nil, &response) + return client.REST("DELETE", path, nil, nil) } func min(a, b int) int { diff --git a/api/queries_pr_test.go b/api/queries_pr_test.go new file mode 100644 index 000000000..3441534bf --- /dev/null +++ b/api/queries_pr_test.go @@ -0,0 +1,47 @@ +package api + +import ( + "testing" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" +) + +func TestBranchDeleteRemote(t *testing.T) { + var tests = []struct { + name string + responseStatus int + responseBody string + expectError bool + }{ + { + name: "success", + responseStatus: 204, + responseBody: "", + expectError: false, + }, + { + name: "error", + responseStatus: 500, + responseBody: `{"message": "oh no"}`, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + http := &httpmock.Registry{} + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/branch"), + httpmock.StatusStringResponse(tt.responseStatus, tt.responseBody)) + + client := NewClient(ReplaceTripper(http)) + repo, _ := ghrepo.FromFullName("OWNER/REPO") + + err := BranchDeleteRemote(client, repo, "branch") + if (err != nil) != tt.expectError { + t.Fatalf("unexpected result: %v", err) + } + }) + } +} diff --git a/api/queries_repo.go b/api/queries_repo.go index 22d42cfbb..f9044b5e1 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -78,7 +78,7 @@ func (r Repository) ViewerCanTriage() bool { func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { query := ` - query($owner: String!, $name: String!) { + query RepositoryInfo($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id hasIssuesEnabled @@ -124,8 +124,8 @@ func RepoParent(client *Client, repo ghrepo.Interface) (ghrepo.Interface, error) "name": githubv4.String(repo.RepoName()), } - v4 := githubv4.NewClient(client.http) - err := v4.Query(context.Background(), &query, variables) + gql := graphQLClient(client.http) + err := gql.QueryNamed(context.Background(), "RepositoryFindParent", &query, variables) if err != nil { return nil, err } @@ -174,7 +174,7 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e } isPrivate } - query { + query RepositoryNetwork { viewer { login } %s } @@ -284,7 +284,7 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { } if err := client.GraphQL(` - query($owner: String!, $repo: String!) { + query RepositoryFindFork($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { forks(first: 1, affiliations: [OWNER, COLLABORATOR]) { nodes { @@ -353,7 +353,7 @@ func RepoCreate(client *Client, input RepoCreateInput) (*Repository, error) { } err := client.GraphQL(` - mutation($input: CreateRepositoryInput!) { + mutation RepositoryCreate($input: CreateRepositoryInput!) { createRepository(input: $input) { repository { id @@ -626,7 +626,7 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes } query := &bytes.Buffer{} - fmt.Fprint(query, "{\n") + fmt.Fprint(query, "query RepositoryResolveMetadataIDs {\n") for i, u := range users { fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) } @@ -710,11 +710,11 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var projects []RepoProject for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "RepositoryProjectList", &query, variables) if err != nil { return nil, err } @@ -754,11 +754,11 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var users []RepoAssignee for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "RepositoryAssignableUsers", &query, variables) if err != nil { return nil, err } @@ -798,11 +798,11 @@ func RepoLabels(client *Client, repo ghrepo.Interface) ([]RepoLabel, error) { "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var labels []RepoLabel for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "RepositoryLabelList", &query, variables) if err != nil { return nil, err } @@ -842,11 +842,11 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err "endCursor": (*githubv4.String)(nil), } - v4 := githubv4.NewClient(client.http) + gql := graphQLClient(client.http) var milestones []RepoMilestone for { - err := v4.Query(context.Background(), &query, variables) + err := gql.QueryNamed(context.Background(), "RepositoryMilestoneList", &query, variables) if err != nil { return nil, err } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 03123cf43..58e4e061c 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -60,7 +60,7 @@ func Test_RepoMetadata(t *testing.T) { } http.Register( - httpmock.GraphQL(`\bassignableUsers\(`), + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` { "data": { "repository": { "assignableUsers": { "nodes": [ @@ -71,7 +71,7 @@ func Test_RepoMetadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\blabels\(`), + httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` { "data": { "repository": { "labels": { "nodes": [ @@ -83,7 +83,7 @@ func Test_RepoMetadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\bmilestones\(`), + httpmock.GraphQL(`query RepositoryMilestoneList\b`), httpmock.StringResponse(` { "data": { "repository": { "milestones": { "nodes": [ @@ -94,7 +94,7 @@ func Test_RepoMetadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\brepository\(.+\bprojects\(`), + httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(` { "data": { "repository": { "projects": { "nodes": [ @@ -105,7 +105,7 @@ func Test_RepoMetadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\borganization\(.+\bprojects\(`), + httpmock.GraphQL(`query OrganizationProjectList\b`), httpmock.StringResponse(` { "data": { "organization": { "projects": { "nodes": [ @@ -115,7 +115,7 @@ func Test_RepoMetadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\borganization\(.+\bteams\(`), + httpmock.GraphQL(`query OrganizationTeamList\b`), httpmock.StringResponse(` { "data": { "organization": { "teams": { "nodes": [ @@ -188,7 +188,7 @@ func Test_RepoResolveMetadataIDs(t *testing.T) { Labels: []string{"bug", "help wanted"}, } - expectedQuery := `{ + expectedQuery := `query RepositoryResolveMetadataIDs { u000: user(login:"monalisa"){id,login} u001: user(login:"hubot"){id,login} u002: user(login:"octocat"){id,login} @@ -219,7 +219,7 @@ t001: team(slug:"robots"){id,slug} ` http.Register( - httpmock.MatchAny, + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), httpmock.GraphQLQuery(responseJSON, func(q string, _ map[string]interface{}) { if q != expectedQuery { t.Errorf("expected query %q, got %q", expectedQuery, q) diff --git a/api/queries_user.go b/api/queries_user.go index c83d3aac2..ea31c59ea 100644 --- a/api/queries_user.go +++ b/api/queries_user.go @@ -2,8 +2,6 @@ package api import ( "context" - - "github.com/shurcooL/githubv4" ) func CurrentLoginName(client *Client) (string, error) { @@ -12,7 +10,7 @@ func CurrentLoginName(client *Client) (string, error) { Login string } } - v4 := githubv4.NewClient(client.http) - err := v4.Query(context.Background(), &query, nil) + gql := graphQLClient(client.http) + err := gql.QueryNamed(context.Background(), "UserCurrent", &query, nil) return query.Viewer.Login, err } diff --git a/command/config.go b/command/config.go index 7f34b8687..0a77e99e5 100644 --- a/command/config.go +++ b/command/config.go @@ -48,6 +48,7 @@ var configSetCmd = &cobra.Command{ Short: "Update configuration with a value for the given key", Example: heredoc.Doc(` $ gh config set editor vim + $ gh config set editor "code --wait" `), Args: cobra.ExactArgs(2), RunE: configSet, diff --git a/command/issue.go b/command/issue.go index 4409aa179..19c473cb9 100644 --- a/command/issue.go +++ b/command/issue.go @@ -22,6 +22,8 @@ import ( ) func init() { + issueCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + RootCmd.AddCommand(issueCmd) issueCmd.AddCommand(issueStatusCmd) @@ -43,6 +45,8 @@ func init() { issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") issueListCmd.Flags().StringP("author", "A", "", "Filter by author") + issueListCmd.Flags().String("mention", "", "Filter by mention") + issueListCmd.Flags().StringP("milestone", "m", "", "Filter by milestone `name`") issueCmd.AddCommand(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") @@ -190,6 +194,16 @@ func issueList(cmd *cobra.Command, args []string) error { return err } + mention, err := cmd.Flags().GetString("mention") + if err != nil { + return err + } + + milestone, err := cmd.Flags().GetString("milestone") + if err != nil { + return err + } + if web { issueListURL := fmt.Sprintf( "https://github.com/%s/issues", @@ -203,7 +217,7 @@ func issueList(cmd *cobra.Command, args []string) error { return utils.OpenInBrowser(openURL) } - listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author) + listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mention, milestone) if err != nil { return err } @@ -211,7 +225,7 @@ func issueList(cmd *cobra.Command, args []string) error { hasFilters := false cmd.Flags().Visit(func(f *pflag.Flag) { switch f.Name { - case "state", "label", "assignee", "author": + case "state", "label", "assignee", "author", "mention", "milestone": hasFilters = true } }) @@ -742,11 +756,11 @@ func issueClose(cmd *cobra.Command, args []string) error { if errors.As(err, &idErr) { return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) } else if err != nil { - return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) + return err } if issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already closed\n", utils.Yellow("!"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already closed\n", utils.Yellow("!"), issue.Number, issue.Title) return nil } @@ -755,7 +769,7 @@ func issueClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d\n", utils.Red("✔"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d (%s)\n", utils.Red("✔"), issue.Number, issue.Title) return nil } @@ -777,11 +791,11 @@ func issueReopen(cmd *cobra.Command, args []string) error { if errors.As(err, &idErr) { return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) } else if err != nil { - return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) + return err } if !issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already open\n", utils.Yellow("!"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already open\n", utils.Yellow("!"), issue.Number, issue.Title) return nil } @@ -790,7 +804,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Green("✔"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d (%s)\n", utils.Green("✔"), issue.Number, issue.Title) return nil } diff --git a/command/issue_test.go b/command/issue_test.go index cc23b7bda..f91e4ae16 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/json" "io/ioutil" - "os" "os/exec" "regexp" "strings" @@ -14,6 +13,7 @@ import ( "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" ) func TestIssueStatus(t *testing.T) { @@ -21,12 +21,11 @@ func TestIssueStatus(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.GraphQL(`\bviewer\b`), + httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - - jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.FileResponse("../test/fixtures/issueStatus.json")) output, err := RunCommand("issue status") if err != nil { @@ -53,17 +52,17 @@ func TestIssueStatus_blankSlate(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.GraphQL(`\bviewer\b`), + httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "assigned": { "nodes": [] }, - "mentioned": { "nodes": [] }, - "authored": { "nodes": [] } - } } } - `)) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "assigned": { "nodes": [] }, + "mentioned": { "nodes": [] }, + "authored": { "nodes": [] } + } } }`)) output, err := RunCommand("issue status") if err != nil { @@ -93,14 +92,14 @@ func TestIssueStatus_disabledIssues(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.GraphQL(`\bviewer\b`), + httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } }`)) _, err := RunCommand("issue status") if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { @@ -112,10 +111,9 @@ func TestIssueList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/issueList.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.FileResponse("../test/fixtures/issueList.json")) output, err := RunCommand("issue list") if err != nil { @@ -145,15 +143,22 @@ func TestIssueList_withFlags(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.GraphQLQuery(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } }`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "probablyCher", params["assignee"].(string)) + assert.Equal(t, "foo", params["author"].(string)) + assert.Equal(t, "me", params["mention"].(string)) + assert.Equal(t, "1.x", params["milestone"].(string)) + assert.Equal(t, []interface{}{"web", "bug"}, params["labels"].([]interface{})) + assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{})) + })) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } } - `)) - - output, err := RunCommand("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 --mention me --milestone 1.x") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -163,22 +168,6 @@ func TestIssueList_withFlags(t *testing.T) { No issues match your search in OWNER/REPO `) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables struct { - Assignee string - Labels []string - States []string - Author string - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Assignee, "probablyCher") - eq(t, reqBody.Variables.Labels, []string{"web", "bug"}) - eq(t, reqBody.Variables.States, []string{"OPEN"}) - eq(t, reqBody.Variables.Author, "foo") } func TestIssueList_withInvalidLimitFlag(t *testing.T) { @@ -394,10 +383,7 @@ func TestIssueView_Preview(t *testing.T) { initBlankContext("", "OWNER/REPO", tc.ownerRepo) http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open(tc.fixture) - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) output, err := RunCommand(tc.command) if err != nil { @@ -429,7 +415,7 @@ func TestIssueView_web_notFound(t *testing.T) { defer restoreCmd() _, err := RunCommand("issue view -w 9999") - if err == nil || err.Error() != "graphql error: 'Could not resolve to an Issue with the number of 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) } @@ -536,10 +522,10 @@ func TestIssueCreate_metadata(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`\bviewerPermission\b`), + httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) http.Register( - httpmock.GraphQL(`\bhasIssuesEnabled\b`), + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -548,7 +534,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } } `)) http.Register( - httpmock.GraphQL(`\bu000:`), + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), httpmock.StringResponse(` { "data": { "u000": { "login": "MonaLisa", "id": "MONAID" }, @@ -559,7 +545,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } `)) http.Register( - httpmock.GraphQL(`\bmilestones\(`), + httpmock.GraphQL(`query RepositoryMilestoneList\b`), httpmock.StringResponse(` { "data": { "repository": { "milestones": { "nodes": [ @@ -570,7 +556,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\brepository\(.+\bprojects\(`), + httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(` { "data": { "repository": { "projects": { "nodes": [ @@ -581,7 +567,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\borganization\(.+\bprojects\(`), + httpmock.GraphQL(`query OrganizationProjectList\b`), httpmock.StringResponse(` { "data": { "organization": null }, "errors": [{ @@ -592,7 +578,7 @@ func TestIssueCreate_metadata(t *testing.T) { } `)) http.Register( - httpmock.GraphQL(`\bcreateIssue\(`), + httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { "URL": "https://github.com/OWNER/REPO/issues/12" @@ -828,7 +814,7 @@ func TestIssueClose(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 13} + "issue": { "number": 13, "title": "The title of the issue"} } } } `)) @@ -839,7 +825,7 @@ func TestIssueClose(t *testing.T) { t.Fatalf("error running command `issue close`: %v", err) } - r := regexp.MustCompile(`Closed issue #13`) + r := regexp.MustCompile(`Closed issue #13 \(The title of the issue\)`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -854,7 +840,7 @@ func TestIssueClose_alreadyClosed(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 13, "closed": true} + "issue": { "number": 13, "title": "The title of the issue", "closed": true} } } } `)) @@ -865,7 +851,7 @@ func TestIssueClose_alreadyClosed(t *testing.T) { t.Fatalf("error running command `issue close`: %v", err) } - r := regexp.MustCompile(`#13 is already closed`) + r := regexp.MustCompile(`Issue #13 \(The title of the issue\) is already closed`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -901,7 +887,7 @@ func TestIssueReopen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": true} + "issue": { "number": 2, "closed": true, "title": "The title of the issue"} } } } `)) @@ -912,7 +898,7 @@ func TestIssueReopen(t *testing.T) { t.Fatalf("error running command `issue reopen`: %v", err) } - r := regexp.MustCompile(`Reopened issue #2`) + r := regexp.MustCompile(`Reopened issue #2 \(The title of the issue\)`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -927,7 +913,7 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": false} + "issue": { "number": 2, "closed": false, "title": "The title of the issue"} } } } `)) @@ -938,7 +924,7 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { t.Fatalf("error running command `issue reopen`: %v", err) } - r := regexp.MustCompile(`#2 is already open`) + r := regexp.MustCompile(`Issue #2 \(The title of the issue\) is already open`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) diff --git a/command/pr.go b/command/pr.go index 4901d3852..440279143 100644 --- a/command/pr.go +++ b/command/pr.go @@ -23,6 +23,8 @@ import ( ) func init() { + prCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + RootCmd.AddCommand(prCmd) prCmd.AddCommand(prCheckoutCmd) prCmd.AddCommand(prCreateCmd) @@ -108,8 +110,14 @@ var prReopenCmd = &cobra.Command{ var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", Short: "Merge a pull request", - Args: cobra.MaximumNArgs(1), - RunE: prMerge, + Long: heredoc.Doc(` + Merge a pull request on GitHub. + + By default, the head branch of the pull request will get deleted on both remote and local repositories. + To retain the branch, use '--delete-branch=false'. + `), + Args: cobra.MaximumNArgs(1), + RunE: prMerge, } var prReadyCmd = &cobra.Command{ Use: "ready [ | | ]", @@ -373,10 +381,10 @@ func prClose(cmd *cobra.Command, args []string) error { } if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d can't be closed because it was already merged", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d (%s) can't be closed because it was already merged", utils.Red("!"), pr.Number, pr.Title) return err } else if pr.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already closed\n", utils.Yellow("!"), pr.Number) + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d (%s) is already closed\n", utils.Yellow("!"), pr.Number, pr.Title) return nil } @@ -385,7 +393,7 @@ func prClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(colorableErr(cmd), "%s Closed pull request #%d\n", utils.Red("✔"), pr.Number) + fmt.Fprintf(colorableErr(cmd), "%s Closed pull request #%d (%s)\n", utils.Red("✔"), pr.Number, pr.Title) return nil } @@ -403,12 +411,12 @@ func prReopen(cmd *cobra.Command, args []string) error { } if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d can't be reopened because it was already merged", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d (%s) can't be reopened because it was already merged", utils.Red("!"), pr.Number, pr.Title) return err } if !pr.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already open\n", utils.Yellow("!"), pr.Number) + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d (%s) is already open\n", utils.Yellow("!"), pr.Number, pr.Title) return nil } @@ -417,7 +425,7 @@ func prReopen(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(colorableErr(cmd), "%s Reopened pull request #%d\n", utils.Green("✔"), pr.Number) + fmt.Fprintf(colorableErr(cmd), "%s Reopened pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) return nil } @@ -435,13 +443,13 @@ func prMerge(cmd *cobra.Command, args []string) error { } if pr.Mergeable == "CONFLICTING" { - err := fmt.Errorf("%s Pull request #%d has conflicts and isn't mergeable ", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) return err } else if pr.Mergeable == "UNKNOWN" { - err := fmt.Errorf("%s Pull request #%d can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) return err } else if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d was already merged", utils.Red("!"), pr.Number) + err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) return err } @@ -502,7 +510,7 @@ func prMerge(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number) + fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) if deleteBranch { repo, err := api.GitHubRepo(apiClient, baseRepo) @@ -543,7 +551,9 @@ func prMerge(cmd *cobra.Command, args []string) error { if !crossRepoPR { err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName) - if err != nil { + var httpErr api.HTTPError + // The ref might have already been deleted by GitHub + if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) { err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) return err } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 2eb0872c7..408a45256 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -72,22 +72,22 @@ func TestPRCreate_metadata(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`\bviewerPermission\b`), + httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) http.Register( - httpmock.GraphQL(`\bforks\(`), + httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` { "data": { "repository": { "forks": { "nodes": [ ] } } } } `)) http.Register( - httpmock.GraphQL(`\bpullRequests\(`), + httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes": [ ] } } } } `)) http.Register( - httpmock.GraphQL(`\bteam\(`), + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), httpmock.StringResponse(` { "data": { "u000": { "login": "MonaLisa", "id": "MONAID" }, @@ -103,7 +103,7 @@ func TestPRCreate_metadata(t *testing.T) { } } `)) http.Register( - httpmock.GraphQL(`\bmilestones\(`), + httpmock.GraphQL(`query RepositoryMilestoneList\b`), httpmock.StringResponse(` { "data": { "repository": { "milestones": { "nodes": [ @@ -114,7 +114,7 @@ func TestPRCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\brepository\(.+\bprojects\(`), + httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(` { "data": { "repository": { "projects": { "nodes": [ @@ -125,7 +125,7 @@ func TestPRCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\borganization\(.+\bprojects\(`), + httpmock.GraphQL(`query OrganizationProjectList\b`), httpmock.StringResponse(` { "data": { "organization": { "projects": { "nodes": [], @@ -133,7 +133,7 @@ func TestPRCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\bcreatePullRequest\(`), + httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "id": "NEWPULLID", @@ -150,7 +150,7 @@ func TestPRCreate_metadata(t *testing.T) { } })) http.Register( - httpmock.GraphQL(`\bupdatePullRequest\(`), + httpmock.GraphQL(`mutation PullRequestCreateMetadata\b`), httpmock.GraphQLMutation(` { "data": { "updatePullRequest": { "clientMutationId": "" @@ -163,7 +163,7 @@ func TestPRCreate_metadata(t *testing.T) { eq(t, inputs["milestoneId"], "BIGONEID") })) http.Register( - httpmock.GraphQL(`\brequestReviews\(`), + httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`), httpmock.GraphQLMutation(` { "data": { "requestReviews": { "clientMutationId": "" @@ -534,16 +534,16 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { initBlankContext("", "OWNER/REPO", "feature") http := initFakeHTTP() 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(` + http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) + http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` { "data": { "repository": { "forks": { "nodes": [ ] } } } } `)) - http.Register(httpmock.GraphQL(`\bpullRequests\(`), httpmock.StringResponse(` + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } `)) - http.Register(httpmock.GraphQL(`\bcreatePullRequest\(`), httpmock.GraphQLMutation(` + http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } diff --git a/command/pr_test.go b/command/pr_test.go index bac0e38d4..8285b9fca 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -2,10 +2,6 @@ package command import ( "bytes" - "encoding/json" - "io" - "io/ioutil" - "os" "os/exec" "reflect" "regexp" @@ -14,8 +10,10 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -29,10 +27,7 @@ func TestPRStatus(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatus.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatus.json")) output, err := RunCommand("pr status") if err != nil { @@ -57,10 +52,7 @@ func TestPRStatus_fork(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatusFork.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusFork.json")) defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { @@ -87,10 +79,7 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusChecks.json")) output, err := RunCommand("pr status") if err != nil { @@ -114,10 +103,7 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) output, err := RunCommand("pr status") if err != nil { @@ -146,10 +132,7 @@ 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) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) output, err := RunCommand("pr status") if err != nil { @@ -166,10 +149,7 @@ func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) output, err := RunCommand("pr status -R OWNER/REPO") if err != nil { @@ -187,10 +167,7 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosed.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosed.json")) output, err := RunCommand("pr status") if err != nil { @@ -208,10 +185,7 @@ func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) output, err := RunCommand("pr status") if err != nil { @@ -229,10 +203,7 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMerged.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMerged.json")) output, err := RunCommand("pr status") if err != nil { @@ -250,10 +221,7 @@ func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json")) output, err := RunCommand("pr status") if err != nil { @@ -271,10 +239,7 @@ func TestPRStatus_blankSlate(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": {} } - `)) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) output, err := RunCommand("pr status") if err != nil { @@ -303,10 +268,7 @@ func TestPRStatus_detachedHead(t *testing.T) { initBlankContext("", "OWNER/REPO", "") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": {} } - `)) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) output, err := RunCommand("pr status") if err != nil { @@ -335,10 +297,7 @@ func TestPRList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prList.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("../test/fixtures/prList.json")) output, err := RunCommand("pr list") if err != nil { @@ -359,9 +318,12 @@ func TestPRList_filtering(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - respBody := bytes.NewBufferString(`{ "data": {} }`) - http.StubResponse(200, respBody) + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{})) + assert.Equal(t, []interface{}{"one", "two", "three"}, params["labels"].([]interface{})) + })) output, err := RunCommand(`pr list -s all -l one,two -l three`) if err != nil { @@ -373,28 +335,15 @@ func TestPRList_filtering(t *testing.T) { No pull requests match your search in OWNER/REPO `) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables struct { - State []string - Labels []string - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.State, []string{"OPEN", "CLOSED", "MERGED"}) - eq(t, reqBody.Variables.Labels, []string{"one", "two", "three"}) } func TestPRList_filteringRemoveDuplicate(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prListWithDuplicates.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.FileResponse("../test/fixtures/prListWithDuplicates.json")) output, err := RunCommand("pr list -l one,two") if err != nil { @@ -411,57 +360,37 @@ func TestPRList_filteringClosed(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - respBody := bytes.NewBufferString(`{ "data": {} }`) - http.StubResponse(200, respBody) + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, []interface{}{"CLOSED", "MERGED"}, params["state"].([]interface{})) + })) _, err := RunCommand(`pr list -s closed`) if err != nil { t.Fatal(err) } - - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables struct { - State []string - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.State, []string{"CLOSED", "MERGED"}) } func TestPRList_filteringAssignee(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - respBody := bytes.NewBufferString(`{ "data": {} }`) - http.StubResponse(200, respBody) + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`, params["q"].(string)) + })) _, err := RunCommand(`pr list -s merged -l "needs tests" -a hubot -B develop`) if err != nil { t.Fatal(err) } - - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables struct { - Q string - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Q, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`) } func TestPRList_filteringAssigneeLabels(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - respBody := bytes.NewBufferString(`{ "data": {} }`) - http.StubResponse(200, respBody) + initFakeHTTP() _, err := RunCommand(`pr list -l one,two -a hubot`) if err == nil && err.Error() != "multiple labels with --assignee are not supported" { @@ -471,8 +400,7 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { func TestPRList_withInvalidLimitFlag(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") + initFakeHTTP() _, err := RunCommand(`pr list --limit=0`) if err == nil && err.Error() != "invalid limit: 0" { @@ -639,10 +567,7 @@ func TestPRView_Preview(t *testing.T) { initBlankContext("", "OWNER/REPO", tc.ownerRepo) http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open(tc.fixture) - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) output, err := RunCommand(tc.args) if err != nil { @@ -660,10 +585,7 @@ func TestPRView_web_currentBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prView.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView.json")) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -698,10 +620,7 @@ func TestPRView_web_noResultsForBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView_NoActiveBranch.json")) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -924,7 +843,7 @@ func TestPrClose(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 96 } + "pullRequest": { "number": 96, "title": "The title of the PR" } } } } `)) @@ -935,7 +854,7 @@ func TestPrClose(t *testing.T) { t.Fatalf("error running command `pr close`: %v", err) } - r := regexp.MustCompile(`Closed pull request #96`) + r := regexp.MustCompile(`Closed pull request #96 \(The title of the PR\)`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -949,7 +868,7 @@ func TestPrClose_alreadyClosed(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 101, "closed": true } + "pullRequest": { "number": 101, "title": "The title of the PR", "closed": true } } } } `)) @@ -960,7 +879,7 @@ func TestPrClose_alreadyClosed(t *testing.T) { t.Fatalf("error running command `pr close`: %v", err) } - r := regexp.MustCompile(`Pull request #101 is already closed`) + r := regexp.MustCompile(`Pull request #101 \(The title of the PR\) is already closed`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -974,7 +893,7 @@ func TestPRReopen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 666, "closed": true} + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true} } } } `)) @@ -985,7 +904,7 @@ func TestPRReopen(t *testing.T) { t.Fatalf("error running command `pr reopen`: %v", err) } - r := regexp.MustCompile(`Reopened pull request #666`) + r := regexp.MustCompile(`Reopened pull request #666 \(The title of the PR\)`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -999,7 +918,7 @@ func TestPRReopen_alreadyOpen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 666, "closed": false} + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": false} } } } `)) @@ -1010,7 +929,7 @@ func TestPRReopen_alreadyOpen(t *testing.T) { t.Fatalf("error running command `pr reopen`: %v", err) } - r := regexp.MustCompile(`Pull request #666 is already open`) + r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) is already open`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -1024,7 +943,7 @@ func TestPRReopen_alreadyMerged(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 666, "closed": true, "state": "MERGED"} + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true, "state": "MERGED"} } } } `)) @@ -1035,36 +954,41 @@ func TestPRReopen_alreadyMerged(t *testing.T) { 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`) + r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) 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()) } } -type stubResponse struct { - ResponseCode int - ResponseBody io.Reader -} - -func initWithStubs(branch string, stubs ...stubResponse) { - initBlankContext("", "OWNER/REPO", branch) +func TestPrMerge(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - for _, s := range stubs { - http.StubResponse(s.ResponseCode, s.ResponseBody) - } - http.StubRepoResponse("OWNER", "REPO") -} - -func TestPrMerge(t *testing.T) { - initWithStubs("master", - stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} - } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} + } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1080,22 +1004,43 @@ func TestPrMerge(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Merged pull request #1`) + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) } } func TestPrMerge_withRepoFlag(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} - } } }`)) - http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`)) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.GraphQLQuery(` + { "data": { "repository": { + "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} + } } }`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "stinky", params["owner"].(string)) + assert.Equal(t, "boi", params["repo"].(string)) + })) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1107,22 +1052,38 @@ func TestPrMerge_withRepoFlag(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Merged pull request #1`) + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) } } func TestPrMerge_deleteBranch(t *testing.T) { - initWithStubs("blueberries", - stubResponse{200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", "id": "THE-ID", "number": 3} - ] } } } }`)}, - stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1138,20 +1099,34 @@ func TestPrMerge_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted branch blueberries") + test.ExpectLines(t, output.String(), `Merged pull request #10 \(Blueberries are a good fruit\)`, "Deleted branch blueberries") } func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "another-branch") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", "id": "THE-ID", "number": 3} - ] } } } }`)) - http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`)) - http.StubResponse(200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)) - http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1165,10 +1140,35 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted branch blueberries") + test.ExpectLines(t, output.String(), `Merged pull request #10 \(Blueberries are a good fruit\)`, "Deleted branch blueberries") } func TestPrMerge_noPrNumberGiven(t *testing.T) { + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1178,35 +1178,46 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { cs.Stub("") // git checkout master cs.Stub("") // git branch -d - jsonFile, _ := os.Open("../test/fixtures/prViewPreviewWithMetadataByBranch.json") - defer jsonFile.Close() - - initWithStubs("blueberries", - stubResponse{200, jsonFile}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) - output, err := RunCommand("pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Merged pull request #10`) + r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) } } func TestPrMerge_rebase(t *testing.T) { - initWithStubs("master", - stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 2, "closed": false, "state": "OPEN"} - } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 2, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} + } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "REBASE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1221,21 +1232,41 @@ func TestPrMerge_rebase(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Rebased and merged pull request #2`) + r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`) if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) } } func TestPrMerge_squash(t *testing.T) { - initWithStubs("master", - stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 3, "closed": false, "state": "OPEN"} - } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 3, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} + } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "SQUASH", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1253,18 +1284,20 @@ func TestPrMerge_squash(t *testing.T) { r := regexp.MustCompile(`Squashed and merged pull request #3`) if !r.MatchString(output.String()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String()) } } func TestPrMerge_alreadyMerged(t *testing.T) { - initWithStubs("master", - stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 4, "closed": true, "state": "MERGED"} - } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - ) + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"} + } } }`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1279,7 +1312,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) { t.Fatalf("expected an error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Pull request #4 was already merged`) + r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`) if !r.MatchString(err.Error()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) @@ -1287,13 +1320,36 @@ func TestPrMerge_alreadyMerged(t *testing.T) { } func TestPRMerge_interactive(t *testing.T) { - initWithStubs("blueberries", - stubResponse{200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", "headRepositoryOwner": {"login": "OWNER"}, "id": "THE-ID", "number": 3} - ] } } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, - stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)}) + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "THE-ID", + "number": 3 + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{ + "data": { + "repository": { + "defaultBranchRef": {"name": "master"} + } + } + }`)) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -1327,12 +1383,7 @@ func TestPRMerge_interactive(t *testing.T) { } func TestPrMerge_multipleMergeMethods(t *testing.T) { - initWithStubs("master", - stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} - } } }`)}, - stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, - ) + initBlankContext("", "OWNER/REPO", "master") _, err := RunCommand("pr merge 1 --merge --squash") if err == nil { diff --git a/command/repo_test.go b/command/repo_test.go index 52928c8f4..56c52fa31 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -459,7 +459,7 @@ func TestRepoClone(t *testing.T) { t.Run(tt.name, func(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.GraphQL(`\brepository\(`), + httpmock.GraphQL(`query RepositoryFindParent\b`), httpmock.StringResponse(` { "data": { "repository": { "parent": null @@ -487,7 +487,7 @@ func TestRepoClone(t *testing.T) { func TestRepoClone_hasParent(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.GraphQL(`\brepository\(`), + httpmock.GraphQL(`query RepositoryFindParent\b`), httpmock.StringResponse(` { "data": { "repository": { "parent": { @@ -515,13 +515,13 @@ func TestRepoClone_hasParent(t *testing.T) { func TestRepo_withoutUsername(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.GraphQL(`\bviewer\b`), + httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(` { "data": { "viewer": { "login": "OWNER" }}}`)) http.Register( - httpmock.GraphQL(`\brepository\(`), + httpmock.GraphQL(`query RepositoryFindParent\b`), httpmock.StringResponse(` { "data": { "repository": { "parent": null @@ -552,7 +552,7 @@ func TestRepoCreate(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.GraphQL(`mutation RepositoryCreate\b`), httpmock.StringResponse(` { "data": { "createRepository": { "repository": { @@ -618,12 +618,12 @@ func TestRepoCreate_org(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.MatchAny, + httpmock.REST("GET", "users/ORG"), httpmock.StringResponse(` { "node_id": "ORGID" }`)) http.Register( - httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.GraphQL(`mutation RepositoryCreate\b`), httpmock.StringResponse(` { "data": { "createRepository": { "repository": { @@ -688,13 +688,13 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { http := initFakeHTTP() http.Register( - httpmock.MatchAny, + httpmock.REST("GET", "orgs/ORG/teams/monkeys"), httpmock.StringResponse(` { "node_id": "TEAMID", "organization": { "node_id": "ORGID" } }`)) http.Register( - httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.GraphQL(`mutation RepositoryCreate\b`), httpmock.StringResponse(` { "data": { "createRepository": { "repository": { @@ -755,7 +755,7 @@ func TestRepoView_web(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.MatchAny, + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { }`)) @@ -789,7 +789,7 @@ func TestRepoView_web_ownerRepo(t *testing.T) { } http := initFakeHTTP() http.Register( - httpmock.MatchAny, + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { }`)) @@ -823,7 +823,7 @@ func TestRepoView_web_fullURL(t *testing.T) { } http := initFakeHTTP() http.Register( - httpmock.MatchAny, + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { }`)) var seenCmd *exec.Cmd @@ -853,14 +853,14 @@ func TestRepoView(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.GraphQL(`\brepository\(`), + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "description": "social distancing" } } }`)) http.Register( - httpmock.MatchAny, + httpmock.REST("GET", "repos/OWNER/REPO/readme"), httpmock.StringResponse(` { "name": "readme.md", "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`)) @@ -883,14 +883,14 @@ func TestRepoView_nonmarkdown_readme(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") http.Register( - httpmock.GraphQL(`\brepository\(`), + httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "description": "social distancing" } } }`)) http.Register( - httpmock.MatchAny, + httpmock.REST("GET", "repos/OWNER/REPO/readme"), httpmock.StringResponse(` { "name": "readme.org", "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`)) @@ -911,8 +911,7 @@ func TestRepoView_blanks(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.MatchAny, httpmock.StringResponse("{}")) - http.Register(httpmock.MatchAny, httpmock.StringResponse("{}")) + http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse("{}")) output, err := RunCommand("repo view") if err != nil { diff --git a/command/root.go b/command/root.go index 693603e05..3693fd6f0 100644 --- a/command/root.go +++ b/command/root.go @@ -52,7 +52,6 @@ func init() { RootCmd.AddCommand(versionCmd) RootCmd.SetVersionTemplate(versionOutput) - RootCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") RootCmd.PersistentFlags().Bool("help", false, "Show help for command") RootCmd.Flags().Bool("version", false, "Show gh version") // TODO: @@ -258,8 +257,8 @@ var ensureScopes = func(ctx context.Context, client *api.Client, wantedScopes .. } return reloadedClient, nil } else { - fmt.Fprintln(os.Stderr, fmt.Sprintf("Warning: gh now requires %s OAuth scopes.", wantedScopes)) - fmt.Fprintln(os.Stderr, fmt.Sprintf("Visit https://github.com/settings/tokens and edit your token to enable %s", wantedScopes)) + fmt.Fprintf(os.Stderr, "Warning: gh now requires %s OAuth scopes.\n", wantedScopes) + fmt.Fprintf(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable %s\n", wantedScopes) if tokenFromEnv { fmt.Fprintln(os.Stderr, "or generate a new token for the GITHUB_TOKEN environment variable") } else { @@ -304,8 +303,8 @@ func changelogURL(version string) string { } func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) { - repo, err := cmd.Flags().GetString("repo") - if err == nil && repo != "" { + repo, _ := cmd.Flags().GetString("repo") + if repo != "" { baseRepo, err := ghrepo.FromFullName(repo) if err != nil { return nil, fmt.Errorf("argument error: %w", err) @@ -313,17 +312,12 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co return baseRepo, nil } - baseOverride, err := cmd.Flags().GetString("repo") - if err != nil { - return nil, err - } - remotes, err := ctx.Remotes() if err != nil { return nil, err } - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") if err != nil { return nil, err } diff --git a/docs/source.md b/docs/source.md index bbeac2025..0c4e3db63 100644 --- a/docs/source.md +++ b/docs/source.md @@ -1,6 +1,6 @@ # Installation from source -0. Verify that you have Go 1.14+ installed +0. Verify that you have Go 1.13.8+ installed ```sh $ go version diff --git a/go.mod b/go.mod index 6857e1947..c6de66d49 100644 --- a/go.mod +++ b/go.mod @@ -18,14 +18,15 @@ require ( github.com/mattn/go-runewidth v0.0.9 // indirect github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b github.com/mitchellh/go-homedir v1.1.0 - github.com/shurcooL/githubv4 v0.0.0-20200414012201-bbc966b061dd - github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f // indirect + github.com/shurcooL/githubv4 v0.0.0-20200627185320-e003124d66e4 + github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.5.1 - golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 - golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2 // indirect + golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 golang.org/x/text v0.3.2 gopkg.in/yaml.v2 v2.2.8 // indirect gopkg.in/yaml.v3 v3.0.0-20200506231410-2ff61e1afc86 ) + +replace github.com/shurcooL/graphql => github.com/cli/shurcooL-graphql v0.0.0-20200707151639-0f7232a2bf7e diff --git a/go.sum b/go.sum index 42f30698a..ed615c5b6 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,8 @@ github.com/briandowns/spinner v1.11.1/go.mod h1:QOuQk7x+EaDASo80FEXwlwiA+j/PPIcX github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058 h1:Ks+RZ6s6UriHnL+yusm3OoaLwpV9WPvMV+FXQ6qMD7M= github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058/go.mod h1:sC1EP6T+3nFnl5vwf0TYEs1inMigQxZ7n912YKoxJow= +github.com/cli/shurcooL-graphql v0.0.0-20200707151639-0f7232a2bf7e h1:aq/1jlmtZoS6nlSp3yLOTZQ50G+dzHdeRNENgE/iBew= +github.com/cli/shurcooL-graphql v0.0.0-20200707151639-0f7232a2bf7e/go.mod h1:it23pLwxmz6OyM6I5O0ATIXQS1S190Nas26L5Kahp4U= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= @@ -72,6 +74,7 @@ github.com/google/goterm v0.0.0-20190703233501-fc88cf888a3f/go.mod h1:nOFQdrUlIl github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= +github.com/graph-gophers/graphql-go v0.0.0-20200622220639-c1d9693c95a6/go.mod h1:9CQHMSxwO4MprSdzoIEobiHpoLtHm77vfxsvsIN5Vuc= github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= @@ -131,6 +134,7 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.4 h1:vHD/YYe1Wolo78koG299f7V/VAS08c6IpCLn+Ejf/w8= github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA= +github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= @@ -151,10 +155,8 @@ github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= -github.com/shurcooL/githubv4 v0.0.0-20200414012201-bbc966b061dd h1:EwtC+kDj8s9OKiaStPZtTv3neldOyr98AXIxvmn3Gss= -github.com/shurcooL/githubv4 v0.0.0-20200414012201-bbc966b061dd/go.mod h1:hAF0iLZy4td2EX+/8Tw+4nodhlMrwN3HupfaXj3zkGo= -github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f h1:tygelZueB1EtXkPI6mQ4o9DQ0+FKW41hTbunoXZCTqk= -github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f/go.mod h1:AuYgA5Kyo4c7HfUmvRGs/6rGlMMV/6B1bVnB9JxJEEg= +github.com/shurcooL/githubv4 v0.0.0-20200627185320-e003124d66e4 h1:cjmR6xY0f89IwBYMSwUrkFs4/1+KKw30Df3SqT7nZ6Q= +github.com/shurcooL/githubv4 v0.0.0-20200627185320-e003124d66e4/go.mod h1:hAF0iLZy4td2EX+/8Tw+4nodhlMrwN3HupfaXj3zkGo= github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= @@ -193,8 +195,8 @@ golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnf golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 h1:8dUaAV7K4uHsF56JQWkprecIQKdPHtR9jCHF5nB8uzc= golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 h1:cg5LA/zNPRzIXIWSCxQW10Rvpy94aQh3LT/ShoCpkHw= -golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -204,8 +206,8 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2 h1:eDrdRpKgkcCqKZQwyZRyeFZgfqt37SL7Kv3tok06cKE= -golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= +golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU= +golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 49a2770d3..2f97d2956 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "os" "path" + "syscall" "github.com/mitchellh/go-homedir" "gopkg.in/yaml.v3" @@ -32,7 +33,7 @@ func ParseDefaultConfig() (Config, error) { var ReadConfigFile = func(filename string) ([]byte, error) { f, err := os.Open(filename) if err != nil { - return nil, err + return nil, pathError(err) } defer f.Close() @@ -47,7 +48,7 @@ var ReadConfigFile = func(filename string) ([]byte, error) { var WriteConfigFile = func(filename string, data []byte) error { err := os.MkdirAll(path.Dir(filename), 0771) if err != nil { - return err + return pathError(err) } cfgFile, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) // cargo coded from setup @@ -138,7 +139,11 @@ func migrateConfig(filename string) error { func ParseConfig(filename string) (Config, error) { _, root, err := parseConfigFile(filename) if err != nil { - return nil, err + if os.IsNotExist(err) { + root = NewBlankRoot() + } else { + return nil, err + } } if isLegacy(root) { @@ -168,3 +173,28 @@ func ParseConfig(filename string) (Config, error) { return NewConfig(root), nil } + +func pathError(err error) error { + var pathError *os.PathError + if errors.As(err, &pathError) && errors.Is(pathError.Err, syscall.ENOTDIR) { + if p := findRegularFile(pathError.Path); p != "" { + return fmt.Errorf("remove or rename regular file `%s` (must be a directory)", p) + } + + } + return err +} + +func findRegularFile(p string) string { + for { + if s, err := os.Stat(p); err == nil && s.Mode().IsRegular() { + return p + } + newPath := path.Dir(p) + if newPath == p || newPath == "/" || newPath == "." { + break + } + p = newPath + } + return "" +} diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index d19130bb5..f921d2c67 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -110,14 +110,36 @@ github.com: } func Test_parseConfigFile(t *testing.T) { - fileContents := []string{"", " ", "\n"} - for _, contents := range fileContents { - t.Run(fmt.Sprintf("contents: %q", contents), func(t *testing.T) { - defer StubConfig(contents, "")() + tests := []struct { + contents string + wantsErr bool + }{ + { + contents: "", + wantsErr: true, + }, + { + contents: " ", + wantsErr: false, + }, + { + contents: "\n", + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("contents: %q", tt.contents), func(t *testing.T) { + defer StubConfig(tt.contents, "")() _, yamlRoot, err := parseConfigFile("config.yml") - eq(t, err, nil) - eq(t, yamlRoot.Content[0].Kind, yaml.MappingNode) - eq(t, len(yamlRoot.Content[0].Content), 0) + if tt.wantsErr != (err != nil) { + t.Fatalf("got error: %v", err) + } + if tt.wantsErr { + return + } + assert.Equal(t, yaml.MappingNode, yamlRoot.Content[0].Kind) + assert.Equal(t, 0, len(yamlRoot.Content[0].Content)) }) } } diff --git a/internal/config/config_type.go b/internal/config/config_type.go index a57d21dec..85e5c0d4a 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -123,7 +123,11 @@ func NewConfig(root *yaml.Node) Config { } func NewBlankConfig() Config { - return NewConfig(&yaml.Node{ + return NewConfig(NewBlankRoot()) +} + +func NewBlankRoot() *yaml.Node { + return &yaml.Node{ Kind: yaml.DocumentNode, Content: []*yaml.Node{ { @@ -168,7 +172,7 @@ func NewBlankConfig() Config { }, }, }, - }) + } } // This type implements a Config interface and represents a config file on disk. diff --git a/internal/config/testing.go b/internal/config/testing.go index 59c2ff212..a49178705 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -42,7 +42,11 @@ func StubConfig(main, hosts string) func() { ReadConfigFile = func(fn string) ([]byte, error) { switch path.Base(fn) { case "config.yml": - return []byte(main), nil + if main == "" { + return []byte(nil), os.ErrNotExist + } else { + return []byte(main), nil + } case "hosts.yml": if hosts == "" { return []byte(nil), os.ErrNotExist diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index e1edabbc0..170c41b55 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -34,6 +34,7 @@ type ApiOptions struct { RequestHeaders []string ShowResponseHeaders bool Paginate bool + Silent bool HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) @@ -134,6 +135,7 @@ original query accepts an '$endCursor: String' variable and that it fetches the cmd.Flags().BoolVarP(&opts.ShowResponseHeaders, "include", "i", false, "Include HTTP response headers in the output") cmd.Flags().BoolVar(&opts.Paginate, "paginate", false, "Make additional HTTP requests to fetch all pages of results") cmd.Flags().StringVar(&opts.RequestInputFile, "input", "", "The file to use as body for the HTTP request") + cmd.Flags().BoolVar(&opts.Silent, "silent", false, "Do not print the response body") return cmd } @@ -178,6 +180,11 @@ func apiRun(opts *ApiOptions) error { return err } + headersOutputStream := opts.IO.Out + if opts.Silent { + opts.IO.Out = ioutil.Discard + } + hasNextPage := true for hasNextPage { resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) @@ -185,7 +192,7 @@ func apiRun(opts *ApiOptions) error { return err } - endCursor, err := processResponse(resp, opts) + endCursor, err := processResponse(resp, opts, headersOutputStream) if err != nil { return err } @@ -211,11 +218,11 @@ func apiRun(opts *ApiOptions) error { return nil } -func processResponse(resp *http.Response, opts *ApiOptions) (endCursor string, err error) { +func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream io.Writer) (endCursor string, err error) { if opts.ShowResponseHeaders { - fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) - printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) - fmt.Fprint(opts.IO.Out, "\r\n") + fmt.Fprintln(headersOutputStream, resp.Proto, resp.Status) + printHeaders(headersOutputStream, resp.Header, opts.IO.ColorEnabled()) + fmt.Fprint(headersOutputStream, "\r\n") } if resp.StatusCode == 204 { diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 7b89d715c..f590d93b4 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -39,6 +39,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: false, + Silent: false, }, wantsErr: false, }, @@ -55,6 +56,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: false, + Silent: false, }, wantsErr: false, }, @@ -71,6 +73,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: false, + Silent: false, }, wantsErr: false, }, @@ -87,6 +90,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string{"accept: text/plain"}, ShowResponseHeaders: true, Paginate: false, + Silent: false, }, wantsErr: false, }, @@ -103,6 +107,24 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: true, + Silent: false, + }, + wantsErr: false, + }, + { + name: "with silenced output", + cli: "repos/OWNER/REPO/issues --silent", + wants: ApiOptions{ + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "repos/OWNER/REPO/issues", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: false, + Silent: true, }, wantsErr: false, }, @@ -124,6 +146,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: true, + Silent: false, }, wantsErr: false, }, @@ -145,6 +168,7 @@ func Test_NewCmdApi(t *testing.T) { RequestHeaders: []string(nil), ShowResponseHeaders: false, Paginate: false, + Silent: false, }, wantsErr: false, }, @@ -264,6 +288,36 @@ func Test_apiRun(t *testing.T) { stdout: `gateway timeout`, stderr: "gh: HTTP 502\n", }, + { + name: "silent", + options: ApiOptions{ + Silent: true, + }, + httpResponse: &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`body`)), + }, + err: nil, + stdout: ``, + stderr: ``, + }, + { + name: "show response headers even when silent", + options: ApiOptions{ + ShowResponseHeaders: true, + Silent: true, + }, + httpResponse: &http.Response{ + Proto: "HTTP/1.1", + Status: "200 Okey-dokey", + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`body`)), + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + err: nil, + stdout: "HTTP/1.1 200 Okey-dokey\nContent-Type: text/plain\r\n\r\n", + stderr: ``, + }, } for _, tt := range tests { diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index 9474c3dd8..2c69be824 100644 --- a/pkg/httpmock/legacy.go +++ b/pkg/httpmock/legacy.go @@ -12,19 +12,19 @@ import ( // 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 + r.Register(MatchAny, func(req *http.Request) (*http.Response, error) { + return httpResponse(status, req, 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) { + r.Register(MatchAny, func(req *http.Request) (*http.Response, error) { if err != nil { return nil, err } - return httpResponse(200, fixtureFile), nil + return httpResponse(200, req, fixtureFile), nil }) return func() { if err == nil { @@ -38,15 +38,15 @@ func (r *Registry) StubRepoResponse(owner, repo string) { } func (r *Registry) StubRepoResponseWithPermission(owner, repo, permission string) { - r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, "master", permission))) + r.Register(GraphQL(`query RepositoryNetwork\b`), StringResponse(RepoNetworkStubResponse(owner, repo, "master", permission))) } func (r *Registry) StubRepoResponseWithDefaultBranch(owner, repo, defaultBranch string) { - r.Register(MatchAny, StringResponse(RepoNetworkStubResponse(owner, repo, defaultBranch, "WRITE"))) + r.Register(GraphQL(`query RepositoryNetwork\b`), StringResponse(RepoNetworkStubResponse(owner, repo, defaultBranch, "WRITE"))) } func (r *Registry) StubForkedRepoResponse(ownRepo, parentRepo string) { - r.Register(MatchAny, StringResponse(RepoNetworkStubForkResponse(ownRepo, parentRepo))) + r.Register(GraphQL(`query RepositoryNetwork\b`), StringResponse(RepoNetworkStubForkResponse(ownRepo, parentRepo))) } func RepoNetworkStubResponse(owner, repo, defaultBranch, permission string) string { diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 48077ed4e..0a25beac8 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" "net/http" + "os" "regexp" "strings" ) @@ -23,6 +24,18 @@ func MatchAny(*http.Request) bool { return true } +func REST(method, p string) Matcher { + return func(req *http.Request) bool { + if !strings.EqualFold(req.Method, method) { + return false + } + if req.URL.Path != "/"+p { + return false + } + return true + } +} + func GraphQL(q string) Matcher { re := regexp.MustCompile(q) @@ -59,15 +72,31 @@ func decodeJSONBody(req *http.Request, dest interface{}) error { } func StringResponse(body string) Responder { - return func(*http.Request) (*http.Response, error) { - return httpResponse(200, bytes.NewBufferString(body)), nil + return func(req *http.Request) (*http.Response, error) { + return httpResponse(200, req, bytes.NewBufferString(body)), nil + } +} + +func StatusStringResponse(status int, body string) Responder { + return func(req *http.Request) (*http.Response, error) { + return httpResponse(status, req, bytes.NewBufferString(body)), nil } } func JSONResponse(body interface{}) Responder { - return func(*http.Request) (*http.Response, error) { + return func(req *http.Request) (*http.Response, error) { b, _ := json.Marshal(body) - return httpResponse(200, bytes.NewBuffer(b)), nil + return httpResponse(200, req, bytes.NewBuffer(b)), nil + } +} + +func FileResponse(filename string) Responder { + return func(req *http.Request) (*http.Response, error) { + f, err := os.Open(filename) + if err != nil { + return nil, err + } + return httpResponse(200, req, f), nil } } @@ -84,7 +113,7 @@ func GraphQLMutation(body string, cb func(map[string]interface{})) Responder { } cb(bodyData.Variables.Input) - return httpResponse(200, bytes.NewBufferString(body)), nil + return httpResponse(200, req, bytes.NewBufferString(body)), nil } } @@ -100,13 +129,14 @@ func GraphQLQuery(body string, cb func(string, map[string]interface{})) Responde } cb(bodyData.Query, bodyData.Variables) - return httpResponse(200, bytes.NewBufferString(body)), nil + return httpResponse(200, req, bytes.NewBufferString(body)), nil } } -func httpResponse(status int, body io.Reader) *http.Response { +func httpResponse(status int, req *http.Request, body io.Reader) *http.Response { return &http.Response{ StatusCode: status, + Request: req, Body: ioutil.NopCloser(body), } } diff --git a/test/fixtures/prViewPreviewWithMetadataByBranch.json b/test/fixtures/prViewPreviewWithMetadataByBranch.json index aaf9c6dfa..73a64ca1c 100644 --- a/test/fixtures/prViewPreviewWithMetadataByBranch.json +++ b/test/fixtures/prViewPreviewWithMetadataByBranch.json @@ -4,6 +4,7 @@ "pullRequests": { "nodes": [ { + "id": "PR_12", "number": 12, "title": "Blueberries are from a fork", "state": "OPEN", @@ -37,6 +38,7 @@ "isDraft": false }, { + "id": "PR_10", "number": 10, "title": "Blueberries are a good fruit", "state": "OPEN", @@ -123,4 +125,4 @@ } } } -} +} \ No newline at end of file diff --git a/utils/utils.go b/utils/utils.go index c84860d9e..885e5ea10 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -26,6 +26,9 @@ func RenderMarkdown(text string) (string, error) { if isColorEnabled() { style = "dark" } + // Glamour rendering preserves carriage return characters in code blocks, but + // we need to ensure that no such characters are present in the output. + text = strings.ReplaceAll(text, "\r\n", "\n") return glamour.Render(text, style) }