Separate out NewCmdBrowse tests from runBrowse tests

Co-authored-by: Benjamin Chadwick <benchadwick87@gmail.com>
Co-authored-by: Jessica Sestak <jlsestak@gmail.com>

pkg/cmd/browse/browse.go

@@ -35,8 +35,7 @@ type BrowseOptions struct {
 	WikiFlag     bool
 }
 
-func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command {
+func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Command {
-
 	opts := &BrowseOptions{
 		Browser:    f.Browser,
 		HttpClient: f.HttpClient,
@@ -93,6 +92,9 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command {
 				opts.SelectorArg = args[0]
 			}
 
+			if runF != nil {
+				return runF(opts)
+			}
 			return runBrowse(opts)
 		},
 	}

pkg/cmd/browse/browse_test.go

@@ -1,160 +1,113 @@
 package browse
 
 import (
-	"bytes"
-	"io/ioutil"
 	"net/http"
 	"testing"
 
 	"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 runCommand(rt http.RoundTripper, repo ghrepo.Interface, cli string) (*test.CmdOut, error) {
+func TestNewCmdBrowse(t *testing.T) {
-	io, _, stdout, stderr := iostreams.Test()
+	f := cmdutil.Factory{}
+	var opts *BrowseOptions
+	// pass a stub implementation of `runBrowse` for testing to avoid having real `runBrowse` called
+	cmd := NewCmdBrowse(&f, func(o *BrowseOptions) error {
+		opts = o
+		return nil
+	})
 
-	browser := &cmdutil.TestBrowser{}
+	cmd.SetArgs([]string{"--branch", "main"})
-	factory := &cmdutil.Factory{
+	_, err := cmd.ExecuteC()
-		IOStreams: io,
+	assert.NoError(t, err)
-		Browser:   browser,
 
-		HttpClient: func() (*http.Client, error) {
+	assert.Equal(t, "main", opts.Branch)
-			return &http.Client{Transport: rt}, nil
+	assert.Equal(t, "", opts.SelectorArg)
-		},
+	assert.Equal(t, false, opts.ProjectsFlag)
-		BaseRepo: func() (ghrepo.Interface, error) {
+	assert.Equal(t, false, opts.WikiFlag)
-			return repo, nil
+	assert.Equal(t, false, opts.SettingsFlag)
-		},
+	assert.Equal(t, 1, opts.FlagAmount)
-	}
-
-	cmd := NewCmdBrowse(factory)
-
-	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,
-		BrowsedURL: browser.BrowsedURL(),
-	}, err
 }
-// func TestNewCmdBrowse(t *testing.T) {
 
-// 	type args struct {
+func Test_runBrowse(t *testing.T) {
-// 		repo ghrepo.Interface
+	tests := []struct {
-// 		cli  string
+		name          string
-// 	}
+		opts          BrowseOptions
-
+		baseRepo      ghrepo.Interface
-// 	tests := []struct {
+		defaultBranch string
-// 		name           string
+		expectedURL   string
-// 		args           args
+		wantsErr      bool
-// 		errorExpected  bool
+	}{
-// 		stdoutExpected string
-// 		stderrExpected string
-// 		urlExpected    string
-// 	}{
-// 		{
-// 			name: "multiple flag",
-// 			args: args{
-// 				repo: ghrepo.New("jessica", "cli"),
-// 				cli:  "--settings --projects",
-// 			},
-// 			errorExpected:  true,
-// 			stdoutExpected: "",
-// 			stderrExpected: "these two flags are incompatible, see below for instructions",
-// 			urlExpected:    "",
-// 		},
-// 		{
-// 			name: "settings flag",
-// 			args: args{
-// 				repo: ghrepo.New("husrav", "cli"),
-// 				cli:  "--settings",
-// 			},
-// 			errorExpected:  false,
-// 			stdoutExpected: "now opening https://github.com/husrav/cli/settings in browser . . .\n",
-// 			stderrExpected: "",
-// 			urlExpected:    "https://github.com/husrav/cli/settings",
-// 		},
-// 		{
-// 			name: "projects flag",
-// 			args: args{
-// 				repo: ghrepo.New("ben", "cli"),
-// 				cli:  "--projects",
-// 			},
-// 			errorExpected:  false,
-// 			stdoutExpected: "now opening https://github.com/ben/cli/projects in browser . . .\n",
-// 			stderrExpected: "",
-// 			urlExpected:    "https://github.com/ben/cli/projects",
-// 		},
-// 		{
-// 			name: "wiki flag",
-// 			args: args{
-// 				repo: ghrepo.New("thanh", "cli"),
-// 				cli:  "--wiki",
-// 			},
-// 			errorExpected:  false,
-// 			stdoutExpected: "now opening https://github.com/thanh/cli/wiki in browser . . .\n",
-// 			stderrExpected: "",
-// 			urlExpected:    "https://github.com/thanh/cli/wiki",
-// 		},
-// 	}
-
-// 	for _, tt := range tests {
-// 		t.Run(tt.name, func(t *testing.T) {
-// 			http := &httpmock.Registry{}
-// 			// http.StubRepoInfoResponse(tt.args.repo.RepoHost(), tt.args.repo.RepoName(), "main")
-// 			defer http.Verify(t)
-
-// 			_, cmdTeardown := run.Stub()
-// 			defer cmdTeardown(t)
-
-// 			output, err := runCommand(http, tt.args.repo, tt.args.cli)
-
-// 			if tt.errorExpected {
-// 				assert.Equal(t, err.Error(), tt.stderrExpected)
-// 			} else {
-// 				assert.Equal(t, err, nil)
-// 			}
-// 			assert.Equal(t, tt.stdoutExpected, output.OutBuf.String())
-// 			assert.Equal(t, tt.urlExpected, output.BrowsedURL)
-// 		})
-// 	}
-// }
-
-func TestBrowse(t *testing.T) {
-	tests := []BrowseOptions{
 		{
-
+			name: "no arguments",
-			BaseRepo   func() (ghrepo.Interface, error)
+			opts: BrowseOptions{
-			Browser    browser
+				SelectorArg: "",
-			HttpClient func() (*http.Client, error)
+				FlagAmount:  0,
-			IO         *iostreams.IOStreams
+			},
-
+			baseRepo:    ghrepo.New("jessica", "cli"),
-			FlagAmount  int
+			expectedURL: "https://github.com/jessica/cli",
-			SelectorArg string
+		},
-
+		{
-			Branch       string
+			name:          "file argument",
-			ProjectsFlag bool
+			opts:          BrowseOptions{SelectorArg: "path/to/file.txt"},
-			RepoFlag     bool
+			baseRepo:      ghrepo.New("bchadwic", "cli"),
-			SettingsFlag bool
+			defaultBranch: "main",
-			WikiFlag     bool
+			expectedURL:   "https://github.com/bchadwic/cli/tree/main/path/to/file.txt",
+		},
+		{
+			name: "branch flag",
+			opts: BrowseOptions{
+				Branch:     "trunk",
+				FlagAmount: 1,
+			},
+			baseRepo:    ghrepo.New("bchadwic", "cli"),
+			expectedURL: "https://github.com/bchadwic/cli/tree/trunk",
+		},
+		{
+			name: "settings flag",
+			opts: BrowseOptions{
+				SettingsFlag: true,
+				FlagAmount:   1,
+			},
+			baseRepo:    ghrepo.New("bchadwic", "cli"),
+			expectedURL: "https://github.com/bchadwic/cli/settings",
 		},
 	}
-	for _,tt range tests {
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			io, _, stdout, stderr := iostreams.Test()
+			browser := cmdutil.TestBrowser{}
 
+			reg := httpmock.Registry{}
+			defer reg.Verify(t)
+			if tt.defaultBranch != "" {
+				reg.StubRepoInfoResponse(tt.baseRepo.RepoOwner(), tt.baseRepo.RepoName(), tt.defaultBranch)
+			}
+
+			opts := tt.opts
+			opts.IO = io
+			opts.BaseRepo = func() (ghrepo.Interface, error) {
+				return tt.baseRepo, nil
+			}
+			opts.HttpClient = func() (*http.Client, error) {
+				return &http.Client{Transport: &reg}, nil
+			}
+			opts.Browser = &browser
+
+			err := runBrowse(&opts)
+			if tt.wantsErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, "", stdout.String())
+			assert.Equal(t, "", stderr.String())
+			browser.Verify(t, tt.expectedURL)
+		})
 	}
 }
 
@@ -198,6 +151,5 @@ func TestFileArgParsing(t *testing.T) {
 			}
 			assert.Equal(t, tt.fileArg, arr[0])
 		}
-
 	}
 }

pkg/cmd/root/root.go

@@ -96,7 +96,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command {
 	repoResolvingCmdFactory := *f
 	repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo(f)
 
-	cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory))
+	cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory, nil))
 	cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory))
 	cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory))
 	cmd.AddCommand(releaseCmd.NewCmdRelease(&repoResolvingCmdFactory))