diff --git a/command/completion_test.go b/command/completion_test.go index 5ea8e8ed8..e66a00124 100644 --- a/command/completion_test.go +++ b/command/completion_test.go @@ -6,24 +6,24 @@ import ( ) func TestCompletion_bash(t *testing.T) { - outStr, err := RunCommand(completionCmd, `completion`) + output, err := RunCommand(completionCmd, `completion`) if err != nil { t.Fatal(err) } - if !strings.Contains(outStr, "complete -o default -F __start_gh gh") { - t.Errorf("problem in bash completion:\n%s", outStr) + if !strings.Contains(output.String(), "complete -o default -F __start_gh gh") { + t.Errorf("problem in bash completion:\n%s", output) } } func TestCompletion_zsh(t *testing.T) { - outStr, err := RunCommand(completionCmd, `completion -s zsh`) + output, err := RunCommand(completionCmd, `completion -s zsh`) if err != nil { t.Fatal(err) } - if !strings.Contains(outStr, "#compdef _gh gh") { - t.Errorf("problem in zsh completion:\n%s", outStr) + if !strings.Contains(output.String(), "#compdef _gh gh") { + t.Errorf("problem in zsh completion:\n%s", output) } } diff --git a/command/issue.go b/command/issue.go index 3bfe603c8..fcbbcd144 100644 --- a/command/issue.go +++ b/command/issue.go @@ -216,7 +216,7 @@ func issueView(cmd *cobra.Command, args []string) error { } openURL := issue.URL - cmd.Printf("Opening %s in your browser.\n", openURL) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } diff --git a/command/issue_test.go b/command/issue_test.go index 432dd8ba3..dbb41e8a7 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -33,7 +33,7 @@ func TestIssueStatus(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -67,7 +67,7 @@ Issues opened by you There are no issues opened by you ` - if output != expectedOutput { + if output.String() != expectedOutput { t.Errorf("expected %q, got %q", expectedOutput, output) } } @@ -92,7 +92,7 @@ func TestIssueList(t *testing.T) { } for _, r := range expectedIssues { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) return } @@ -104,11 +104,14 @@ func TestIssueList_withFlags(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response - _, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") + output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open") if err != nil { t.Errorf("error running command `issue list`: %v", err) } + eq(t, output.String(), "") + eq(t, output.Stderr(), "No issues match your search\n") + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -147,9 +150,8 @@ func TestIssueView(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -208,9 +210,7 @@ func TestIssueView_urlArg(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -255,7 +255,7 @@ func TestIssueCreate(t *testing.T) { eq(t, reqBody.Variables.Input.Title, "hello") eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - eq(t, output, ` + eq(t, output.String(), ` Creating issue in OWNER/REPO https://github.com/OWNER/REPO/issues/12 @@ -283,7 +283,7 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output, ` + eq(t, output.String(), ` Creating issue in OWNER/REPO Opening https://github.com/OWNER/REPO/issues/new in your browser. diff --git a/command/pr.go b/command/pr.go index 6a462231b..158c873ec 100644 --- a/command/pr.go +++ b/command/pr.go @@ -282,7 +282,7 @@ func prView(cmd *cobra.Command, args []string) error { } } - cmd.Printf("Opening %s in your browser.\n", openURL) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index b3b991c39..93eaf81bb 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -53,7 +53,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") @@ -105,7 +105,7 @@ func TestPRCheckout_urlArg(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OWNER/REPO/pull/123/files`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") @@ -154,7 +154,7 @@ func TestPRCheckout_branchArg(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout hubot:feature`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 5) eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") @@ -203,7 +203,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin +refs/heads/feature:refs/remotes/origin/feature") @@ -255,7 +255,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch robot-fork +refs/heads/feature:refs/remotes/robot-fork/feature") @@ -307,7 +307,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") @@ -359,7 +359,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 2) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") @@ -409,7 +409,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 2) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head") @@ -459,7 +459,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { output, err := RunCommand(prCheckoutCmd, `pr checkout 123`) eq(t, err, nil) - eq(t, output, "") + eq(t, output.String(), "") eq(t, len(ranCommands), 4) eq(t, strings.Join(ranCommands[0], " "), "git fetch origin refs/pull/123/head:feature") diff --git a/command/pr_create.go b/command/pr_create.go index dbfde093d..132cb3244 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -27,7 +27,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { noun = noun + "s" } - cmd.Printf("Warning: %d uncommitted %s\n", ucc, noun) + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %d uncommitted %s\n", ucc, noun) } repo, err := ctx.BaseRepo() @@ -57,7 +57,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { } if isWeb { openURL := fmt.Sprintf(`https://github.com/%s/%s/pull/%s`, repo.RepoOwner(), repo.RepoName(), head) - cmd.Printf("Opening %s in your browser.\n", openURL) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index a1cdc7b94..7bbd249fa 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -91,7 +91,7 @@ func TestPRCreate(t *testing.T) { eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output, ` + eq(t, output.String(), ` Creating pull request for feature in OWNER/REPO https://github.com/OWNER/REPO/pull/12 @@ -119,9 +119,8 @@ func TestPRCreate_web(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create --web`) eq(t, err, nil) - if output == "" { - t.Fatal("expected output") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature") @@ -159,10 +158,10 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - eq(t, output, `Warning: 1 uncommitted change - + eq(t, output.String(), ` Creating pull request for feature in OWNER/REPO https://github.com/OWNER/REPO/pull/12 `) + eq(t, output.Stderr(), "Warning: 1 uncommitted change\n") } diff --git a/command/pr_test.go b/command/pr_test.go index f8bdf7283..44d00d34f 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -24,16 +24,30 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -func RunCommand(cmd *cobra.Command, args string) (string, error) { +type cmdOut struct { + outBuf, errBuf *bytes.Buffer +} + +func (c cmdOut) String() string { + return c.outBuf.String() +} + +func (c cmdOut) Stderr() string { + return c.errBuf.String() +} + +func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { rootCmd := cmd.Root() argv, err := shlex.Split(args) if err != nil { - return "", err + return nil, err } rootCmd.SetArgs(argv) outBuf := bytes.Buffer{} cmd.SetOut(&outBuf) + errBuf := bytes.Buffer{} + cmd.SetErr(&errBuf) // Reset flag values so they don't leak between tests cmd.Flags().VisitAll(func(f *pflag.Flag) { @@ -48,7 +62,10 @@ func RunCommand(cmd *cobra.Command, args string) (string, error) { }) _, err = rootCmd.ExecuteC() - return outBuf.String(), err + cmd.SetOut(nil) + cmd.SetErr(nil) + + return &cmdOut{&outBuf, &errBuf}, err } func TestPRStatus(t *testing.T) { @@ -72,7 +89,7 @@ func TestPRStatus(t *testing.T) { } for _, r := range expectedPrs { - if !r.MatchString(output) { + if !r.MatchString(output.String()) { t.Errorf("output did not match regexp /%s/", r) } } @@ -91,7 +108,7 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - eq(t, output, ` + eq(t, output.String(), ` Pull requests for OWNER/REPO 32 New feature feature @@ -107,11 +124,14 @@ func TestPRList_filtering(t *testing.T) { respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) - _, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) + output, err := RunCommand(prListCmd, `pr list -s all -l one,two -l three`) if err != nil { t.Fatal(err) } + eq(t, output.String(), "") + eq(t, output.Stderr(), "No pull requests match your search\n") + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { @@ -186,9 +206,8 @@ func TestPRView_currentBranch(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -251,9 +270,7 @@ func TestPRView_numberArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -284,9 +301,7 @@ func TestPRView_urlArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -319,9 +334,7 @@ func TestPRView_branchArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") @@ -355,9 +368,7 @@ func TestPRView_branchWithOwnerArg(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - if output == "" { - t.Errorf("command output expected got an empty string") - } + eq(t, output.String(), "") if seenCmd == nil { t.Fatal("expected a command to run") diff --git a/command/title_body_survey.go b/command/title_body_survey.go index af4dc34e1..450742d7a 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -92,7 +92,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle string, providedBody stri case _unconfirmed: continue case _cancel: - cmd.Println("Discarding.") + fmt.Fprintln(cmd.ErrOrStderr(), "Discarding.") return nil, nil default: panic("reached unreachable case") diff --git a/context/config_file.go b/context/config_file.go index 5236930a1..6c053286d 100644 --- a/context/config_file.go +++ b/context/config_file.go @@ -8,7 +8,6 @@ import ( "os" "path" - "github.com/mitchellh/go-homedir" "gopkg.in/yaml.v3" ) @@ -36,11 +35,9 @@ func parseConfigFile(fn string) (*configEntry, error) { return parseConfig(f) } -// ParseDefaultConfig reads the configuration from ~/.config/gh +// ParseDefaultConfig reads the configuration file func ParseDefaultConfig() (*configEntry, error) { - // FIXME: this duplicates fsContext.configFile - fn, _ := homedir.Expand("~/.config/gh") - return parseConfigFile(fn) + return parseConfigFile(configFile()) } func parseConfig(r io.Reader) (*configEntry, error) { @@ -72,10 +69,10 @@ func parseConfig(r io.Reader) (*configEntry, error) { // This is a temporary function that will migrate the config file. It can be removed // in January. // -// If ~/.config/gh is a file, convert it to a directory and place the file -// into ~/.config/gh/config.yml +// If the config dir is a file, convert it to a directory and place the file +// into a file named config.yml func migrateConfigFile() { - p, _ := homedir.Expand("~/.config/gh") + p := ConfigDir() fi, err := os.Stat(p) if err != nil { // This means the file doesn't exist, and that is fine. return diff --git a/context/context.go b/context/context.go index 20a921ffc..e5d74634c 100644 --- a/context/context.go +++ b/context/context.go @@ -1,6 +1,7 @@ package context import ( + "path" "strings" "github.com/github/gh-cli/git" @@ -39,14 +40,18 @@ type fsContext struct { authToken string } -func (c *fsContext) configFile() string { - dir, _ := homedir.Expand("~/.config/gh/config.yml") +func ConfigDir() string { + dir, _ := homedir.Expand("~/.config/gh") return dir } +func configFile() string { + return path.Join(ConfigDir(), "config.yml") +} + func (c *fsContext) getConfig() (*configEntry, error) { if c.config == nil { - entry, err := parseOrSetupConfigFile(c.configFile()) + entry, err := parseOrSetupConfigFile(configFile()) if err != nil { return nil, err } diff --git a/main.go b/main.go index f09edb18d..726b50784 100644 --- a/main.go +++ b/main.go @@ -3,14 +3,15 @@ package main import ( "fmt" "os" + "path" "strings" "github.com/github/gh-cli/command" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/update" "github.com/github/gh-cli/utils" "github.com/mattn/go-isatty" "github.com/mgutz/ansi" - "github.com/mitchellh/go-homedir" ) var updaterEnabled = "" @@ -61,9 +62,6 @@ func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { } repo := updaterEnabled - stateFilePath, err := homedir.Expand("~/.config/gh/state.yml") - if err != nil { - return nil, err - } + stateFilePath := path.Join(context.ConfigDir(), "state.yml") return update.CheckForUpdate(client, stateFilePath, repo, currentVersion) }