Isolate pr review command

command/root.go

@@ -16,11 +16,13 @@ import (
 	"github.com/MakeNowJust/heredoc"
 	"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"
 	apiCmd "github.com/cli/cli/pkg/cmd/api"
 	gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create"
+	prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review"
 	repoCmd "github.com/cli/cli/pkg/cmd/repo"
 	repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone"
 	repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create"
@@ -113,6 +115,13 @@ func init() {
 			}
 			return cfg, nil
 		},
+		Branch: func() (string, error) {
+			currentBranch, err := git.CurrentBranch()
+			if err != nil {
+				return "", fmt.Errorf("could not determine current branch: %w", err)
+			}
+			return currentBranch, nil
+		},
 	}
 	RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil))
 
@@ -160,6 +169,8 @@ func init() {
 	repoCmd.Cmd.AddCommand(repoCreateCmd.NewCmdCreate(cmdFactory, nil))
 	repoCmd.Cmd.AddCommand(creditsCmd.NewCmdRepoCredits(&repoResolvingCmdFactory, nil))
 
+	prCmd.AddCommand(prReviewCmd.NewCmdReview(&repoResolvingCmdFactory, nil))
+
 	RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil))
 }
 
@@ -401,6 +412,7 @@ func formatRemoteURL(cmd *cobra.Command, repo ghrepo.Interface) string {
 	return fmt.Sprintf("https://%s/%s/%s.git", repo.RepoHost(), repo.RepoOwner(), repo.RepoName())
 }
 
