From 79da79fb6845f7b603c48a905738b2078791b4b9 Mon Sep 17 00:00:00 2001 From: ravocean Date: Mon, 14 Jun 2021 21:40:52 -0700 Subject: [PATCH 1/2] 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/2] 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/", }, }