From 2943703d4a0a35855476ee8d884b6ef6bbab3cce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Thu, 12 Mar 2020 21:40:11 -0600 Subject: [PATCH 01/48] Add mentioned flag --- api/queries_issue.go | 7 +++++-- api/queries_issue_test.go | 2 +- command/issue.go | 8 +++++++- command/issue_test.go | 12 +++++++----- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 56a04edd9..3da827ae1 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -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, mentionedString string) (*IssuesAndTotalCount, error) { var states []string switch state { case "open", "": @@ -215,7 +215,7 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: 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: $mentioned}) { totalCount nodes { ...issue @@ -243,6 +243,9 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str if authorString != "" { variables["author"] = authorString } + if mentionedString != "" { + variables["mentioned"] = mentionedString + } var response struct { Repository struct { diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index ffe4aaad1..4eff67804 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/command/issue.go b/command/issue.go index 526d02871..85e0696a6 100644 --- a/command/issue.go +++ b/command/issue.go @@ -41,6 +41,7 @@ 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().StringP("mentioned", "", "", "Filter by mention") issueCmd.AddCommand(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") @@ -141,7 +142,12 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author) + mentioned, err := cmd.Flags().GetString("mentioned") + if err != nil { + return err + } + + listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mentioned) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index 418de25d3..ce91c98e2 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -153,7 +153,7 @@ func TestIssueList_withFlags(t *testing.T) { } } } `)) - 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 --mentioned me") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -167,10 +167,11 @@ 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 + Assignee string + Labels []string + States []string + Author string + Mentioned string } }{} _ = json.Unmarshal(bodyBytes, &reqBody) @@ -179,6 +180,7 @@ No issues match your search in OWNER/REPO eq(t, reqBody.Variables.Labels, []string{"web", "bug"}) eq(t, reqBody.Variables.States, []string{"OPEN"}) eq(t, reqBody.Variables.Author, "foo") + eq(t, reqBody.Variables.Mentioned, "me") } func TestIssueList_withInvalidLimitFlag(t *testing.T) { From 9aebb66a3c00ccec2d0af6db968a3ddf37e3df94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Thu, 12 Mar 2020 21:53:45 -0600 Subject: [PATCH 02/48] Add milestone filter --- api/queries_issue.go | 7 +++++-- api/queries_issue_test.go | 2 +- command/issue.go | 8 +++++++- command/issue_test.go | 4 +++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 3da827ae1..e43fb8ea3 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -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, mentionedString string) (*IssuesAndTotalCount, error) { +func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionedString string, milestoneString string) (*IssuesAndTotalCount, error) { var states []string switch state { case "open", "": @@ -215,7 +215,7 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: 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, mentioned: $mentioned}) { + issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mentioned, milestone: $milestone}) { totalCount nodes { ...issue @@ -246,6 +246,9 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str if mentionedString != "" { variables["mentioned"] = mentionedString } + if milestoneString != "" { + variables["milestone"] = milestoneString + } var response struct { Repository struct { diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index 4eff67804..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/command/issue.go b/command/issue.go index 85e0696a6..1b4befd67 100644 --- a/command/issue.go +++ b/command/issue.go @@ -42,6 +42,7 @@ func init() { issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") issueListCmd.Flags().StringP("author", "A", "", "Filter by author") issueListCmd.Flags().StringP("mentioned", "", "", "Filter by mention") + issueListCmd.Flags().StringP("milestone", "", "", "Filter by milestone") issueCmd.AddCommand(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") @@ -147,7 +148,12 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mentioned) + milestone, err := cmd.Flags().GetString("milestone") + if err != nil { + return err + } + + listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mentioned, milestone) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index ce91c98e2..b834f26c7 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -153,7 +153,7 @@ func TestIssueList_withFlags(t *testing.T) { } } } `)) - output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mentioned me") + output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mentioned me --milestone 1.x") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -172,6 +172,7 @@ No issues match your search in OWNER/REPO States []string Author string Mentioned string + Milestone string } }{} _ = json.Unmarshal(bodyBytes, &reqBody) @@ -181,6 +182,7 @@ No issues match your search in OWNER/REPO eq(t, reqBody.Variables.States, []string{"OPEN"}) eq(t, reqBody.Variables.Author, "foo") eq(t, reqBody.Variables.Mentioned, "me") + eq(t, reqBody.Variables.Milestone, "1.x") } func TestIssueList_withInvalidLimitFlag(t *testing.T) { From a16405650f7b99e42e99938ced5d81247edd1a49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Sat, 13 Jun 2020 20:55:55 -0500 Subject: [PATCH 03/48] Define query variables --- api/queries_issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index e43fb8ea3..55d7515f0 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -212,7 +212,7 @@ 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($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mentioned: 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, mentioned: $mentioned, milestone: $milestone}) { From 8a96299735711353a7ae9da9882123650a82341f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Sat, 13 Jun 2020 21:07:12 -0500 Subject: [PATCH 04/48] Fix lint --- command/issue_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index b834f26c7..19ac94d01 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -167,12 +167,12 @@ 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 - Mentioned string - Milestone string + Assignee string + Labels []string + States []string + Author string + Mentioned string + Milestone string } }{} _ = json.Unmarshal(bodyBytes, &reqBody) From cffd56f717570a33ca61e44348ae3319ebbfe91e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Mon, 15 Jun 2020 15:58:07 -0500 Subject: [PATCH 05/48] Rename to mention --- api/queries_issue.go | 10 +++++----- command/issue.go | 6 +++--- command/issue_test.go | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 55d7515f0..76981f8d7 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -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, mentionedString string, milestoneString 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, $mentioned: String, $milestone: String) { + query($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, mentioned: $mentioned, milestone: $milestone}) { + 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,8 +243,8 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str if authorString != "" { variables["author"] = authorString } - if mentionedString != "" { - variables["mentioned"] = mentionedString + if mentionString != "" { + variables["mention"] = mentionString } if milestoneString != "" { variables["milestone"] = milestoneString diff --git a/command/issue.go b/command/issue.go index 1b4befd67..9dd031ee5 100644 --- a/command/issue.go +++ b/command/issue.go @@ -41,7 +41,7 @@ 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().StringP("mentioned", "", "", "Filter by mention") + issueListCmd.Flags().StringP("mention", "", "", "Filter by mention") issueListCmd.Flags().StringP("milestone", "", "", "Filter by milestone") issueCmd.AddCommand(issueViewCmd) @@ -143,7 +143,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - mentioned, err := cmd.Flags().GetString("mentioned") + mention, err := cmd.Flags().GetString("mention") if err != nil { return err } @@ -153,7 +153,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mentioned, milestone) + listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mention, milestone) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index 19ac94d01..b3cb1e5a4 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -153,7 +153,7 @@ func TestIssueList_withFlags(t *testing.T) { } } } `)) - output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mentioned me --milestone 1.x") + 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) } @@ -171,7 +171,7 @@ No issues match your search in OWNER/REPO Labels []string States []string Author string - Mentioned string + Mention string Milestone string } }{} @@ -181,7 +181,7 @@ No issues match your search in OWNER/REPO eq(t, reqBody.Variables.Labels, []string{"web", "bug"}) eq(t, reqBody.Variables.States, []string{"OPEN"}) eq(t, reqBody.Variables.Author, "foo") - eq(t, reqBody.Variables.Mentioned, "me") + eq(t, reqBody.Variables.Mention, "me") eq(t, reqBody.Variables.Milestone, "1.x") } From 8a4872bab38611904db7a58244c40c92c76cc8e0 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 22 Jun 2020 14:07:49 -0400 Subject: [PATCH 06/48] Remove global repo flag --- command/root.go | 1 - 1 file changed, 1 deletion(-) diff --git a/command/root.go b/command/root.go index 693603e05..3f83ba278 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: From dd8cbccbd54ed5b1c94560ab0d917f853ae716c9 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 22 Jun 2020 14:26:41 -0400 Subject: [PATCH 07/48] Add repo flag to issue commands --- command/issue.go | 6 ++++++ command/util.go | 8 ++++++++ 2 files changed, 14 insertions(+) create mode 100644 command/util.go diff --git a/command/issue.go b/command/issue.go index 372ad8bd1..ca9087dbb 100644 --- a/command/issue.go +++ b/command/issue.go @@ -24,8 +24,10 @@ import ( func init() { RootCmd.AddCommand(issueCmd) issueCmd.AddCommand(issueStatusCmd) + includeRepoFlag(issueStatusCmd) issueCmd.AddCommand(issueCreateCmd) + includeRepoFlag(issueCreateCmd) issueCreateCmd.Flags().StringP("title", "t", "", "Supply a title. Will prompt for one otherwise.") issueCreateCmd.Flags().StringP("body", "b", "", @@ -37,6 +39,7 @@ func init() { issueCreateCmd.Flags().StringP("milestone", "m", "", "Add the issue to a milestone by `name`") issueCmd.AddCommand(issueListCmd) + includeRepoFlag(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") @@ -44,10 +47,13 @@ func init() { issueListCmd.Flags().StringP("author", "A", "", "Filter by author") issueCmd.AddCommand(issueViewCmd) + includeRepoFlag(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") issueCmd.AddCommand(issueCloseCmd) + includeRepoFlag(issueCloseCmd) issueCmd.AddCommand(issueReopenCmd) + includeRepoFlag(issueReopenCmd) } var issueCmd = &cobra.Command{ diff --git a/command/util.go b/command/util.go new file mode 100644 index 000000000..a7d4a0f31 --- /dev/null +++ b/command/util.go @@ -0,0 +1,8 @@ +package Command + +import "github.com/spf13/cobra" +package Command + +func includeRepoFlag(cmd *cobra.Command) { + cmd.StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") +} From 6a02ad3f1b83cab385d36cdc0e59ef436d38cd1e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 22 Jun 2020 15:09:21 -0400 Subject: [PATCH 08/48] Add repoflag to pr commands --- command/pr.go | 7 +++++++ command/pr_diff.go | 1 + command/pr_review.go | 1 + 3 files changed, 9 insertions(+) diff --git a/command/pr.go b/command/pr.go index 18150d4eb..0aa7d25b1 100644 --- a/command/pr.go +++ b/command/pr.go @@ -27,16 +27,22 @@ func init() { prCmd.AddCommand(prCheckoutCmd) prCmd.AddCommand(prCreateCmd) prCmd.AddCommand(prStatusCmd) + includeRepoFlag(prStatusCmd) prCmd.AddCommand(prCloseCmd) + includeRepoFlag(prCloseCmd) prCmd.AddCommand(prReopenCmd) + includeRepoFlag(prReopenCmd) prCmd.AddCommand(prMergeCmd) + includeRepoFlag(prMergeCmd) prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local and remote branch after merge") prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") prCmd.AddCommand(prReadyCmd) + includeRepoFlag(prReadyCmd) prCmd.AddCommand(prListCmd) + includeRepoFlag(prListCmd) prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") prListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|merged|all}") prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") @@ -44,6 +50,7 @@ func init() { prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") prCmd.AddCommand(prViewCmd) + includeRepoFlag(prViewCmd) prViewCmd.Flags().BoolP("web", "w", false, "Open a pull request in the browser") } diff --git a/command/pr_diff.go b/command/pr_diff.go index ae50a2618..0bc9c8613 100644 --- a/command/pr_diff.go +++ b/command/pr_diff.go @@ -19,6 +19,7 @@ func init() { prDiffCmd.Flags().StringP("color", "c", "auto", "Whether or not to output color: {always|never|auto}") prCmd.AddCommand(prDiffCmd) + includeRepoFlag(prDiffCmd) } func prDiff(cmd *cobra.Command, args []string) error { diff --git a/command/pr_review.go b/command/pr_review.go index 08efc638d..77723ab44 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -15,6 +15,7 @@ import ( func init() { prCmd.AddCommand(prReviewCmd) + includeRepoFlag(prCmd) prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request") prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request") From 2761739c29c7f12fb9a1f21e7f5c5bb8779c212f Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 22 Jun 2020 15:19:33 -0400 Subject: [PATCH 09/48] Correct package name --- command/util.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/util.go b/command/util.go index a7d4a0f31..130f97844 100644 --- a/command/util.go +++ b/command/util.go @@ -1,8 +1,7 @@ -package Command +package command import "github.com/spf13/cobra" -package Command func includeRepoFlag(cmd *cobra.Command) { - cmd.StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + cmd.Flags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") } From 625b673b587095df7d985a06e5e5883559215687 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 22 Jun 2020 15:30:22 -0400 Subject: [PATCH 10/48] Ignore repo flag errors in determineBaseRepo --- command/root.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/command/root.go b/command/root.go index 3f83ba278..83dfbd3f0 100644 --- a/command/root.go +++ b/command/root.go @@ -303,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) @@ -312,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, repo) if err != nil { return nil, err } From aa8f8e8904d109e32a902c871bb75e53f03bf752 Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Sat, 27 Jun 2020 18:47:06 +0300 Subject: [PATCH 11/48] httpmock: propagate original HTTP request to HTTP response So that it's possible to access it in mocked HTTP tests. Signed-off-by: Pavel Borzenkov --- pkg/httpmock/legacy.go | 8 ++++---- pkg/httpmock/stub.go | 15 ++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index 9474c3dd8..9402c21c7 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 { diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 48077ed4e..cc42dfd6c 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -59,15 +59,15 @@ 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 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 } } @@ -84,7 +84,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 +100,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), } } From c66eebc6fb248b77ec8e960ededc19c9595414f9 Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Sat, 27 Jun 2020 18:47:34 +0300 Subject: [PATCH 12/48] api: return structured error for failed API calls `fmt.Errorf` hides information and makes it hard to test for specific conditions in returned error. Return a structured error instead. Signed-off-by: Pavel Borzenkov --- api/client.go | 17 ++++++++++++++++- api/client_test.go | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/api/client.go b/api/client.go index 1c8c1eaa2..6880609bb 100644 --- a/api/client.go +++ b/api/client.go @@ -157,6 +157,17 @@ func (gr GraphQLErrorResponse) Error() string { return fmt.Sprintf("graphql error: '%s'", strings.Join(errorMessages, ", ")) } +// HTTPError is an error returned by a failed API call +type HTTPError struct { + Code int + URL string + Message string +} + +func (e HTTPError) Error() string { + return fmt.Sprintf("http error, '%s' failed (%d): '%s'", e.URL, e.Code, e.Message) +} + // Returns whether or not scopes are present, appID, and error func (c Client) HasScopes(wantedScopes ...string) (bool, string, error) { url := "https://api.github.com/user" @@ -298,7 +309,11 @@ func handleHTTPError(resp *http.Response) error { message = parsedBody.Message } - return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message) + return HTTPError{ + Code: resp.StatusCode, + URL: resp.Request.URL.String(), + Message: message, + } } var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`) diff --git a/api/client_test.go b/api/client_test.go index 4c81bf315..9c908be12 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "errors" "io/ioutil" "reflect" "testing" @@ -66,3 +67,16 @@ 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) || httpErr.Code != 422 { + t.Fatalf("got %q", err.Error()) + } +} From 3afec6f90aea8707de7610ce6a539faaf766c6fa Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Sat, 27 Jun 2020 19:03:25 +0300 Subject: [PATCH 13/48] api: gracefully handle already deleted remote refs If a GitHub repo is configured to automatically delete branches after PR is merged, `gh pr merge` fails with error like: failed to delete remote branch: ... (422): 'Reference does not exist' Gracefully handle such case and don't report the error. Signed-off-by: Pavel Borzenkov --- api/queries_pr.go | 11 ++++++++++- api/queries_pr_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 api/queries_pr_test.go diff --git a/api/queries_pr.go b/api/queries_pr.go index 00d01661a..b42a94eec 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -1015,7 +1015,16 @@ func BranchDeleteRemote(client *Client, repo ghrepo.Interface, branch string) er 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) + err := client.REST("DELETE", path, nil, &response) + if err != nil { + var httpErr HTTPError + // The ref might have already been deleted by GitHub + if !errors.As(err, &httpErr) || httpErr.Code != 422 { + return err + } + } + + return 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..3b8bf0a65 --- /dev/null +++ b/api/queries_pr_test.go @@ -0,0 +1,42 @@ +package api + +import ( + "bytes" + "testing" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/httpmock" +) + +func TestBranchDeleteRemote(t *testing.T) { + var tests = []struct { + name string + code int + body string + expectError bool + }{ + {name: "success", code: 204, body: "", expectError: false}, + {name: "error", code: 500, body: `{"message": "oh no"}`, expectError: true}, + { + name: "already_deleted", + code: 422, + body: `{"message": "Reference does not exist"}`, + expectError: false, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + http := &httpmock.Registry{} + client := NewClient(ReplaceTripper(http)) + + http.StubResponse(tc.code, bytes.NewBufferString(tc.body)) + repo, _ := ghrepo.FromFullName("OWNER/REPO") + err := BranchDeleteRemote(client, repo, "branch") + if isError := err != nil; isError != tc.expectError { + t.Fatalf("unexpected result: %v", err) + } + }) + } +} From cb2308c07767f990b3f3ddffd37ce2b7eeade03e Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Mon, 29 Jun 2020 00:15:34 +0530 Subject: [PATCH 14/48] --silent flag in api --- pkg/cmd/api/api.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index e1edabbc0..6b393cc42 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, "Silence the output") return cmd } @@ -178,6 +180,10 @@ func apiRun(opts *ApiOptions) error { return err } + if opts.Silent { + opts.IO.Out = ioutil.Discard + } + hasNextPage := true for hasNextPage { resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) From 5e56e3138449a1a12ff2f7838ad70ea0ac8d1be9 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Mon, 29 Jun 2020 00:30:24 +0530 Subject: [PATCH 15/48] Added Test for --silent flag in api --- pkg/cmd/api/api_test.go | 55 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 7b89d715c..a41f82c54 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, }, @@ -425,6 +449,37 @@ func Test_apiRun_paginationGraphQL(t *testing.T) { assert.Equal(t, "PAGE1_END", endCursor) } +func Test_apiRun_silent(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + response := &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`body`)), + Header: http.Header{"Content-Type": []string{"text/plain"}}, + } + + options := ApiOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + resp := response + resp.Request = req + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + RequestPath: "issues", + Silent: true, + } + + err := apiRun(&options) + assert.NoError(t, err) + + assert.Equal(t, "", stdout.String(), "stdout") + assert.Equal(t, "", stderr.String(), "stderr") + + assert.Equal(t, "https://api.github.com/issues", response.Request.URL.String()) +} + func Test_apiRun_inputFile(t *testing.T) { tests := []struct { name string From aa43c55f60d4818f482ae64ef74f46e7e957c2c3 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Mon, 29 Jun 2020 07:13:06 +0530 Subject: [PATCH 16/48] Skip printing headers when --silent in api --- pkg/cmd/api/api.go | 2 +- pkg/cmd/api/api_test.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 6b393cc42..15a2b3be4 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -218,7 +218,7 @@ func apiRun(opts *ApiOptions) error { } func processResponse(resp *http.Response, opts *ApiOptions) (endCursor string, err error) { - if opts.ShowResponseHeaders { + if opts.ShowResponseHeaders && !opts.Silent { 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") diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index a41f82c54..75c8eaa4a 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -467,8 +467,9 @@ func Test_apiRun_silent(t *testing.T) { } return &http.Client{Transport: tr}, nil }, - RequestPath: "issues", - Silent: true, + RequestPath: "issues", + ShowResponseHeaders: true, + Silent: true, } err := apiRun(&options) From 4827ffbf03e316323ea06dea71f4a73f7dae6f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 30 Jun 2020 13:47:15 +0200 Subject: [PATCH 17/48] Raise more informative path error when reading config file Example: remove or rename regular file `/home/foo/.config/gh` (must be a directory) --- internal/config/config_file.go | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 49a2770d3..d2ef9891b 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 @@ -168,3 +169,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 "" +} From 757bd05c7ad9e32079d91e214d3ef4d588321b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 30 Jun 2020 14:24:01 +0200 Subject: [PATCH 18/48] :nail_care: tweaks for mention/milestone filter --- command/issue.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/issue.go b/command/issue.go index 9dd031ee5..c00d624af 100644 --- a/command/issue.go +++ b/command/issue.go @@ -41,8 +41,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().StringP("mention", "", "", "Filter by mention") - issueListCmd.Flags().StringP("milestone", "", "", "Filter by milestone") + 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") @@ -161,7 +161,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 } }) From 7a04bf1672eeffaa544b6f10e461d277b5180e41 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Tue, 30 Jun 2020 19:18:28 +0530 Subject: [PATCH 19/48] `api --silent` Changes: Show Response Headers (if requested) even with `--silent` flag Shift silent tests to `Test_apiRun` Changed usage string of `--silent` flag --- pkg/cmd/api/api.go | 15 +++++----- pkg/cmd/api/api_test.go | 62 ++++++++++++++++++++--------------------- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 15a2b3be4..170c41b55 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -135,7 +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, "Silence the output") + cmd.Flags().BoolVar(&opts.Silent, "silent", false, "Do not print the response body") return cmd } @@ -180,6 +180,7 @@ func apiRun(opts *ApiOptions) error { return err } + headersOutputStream := opts.IO.Out if opts.Silent { opts.IO.Out = ioutil.Discard } @@ -191,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 } @@ -217,11 +218,11 @@ func apiRun(opts *ApiOptions) error { return nil } -func processResponse(resp *http.Response, opts *ApiOptions) (endCursor string, err error) { - if opts.ShowResponseHeaders && !opts.Silent { - 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") +func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream io.Writer) (endCursor string, err error) { + if opts.ShowResponseHeaders { + 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 75c8eaa4a..f590d93b4 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -288,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 { @@ -449,38 +479,6 @@ func Test_apiRun_paginationGraphQL(t *testing.T) { assert.Equal(t, "PAGE1_END", endCursor) } -func Test_apiRun_silent(t *testing.T) { - io, _, stdout, stderr := iostreams.Test() - response := &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(bytes.NewBufferString(`body`)), - Header: http.Header{"Content-Type": []string{"text/plain"}}, - } - - options := ApiOptions{ - IO: io, - HttpClient: func() (*http.Client, error) { - var tr roundTripper = func(req *http.Request) (*http.Response, error) { - resp := response - resp.Request = req - return resp, nil - } - return &http.Client{Transport: tr}, nil - }, - RequestPath: "issues", - ShowResponseHeaders: true, - Silent: true, - } - - err := apiRun(&options) - assert.NoError(t, err) - - assert.Equal(t, "", stdout.String(), "stdout") - assert.Equal(t, "", stderr.String(), "stderr") - - assert.Equal(t, "https://api.github.com/issues", response.Request.URL.String()) -} - func Test_apiRun_inputFile(t *testing.T) { tests := []struct { name string From db88ac415523282f6c7cba3fa6434586934ab737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 30 Jun 2020 18:51:53 +0200 Subject: [PATCH 20/48] Declare `-R, --repo` flag on all `issue` and `pr` subcommands --- command/issue.go | 8 ++------ command/pr.go | 9 ++------- command/pr_diff.go | 1 - command/pr_review.go | 1 - command/util.go | 7 ------- 5 files changed, 4 insertions(+), 22 deletions(-) delete mode 100644 command/util.go diff --git a/command/issue.go b/command/issue.go index 9569c9603..287ca7260 100644 --- a/command/issue.go +++ b/command/issue.go @@ -22,12 +22,12 @@ import ( ) func init() { + issueCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + RootCmd.AddCommand(issueCmd) issueCmd.AddCommand(issueStatusCmd) - includeRepoFlag(issueStatusCmd) issueCmd.AddCommand(issueCreateCmd) - includeRepoFlag(issueCreateCmd) issueCreateCmd.Flags().StringP("title", "t", "", "Supply a title. Will prompt for one otherwise.") issueCreateCmd.Flags().StringP("body", "b", "", @@ -39,7 +39,6 @@ func init() { issueCreateCmd.Flags().StringP("milestone", "m", "", "Add the issue to a milestone by `name`") issueCmd.AddCommand(issueListCmd) - includeRepoFlag(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels") issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") @@ -47,13 +46,10 @@ func init() { issueListCmd.Flags().StringP("author", "A", "", "Filter by author") issueCmd.AddCommand(issueViewCmd) - includeRepoFlag(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") issueCmd.AddCommand(issueCloseCmd) - includeRepoFlag(issueCloseCmd) issueCmd.AddCommand(issueReopenCmd) - includeRepoFlag(issueReopenCmd) } var issueCmd = &cobra.Command{ diff --git a/command/pr.go b/command/pr.go index 961cdbf1a..6d8ae22aa 100644 --- a/command/pr.go +++ b/command/pr.go @@ -23,26 +23,22 @@ 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) prCmd.AddCommand(prStatusCmd) - includeRepoFlag(prStatusCmd) prCmd.AddCommand(prCloseCmd) - includeRepoFlag(prCloseCmd) prCmd.AddCommand(prReopenCmd) - includeRepoFlag(prReopenCmd) prCmd.AddCommand(prMergeCmd) - includeRepoFlag(prMergeCmd) prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local and remote branch after merge") prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") prCmd.AddCommand(prReadyCmd) - includeRepoFlag(prReadyCmd) prCmd.AddCommand(prListCmd) - includeRepoFlag(prListCmd) prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") prListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|merged|all}") prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") @@ -50,7 +46,6 @@ func init() { prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") prCmd.AddCommand(prViewCmd) - includeRepoFlag(prViewCmd) prViewCmd.Flags().BoolP("web", "w", false, "Open a pull request in the browser") } diff --git a/command/pr_diff.go b/command/pr_diff.go index 0bc9c8613..ae50a2618 100644 --- a/command/pr_diff.go +++ b/command/pr_diff.go @@ -19,7 +19,6 @@ func init() { prDiffCmd.Flags().StringP("color", "c", "auto", "Whether or not to output color: {always|never|auto}") prCmd.AddCommand(prDiffCmd) - includeRepoFlag(prDiffCmd) } func prDiff(cmd *cobra.Command, args []string) error { diff --git a/command/pr_review.go b/command/pr_review.go index 77723ab44..08efc638d 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -15,7 +15,6 @@ import ( func init() { prCmd.AddCommand(prReviewCmd) - includeRepoFlag(prCmd) prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request") prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request") diff --git a/command/util.go b/command/util.go deleted file mode 100644 index 130f97844..000000000 --- a/command/util.go +++ /dev/null @@ -1,7 +0,0 @@ -package command - -import "github.com/spf13/cobra" - -func includeRepoFlag(cmd *cobra.Command) { - cmd.Flags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") -} From 1ca3d171e6de6cfdaec29b0e6bc1b76780476833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 30 Jun 2020 19:13:54 +0200 Subject: [PATCH 21/48] Tweak HTTP 422 handling when deleting branches --- api/client.go | 45 ++++++++++++++++++++++-------------------- api/client_test.go | 23 ++++++++++++++++----- api/queries_pr.go | 14 +------------ api/queries_pr_test.go | 37 ++++++++++++++++++---------------- command/issue_test.go | 2 +- command/pr.go | 4 +++- pkg/httpmock/stub.go | 6 ++++++ 7 files changed, 73 insertions(+), 58 deletions(-) diff --git a/api/client.go b/api/client.go index 6880609bb..74a5a0417 100644 --- a/api/client.go +++ b/api/client.go @@ -7,6 +7,7 @@ import ( "io" "io/ioutil" "net/http" + "net/url" "regexp" "strings" @@ -154,18 +155,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 { - Code int - URL string - Message string + StatusCode int + RequestURL *url.URL + Message string } -func (e HTTPError) Error() string { - return fmt.Sprintf("http error, '%s' failed (%d): '%s'", e.URL, e.Code, e.Message) +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 @@ -294,26 +298,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 HTTPError{ - Code: resp.StatusCode, - URL: resp.Request.URL.String(), - Message: message, - } + return httpError } var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`) diff --git a/api/client_test.go b/api/client_test.go index 9c908be12..b7c226c8f 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -47,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()) } } @@ -75,8 +81,15 @@ func TestRESTError(t *testing.T) { 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) || httpErr.Code != 422 { - t.Fatalf("got %q", err.Error()) + 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_pr.go b/api/queries_pr.go index b42a94eec..56582d0e3 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -1011,20 +1011,8 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er } 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) - err := client.REST("DELETE", path, nil, &response) - if err != nil { - var httpErr HTTPError - // The ref might have already been deleted by GitHub - if !errors.As(err, &httpErr) || httpErr.Code != 422 { - return err - } - } - - return nil + 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 index 3b8bf0a65..07370023b 100644 --- a/api/queries_pr_test.go +++ b/api/queries_pr_test.go @@ -1,7 +1,6 @@ package api import ( - "bytes" "testing" "github.com/cli/cli/internal/ghrepo" @@ -10,31 +9,35 @@ import ( func TestBranchDeleteRemote(t *testing.T) { var tests = []struct { - name string - code int - body string - expectError bool + name string + responseStatus int + responseBody string + expectError bool }{ - {name: "success", code: 204, body: "", expectError: false}, - {name: "error", code: 500, body: `{"message": "oh no"}`, expectError: true}, { - name: "already_deleted", - code: 422, - body: `{"message": "Reference does not exist"}`, - expectError: false, + name: "success", + responseStatus: 204, + responseBody: "", + expectError: false, + }, + { + name: "error", + responseStatus: 500, + responseBody: `{"message": "oh no"}`, + expectError: true, }, } - for _, tc := range tests { - tc := tc - t.Run(tc.name, func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { http := &httpmock.Registry{} - client := NewClient(ReplaceTripper(http)) + http.Register(httpmock.MatchAny, httpmock.StatusStringResponse(tt.responseStatus, tt.responseBody)) - http.StubResponse(tc.code, bytes.NewBufferString(tc.body)) + client := NewClient(ReplaceTripper(http)) repo, _ := ghrepo.FromFullName("OWNER/REPO") + err := BranchDeleteRemote(client, repo, "branch") - if isError := err != nil; isError != tc.expectError { + if (err != nil) != tt.expectError { t.Fatalf("unexpected result: %v", err) } }) diff --git a/command/issue_test.go b/command/issue_test.go index 418de25d3..b7db7a7e2 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -402,7 +402,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) } diff --git a/command/pr.go b/command/pr.go index 2068a10e8..d1720ca42 100644 --- a/command/pr.go +++ b/command/pr.go @@ -523,7 +523,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/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index cc42dfd6c..c57b7a1ad 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -64,6 +64,12 @@ func StringResponse(body string) Responder { } } +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(req *http.Request) (*http.Response, error) { b, _ := json.Marshal(body) From 10b4c8ab26b7a65ed58a403f20a8eb76be4a1512 Mon Sep 17 00:00:00 2001 From: wilso199 Date: Tue, 30 Jun 2020 18:17:05 -0400 Subject: [PATCH 22/48] Adding config set example for vscode to docs --- command/config.go | 1 + 1 file changed, 1 insertion(+) 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, From 569645a050dec9db212b9e49bde6ed2537012407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 1 Jul 2020 14:34:44 +0200 Subject: [PATCH 23/48] Support `hosts.yml` existing while `config.yml` does not After a person copies `hosts.yml` to a headless system, they will still be forced to go through re-authentication flow unless they copy or initialize a `config.yml` as well. This fixes respecting authentication info from `hosts.yml` even if `config.yml` is missing. --- internal/config/config_file.go | 6 ++++- internal/config/config_file_test.go | 36 +++++++++++++++++++++++------ internal/config/config_type.go | 8 +++++-- internal/config/testing.go | 6 ++++- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 49a2770d3..af2c2d560 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -138,7 +138,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) { 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 From b6206aa036c59ef0a6d0114ea2f7f65ce9d3b902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 1 Jul 2020 16:14:11 +0200 Subject: [PATCH 24/48] Update the branch name in the `github/gh/gh` brew formula --- .goreleaser.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: | From 9bbdf4af990c108ad061579085a8519de8714414 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Wed, 1 Jul 2020 11:04:51 -0400 Subject: [PATCH 25/48] Make: properly add environment C compiler flags to CGO Do not pass LDFLAGS as arguments to -ldflags, since these are passed to 'go tool link' and C compiler ldflags need to be passed as -ldflags "-extldflags \"$LDFLAGS\"" with unreliable handling. Instead copy over (unmodified) the standard environment variable to the special golang-specific variable which the go compiler chooses to respect. While we are at it, handle CPPFLAGS and CFLAGS too. --- Makefile | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index b925c11fd..fd219d42d 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 ./... From cd5a0d69fbddd10214895bcd58bd6a1e66dd596d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 2 Jul 2020 12:36:31 +0200 Subject: [PATCH 26/48] :nail_polish: be clearer about the value passed to ResolveRemotesToRepos `repo` will always be blank here, so replace the argument with a blank literal instead. --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 83dfbd3f0..cf0e3cbb1 100644 --- a/command/root.go +++ b/command/root.go @@ -317,7 +317,7 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co return nil, err } - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, repo) + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") if err != nil { return nil, err } From ac7b56fc61f04ee0f64a33570e65931b87657925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 2 Jul 2020 17:01:37 +0200 Subject: [PATCH 27/48] Fix linter warning about sprintf within println --- command/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/root.go b/command/root.go index cf0e3cbb1..3693fd6f0 100644 --- a/command/root.go +++ b/command/root.go @@ -257,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 { From 9c7d52f9f61a5afc79da2ecf723d060c11da4489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 2 Jul 2020 17:02:38 +0200 Subject: [PATCH 28/48] Bump golangci-lint --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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/ From 9b890d9a10946d8de02f0990c4c50121f517c634 Mon Sep 17 00:00:00 2001 From: danshearer Date: Sat, 4 Jul 2020 16:14:47 +0100 Subject: [PATCH 29/48] Reduce minimum reuqired version from 1.14 to 1.13.8. This is the version that ships with Ubuntu 20.04 Long Term Support release and it seems to work fine. --- docs/source.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 37d6be58b85e34fc8c18a6b9db5c192e083677d0 Mon Sep 17 00:00:00 2001 From: ShubhankarKG Date: Mon, 6 Jul 2020 17:57:34 +0530 Subject: [PATCH 30/48] Fixed gh issue close and gh issue reopen when issue number invalid --- command/issue.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/issue.go b/command/issue.go index 1cd25941c..485c6b966 100644 --- a/command/issue.go +++ b/command/issue.go @@ -709,7 +709,7 @@ 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 { @@ -744,7 +744,7 @@ 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 { From 742bb155837fe51d0bb7c75358ff115f66b60212 Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Mon, 6 Jul 2020 19:33:17 +0530 Subject: [PATCH 31/48] Update pr.go adding long description for pr merge subcommand to clarify on how the source branch can be retained after a merge --- command/pr.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 79c65b874..c70ab4da2 100644 --- a/command/pr.go +++ b/command/pr.go @@ -108,6 +108,11 @@ var prReopenCmd = &cobra.Command{ var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", Short: "Merge a pull request", + Long: `Merge a pull request by providing the pull request number or url or the branch. + + By default, the source branch gets deleted after the merge is complete. + + If the source branch needs be retained, '--delete-branch=false' should be specified.`, Args: cobra.MaximumNArgs(1), RunE: prMerge, } @@ -123,7 +128,7 @@ func prStatus(cmd *cobra.Command, args []string) error { apiClient, err := apiClientForContext(ctx) if err != nil { return err - } + } baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { From 143c80c56c22fa7ceffbd698b10cb48220ae037e Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Mon, 6 Jul 2020 19:40:49 +0530 Subject: [PATCH 32/48] Update pr.go --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index c70ab4da2..ae65b80a1 100644 --- a/command/pr.go +++ b/command/pr.go @@ -107,13 +107,13 @@ var prReopenCmd = &cobra.Command{ } var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", + Args: cobra.MaximumNArgs(1), Short: "Merge a pull request", Long: `Merge a pull request by providing the pull request number or url or the branch. By default, the source branch gets deleted after the merge is complete. If the source branch needs be retained, '--delete-branch=false' should be specified.`, - Args: cobra.MaximumNArgs(1), RunE: prMerge, } var prReadyCmd = &cobra.Command{ From 3a87124454609ecb401cd79cefbd7d01fb5eb666 Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Mon, 6 Jul 2020 19:44:29 +0530 Subject: [PATCH 33/48] Update pr.go ran gofmt on this --- command/pr.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index ae65b80a1..2caa331b7 100644 --- a/command/pr.go +++ b/command/pr.go @@ -111,9 +111,9 @@ var prMergeCmd = &cobra.Command{ Short: "Merge a pull request", Long: `Merge a pull request by providing the pull request number or url or the branch. - By default, the source branch gets deleted after the merge is complete. - - If the source branch needs be retained, '--delete-branch=false' should be specified.`, +By default, the source branch gets deleted after the merge is complete. + +If the source branch needs be retained, '--delete-branch=false' should be specified.`, RunE: prMerge, } var prReadyCmd = &cobra.Command{ From 5526dfdd231a5d59ff00c69ef4dd47dd6d02b386 Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Mon, 6 Jul 2020 19:50:31 +0530 Subject: [PATCH 34/48] Update pr.go proper go fmted --- command/pr.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index 2caa331b7..e11884bbb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -111,10 +111,10 @@ var prMergeCmd = &cobra.Command{ Short: "Merge a pull request", Long: `Merge a pull request by providing the pull request number or url or the branch. -By default, the source branch gets deleted after the merge is complete. +By default, the source branch gets deleted after the pull request merge is complete. If the source branch needs be retained, '--delete-branch=false' should be specified.`, - RunE: prMerge, + RunE: prMerge, } var prReadyCmd = &cobra.Command{ Use: "ready [ | | ]", @@ -128,7 +128,7 @@ func prStatus(cmd *cobra.Command, args []string) error { apiClient, err := apiClientForContext(ctx) if err != nil { return err - } + } baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { From 6cbcdf2a74e7b75b7741a3a6a733e4d3ce52920b Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Mon, 6 Jul 2020 19:33:17 +0530 Subject: [PATCH 35/48] Clarify how the branch can be retained after `pr merge` --- command/pr.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index 79c65b874..e11884bbb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -107,9 +107,14 @@ var prReopenCmd = &cobra.Command{ } var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", - Short: "Merge a pull request", Args: cobra.MaximumNArgs(1), - RunE: prMerge, + Short: "Merge a pull request", + Long: `Merge a pull request by providing the pull request number or url or the branch. + +By default, the source branch gets deleted after the pull request merge is complete. + +If the source branch needs be retained, '--delete-branch=false' should be specified.`, + RunE: prMerge, } var prReadyCmd = &cobra.Command{ Use: "ready [ | | ]", From e319aaa6abc1fc9338570d77d9c6ab65bc1567f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jul 2020 12:21:25 +0200 Subject: [PATCH 36/48] Tweak `pr merge` docs --- command/pr.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/command/pr.go b/command/pr.go index e11884bbb..0c851b511 100644 --- a/command/pr.go +++ b/command/pr.go @@ -107,13 +107,14 @@ var prReopenCmd = &cobra.Command{ } var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", - Args: cobra.MaximumNArgs(1), Short: "Merge a pull request", - Long: `Merge a pull request by providing the pull request number or url or the branch. + Long: heredoc.Doc(` + Merge a pull request on GitHub. -By default, the source branch gets deleted after the pull request merge is complete. - -If the source branch needs be retained, '--delete-branch=false' should be specified.`, + 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{ From 4f503a65bdba19839acfaadc373111d4ca8becb2 Mon Sep 17 00:00:00 2001 From: Ravikanth C Date: Tue, 7 Jul 2020 17:52:08 +0530 Subject: [PATCH 37/48] Merge remote-tracking branch 'upstream/trunk' into trunk --- .github/workflows/lint.yml | 2 +- .goreleaser.yml | 2 +- command/root.go | 4 ++-- internal/config/config_file.go | 6 ++++- internal/config/config_file_test.go | 36 +++++++++++++++++++++++------ internal/config/config_type.go | 8 +++++-- internal/config/testing.go | 6 ++++- 7 files changed, 49 insertions(+), 15 deletions(-) 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/command/root.go b/command/root.go index cf0e3cbb1..3693fd6f0 100644 --- a/command/root.go +++ b/command/root.go @@ -257,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 { diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 49a2770d3..af2c2d560 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -138,7 +138,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) { 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 From 0ed8c0645cbea9e6cbac171dd1b1a44d40b78665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jul 2020 16:01:14 +0200 Subject: [PATCH 38/48] Fix version and OAuth client information being injected via `make` --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index fd219d42d..09d66b982 100644 --- a/Makefile +++ b/Makefile @@ -20,10 +20,10 @@ ifndef CGO_LDFLAGS endif GO_LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) -GO_LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) +GO_LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(GO_LDFLAGS) ifdef GH_OAUTH_CLIENT_SECRET - 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) + GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(GO_LDFLAGS) + GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(GO_LDFLAGS) endif bin/gh: $(BUILD_FILES) From a7806a3e7831cb2b3eb5ad99741c944228bc753b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jul 2020 16:34:43 +0200 Subject: [PATCH 39/48] Add names to queries made over our homegrown GraphQL adapter --- api/queries_issue.go | 6 +++--- api/queries_pr.go | 18 +++++++++--------- api/queries_repo.go | 10 +++++----- api/queries_repo_test.go | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 76981f8d7..f42ae5c3c 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -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}) { @@ -212,7 +212,7 @@ 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, $mention: String, $milestone: 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, mentioned: $mention, milestone: $milestone}) { @@ -306,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) { diff --git a/api/queries_pr.go b/api/queries_pr.go index 56582d0e3..3339492f1 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 @@ -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, diff --git a/api/queries_repo.go b/api/queries_repo.go index 22d42cfbb..eb6e0ccfe 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 @@ -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) } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 03123cf43..950359662 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -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} From f4c4ce0b0aa2737599c431c90e2f3dffcb1d6d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jul 2020 17:52:47 +0200 Subject: [PATCH 40/48] Add names to GraphQL queries made over shurcooL adapter --- api/client.go | 5 ++++ api/queries_issue.go | 20 +++++++++------ api/queries_org.go | 8 +++--- api/queries_pr.go | 58 ++++++++++++++++++++++++++------------------ api/queries_repo.go | 8 +++--- api/queries_user.go | 6 ++--- go.mod | 9 ++++--- go.sum | 10 ++++++++ 8 files changed, 76 insertions(+), 48 deletions(-) diff --git a/api/client.go b/api/client.go index 74a5a0417..f307b2bf5 100644 --- a/api/client.go +++ b/api/client.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/henvic/httpretty" + "github.com/shurcooL/graphql" ) // ClientOption represents an argument to NewClient @@ -235,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 diff --git a/api/queries_issue.go b/api/queries_issue.go index f42ae5c3c..5b68862b5 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -383,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 @@ -406,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_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 3339492f1..999d67126 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -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) { @@ -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,10 +1010,14 @@ 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 { diff --git a/api/queries_repo.go b/api/queries_repo.go index eb6e0ccfe..46ed88baa 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -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 } @@ -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 } 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/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..d875b47f0 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= @@ -153,6 +157,8 @@ 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/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/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/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= @@ -195,6 +201,8 @@ golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5 h1:8dUaAV7K4uHsF56JQWkpre 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= @@ -206,6 +214,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn 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= From fbf425f048fa5c4d349641c3dc953f3653a88309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jul 2020 17:57:52 +0200 Subject: [PATCH 41/48] go mod tidy --- go.sum | 8 -------- 1 file changed, 8 deletions(-) diff --git a/go.sum b/go.sum index d875b47f0..ed615c5b6 100644 --- a/go.sum +++ b/go.sum @@ -155,12 +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/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/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/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= @@ -199,8 +195,6 @@ 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= @@ -212,8 +206,6 @@ 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= From 125547fb7be63b2b3d0c451745b170f74e589762 Mon Sep 17 00:00:00 2001 From: Sibi Date: Wed, 8 Jul 2020 01:25:19 +0530 Subject: [PATCH 42/48] Show title while closing/reopening issue and meriging PR --- command/issue.go | 4 ++-- command/pr.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/command/issue.go b/command/issue.go index 485c6b966..9017c3e20 100644 --- a/command/issue.go +++ b/command/issue.go @@ -722,7 +722,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 } @@ -757,7 +757,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/pr.go b/command/pr.go index 0c851b511..ecff199c8 100644 --- a/command/pr.go +++ b/command/pr.go @@ -423,13 +423,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 } @@ -490,7 +490,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) From 2491e98c413ae459ded9f3b8dc4c4e8a6309d1b7 Mon Sep 17 00:00:00 2001 From: Sibi Date: Wed, 8 Jul 2020 12:22:38 +0530 Subject: [PATCH 43/48] Title of the PR will be shown while closing, merging and re-opening & test cases updated --- command/issue.go | 4 ++-- command/issue_test.go | 16 +++++++-------- command/pr.go | 12 +++++------ command/pr_test.go | 46 +++++++++++++++++++++---------------------- 4 files changed, 39 insertions(+), 39 deletions(-) diff --git a/command/issue.go b/command/issue.go index 9017c3e20..67c315a9d 100644 --- a/command/issue.go +++ b/command/issue.go @@ -713,7 +713,7 @@ func issueClose(cmd *cobra.Command, args []string) error { } 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 } @@ -748,7 +748,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { } 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 } diff --git a/command/issue_test.go b/command/issue_test.go index 8a56a3d43..39f7c54f1 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -805,7 +805,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"} } } } `)) @@ -816,7 +816,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()) @@ -831,7 +831,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} } } } `)) @@ -842,7 +842,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()) @@ -878,7 +878,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"} } } } `)) @@ -889,7 +889,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()) @@ -904,7 +904,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"} } } } `)) @@ -915,7 +915,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 ecff199c8..96cbee4ae 100644 --- a/command/pr.go +++ b/command/pr.go @@ -361,10 +361,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 } @@ -373,7 +373,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 } @@ -391,12 +391,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 } @@ -405,7 +405,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 } diff --git a/command/pr_test.go b/command/pr_test.go index e7e1e2dc3..a703b554a 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -897,7 +897,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" } } } } `)) @@ -908,7 +908,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()) @@ -922,7 +922,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 } } } } `)) @@ -933,7 +933,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()) @@ -947,7 +947,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} } } } `)) @@ -958,7 +958,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()) @@ -972,7 +972,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} } } } `)) @@ -983,7 +983,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()) @@ -997,7 +997,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"} } } } `)) @@ -1008,7 +1008,7 @@ 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()) @@ -1033,7 +1033,7 @@ func initWithStubs(branch string, stubs ...stubResponse) { func TestPrMerge(t *testing.T) { initWithStubs("master", stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 1, "closed": false, "state": "OPEN"} + "pullRequest": { "number": 1, "title": "The title of the PR", "closed": false, "state": "OPEN"} } } }`)}, stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, @@ -1053,7 +1053,7 @@ 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()) @@ -1064,7 +1064,7 @@ 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"} + "pullRequest": { "number": 1, "title": "The title of the PR", "closed": false, "state": "OPEN"} } } }`)) http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`)) http.StubRepoResponse("OWNER", "REPO") @@ -1080,7 +1080,7 @@ 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()) @@ -1091,7 +1091,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { initWithStubs("blueberries", stubResponse{200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", "id": "THE-ID", "number": 3} + { "headRefName": "blueberries", "id": "THE-ID", "number": 3, "title": "The title of the PR"} ] } } } }`)}, stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)}, stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, @@ -1111,7 +1111,7 @@ 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 #3 \(The title of the PR\)`, "Deleted branch blueberries") } func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { @@ -1120,7 +1120,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", "id": "THE-ID", "number": 3} + { "headRefName": "blueberries", "id": "THE-ID", "number": 3, "title": "The title of the PR"} ] } } } }`)) http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`)) http.StubResponse(200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)) @@ -1138,7 +1138,7 @@ 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 #3 \(The title of the PR\)`, "Deleted branch blueberries") } func TestPrMerge_noPrNumberGiven(t *testing.T) { @@ -1165,7 +1165,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { 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()) @@ -1175,7 +1175,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { func TestPrMerge_rebase(t *testing.T) { initWithStubs("master", stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 2, "closed": false, "state": "OPEN"} + "pullRequest": { "number": 2, "title": "The title of the PR", "closed": false, "state": "OPEN"} } } }`)}, stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, @@ -1194,7 +1194,7 @@ 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()) @@ -1233,7 +1233,7 @@ func TestPrMerge_squash(t *testing.T) { func TestPrMerge_alreadyMerged(t *testing.T) { initWithStubs("master", stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": { - "pullRequest": { "number": 4, "closed": true, "state": "MERGED"} + "pullRequest": { "number": 4, "title": "The title of the PR", "closed": true, "state": "MERGED"} } } }`)}, stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)}, stubResponse{200, bytes.NewBufferString(`{"node_id": "THE-ID"}`)}, @@ -1252,7 +1252,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()) From 909aff827cc706f9e00c7b48e7cc5a83df14dde2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 10 Jul 2020 13:41:55 +0200 Subject: [PATCH 44/48] Use makefile append operator --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 09d66b982..504564dee 100644 --- a/Makefile +++ b/Makefile @@ -20,10 +20,10 @@ ifndef CGO_LDFLAGS endif GO_LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) -GO_LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(GO_LDFLAGS) +GO_LDFLAGS += -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) ifdef GH_OAUTH_CLIENT_SECRET - GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(GO_LDFLAGS) - GO_LDFLAGS := -X github.com/cli/cli/internal/config.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(GO_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) From 1ddb4d76a750748012c91204740d49991ad2f968 Mon Sep 17 00:00:00 2001 From: Shubhankar Kanchan Gupta Date: Fri, 10 Jul 2020 19:31:15 +0530 Subject: [PATCH 45/48] Strip carriage returns in markdown rendering (#1351) --- utils/utils.go | 3 +++ 1 file changed, 3 insertions(+) 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) } From d96e88c223f6a64a1323a4be63bee0968f20cb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 10 Jul 2020 18:08:16 +0200 Subject: [PATCH 46/48] Rename `IssueCreate` mutation This follows the "NounVerb" convention we got going on --- api/queries_issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 5b68862b5..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 From 13b9c98b2b4c94a95dff2824322382c19da7e840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 10 Jul 2020 20:04:54 +0200 Subject: [PATCH 47/48] Match named queries in test stubs --- api/queries_repo.go | 12 +- command/issue_test.go | 112 ++--- command/pr_create_test.go | 20 +- command/pr_test.go | 475 ++++++++++-------- pkg/httpmock/legacy.go | 6 +- pkg/httpmock/stub.go | 23 + .../prViewPreviewWithMetadataByBranch.json | 4 +- 7 files changed, 355 insertions(+), 297 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 46ed88baa..f9044b5e1 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -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/command/issue_test.go b/command/issue_test.go index 39f7c54f1..a3ad56b3a 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,13 +143,20 @@ func TestIssueList_withFlags(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } } - `)) + 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{})) + })) output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x") if err != nil { @@ -163,26 +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 - Mention string - Milestone 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") - eq(t, reqBody.Variables.Mention, "me") - eq(t, reqBody.Variables.Milestone, "1.x") } func TestIssueList_withInvalidLimitFlag(t *testing.T) { @@ -371,10 +356,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 { @@ -513,10 +495,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", @@ -525,7 +507,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" }, @@ -536,7 +518,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } `)) http.Register( - httpmock.GraphQL(`\bmilestones\(`), + httpmock.GraphQL(`query RepositoryMilestoneList\b`), httpmock.StringResponse(` { "data": { "repository": { "milestones": { "nodes": [ @@ -547,7 +529,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\brepository\(.+\bprojects\(`), + httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(` { "data": { "repository": { "projects": { "nodes": [ @@ -558,7 +540,7 @@ func TestIssueCreate_metadata(t *testing.T) { } } } } `)) http.Register( - httpmock.GraphQL(`\borganization\(.+\bprojects\(`), + httpmock.GraphQL(`query OrganizationProjectList\b`), httpmock.StringResponse(` { "data": { "organization": null }, "errors": [{ @@ -569,7 +551,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" diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 2eb0872c7..568882de0 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": "" diff --git a/command/pr_test.go b/command/pr_test.go index a703b554a..c3403252e 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" { @@ -612,10 +540,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 { @@ -633,10 +558,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 { @@ -671,10 +593,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 { @@ -1015,29 +934,34 @@ func TestPRReopen_alreadyMerged(t *testing.T) { } } -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, "title": "The title of the PR", "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() @@ -1056,19 +980,40 @@ func TestPrMerge(t *testing.T) { 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, "title": "The title of the PR", "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() @@ -1083,19 +1028,35 @@ func TestPrMerge_withRepoFlag(t *testing.T) { 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, "title": "The title of the PR"} - ] } } } }`)}, - 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() @@ -1111,20 +1072,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 \(The title of the PR\)`, "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, "title": "The title of the PR"} - ] } } } }`)) - 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() @@ -1138,10 +1113,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 \(The title of the PR\)`, "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() @@ -1151,15 +1151,6 @@ 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) @@ -1168,18 +1159,38 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { 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, "title": "The title of the PR", "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() @@ -1197,18 +1208,38 @@ func TestPrMerge_rebase(t *testing.T) { 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() @@ -1226,18 +1257,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, "title": "The title of the PR", "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() @@ -1260,13 +1293,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() @@ -1300,12 +1356,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/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index 9402c21c7..2c69be824 100644 --- a/pkg/httpmock/legacy.go +++ b/pkg/httpmock/legacy.go @@ -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 c57b7a1ad..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) @@ -77,6 +90,16 @@ func JSONResponse(body interface{}) Responder { } } +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 + } +} + func GraphQLMutation(body string, cb func(map[string]interface{})) Responder { return func(req *http.Request) (*http.Response, error) { var bodyData struct { 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 From 7a289861d26368f54224dd022bb0455d99e55094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 10 Jul 2020 20:20:33 +0200 Subject: [PATCH 48/48] Name more queries in test stubs --- api/queries_pr_test.go | 4 +++- api/queries_repo_test.go | 14 +++++++------- command/pr_create_test.go | 8 ++++---- command/repo_test.go | 35 +++++++++++++++++------------------ 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/api/queries_pr_test.go b/api/queries_pr_test.go index 07370023b..3441534bf 100644 --- a/api/queries_pr_test.go +++ b/api/queries_pr_test.go @@ -31,7 +31,9 @@ func TestBranchDeleteRemote(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { http := &httpmock.Registry{} - http.Register(httpmock.MatchAny, httpmock.StatusStringResponse(tt.responseStatus, tt.responseBody)) + 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") diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 950359662..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": [ @@ -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/command/pr_create_test.go b/command/pr_create_test.go index 568882de0..408a45256 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -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/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 {