From 569d2fd58afb9f80108e4d1b3c8d3c3975f14e99 Mon Sep 17 00:00:00 2001 From: bchadwic Date: Mon, 7 Jun 2021 21:47:08 -0700 Subject: [PATCH 1/4] Got tests objects to fully work! Currently working on making more test cases --- .vscode/settings.json | 8 ++- pkg/cmd/browse/browse.go | 119 ++++++++++++++-------------------- pkg/cmd/browse/browse_test.go | 91 ++++++++++++++------------ pkg/cmd/root/root.go | 2 +- 4 files changed, 108 insertions(+), 112 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 99a52d6d6..9d108c6d7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,11 @@ { "search.exclude": { "vendor/**": true - } + }, + "vim.easymotion": true, + "vim.handleKeys": { + + "": true + }, + "vim.leader": "Space" } \ No newline at end of file diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index b0279b7f4..834226644 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -33,18 +33,6 @@ type BrowseOptions struct { BranchFlag bool } -type exitCode int - -const ( - exitUrlSuccess exitCode = 0 - exitNonUrlSuccess exitCode = 1 - exitNotInRepo exitCode = 2 - exitTooManyFlags exitCode = 3 - exitTooManyArgs exitCode = 4 - exitExpectedArg exitCode = 5 - exitInvalidCombo exitCode = 6 -) - func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { opts := &BrowseOptions{ @@ -108,84 +96,105 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command { } func openInBrowser(cmd *cobra.Command, opts *BrowseOptions) error { - + help := "Use 'gh browse --help' for more information about browse\n" baseRepo, err := opts.BaseRepo() + if err != nil { + return fmt.Errorf("unable to determine base repository: %w\n%s", err, help) + } + httpClient, _ := opts.HttpClient() apiClient := api.NewClientFromHTTP(httpClient) branchName, _ := api.RepoDefaultBranch(apiClient, baseRepo) - if !inRepo(err) { - return printExit(exitNotInRepo, cmd, opts, "") - } - if getFlagAmount(cmd) > 1 { - return printExit(exitTooManyFlags, cmd, opts, "") + return fmt.Errorf("accepts 1 flag, %d flag(s) were recieved\n%s", getFlagAmount(cmd), help) } repoUrl := ghrepo.GenerateRepoURL(baseRepo, "") - response := exitUrlSuccess if !hasArg(opts) && hasFlag(cmd) { - response, repoUrl = addFlag(opts, repoUrl) + err, repoUrl = addFlag(opts, repoUrl) + if err != nil { + return err + } } else if hasArg(opts) && !hasFlag(cmd) { - response, repoUrl = addArg(opts, repoUrl, branchName) + err, repoUrl = addArg(opts, repoUrl, branchName) + if err != nil { + return err + } } else if hasArg(opts) && hasFlag(cmd) { - response, repoUrl = addCombined(opts, repoUrl, branchName) + err, repoUrl = addCombined(opts, repoUrl, branchName) + if err != nil { + return err + } } - if response == exitUrlSuccess || response == exitNonUrlSuccess { - opts.Browser.Browse(repoUrl) + if repoUrl == ghrepo.GenerateRepoURL(baseRepo, "") { + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", repoUrl) } + opts.Browser.Browse(repoUrl) - return printExit(response, cmd, opts, repoUrl) + return nil } -func addCombined(opts *BrowseOptions, url string, branchName string) (exitCode, string) { +func addCombined(opts *BrowseOptions, url string, branchName string) (error, string) { + help := "Use 'gh browse --help' for more information about browse\n" if !opts.BranchFlag { - return exitInvalidCombo, "" + return fmt.Errorf("invalid use of flag and argument\n%s", help), "" } if opts.AdditionalArg == "" { - return exitUrlSuccess, url + "/tree/" + opts.SelectorArg + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } arr := parseFileArg(opts) if len(arr) > 1 { - return exitUrlSuccess, url + "/tree/" + opts.AdditionalArg + "/" + arr[0] + "#L" + arr[1] + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } - return exitUrlSuccess, url + "/tree/" + opts.AdditionalArg + "/" + arr[0] + url += "/tree/" + opts.SelectorArg + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } -func addFlag(opts *BrowseOptions, url string) (exitCode, string) { +func addFlag(opts *BrowseOptions, url string) (error, string) { if opts.ProjectsFlag { - return exitUrlSuccess, url + "/projects" + url += "/projects" } else if opts.SettingsFlag { - return exitUrlSuccess, url + "/settings" + url += "/settings" } else if opts.WikiFlag { - return exitUrlSuccess, url + "/wiki" + url += "/wiki" } - return exitExpectedArg, "" + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url } -func addArg(opts *BrowseOptions, url string, branchName string) (exitCode, string) { +func addArg(opts *BrowseOptions, url string, branchName string) (error, string) { + help := "Use 'gh browse --help' for more information about browse\n" if opts.AdditionalArg != "" { - return exitTooManyArgs, "" + return fmt.Errorf("accepts 1 arg, 2 arg(s) were recieved\n%s", help), "" } if isNumber(opts.SelectorArg) { url += "/issues/" + opts.SelectorArg - return exitNonUrlSuccess, url + fmt.Fprintf(opts.IO.Out, "now opening issue/pr in browser . . .\n") + return nil, url } arr := parseFileArg(opts) if len(arr) > 1 { - return exitUrlSuccess, url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1] } - return exitUrlSuccess, url + "/tree/" + branchName + "/" + arr[0] + fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url) + return nil, url + "/tree/" + branchName + "/" + arr[0] } func parseFileArg(opts *BrowseOptions) []string { @@ -193,32 +202,6 @@ func parseFileArg(opts *BrowseOptions) []string { return arr } -func printExit(exit exitCode, cmd *cobra.Command, opts *BrowseOptions, url string) error { - w := opts.IO.ErrOut - cs := opts.IO.ColorScheme() - help := "Use 'gh browse --help' for more information about browse\n" - - switch exit { - case exitUrlSuccess: - //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.Fprintln(w, "Hello World Two") - case exitNotInRepo: - return fmt.Errorf("change directory to a repository to open in browser\n%s", help) - case exitTooManyFlags: - return fmt.Errorf("accepts 1 flag, %d flag(s) were recieved\n%s", getFlagAmount(cmd), help) - case exitTooManyArgs: - return fmt.Errorf("accepts 1 arg, 2 arg(s) were received \n%s", help) - case exitExpectedArg: - return fmt.Errorf("expected argument with this flag %s\n%s", cs.Bold(url), help) - case exitInvalidCombo: - return fmt.Errorf("invalid use of flag and argument\n%s", help) - } - return nil -} - func hasFlag(cmd *cobra.Command) bool { return getFlagAmount(cmd) > 0 } @@ -231,10 +214,6 @@ func getFlagAmount(cmd *cobra.Command) int { return cmd.Flags().NFlag() } -func inRepo(err error) bool { - return err == nil -} - func isNumber(arg string) bool { _, err := strconv.Atoi(arg) if err != nil { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index abb9975bd..1862480b1 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -2,10 +2,11 @@ package browse import ( "bytes" - //"io/ioutil" + "io/ioutil" "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" @@ -26,6 +27,7 @@ type testCase struct { errorExpected bool stdoutExpected string stderrExpected string + urlExpected string } func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { @@ -39,8 +41,11 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { 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 + return t.args.repo, nil }, } @@ -53,8 +58,8 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { cmd.SetArgs(argv) cmd.SetIn(&bytes.Buffer{}) - cmd.SetOut(stdout) - cmd.SetErr(stderr) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) _, err = cmd.ExecuteC() return &test.CmdOut{ @@ -64,33 +69,55 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) { }, err } func TestNewCmdBrowse(t *testing.T) { - var tests = []testCase{ { - name: "test1", + name: "multiple flag", args: args{ - repo: ghrepo.New("bchadwic", "cli"), + repo: ghrepo.New("jessica", "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", + stderrExpected: "accepts 1 flag, 2 flag(s) were recieved\nUse 'gh browse --help' for more information about browse\n", + 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", }, - // { - // name: "test2", - // args: args{ - // repo: ghrepo.New("bchadwic", "cli"), - // cli: "--settings", - // }, - // errorExpected: false, - // stdoutExpected: "hello world", - // stderrExpected: "hello world", - // }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - http := &httpmock.Registry{} defer http.Verify(t) @@ -100,28 +127,12 @@ func TestNewCmdBrowse(t *testing.T) { output, err := runCommand(http, tt) if tt.errorExpected { - assert.Error(t, err) + assert.Equal(t, err.Error(), tt.stderrExpected) + } else { + assert.Equal(t, err, nil) } - - assert.Contains(t, output.OutBuf.String(), tt.stdoutExpected) // success outputs - - assert.Contains(t, output.ErrBuf.String(), tt.stderrExpected) // error outputs - + assert.Equal(t, output.OutBuf.String(), tt.stdoutExpected) + assert.Equal(t, tt.urlExpected, output.BrowsedURL) }) } } - -// http := initFakeHTTP() -// defer http.Verify(t) - -// _, cmdTeardown := run.Stub() -// defer cmdTeardown(t) - -// 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) -// } - -// 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) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 17b91ea81..ab3880e55 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(&repoResolvingCmdFactory)) // adds to the commands Commands() + cmd.AddCommand(browseCmd.NewCmdBrowse(&repoResolvingCmdFactory)) cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) cmd.AddCommand(releaseCmd.NewCmdRelease(&repoResolvingCmdFactory)) From 898d585e6a8d9d7be7ffc2da1e7a469d652880be Mon Sep 17 00:00:00 2001 From: ravocean Date: Mon, 7 Jun 2021 22:00:05 -0700 Subject: [PATCH 2/4] A small change in code --- pkg/cmd/browse/browse_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 1862480b1..d1c9442c3 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -114,6 +114,17 @@ func TestNewCmdBrowse(t *testing.T) { stderrExpected: "", urlExpected: "https://github.com/thanh/cli/wiki", }, + { + name: "branch flag", + args: args{ + repo: ghrepo.New("ken", "cli"), + cli: "--branch", + }, + errorExpected: false, + stdoutExpected: "", + stderrExpected: "Aa", + urlExpected: "", + }, } for _, tt := range tests { From 5efe9f8df3bb8c4a3892a4237a032442d7a67967 Mon Sep 17 00:00:00 2001 From: Benjamin Chadwick <73488828+bchadwic@users.noreply.github.com> Date: Tue, 8 Jun 2021 13:10:33 -0700 Subject: [PATCH 3/4] Update settings.json --- .vscode/settings.json | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 9d108c6d7..05ad8530a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,10 +2,4 @@ "search.exclude": { "vendor/**": true }, - "vim.easymotion": true, - "vim.handleKeys": { - - "": true - }, - "vim.leader": "Space" -} \ No newline at end of file +} From b0590541021a22bf3a0a90cc1dab1ffcd0690fca Mon Sep 17 00:00:00 2001 From: Benjamin Chadwick <73488828+bchadwic@users.noreply.github.com> Date: Tue, 8 Jun 2021 13:12:11 -0700 Subject: [PATCH 4/4] Update settings.json --- .vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 05ad8530a..6caa6e9f6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,5 @@ { "search.exclude": { "vendor/**": true - }, + } }