pr command scriptability improvements

This commit improves behavior and error handling for pr commands when
run unattached to a tty.

- error if pr create and no -t/-f
- error if pr create and -w
- machine readable pr list
- machine readable pr view
- various cleanup of informational messages
This commit is contained in:
vilmibm 2020-07-14 16:47:32 -05:00
parent e18776c227
commit 168bd33bc9
7 changed files with 679 additions and 38 deletions

View file

@ -293,8 +293,9 @@ func prList(cmd *cobra.Command, args []string) error {
})
title := listHeader(ghrepo.FullName(baseRepo), "pull request", len(listResult.PullRequests), listResult.TotalCount, hasFilters)
// TODO: avoid printing header if piped to a script
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
}
table := utils.NewTablePrinter(cmd.OutOrStdout())
for _, pr := range listResult.PullRequests {
@ -303,6 +304,10 @@ func prList(cmd *cobra.Command, args []string) error {
prNum = "#" + prNum
}
table.AddField(prNum, nil, colorFuncForPR(pr))
if !table.IsTTY() {
table.AddField(pr.State, nil, nil)
table.AddField(prReadyState(&pr), nil, nil)
}
table.AddField(replaceExcessiveWhitespace(pr.Title), nil, nil)
table.AddField(pr.HeadLabel(), nil, utils.Cyan)
table.EndRow()
@ -357,6 +362,10 @@ func prView(cmd *cobra.Command, args []string) error {
return err
}
if web && !connectedToTerminal(cmd) {
return errors.New("--web unsupported when not attached to a tty")
}
pr, _, err := prFromArgs(ctx, apiClient, cmd, args)
if err != nil {
return err
@ -367,8 +376,13 @@ func prView(cmd *cobra.Command, args []string) error {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}
out := colorableOut(cmd)
return printPrPreview(out, pr)
if connectedToTerminal(cmd) {
out := colorableOut(cmd)
return printHumanPrPreview(out, pr)
}
return printRawPrPreview(cmd.OutOrStdout(), pr)
}
func prClose(cmd *cobra.Command, args []string) error {
@ -482,6 +496,9 @@ func prMerge(cmd *cobra.Command, args []string) error {
}
if enabledFlagCount == 0 {
if !connectedToTerminal(cmd) {
return errors.New("--merge, --rebase, or --squash required when not attached to a tty")
}
isInteractive = true
} else if enabledFlagCount > 1 {
return errors.New("expected exactly one of --merge, --rebase, or --squash to be true")
@ -513,7 +530,9 @@ func prMerge(cmd *cobra.Command, args []string) error {
return fmt.Errorf("API call failed: %w", err)
}
fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
}
if deleteBranch {
branchSwitchString := ""
@ -560,7 +579,9 @@ func prMerge(cmd *cobra.Command, args []string) error {
}
}
fmt.Fprintf(colorableOut(cmd), "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableOut(cmd), "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString)
}
}
return nil
@ -620,7 +641,46 @@ func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullReque
return mergeMethod, deleteBranch, nil
}
func printPrPreview(out io.Writer, pr *api.PullRequest) error {
func prReadyState(pr *api.PullRequest) string {
switch pr.State {
case "OPEN":
if pr.IsDraft {
return "draft"
} else {
return "ready"
}
case "CLOSED":
return "unreviewable"
case "MERGED":
return "unreviewable"
}
return "unknown"
}
func printRawPrPreview(out io.Writer, pr *api.PullRequest) error {
reviewers := prReviewerList(*pr)
assignees := prAssigneeList(*pr)
labels := prLabelList(*pr)
projects := prProjectList(*pr)
fmt.Fprintf(out, "title:\t%s\n", pr.Title)
fmt.Fprintf(out, "state:\t%s\n", pr.State)
fmt.Fprintf(out, "ready:\t%s\n", prReadyState(pr))
fmt.Fprintf(out, "author:\t%s\n", pr.Author.Login)
fmt.Fprintf(out, "labels:\t%s\n", labels)
fmt.Fprintf(out, "assignees:\t%s\n", assignees)
fmt.Fprintf(out, "reviewers:\t%s\n", reviewers)
fmt.Fprintf(out, "projects:\t%s\n", projects)
fmt.Fprintf(out, "milestone:\t%s\n", pr.Milestone.Title)
fmt.Fprintln(out, "--")
fmt.Fprintln(out, pr.Body)
return nil
}
func printHumanPrPreview(out io.Writer, pr *api.PullRequest) error {
// Header (Title and State)
fmt.Fprintln(out, utils.Bold(pr.Title))
fmt.Fprintf(out, "%s", prStateTitleWithColor(*pr))

View file

@ -205,12 +205,14 @@ func prCreate(cmd *cobra.Command, _ []string) error {
message = "\nCreating draft pull request for %s into %s in %s\n\n"
}
fmt.Fprintf(colorableErr(cmd), message,
utils.Cyan(headBranch),
utils.Cyan(baseBranch),
ghrepo.FullName(baseRepo))
if (title == "" || body == "") && defaultsErr != nil {
fmt.Fprintf(colorableErr(cmd), "%s warning: could not compute title or body defaults: %s\n", utils.Yellow("!"), defaultsErr)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableErr(cmd), message,
utils.Cyan(headBranch),
utils.Cyan(baseBranch),
ghrepo.FullName(baseRepo))
if (title == "" || body == "") && defaultsErr != nil {
fmt.Fprintf(colorableErr(cmd), "%s warning: could not compute title or body defaults: %s\n", utils.Yellow("!"), defaultsErr)
}
}
}
@ -223,7 +225,16 @@ func prCreate(cmd *cobra.Command, _ []string) error {
Milestones: milestoneTitles,
}
interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body"))
if !connectedToTerminal(cmd) {
if isWeb {
return errors.New("--web unsupported when not attached to a tty")
}
if !cmd.Flags().Changed("title") && !autofill {
return errors.New("--title or --fill required when not attached to a tty")
}
}
interactive := connectedToTerminal(cmd) && !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body"))
if !isWeb && !autofill && interactive {
var nonLegacyTemplateFiles []string

View file

@ -12,10 +12,91 @@ import (
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/test"
"github.com/stretchr/testify/assert"
)
func TestPRCreate_nontty_insufficient_flags(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "forks": { "nodes": [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes" : [
] } } } }
`))
output, err := RunCommand("pr create")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "--title or --fill required when not attached to a tty", err.Error())
assert.Equal(t, "", output.String())
}
func TestPRCreate_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "forks": { "nodes": [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes" : [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "createPullRequest": { "pullRequest": {
"URL": "https://github.com/OWNER/REPO/pull/12"
} } } }
`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
cs.Stub("") // git config --get-regexp (determineTrackingBranch)
cs.Stub("") // git show-ref --verify (determineTrackingBranch)
cs.Stub("") // git status
cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log
cs.Stub("") // git push
output, err := RunCommand(`pr create -t "my title" -b "my body"`)
eq(t, err, nil)
bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body)
reqBody := struct {
Variables struct {
Input struct {
RepositoryID string
Title string
Body string
BaseRefName string
HeadRefName string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID)
assert.Equal(t, "my title", reqBody.Variables.Input.Title)
assert.Equal(t, "my body", reqBody.Variables.Input.Body)
assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName)
assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName)
assert.Equal(t, "", output.Stderr())
assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String())
}
func TestPRCreate(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -69,6 +150,7 @@ func TestPRCreate(t *testing.T) {
func TestPRCreate_metadata(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
@ -193,6 +275,7 @@ func TestPRCreate_metadata(t *testing.T) {
func TestPRCreate_withForking(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponseWithPermission("OWNER", "REPO", "READ")
http.StubResponse(200, bytes.NewBufferString(`
@ -236,6 +319,7 @@ func TestPRCreate_withForking(t *testing.T) {
func TestPRCreate_alreadyExists(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -269,6 +353,7 @@ func TestPRCreate_alreadyExists(t *testing.T) {
func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -301,6 +386,7 @@ func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) {
func TestPRCreate_web(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -332,6 +418,7 @@ func TestPRCreate_web(t *testing.T) {
func TestPRCreate_ReportsUncommittedChanges(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -369,6 +456,7 @@ Creating pull request for feature into master in OWNER/REPO
`)
}
func TestPRCreate_cross_repo_same_branch(t *testing.T) {
defer stubTerminal(true)()
ctx := context.NewBlank()
ctx.SetBranch("default")
ctx.SetRemotes(map[string]string{
@ -457,6 +545,7 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) {
func TestPRCreate_survey_defaults_multicommit(t *testing.T) {
initBlankContext("", "OWNER/REPO", "cool_bug-fixes")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -533,6 +622,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) {
func TestPRCreate_survey_defaults_monocommit(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE")))
@ -592,8 +682,70 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) {
eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n")
}
func TestPRCreate_survey_autofill_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "forks": { "nodes": [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes" : [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "createPullRequest": { "pullRequest": {
"URL": "https://github.com/OWNER/REPO/pull/12"
} } } }
`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
cs.Stub("") // git config --get-regexp (determineTrackingBranch)
cs.Stub("") // git show-ref --verify (determineTrackingBranch)
cs.Stub("") // git status
cs.Stub("1234567890,the sky above the port") // git log
cs.Stub("was the color of a television, turned to a dead channel") // git show
cs.Stub("") // git rev-parse
cs.Stub("") // git push
cs.Stub("") // browser open
output, err := RunCommand(`pr create -f`)
eq(t, err, nil)
bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body)
reqBody := struct {
Variables struct {
Input struct {
RepositoryID string
Title string
Body string
BaseRefName string
HeadRefName string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
expectedBody := "was the color of a television, turned to a dead channel"
assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID)
assert.Equal(t, "the sky above the port", reqBody.Variables.Input.Title)
assert.Equal(t, expectedBody, reqBody.Variables.Input.Body)
assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName)
assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName)
assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String())
assert.Equal(t, "", output.Stderr())
}
func TestPRCreate_survey_autofill(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -652,6 +804,7 @@ func TestPRCreate_survey_autofill(t *testing.T) {
func TestPRCreate_defaults_error_autofill(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -670,6 +823,7 @@ func TestPRCreate_defaults_error_autofill(t *testing.T) {
func TestPRCreate_defaults_error_web(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -688,6 +842,7 @@ func TestPRCreate_defaults_error_web(t *testing.T) {
func TestPRCreate_defaults_error_interactive(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`

View file

@ -72,7 +72,10 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
}
if found == 0 && body == "" {
return nil, nil // signal interactive mode
if connectedToTerminal(cmd) {
return nil, nil // signal interactive mode
}
return nil, errors.New("--approve, --request-changes, or --comment required when not attached to a tty")
} else if found == 0 && body != "" {
return nil, errors.New("--body unsupported without --approve, --request-changes, or --comment")
} else if found > 1 {
@ -106,7 +109,7 @@ func prReview(cmd *cobra.Command, args []string) error {
return fmt.Errorf("did not understand desired review action: %w", err)
}
out := colorableOut(cmd)
stderr := cmd.ErrOrStderr()
if reviewData == nil {
reviewData, err = reviewSurvey(cmd)
@ -114,7 +117,7 @@ func prReview(cmd *cobra.Command, args []string) error {
return err
}
if reviewData == nil && err == nil {
fmt.Fprint(out, "Discarding.\n")
fmt.Fprint(stderr, "Discarding.\n")
return nil
}
}
@ -124,13 +127,17 @@ func prReview(cmd *cobra.Command, args []string) error {
return fmt.Errorf("failed to create review: %w", err)
}
if !connectedToTerminal(cmd) {
return nil
}
switch reviewData.State {
case api.ReviewComment:
fmt.Fprintf(out, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
fmt.Fprintf(stderr, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
case api.ReviewApprove:
fmt.Fprintf(out, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
fmt.Fprintf(stderr, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
case api.ReviewRequestChanges:
fmt.Fprintf(out, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
fmt.Fprintf(stderr, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
}
return nil

View file

@ -8,6 +8,7 @@ import (
"testing"
"github.com/cli/cli/test"
"github.com/stretchr/testify/assert"
)
func TestPRReview_validation(t *testing.T) {
@ -49,6 +50,7 @@ func TestPRReview_bad_body(t *testing.T) {
func TestPRReview_url_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequest": {
@ -74,7 +76,7 @@ func TestPRReview_url_arg(t *testing.T) {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
@ -95,6 +97,7 @@ func TestPRReview_url_arg(t *testing.T) {
func TestPRReview_number_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -121,7 +124,7 @@ func TestPRReview_number_arg(t *testing.T) {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
@ -142,6 +145,7 @@ func TestPRReview_number_arg(t *testing.T) {
func TestPRReview_no_arg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -159,7 +163,7 @@ func TestPRReview_no_arg(t *testing.T) {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "- Reviewed pull request #123")
test.ExpectLines(t, output.Stderr(), "- Reviewed pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
@ -254,8 +258,73 @@ func TestPRReview(t *testing.T) {
}
}
func TestPRReview_nontty_insufficient_flags(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
"number": 123,
"id": "foobar123",
"headRefName": "feature",
"baseRefName": "master" }
] } } } }
`))
output, err := RunCommand("pr review")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "", output.String())
assert.Equal(t, "did not understand desired review action: --approve, --request-changes, or --comment required when not attached to a tty", err.Error())
}
func TestPRReview_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "url": "https://github.com/OWNER/REPO/pull/123",
"number": 123,
"id": "foobar123",
"headRefName": "feature",
"baseRefName": "master" }
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
output, err := RunCommand("pr review -c -bcool")
if err != nil {
t.Fatalf("unexpected error running command: %s", err)
}
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
Input struct {
Event string
Body string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
assert.Equal(t, "COMMENT", reqBody.Variables.Input.Event)
assert.Equal(t, "cool", reqBody.Variables.Input.Body)
}
func TestPRReview_interactive(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -295,10 +364,11 @@ func TestPRReview_interactive(t *testing.T) {
t.Fatalf("got unexpected error running pr review: %s", err)
}
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
test.ExpectLines(t, output.String(),
"Approved pull request #123",
"Got:",
"cool.*story") // weird because markdown rendering puts a bunch of junk between works
"cool.*story")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
@ -317,6 +387,7 @@ func TestPRReview_interactive(t *testing.T) {
func TestPRReview_interactive_no_body(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -359,6 +430,7 @@ func TestPRReview_interactive_no_body(t *testing.T) {
func TestPRReview_interactive_blank_approve(t *testing.T) {
initBlankContext("", "OWNER/REPO", "feature")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
@ -403,7 +475,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
t.Errorf("did not expect to see body printed in %s", output.String())
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {

View file

@ -295,6 +295,7 @@ Requesting a code review from you
func TestPRList(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("../test/fixtures/prList.json"))
@ -304,18 +305,40 @@ func TestPRList(t *testing.T) {
t.Fatal(err)
}
eq(t, output.Stderr(), `
assert.Equal(t, `
Showing 3 of 3 pull requests in OWNER/REPO
`)
eq(t, output.String(), `32 New feature feature
29 Fixed bad bug hubot:bug-fix
28 Improve documentation docs
`)
`, output.Stderr())
assert.Equal(t, `#32 New feature feature
#29 Fixed bad bug hubot:bug-fix
#28 Improve documentation docs
`, output.String())
}
func TestPRList_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("../test/fixtures/prList.json"))
output, err := RunCommand("pr list")
if err != nil {
t.Fatal(err)
}
assert.Equal(t, "", output.Stderr())
assert.Equal(t, `32 OPEN draft New feature feature
29 OPEN ready Fixed bad bug hubot:bug-fix
28 MERGED unreviewable Improve documentation docs
`, output.String())
}
func TestPRList_filtering(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -339,6 +362,7 @@ No pull requests match your search in OWNER/REPO
func TestPRList_filteringRemoveDuplicate(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -350,14 +374,15 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) {
t.Fatal(err)
}
eq(t, output.String(), `32 New feature feature
29 Fixed bad bug hubot:bug-fix
28 Improve documentation docs
`)
assert.Equal(t, `#32 New feature feature
#29 Fixed bad bug hubot:bug-fix
#28 Improve documentation docs
`, output.String())
}
func TestPRList_filteringClosed(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -374,6 +399,7 @@ func TestPRList_filteringClosed(t *testing.T) {
func TestPRList_filteringAssignee(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -390,6 +416,7 @@ func TestPRList_filteringAssignee(t *testing.T) {
func TestPRList_filteringAssigneeLabels(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
initFakeHTTP()
_, err := RunCommand(`pr list -l one,two -a hubot`)
@ -400,6 +427,7 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) {
func TestPRList_withInvalidLimitFlag(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
initFakeHTTP()
_, err := RunCommand(`pr list --limit=0`)
@ -410,6 +438,7 @@ func TestPRList_withInvalidLimitFlag(t *testing.T) {
func TestPRList_web(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -437,7 +466,197 @@ func TestPRList_web(t *testing.T) {
eq(t, url, expectedURL)
}
func TestPRView_Preview_nontty(t *testing.T) {
defer stubTerminal(false)()
tests := map[string]struct {
ownerRepo string
args string
fixture string
expectedOutputs []string
}{
"Open PR without metadata": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreview.json",
expectedOutputs: []string{
`title:\tBlueberries are from a fork\n`,
`state:\tOPEN\n`,
`author:\tnobody\n`,
`labels:\t\n`,
`ready:\tready\n`,
`assignees:\t\n`,
`reviewers:\t\n`,
`projects:\t\n`,
`milestone:\t\n`,
`blueberries taste good`,
},
},
"Open PR with metadata by number": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewWithMetadataByNumber.json",
expectedOutputs: []string{
`title:\tBlueberries are from a fork\n`,
`reviewers:\t2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`,
`assignees:\tmarseilles, monaco\n`,
`labels:\tone, two, three, four, five\n`,
`ready:\tready\n`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`\*\*blueberries taste good\*\*`,
},
},
"Open PR with reviewers by number": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json",
expectedOutputs: []string{
`title:\tBlueberries are from a fork\n`,
`state:\tOPEN\n`,
`author:\tnobody\n`,
`labels:\t\n`,
`assignees:\t\n`,
`projects:\t\n`,
`ready:\tready\n`,
`milestone:\t\n`,
`reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`,
`\*\*blueberries taste good\*\*`,
},
},
"Open PR with metadata by branch": {
ownerRepo: "master",
args: "pr view blueberries",
fixture: "../test/fixtures/prViewPreviewWithMetadataByBranch.json",
expectedOutputs: []string{
`title:\tBlueberries are a good fruit`,
`state:\tOPEN`,
`author:\tnobody`,
`assignees:\tmarseilles, monaco\n`,
`ready:\tready\n`,
`labels:\tone, two, three, four, five\n`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`,
`milestone:\tuluru\n`,
`blueberries taste good`,
},
},
"Open PR for the current branch": {
ownerRepo: "blueberries",
args: "pr view",
fixture: "../test/fixtures/prView.json",
expectedOutputs: []string{
`title:\tBlueberries are a good fruit`,
`state:\tOPEN`,
`author:\tnobody`,
`ready:\tready\n`,
`assignees:\t\n`,
`labels:\t\n`,
`projects:\t\n`,
`milestone:\t\n`,
`\*\*blueberries taste good\*\*`,
},
},
"Open PR wth empty body for the current branch": {
ownerRepo: "blueberries",
args: "pr view",
fixture: "../test/fixtures/prView_EmptyBody.json",
expectedOutputs: []string{
`title:\tBlueberries are a good fruit`,
`state:\tOPEN`,
`ready:\tready\n`,
`author:\tnobody`,
`assignees:\t\n`,
`labels:\t\n`,
`projects:\t\n`,
`milestone:\t\n`,
},
},
"Closed PR": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewClosedState.json",
expectedOutputs: []string{
`state:\tCLOSED\n`,
`author:\tnobody\n`,
`labels:\t\n`,
`ready:\tunreviewable\n`,
`assignees:\t\n`,
`reviewers:\t\n`,
`projects:\t\n`,
`milestone:\t\n`,
`\*\*blueberries taste good\*\*`,
},
},
"Merged PR": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewMergedState.json",
expectedOutputs: []string{
`state:\tMERGED\n`,
`author:\tnobody\n`,
`labels:\t\n`,
`assignees:\t\n`,
`ready:\tunreviewable\n`,
`reviewers:\t\n`,
`projects:\t\n`,
`milestone:\t\n`,
`\*\*blueberries taste good\*\*`,
},
},
"Draft PR": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewDraftState.json",
expectedOutputs: []string{
`title:\tBlueberries are from a fork\n`,
`state:\tOPEN\n`,
`author:\tnobody\n`,
`labels:`,
`ready:\tdraft\n`,
`assignees:`,
`projects:`,
`milestone:`,
`\*\*blueberries taste good\*\*`,
},
},
"Draft PR by branch": {
ownerRepo: "master",
args: "pr view blueberries",
fixture: "../test/fixtures/prViewPreviewDraftStatebyBranch.json",
expectedOutputs: []string{
`title:\tBlueberries are a good fruit\n`,
`state:\tOPEN\n`,
`author:\tnobody\n`,
`labels:`,
`ready:\tdraft\n`,
`assignees:`,
`projects:`,
`milestone:`,
`\*\*blueberries taste good\*\*`,
},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
initBlankContext("", "OWNER/REPO", tc.ownerRepo)
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture))
output, err := RunCommand(tc.args)
if err != nil {
t.Errorf("error running command `%v`: %v", tc.args, err)
}
eq(t, output.Stderr(), "")
test.ExpectLines(t, output.String(), tc.expectedOutputs...)
})
}
}
func TestPRView_Preview(t *testing.T) {
defer stubTerminal(true)()
tests := map[string]struct {
ownerRepo string
args string
@ -585,6 +804,7 @@ func TestPRView_Preview(t *testing.T) {
func TestPRView_web_currentBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView.json"))
@ -620,6 +840,7 @@ func TestPRView_web_currentBranch(t *testing.T) {
func TestPRView_web_noResultsForBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView_NoActiveBranch.json"))
@ -648,6 +869,7 @@ func TestPRView_web_noResultsForBranch(t *testing.T) {
func TestPRView_web_numberArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -680,6 +902,7 @@ func TestPRView_web_numberArg(t *testing.T) {
func TestPRView_web_numberArgWithHash(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -712,6 +935,7 @@ func TestPRView_web_numberArgWithHash(t *testing.T) {
func TestPRView_web_urlArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequest": {
@ -742,6 +966,7 @@ func TestPRView_web_urlArg(t *testing.T) {
func TestPRView_web_branchArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -776,6 +1001,7 @@ func TestPRView_web_branchArg(t *testing.T) {
func TestPRView_web_branchWithOwnerArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -809,6 +1035,21 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) {
eq(t, url, "https://github.com/hubot/REPO/pull/23")
}
func TestPrView_web_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
output, err := RunCommand("pr view -w")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "--web unsupported when not attached to a tty", err.Error())
assert.Equal(t, "", output.String())
}
func TestReplaceExcessiveWhitespace(t *testing.T) {
eq(t, replaceExcessiveWhitespace("hello\ngoodbye"), "hello goodbye")
eq(t, replaceExcessiveWhitespace(" hello goodbye "), "hello goodbye")
@ -965,6 +1206,7 @@ func TestPRReopen_alreadyMerged(t *testing.T) {
func TestPrMerge(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1010,8 +1252,78 @@ func TestPrMerge(t *testing.T) {
}
}
func TestPrMerge_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"}
} } }`))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
cs.Stub("") // git symbolic-ref --quiet --short HEAD
cs.Stub("") // git checkout master
cs.Stub("")
output, err := RunCommand("pr merge 1 --merge")
if err != nil {
t.Fatalf("error running command `pr merge`: %v", err)
}
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
}
func TestPrMerge_nontty_insufficient_flags(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"}
} } }`))
output, err := RunCommand("pr merge 1")
if err == nil {
t.Fatal("expected error")
}
assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a tty", err.Error())
assert.Equal(t, "", output.String())
}
func TestPrMerge_withRepoFlag(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.Register(
@ -1049,6 +1361,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
func TestPrMerge_deleteBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1084,6 +1397,7 @@ func TestPrMerge_deleteBranch(t *testing.T) {
func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "another-branch")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1117,6 +1431,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
func TestPrMerge_noPrNumberGiven(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1156,6 +1471,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
func TestPrMerge_rebase(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1202,6 +1518,7 @@ func TestPrMerge_rebase(t *testing.T) {
func TestPrMerge_squash(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1245,6 +1562,7 @@ func TestPrMerge_squash(t *testing.T) {
func TestPrMerge_alreadyMerged(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1277,6 +1595,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
func TestPRMerge_interactive(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
defer stubTerminal(true)()
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
@ -1332,6 +1651,17 @@ func TestPRMerge_interactive(t *testing.T) {
func TestPrMerge_multipleMergeMethods(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
_, err := RunCommand("pr merge 1 --merge --squash")
if err == nil {
t.Fatal("expected error running `pr merge` with multiple merge methods")
}
}
func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
_, err := RunCommand("pr merge 1 --merge --squash")
if err == nil {

View file

@ -9,7 +9,9 @@
"number": 32,
"title": "New feature",
"url": "https://github.com/monalisa/hello/pull/32",
"headRefName": "feature"
"headRefName": "feature",
"state": "OPEN",
"isDraft": true
}
},
{
@ -18,6 +20,8 @@
"title": "Fixed bad bug",
"url": "https://github.com/monalisa/hello/pull/29",
"headRefName": "bug-fix",
"state": "OPEN",
"isDraft": false,
"isCrossRepository": true,
"headRepositoryOwner": {
"login": "hubot"
@ -27,6 +31,8 @@
{
"node": {
"number": 28,
"state": "MERGED",
"isDraft": false,
"title": "Improve documentation",
"url": "https://github.com/monalisa/hello/pull/28",
"headRefName": "docs"
@ -40,4 +46,4 @@
}
}
}
}
}