+// TODO there is a parallel implementation for isolated commands
 func determineEditor(cmd *cobra.Command) (string, error) {
 	editorCommand := os.Getenv("GH_EDITOR")
 	if editorCommand == "" {

pkg/cmd/pr/review/review.go

@@ -1,79 +1,171 @@
-package command
+package review
 
 import (
 	"errors"
 	"fmt"
+	"net/http"
 
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/MakeNowJust/heredoc"
-	"github.com/spf13/cobra"
-
 	"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"
 )
 
-func init() {
+type ReviewOptions struct {
-	prCmd.AddCommand(prReviewCmd)
+	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)
 
-	prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request")
+	SelectorArg    string
-	prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request")
+	Approve        bool
-	prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request")
+	RequestChanges bool
-	prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review")
+	Comment        bool
+	Body           string
 }
 
-var prReviewCmd = &cobra.Command{
+func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Command {
-	Use:   "review [<number> | <url> | <branch>]",
+	opts := &ReviewOptions{
-	Short: "Add a review to a pull request",
+		IO:         f.IOStreams,
-	Long: `Add a review to a pull request.
+		HttpClient: f.HttpClient,
+		Config:     f.Config,
+		BaseRepo:   f.BaseRepo,
+		Remotes:    f.Remotes,
+		Branch:     f.Branch,
+	}
 
-Without an argument, the pull request that belongs to the current branch is reviewed.`,
+	cmd := &cobra.Command{
-	Example: heredoc.Doc(`
+		Use:   "review [<number> | <url> | <branch>]",
-	# approve the pull request of the current branch
+		Short: "Add a review to a pull request",
-	$ gh pr review --approve
+		Long: heredoc.Doc(`
-	
+			Add a review to a pull request.
-	# leave a review comment for the current branch
+
-	$ gh pr review --comment -b "interesting"
+			Without an argument, the pull request that belongs to the current branch is reviewed.
-	
+		`),
-	# add a review for a specific pull request
+		Example: heredoc.Doc(`
-	$ gh pr review 123
+			# approve the pull request of the current branch
-	
+			$ gh pr review --approve
-	# request changes on a specific pull request
+			
-	$ gh pr review 123 -r -b "needs more ASCII art"
+			# leave a review comment for the current branch
-	`),
+			$ gh pr review --comment -b "interesting"
-	Args: cobra.MaximumNArgs(1),
+			
-	RunE: prReview,
+			# 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 processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
+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, _, 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, 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 cmd.Flags().Changed("approve") {
+	if opts.Approve {
 		found++
 		flag = "approve"
 		state = api.ReviewApprove
 	}
-	if cmd.Flags().Changed("request-changes") {
+	if opts.RequestChanges {
 		found++
 		flag = "request-changes"
 		state = api.ReviewRequestChanges
 	}
-	if cmd.Flags().Changed("comment") {
+	if opts.Comment {
 		found++
 		flag = "comment"
 		state = api.ReviewComment
 	}
 
-	body, err := cmd.Flags().GetString("body")
+	body := opts.Body
-	if err != nil {
-		return nil, err
-	}
 
 	if found == 0 && body == "" {
-		if connectedToTerminal(cmd) {
+		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")
@@ -93,63 +185,7 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
 	}, nil
 }
 
-func prReview(cmd *cobra.Command, args []string) error {
+func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullRequestReviewInput, error) {
-	ctx := contextForCommand(cmd)
-	apiClient, err := apiClientForContext(ctx)
-	if err != nil {
-		return err
-	}
-
-	pr, _, err := prFromArgs(ctx, apiClient, cmd, args)
-	if err != nil {
-		return err
-	}
-
-	reviewData, err := processReviewOpt(cmd)
-	if err != nil {
-		return fmt.Errorf("did not understand desired review action: %w", err)
-	}
-
-	stderr := colorableErr(cmd)
-
-	if reviewData == nil {
-		reviewData, err = reviewSurvey(cmd)
-		if err != nil {
-			return err
-		}
-		if reviewData == nil && err == nil {
-			fmt.Fprint(stderr, "Discarding.\n")
-			return nil
-		}
-	}
-
-	err = api.AddReview(apiClient, pr, reviewData)
-	if err != nil {
-		return fmt.Errorf("failed to create review: %w", err)
-	}
-
-	if !connectedToTerminal(cmd) {
-		return nil
-	}
-
-	switch reviewData.State {
-	case api.ReviewComment:
-		fmt.Fprintf(stderr, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number)
-	case api.ReviewApprove:
-		fmt.Fprintf(stderr, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number)
-	case api.ReviewRequestChanges:
-		fmt.Fprintf(stderr, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number)
-	}
-
-	return nil
-}
-
-func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
-	editorCommand, err := determineEditor(cmd)
-	if err != nil {
-		return nil, err
-	}
-
 	typeAnswers := struct {
 		ReviewType string
 	}{}
@@ -167,7 +203,7 @@ func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
 		},
 	}
 
-	err = prompt.SurveyAsk(typeQs, &typeAnswers)
+	err := prompt.SurveyAsk(typeQs, &typeAnswers)
 	if err != nil {
 		return nil, err
 	}
@@ -218,13 +254,12 @@ func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
 	}
 
 	if len(bodyAnswers.Body) > 0 {
-		out := colorableOut(cmd)
 		renderedBody, err := utils.RenderMarkdown(bodyAnswers.Body)
 		if err != nil {
 			return nil, err
 		}
 
-		fmt.Fprintf(out, "Got:\n%s", renderedBody)
+		fmt.Fprintf(io.Out, "Got:\n%s", renderedBody)
 	}
 
 	confirm := false

pkg/cmd/pr/review/review_test.go

@@ -1,58 +1,112 @@
-package command
+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) {
-	initBlankContext("", "OWNER/REPO", "master")
-	http := initFakeHTTP()
 	for _, cmd := range []string{
-		`pr review --approve --comment 123`,
+		`--approve --comment 123`,
-		`pr review --approve --comment -b"hey" 123`,
+		`--approve --comment -b"hey" 123`,
 	} {
-		http.StubRepoResponse("OWNER", "REPO")
+		t.Run(cmd, func(t *testing.T) {
-		http.StubResponse(200, bytes.NewBufferString(`
+			http := &httpmock.Registry{}
-			{ "data": { "repository": {
+			defer http.Verify(t)
-				"pullRequest": { "number": 123 }
+
-			} } }
+			_, err := runCommand(http, nil, false, cmd)
-		`))
+			if err == nil {
-		_, err := RunCommand(cmd)
+				t.Fatal("expected error")
-		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())
-		}
+		})
-		eq(t, err.Error(), "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment")
+
 	}
 }
 
 func TestPRReview_bad_body(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "master")
+	http := &httpmock.Registry{}
-	http := initFakeHTTP()
+	defer http.Verify(t)
-	http.StubRepoResponse("OWNER", "REPO")
+
-	http.StubResponse(200, bytes.NewBufferString(`
+	_, err := runCommand(http, nil, false, `123 -b "radical"`)
-		{ "data": { "repository": {
-			"pullRequest": { "number": 123 }
-		} } }
-	`))
-	_, err := RunCommand(`pr review 123 -b "radical"`)
 	if err == nil {
 		t.Fatal("expected error")
 	}
-	eq(t, err.Error(), "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment")
+	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) {
-	initBlankContext("", "OWNER/REPO", "master")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequest": {
 			"id": "foobar123",
@@ -72,7 +126,7 @@ func TestPRReview_url_arg(t *testing.T) {
 		} } } } `))
 	http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
 
-	output, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123")
+	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)
 	}
@@ -91,16 +145,14 @@ func TestPRReview_url_arg(t *testing.T) {
 	}{}
 	_ = json.Unmarshal(bodyBytes, &reqBody)
 
-	eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
+	assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
-	eq(t, reqBody.Variables.Input.Event, "APPROVE")
+	assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
-	eq(t, reqBody.Variables.Input.Body, "")
+	assert.Equal(t, "", reqBody.Variables.Input.Body)
 }
 
 func TestPRReview_number_arg(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "master")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequest": {
 			"id": "foobar123",
@@ -120,14 +172,14 @@ func TestPRReview_number_arg(t *testing.T) {
 		} } } } `))
 	http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
 
-	output, err := RunCommand("pr review --approve 123")
+	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[2].Body)
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
 	reqBody := struct {
 		Variables struct {
 			Input struct {
@@ -139,16 +191,14 @@ func TestPRReview_number_arg(t *testing.T) {
 	}{}
 	_ = json.Unmarshal(bodyBytes, &reqBody)
 
-	eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
+	assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
-	eq(t, reqBody.Variables.Input.Event, "APPROVE")
+	assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
-	eq(t, reqBody.Variables.Input.Body, "")
+	assert.Equal(t, "", reqBody.Variables.Input.Body)
 }
 
 func TestPRReview_no_arg(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
@@ -159,14 +209,14 @@ func TestPRReview_no_arg(t *testing.T) {
 		] } } } }`))
 	http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
 
