Merge remote-tracking branch 'origin/master' into appropriate-context-and-feedback

This commit is contained in:
Mislav Marohnić 2019-12-17 13:00:10 +01:00
commit 7b7c985e79
12 changed files with 85 additions and 75 deletions

View file

@ -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)
}
}

View file

@ -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)
}

View file

@ -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.

View file

@ -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)
}

View file

@ -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")

View file

@ -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)
}

View file

@ -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")
}

View file

@ -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")

View file

@ -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")

View file

@ -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

View file

@ -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
}

View file

@ -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)
}