From 94a1e6a830bec6db5680e86a03c46640130661e6 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 02:25:27 +0900 Subject: [PATCH 01/23] Add 'state' and 'createdAt' to the query for IssueByNumber --- api/queries_issue.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/queries_issue.go b/api/queries_issue.go index f83e43d92..7207f481f 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -24,6 +24,7 @@ type Issue struct { URL string State string Body string + CreatedAt time.Time UpdatedAt time.Time Comments struct { TotalCount int @@ -275,6 +276,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e hasIssuesEnabled issue(number: $issue_number) { title + state body author { login @@ -289,6 +291,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } number url + createdAt } } }` From d429babe2b40364c5599eab854e85d2dd7a2199f Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 02:27:33 +0900 Subject: [PATCH 02/23] Add 'state' to queries for pull requests --- api/queries_pr.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/queries_pr.go b/api/queries_pr.go index ee80c2f7f..a125a7623 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -301,6 +301,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu url number title + state body author { login @@ -356,6 +357,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) nodes { number title + state body author { login From faa4bed67abd77c736e3ac504afaa01f51be72ce Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 02:29:22 +0900 Subject: [PATCH 03/23] Move colorFuncForState for a PR/issue state to the utils package --- command/issue.go | 2 +- command/pr.go | 15 +-------------- utils/utils.go | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/command/issue.go b/command/issue.go index 2b85efc75..fd6e23d7f 100644 --- a/command/issue.go +++ b/command/issue.go @@ -148,7 +148,7 @@ func issueList(cmd *cobra.Command, args []string) error { if labels != "" && table.IsTTY() { labels = fmt.Sprintf("(%s)", labels) } - table.AddField(issueNum, nil, colorFuncForState(issue.State)) + table.AddField(issueNum, nil, utils.ColorFuncForState(issue.State)) table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil) table.AddField(labels, nil, utils.Gray) table.EndRow() diff --git a/command/pr.go b/command/pr.go index e1c8e796b..161f21263 100644 --- a/command/pr.go +++ b/command/pr.go @@ -232,20 +232,7 @@ func colorFuncForPR(pr api.PullRequest) func(string) string { if pr.State == "OPEN" && pr.IsDraft { return utils.Gray } else { - return colorFuncForState(pr.State) - } -} - -func colorFuncForState(state string) func(string) string { - switch state { - case "OPEN": - return utils.Green - case "CLOSED": - return utils.Red - case "MERGED": - return utils.Magenta - default: - return nil + return utils.ColorFuncForState(pr.State) } } diff --git a/utils/utils.go b/utils/utils.go index 065c4988e..3d157beb5 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -57,3 +57,17 @@ func FuzzyAgo(ago time.Duration) string { func Spinner() *spinner.Spinner { return spinner.New(spinner.CharSets[11], 400*time.Millisecond) } + +// ColorFuncForState returns a color function for a PR/Issue state +func ColorFuncForState(state string) func(string) string { + switch state { + case "OPEN": + return Green + case "CLOSED": + return Red + case "MERGED": + return Magenta + default: + return nil + } +} From caf26e4093e4e9d9821cb39f39b25e080eb9ac29 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 02:35:38 +0900 Subject: [PATCH 04/23] Show the issue state for viewing issues in CLI --- command/issue.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/command/issue.go b/command/issue.go index fd6e23d7f..96e33643a 100644 --- a/command/issue.go +++ b/command/issue.go @@ -251,16 +251,25 @@ func issueView(cmd *cobra.Command, args []string) error { func printIssuePreview(out io.Writer, issue *api.Issue) error { coloredLabels := labelList(*issue) if coloredLabels != "" { - coloredLabels = utils.Gray(fmt.Sprintf("(%s)", coloredLabels)) + coloredLabels = fmt.Sprintf("%s", coloredLabels) } + now := time.Now() + ago := now.Sub(issue.CreatedAt) + issueStateColorFunc := utils.ColorFuncForState(issue.State) + fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf( - "opened by %s. %s. %s", + fmt.Fprintf(out, "%s", issueStateColorFunc(issue.State)) + fmt.Fprint(out, utils.Gray(fmt.Sprintf( + " • %s opened %s • %s • ", issue.Author.Login, + utils.FuzzyAgo(ago), utils.Pluralize(issue.Comments.TotalCount, "comment"), - coloredLabels, ))) + if coloredLabels != "" { + fmt.Fprintf(out, "%s", utils.Gray(coloredLabels)) + } + fmt.Fprintf(out, "\n") if issue.Body != "" { fmt.Fprintln(out) From b8edb9f62d3a467d51fdf1062af41366c5ac1111 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 02:41:02 +0900 Subject: [PATCH 05/23] Show the PR state for viewing PRs in CLI --- command/pr.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 161f21263..5443cccec 100644 --- a/command/pr.go +++ b/command/pr.go @@ -296,9 +296,12 @@ func prView(cmd *cobra.Command, args []string) error { } func printPrPreview(out io.Writer, pr *api.PullRequest) error { + prStateColorFunc := colorFuncForPR(*pr) + fmt.Fprintln(out, utils.Bold(pr.Title)) + fmt.Fprintf(out, "%s", prStateColorFunc(pr.State)) fmt.Fprintln(out, utils.Gray(fmt.Sprintf( - "%s wants to merge %s into %s from %s", + " • %s wants to merge %s into %s from %s", pr.Author.Login, utils.Pluralize(pr.Commits.TotalCount, "commit"), pr.BaseRefName, From e0bfd67c69a084b8dbd513ae665dcf24324d4237 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 03:07:52 +0900 Subject: [PATCH 06/23] Fix tests for preview messages with a issue/PR state --- command/issue_test.go | 8 ++++++-- command/pr_test.go | 6 +++--- test/fixtures/prView.json | 2 ++ test/fixtures/prViewPreview.json | 1 + test/fixtures/prView_EmptyBody.json | 2 ++ 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index a4de959e1..e11826bfc 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -289,6 +289,8 @@ func TestIssueView_preview(t *testing.T) { "number": 123, "body": "**bold story**", "title": "ix of coins", + "state": "OPEN", + "created_at": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, @@ -313,7 +315,7 @@ func TestIssueView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), + regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments • tarot`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -335,6 +337,8 @@ func TestIssueView_previewWithEmptyBody(t *testing.T) { "number": 123, "body": "", "title": "ix of coins", + "state": "OPEN", + "created_at": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, @@ -359,7 +363,7 @@ func TestIssueView_previewWithEmptyBody(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), + regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments • tarot`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } for _, r := range expectedLines { diff --git a/command/pr_test.go b/command/pr_test.go index f024792e3..f3884b46a 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -399,7 +399,7 @@ func TestPRView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`OPEN • nobody wants to merge 12 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), } @@ -434,7 +434,7 @@ func TestPRView_previewCurrentBranch(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`OPEN • nobody wants to merge 8 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), } @@ -469,7 +469,7 @@ func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`OPEN • nobody wants to merge 8 commits into master from blueberries`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), } for _, r := range expectedLines { diff --git a/test/fixtures/prView.json b/test/fixtures/prView.json index 02277d7a5..2b6b4f9b5 100644 --- a/test/fixtures/prView.json +++ b/test/fixtures/prView.json @@ -6,6 +6,7 @@ { "number": 12, "title": "Blueberries are from a fork", + "state": "OPEN", "body": "yeah", "url": "https://github.com/OWNER/REPO/pull/12", "headRefName": "blueberries", @@ -24,6 +25,7 @@ { "number": 10, "title": "Blueberries are a good fruit", + "state": "OPEN", "body": "**blueberries taste good**", "url": "https://github.com/OWNER/REPO/pull/10", "baseRefName": "master", diff --git a/test/fixtures/prViewPreview.json b/test/fixtures/prViewPreview.json index e40946774..2b551a18f 100644 --- a/test/fixtures/prViewPreview.json +++ b/test/fixtures/prViewPreview.json @@ -4,6 +4,7 @@ "pullRequest": { "number": 12, "title": "Blueberries are from a fork", + "state": "OPEN", "body": "**blueberries taste good**", "url": "https://github.com/OWNER/REPO/pull/12", "author": { diff --git a/test/fixtures/prView_EmptyBody.json b/test/fixtures/prView_EmptyBody.json index 651f49ce0..36fdc308b 100644 --- a/test/fixtures/prView_EmptyBody.json +++ b/test/fixtures/prView_EmptyBody.json @@ -6,6 +6,7 @@ { "number": 12, "title": "Blueberries are from a fork", + "state": "OPEN", "body": "yeah", "url": "https://github.com/OWNER/REPO/pull/12", "headRefName": "blueberries", @@ -24,6 +25,7 @@ { "number": 10, "title": "Blueberries are a good fruit", + "state": "OPEN", "body": "", "url": "https://github.com/OWNER/REPO/pull/10", "baseRefName": "master", From 0475cf0112001891843527e6263a88630b369707 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 03:21:03 +0900 Subject: [PATCH 07/23] Extract test fixtures for 'issue view --preview' --- command/issue_test.go | 48 +++---------------- test/fixtures/issueView_preview.json | 28 +++++++++++ .../issueView_previewWithEmptyBody.json | 28 +++++++++++ 3 files changed, 62 insertions(+), 42 deletions(-) create mode 100644 test/fixtures/issueView_preview.json create mode 100644 test/fixtures/issueView_previewWithEmptyBody.json diff --git a/command/issue_test.go b/command/issue_test.go index e11826bfc..c18457623 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -284,27 +284,9 @@ func TestIssueView_preview(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "hasIssuesEnabled": true, "issue": { - "number": 123, - "body": "**bold story**", - "title": "ix of coins", - "state": "OPEN", - "created_at": "2011-01-26T19:01:12Z", - "author": { - "login": "marseilles" - }, - "labels": { - "nodes": [ - {"name": "tarot"} - ] - }, - "comments": { - "totalCount": 9 - }, - "url": "https://github.com/OWNER/REPO/issues/123" - } } } } - `)) + jsonFile, _ := os.Open("../test/fixtures/issueView_preview.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) output, err := RunCommand(issueViewCmd, "issue view -p 123") if err != nil { @@ -332,27 +314,9 @@ func TestIssueView_previewWithEmptyBody(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "hasIssuesEnabled": true, "issue": { - "number": 123, - "body": "", - "title": "ix of coins", - "state": "OPEN", - "created_at": "2011-01-26T19:01:12Z", - "author": { - "login": "marseilles" - }, - "labels": { - "nodes": [ - {"name": "tarot"} - ] - }, - "comments": { - "totalCount": 9 - }, - "url": "https://github.com/OWNER/REPO/issues/123" - } } } } - `)) + jsonFile, _ := os.Open("../test/fixtures/issueView_previewWithEmptyBody.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) output, err := RunCommand(issueViewCmd, "issue view -p 123") if err != nil { diff --git a/test/fixtures/issueView_preview.json b/test/fixtures/issueView_preview.json new file mode 100644 index 000000000..db971d849 --- /dev/null +++ b/test/fixtures/issueView_preview.json @@ -0,0 +1,28 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "**bold story**", + "title": "ix of coins", + "state": "OPEN", + "created_at": "2011-01-26T19:01:12Z", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + { + "name": "tarot" + } + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} diff --git a/test/fixtures/issueView_previewWithEmptyBody.json b/test/fixtures/issueView_previewWithEmptyBody.json new file mode 100644 index 000000000..6e87a42b6 --- /dev/null +++ b/test/fixtures/issueView_previewWithEmptyBody.json @@ -0,0 +1,28 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "", + "title": "ix of coins", + "state": "OPEN", + "created_at": "2011-01-26T19:01:12Z", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + { + "name": "tarot" + } + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} From 8cd6932f9ebf585311dff7a55a211dc4a234c27f Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 04:12:26 +0900 Subject: [PATCH 08/23] Add a test for the closed issue preview --- command/issue_test.go | 30 +++++++++++++++++++ .../issueView_previewClosedState.json | 28 +++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 test/fixtures/issueView_previewClosedState.json diff --git a/command/issue_test.go b/command/issue_test.go index c18457623..845f2aca9 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -309,6 +309,36 @@ func TestIssueView_preview(t *testing.T) { } } +func TestIssueView_previewClosedState(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/issueView_previewClosedState.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(issueViewCmd, "issue view -p 123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`CLOSED • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`bold story`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestIssueView_previewWithEmptyBody(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/test/fixtures/issueView_previewClosedState.json b/test/fixtures/issueView_previewClosedState.json new file mode 100644 index 000000000..978927125 --- /dev/null +++ b/test/fixtures/issueView_previewClosedState.json @@ -0,0 +1,28 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "**bold story**", + "title": "ix of coins", + "state": "CLOSED", + "created_at": "2011-01-26T19:01:12Z", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + { + "name": "tarot" + } + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} From 5a23113b5bad8fd37efdffe0b33005f6983e3b11 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 04:21:35 +0900 Subject: [PATCH 09/23] Add tests for the Closed/Merged PR preview --- command/pr_test.go | 60 +++++++++++++++++++++ test/fixtures/prViewPreviewClosedState.json | 25 +++++++++ test/fixtures/prViewPreviewMergedState.json | 25 +++++++++ 3 files changed, 110 insertions(+) create mode 100644 test/fixtures/prViewPreviewClosedState.json create mode 100644 test/fixtures/prViewPreviewMergedState.json diff --git a/command/pr_test.go b/command/pr_test.go index f3884b46a..b3336b640 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -411,6 +411,66 @@ func TestPRView_preview(t *testing.T) { } } +func TestPRView_previewClosedState(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prViewPreviewClosedState.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prViewCmd, "pr view -p 12") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`CLOSED • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + +func TestPRView_previewMergedState(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prViewPreviewMergedState.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prViewCmd, "pr view -p 12") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`MERGED • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_previewCurrentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() diff --git a/test/fixtures/prViewPreviewClosedState.json b/test/fixtures/prViewPreviewClosedState.json new file mode 100644 index 000000000..9df2c1038 --- /dev/null +++ b/test/fixtures/prViewPreviewClosedState.json @@ -0,0 +1,25 @@ +{ + "data": { + "repository": { + "pullRequest": { + "number": 12, + "title": "Blueberries are from a fork", + "state": "CLOSED", + "body": "**blueberries taste good**", + "url": "https://github.com/OWNER/REPO/pull/12", + "author": { + "login": "nobody" + }, + "commits": { + "totalCount": 12 + }, + "baseRefName": "master", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true + } + } + } +} diff --git a/test/fixtures/prViewPreviewMergedState.json b/test/fixtures/prViewPreviewMergedState.json new file mode 100644 index 000000000..d62449092 --- /dev/null +++ b/test/fixtures/prViewPreviewMergedState.json @@ -0,0 +1,25 @@ +{ + "data": { + "repository": { + "pullRequest": { + "number": 12, + "title": "Blueberries are from a fork", + "state": "MERGED", + "body": "**blueberries taste good**", + "url": "https://github.com/OWNER/REPO/pull/12", + "author": { + "login": "nobody" + }, + "commits": { + "totalCount": 12 + }, + "baseRefName": "master", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true + } + } + } +} From c8a72c3bcb17a80db6bec81d7585bb10917c4cd2 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 04:23:05 +0900 Subject: [PATCH 10/23] Add a test for viewing an issue without labels in CLI --- command/issue.go | 4 +-- command/issue_test.go | 30 +++++++++++++++++++++ test/fixtures/issueView_previewNoLabel.json | 25 +++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/issueView_previewNoLabel.json diff --git a/command/issue.go b/command/issue.go index 96e33643a..5111fdec8 100644 --- a/command/issue.go +++ b/command/issue.go @@ -261,13 +261,13 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error { fmt.Fprintln(out, utils.Bold(issue.Title)) fmt.Fprintf(out, "%s", issueStateColorFunc(issue.State)) fmt.Fprint(out, utils.Gray(fmt.Sprintf( - " • %s opened %s • %s • ", + " • %s opened %s • %s", issue.Author.Login, utils.FuzzyAgo(ago), utils.Pluralize(issue.Comments.TotalCount, "comment"), ))) if coloredLabels != "" { - fmt.Fprintf(out, "%s", utils.Gray(coloredLabels)) + fmt.Fprintf(out, utils.Gray(fmt.Sprintf(" • %s", coloredLabels))) } fmt.Fprintf(out, "\n") diff --git a/command/issue_test.go b/command/issue_test.go index 845f2aca9..c4a9e3405 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -309,6 +309,36 @@ func TestIssueView_preview(t *testing.T) { } } +func TestIssueView_previewNoLabel(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/issueView_previewNoLabel.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(issueViewCmd, "issue view -p 123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`bold story`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestIssueView_previewClosedState(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/test/fixtures/issueView_previewNoLabel.json b/test/fixtures/issueView_previewNoLabel.json new file mode 100644 index 000000000..1891100eb --- /dev/null +++ b/test/fixtures/issueView_previewNoLabel.json @@ -0,0 +1,25 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "**bold story**", + "title": "ix of coins", + "state": "OPEN", + "created_at": "2011-01-26T19:01:12Z", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} From 9a6026e9fc8c0b942a25d865f8c1e2621e8c699c Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Mon, 16 Mar 2020 04:32:41 +0900 Subject: [PATCH 11/23] Format PR/Issue states with color --- command/issue.go | 8 ++++++-- command/issue_test.go | 8 ++++---- command/pr.go | 12 +++++++----- command/pr_test.go | 10 +++++----- test/fixtures/prStatus.json | 18 ++++++++++++------ test/fixtures/prStatusFork.json | 2 ++ 6 files changed, 36 insertions(+), 22 deletions(-) diff --git a/command/issue.go b/command/issue.go index 5111fdec8..c6fba0452 100644 --- a/command/issue.go +++ b/command/issue.go @@ -248,6 +248,11 @@ func issueView(cmd *cobra.Command, args []string) error { } +func IssueStateTitleWithColor(state string) string { + colorFunc := utils.ColorFuncForState(state) + return colorFunc(strings.Title(strings.ToLower(state))) +} + func printIssuePreview(out io.Writer, issue *api.Issue) error { coloredLabels := labelList(*issue) if coloredLabels != "" { @@ -256,10 +261,9 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error { now := time.Now() ago := now.Sub(issue.CreatedAt) - issueStateColorFunc := utils.ColorFuncForState(issue.State) fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprintf(out, "%s", issueStateColorFunc(issue.State)) + fmt.Fprintf(out, "%s", IssueStateTitleWithColor(issue.State)) fmt.Fprint(out, utils.Gray(fmt.Sprintf( " • %s opened %s • %s", issue.Author.Login, diff --git a/command/issue_test.go b/command/issue_test.go index c4a9e3405..4730ef811 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -297,7 +297,7 @@ func TestIssueView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments • tarot`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -327,7 +327,7 @@ func TestIssueView_previewNoLabel(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -357,7 +357,7 @@ func TestIssueView_previewClosedState(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`CLOSED • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments • tarot`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -387,7 +387,7 @@ func TestIssueView_previewWithEmptyBody(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`OPEN • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments • tarot`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } for _, r := range expectedLines { diff --git a/command/pr.go b/command/pr.go index 5443cccec..0cf691291 100644 --- a/command/pr.go +++ b/command/pr.go @@ -228,6 +228,11 @@ func prList(cmd *cobra.Command, args []string) error { return nil } +func PRStateTitleWithColor(pr api.PullRequest) string { + prStateColorFunc := colorFuncForPR(pr) + return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) +} + func colorFuncForPR(pr api.PullRequest) func(string) string { if pr.State == "OPEN" && pr.IsDraft { return utils.Gray @@ -296,10 +301,8 @@ func prView(cmd *cobra.Command, args []string) error { } func printPrPreview(out io.Writer, pr *api.PullRequest) error { - prStateColorFunc := colorFuncForPR(*pr) - fmt.Fprintln(out, utils.Bold(pr.Title)) - fmt.Fprintf(out, "%s", prStateColorFunc(pr.State)) + fmt.Fprintf(out, "%s", PRStateTitleWithColor(*pr)) fmt.Fprintln(out, utils.Gray(fmt.Sprintf( " • %s wants to merge %s into %s from %s", pr.Author.Login, @@ -424,8 +427,7 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { fmt.Fprintf(w, " - %s", utils.Green("Approved")) } } else { - s := strings.Title(strings.ToLower(pr.State)) - fmt.Fprintf(w, " - %s", prStateColorFunc(s)) + fmt.Fprintf(w, " - %s", PRStateTitleWithColor(pr)) } fmt.Fprint(w, "\n") diff --git a/command/pr_test.go b/command/pr_test.go index b3336b640..a7a039c2e 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -399,7 +399,7 @@ func TestPRView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`OPEN • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), } @@ -429,7 +429,7 @@ func TestPRView_previewClosedState(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`CLOSED • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`Closed • nobody wants to merge 12 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), } @@ -459,7 +459,7 @@ func TestPRView_previewMergedState(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`MERGED • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`Merged • nobody wants to merge 12 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), } @@ -494,7 +494,7 @@ func TestPRView_previewCurrentBranch(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`OPEN • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), } @@ -529,7 +529,7 @@ func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`OPEN • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), } for _, r := range expectedLines { diff --git a/test/fixtures/prStatus.json b/test/fixtures/prStatus.json index d2996b71c..a226663a3 100644 --- a/test/fixtures/prStatus.json +++ b/test/fixtures/prStatus.json @@ -8,8 +8,10 @@ "node": { "number": 10, "title": "Blueberries are a good fruit", + "state": "OPEN", "url": "https://github.com/cli/cli/pull/10", "headRefName": "blueberries", + "isDraft": false, "headRepositoryOwner": { "login": "OWNER" }, @@ -26,8 +28,10 @@ "node": { "number": 8, "title": "Strawberries are not actually berries", + "state": "OPEN", "url": "https://github.com/cli/cli/pull/8", - "headRefName": "strawberries" + "headRefName": "strawberries", + "isDraft": false } } ] @@ -39,16 +43,18 @@ "node": { "number": 9, "title": "Apples are tasty", + "state": "OPEN", "url": "https://github.com/cli/cli/pull/9", - "headRefName": "apples" - } - }, - { + "headRefName": "apples", + "isDraft": false + } }, { "node": { "number": 11, "title": "Figs are my favorite", + "state": "OPEN", "url": "https://github.com/cli/cli/pull/1", - "headRefName": "figs" + "headRefName": "figs", + "isDraft": true } } ] diff --git a/test/fixtures/prStatusFork.json b/test/fixtures/prStatusFork.json index df184aaea..c9a7a5b3a 100644 --- a/test/fixtures/prStatusFork.json +++ b/test/fixtures/prStatusFork.json @@ -8,8 +8,10 @@ "node": { "number": 10, "title": "Blueberries are a good fruit", + "state": "OPEN", "url": "https://github.com/PARENT/REPO/pull/10", "headRefName": "blueberries", + "isDraft": false, "headRepositoryOwner": { "login": "OWNER" }, From e85259d84f56db5ceb530b151b6beaec0b084692 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 04:42:50 +0900 Subject: [PATCH 12/23] Remove labels from the issue preview header See https://github.com/cli/cli/issues/652#issuecomment-598940740 --- command/issue.go | 11 +---------- command/issue_test.go | 6 +++--- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/command/issue.go b/command/issue.go index c6fba0452..38f3c1a8b 100644 --- a/command/issue.go +++ b/command/issue.go @@ -254,26 +254,17 @@ func IssueStateTitleWithColor(state string) string { } func printIssuePreview(out io.Writer, issue *api.Issue) error { - coloredLabels := labelList(*issue) - if coloredLabels != "" { - coloredLabels = fmt.Sprintf("%s", coloredLabels) - } - now := time.Now() ago := now.Sub(issue.CreatedAt) fmt.Fprintln(out, utils.Bold(issue.Title)) fmt.Fprintf(out, "%s", IssueStateTitleWithColor(issue.State)) - fmt.Fprint(out, utils.Gray(fmt.Sprintf( + fmt.Fprintln(out, utils.Gray(fmt.Sprintf( " • %s opened %s • %s", issue.Author.Login, utils.FuzzyAgo(ago), utils.Pluralize(issue.Comments.TotalCount, "comment"), ))) - if coloredLabels != "" { - fmt.Fprintf(out, utils.Gray(fmt.Sprintf(" • %s", coloredLabels))) - } - fmt.Fprintf(out, "\n") if issue.Body != "" { fmt.Fprintln(out) diff --git a/command/issue_test.go b/command/issue_test.go index 4730ef811..7552b1e17 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -297,7 +297,7 @@ func TestIssueView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -357,7 +357,7 @@ func TestIssueView_previewClosedState(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } @@ -387,7 +387,7 @@ func TestIssueView_previewWithEmptyBody(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments • tarot`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } for _, r := range expectedLines { From 46a632c8f6436facb96944155a4c3800fcb8543c Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 04:52:03 +0900 Subject: [PATCH 13/23] Fix utility function scopes --- command/issue.go | 4 ++-- command/pr.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/command/issue.go b/command/issue.go index 38f3c1a8b..41d285c3e 100644 --- a/command/issue.go +++ b/command/issue.go @@ -248,7 +248,7 @@ func issueView(cmd *cobra.Command, args []string) error { } -func IssueStateTitleWithColor(state string) string { +func issueStateTitleWithColor(state string) string { colorFunc := utils.ColorFuncForState(state) return colorFunc(strings.Title(strings.ToLower(state))) } @@ -258,7 +258,7 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error { ago := now.Sub(issue.CreatedAt) fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprintf(out, "%s", IssueStateTitleWithColor(issue.State)) + fmt.Fprintf(out, "%s", issueStateTitleWithColor(issue.State)) fmt.Fprintln(out, utils.Gray(fmt.Sprintf( " • %s opened %s • %s", issue.Author.Login, diff --git a/command/pr.go b/command/pr.go index 0cf691291..af33d18c0 100644 --- a/command/pr.go +++ b/command/pr.go @@ -228,7 +228,7 @@ func prList(cmd *cobra.Command, args []string) error { return nil } -func PRStateTitleWithColor(pr api.PullRequest) string { +func prStateTitleWithColor(pr api.PullRequest) string { prStateColorFunc := colorFuncForPR(pr) return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) } @@ -302,7 +302,7 @@ func prView(cmd *cobra.Command, args []string) error { func printPrPreview(out io.Writer, pr *api.PullRequest) error { fmt.Fprintln(out, utils.Bold(pr.Title)) - fmt.Fprintf(out, "%s", PRStateTitleWithColor(*pr)) + fmt.Fprintf(out, "%s", prStateTitleWithColor(*pr)) fmt.Fprintln(out, utils.Gray(fmt.Sprintf( " • %s wants to merge %s into %s from %s", pr.Author.Login, @@ -427,7 +427,7 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { fmt.Fprintf(w, " - %s", utils.Green("Approved")) } } else { - fmt.Fprintf(w, " - %s", PRStateTitleWithColor(pr)) + fmt.Fprintf(w, " - %s", prStateTitleWithColor(pr)) } fmt.Fprint(w, "\n") From 0cfa4be86a54116e6d100cf740e95b262bf6ce2a Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 07:30:28 +0900 Subject: [PATCH 14/23] Add tests for utilities of PR/issue state format --- command/issue_test.go | 22 ++++++++++++++++++++++ command/pr.go | 3 +-- command/pr_test.go | 23 +++++++++++++++++++++++ go.mod | 1 + go.sum | 1 + 5 files changed, 48 insertions(+), 2 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 7552b1e17..2eeec2bb1 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/cli/cli/utils" + "github.com/google/go-cmp/cmp" ) func TestIssueStatus(t *testing.T) { @@ -585,3 +586,24 @@ func TestIssueCreate_webTitleBody(t *testing.T) { eq(t, url, "https://github.com/OWNER/REPO/issues/new?title=mytitle&body=mybody") eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") } + +func TestIssueStateTitleWithColor(t *testing.T) { + tests := map[string]struct { + state string + want string + }{ + + "Open state": {state: "OPEN", want: "Open"}, + "Closed state": {state: "CLOSED", want: "Closed"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := issueStateTitleWithColor(tc.state) + diff := cmp.Diff(tc.want, got) + if diff != "" { + t.Fatalf(diff) + } + }) + } +} diff --git a/command/pr.go b/command/pr.go index af33d18c0..542eafbc5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -236,9 +236,8 @@ func prStateTitleWithColor(pr api.PullRequest) string { func colorFuncForPR(pr api.PullRequest) func(string) string { if pr.State == "OPEN" && pr.IsDraft { return utils.Gray - } else { - return utils.ColorFuncForState(pr.State) } + return utils.ColorFuncForState(pr.State) } func prView(cmd *cobra.Command, args []string) error { diff --git a/command/pr_test.go b/command/pr_test.go index a7a039c2e..d007ac1ea 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/cli/cli/utils" + "github.com/google/go-cmp/cmp" "github.com/google/shlex" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -780,3 +781,25 @@ func TestReplaceExcessiveWhitespace(t *testing.T) { eq(t, replaceExcessiveWhitespace("hello goodbye"), "hello goodbye") eq(t, replaceExcessiveWhitespace(" hello \n goodbye "), "hello goodbye") } + +func TestPrStateTitleWithColor(t *testing.T) { + tests := map[string]struct { + state string + want string + }{ + + "Format OPEN state": {state: "OPEN", want: "Open"}, + "Format CLOSED state": {state: "CLOSED", want: "Closed"}, + "Format MERGED state": {state: "MERGED", want: "Merged"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := issueStateTitleWithColor(tc.state) + diff := cmp.Diff(tc.want, got) + if diff != "" { + t.Fatalf(diff) + } + }) + } +} diff --git a/go.mod b/go.mod index 63a39c1bf..f45918576 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/briandowns/spinner v1.9.0 github.com/charmbracelet/glamour v0.1.1-0.20200304134224-7e5c90143acc github.com/dlclark/regexp2 v1.2.0 // indirect + github.com/google/go-cmp v0.2.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/hashicorp/go-version v1.2.0 github.com/henvic/httpretty v0.0.4 diff --git a/go.sum b/go.sum index 8d7665e96..737961e33 100644 --- a/go.sum +++ b/go.sum @@ -60,6 +60,7 @@ github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfb github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= +github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/goterm v0.0.0-20190703233501-fc88cf888a3f h1:5CjVwnuUcp5adK4gmY6i72gpVFVnZDP2h5TmPScB6u4= github.com/google/goterm v0.0.0-20190703233501-fc88cf888a3f/go.mod h1:nOFQdrUlIlx6M6ODdSpBj1NVA+VgLC6kmw60mkw34H4= From 7b5a0b56945500fae643190cfe53dc6d77102a77 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 07:32:18 +0900 Subject: [PATCH 15/23] Add a missing isDraft for querying PR by number --- api/queries_pr.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/queries_pr.go b/api/queries_pr.go index a125a7623..b30b42209 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -321,6 +321,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu } } isCrossRepository + isDraft maintainerCanModify } } From 762e806c9eed784d732899425c16f6adb51c6f61 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 08:23:29 +0900 Subject: [PATCH 16/23] Add tests for viewing a Draft PR in CLI --- command/pr_test.go | 62 +++++++++++++++++++++++++++++++- test/fixtures/prView.json | 6 ++-- test/fixtures/prViewPreview.json | 3 +- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/command/pr_test.go b/command/pr_test.go index d007ac1ea..7eec86d9f 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -382,7 +382,7 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { } } -func TestPRView_preview(t *testing.T) { +func TestPRView_previewOpenState(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") @@ -472,6 +472,36 @@ func TestPRView_previewMergedState(t *testing.T) { } } +func TestPRView_previewDraftState(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prViewPreviewDraftState.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prViewCmd, "pr view -p 12") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_previewCurrentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -507,6 +537,36 @@ func TestPRView_previewCurrentBranch(t *testing.T) { } } +func TestPRView_previewDraftStatebyBranch(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prViewPreviewDraftStatebyBranch.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prViewCmd, "pr view -p blueberries") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() diff --git a/test/fixtures/prView.json b/test/fixtures/prView.json index 2b6b4f9b5..826f10100 100644 --- a/test/fixtures/prView.json +++ b/test/fixtures/prView.json @@ -20,7 +20,8 @@ "author": { "login": "nobody" }, - "isCrossRepository": true + "isCrossRepository": true, + "isDraft": false }, { "number": 10, @@ -39,7 +40,8 @@ "commits": { "totalCount": 8 }, - "isCrossRepository": false + "isCrossRepository": false, + "isDraft": false } ] } diff --git a/test/fixtures/prViewPreview.json b/test/fixtures/prViewPreview.json index 2b551a18f..379fd253b 100644 --- a/test/fixtures/prViewPreview.json +++ b/test/fixtures/prViewPreview.json @@ -18,7 +18,8 @@ "headRepositoryOwner": { "login": "hubot" }, - "isCrossRepository": true + "isCrossRepository": true, + "isDraft": false } } } From cf69d7e47f1164ea13974e9a81c69d0fd3b3a572 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 08:53:12 +0900 Subject: [PATCH 17/23] Add test fixtures for viewing Draft state in CLI --- test/fixtures/prViewPreviewDraftState.json | 26 ++++++++++ .../prViewPreviewDraftStatebyBranch.json | 50 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 test/fixtures/prViewPreviewDraftState.json create mode 100644 test/fixtures/prViewPreviewDraftStatebyBranch.json diff --git a/test/fixtures/prViewPreviewDraftState.json b/test/fixtures/prViewPreviewDraftState.json new file mode 100644 index 000000000..69ab57b11 --- /dev/null +++ b/test/fixtures/prViewPreviewDraftState.json @@ -0,0 +1,26 @@ +{ + "data": { + "repository": { + "pullRequest": { + "number": 12, + "title": "Blueberries are from a fork", + "state": "OPEN", + "body": "**blueberries taste good**", + "url": "https://github.com/OWNER/REPO/pull/12", + "author": { + "login": "nobody" + }, + "commits": { + "totalCount": 12 + }, + "baseRefName": "master", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true, + "isDraft": true + } + } + } +} diff --git a/test/fixtures/prViewPreviewDraftStatebyBranch.json b/test/fixtures/prViewPreviewDraftStatebyBranch.json new file mode 100644 index 000000000..c9d0eef59 --- /dev/null +++ b/test/fixtures/prViewPreviewDraftStatebyBranch.json @@ -0,0 +1,50 @@ +{ + "data": { + "repository": { + "pullRequests": { + "nodes": [ + { + "number": 12, + "title": "Blueberries are from a fork", + "state": "OPEN", + "body": "yeah", + "url": "https://github.com/OWNER/REPO/pull/12", + "headRefName": "blueberries", + "baseRefName": "master", + "headRepositoryOwner": { + "login": "hubot" + }, + "commits": { + "totalCount": 12 + }, + "author": { + "login": "nobody" + }, + "isCrossRepository": true, + "isDraft": false + }, + { + "number": 10, + "title": "Blueberries are a good fruit", + "state": "OPEN", + "body": "**blueberries taste good**", + "url": "https://github.com/OWNER/REPO/pull/10", + "baseRefName": "master", + "headRefName": "blueberries", + "author": { + "login": "nobody" + }, + "headRepositoryOwner": { + "login": "OWNER" + }, + "commits": { + "totalCount": 8 + }, + "isCrossRepository": false, + "isDraft": true + } + ] + } + } + } +} From e50ba549d81ac50e2f65576fc5d4c932ff0443cc Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 09:55:47 +0900 Subject: [PATCH 18/23] Rename OPEN state to Draft for a Draft PR --- command/pr.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/command/pr.go b/command/pr.go index 542eafbc5..2397e96a8 100644 --- a/command/pr.go +++ b/command/pr.go @@ -230,6 +230,9 @@ func prList(cmd *cobra.Command, args []string) error { func prStateTitleWithColor(pr api.PullRequest) string { prStateColorFunc := colorFuncForPR(pr) + if pr.State == "OPEN" && pr.IsDraft { + return prStateColorFunc(strings.Title(strings.ToLower("Draft"))) + } return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) } From 327a80495983de42ad87be8da8ee4c28c88d8d5d Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 09:57:46 +0900 Subject: [PATCH 19/23] Fix tests to support the "Draft" state badge --- command/pr_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/command/pr_test.go b/command/pr_test.go index b178b9347..a73d6c99e 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/cli/cli/api" "github.com/cli/cli/test" "github.com/cli/cli/utils" "github.com/google/go-cmp/cmp" @@ -491,7 +492,7 @@ func TestPRView_previewDraftState(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`Draft • nobody wants to merge 12 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), } @@ -556,7 +557,7 @@ func TestPRView_previewDraftStatebyBranch(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`Draft • nobody wants to merge 8 commits into master from blueberries`), regexp.MustCompile(`blueberries taste good`), regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), } @@ -844,19 +845,21 @@ func TestReplaceExcessiveWhitespace(t *testing.T) { } func TestPrStateTitleWithColor(t *testing.T) { + tests := map[string]struct { - state string - want string + pr api.PullRequest + want string }{ - "Format OPEN state": {state: "OPEN", want: "Open"}, - "Format CLOSED state": {state: "CLOSED", want: "Closed"}, - "Format MERGED state": {state: "MERGED", want: "Merged"}, + "Format OPEN state": {pr: api.PullRequest{State: "OPEN", IsDraft: false}, want: "Open"}, + "Format OPEN state for Draft PR": {pr: api.PullRequest{State: "OPEN", IsDraft: true}, want: "Draft"}, + "Format CLOSED state": {pr: api.PullRequest{State: "CLOSED", IsDraft: false}, want: "Closed"}, + "Format MERGED state": {pr: api.PullRequest{State: "MERGED", IsDraft: false}, want: "Merged"}, } for name, tc := range tests { t.Run(name, func(t *testing.T) { - got := issueStateTitleWithColor(tc.state) + got := prStateTitleWithColor(tc.pr) diff := cmp.Diff(tc.want, got) if diff != "" { t.Fatalf(diff) From a5bd3130c413576f33f4f16bbe320a9a861c9449 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 18 Mar 2020 10:03:04 +0900 Subject: [PATCH 20/23] Cleanup --- command/issue_test.go | 1 - command/pr_test.go | 2 -- 2 files changed, 3 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 9ee2a692b..6d009ca98 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -593,7 +593,6 @@ func TestIssueStateTitleWithColor(t *testing.T) { state string want string }{ - "Open state": {state: "OPEN", want: "Open"}, "Closed state": {state: "CLOSED", want: "Closed"}, } diff --git a/command/pr_test.go b/command/pr_test.go index a73d6c99e..27f0a064d 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -845,12 +845,10 @@ func TestReplaceExcessiveWhitespace(t *testing.T) { } func TestPrStateTitleWithColor(t *testing.T) { - tests := map[string]struct { pr api.PullRequest want string }{ - "Format OPEN state": {pr: api.PullRequest{State: "OPEN", IsDraft: false}, want: "Open"}, "Format OPEN state for Draft PR": {pr: api.PullRequest{State: "OPEN", IsDraft: true}, want: "Draft"}, "Format CLOSED state": {pr: api.PullRequest{State: "CLOSED", IsDraft: false}, want: "Closed"}, From 1a5e9f1b6120509bfc0e9dac2188b258d72a4589 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 20 Mar 2020 16:32:52 +0900 Subject: [PATCH 21/23] Apply table driven testing for pr/issue preview commands --- command/issue_test.go | 181 ++++++++++-------------- command/pr_test.go | 313 ++++++++++++++---------------------------- 2 files changed, 173 insertions(+), 321 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 6d009ca98..3ea2539bf 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -281,122 +281,81 @@ func TestIssueView_numberArgWithHash(t *testing.T) { eq(t, url, "https://github.com/OWNER/REPO/issues/123") } -func TestIssueView_preview(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/issueView_preview.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(issueViewCmd, "issue view -p 123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) +func TestIssueView_Preview(t *testing.T) { + tests := map[string]struct { + ownerRepo string + command string + fixture string + expectedOutputs []*regexp.Regexp + }{ + "Open issue": { + ownerRepo: "master", + command: "issue view -p 123", + fixture: "../test/fixtures/issueView_preview.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`bold story`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + }, + }, + "Open issue with no label": { + ownerRepo: "master", + command: "issue view -p 123", + fixture: "../test/fixtures/issueView_previewNoLabel.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`bold story`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + }, + }, + "Open issue with empty body": { + ownerRepo: "master", + command: "issue view -p 123", + fixture: "../test/fixtures/issueView_previewWithEmptyBody.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + }, + }, + "Closed issue": { + ownerRepo: "master", + command: "issue view -p 123", + fixture: "../test/fixtures/issueView_previewClosedState.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments`), + regexp.MustCompile(`bold story`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + }, + }, } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + initBlankContext("OWNER/REPO", tc.ownerRepo) + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") - eq(t, output.Stderr(), "") + jsonFile, _ := os.Open(tc.fixture) + defer jsonFile.Close() + http.StubResponse(200, jsonFile) - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} + output, err := RunCommand(issueViewCmd, tc.command) + if err != nil { + t.Errorf("error running command `%v`: %v", tc.command, err) + } -func TestIssueView_previewNoLabel(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") + eq(t, output.Stderr(), "") - jsonFile, _ := os.Open("../test/fixtures/issueView_previewNoLabel.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(issueViewCmd, "issue view -p 123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestIssueView_previewClosedState(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/issueView_previewClosedState.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(issueViewCmd, "issue view -p 123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestIssueView_previewWithEmptyBody(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/issueView_previewWithEmptyBody.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(issueViewCmd, "issue view -p 123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } + for _, r := range tc.expectedOutputs { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } + }) } } diff --git a/command/pr_test.go b/command/pr_test.go index 27f0a064d..d3864a046 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -384,222 +384,115 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { } } -func TestPRView_previewOpenState(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prViewPreview.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(prViewCmd, "pr view -p 12") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) +func TestPRView_Preview(t *testing.T) { + tests := map[string]struct { + ownerRepo string + args string + fixture string + expectedOutputs []*regexp.Regexp + }{ + "Open PR": { + ownerRepo: "master", + args: "pr view -p 12", + fixture: "../test/fixtures/prViewPreview.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + }, + }, + "Open PR for the current branch": { + ownerRepo: "blueberries", + args: "pr view -p", + fixture: "../test/fixtures/prView.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + }, + }, + "Open PR wth empty body for the current branch": { + ownerRepo: "blueberries", + args: "pr view -p", + fixture: "../test/fixtures/prView_EmptyBody.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + }, + }, + "Closed PR": { + ownerRepo: "master", + args: "pr view -p 12", + fixture: "../test/fixtures/prViewPreviewClosedState.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`Closed • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + }, + }, + "Merged PR": { + ownerRepo: "master", + args: "pr view -p 12", + fixture: "../test/fixtures/prViewPreviewMergedState.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`Merged • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + }, + }, + "Draft PR": { + ownerRepo: "master", + args: "pr view -p 12", + fixture: "../test/fixtures/prViewPreviewDraftState.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are from a fork`), + regexp.MustCompile(`Draft • nobody wants to merge 12 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + }, + }, + "Draft PR by branch": { + ownerRepo: "master", + args: "pr view -p blueberries", + fixture: "../test/fixtures/prViewPreviewDraftStatebyBranch.json", + expectedOutputs: []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`Draft • nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + }, + }, } - eq(t, output.Stderr(), "") + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + initBlankContext("OWNER/REPO", tc.ownerRepo) + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} + jsonFile, _ := os.Open(tc.fixture) + defer jsonFile.Close() + http.StubResponse(200, jsonFile) -func TestPRView_previewClosedState(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") + output, err := RunCommand(prViewCmd, tc.args) + if err != nil { + t.Errorf("error running command `%v`: %v", tc.args, err) + } - jsonFile, _ := os.Open("../test/fixtures/prViewPreviewClosedState.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + eq(t, output.Stderr(), "") - output, err := RunCommand(prViewCmd, "pr view -p 12") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Closed • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRView_previewMergedState(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prViewPreviewMergedState.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(prViewCmd, "pr view -p 12") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Merged • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRView_previewDraftState(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prViewPreviewDraftState.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(prViewCmd, "pr view -p 12") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Draft • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRView_previewCurrentBranch(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) - - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand(prViewCmd, "pr view -p") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRView_previewDraftStatebyBranch(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prViewPreviewDraftStatebyBranch.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(prViewCmd, "pr view -p blueberries") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Draft • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - jsonFile, _ := os.Open("../test/fixtures/prView_EmptyBody.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand(prViewCmd, "pr view -p") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.Stderr(), "") - - expectedLines := []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), - } - for _, r := range expectedLines { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } + for _, r := range tc.expectedOutputs { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } + }) } } From 4c2f15fe37e7eaf9b0212e67cae200e46f5c6893 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Thu, 2 Apr 2020 23:58:28 +0900 Subject: [PATCH 22/23] Roll back the place of colorFuncForState --- command/issue.go | 4 ++-- command/pr.go | 16 +++++++++++++++- utils/utils.go | 14 -------------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/command/issue.go b/command/issue.go index ecbb3930a..b970a2a59 100644 --- a/command/issue.go +++ b/command/issue.go @@ -229,7 +229,7 @@ func issueView(cmd *cobra.Command, args []string) error { } func issueStateTitleWithColor(state string) string { - colorFunc := utils.ColorFuncForState(state) + colorFunc := colorFuncForState(state) return colorFunc(strings.Title(strings.ToLower(state))) } @@ -425,7 +425,7 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue) } now := time.Now() ago := now.Sub(issue.UpdatedAt) - table.AddField(issueNum, nil, utils.ColorFuncForState(issue.State)) + table.AddField(issueNum, nil, colorFuncForState(issue.State)) table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil) table.AddField(labels, nil, utils.Gray) table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray) diff --git a/command/pr.go b/command/pr.go index cdfca9be3..e494d88b5 100644 --- a/command/pr.go +++ b/command/pr.go @@ -238,7 +238,21 @@ func colorFuncForPR(pr api.PullRequest) func(string) string { if pr.State == "OPEN" && pr.IsDraft { return utils.Gray } - return utils.ColorFuncForState(pr.State) + return colorFuncForState(pr.State) +} + +// colorFuncForState returns a color function for a PR/Issue state +func colorFuncForState(state string) func(string) string { + switch state { + case "OPEN": + return utils.Green + case "CLOSED": + return utils.Red + case "MERGED": + return utils.Magenta + default: + return nil + } } func prView(cmd *cobra.Command, args []string) error { diff --git a/utils/utils.go b/utils/utils.go index 5c221bfa3..4510db793 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -75,17 +75,3 @@ func Spinner(w io.Writer) *spinner.Spinner { s.Writer = w return s } - -// ColorFuncForState returns a color function for a PR/Issue state -func ColorFuncForState(state string) func(string) string { - switch state { - case "OPEN": - return Green - case "CLOSED": - return Red - case "MERGED": - return Magenta - default: - return nil - } -} From 7eabbf58819cb0371340d6261131a57c3e949b38 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 3 Apr 2020 00:35:48 +0900 Subject: [PATCH 23/23] Simplify expected output definitions with new line testing helper --- command/issue_test.go | 47 ++++++++++++-------------- command/pr_test.go | 77 ++++++++++++++++++++----------------------- 2 files changed, 57 insertions(+), 67 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index fc29a7a81..d9e2846d0 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -290,49 +290,49 @@ func TestIssueView_Preview(t *testing.T) { ownerRepo string command string fixture string - expectedOutputs []*regexp.Regexp + expectedOutputs []string }{ "Open issue": { ownerRepo: "master", command: "issue view 123", fixture: "../test/fixtures/issueView_preview.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + expectedOutputs: []string{ + "ix of coins", + "Open • marseilles opened about 292 years ago • 9 comments", + "bold story", + "View this issue on GitHub: https://github.com/OWNER/REPO/issues/123", }, }, "Open issue with no label": { ownerRepo: "master", command: "issue view 123", fixture: "../test/fixtures/issueView_previewNoLabel.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + expectedOutputs: []string{ + "ix of coins", + "Open • marseilles opened about 292 years ago • 9 comments", + "bold story", + "View this issue on GitHub: https://github.com/OWNER/REPO/issues/123", }, }, "Open issue with empty body": { ownerRepo: "master", command: "issue view 123", fixture: "../test/fixtures/issueView_previewWithEmptyBody.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Open • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + expectedOutputs: []string{ + "ix of coins", + "Open • marseilles opened about 292 years ago • 9 comments", + "View this issue on GitHub: https://github.com/OWNER/REPO/issues/123", }, }, "Closed issue": { ownerRepo: "master", command: "issue view 123", fixture: "../test/fixtures/issueView_previewClosedState.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`Closed • marseilles opened about 292 years ago • 9 comments`), - regexp.MustCompile(`bold story`), - regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + expectedOutputs: []string{ + "ix of coins", + "Closed • marseilles opened about 292 years ago • 9 comments", + "bold story", + "View this issue on GitHub: https://github.com/OWNER/REPO/issues/123", }, }, } @@ -353,12 +353,7 @@ func TestIssueView_Preview(t *testing.T) { eq(t, output.Stderr(), "") - for _, r := range tc.expectedOutputs { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } + test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } } diff --git a/command/pr_test.go b/command/pr_test.go index a19279a9e..2a68781b0 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -407,82 +407,82 @@ func TestPRView_Preview(t *testing.T) { ownerRepo string args string fixture string - expectedOutputs []*regexp.Regexp + expectedOutputs []string }{ "Open PR": { ownerRepo: "master", args: "pr view 12", fixture: "../test/fixtures/prViewPreview.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Open • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + expectedOutputs: []string{ + "Blueberries are from a fork", + "Open • nobody wants to merge 12 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12", }, }, "Open PR for the current branch": { ownerRepo: "blueberries", args: "pr view", fixture: "../test/fixtures/prView.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + expectedOutputs: []string{ + "Blueberries are a good fruit", + "Open • nobody wants to merge 8 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10", }, }, "Open PR wth empty body for the current branch": { ownerRepo: "blueberries", args: "pr view", fixture: "../test/fixtures/prView_EmptyBody.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Open • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + expectedOutputs: []string{ + "Blueberries are a good fruit", + "Open • nobody wants to merge 8 commits into master from blueberries", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10", }, }, "Closed PR": { ownerRepo: "master", args: "pr view 12", fixture: "../test/fixtures/prViewPreviewClosedState.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Closed • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + expectedOutputs: []string{ + "Blueberries are from a fork", + "Closed • nobody wants to merge 12 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12", }, }, "Merged PR": { ownerRepo: "master", args: "pr view 12", fixture: "../test/fixtures/prViewPreviewMergedState.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Merged • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + expectedOutputs: []string{ + "Blueberries are from a fork", + "Merged • nobody wants to merge 12 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12", }, }, "Draft PR": { ownerRepo: "master", args: "pr view 12", fixture: "../test/fixtures/prViewPreviewDraftState.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are from a fork`), - regexp.MustCompile(`Draft • nobody wants to merge 12 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`), + expectedOutputs: []string{ + "Blueberries are from a fork", + "Draft • nobody wants to merge 12 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12", }, }, "Draft PR by branch": { ownerRepo: "master", args: "pr view blueberries", fixture: "../test/fixtures/prViewPreviewDraftStatebyBranch.json", - expectedOutputs: []*regexp.Regexp{ - regexp.MustCompile(`Blueberries are a good fruit`), - regexp.MustCompile(`Draft • nobody wants to merge 8 commits into master from blueberries`), - regexp.MustCompile(`blueberries taste good`), - regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + expectedOutputs: []string{ + "Blueberries are a good fruit", + "Draft • nobody wants to merge 8 commits into master from blueberries", + "blueberries taste good", + "View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10", }, }, } @@ -504,12 +504,7 @@ func TestPRView_Preview(t *testing.T) { eq(t, output.Stderr(), "") - for _, r := range tc.expectedOutputs { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } + test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } }