From 79da79fb6845f7b603c48a905738b2078791b4b9 Mon Sep 17 00:00:00 2001 From: ravocean Date: Mon, 14 Jun 2021 21:40:52 -0700 Subject: [PATCH 1/3] We added new tests to bring the coverage to 90%+ --- pkg/cmd/browse/browse.go | 97 ++++++++++++++++------------------- pkg/cmd/browse/browse_test.go | 38 +++++++++++--- 2 files changed, 76 insertions(+), 59 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 63435e743..6a92d945b 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -78,22 +78,13 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co opts.SelectorArg = args[0] } - if err := cmdutil.MutuallyExclusive("cannot use --projects with --settings", opts.ProjectsFlag, opts.SettingsFlag); err != nil { - return err - } - if err := cmdutil.MutuallyExclusive("cannot use --projects with --wiki", opts.ProjectsFlag, opts.WikiFlag); err != nil { - return err - } - if err := cmdutil.MutuallyExclusive("cannot use --projects with --branch", opts.ProjectsFlag, opts.Branch != ""); err != nil { - return err - } - if err := cmdutil.MutuallyExclusive("cannot use --settings with --wiki", opts.SettingsFlag, opts.WikiFlag); err != nil { - return err - } - if err := cmdutil.MutuallyExclusive("cannot use --settings with --branch", opts.SettingsFlag, opts.Branch != ""); err != nil { - return err - } - if err := cmdutil.MutuallyExclusive("cannot use --wiki with --branch", opts.WikiFlag, opts.Branch != ""); err != nil { + if err := cmdutil.MutuallyExclusive( + "specify only one of `--branch`, `--projects`, `--wiki`, or `--settings`", + opts.Branch != "", + opts.WikiFlag, + opts.SettingsFlag, + opts.ProjectsFlag, + ); err != nil { return err } @@ -127,54 +118,54 @@ func runBrowse(opts *BrowseOptions) error { url := ghrepo.GenerateRepoURL(baseRepo, "") - if opts.ProjectsFlag { - err := opts.Browser.Browse(url + "/projects") - return err - } + if opts.SelectorArg == "" { - if opts.SettingsFlag { - err := opts.Browser.Browse(url + "/settings") - return err - } + if opts.ProjectsFlag { + url += "/projects" + } - if opts.WikiFlag { - err := opts.Browser.Browse(url + "/wiki") - return err - } + if opts.SettingsFlag { + url += "/settings" + } - if isNumber(opts.SelectorArg) { - url += "/issues/" + opts.SelectorArg - err := opts.Browser.Browse(url) - return err - } - - if opts.Branch != "" { - url += "/tree/" + opts.Branch + "/" + if opts.WikiFlag { + url += "/wiki" + } + if opts.Branch != "" { + url += "/tree/" + opts.Branch + "/" + } } else { - apiClient := api.NewClientFromHTTP(httpClient) - branchName, err := api.RepoDefaultBranch(apiClient, baseRepo) - if err != nil { - return err - } - url += "/tree/" + branchName + "/" - } - - if opts.SelectorArg != "" { - arr, err := parseFileArg(opts.SelectorArg) - if err != nil { - return err - } - if len(arr) > 1 { - url += arr[0] + "#L" + arr[1] + if isNumber(opts.SelectorArg) { + url += "/issues/" + opts.SelectorArg } else { - url += arr[0] + arr, err := parseFileArg(opts.SelectorArg) + if err != nil { + return err + } + if opts.Branch != "" { + url += "/tree/" + opts.Branch + "/" + } else { + apiClient := api.NewClientFromHTTP(httpClient) + branchName, err := api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + url += "/tree/" + branchName + "/" + } + if opts.SelectorArg != "" { + if len(arr) > 1 { + url += arr[0] + "#L" + arr[1] + } else { + url += arr[0] + } + } } } - err = opts.Browser.Browse(url) if opts.IO.IsStdoutTTY() && err == nil { fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url) } + return err } func parseFileArg(fileArg string) ([]string, error) { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 35694158e..123f1eea2 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -138,9 +138,8 @@ func Test_runBrowse(t *testing.T) { opts: BrowseOptions{ SelectorArg: "", }, - baseRepo: ghrepo.New("jessica", "cli"), - defaultBranch: "trunk", - expectedURL: "https://github.com/jessica/cli/tree/trunk/", + baseRepo: ghrepo.New("jessica", "cli"), + expectedURL: "https://github.com/jessica/cli", }, { name: "file argument", @@ -157,6 +156,15 @@ func Test_runBrowse(t *testing.T) { baseRepo: ghrepo.New("thanh", "cli"), expectedURL: "https://github.com/thanh/cli/tree/trunk/", }, + { + name: "branch flag with file", + opts: BrowseOptions{ + Branch: "trunk", + SelectorArg: "main.go", + }, + baseRepo: ghrepo.New("thanh", "cli"), + expectedURL: "https://github.com/thanh/cli/tree/trunk/main.go", + }, { name: "settings flag", opts: BrowseOptions{ @@ -203,9 +211,27 @@ func Test_runBrowse(t *testing.T) { opts: BrowseOptions{ SelectorArg: "path/to/file.txt:32:32", }, - baseRepo: ghrepo.New("bchadwic", "cli"), - defaultBranch: "trunk", - wantsErr: true, + baseRepo: ghrepo.New("bchadwic", "cli"), + wantsErr: true, + }, + { + name: "branch with issue number", + opts: BrowseOptions{ + SelectorArg: "217", + Branch: "trunk", + }, + baseRepo: ghrepo.New("bchadwic", "cli"), + wantsErr: false, + expectedURL: "https://github.com/bchadwic/cli/issues/217", + }, + { + name: "opening branch", + opts: BrowseOptions{ + Branch: "first-browse-pull", + }, + baseRepo: ghrepo.New("ravocean", "cli"), + wantsErr: false, + expectedURL: "https://github.com/ravocean/cli/tree/first-browse-pull/", }, { name: "file with line argument", From 21691c8d2e099ba172960287c2c1afeae7688338 Mon Sep 17 00:00:00 2001 From: ravocean Date: Mon, 14 Jun 2021 22:16:58 -0700 Subject: [PATCH 2/3] Reorganized tests --- pkg/cmd/browse/browse_test.go | 103 ++++++++++++++++------------------ 1 file changed, 47 insertions(+), 56 deletions(-) diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 123f1eea2..e9082b5a2 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -138,23 +138,55 @@ func Test_runBrowse(t *testing.T) { opts: BrowseOptions{ SelectorArg: "", }, - baseRepo: ghrepo.New("jessica", "cli"), - expectedURL: "https://github.com/jessica/cli", + baseRepo: ghrepo.New("jlsestak", "cli"), + expectedURL: "https://github.com/jlsestak/cli", + }, + { + name: "settings flag", + opts: BrowseOptions{ + SettingsFlag: true, + }, + baseRepo: ghrepo.New("bchadwic", "ObscuredByClouds"), + expectedURL: "https://github.com/bchadwic/ObscuredByClouds/settings", + }, + { + name: "projects flag", + opts: BrowseOptions{ + ProjectsFlag: true, + }, + baseRepo: ghrepo.New("ttran112", "7ate9"), + expectedURL: "https://github.com/ttran112/7ate9/projects", + }, + { + name: "wiki flag", + opts: BrowseOptions{ + WikiFlag: true, + }, + baseRepo: ghrepo.New("ravocean", "ThreatLevelMidnight"), + expectedURL: "https://github.com/ravocean/ThreatLevelMidnight/wiki", }, { name: "file argument", opts: BrowseOptions{SelectorArg: "path/to/file.txt"}, - baseRepo: ghrepo.New("ken", "cli"), + baseRepo: ghrepo.New("ken", "mrprofessor"), defaultBranch: "main", - expectedURL: "https://github.com/ken/cli/tree/main/path/to/file.txt", + expectedURL: "https://github.com/ken/mrprofessor/tree/main/path/to/file.txt", + }, + { + name: "issue argument", + opts: BrowseOptions{ + SelectorArg: "217", + }, + baseRepo: ghrepo.New("kevin", "MinTy"), + expectedURL: "https://github.com/kevin/MinTy/issues/217", }, { name: "branch flag", opts: BrowseOptions{ Branch: "trunk", }, - baseRepo: ghrepo.New("thanh", "cli"), - expectedURL: "https://github.com/thanh/cli/tree/trunk/", + baseRepo: ghrepo.New("jlsestak", "vegan"), + expectedURL: "https://github.com/jlsestak/vegan/tree/trunk/", }, { name: "branch flag with file", @@ -162,56 +194,24 @@ func Test_runBrowse(t *testing.T) { Branch: "trunk", SelectorArg: "main.go", }, - baseRepo: ghrepo.New("thanh", "cli"), - expectedURL: "https://github.com/thanh/cli/tree/trunk/main.go", - }, - { - name: "settings flag", - opts: BrowseOptions{ - SettingsFlag: true, - }, - baseRepo: ghrepo.New("bchadwic", "cli"), - expectedURL: "https://github.com/bchadwic/cli/settings", - }, - { - name: "projects flag", - opts: BrowseOptions{ - ProjectsFlag: true, - }, - baseRepo: ghrepo.New("bchadwic", "cli"), - expectedURL: "https://github.com/bchadwic/cli/projects", - }, - { - name: "wiki flag", - opts: BrowseOptions{ - WikiFlag: true, - }, - baseRepo: ghrepo.New("bchadwic", "cli"), - expectedURL: "https://github.com/bchadwic/cli/wiki", - }, - { - name: "issue argument", - opts: BrowseOptions{ - SelectorArg: "217", - }, - baseRepo: ghrepo.New("bchadwic", "cli"), - expectedURL: "https://github.com/bchadwic/cli/issues/217", + baseRepo: ghrepo.New("bchadwic", "LedZeppelinIV"), + expectedURL: "https://github.com/bchadwic/LedZeppelinIV/tree/trunk/main.go", }, { name: "file with line number", opts: BrowseOptions{ SelectorArg: "path/to/file.txt:32", }, - baseRepo: ghrepo.New("bchadwic", "cli"), + baseRepo: ghrepo.New("ravocean", "angur"), defaultBranch: "trunk", - expectedURL: "https://github.com/bchadwic/cli/tree/trunk/path/to/file.txt#L32", + expectedURL: "https://github.com/ravocean/angur/tree/trunk/path/to/file.txt#L32", }, { name: "file with invalid line number", opts: BrowseOptions{ SelectorArg: "path/to/file.txt:32:32", }, - baseRepo: ghrepo.New("bchadwic", "cli"), + baseRepo: ghrepo.New("ttran112", "ttrain211"), wantsErr: true, }, { @@ -220,27 +220,18 @@ func Test_runBrowse(t *testing.T) { SelectorArg: "217", Branch: "trunk", }, - baseRepo: ghrepo.New("bchadwic", "cli"), + baseRepo: ghrepo.New("ken", "grc"), wantsErr: false, - expectedURL: "https://github.com/bchadwic/cli/issues/217", + expectedURL: "https://github.com/ken/grc/issues/217", }, { name: "opening branch", opts: BrowseOptions{ Branch: "first-browse-pull", }, - baseRepo: ghrepo.New("ravocean", "cli"), + baseRepo: ghrepo.New("github", "ThankYouGitHub"), wantsErr: false, - expectedURL: "https://github.com/ravocean/cli/tree/first-browse-pull/", - }, - { - name: "file with line argument", - opts: BrowseOptions{ - SelectorArg: "path/to/file.txt:32", - }, - baseRepo: ghrepo.New("bchadwic", "cli"), - defaultBranch: "trunk", - expectedURL: "https://github.com/bchadwic/cli/tree/trunk/path/to/file.txt#L32", + expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/", }, } From 679d396c8d78d398db7699c98c2c322de6f23b9a Mon Sep 17 00:00:00 2001 From: bchadwic Date: Mon, 14 Jun 2021 22:31:29 -0700 Subject: [PATCH 3/3] reformatted spacing and final touches --- pkg/cmd/browse/browse.go | 8 ++------ pkg/cmd/browse/browse_test.go | 11 ++++++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 6a92d945b..c28ad2d5d 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -105,7 +105,6 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co } func runBrowse(opts *BrowseOptions) error { - baseRepo, err := opts.BaseRepo() if err != nil { return fmt.Errorf("unable to determine base repository: %w\nUse 'gh browse --help' for more information about browse\n", err) @@ -115,19 +114,15 @@ func runBrowse(opts *BrowseOptions) error { if err != nil { return fmt.Errorf("unable to create an http client: %w\nUse 'gh browse --help' for more information about browse\n", err) } - url := ghrepo.GenerateRepoURL(baseRepo, "") if opts.SelectorArg == "" { - if opts.ProjectsFlag { url += "/projects" } - if opts.SettingsFlag { url += "/settings" } - if opts.WikiFlag { url += "/wiki" } @@ -161,6 +156,7 @@ func runBrowse(opts *BrowseOptions) error { } } } + err = opts.Browser.Browse(url) if opts.IO.IsStdoutTTY() && err == nil { fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url) @@ -168,6 +164,7 @@ func runBrowse(opts *BrowseOptions) error { return err } + func parseFileArg(fileArg string) ([]string, error) { arr := strings.Split(fileArg, ":") if len(arr) > 2 { @@ -176,7 +173,6 @@ func parseFileArg(fileArg string) ([]string, error) { if len(arr) > 1 && !isNumber(arr[1]) { return arr, fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n") } - return arr, nil } diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index e9082b5a2..e9bea0dfc 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -185,8 +185,8 @@ func Test_runBrowse(t *testing.T) { opts: BrowseOptions{ Branch: "trunk", }, - baseRepo: ghrepo.New("jlsestak", "vegan"), - expectedURL: "https://github.com/jlsestak/vegan/tree/trunk/", + baseRepo: ghrepo.New("jlsestak", "CouldNotThinkOfARepoName"), + expectedURL: "https://github.com/jlsestak/CouldNotThinkOfARepoName/tree/trunk/", }, { name: "branch flag with file", @@ -225,13 +225,14 @@ func Test_runBrowse(t *testing.T) { expectedURL: "https://github.com/ken/grc/issues/217", }, { - name: "opening branch", + name: "opening branch file with line number", opts: BrowseOptions{ - Branch: "first-browse-pull", + Branch: "first-browse-pull", + SelectorArg: "browse.go:32", }, baseRepo: ghrepo.New("github", "ThankYouGitHub"), wantsErr: false, - expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/", + expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/browse.go#L32", }, }