-	output, err := RunCommand(`pr review --comment -b "cool story"`)
+	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[2].Body)
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
 	reqBody := struct {
 		Variables struct {
 			Input struct {
@@ -178,37 +228,25 @@ func TestPRReview_no_arg(t *testing.T) {
 	}{}
 	_ = json.Unmarshal(bodyBytes, &reqBody)
 
-	eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
+	assert.Equal(t, "foobar123", reqBody.Variables.Input.PullRequestID)
-	eq(t, reqBody.Variables.Input.Event, "COMMENT")
+	assert.Equal(t, "COMMENT", reqBody.Variables.Input.Event)
-	eq(t, reqBody.Variables.Input.Body, "cool story")
+	assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
 }
 
 func TestPRReview_blank_comment(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "master")
+	http := &httpmock.Registry{}
-	http := initFakeHTTP()
+	defer http.Verify(t)
-	http.StubRepoResponse("OWNER", "REPO")
-	http.StubResponse(200, bytes.NewBufferString(`
-		{ "data": { "repository": {
-			"pullRequest": { "number": 123 }
-		} } }
-	`))
 
-	_, err := RunCommand(`pr review --comment 123`)
+	_, err := runCommand(http, nil, false, `--comment 123`)
-	eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review")
+	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) {
-	initBlankContext("", "OWNER/REPO", "master")
+	http := &httpmock.Registry{}
-	http := initFakeHTTP()
+	defer http.Verify(t)
-	http.StubRepoResponse("OWNER", "REPO")
-	http.StubResponse(200, bytes.NewBufferString(`
-		{ "data": { "repository": {
-			"pullRequest": { "number": 123 }
-		} } }
-	`))
 
-	_, err := RunCommand(`pr review -r 123`)
+	_, err := runCommand(http, nil, false, `-r 123`)
-	eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review")
+	assert.Equal(t, "did not understand desired review action: body cannot be blank for request-changes review", err.Error())
 }
 
 func TestPRReview(t *testing.T) {
@@ -218,63 +256,53 @@ func TestPRReview(t *testing.T) {
 		ExpectedBody  string
 	}
 	cases := []c{
-		{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
+		{`--request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
-		{`pr review --approve`, "APPROVE", ""},
+		{`--approve`, "APPROVE", ""},
-		{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"},
+		{`--approve -b"hot damn"`, "APPROVE", "hot damn"},
-		{`pr review --comment --body "i donno"`, "COMMENT", "i donno"},
+		{`--comment --body "i donno"`, "COMMENT", "i donno"},
 	}
 
 	for _, kase := range cases {
-		initBlankContext("", "OWNER/REPO", "feature")
+		t.Run(kase.Cmd, func(t *testing.T) {
-		http := initFakeHTTP()
+			http := &httpmock.Registry{}
-		http.StubRepoResponse("OWNER", "REPO")
+			defer http.Verify(t)
-		http.StubResponse(200, bytes.NewBufferString(`
+			http.StubResponse(200, bytes.NewBufferString(`
-		{ "data": { "repository": { "pullRequests": { "nodes": [
+				{ "data": { "repository": { "pullRequests": { "nodes": [
-			{ "url": "https://github.com/OWNER/REPO/pull/123",
+					{ "url": "https://github.com/OWNER/REPO/pull/123",
-			  "id": "foobar123",
+					"id": "foobar123",
-			  "headRefName": "feature",
+					"headRefName": "feature",
-				"baseRefName": "master" }
+						"baseRefName": "master" }
-		] } } } }
+				] } } } }
-	`))
+			`))
-		http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
+			http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
 
-		_, err := RunCommand(kase.Cmd)
+			_, err := runCommand(http, nil, false, kase.Cmd)
-		if err != nil {
+			if err != nil {
-			t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err)
+				t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err)
-		}
-
-		bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
-		reqBody := struct {
-			Variables struct {
-				Input struct {
-					Event string
-					Body  string
-				}
 			}
-		}{}
-		_ = json.Unmarshal(bodyBytes, &reqBody)
 
-		eq(t, reqBody.Variables.Input.Event, kase.ExpectedEvent)
+			bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
-		eq(t, reqBody.Variables.Input.Body, kase.ExpectedBody)
+			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) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(false)()
+	defer http.Verify(t)
-	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")
+	output, err := runCommand(http, nil, false, "")
 	if err == nil {
 		t.Fatal("expected error")
 	}
@@ -285,10 +313,8 @@ func TestPRReview_nontty_insufficient_flags(t *testing.T) {
 }
 
 func TestPRReview_nontty(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(false)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
@@ -300,7 +326,7 @@ func TestPRReview_nontty(t *testing.T) {
 	`))
 
 	http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
-	output, err := RunCommand("pr review -c -bcool")
+	output, err := runCommand(http, nil, false, "-c -bcool")
 	if err != nil {
 		t.Fatalf("unexpected error running command: %s", err)
 	}
@@ -308,7 +334,7 @@ func TestPRReview_nontty(t *testing.T) {
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "", output.Stderr())
 
-	bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
 	reqBody := struct {
 		Variables struct {
 			Input struct {
@@ -324,10 +350,8 @@ func TestPRReview_nontty(t *testing.T) {
 }
 
 func TestPRReview_interactive(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
@@ -360,7 +384,7 @@ func TestPRReview_interactive(t *testing.T) {
 		},
 	})
 
-	output, err := RunCommand(`pr review`)
+	output, err := runCommand(http, nil, true, "")
 	if err != nil {
 		t.Fatalf("got unexpected error running pr review: %s", err)
 	}
@@ -371,7 +395,7 @@ func TestPRReview_interactive(t *testing.T) {
 		"Got:",
 		"cool.*story")
 
-	bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
 	reqBody := struct {
 		Variables struct {
 			Input struct {
@@ -382,15 +406,13 @@ func TestPRReview_interactive(t *testing.T) {
 	}{}
 	_ = json.Unmarshal(bodyBytes, &reqBody)
 
-	eq(t, reqBody.Variables.Input.Event, "APPROVE")
+	assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
-	eq(t, reqBody.Variables.Input.Body, "cool story")
+	assert.Equal(t, "cool story", reqBody.Variables.Input.Body)
 }
 
 func TestPRReview_interactive_no_body(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
@@ -399,7 +421,7 @@ func TestPRReview_interactive_no_body(t *testing.T) {
 				"baseRefName": "master" }
 		] } } } }
 	`))
-	http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
+
 	as, teardown := prompt.InitAskStubber()
 	defer teardown()
 
@@ -422,18 +444,16 @@ func TestPRReview_interactive_no_body(t *testing.T) {
 		},
 	})
 
-	_, err := RunCommand(`pr review`)
+	_, err := runCommand(http, nil, true, "")
 	if err == nil {
 		t.Fatal("expected error")
 	}
-	eq(t, err.Error(), "this type of review cannot be blank")
+	assert.Equal(t, "this type of review cannot be blank", err.Error())
 }
 
 func TestPRReview_interactive_blank_approve(t *testing.T) {
-	initBlankContext("", "OWNER/REPO", "feature")
+	http := &httpmock.Registry{}
-	defer stubTerminal(true)()
+	defer http.Verify(t)
-	http := initFakeHTTP()
-	http.StubRepoResponse("OWNER", "REPO")
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
@@ -466,7 +486,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
 		},
 	})
 
-	output, err := RunCommand(`pr review`)
+	output, err := runCommand(http, nil, true, "")
 	if err != nil {
 		t.Fatalf("got unexpected error running pr review: %s", err)
 	}
@@ -478,7 +498,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
 
 	test.ExpectLines(t, output.Stderr(), "Approved pull request #123")
 
-	bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
 	reqBody := struct {
 		Variables struct {
 			Input struct {
@@ -489,7 +509,6 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
 	}{}
 	_ = json.Unmarshal(bodyBytes, &reqBody)
 
-	eq(t, reqBody.Variables.Input.Event, "APPROVE")
+	assert.Equal(t, "APPROVE", reqBody.Variables.Input.Event)
-	eq(t, reqBody.Variables.Input.Body, "")
+	assert.Equal(t, "", reqBody.Variables.Input.Body)
-
 }

pkg/cmdutil/factory.go

@@ -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)
 }

pkg/cmdutil/legacy.go

@@ -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
+}