From 2b53172ebe681fc75f617780dc34a00e3799fa4a Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 16 Dec 2019 10:54:22 -0800 Subject: [PATCH 01/10] Add line for pr create --- command/pr_create.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/command/pr_create.go b/command/pr_create.go index abff4e5be..dbc6f19de 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -45,6 +45,8 @@ func prCreate(cmd *cobra.Command, _ []string) error { return err } + fmt.Fprintf(colorableOut(cmd), "Creating pull request for %s in %s/%s\n", utils.Cyan("["+head+"]"), repo.RepoOwner(), repo.RepoName()) + if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { return err } From 6d0f4e7a90d3953c7d5ae74d71a1a6a44a9684cb Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 16 Dec 2019 11:41:56 -0800 Subject: [PATCH 02/10] Add more text --- command/issue.go | 4 ++++ command/pr.go | 2 ++ command/pr_create.go | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index c46caf5af..3bfe603c8 100644 --- a/command/issue.go +++ b/command/issue.go @@ -103,6 +103,8 @@ func issueList(cmd *cobra.Command, args []string) error { return err } + fmt.Fprintf(colorableOut(cmd), "\nIssues for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) if err != nil { return err @@ -241,6 +243,8 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } + fmt.Fprintf(colorableOut(cmd), "\nCreating issue in %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository openURL := fmt.Sprintf("https://github.com/%s/%s/issues/new", baseRepo.RepoOwner(), baseRepo.RepoName()) diff --git a/command/pr.go b/command/pr.go index 584fd3a8f..6a462231b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -137,6 +137,8 @@ func prList(cmd *cobra.Command, args []string) error { return err } + fmt.Fprintf(colorableOut(cmd), "\nPull requests for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + limit, err := cmd.Flags().GetInt("limit") if err != nil { return err diff --git a/command/pr_create.go b/command/pr_create.go index dbc6f19de..dbfde093d 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -45,7 +45,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { return err } - fmt.Fprintf(colorableOut(cmd), "Creating pull request for %s in %s/%s\n", utils.Cyan("["+head+"]"), repo.RepoOwner(), repo.RepoName()) + fmt.Fprintf(colorableOut(cmd), "\nCreating pull request for %s in %s/%s\n\n", utils.Cyan(head), repo.RepoOwner(), repo.RepoName()) if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { return err From 91267a65fad96945d0a4a6608e30a0bf4a608b27 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 16 Dec 2019 13:18:16 -0800 Subject: [PATCH 03/10] Fix tests --- command/issue_test.go | 12 ++++++++++-- command/pr_create_test.go | 9 ++++++++- command/pr_test.go | 5 ++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 45abfe04b..432dd8ba3 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -255,7 +255,11 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, output, "https://github.com/OWNER/REPO/issues/12\n") + eq(t, output, ` +Creating issue in OWNER/REPO + +https://github.com/OWNER/REPO/issues/12 +`) } func TestIssueCreate_web(t *testing.T) { @@ -279,5 +283,9 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output, "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") + eq(t, output, ` +Creating issue in OWNER/REPO + +Opening https://github.com/OWNER/REPO/issues/new in your browser. +`) } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 50e280d69..a1cdc7b94 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -91,7 +91,11 @@ func TestPRCreate(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output, "https://github.com/OWNER/REPO/pull/12\n") + eq(t, output, ` +Creating pull request for feature in OWNER/REPO + +https://github.com/OWNER/REPO/pull/12 +`) } func TestPRCreate_web(t *testing.T) { @@ -156,6 +160,9 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { eq(t, err, nil) eq(t, output, `Warning: 1 uncommitted change + +Creating pull request for feature in OWNER/REPO + https://github.com/OWNER/REPO/pull/12 `) } diff --git a/command/pr_test.go b/command/pr_test.go index dadeeada0..f8bdf7283 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -91,7 +91,10 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - eq(t, output, `32 New feature feature + eq(t, output, ` +Pull requests for OWNER/REPO + +32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) From 2c1fc56967e2e0257c50e69659f578ad68821a8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 17 Dec 2019 13:01:45 +0100 Subject: [PATCH 04/10] Update output expectations --- command/issue_test.go | 2 +- command/pr_create_test.go | 2 +- command/pr_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index dbb41e8a7..90588730a 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -109,7 +109,7 @@ func TestIssueList_withFlags(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.String(), "") + eq(t, output.String(), "\nIssues for OWNER/REPO\n\n") eq(t, output.Stderr(), "No issues match your search\n") bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 7bbd249fa..433608d01 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -119,7 +119,7 @@ func TestPRCreate_web(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create --web`) eq(t, err, nil) - eq(t, output.String(), "") + eq(t, output.String(), "\nCreating pull request for feature in OWNER/REPO\n\n") eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") eq(t, len(ranCommands), 3) diff --git a/command/pr_test.go b/command/pr_test.go index 44d00d34f..09e050334 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -129,7 +129,7 @@ func TestPRList_filtering(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), "") + eq(t, output.String(), "\nPull requests for OWNER/REPO\n\n") eq(t, output.Stderr(), "No pull requests match your search\n") bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) From b98bd2cfb825ec6377bf69aa8adc820268e04017 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 10:58:25 -0800 Subject: [PATCH 05/10] Use base branch --- command/pr_create.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/command/pr_create.go b/command/pr_create.go index 132cb3244..df457d407 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -45,7 +45,15 @@ func prCreate(cmd *cobra.Command, _ []string) error { return err } - fmt.Fprintf(colorableOut(cmd), "\nCreating pull request for %s in %s/%s\n\n", utils.Cyan(head), repo.RepoOwner(), repo.RepoName()) + target, err := cmd.Flags().GetString("base") + if err != nil { + return err + } + if target == "" { + target = "master" + } + + fmt.Fprintf(colorableErr(cmd), "\nCreating pull request for %s into %s in %s/%s\n\n", utils.Cyan(head), utils.Cyan(target), repo.RepoOwner(), repo.RepoName()) if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { return err From b66c34e0ecdde82a30d51666f4aa6063bbffcdc5 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 11:16:27 -0800 Subject: [PATCH 06/10] Output to stderr --- command/issue.go | 4 ++-- command/pr.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/command/issue.go b/command/issue.go index fcbbcd144..2847fce5e 100644 --- a/command/issue.go +++ b/command/issue.go @@ -103,7 +103,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableOut(cmd), "\nIssues for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nIssues for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) if err != nil { @@ -243,7 +243,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableOut(cmd), "\nCreating issue in %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository diff --git a/command/pr.go b/command/pr.go index 158c873ec..97e539e6a 100644 --- a/command/pr.go +++ b/command/pr.go @@ -137,7 +137,7 @@ func prList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableOut(cmd), "\nPull requests for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) limit, err := cmd.Flags().GetInt("limit") if err != nil { From d025d2d4f3c968c93a737fed3f2bba65776eb251 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 11:24:03 -0800 Subject: [PATCH 07/10] Revert test changes --- command/issue_test.go | 111 ++++++++++++++++++++++++++++++++------ command/pr_create_test.go | 14 ++--- command/pr_test.go | 65 +++++++++++++++++++--- 3 files changed, 156 insertions(+), 34 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 90588730a..47924e865 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -45,11 +45,12 @@ func TestIssueStatus_blankSlate(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { - "assigned": { "issues": { "nodes": [] } }, - "mentioned": { "issues": { "nodes": [] } }, - "authored": { "issues": { "nodes": [] } } - } } + { "data": { "repository": { + "hasIssuesEnabled": true, + "assigned": { "nodes": [] }, + "mentioned": { "nodes": [] }, + "authored": { "nodes": [] } + } } } `)) output, err := RunCommand(issueStatusCmd, "issue status") @@ -72,6 +73,22 @@ Issues opened by you } } +func TestIssueStatus_disabledIssues(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueStatusCmd, "issue status") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue status`: %v", err) + } +} + func TestIssueList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -100,16 +117,22 @@ func TestIssueList(t *testing.T) { } func TestIssueList_withFlags(t *testing.T) { + initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } } + `)) output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") if err != nil { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.String(), "\nIssues for OWNER/REPO\n\n") + eq(t, output.String(), "") eq(t, output.Stderr(), "No issues match your search\n") bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) @@ -127,6 +150,50 @@ func TestIssueList_withFlags(t *testing.T) { eq(t, reqBody.Variables.States, []string{"OPEN"}) } +func TestIssueList_nullAssigneeLabels(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } } + `)) + + _, err := RunCommand(issueListCmd, "issue list") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + reqBody := struct { + Variables map[string]interface{} + }{} + json.Unmarshal(bodyBytes, &reqBody) + + _, assigneeDeclared := reqBody.Variables["assignee"] + _, labelsDeclared := reqBody.Variables["labels"] + eq(t, assigneeDeclared, false) + eq(t, labelsDeclared, false) +} + +func TestIssueList_disabledIssues(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueListCmd, "issue list") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue list`: %v", err) + } +} + func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -225,7 +292,8 @@ func TestIssueCreate(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "id": "REPOID" + "id": "REPOID", + "hasIssuesEnabled": true } } } `)) http.StubResponse(200, bytes.NewBufferString(` @@ -255,11 +323,24 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, output.String(), ` -Creating issue in OWNER/REPO + eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") +} -https://github.com/OWNER/REPO/issues/12 -`) +func TestIssueCreate_disabledIssues(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueCreateCmd, `issue create -t heres -b johnny`) + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue create`: %v", err) + } } func TestIssueCreate_web(t *testing.T) { @@ -283,9 +364,5 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output.String(), ` -Creating issue in OWNER/REPO - -Opening https://github.com/OWNER/REPO/issues/new in your browser. -`) + eq(t, output.String(), "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 433608d01..badfe7204 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -91,11 +91,7 @@ func TestPRCreate(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), ` -Creating pull request for feature in OWNER/REPO - -https://github.com/OWNER/REPO/pull/12 -`) + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } func TestPRCreate_web(t *testing.T) { @@ -119,7 +115,7 @@ func TestPRCreate_web(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create --web`) eq(t, err, nil) - eq(t, output.String(), "\nCreating pull request for feature in OWNER/REPO\n\n") + eq(t, output.String(), "") eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") eq(t, len(ranCommands), 3) @@ -158,10 +154,6 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - eq(t, output.String(), ` -Creating pull request for feature in OWNER/REPO - -https://github.com/OWNER/REPO/pull/12 -`) + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") eq(t, output.Stderr(), "Warning: 1 uncommitted change\n") } diff --git a/command/pr_test.go b/command/pr_test.go index 09e050334..91ec11a13 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -50,12 +50,14 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { cmd.SetErr(&errBuf) // Reset flag values so they don't leak between tests + // FIXME: change how we initialize Cobra commands to render this hack unnecessary cmd.Flags().VisitAll(func(f *pflag.Flag) { switch v := f.Value.(type) { case pflag.SliceValue: v.Replace([]string{}) default: - if v.Type() == "bool" { + switch v.Type() { + case "bool", "string": v.Set(f.DefValue) } } @@ -95,6 +97,60 @@ func TestPRStatus(t *testing.T) { } } +func TestPRStatus_reviewsAndChecks(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prStatusCmd, "pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := []string{ + "- Checks passing - changes requested", + "- Checks pending - approved", + "- 1/3 checks failing - review required", + } + + for _, line := range expected { + if !strings.Contains(output.String(), line) { + t.Errorf("output did not contain %q: %q", line, output.String()) + } + } +} + +func TestPRStatus_blankSlate(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": {} } + `)) + + output, err := RunCommand(prStatusCmd, "pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := `Current branch + There is no pull request associated with [blueberries] + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} + func TestPRList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -108,10 +164,7 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), ` -Pull requests for OWNER/REPO - -32 New feature feature + eq(t, output.String(), `32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) @@ -129,7 +182,7 @@ func TestPRList_filtering(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), "\nPull requests for OWNER/REPO\n\n") + eq(t, output.String(), "") eq(t, output.Stderr(), "No pull requests match your search\n") bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) From bcf1f98702fb008fe85843848296a5995c0cbf2e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 11:32:27 -0800 Subject: [PATCH 08/10] Do a better job of reverting test changes --- command/issue_test.go | 123 ++++++-------------------------------- command/pr_create_test.go | 12 ++-- command/pr_test.go | 111 +++++++--------------------------- 3 files changed, 48 insertions(+), 198 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 47924e865..45abfe04b 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -33,7 +33,7 @@ func TestIssueStatus(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output.String()) { + if !r.MatchString(output) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -45,12 +45,11 @@ func TestIssueStatus_blankSlate(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "assigned": { "nodes": [] }, - "mentioned": { "nodes": [] }, - "authored": { "nodes": [] } - } } } + { "data": { + "assigned": { "issues": { "nodes": [] } }, + "mentioned": { "issues": { "nodes": [] } }, + "authored": { "issues": { "nodes": [] } } + } } `)) output, err := RunCommand(issueStatusCmd, "issue status") @@ -68,27 +67,11 @@ Issues opened by you There are no issues opened by you ` - if output.String() != expectedOutput { + if output != expectedOutput { t.Errorf("expected %q, got %q", expectedOutput, output) } } -func TestIssueStatus_disabledIssues(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand(issueStatusCmd, "issue status") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue status`: %v", err) - } -} - func TestIssueList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -109,7 +92,7 @@ func TestIssueList(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output.String()) { + if !r.MatchString(output) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -117,24 +100,15 @@ func TestIssueList(t *testing.T) { } func TestIssueList_withFlags(t *testing.T) { - initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } } - `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response - output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") + _, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") if err != nil { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "No issues match your search\n") - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -150,50 +124,6 @@ func TestIssueList_withFlags(t *testing.T) { eq(t, reqBody.Variables.States, []string{"OPEN"}) } -func TestIssueList_nullAssigneeLabels(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } } - `)) - - _, err := RunCommand(issueListCmd, "issue list") - if err != nil { - t.Errorf("error running command `issue list`: %v", err) - } - - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) - reqBody := struct { - Variables map[string]interface{} - }{} - json.Unmarshal(bodyBytes, &reqBody) - - _, assigneeDeclared := reqBody.Variables["assignee"] - _, labelsDeclared := reqBody.Variables["labels"] - eq(t, assigneeDeclared, false) - eq(t, labelsDeclared, false) -} - -func TestIssueList_disabledIssues(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand(issueListCmd, "issue list") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue list`: %v", err) - } -} - func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -217,8 +147,9 @@ func TestIssueView(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -277,7 +208,9 @@ func TestIssueView_urlArg(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - eq(t, output.String(), "") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -292,8 +225,7 @@ func TestIssueCreate(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true + "id": "REPOID" } } } `)) http.StubResponse(200, bytes.NewBufferString(` @@ -323,24 +255,7 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") -} - -func TestIssueCreate_disabledIssues(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand(issueCreateCmd, `issue create -t heres -b johnny`) - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue create`: %v", err) - } + eq(t, output, "https://github.com/OWNER/REPO/issues/12\n") } func TestIssueCreate_web(t *testing.T) { @@ -364,5 +279,5 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output.String(), "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") + eq(t, output, "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index badfe7204..50e280d69 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -91,7 +91,7 @@ func TestPRCreate(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + eq(t, output, "https://github.com/OWNER/REPO/pull/12\n") } func TestPRCreate_web(t *testing.T) { @@ -115,8 +115,9 @@ func TestPRCreate_web(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create --web`) eq(t, err, nil) - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") + if output == "" { + t.Fatal("expected output") + } eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature") @@ -154,6 +155,7 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - eq(t, output.Stderr(), "Warning: 1 uncommitted change\n") + eq(t, output, `Warning: 1 uncommitted change +https://github.com/OWNER/REPO/pull/12 +`) } diff --git a/command/pr_test.go b/command/pr_test.go index 91ec11a13..dadeeada0 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -24,50 +24,31 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -type cmdOut struct { - outBuf, errBuf *bytes.Buffer -} - -func (c cmdOut) String() string { - return c.outBuf.String() -} - -func (c cmdOut) Stderr() string { - return c.errBuf.String() -} - -func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { +func RunCommand(cmd *cobra.Command, args string) (string, error) { rootCmd := cmd.Root() argv, err := shlex.Split(args) if err != nil { - return nil, err + return "", err } rootCmd.SetArgs(argv) outBuf := bytes.Buffer{} cmd.SetOut(&outBuf) - errBuf := bytes.Buffer{} - cmd.SetErr(&errBuf) // Reset flag values so they don't leak between tests - // FIXME: change how we initialize Cobra commands to render this hack unnecessary cmd.Flags().VisitAll(func(f *pflag.Flag) { switch v := f.Value.(type) { case pflag.SliceValue: v.Replace([]string{}) default: - switch v.Type() { - case "bool", "string": + if v.Type() == "bool" { v.Set(f.DefValue) } } }) _, err = rootCmd.ExecuteC() - cmd.SetOut(nil) - cmd.SetErr(nil) - - return &cmdOut{&outBuf, &errBuf}, err + return outBuf.String(), err } func TestPRStatus(t *testing.T) { @@ -91,66 +72,12 @@ func TestPRStatus(t *testing.T) { } for _, r := range expectedPrs { - if !r.MatchString(output.String()) { + if !r.MatchString(output) { t.Errorf("output did not match regexp /%s/", r) } } } -func TestPRStatus_reviewsAndChecks(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") - http := initFakeHTTP() - - jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) - - output, err := RunCommand(prStatusCmd, "pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := []string{ - "- Checks passing - changes requested", - "- Checks pending - approved", - "- 1/3 checks failing - review required", - } - - for _, line := range expected { - if !strings.Contains(output.String(), line) { - t.Errorf("output did not contain %q: %q", line, output.String()) - } - } -} - -func TestPRStatus_blankSlate(t *testing.T) { - initBlankContext("OWNER/REPO", "blueberries") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": {} } - `)) - - output, err := RunCommand(prStatusCmd, "pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := `Current branch - There is no pull request associated with [blueberries] - -Created by you - You have no open pull requests - -Requesting a code review from you - You have no pull requests to review - -` - if output.String() != expected { - t.Errorf("expected %q, got %q", expected, output.String()) - } -} - func TestPRList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -164,7 +91,7 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), `32 New feature feature + eq(t, output, `32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) @@ -177,14 +104,11 @@ func TestPRList_filtering(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) + _, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) if err != nil { t.Fatal(err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "No pull requests match your search\n") - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -259,8 +183,9 @@ func TestPRView_currentBranch(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -323,7 +248,9 @@ func TestPRView_numberArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -354,7 +281,9 @@ func TestPRView_urlArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -387,7 +316,9 @@ func TestPRView_branchArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") @@ -421,7 +352,9 @@ func TestPRView_branchWithOwnerArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") + if output == "" { + t.Errorf("command output expected got an empty string") + } if seenCmd == nil { t.Fatal("expected a command to run") From 58761a8dfc1406341c519d69828bfcd1683d5f90 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 12:04:23 -0800 Subject: [PATCH 09/10] Fix tests --- command/issue_test.go | 24 +++++++++--------- command/pr_create_test.go | 12 ++++----- command/pr_test.go | 53 +++++++++++++++++++++++---------------- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index b10828368..47924e865 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -33,7 +33,7 @@ func TestIssueStatus(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -68,7 +68,7 @@ Issues opened by you There are no issues opened by you ` - if output != expectedOutput { + if output.String() != expectedOutput { t.Errorf("expected %q, got %q", expectedOutput, output) } } @@ -109,7 +109,7 @@ func TestIssueList(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -127,11 +127,14 @@ func TestIssueList_withFlags(t *testing.T) { } } } `)) - _, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") + output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") if err != nil { t.Errorf("error running command `issue list`: %v", err) } + eq(t, output.String(), "") + eq(t, output.Stderr(), "No issues match your search\n") + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -214,9 +217,8 @@ func TestIssueView(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -275,9 +277,7 @@ func TestIssueView_urlArg(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -323,7 +323,7 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, output, "https://github.com/OWNER/REPO/issues/12\n") + eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } func TestIssueCreate_disabledIssues(t *testing.T) { @@ -364,5 +364,5 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output, "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") + eq(t, output.String(), "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 50e280d69..badfe7204 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -91,7 +91,7 @@ func TestPRCreate(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output, "https://github.com/OWNER/REPO/pull/12\n") + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } func TestPRCreate_web(t *testing.T) { @@ -115,9 +115,8 @@ func TestPRCreate_web(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create --web`) eq(t, err, nil) - if output == "" { - t.Fatal("expected output") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature") @@ -155,7 +154,6 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - eq(t, output, `Warning: 1 uncommitted change -https://github.com/OWNER/REPO/pull/12 -`) + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + eq(t, output.Stderr(), "Warning: 1 uncommitted change\n") } diff --git a/command/pr_test.go b/command/pr_test.go index 6b9c73834..91ec11a13 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -24,16 +24,30 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -func RunCommand(cmd *cobra.Command, args string) (string, error) { +type cmdOut struct { + outBuf, errBuf *bytes.Buffer +} + +func (c cmdOut) String() string { + return c.outBuf.String() +} + +func (c cmdOut) Stderr() string { + return c.errBuf.String() +} + +func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { rootCmd := cmd.Root() argv, err := shlex.Split(args) if err != nil { - return "", err + return nil, err } rootCmd.SetArgs(argv) outBuf := bytes.Buffer{} cmd.SetOut(&outBuf) + errBuf := bytes.Buffer{} + cmd.SetErr(&errBuf) // Reset flag values so they don't leak between tests // FIXME: change how we initialize Cobra commands to render this hack unnecessary @@ -50,7 +64,10 @@ func RunCommand(cmd *cobra.Command, args string) (string, error) { }) _, err = rootCmd.ExecuteC() - return outBuf.String(), err + cmd.SetOut(nil) + cmd.SetErr(nil) + + return &cmdOut{&outBuf, &errBuf}, err } func TestPRStatus(t *testing.T) { @@ -74,7 +91,7 @@ func TestPRStatus(t *testing.T) { } for _, r := range expectedPrs { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/", r) } } @@ -147,7 +164,7 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - eq(t, output, `32 New feature feature + eq(t, output.String(), `32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs `) @@ -160,11 +177,14 @@ func TestPRList_filtering(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - _, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) + output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) if err != nil { t.Fatal(err) } + eq(t, output.String(), "") + eq(t, output.Stderr(), "No pull requests match your search\n") + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -239,9 +259,8 @@ func TestPRView_currentBranch(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -304,9 +323,7 @@ func TestPRView_numberArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -337,9 +354,7 @@ func TestPRView_urlArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -372,9 +387,7 @@ func TestPRView_branchArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -408,9 +421,7 @@ func TestPRView_branchWithOwnerArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") From f2afbbce7479f5de509fb1785282fd8a9afff412 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 20 Dec 2019 12:07:53 -0800 Subject: [PATCH 10/10] Fix test output --- command/issue_test.go | 6 +++++- command/pr_create_test.go | 12 ++++++++++-- command/pr_test.go | 6 +++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 47924e865..78189e4c7 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -133,7 +133,11 @@ func TestIssueList_withFlags(t *testing.T) { } eq(t, output.String(), "") - eq(t, output.Stderr(), "No issues match your search\n") + eq(t, output.Stderr(), ` +Issues for OWNER/REPO + +No issues match your search +`) bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { diff --git a/command/pr_create_test.go b/command/pr_create_test.go index badfe7204..96ac2eb25 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -116,7 +116,11 @@ func TestPRCreate_web(t *testing.T) { eq(t, err, nil) eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") + eq(t, output.Stderr(), ` +Creating pull request for feature into master in OWNER/REPO + +Opening https://github.com/OWNER/REPO/pull/feature in your browser. +`) eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature") @@ -155,5 +159,9 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { eq(t, err, nil) eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - eq(t, output.Stderr(), "Warning: 1 uncommitted change\n") + eq(t, output.Stderr(), `Warning: 1 uncommitted change + +Creating pull request for feature into master in OWNER/REPO + +`) } diff --git a/command/pr_test.go b/command/pr_test.go index 91ec11a13..360b6d852 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -183,7 +183,11 @@ func TestPRList_filtering(t *testing.T) { } eq(t, output.String(), "") - eq(t, output.Stderr(), "No pull requests match your search\n") + eq(t, output.Stderr(), ` +Pull requests for OWNER/REPO + +No pull requests match your search +`) bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct {