From 1a9704dbfb58db378e62025c2607360b997eb89a Mon Sep 17 00:00:00 2001 From: ttran112 Date: Sat, 5 Jun 2021 22:46:46 -0700 Subject: [PATCH 1/4] fix the route --- pkg/cmd/browse/browse.go | 7 +------ pkg/cmd/root/root.go | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index c1b5c8592..90f0ac9bb 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -31,7 +31,6 @@ type BrowseOptions struct { WikiFlag bool SettingsFlag bool BranchFlag bool - //LineFlag bool } type exitCode int @@ -139,7 +138,6 @@ func openInBrowser(cmd *cobra.Command, opts *BrowseOptions) error { } return printExit(response, cmd, opts, repoUrl) - } func addCombined(opts *BrowseOptions, url string, branchName string) (exitCode, string) { @@ -158,7 +156,6 @@ func addCombined(opts *BrowseOptions, url string, branchName string) (exitCode, } return exitUrlSuccess, url + "/tree/" + opts.AdditionalArg + "/" + arr[0] - } func addFlag(opts *BrowseOptions, url string) (exitCode, string) { @@ -169,7 +166,7 @@ func addFlag(opts *BrowseOptions, url string) (exitCode, string) { } else if opts.WikiFlag { return exitUrlSuccess, url + "/wiki" } - return exitExpectedArg, "" // Flag is a branch and needs an argument + return exitExpectedArg, "" } func addArg(opts *BrowseOptions, url string, branchName string) (exitCode, string) { @@ -204,10 +201,8 @@ func printExit(exit exitCode, cmd *cobra.Command, opts *BrowseOptions, url strin switch exit { case exitUrlSuccess: fmt.Fprintf(w, "now opening %s in browser . . .\n", cs.Bold(url)) - break case exitNonUrlSuccess: fmt.Fprintf(w, "now opening issue/pr in browser . . .\n") - break case exitNotInRepo: return fmt.Errorf("change directory to a repository to open in browser\n%s", help) case exitTooManyFlags: diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 76099a4b7..17b91ea81 100644 --- a/pkg/cmd/root/root.go +++ b/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(f)) // adds to the commands Commands() + cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory)) // adds to the commands Commands() cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) cmd.AddCommand(releaseCmd.NewCmdRelease(&repoResolvingCmdFactory)) From e425e897a68fd14642f550ee2f12b357d4b186ca Mon Sep 17 00:00:00 2001 From: ttran112 Date: Sat, 5 Jun 2021 23:31:19 -0700 Subject: [PATCH 2/4] create new test file for first browse branch --- pkg/cmd/browse/browse_test.go | 116 ++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 pkg/cmd/browse/browse_test.go diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go new file mode 100644 index 000000000..f4bf7cb42 --- /dev/null +++ b/pkg/cmd/browse/browse_test.go @@ -0,0 +1,116 @@ +package browse + +import ( + "bytes" + "io/ioutil" + "net/http" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/spf13/cobra" +) + +func TestNewCmdBrowse(t *testing.T) { + // TODO test the use of the api using "gh browse" + // instead of opening multiple browsers for each test, + // we can test the http code sent back after calling a site + +} + +func runCommand(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 + }, + } + + 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, + }, err +} + +func TestBrowseOpen(t *testing.T) { + runCommand(true, "") +} + +func Test_browseList(t *testing.T) { + type args struct { + repo ghrepo.Interface + cli string + } + tests := []struct { + name string + args args + urlExpected string + exitExpected exitCode + }{} + for _, test := range tests { + + } +} + +func createCommand(repo ghrepo.Interface, cli string) *cobra.Command { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(false) + io.SetStdinTTY(false) // Ask the team about TTY + io.SetStderrTTY(false) + + factory := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdView(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 +} From 6ed990008497c464da2d0135ba1d13fde368c167 Mon Sep 17 00:00:00 2001 From: bchadwic Date: Sun, 6 Jun 2021 01:09:43 -0700 Subject: [PATCH 3/4] working on the test file for the browse.go --- pkg/cmd/browse/browse_test.go | 212 +++++++++++++++++++++------------- 1 file changed, 129 insertions(+), 83 deletions(-) diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index f4bf7cb42..808e8efe1 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -16,101 +16,147 @@ import ( ) func TestNewCmdBrowse(t *testing.T) { - // TODO test the use of the api using "gh browse" - // instead of opening multiple browsers for each test, - // we can test the http code sent back after calling a site -} - -func runCommand(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 - }, - } - - 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, - }, err -} - -func TestBrowseOpen(t *testing.T) { - runCommand(true, "") -} - -func Test_browseList(t *testing.T) { type args struct { repo ghrepo.Interface cli string } tests := []struct { - name string - args args - urlExpected string - exitExpected exitCode - }{} - for _, test := range tests { + name string + args args + errorExpected error + stdoutExpected string + stderrExpected string + }{ + name: "test1", + arg : args{ + repo: ghrepo.New("bchadwic","cli"), + cli: "--settings", + }, + errorExpected: nil, + stdoutExpected: "now opening https://github.com/bchadwic/cli/settings in browser . . .\n", + stderrExpected: "", + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, stdout, stderr := iostreams.Test() + // tt.topic + + factory := &cmdutil.Factory{ + IOStreams: io, + //os.Getenv("BROWSER") + Browser: cmdutil.NewBrowser("", stdout, stderr), + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + BaseRepo: tt.args.repo, + }, + } + cmd := NewCmdBrowse(factory) + + argv, err := shlex.Split(cli) + if err != nil { + return + } + cmd.SetArgs(argv) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + + _, err := cmd.ExecuteC() + assert.Error(t, err) + if stdoutExpected != "" { + assert.Contains(t, stdout.String(), tt.outputExpected) // success outputs + } + if stderrExpected != "" { + assert.Contains(t, stderr.String(), tt.outputExpected) // error outputs + } + }) } } -func createCommand(repo ghrepo.Interface, cli string) *cobra.Command { - io, _, stdout, stderr := iostreams.Test() - io.SetStdoutTTY(false) - io.SetStdinTTY(false) // Ask the team about TTY - io.SetStderrTTY(false) - factory := &cmdutil.Factory{ - IOStreams: io, - Config: func() (config.Config, error) { - return config.NewBlankConfig(), nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - } - cmd := NewCmdView(factory, nil) +// func runCommand(isTTY bool, cli string) (*test.CmdOut, error) { +// io, _, stdout, stderr := iostreams.Test() +// io.SetStdoutTTY(isTTY) +// io.SetStdinTTY(isTTY) +// io.SetStderrTTY(isTTY) - argv, err := shlex.Split(cli) - if err != nil { - return nil, err - } - cmd.SetArgs(argv) +// 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 +// }, +// } - cmd.SetIn(&bytes.Buffer{}) - cmd.SetOut(ioutil.Discard) - cmd.SetErr(ioutil.Discard) +// cmd := NewCmdBrowse(factory) - _, err = cmd.ExecuteC() - return &test.CmdOut{ - OutBuf: stdout, - ErrBuf: stderr, - }, err -} +// 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 TestBrowseOpen(t *testing.T) { +// runCommand(true, "") +// } + +// func Test_browseList(t *testing.T) { +// for _, test := range tests { +// arg = test.args +// cmd := createCommand(arg.repo, arg.cli) +// err := cmd.RunE(); +// } +// } + +// func createCommand(repo ghrepo.Interface, cli string) *cobra.Command { +// io, _, stdout, stderr := iostreams.Test() +// io.SetStdoutTTY(false) +// io.SetStdinTTY(false) // Ask the team about TTY +// io.SetStderrTTY(false) + +// factory := &cmdutil.Factory{ +// IOStreams: io, +// Config: func() (config.Config, error) { +// return config.NewBlankConfig(), nil +// }, +// BaseRepo: repo +// }, +// } + +// cmd := NewCmdBrowse(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.RunE() +// return &test.CmdOut{ +// OutBuf: stdout, +// ErrBuf: stderr, +// }, err +// } From f79270400313a43d33889bb145e3a5184aa4968e Mon Sep 17 00:00:00 2001 From: Jessica Sestak Date: Mon, 7 Jun 2021 11:04:29 -0700 Subject: [PATCH 4/4] Worked on browse_test.go, still not getting errors --- pkg/cmd/browse/browse.go | 6 +- pkg/cmd/browse/browse_test.go | 221 ++++++++++++++-------------------- 2 files changed, 97 insertions(+), 130 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 90f0ac9bb..442a4e8f1 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -200,9 +200,11 @@ func printExit(exit exitCode, cmd *cobra.Command, opts *BrowseOptions, url strin switch exit { case exitUrlSuccess: - fmt.Fprintf(w, "now opening %s in browser . . .\n", cs.Bold(url)) + //fmt.Fprintf(w, "now opening %s in browser . . .\n", cs.Bold(url)) + fmt.Fprintln(w, "Hello World One") case exitNonUrlSuccess: - fmt.Fprintf(w, "now opening issue/pr in browser . . .\n") + //fmt.Fprintf(w, "now opening issue/pr in browser . . .\n") + fmt.Fprintln(w, "Hello World Two") case exitNotInRepo: return fmt.Errorf("change directory to a repository to open in browser\n%s", help) case exitTooManyFlags: diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 808e8efe1..780d69e22 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -6,157 +6,122 @@ import ( "net/http" "testing" - "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/spf13/cobra" + "github.com/stretchr/testify/assert" ) +type args struct { + repo ghrepo.Interface + cli string +} +type testCase struct { + name string + args args + errorExpected bool + stdoutExpected string + stderrExpected string +} + +func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + + browser := &cmdutil.TestBrowser{} + factory := &cmdutil.Factory{ + IOStreams: io, + Browser: browser, + + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdBrowse(factory) + + argv, err := shlex.Split(t.args.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 { - repo ghrepo.Interface - cli string - } - tests := []struct { - name string - args args - errorExpected error - stdoutExpected string - stderrExpected string - }{ - name: "test1", - arg : args{ - repo: ghrepo.New("bchadwic","cli"), - cli: "--settings", + var tests = []testCase{ + { + name: "test1", + args: args{ + repo: ghrepo.New("bchadwic", "cli"), + cli: "--settings --projects", + }, + errorExpected: true, + stdoutExpected: "", + stderrExpected: "Error: accepts 1 flag, 2 flag(s) were recieved\nUse 'gh browse --help' for more information about browse\n\n", + }, + { + name: "test2", + args: args{ + repo: ghrepo.New("bchadwic", "cli"), + cli: "--settings", + }, + errorExpected: false, + stdoutExpected: "hello world", + stderrExpected: "hello world", }, - errorExpected: nil, - stdoutExpected: "now opening https://github.com/bchadwic/cli/settings in browser . . .\n", - stderrExpected: "", } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, _, stdout, stderr := iostreams.Test() - // tt.topic - factory := &cmdutil.Factory{ - IOStreams: io, - //os.Getenv("BROWSER") - Browser: cmdutil.NewBrowser("", stdout, stderr), - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: rt}, nil - }, - BaseRepo: tt.args.repo, - }, - } - cmd := NewCmdBrowse(factory) + http := &httpmock.Registry{} + defer http.Verify(t) - argv, err := shlex.Split(cli) - if err != nil { - return - } - cmd.SetArgs(argv) - cmd.SetOut(stdout) - cmd.SetErr(stderr) + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) - _, err := cmd.ExecuteC() - assert.Error(t, err) - if stdoutExpected != "" { - assert.Contains(t, stdout.String(), tt.outputExpected) // success outputs - } - if stderrExpected != "" { - assert.Contains(t, stderr.String(), tt.outputExpected) // error outputs + output, err := runCommand(http, tt) + + if tt.errorExpected { + assert.Error(t, err) } + + assert.Contains(t, output.String(), tt.stdoutExpected) // success outputs + + assert.Contains(t, output.Stderr(), tt.stderrExpected) // error outputs + }) } } +// http := initFakeHTTP() +// defer http.Verify(t) +// _, cmdTeardown := run.Stub() +// defer cmdTeardown(t) -// func runCommand(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 -// }, -// } - -// 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, -// }, err +// output, err := runCommand(http, true, "--web -a peter -l bug -l docs -L 10 -s merged -B trunk") +// if err != nil { +// t.Errorf("error running command `pr list` with `--web` flag: %v", err) // } -// func TestBrowseOpen(t *testing.T) { -// runCommand(true, "") -// } - -// func Test_browseList(t *testing.T) { -// for _, test := range tests { -// arg = test.args -// cmd := createCommand(arg.repo, arg.cli) -// err := cmd.RunE(); -// } -// } - -// func createCommand(repo ghrepo.Interface, cli string) *cobra.Command { -// io, _, stdout, stderr := iostreams.Test() -// io.SetStdoutTTY(false) -// io.SetStdinTTY(false) // Ask the team about TTY -// io.SetStderrTTY(false) - -// factory := &cmdutil.Factory{ -// IOStreams: io, -// Config: func() (config.Config, error) { -// return config.NewBlankConfig(), nil -// }, -// BaseRepo: repo -// }, -// } - -// cmd := NewCmdBrowse(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.RunE() -// return &test.CmdOut{ -// OutBuf: stdout, -// ErrBuf: stderr, -// }, err -// } +// assert.Equal(t, "", output.String()) +// assert.Equal(t, "Opening github.com/OWNER/REPO/pulls in your browser.\n", output.Stderr()) +// assert.Equal(t, "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk", output.BrowsedURL)