diff --git a/git/fixtures/.gitignore b/git/fixtures/.gitignore new file mode 100644 index 000000000..abae30d02 --- /dev/null +++ b/git/fixtures/.gitignore @@ -0,0 +1 @@ +*.git/COMMIT_EDITMSG diff --git a/git/fixtures/simple.git/HEAD b/git/fixtures/simple.git/HEAD new file mode 100644 index 000000000..b870d8262 --- /dev/null +++ b/git/fixtures/simple.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/main diff --git a/git/fixtures/simple.git/config b/git/fixtures/simple.git/config new file mode 100644 index 000000000..f0858dd73 --- /dev/null +++ b/git/fixtures/simple.git/config @@ -0,0 +1,9 @@ +[core] + repositoryformatversion = 0 + filemode = true + ;bare = true + ignorecase = true + precomposeunicode = true +[user] + name = Mona the Cat + email = monalisa@github.com diff --git a/git/fixtures/simple.git/index b/git/fixtures/simple.git/index new file mode 100644 index 000000000..65d675154 Binary files /dev/null and b/git/fixtures/simple.git/index differ diff --git a/git/fixtures/simple.git/logs/HEAD b/git/fixtures/simple.git/logs/HEAD new file mode 100644 index 000000000..216887f9e --- /dev/null +++ b/git/fixtures/simple.git/logs/HEAD @@ -0,0 +1,2 @@ +0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat 1614174263 +0100 commit (initial): Initial commit +d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat 1614174275 +0100 commit: Second commit diff --git a/git/fixtures/simple.git/logs/refs/heads/main b/git/fixtures/simple.git/logs/refs/heads/main new file mode 100644 index 000000000..216887f9e --- /dev/null +++ b/git/fixtures/simple.git/logs/refs/heads/main @@ -0,0 +1,2 @@ +0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat 1614174263 +0100 commit (initial): Initial commit +d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat 1614174275 +0100 commit: Second commit diff --git a/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 b/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 new file mode 100644 index 000000000..adf64119a Binary files /dev/null and b/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 differ diff --git a/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 b/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 new file mode 100644 index 000000000..8f2fded03 Binary files /dev/null and b/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 differ diff --git a/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f b/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f new file mode 100644 index 000000000..ec3ada617 --- /dev/null +++ b/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f @@ -0,0 +1,3 @@ +xNK +0tS ILc +"+@M뢷7"^@ bZxFhb轴;lKr3<33Kc#-"k8Z.2d=^*)ES&iqɏiϋP jAy 3*H/ \ No newline at end of file diff --git a/git/fixtures/simple.git/refs/heads/main b/git/fixtures/simple.git/refs/heads/main new file mode 100644 index 000000000..8316cdaf5 --- /dev/null +++ b/git/fixtures/simple.git/refs/heads/main @@ -0,0 +1 @@ +6f1a2405cace1633d89a79c74c65f22fe78f9659 diff --git a/git/git.go b/git/git.go index 684b00c10..abff866d6 100644 --- a/git/git.go +++ b/git/git.go @@ -180,43 +180,30 @@ func Commits(baseRef, headRef string) ([]*Commit, error) { return commits, nil } +func lookupCommit(sha, format string) ([]byte, error) { + logCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:"+format, sha) + if err != nil { + return nil, err + } + return run.PrepareCmd(logCmd).Output() +} + func LastCommit() (*Commit, error) { - logCmd, err := GitCommand("-c", "log.ShowSignature=false", "log", "--pretty=format:%H,%s", "-1") + output, err := lookupCommit("HEAD", "%H,%s") if err != nil { return nil, err } - output, err := run.PrepareCmd(logCmd).Output() - if err != nil { - return nil, err - } - - lines := outputLines(output) - if len(lines) != 1 { - return nil, ErrNotOnAnyBranch - } - - split := strings.SplitN(lines[0], ",", 2) - if len(split) != 2 { - return nil, ErrNotOnAnyBranch - } - + idx := bytes.IndexByte(output, ',') return &Commit{ - Sha: split[0], - Title: split[1], + Sha: string(output[0:idx]), + Title: strings.TrimSpace(string(output[idx+1:])), }, nil } func CommitBody(sha string) (string, error) { - showCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:%b", sha) - if err != nil { - return "", err - } - output, err := run.PrepareCmd(showCmd).Output() - if err != nil { - return "", err - } - return string(output), nil + output, err := lookupCommit(sha, "%b") + return string(output), err } // Push publishes a git ref to a remote and sets up upstream configuration diff --git a/git/git_test.go b/git/git_test.go index 899ca3b49..685b4d1c0 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -1,12 +1,53 @@ package git import ( + "os" "reflect" "testing" "github.com/cli/cli/internal/run" ) +func setGitDir(t *testing.T, dir string) { + // TODO: also set XDG_CONFIG_HOME, GIT_CONFIG_NOSYSTEM + old_GIT_DIR := os.Getenv("GIT_DIR") + os.Setenv("GIT_DIR", dir) + t.Cleanup(func() { + os.Setenv("GIT_DIR", old_GIT_DIR) + }) +} + +func TestLastCommit(t *testing.T) { + setGitDir(t, "./fixtures/simple.git") + c, err := LastCommit() + if err != nil { + t.Fatalf("LastCommit error: %v", err) + } + if c.Sha != "6f1a2405cace1633d89a79c74c65f22fe78f9659" { + t.Errorf("expected sha %q, got %q", "6f1a2405cace1633d89a79c74c65f22fe78f9659", c.Sha) + } + if c.Title != "Second commit" { + t.Errorf("expected title %q, got %q", "Second commit", c.Title) + } +} + +func TestCommitBody(t *testing.T) { + setGitDir(t, "./fixtures/simple.git") + body, err := CommitBody("6f1a2405cace1633d89a79c74c65f22fe78f9659") + if err != nil { + t.Fatalf("CommitBody error: %v", err) + } + if body != "I'm starting to get the hang of things\n" { + t.Errorf("expected %q, got %q", "I'm starting to get the hang of things\n", body) + } +} + +/* + NOTE: below this are stubbed git tests, i.e. those that do not actually invoke `git`. If possible, utilize + `setGitDir()` to allow new tests to interact with `git`. For write operations, you can use `t.TempDir()` to + host a temporary git repository that is safe to be changed. +*/ + func Test_UncommittedChangeCount(t *testing.T) { type c struct { Label string diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index a8fa4d683..27df4a104 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,7 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" "net/http" + "path/filepath" "sort" "strings" @@ -28,8 +30,9 @@ type EditOptions struct { Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Selector string - Filename string + Selector string + EditFilename string + AddFilename string } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -60,7 +63,9 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Select a file to edit") + + cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a new file to the gist") + cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd } @@ -85,6 +90,9 @@ func editRun(opts *EditOptions) error { gist, err := shared.GetGist(client, ghinstance.OverridableDefault(), gistID) if err != nil { + if errors.Is(err, shared.NotFoundErr) { + return fmt.Errorf("gist not found: %s", gistID) + } return err } @@ -97,10 +105,20 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("You do not own this gist.") } + if opts.AddFilename != "" { + files, err := getFilesToAdd(opts.AddFilename) + if err != nil { + return err + } + + gist.Files = files + return updateGist(apiClient, ghinstance.OverridableDefault(), gist) + } + filesToUpdate := map[string]string{} for { - filename := opts.Filename + filename := opts.EditFilename candidates := []string{} for filename := range gist.Files { candidates = append(candidates, filename) @@ -222,3 +240,22 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } + +func getFilesToAdd(file string) (map[string]*shared.GistFile, error) { + content, err := ioutil.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", file, err) + } + + if len(content) == 0 { + return nil, errors.New("file contents cannot be empty") + } + + filename := filepath.Base(file) + return map[string]*shared.GistFile{ + filename: { + Filename: filename, + Content: string(content), + }, + }, nil +} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index b9b711a38..73a53e523 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path/filepath" "testing" "github.com/cli/cli/internal/config" @@ -15,8 +16,26 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_getFilesToAdd(t *testing.T) { + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + + gf, err := getFilesToAdd(fileToAdd) + require.NoError(t, err) + + filename := filepath.Base(fileToAdd) + assert.Equal(t, map[string]*shared.GistFile{ + filename: { + Filename: filename, + Content: "hello", + }, + }, gf) +} + func TestNewCmdEdit(t *testing.T) { tests := []struct { name string @@ -34,8 +53,16 @@ func TestNewCmdEdit(t *testing.T) { name: "filename", cli: "123 --filename cool.md", wants: EditOptions{ - Selector: "123", - Filename: "cool.md", + Selector: "123", + EditFilename: "cool.md", + }, + }, + { + name: "add", + cli: "123 --add cool.md", + wants: EditOptions{ + Selector: "123", + AddFilename: "cool.md", }, }, } @@ -60,13 +87,18 @@ func TestNewCmdEdit(t *testing.T) { _, err = cmd.ExecuteC() assert.NoError(t, err) - assert.Equal(t, tt.wants.Filename, gotOpts.Filename) + assert.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename) + assert.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename) assert.Equal(t, tt.wants.Selector, gotOpts.Selector) }) } } func Test_editRun(t *testing.T) { + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + tests := []struct { name string opts *EditOptions @@ -74,13 +106,12 @@ func Test_editRun(t *testing.T) { httpStubs func(*httpmock.Registry) askStubs func(*prompt.AskStubber) nontty bool - wantErr bool - wantStderr string + wantErr string wantParams map[string]interface{} }{ { name: "no such gist", - wantErr: true, + wantErr: "gist not found: 1234", }, { name: "one file", @@ -163,7 +194,7 @@ func Test_editRun(t *testing.T) { as.StubOne("unix.md") as.StubOne("Cancel") }, - wantErr: true, + wantErr: "SilentError", gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ @@ -208,8 +239,28 @@ func Test_editRun(t *testing.T) { }, Owner: &shared.GistOwner{Login: "octocat2"}, }, - wantErr: true, - wantStderr: "You do not own this gist.", + wantErr: "You do not own this gist.", + }, + { + name: "add file to existing gist", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: fileToAdd, + }, }, } @@ -246,7 +297,7 @@ func Test_editRun(t *testing.T) { tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - io, _, _, _ := iostreams.Test() + io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(!tt.nontty) io.SetStdinTTY(!tt.nontty) tt.opts.IO = io @@ -260,11 +311,8 @@ func Test_editRun(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := editRun(tt.opts) reg.Verify(t) - if tt.wantErr { - assert.Error(t, err) - if tt.wantStderr != "" { - assert.EqualError(t, err, tt.wantStderr) - } + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) return } assert.NoError(t, err) @@ -278,6 +326,9 @@ func Test_editRun(t *testing.T) { } assert.Equal(t, tt.wantParams, reqBody) } + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) }) } } diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 36932a4e8..56bb3bb20 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -2,6 +2,7 @@ package shared import ( "context" + "errors" "fmt" "net/http" "net/url" @@ -36,6 +37,8 @@ type Gist struct { Owner *GistOwner `json:"owner,omitempty"` } +var NotFoundErr = errors.New("not found") + func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { gist := Gist{} path := fmt.Sprintf("gists/%s", gistID) @@ -43,6 +46,10 @@ func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { apiClient := api.NewClientFromHTTP(client) err := apiClient.REST(hostname, "GET", path, nil, &gist) if err != nil { + var httpErr api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + return nil, NotFoundErr + } return nil, err } diff --git a/pkg/cmd/pr/comment/comment.go b/pkg/cmd/pr/comment/comment.go index 27847b4fa..8e7c98cb1 100644 --- a/pkg/cmd/pr/comment/comment.go +++ b/pkg/cmd/pr/comment/comment.go @@ -30,6 +30,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err Example: heredoc.Doc(` $ gh pr comment 22 --body "This looks great, lets get it deployed." `), + Args: cobra.MaximumNArgs(1), PreRunE: func(cmd *cobra.Command, args []string) error { if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} diff --git a/pkg/cmd/pr/comment/comment_test.go b/pkg/cmd/pr/comment/comment_test.go index 52256d431..94c58a720 100644 --- a/pkg/cmd/pr/comment/comment_test.go +++ b/pkg/cmd/pr/comment/comment_test.go @@ -32,6 +32,12 @@ func TestNewCmdComment(t *testing.T) { }, wantsErr: false, }, + { + name: "two arguments", + input: "1 2", + output: shared.CommentableOptions{}, + wantsErr: true, + }, { name: "pr number", input: "1", diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 3fc1c8f10..7319b55b4 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -46,7 +46,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } cmd := &cobra.Command{ - Use: "edit { | }", + Use: "edit [ | | ]", Short: "Edit a pull request", Example: heredoc.Doc(` $ gh pr edit 23 --title "I found a bug" --body "Nothing works" @@ -56,12 +56,14 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman $ gh pr edit 23 --add-project "Roadmap" --remove-project v1,v2 $ gh pr edit 23 --milestone "Version 1" `), - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - opts.SelectorArg = args[0] + if len(args) > 0 { + opts.SelectorArg = args[0] + } flags := cmd.Flags() if flags.Changed("title") { diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index da328ba5b..ae1cc72c8 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -23,8 +23,17 @@ func TestNewCmdEdit(t *testing.T) { wantsErr bool }{ { - name: "no argument", - input: "", + name: "no argument", + input: "", + output: EditOptions{ + SelectorArg: "", + Interactive: true, + }, + wantsErr: false, + }, + { + name: "two arguments", + input: "1 2", output: EditOptions{}, wantsErr: true, }, diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 67498b726..c979a7f91 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -157,9 +157,8 @@ func mergeRun(opts *MergeOptions) error { return nil } - if opts.SelectorArg == "" { - localBranchLastCommit, err := git.LastCommit() - if err == nil { + if opts.SelectorArg == "" && len(pr.Commits.Nodes) > 0 { + if localBranchLastCommit, err := git.LastCommit(); err == nil { if localBranchLastCommit.Sha != pr.Commits.Nodes[0].Commit.Oid { fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) has diverged from local branch\n", cs.Yellow("!"), pr.Number, pr.Title) diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index dd4ff80cc..538f75056 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" @@ -336,7 +337,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") + cs.Register(`git .+ show .+ HEAD`, 1, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") @@ -347,8 +348,11 @@ func TestPrMerge_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries and switched to branch master + `), output.Stderr()) } func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { @@ -380,8 +384,11 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries + `), output.Stderr()) } func TestPrMerge_noPrNumberGiven(t *testing.T) { @@ -402,7 +409,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") + cs.Register(`git .+ show .+ HEAD`, 1, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") @@ -410,20 +417,33 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) } -func Test_divergingPullRequestWarning(t *testing.T) { +func Test_nonDivergingPullRequest(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) http.Register( httpmock.GraphQL(`query PullRequestForBranch\b`), - // FIXME: references fixture from another package - httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [{ + "commit": { + "oid": "COMMITSHA1" + } + }], + "totalCount": 1 + } + }] } } } }`)) http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { @@ -435,7 +455,7 @@ func Test_divergingPullRequestWarning(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "deadbeef,title") + cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA1,title") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") @@ -443,11 +463,95 @@ func Test_divergingPullRequestWarning(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`. Pull request #10 \(Blueberries are a good fruit\) has diverged from local branch\n. Merged pull request #10 \(Blueberries are a good fruit\)\n`) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) +} - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) +func Test_divergingPullRequestWarning(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [{ + "commit": { + "oid": "COMMITSHA1" + } + }], + "totalCount": 1 + } + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA2,title") + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) } + + assert.Equal(t, heredoc.Doc(` + ! Pull request #10 (Blueberries are a good fruit) has diverged from local branch + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) +} + +func Test_pullRequestWithoutCommits(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [], + "totalCount": 0 + } + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) } func TestPrMerge_rebase(t *testing.T) { @@ -517,8 +621,10 @@ func TestPrMerge_squash(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3") + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Squashed and merged pull request #3 (The title of the PR) + `), output.Stderr()) } func TestPrMerge_alreadyMerged(t *testing.T) { @@ -614,7 +720,6 @@ func TestPRMerge_interactive(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") as, surveyTeardown := prompt.InitAskStubber() @@ -643,6 +748,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { "headRefName": "blueberries", "headRepositoryOwner": {"login": "OWNER"}, "id": "THE-ID", + "title": "It was the best of times", "number": 3 }] } } } }`)) http.Register( @@ -667,7 +773,6 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") @@ -684,8 +789,11 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master") + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #3 (It was the best of times) + ✓ Deleted branch blueberries and switched to branch master + `), output.Stderr()) } func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) { @@ -777,7 +885,6 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") as, surveyTeardown := prompt.InitAskStubber() diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json new file mode 100644 index 000000000..18bb5eff8 --- /dev/null +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -0,0 +1,40 @@ +{ + "data": { + "repositoryOwner": { + "login": "octocat", + "repositories": { + "totalCount": 3, + "nodes": [ + { + "nameWithOwner": "octocat/hello-world", + "description": "My first repository", + "isFork": false, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:34:58Z" + }, + { + "nameWithOwner": "octocat/cli", + "description": "GitHub CLI", + "isFork": true, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:06:06Z" + }, + { + "nameWithOwner": "octocat/testing", + "description": null, + "isFork": false, + "isPrivate": true, + "isArchived": false, + "pushedAt": "2021-02-11T22:32:05Z" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" + } + } + } + } +} diff --git a/pkg/cmd/repo/list/fixtures/repoSearch.json b/pkg/cmd/repo/list/fixtures/repoSearch.json new file mode 100644 index 000000000..eecae8ac4 --- /dev/null +++ b/pkg/cmd/repo/list/fixtures/repoSearch.json @@ -0,0 +1,37 @@ +{ + "data": { + "search": { + "repositoryCount": 3, + "nodes": [ + { + "nameWithOwner": "octocat/hello-world", + "description": "My first repository", + "isFork": false, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:34:58Z" + }, + { + "nameWithOwner": "octocat/cli", + "description": "GitHub CLI", + "isFork": true, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:06:06Z" + }, + { + "nameWithOwner": "octocat/testing", + "description": null, + "isFork": false, + "isPrivate": true, + "isArchived": false, + "pushedAt": "2021-02-11T22:32:05Z" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" + } + } + } +} diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go new file mode 100644 index 000000000..7e2ccecd2 --- /dev/null +++ b/pkg/cmd/repo/list/http.go @@ -0,0 +1,235 @@ +package list + +import ( + "context" + "fmt" + "net/http" + "reflect" + "strings" + "time" + + "github.com/cli/cli/internal/ghinstance" + "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" +) + +type Repository struct { + NameWithOwner string + Description string + IsFork bool + IsPrivate bool + IsArchived bool + PushedAt time.Time +} + +func (r Repository) Info() string { + var tags []string + + if r.IsPrivate { + tags = append(tags, "private") + } else { + tags = append(tags, "public") + } + if r.IsFork { + tags = append(tags, "fork") + } + if r.IsArchived { + tags = append(tags, "archived") + } + + return strings.Join(tags, ", ") +} + +type RepositoryList struct { + Owner string + Repositories []Repository + TotalCount int + FromSearch bool +} + +type FilterOptions struct { + Visibility string // private, public + Fork bool + Source bool + Language string + Archived bool + NonArchived bool +} + +func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + if filter.Language != "" || filter.Archived || filter.NonArchived { + return searchRepos(client, hostname, limit, owner, filter) + } + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "perPage": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + } + + if filter.Visibility != "" { + variables["privacy"] = githubv4.RepositoryPrivacy(strings.ToUpper(filter.Visibility)) + } else { + variables["privacy"] = (*githubv4.RepositoryPrivacy)(nil) + } + + if filter.Fork { + variables["fork"] = githubv4.Boolean(true) + } else if filter.Source { + variables["fork"] = githubv4.Boolean(false) + } else { + variables["fork"] = (*githubv4.Boolean)(nil) + } + + var ownerConnection string + if owner == "" { + ownerConnection = `graphql:"repositoryOwner: viewer"` + } else { + ownerConnection = `graphql:"repositoryOwner(login: $owner)"` + variables["owner"] = githubv4.String(owner) + } + + type repositoryOwner struct { + Login string + Repositories struct { + Nodes []Repository + TotalCount int + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` + } + query := reflect.StructOf([]reflect.StructField{ + { + Name: "RepositoryOwner", + Type: reflect.TypeOf(repositoryOwner{}), + Tag: reflect.StructTag(ownerConnection), + }, + }) + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + listResult := RepositoryList{} +pagination: + for { + result := reflect.New(query) + err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) + if err != nil { + return nil, err + } + + owner := result.Elem().FieldByName("RepositoryOwner").Interface().(repositoryOwner) + listResult.TotalCount = owner.Repositories.TotalCount + listResult.Owner = owner.Login + + for _, repo := range owner.Repositories.Nodes { + listResult.Repositories = append(listResult.Repositories, repo) + if len(listResult.Repositories) >= limit { + break pagination + } + } + + if !owner.Repositories.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(owner.Repositories.PageInfo.EndCursor) + } + + return &listResult, nil +} + +func searchRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + type query struct { + Search struct { + RepositoryCount int + Nodes []struct { + Repository Repository `graphql:"...on Repository"` + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor)"` + } + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "query": githubv4.String(searchQuery(owner, filter)), + "perPage": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + listResult := RepositoryList{FromSearch: true} +pagination: + for { + var result query + err := gql.QueryNamed(context.Background(), "RepositoryListSearch", &result, variables) + if err != nil { + return nil, err + } + + listResult.TotalCount = result.Search.RepositoryCount + for _, node := range result.Search.Nodes { + if listResult.Owner == "" { + idx := strings.IndexRune(node.Repository.NameWithOwner, '/') + listResult.Owner = node.Repository.NameWithOwner[:idx] + } + listResult.Repositories = append(listResult.Repositories, node.Repository) + if len(listResult.Repositories) >= limit { + break pagination + } + } + + if !result.Search.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(result.Search.PageInfo.EndCursor) + } + + return &listResult, nil +} + +func searchQuery(owner string, filter FilterOptions) string { + queryParts := []string{"sort:updated-desc"} + if owner == "" { + queryParts = append(queryParts, "user:@me") + } else { + queryParts = append(queryParts, "user:"+owner) + } + + if filter.Fork { + queryParts = append(queryParts, "fork:only") + } else if filter.Source { + queryParts = append(queryParts, "fork:false") + } else { + queryParts = append(queryParts, "fork:true") + } + + if filter.Language != "" { + queryParts = append(queryParts, fmt.Sprintf("language:%q", filter.Language)) + } + + switch filter.Visibility { + case "public": + queryParts = append(queryParts, "is:public") + case "private": + queryParts = append(queryParts, "is:private") + } + + if filter.Archived { + queryParts = append(queryParts, "archived:true") + } else if filter.NonArchived { + queryParts = append(queryParts, "archived:false") + } + + return strings.Join(queryParts, " ") +} diff --git a/pkg/cmd/repo/list/http_test.go b/pkg/cmd/repo/list/http_test.go new file mode 100644 index 000000000..0544a750b --- /dev/null +++ b/pkg/cmd/repo/list/http_test.go @@ -0,0 +1,162 @@ +package list + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "os" + "testing" + + "github.com/cli/cli/pkg/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_listReposWithLanguage(t *testing.T) { + reg := httpmock.Registry{} + defer reg.Verify(t) + + var searchData struct { + Query string + Variables map[string]interface{} + } + reg.Register( + httpmock.GraphQL(`query RepositoryListSearch\b`), + func(req *http.Request) (*http.Response, error) { + jsonData, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + err = json.Unmarshal(jsonData, &searchData) + if err != nil { + return nil, err + } + + respBody, err := os.Open("./fixtures/repoSearch.json") + if err != nil { + return nil, err + } + + return &http.Response{ + StatusCode: 200, + Request: req, + Body: respBody, + }, nil + }, + ) + + client := http.Client{Transport: ®} + res, err := listRepos(&client, "github.com", 10, "", FilterOptions{ + Language: "go", + }) + require.NoError(t, err) + + assert.Equal(t, 3, res.TotalCount) + assert.Equal(t, true, res.FromSearch) + assert.Equal(t, "octocat", res.Owner) + assert.Equal(t, "octocat/hello-world", res.Repositories[0].NameWithOwner) + + assert.Equal(t, float64(10), searchData.Variables["perPage"]) + assert.Equal(t, `sort:updated-desc user:@me fork:true language:"go"`, searchData.Variables["query"]) +} + +func Test_searchQuery(t *testing.T) { + type args struct { + owner string + filter FilterOptions + } + tests := []struct { + name string + args args + want string + }{ + { + name: "blank", + want: "sort:updated-desc user:@me fork:true", + }, + { + name: "in org", + args: args{ + owner: "cli", + }, + want: "sort:updated-desc user:cli fork:true", + }, + { + name: "only public", + args: args{ + owner: "", + filter: FilterOptions{ + Visibility: "public", + }, + }, + want: "sort:updated-desc user:@me fork:true is:public", + }, + { + name: "only private", + args: args{ + owner: "", + filter: FilterOptions{ + Visibility: "private", + }, + }, + want: "sort:updated-desc user:@me fork:true is:private", + }, + { + name: "only forks", + args: args{ + owner: "", + filter: FilterOptions{ + Fork: true, + }, + }, + want: "sort:updated-desc user:@me fork:only", + }, + { + name: "no forks", + args: args{ + owner: "", + filter: FilterOptions{ + Source: true, + }, + }, + want: "sort:updated-desc user:@me fork:false", + }, + { + name: "with language", + args: args{ + owner: "", + filter: FilterOptions{ + Language: "ruby", + }, + }, + want: "sort:updated-desc user:@me fork:true language:\"ruby\"", + }, + { + name: "only archived", + args: args{ + owner: "", + filter: FilterOptions{ + Archived: true, + }, + }, + want: "sort:updated-desc user:@me fork:true archived:true", + }, + { + name: "only non-archived", + args: args{ + owner: "", + filter: FilterOptions{ + NonArchived: true, + }, + }, + want: "sort:updated-desc user:@me fork:true archived:false", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := searchQuery(tt.args.owner, tt.args.filter); got != tt.want { + t.Errorf("searchQuery() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go new file mode 100644 index 000000000..0b0b5fd5e --- /dev/null +++ b/pkg/cmd/repo/list/list.go @@ -0,0 +1,169 @@ +package list + +import ( + "fmt" + "net/http" + "time" + + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ListOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + + Limit int + Owner string + + Visibility string + Fork bool + Source bool + Language string + Archived bool + NonArchived bool + + Now func() time.Time +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Now: time.Now, + } + + var ( + flagPublic bool + flagPrivate bool + ) + + cmd := &cobra.Command{ + Use: "list []", + Args: cobra.MaximumNArgs(1), + Short: "List repositories owned by user or organization", + RunE: func(c *cobra.Command, args []string) error { + if opts.Limit < 1 { + return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)} + } + + if flagPrivate && flagPublic { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--public` or `--private`")} + } + if opts.Source && opts.Fork { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--source` or `--fork`")} + } + if opts.Archived && opts.NonArchived { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--archived` or `--no-archived`")} + } + + if flagPrivate { + opts.Visibility = "private" + } else if flagPublic { + opts.Visibility = "public" + } + + if len(args) > 0 { + opts.Owner = args[0] + } + + if runF != nil { + return runF(&opts) + } + return listRun(&opts) + }, + } + + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of repositories to list") + cmd.Flags().BoolVar(&flagPrivate, "private", false, "Show only private repositories") + cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public repositories") + cmd.Flags().BoolVar(&opts.Source, "source", false, "Show only non-forks") + cmd.Flags().BoolVar(&opts.Fork, "fork", false, "Show only forks") + cmd.Flags().StringVarP(&opts.Language, "language", "l", "", "Filter by primary coding language") + cmd.Flags().BoolVar(&opts.Archived, "archived", false, "Show only archived repositories") + cmd.Flags().BoolVar(&opts.NonArchived, "no-archived", false, "Omit archived repositories") + + return cmd +} + +func listRun(opts *ListOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + filter := FilterOptions{ + Visibility: opts.Visibility, + Fork: opts.Fork, + Source: opts.Source, + Language: opts.Language, + Archived: opts.Archived, + NonArchived: opts.NonArchived, + } + + listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, opts.Owner, filter) + if err != nil { + return err + } + + if err := opts.IO.StartPager(); err != nil { + fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) + } + defer opts.IO.StopPager() + + cs := opts.IO.ColorScheme() + tp := utils.NewTablePrinter(opts.IO) + now := opts.Now() + + for _, repo := range listResult.Repositories { + info := repo.Info() + infoColor := cs.Gray + if repo.IsPrivate { + infoColor = cs.Yellow + } + + t := repo.PushedAt + // if listResult.FromSearch { + // t = repo.UpdatedAt + // } + + tp.AddField(repo.NameWithOwner, nil, cs.Bold) + tp.AddField(text.ReplaceExcessiveWhitespace(repo.Description), nil, nil) + tp.AddField(info, nil, infoColor) + if tp.IsTTY() { + tp.AddField(utils.FuzzyAgoAbbr(now, t), nil, cs.Gray) + } else { + tp.AddField(t.Format(time.RFC3339), nil, nil) + } + tp.EndRow() + } + + if opts.IO.IsStdoutTTY() { + hasFilters := filter.Visibility != "" || filter.Fork || filter.Source || filter.Language != "" + title := listHeader(listResult.Owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) + } + + return tp.Render() +} + +func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) string { + if totalMatchCount == 0 { + if hasFilters { + return "No results match your search" + } else if owner != "" { + return "There are no repositories in @" + owner + } + return "No results" + } + + var matchStr string + if hasFilters { + matchStr = " that match your search" + } + return fmt.Sprintf("Showing %d of %d repositories in @%s%s", matchCount, totalMatchCount, owner, matchStr) +} diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go new file mode 100644 index 000000000..59552beda --- /dev/null +++ b/pkg/cmd/repo/list/list_test.go @@ -0,0 +1,359 @@ +package list + +import ( + "bytes" + "io/ioutil" + "net/http" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + cli string + wants ListOptions + wantsErr string + }{ + { + name: "no arguments", + cli: "", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "with owner", + cli: "monalisa", + wants: ListOptions{ + Limit: 30, + Owner: "monalisa", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "with limit", + cli: "-L 101", + wants: ListOptions{ + Limit: 101, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only public", + cli: "--public", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "public", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only private", + cli: "--private", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "private", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only forks", + cli: "--fork", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: true, + Source: false, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only sources", + cli: "--source", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: true, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "with language", + cli: "-l go", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "go", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only archived", + cli: "--archived", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: true, + NonArchived: false, + }, + }, + { + name: "only non-archived", + cli: "--no-archived", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: true, + }, + }, + { + name: "no public and private", + cli: "--public --private", + wantsErr: "specify only one of `--public` or `--private`", + }, + { + name: "no forks with sources", + cli: "--fork --source", + wantsErr: "specify only one of `--source` or `--fork`", + }, + { + name: "conflicting archived", + cli: "--archived --no-archived", + wantsErr: "specify only one of `--archived` or `--no-archived`", + }, + { + name: "too many arguments", + cli: "monalisa hubot", + wantsErr: "accepts at most 1 arg(s), received 2", + }, + { + name: "invalid limit", + cli: "-L 0", + wantsErr: "invalid limit: 0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *ListOptions + cmd := NewCmdList(f, func(opts *ListOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr != "" { + assert.EqualError(t, err, tt.wantsErr) + return + } + require.NoError(t, err) + + assert.Equal(t, tt.wants.Limit, gotOpts.Limit) + assert.Equal(t, tt.wants.Owner, gotOpts.Owner) + assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) + assert.Equal(t, tt.wants.Fork, gotOpts.Fork) + assert.Equal(t, tt.wants.Source, gotOpts.Source) + assert.Equal(t, tt.wants.Archived, gotOpts.Archived) + assert.Equal(t, tt.wants.NonArchived, gotOpts.NonArchived) + }) + } +} + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + } + + cmd := NewCmdList(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestRepoList_nontty(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(false) + io.SetStdinTTY(false) + io.SetStderrTTY(false) + + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query RepositoryList\b`), + httpmock.FileResponse("./fixtures/repoList.json"), + ) + + opts := ListOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + }, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC") + return t + }, + Limit: 30, + } + + err := listRun(&opts) + assert.NoError(t, err) + + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, heredoc.Doc(` + octocat/hello-world My first repository public 2021-02-19T06:34:58Z + octocat/cli GitHub CLI public, fork 2021-02-19T06:06:06Z + octocat/testing private 2021-02-11T22:32:05Z + `), stdout.String()) +} + +func TestRepoList_tty(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query RepositoryList\b`), + httpmock.FileResponse("./fixtures/repoList.json"), + ) + + opts := ListOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + }, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC") + return t + }, + Limit: 30, + } + + err := listRun(&opts) + assert.NoError(t, err) + + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, heredoc.Doc(` + + Showing 3 of 3 repositories in @octocat + + octocat/hello-world My first repository public 8h + octocat/cli GitHub CLI public, fork 8h + octocat/testing private 7d + `), stdout.String()) +} + +func TestRepoList_filtering(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query RepositoryList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "PRIVATE", params["privacy"]) + assert.Equal(t, float64(2), params["perPage"]) + }), + ) + + output, err := runCommand(http, true, `--private --limit 2 `) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "", output.Stderr()) + assert.Equal(t, "\nNo results match your search\n\n", output.String()) +} diff --git a/pkg/cmd/repo/repo.go b/pkg/cmd/repo/repo.go index 02e6c368b..4fee2b19c 100644 --- a/pkg/cmd/repo/repo.go +++ b/pkg/cmd/repo/repo.go @@ -7,6 +7,7 @@ import ( creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" repoForkCmd "github.com/cli/cli/pkg/cmd/repo/fork" gardenCmd "github.com/cli/cli/pkg/cmd/repo/garden" + repoListCmd "github.com/cli/cli/pkg/cmd/repo/list" repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" @@ -36,6 +37,7 @@ func NewCmdRepo(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(repoForkCmd.NewCmdFork(f, nil)) cmd.AddCommand(repoCloneCmd.NewCmdClone(f, nil)) cmd.AddCommand(repoCreateCmd.NewCmdCreate(f, nil)) + cmd.AddCommand(repoListCmd.NewCmdList(f, nil)) cmd.AddCommand(creditsCmd.NewCmdRepoCredits(f, nil)) cmd.AddCommand(gardenCmd.NewCmdGarden(f, nil))