Merge branch 'trunk' into newConfirmations
This commit is contained in:
ShubhankarKG
commit
01bcdbc8a1
22 changed files with 1008 additions and 765 deletions
177
pkg/cmd/pr/checkout/checkout.go
Normal file
177
pkg/cmd/pr/checkout/checkout.go
Normal file
|
|
@ -0,0 +1,177 @@
|
|||
package checkout
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
|
||||
"github.com/cli/cli/api"
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/internal/run"
|
||||
"github.com/cli/cli/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
type CheckoutOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
Config func() (config.Config, error)
|
||||
IO *iostreams.IOStreams
|
||||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Remotes func() (context.Remotes, error)
|
||||
Branch func() (string, error)
|
||||
|
||||
SelectorArg string
|
||||
}
|
||||
|
||||
func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobra.Command {
|
||||
opts := &CheckoutOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
Config: f.Config,
|
||||
BaseRepo: f.BaseRepo,
|
||||
Remotes: f.Remotes,
|
||||
Branch: f.Branch,
|
||||
}
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "checkout {<number> | <url> | <branch>}",
|
||||
Short: "Check out a pull request in git",
|
||||
Args: func(cmd *cobra.Command, args []string) error {
|
||||
if len(args) == 0 {
|
||||
return &cmdutil.FlagError{Err: errors.New("argument required")}
|
||||
}
|
||||
return nil
|
||||
},
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
if len(args) > 0 {
|
||||
opts.SelectorArg = args[0]
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
}
|
||||
return checkoutRun(opts)
|
||||
},
|
||||
}
|
||||
|
||||
return cmd
|
||||
}
|
||||
|
||||
func checkoutRun(opts *CheckoutOptions) error {
|
||||
currentBranch, err := opts.Branch()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
remotes, err := opts.Remotes()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
cfg, err := opts.Config()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
protocol, _ := cfg.Get(baseRepo.RepoHost(), "git_protocol")
|
||||
|
||||
baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
|
||||
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
|
||||
baseURLOrName := ghrepo.FormatRemoteURL(baseRepo, protocol)
|
||||
if baseRemote != nil {
|
||||
baseURLOrName = baseRemote.Name
|
||||
}
|
||||
|
||||
headRemote := baseRemote
|
||||
if pr.IsCrossRepository {
|
||||
headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)
|
||||
}
|
||||
|
||||
var cmdQueue [][]string
|
||||
newBranchName := pr.HeadRefName
|
||||
if strings.HasPrefix(newBranchName, "-") {
|
||||
return fmt.Errorf("invalid branch name: %q", newBranchName)
|
||||
}
|
||||
|
||||
if headRemote != nil {
|
||||
// there is an existing git remote for PR head
|
||||
remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName)
|
||||
refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pr.HeadRefName, remoteBranch)
|
||||
|
||||
cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec})
|
||||
|
||||
// local branch already exists
|
||||
if _, err := git.ShowRefs("refs/heads/" + newBranchName); err == nil {
|
||||
cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)})
|
||||
} else {
|
||||
cmdQueue = append(cmdQueue, []string{"git", "checkout", "-b", newBranchName, "--no-track", remoteBranch})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), headRemote.Name})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), "refs/heads/" + pr.HeadRefName})
|
||||
}
|
||||
} else {
|
||||
// no git remote for PR head
|
||||
|
||||
defaultBranchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// avoid naming the new branch the same as the default branch
|
||||
if newBranchName == defaultBranchName {
|
||||
newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName)
|
||||
}
|
||||
|
||||
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)
|
||||
if newBranchName == currentBranch {
|
||||
// PR head matches currently checked out branch
|
||||
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, ref})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"})
|
||||
} else {
|
||||
// create a new branch
|
||||
cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName)})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName})
|
||||
}
|
||||
|
||||
remote := baseURLOrName
|
||||
mergeRef := ref
|
||||
if pr.MaintainerCanModify {
|
||||
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, baseRepo.RepoHost())
|
||||
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
|
||||
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
|
||||
}
|
||||
if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" {
|
||||
cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote})
|
||||
cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef})
|
||||
}
|
||||
}
|
||||
|
||||
for _, args := range cmdQueue {
|
||||
cmd := exec.Command(args[0], args[1:]...)
|
||||
cmd.Stdout = os.Stdout
|
||||
cmd.Stderr = os.Stderr
|
||||
if err := run.PrepareCmd(cmd).Run(); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
567
pkg/cmd/pr/checkout/checkout_test.go
Normal file
567
pkg/cmd/pr/checkout/checkout_test.go
Normal file
|
|
@ -0,0 +1,567 @@
|
|||
package checkout
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"os/exec"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/api"
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/internal/run"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/httpmock"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/cli/cli/test"
|
||||
"github.com/google/shlex"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func eq(t *testing.T, got interface{}, expected interface{}) {
|
||||
t.Helper()
|
||||
if !reflect.DeepEqual(got, expected) {
|
||||
t.Errorf("expected: %v, got: %v", expected, got)
|
||||
}
|
||||
}
|
||||
|
||||
type errorStub struct {
|
||||
message string
|
||||
}
|
||||
|
||||
func (s errorStub) Output() ([]byte, error) {
|
||||
return nil, errors.New(s.message)
|
||||
}
|
||||
|
||||
func (s errorStub) Run() error {
|
||||
return errors.New(s.message)
|
||||
}
|
||||
|
||||
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) {
|
||||
io, _, stdout, stderr := iostreams.Test()
|
||||
|
||||
factory := &cmdutil.Factory{
|
||||
IOStreams: io,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: rt}, nil
|
||||
},
|
||||
Config: func() (config.Config, error) {
|
||||
return config.NewBlankConfig(), nil
|
||||
},
|
||||
BaseRepo: func() (ghrepo.Interface, error) {
|
||||
return api.InitRepoHostname(&api.Repository{
|
||||
Name: "REPO",
|
||||
Owner: api.RepositoryOwner{Login: "OWNER"},
|
||||
DefaultBranchRef: api.BranchRef{Name: "master"},
|
||||
}, "github.com"), nil
|
||||
},
|
||||
Remotes: func() (context.Remotes, error) {
|
||||
if remotes == nil {
|
||||
return context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
return remotes, nil
|
||||
},
|
||||
Branch: func() (string, error) {
|
||||
return branch, nil
|
||||
},
|
||||
}
|
||||
|
||||
cmd := NewCmdCheckout(factory, nil)
|
||||
|
||||
argv, err := shlex.Split(cli)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cmd.SetArgs(argv)
|
||||
|
||||
cmd.SetIn(&bytes.Buffer{})
|
||||
cmd.SetOut(ioutil.Discard)
|
||||
cmd.SetErr(ioutil.Discard)
|
||||
|
||||
_, err = cmd.ExecuteC()
|
||||
return &test.CmdOut{
|
||||
OutBuf: stdout,
|
||||
ErrBuf: stderr,
|
||||
}, err
|
||||
}
|
||||
|
||||
func TestPRCheckout_sameRepo(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &errorStub{"exit status: 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
if !assert.NoError(t, err) {
|
||||
return
|
||||
}
|
||||
|
||||
assert.Equal(t, "", output.String())
|
||||
if !assert.Equal(t, 4, len(ranCommands)) {
|
||||
return
|
||||
}
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
|
||||
eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin")
|
||||
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_urlArg(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &errorStub{"exit status: 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `https://github.com/OWNER/REPO/pull/123/files`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 4)
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_urlArg_differentBase(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "POE"
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": {
|
||||
"defaultBranchRef": {"name": "master"}
|
||||
} } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &errorStub{"exit status: 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `https://github.com/OTHER/POE/pull/123/files`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Owner string
|
||||
Repo string
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
eq(t, reqBody.Variables.Owner, "OTHER")
|
||||
eq(t, reqBody.Variables.Repo, "POE")
|
||||
|
||||
eq(t, len(ranCommands), 5)
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git fetch https://github.com/OTHER/POE.git refs/pull/123/head:feature")
|
||||
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote https://github.com/OTHER/POE.git")
|
||||
}
|
||||
|
||||
func TestPRCheckout_branchArg(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequests": { "nodes": [
|
||||
{ "number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false }
|
||||
] } } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &errorStub{"exit status: 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `hubot:feature`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 5)
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_existingBranch(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &test.OutputStub{}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 3)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout feature")
|
||||
eq(t, strings.Join(ranCommands[2], " "), "git merge --ff-only refs/remotes/origin/feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
|
||||
remotes := context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
{
|
||||
Remote: &git.Remote{Name: "robot-fork"},
|
||||
Repo: ghrepo.New("hubot", "REPO"),
|
||||
},
|
||||
}
|
||||
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git show-ref --verify -- refs/heads/feature":
|
||||
return &errorStub{"exit status: 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, remotes, "master", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 4)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track robot-fork/feature")
|
||||
eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote robot-fork")
|
||||
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_differentRepo(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git config branch.feature.merge":
|
||||
return &errorStub{"exit status 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 4)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout feature")
|
||||
eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote origin")
|
||||
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/pull/123/head")
|
||||
}
|
||||
|
||||
func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git config branch.feature.merge":
|
||||
return &test.OutputStub{Out: []byte("refs/heads/feature\n")}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 2)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout feature")
|
||||
}
|
||||
|
||||
func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git config branch.feature.merge":
|
||||
return &test.OutputStub{Out: []byte("refs/heads/feature\n")}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "feature", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 2)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git merge --ff-only FETCH_HEAD")
|
||||
}
|
||||
|
||||
func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "-foo",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": false
|
||||
} } } }
|
||||
`))
|
||||
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
t.Errorf("unexpected external invocation: %v", cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
if assert.Errorf(t, err, "expected command to fail") {
|
||||
assert.Equal(t, `invalid branch name: "-foo"`, err.Error())
|
||||
}
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
}
|
||||
|
||||
func TestPRCheckout_maintainerCanModify(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO"
|
||||
},
|
||||
"isCrossRepository": true,
|
||||
"maintainerCanModify": true
|
||||
} } } }
|
||||
`))
|
||||
|
||||
ranCommands := [][]string{}
|
||||
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
|
||||
switch strings.Join(cmd.Args, " ") {
|
||||
case "git config branch.feature.merge":
|
||||
return &errorStub{"exit status 1"}
|
||||
default:
|
||||
ranCommands = append(ranCommands, cmd.Args)
|
||||
return &test.OutputStub{}
|
||||
}
|
||||
})
|
||||
defer restoreCmd()
|
||||
|
||||
output, err := runCommand(http, nil, "master", `123`)
|
||||
eq(t, err, nil)
|
||||
eq(t, output.String(), "")
|
||||
|
||||
eq(t, len(ranCommands), 4)
|
||||
eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature")
|
||||
eq(t, strings.Join(ranCommands[1], " "), "git checkout feature")
|
||||
eq(t, strings.Join(ranCommands[2], " "), "git config branch.feature.remote https://github.com/hubot/REPO.git")
|
||||
eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature")
|
||||
}
|
||||
134
pkg/cmd/pr/diff/diff.go
Normal file
134
pkg/cmd/pr/diff/diff.go
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
package diff
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/cli/cli/api"
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
type DiffOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
IO *iostreams.IOStreams
|
||||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Remotes func() (context.Remotes, error)
|
||||
Branch func() (string, error)
|
||||
|
||||
SelectorArg string
|
||||
UseColor string
|
||||
}
|
||||
|
||||
func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Command {
|
||||
opts := &DiffOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
BaseRepo: f.BaseRepo,
|
||||
Remotes: f.Remotes,
|
||||
Branch: f.Branch,
|
||||
}
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "diff [<number> | <url> | <branch>]",
|
||||
Short: "View changes in a pull request",
|
||||
Args: cobra.MaximumNArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
if len(args) > 0 {
|
||||
opts.SelectorArg = args[0]
|
||||
}
|
||||
|
||||
if !validColorFlag(opts.UseColor) {
|
||||
return &cmdutil.FlagError{Err: fmt.Errorf("did not understand color: %q. Expected one of always, never, or auto", opts.UseColor)}
|
||||
}
|
||||
|
||||
if opts.UseColor == "auto" && !opts.IO.IsStdoutTTY() {
|
||||
opts.UseColor = "never"
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
}
|
||||
return diffRun(opts)
|
||||
},
|
||||
}
|
||||
|
||||
cmd.Flags().StringVar(&opts.UseColor, "color", "auto", "Use color in diff output: {always|never|auto}")
|
||||
|
||||
return cmd
|
||||
}
|
||||
|
||||
func diffRun(opts *DiffOptions) error {
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
diff, err := apiClient.PullRequestDiff(baseRepo, pr.Number)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not find pull request diff: %w", err)
|
||||
}
|
||||
defer diff.Close()
|
||||
|
||||
if opts.UseColor == "never" {
|
||||
_, err = io.Copy(opts.IO.Out, diff)
|
||||
return err
|
||||
}
|
||||
|
||||
diffLines := bufio.NewScanner(diff)
|
||||
for diffLines.Scan() {
|
||||
diffLine := diffLines.Text()
|
||||
switch {
|
||||
case isHeaderLine(diffLine):
|
||||
fmt.Fprintf(opts.IO.Out, "\x1b[1;38m%s\x1b[m\n", diffLine)
|
||||
case isAdditionLine(diffLine):
|
||||
fmt.Fprintf(opts.IO.Out, "\x1b[32m%s\x1b[m\n", diffLine)
|
||||
case isRemovalLine(diffLine):
|
||||
fmt.Fprintf(opts.IO.Out, "\x1b[31m%s\x1b[m\n", diffLine)
|
||||
default:
|
||||
fmt.Fprintln(opts.IO.Out, diffLine)
|
||||
}
|
||||
}
|
||||
|
||||
if err := diffLines.Err(); err != nil {
|
||||
return fmt.Errorf("error reading pull request diff: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
var diffHeaderPrefixes = []string{"+++", "---", "diff", "index"}
|
||||
|
||||
func isHeaderLine(dl string) bool {
|
||||
for _, p := range diffHeaderPrefixes {
|
||||
if strings.HasPrefix(dl, p) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func isAdditionLine(dl string) bool {
|
||||
return strings.HasPrefix(dl, "+")
|
||||
}
|
||||
|
||||
func isRemovalLine(dl string) bool {
|
||||
return strings.HasPrefix(dl, "-")
|
||||
}
|
||||
|
||||
func validColorFlag(c string) bool {
|
||||
return c == "auto" || c == "always" || c == "never"
|
||||
}
|
||||
183
pkg/cmd/pr/diff/diff_test.go
Normal file
183
pkg/cmd/pr/diff/diff_test.go
Normal file
|
|
@ -0,0 +1,183 @@
|
|||
package diff
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/httpmock"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/cli/cli/test"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/google/shlex"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) {
|
||||
io, _, stdout, stderr := iostreams.Test()
|
||||
io.SetStdoutTTY(isTTY)
|
||||
io.SetStdinTTY(isTTY)
|
||||
io.SetStderrTTY(isTTY)
|
||||
|
||||
factory := &cmdutil.Factory{
|
||||
IOStreams: io,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: rt}, nil
|
||||
},
|
||||
Config: func() (config.Config, error) {
|
||||
return config.NewBlankConfig(), nil
|
||||
},
|
||||
BaseRepo: func() (ghrepo.Interface, error) {
|
||||
return ghrepo.New("OWNER", "REPO"), nil
|
||||
},
|
||||
Remotes: func() (context.Remotes, error) {
|
||||
if remotes == nil {
|
||||
return context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
return remotes, nil
|
||||
},
|
||||
Branch: func() (string, error) {
|
||||
return "feature", nil
|
||||
},
|
||||
}
|
||||
|
||||
cmd := NewCmdDiff(factory, nil)
|
||||
|
||||
argv, err := shlex.Split(cli)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cmd.SetArgs(argv)
|
||||
|
||||
cmd.SetIn(&bytes.Buffer{})
|
||||
cmd.SetOut(ioutil.Discard)
|
||||
cmd.SetErr(ioutil.Discard)
|
||||
|
||||
_, err = cmd.ExecuteC()
|
||||
return &test.CmdOut{
|
||||
OutBuf: stdout,
|
||||
ErrBuf: stderr,
|
||||
}, err
|
||||
}
|
||||
|
||||
func TestPRDiff_validation(t *testing.T) {
|
||||
_, err := runCommand(nil, nil, false, "--color=doublerainbow")
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
assert.Equal(t, `did not understand color: "doublerainbow". Expected one of always, never, or auto`, err.Error())
|
||||
}
|
||||
|
||||
func TestPRDiff_no_current_pr(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": { "pullRequests": { "nodes": [] } } } }
|
||||
`))
|
||||
_, err := runCommand(http, nil, false, "")
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
assert.Equal(t, `no open pull requests found for branch "feature"`, err.Error())
|
||||
}
|
||||
|
||||
func TestPRDiff_argument_not_found(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": {
|
||||
"pullRequest": { "number": 123 }
|
||||
} } }
|
||||
`))
|
||||
http.StubResponse(404, bytes.NewBufferString(""))
|
||||
_, err := runCommand(http, nil, false, "123")
|
||||
if err == nil {
|
||||
t.Fatal("expected error", err)
|
||||
}
|
||||
assert.Equal(t, `could not find pull request diff: pull request not found`, err.Error())
|
||||
}
|
||||
|
||||
func TestPRDiff_notty(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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(testDiff))
|
||||
output, err := runCommand(http, nil, false, "")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
}
|
||||
if diff := cmp.Diff(testDiff, output.String()); diff != "" {
|
||||
t.Errorf("command output did not match:\n%s", diff)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRDiff_tty(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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(testDiff))
|
||||
output, err := runCommand(http, nil, true, "")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
}
|
||||
assert.Contains(t, output.String(), "\x1b[32m+site: bin/gh\x1b[m")
|
||||
}
|
||||
|
||||
const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml
|
||||
index 73974448..b7fc0154 100644
|
||||
--- a/.github/workflows/releases.yml
|
||||
+++ b/.github/workflows/releases.yml
|
||||
@@ -44,6 +44,11 @@ jobs:
|
||||
token: ${{secrets.SITE_GITHUB_TOKEN}}
|
||||
- name: Publish documentation site
|
||||
if: "!contains(github.ref, '-')" # skip prereleases
|
||||
+ env:
|
||||
+ GIT_COMMITTER_NAME: cli automation
|
||||
+ GIT_AUTHOR_NAME: cli automation
|
||||
+ GIT_COMMITTER_EMAIL: noreply@github.com
|
||||
+ GIT_AUTHOR_EMAIL: noreply@github.com
|
||||
run: make site-publish
|
||||
- name: Move project cards
|
||||
if: "!contains(github.ref, '-')" # skip prereleases
|
||||
diff --git a/Makefile b/Makefile
|
||||
index f2b4805c..3d7bd0f9 100644
|
||||
--- a/Makefile
|
||||
+++ b/Makefile
|
||||
@@ -22,8 +22,8 @@ test:
|
||||
go test ./...
|
||||
.PHONY: test
|
||||
|
||||
-site:
|
||||
- git clone https://github.com/github/cli.github.com.git "$@"
|
||||
+site: bin/gh
|
||||
+ bin/gh repo clone github/cli.github.com "$@"
|
||||
|
||||
site-docs: site
|
||||
git -C site pull
|
||||
`
|
||||
289
pkg/cmd/pr/review/review.go
Normal file
289
pkg/cmd/pr/review/review.go
Normal file
|
|
@ -0,0 +1,289 @@
|
|||
package review
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/AlecAivazis/survey/v2"
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
"github.com/cli/cli/api"
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/cli/cli/pkg/prompt"
|
||||
"github.com/cli/cli/pkg/surveyext"
|
||||
"github.com/cli/cli/utils"
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
type ReviewOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
Config func() (config.Config, error)
|
||||
IO *iostreams.IOStreams
|
||||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Remotes func() (context.Remotes, error)
|
||||
Branch func() (string, error)
|
||||
|
||||
SelectorArg string
|
||||
Approve bool
|
||||
RequestChanges bool
|
||||
Comment bool
|
||||
Body string
|
||||
}
|
||||
|
||||
func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Command {
|
||||
opts := &ReviewOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
Config: f.Config,
|
||||
BaseRepo: f.BaseRepo,
|
||||
Remotes: f.Remotes,
|
||||
Branch: f.Branch,
|
||||
}
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "review [<number> | <url> | <branch>]",
|
||||
Short: "Add a review to a pull request",
|
||||
Long: heredoc.Doc(`
|
||||
Add a review to a pull request.
|
||||
|
||||
Without an argument, the pull request that belongs to the current branch is reviewed.
|
||||
`),
|
||||
Example: heredoc.Doc(`
|
||||
# approve the pull request of the current branch
|
||||
$ gh pr review --approve
|
||||
|
||||
# leave a review comment for the current branch
|
||||
$ gh pr review --comment -b "interesting"
|
||||
|
||||
# add a review for a specific pull request
|
||||
$ gh pr review 123
|
||||
|
||||
# request changes on a specific pull request
|
||||
$ gh pr review 123 -r -b "needs more ASCII art"
|
||||
`),
|
||||
Args: cobra.MaximumNArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
if len(args) > 0 {
|
||||
opts.SelectorArg = args[0]
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
}
|
||||
return reviewRun(opts)
|
||||
},
|
||||
}
|
||||
|
||||
cmd.Flags().BoolVarP(&opts.Approve, "approve", "a", false, "Approve pull request")
|
||||
cmd.Flags().BoolVarP(&opts.RequestChanges, "request-changes", "r", false, "Request changes on a pull request")
|
||||
cmd.Flags().BoolVarP(&opts.Comment, "comment", "c", false, "Comment on a pull request")
|
||||
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Specify the body of a review")
|
||||
|
||||
return cmd
|
||||
}
|
||||
|
||||
func reviewRun(opts *ReviewOptions) error {
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
reviewData, err := processReviewOpt(opts)
|
||||
if err != nil {
|
||||
return fmt.Errorf("did not understand desired review action: %w", err)
|
||||
}
|
||||
|
||||
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if reviewData == nil {
|
||||
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
reviewData, err = reviewSurvey(opts.IO, editorCommand)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if reviewData == nil && err == nil {
|
||||
fmt.Fprint(opts.IO.ErrOut, "Discarding.\n")
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
err = api.AddReview(apiClient, baseRepo, pr, reviewData)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create review: %w", err)
|
||||
}
|
||||
|
||||
if !opts.IO.IsStdoutTTY() || !opts.IO.IsStderrTTY() {
|
||||
return nil
|
||||
}
|
||||
|
||||
switch reviewData.State {
|
||||
case api.ReviewComment:
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
|
||||
case api.ReviewApprove:
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
|
||||
case api.ReviewRequestChanges:
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// TODO: move to Command.Args, raise FlagError
|
||||
func processReviewOpt(opts *ReviewOptions) (*api.PullRequestReviewInput, error) {
|
||||
found := 0
|
||||
flag := ""
|
||||
var state api.PullRequestReviewState
|
||||
|
||||
if opts.Approve {
|
||||
found++
|
||||
flag = "approve"
|
||||
state = api.ReviewApprove
|
||||
}
|
||||
if opts.RequestChanges {
|
||||
found++
|
||||
flag = "request-changes"
|
||||
state = api.ReviewRequestChanges
|
||||
}
|
||||
if opts.Comment {
|
||||
found++
|
||||
flag = "comment"
|
||||
state = api.ReviewComment
|
||||
}
|
||||
|
||||
body := opts.Body
|
||||
|
||||
if found == 0 && body == "" {
|
||||
if opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() {
|
||||
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 {
|
||||
return nil, errors.New("need exactly one of --approve, --request-changes, or --comment")
|
||||
}
|
||||
|
||||
if (flag == "request-changes" || flag == "comment") && body == "" {
|
||||
return nil, fmt.Errorf("body cannot be blank for %s review", flag)
|
||||
}
|
||||
|
||||
return &api.PullRequestReviewInput{
|
||||
Body: body,
|
||||
State: state,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullRequestReviewInput, error) {
|
||||
typeAnswers := struct {
|
||||
ReviewType string
|
||||
}{}
|
||||
typeQs := []*survey.Question{
|
||||
{
|
||||
Name: "reviewType",
|
||||
Prompt: &survey.Select{
|
||||
Message: "What kind of review do you want to give?",
|
||||
Options: []string{
|
||||
"Comment",
|
||||
"Approve",
|
||||
"Request changes",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
err := prompt.SurveyAsk(typeQs, &typeAnswers)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var reviewState api.PullRequestReviewState
|
||||
|
||||
switch typeAnswers.ReviewType {
|
||||
case "Approve":
|
||||
reviewState = api.ReviewApprove
|
||||
case "Request changes":
|
||||
reviewState = api.ReviewRequestChanges
|
||||
case "Comment":
|
||||
reviewState = api.ReviewComment
|
||||
default:
|
||||
panic("unreachable state")
|
||||
}
|
||||
|
||||
bodyAnswers := struct {
|
||||
Body string
|
||||
}{}
|
||||
|
||||
blankAllowed := false
|
||||
if reviewState == api.ReviewApprove {
|
||||
blankAllowed = true
|
||||
}
|
||||
|
||||
bodyQs := []*survey.Question{
|
||||
{
|
||||
Name: "body",
|
||||
Prompt: &surveyext.GhEditor{
|
||||
BlankAllowed: blankAllowed,
|
||||
EditorCommand: editorCommand,
|
||||
Editor: &survey.Editor{
|
||||
Message: "Review body",
|
||||
FileName: "*.md",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
err = prompt.SurveyAsk(bodyQs, &bodyAnswers)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if bodyAnswers.Body == "" && (reviewState == api.ReviewComment || reviewState == api.ReviewRequestChanges) {
|
||||
return nil, errors.New("this type of review cannot be blank")
|
||||
}
|
||||
|
||||
if len(bodyAnswers.Body) > 0 {
|
||||
renderedBody, err := utils.RenderMarkdown(bodyAnswers.Body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
fmt.Fprintf(io.Out, "Got:\n%s", renderedBody)
|
||||
}
|
||||
|
||||
confirm := false
|
||||
confirmQs := []*survey.Question{
|
||||
{
|
||||
Name: "confirm",
|
||||
Prompt: &survey.Confirm{
|
||||
Message: "Submit?",
|
||||
Default: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
err = prompt.SurveyAsk(confirmQs, &confirm)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if !confirm {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
return &api.PullRequestReviewInput{
|
||||
Body: bodyAnswers.Body,
|
||||
State: reviewState,
|
||||
}, nil
|
||||
}
|
||||
514
pkg/cmd/pr/review/review_test.go
Normal file
514
pkg/cmd/pr/review/review_test.go
Normal file
|
|
@ -0,0 +1,514 @@
|
|||
package review
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/httpmock"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
"github.com/cli/cli/pkg/prompt"
|
||||
"github.com/cli/cli/test"
|
||||
"github.com/google/shlex"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func runCommand(rt http.RoundTripper, remotes context.Remotes, isTTY bool, cli string) (*test.CmdOut, error) {
|
||||
io, _, stdout, stderr := iostreams.Test()
|
||||
io.SetStdoutTTY(isTTY)
|
||||
io.SetStdinTTY(isTTY)
|
||||
io.SetStderrTTY(isTTY)
|
||||
|
||||
factory := &cmdutil.Factory{
|
||||
IOStreams: io,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: rt}, nil
|
||||
},
|
||||
Config: func() (config.Config, error) {
|
||||
return config.NewBlankConfig(), nil
|
||||
},
|
||||
BaseRepo: func() (ghrepo.Interface, error) {
|
||||
return ghrepo.New("OWNER", "REPO"), nil
|
||||
},
|
||||
Remotes: func() (context.Remotes, error) {
|
||||
if remotes == nil {
|
||||
return context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
return remotes, nil
|
||||
},
|
||||
Branch: func() (string, error) {
|
||||
return "feature", nil
|
||||
},
|
||||
}
|
||||
|
||||
cmd := NewCmdReview(factory, nil)
|
||||
|
||||
argv, err := shlex.Split(cli)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cmd.SetArgs(argv)
|
||||
|
||||
cmd.SetIn(&bytes.Buffer{})
|
||||
cmd.SetOut(ioutil.Discard)
|
||||
cmd.SetErr(ioutil.Discard)
|
||||
|
||||
_, err = cmd.ExecuteC()
|
||||
return &test.CmdOut{
|
||||
OutBuf: stdout,
|
||||
ErrBuf: stderr,
|
||||
}, err
|
||||
}
|
||||
|
||||
func TestPRReview_validation(t *testing.T) {
|
||||
for _, cmd := range []string{
|
||||
`--approve --comment 123`,
|
||||
`--approve --comment -b"hey" 123`,
|
||||
} {
|
||||
t.Run(cmd, func(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
_, err := runCommand(http, nil, false, cmd)
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
assert.Equal(t, "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment", err.Error())
|
||||
})
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRReview_bad_body(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
_, err := runCommand(http, nil, false, `123 -b "radical"`)
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
assert.Equal(t, "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment", err.Error())
|
||||
}
|
||||
|
||||
func TestPRReview_url_arg(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"id": "foobar123",
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO",
|
||||
"defaultBranchRef": {
|
||||
"name": "master"
|
||||
}
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } } `))
|
||||
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
|
||||
|
||||
output, err := runCommand(http, nil, true, "--approve https://github.com/OWNER/REPO/pull/123")
|
||||
if err != nil {
|
||||
t.Fatalf("error running pr review: %s", err)
|
||||
}
|
||||
|
||||
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
PullRequestID string
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
|
||||
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, "", reqBody.Variables.Input.Body)
|
||||
}
|
||||
|
||||
func TestPRReview_number_arg(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": { "pullRequest": {
|
||||
"id": "foobar123",
|
||||
"number": 123,
|
||||
"headRefName": "feature",
|
||||
"headRepositoryOwner": {
|
||||
"login": "hubot"
|
||||
},
|
||||
"headRepository": {
|
||||
"name": "REPO",
|
||||
"defaultBranchRef": {
|
||||
"name": "master"
|
||||
}
|
||||
},
|
||||
"isCrossRepository": false,
|
||||
"maintainerCanModify": false
|
||||
} } } } `))
|
||||
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
|
||||
|
||||
output, err := runCommand(http, nil, true, "--approve 123")
|
||||
if err != nil {
|
||||
t.Fatalf("error running pr review: %s", err)
|
||||
}
|
||||
|
||||
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
PullRequestID string
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
|
||||
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, "", reqBody.Variables.Input.Body)
|
||||
}
|
||||
|
||||
func TestPRReview_no_arg(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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(http, nil, true, `--comment -b "cool story"`)
|
||||
if err != nil {
|
||||
t.Fatalf("error running pr review: %s", err)
|
||||
}
|
||||
|
||||
test.ExpectLines(t, output.Stderr(), "Reviewed pull request #123")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
PullRequestID string
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
|
||||
assert.Equal(t, "COMMENT", reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
|
||||
}
|
||||
|
||||
func TestPRReview_blank_comment(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
_, err := runCommand(http, nil, false, `--comment 123`)
|
||||
assert.Equal(t, "did not understand desired review action: body cannot be blank for comment review", err.Error())
|
||||
}
|
||||
|
||||
func TestPRReview_blank_request_changes(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
_, err := runCommand(http, nil, false, `-r 123`)
|
||||
assert.Equal(t, "did not understand desired review action: body cannot be blank for request-changes review", err.Error())
|
||||
}
|
||||
|
||||
func TestPRReview(t *testing.T) {
|
||||
type c struct {
|
||||
Cmd string
|
||||
ExpectedEvent string
|
||||
ExpectedBody string
|
||||
}
|
||||
cases := []c{
|
||||
{`--request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
|
||||
{`--approve`, "APPROVE", ""},
|
||||
{`--approve -b"hot damn"`, "APPROVE", "hot damn"},
|
||||
{`--comment --body "i donno"`, "COMMENT", "i donno"},
|
||||
}
|
||||
|
||||
for _, kase := range cases {
|
||||
t.Run(kase.Cmd, func(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": { "pullRequests": { "nodes": [
|
||||
{ "url": "https://github.com/OWNER/REPO/pull/123",
|
||||
"id": "foobar123",
|
||||
"headRefName": "feature",
|
||||
"baseRefName": "master" }
|
||||
] } } } }
|
||||
`))
|
||||
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
|
||||
|
||||
_, err := runCommand(http, nil, false, kase.Cmd)
|
||||
if err != nil {
|
||||
t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err)
|
||||
}
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, kase.ExpectedEvent, reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, kase.ExpectedBody, reqBody.Variables.Input.Body)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRReview_nontty_insufficient_flags(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
output, err := runCommand(http, nil, false, "")
|
||||
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) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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(http, nil, false, "-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[1].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) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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": {} }`))
|
||||
as, teardown := prompt.InitAskStubber()
|
||||
defer teardown()
|
||||
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "reviewType",
|
||||
Value: "Approve",
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "body",
|
||||
Value: "cool story",
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "confirm",
|
||||
Value: true,
|
||||
},
|
||||
})
|
||||
|
||||
output, err := runCommand(http, nil, true, "")
|
||||
if err != nil {
|
||||
t.Fatalf("got unexpected error running pr review: %s", err)
|
||||
}
|
||||
|
||||
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
|
||||
|
||||
test.ExpectLines(t, output.String(),
|
||||
"Got:",
|
||||
"cool.*story")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
|
||||
}
|
||||
|
||||
func TestPRReview_interactive_no_body(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
http.StubResponse(200, bytes.NewBufferString(`
|
||||
{ "data": { "repository": { "pullRequests": { "nodes": [
|
||||
{ "url": "https://github.com/OWNER/REPO/pull/123",
|
||||
"id": "foobar123",
|
||||
"headRefName": "feature",
|
||||
"baseRefName": "master" }
|
||||
] } } } }
|
||||
`))
|
||||
|
||||
as, teardown := prompt.InitAskStubber()
|
||||
defer teardown()
|
||||
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "reviewType",
|
||||
Value: "Request changes",
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "body",
|
||||
Default: true,
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "confirm",
|
||||
Value: true,
|
||||
},
|
||||
})
|
||||
|
||||
_, err := runCommand(http, nil, true, "")
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
assert.Equal(t, "this type of review cannot be blank", err.Error())
|
||||
}
|
||||
|
||||
func TestPRReview_interactive_blank_approve(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
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": {} }`))
|
||||
as, teardown := prompt.InitAskStubber()
|
||||
defer teardown()
|
||||
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "reviewType",
|
||||
Value: "Approve",
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "body",
|
||||
Default: true,
|
||||
},
|
||||
})
|
||||
as.Stub([]*prompt.QuestionStub{
|
||||
{
|
||||
Name: "confirm",
|
||||
Value: true,
|
||||
},
|
||||
})
|
||||
|
||||
output, err := runCommand(http, nil, true, "")
|
||||
if err != nil {
|
||||
t.Fatalf("got unexpected error running pr review: %s", err)
|
||||
}
|
||||
|
||||
unexpect := regexp.MustCompile("Got:")
|
||||
if unexpect.MatchString(output.String()) {
|
||||
t.Errorf("did not expect to see body printed in %s", output.String())
|
||||
}
|
||||
|
||||
test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
|
||||
|
||||
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
|
||||
reqBody := struct {
|
||||
Variables struct {
|
||||
Input struct {
|
||||
Event string
|
||||
Body string
|
||||
}
|
||||
}
|
||||
}{}
|
||||
_ = json.Unmarshal(bodyBytes, &reqBody)
|
||||
|
||||
assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
|
||||
assert.Equal(t, "", reqBody.Variables.Input.Body)
|
||||
}
|
||||
121
pkg/cmd/pr/shared/lookup.go
Normal file
121
pkg/cmd/pr/shared/lookup.go
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
package shared
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/cli/cli/api"
|
||||
"github.com/cli/cli/context"
|
||||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
)
|
||||
|
||||
// PRFromArgs looks up the pull request from either the number/branch/URL argument or one belonging to the current branch
|
||||
//
|
||||
// NOTE: this API isn't great, but is here as a compatibility layer between old-style and new-style commands
|
||||
func PRFromArgs(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), branchFn func() (string, error), remotesFn func() (context.Remotes, error), arg string) (*api.PullRequest, ghrepo.Interface, error) {
|
||||
if arg != "" {
|
||||
// First check to see if the prString is a url, return repo from url if found. This
|
||||
// is run first because we don't need to run determineBaseRepo for this path
|
||||
pr, r, err := prFromURL(apiClient, arg)
|
||||
if pr != nil || err != nil {
|
||||
return pr, r, err
|
||||
}
|
||||
}
|
||||
|
||||
repo, err := baseRepoFn()
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
|
||||
}
|
||||
|
||||
// If there are no args see if we can guess the PR from the current branch
|
||||
if arg == "" {
|
||||
pr, err := prForCurrentBranch(apiClient, repo, branchFn, remotesFn)
|
||||
return pr, repo, err
|
||||
} else {
|
||||
// Next see if the prString is a number and use that to look up the url
|
||||
pr, err := prFromNumberString(apiClient, repo, arg)
|
||||
if pr != nil || err != nil {
|
||||
return pr, repo, err
|
||||
}
|
||||
|
||||
// Last see if it is a branch name
|
||||
pr, err = api.PullRequestForBranch(apiClient, repo, "", arg)
|
||||
return pr, repo, err
|
||||
}
|
||||
}
|
||||
|
||||
func prFromNumberString(apiClient *api.Client, repo ghrepo.Interface, s string) (*api.PullRequest, error) {
|
||||
if prNumber, err := strconv.Atoi(strings.TrimPrefix(s, "#")); err == nil {
|
||||
return api.PullRequestByNumber(apiClient, repo, prNumber)
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
|
||||
|
||||
func prFromURL(apiClient *api.Client, s string) (*api.PullRequest, ghrepo.Interface, error) {
|
||||
u, err := url.Parse(s)
|
||||
if err != nil {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
if u.Scheme != "https" && u.Scheme != "http" {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
m := pullURLRE.FindStringSubmatch(u.Path)
|
||||
if m == nil {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
|
||||
prNumberString := m[3]
|
||||
pr, err := prFromNumberString(apiClient, repo, prNumberString)
|
||||
return pr, repo, err
|
||||
}
|
||||
|
||||
func prForCurrentBranch(apiClient *api.Client, repo ghrepo.Interface, branchFn func() (string, error), remotesFn func() (context.Remotes, error)) (*api.PullRequest, error) {
|
||||
prHeadRef, err := branchFn()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
branchConfig := git.ReadBranchConfig(prHeadRef)
|
||||
|
||||
// the branch is configured to merge a special PR head ref
|
||||
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
|
||||
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
|
||||
return prFromNumberString(apiClient, repo, m[1])
|
||||
}
|
||||
|
||||
var branchOwner string
|
||||
if branchConfig.RemoteURL != nil {
|
||||
// the branch merges from a remote specified by URL
|
||||
if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil {
|
||||
branchOwner = r.RepoOwner()
|
||||
}
|
||||
} else if branchConfig.RemoteName != "" {
|
||||
// the branch merges from a remote specified by name
|
||||
rem, _ := remotesFn()
|
||||
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
|
||||
branchOwner = r.RepoOwner()
|
||||
}
|
||||
}
|
||||
|
||||
if branchOwner != "" {
|
||||
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
|
||||
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
|
||||
}
|
||||
// prepend `OWNER:` if this branch is pushed to a fork
|
||||
if !strings.EqualFold(branchOwner, repo.RepoOwner()) {
|
||||
prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef)
|
||||
}
|
||||
}
|
||||
|
||||
return api.PullRequestForBranch(apiClient, repo, "", prHeadRef)
|
||||
}
|
||||
|
|
@ -70,6 +70,7 @@ func cloneRun(opts *CloneOptions) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// TODO: GHE support
|
||||
protocol, err := cfg.Get("", "git_protocol")
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -198,17 +198,12 @@ func createRun(opts *CreateOptions) error {
|
|||
greenCheck := utils.Green("✓")
|
||||
isTTY := opts.IO.IsStdoutTTY()
|
||||
|
||||
if isTTY {
|
||||
fmt.Fprintf(stderr, "%s Created repository %s on GitHub\n", greenCheck, ghrepo.FullName(repo))
|
||||
} else {
|
||||
fmt.Fprintln(stdout, repo.URL)
|
||||
}
|
||||
|
||||
// TODO This is overly wordy and I'd like to streamline this.
|
||||
cfg, err := opts.Config()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// TODO: GHE support
|
||||
protocol, err := cfg.Get("", "git_protocol")
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -186,6 +186,7 @@ func forkRun(opts *ForkOptions) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// TODO: GHE support
|
||||
protocol, err := cfg.Get("", "git_protocol")
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -15,4 +15,5 @@ type Factory struct {
|
|||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Remotes func() (context.Remotes, error)
|
||||
Config func() (config.Config, error)
|
||||
Branch func() (string, error)
|
||||
}
|
||||
|
|
|
|||
23
pkg/cmdutil/legacy.go
Normal file
23
pkg/cmdutil/legacy.go
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
package cmdutil
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
|
||||
"github.com/cli/cli/internal/config"
|
||||
)
|
||||
|
||||
// TODO: consider passing via Factory
|
||||
// TODO: support per-hostname settings
|
||||
func DetermineEditor(cf func() (config.Config, error)) (string, error) {
|
||||
editorCommand := os.Getenv("GH_EDITOR")
|
||||
if editorCommand == "" {
|
||||
cfg, err := cf()
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("could not read config: %w", err)
|
||||
}
|
||||
editorCommand, _ = cfg.Get("", "editor")
|
||||
}
|
||||
|
||||
return editorCommand, nil
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue