From 969321bff23b388571696ec3865e4a7b1da16fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 19:09:16 +0200 Subject: [PATCH 1/5] :fire: cleanup in `context` --- context/blank_context.go | 24 ------------------------ context/context.go | 30 ------------------------------ 2 files changed, 54 deletions(-) delete mode 100644 context/blank_context.go diff --git a/context/blank_context.go b/context/blank_context.go deleted file mode 100644 index 96f599981..000000000 --- a/context/blank_context.go +++ /dev/null @@ -1,24 +0,0 @@ -package context - -import ( - "fmt" - - "github.com/cli/cli/internal/config" -) - -// NewBlank initializes a blank Context suitable for testing -func NewBlank() *blankContext { - return &blankContext{} -} - -// A Context implementation that queries the filesystem -type blankContext struct { -} - -func (c *blankContext) Config() (config.Config, error) { - cfg, err := config.ParseConfig("config.yml") - if err != nil { - panic(fmt.Sprintf("failed to parse config during tests. did you remember to stub? error: %s", err)) - } - return cfg, nil -} diff --git a/context/context.go b/context/context.go index 9b2fc47c1..29ea74995 100644 --- a/context/context.go +++ b/context/context.go @@ -3,20 +3,13 @@ package context import ( "errors" "fmt" - "os" "sort" "strings" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" ) -// Context represents the interface for querying information about the current environment -type Context interface { - Config() (config.Config, error) -} - // cap the number of git remotes looked up, since the user might have an // unusually large number of git remotes const maxRemotesForLookup = 5 @@ -150,26 +143,3 @@ func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { } return nil, errors.New("not found") } - -// New initializes a Context that reads from the filesystem -func New() Context { - return &fsContext{} -} - -// A Context implementation that queries the filesystem -type fsContext struct { - config config.Config -} - -func (c *fsContext) Config() (config.Config, error) { - if c.config == nil { - cfg, err := config.ParseDefaultConfig() - if errors.Is(err, os.ErrNotExist) { - cfg = config.NewBlankConfig() - } else if err != nil { - return nil, err - } - c.config = cfg - } - return c.config, nil -} From d534a94d1bf4ca001ebff8088fddb3b9a2480d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 21:27:12 +0200 Subject: [PATCH 2/5] Change how base repository is resolved On first run in a git repository, `BaseRepo()` will now prompt the user which repository should be queried as base repository if there are multiple git remotes or when we are in the context of a fork. In non-interactive mode, the prompt is skipped and we default to the first git remote instead. After the base repo is resolved, the result is cached in the local repository using `git config` so that RepositoryNetwork API lookups can be avoided in the future. --- context/context.go | 163 ++++++++++++++++++++++-------------- context/remote_test.go | 163 ------------------------------------ git/remote.go | 32 ++++++- pkg/cmd/pr/create/create.go | 16 +++- pkg/cmd/root/root.go | 2 +- 5 files changed, 145 insertions(+), 231 deletions(-) diff --git a/context/context.go b/context/context.go index 29ea74995..746c866a2 100644 --- a/context/context.go +++ b/context/context.go @@ -1,26 +1,27 @@ +// TODO: rename this package to avoid clash with stdlib package context import ( "errors" - "fmt" "sort" - "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" + "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" ) // cap the number of git remotes looked up, since the user might have an // unusually large number of git remotes const maxRemotesForLookup = 5 -// ResolveRemotesToRepos takes in a list of git remotes and fetches more information about the repositories they map to. -// Only the git remotes belonging to the same hostname are ever looked up; all others are ignored. -func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (ResolvedRemotes, error) { +func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (*ResolvedRemotes, error) { sort.Stable(remotes) - result := ResolvedRemotes{ - Remotes: remotes, + result := &ResolvedRemotes{ + remotes: remotes, apiClient: client, } @@ -31,84 +32,123 @@ func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (Re if err != nil { return result, err } - result.BaseOverride = baseOverride + result.baseOverride = baseOverride } - foundBaseOverride := false - var hostname string + return result, nil +} + +func resolveNetwork(result *ResolvedRemotes) error { var repos []ghrepo.Interface - for i, r := range remotes { - if i == 0 { - hostname = r.RepoHost() - } else if !strings.EqualFold(r.RepoHost(), hostname) { - // ignore all remotes for a hostname different to that of the 1st remote - continue - } + for _, r := range result.remotes { repos = append(repos, r) - if baseOverride != nil && ghrepo.IsSame(r, baseOverride) { - foundBaseOverride = true - } if len(repos) == maxRemotesForLookup { break } } - if baseOverride != nil && !foundBaseOverride { - // additionally, look up the explicitly specified base repo if it's not - // already covered by git remotes - repos = append(repos, baseOverride) - } - networkResult, err := api.RepoNetwork(client, repos) - if err != nil { - return result, err - } - result.Network = networkResult - return result, nil + networkResult, err := api.RepoNetwork(result.apiClient, repos) + result.network = &networkResult + return err } type ResolvedRemotes struct { - BaseOverride ghrepo.Interface - Remotes Remotes - Network api.RepoNetworkResult + baseOverride ghrepo.Interface + remotes Remotes + network *api.RepoNetworkResult apiClient *api.Client } -// BaseRepo is the first found repository in the "upstream", "github", "origin" -// git remote order, resolved to the parent repo if the git remote points to a fork -func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { - if r.BaseOverride != nil { - for _, repo := range r.Network.Repositories { - if repo != nil && ghrepo.IsSame(repo, r.BaseOverride) { - return repo, nil - } - } - return nil, fmt.Errorf("failed looking up information about the '%s' repository", - ghrepo.FullName(r.BaseOverride)) +func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, error) { + if r.baseOverride != nil { + return r.baseOverride, nil } - for _, repo := range r.Network.Repositories { + // if any of the remotes already has a resolution, respect that + for _, r := range r.remotes { + if r.Resolved == "base" { + return r, nil + } else if r.Resolved != "" { + repo, err := ghrepo.FromFullName(r.Resolved) + if err != nil { + return nil, err + } + return ghrepo.NewWithHost(repo.RepoOwner(), repo.RepoName(), r.RepoHost()), nil + } + } + + if !io.CanPrompt() { + // we cannot prompt, so just resort to the 1st remote + return r.remotes[0], nil + } + + // from here on, consult the API + if r.network == nil { + err := resolveNetwork(r) + if err != nil { + return nil, err + } + } + + var repoNames []string + repoMap := map[string]*api.Repository{} + add := func(r *api.Repository) { + fn := ghrepo.FullName(r) + if _, ok := repoMap[fn]; !ok { + repoMap[fn] = r + repoNames = append(repoNames, fn) + } + } + + for _, repo := range r.network.Repositories { if repo == nil { continue } if repo.IsFork() { - return repo.Parent, nil + add(repo.Parent) } - return repo, nil + add(repo) } - return nil, errors.New("not found") + if len(repoNames) == 0 { + return r.remotes[0], nil + } + + baseName := repoNames[0] + if len(repoNames) > 1 { + err := prompt.SurveyAskOne(&survey.Select{ + Message: "Which should be the base repository (used for e.g. querying issues) for this directory?", + Options: repoNames, + }, &baseName) + if err != nil { + return nil, err + } + } + + // determine corresponding git remote + selectedRepo := repoMap[baseName] + resolution := "base" + remote, _ := r.RemoteForRepo(selectedRepo) + if remote == nil { + remote = r.remotes[0] + resolution = ghrepo.FullName(selectedRepo) + } + + // cache the result to git config + err := git.SetRemoteResolution(remote.Name, resolution) + return selectedRepo, err } -// HeadRepo is a fork of base repo (if any), or the first found repository that -// has push access -func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { - baseRepo, err := r.BaseRepo() - if err != nil { - return nil, err +func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, error) { + if r.network == nil { + err := resolveNetwork(r) + if err != nil { + return nil, err + } } // try to find a pushable fork among existing remotes - for _, repo := range r.Network.Repositories { + for _, repo := range r.network.Repositories { if repo != nil && repo.Parent != nil && repo.ViewerCanPush() && ghrepo.IsSame(repo.Parent, baseRepo) { return repo, nil } @@ -123,7 +163,7 @@ func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { } // fall back to any listed repository that has push access - for _, repo := range r.Network.Repositories { + for _, repo := range r.network.Repositories { if repo != nil && repo.ViewerCanPush() { return repo, nil } @@ -132,12 +172,9 @@ func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { } // RemoteForRepo finds the git remote that points to a repository -func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { - for i, remote := range r.Remotes { - if ghrepo.IsSame(remote, repo) || - // additionally, look up the resolved repository name in case this - // git remote points to this repository via a redirect - (r.Network.Repositories[i] != nil && ghrepo.IsSame(r.Network.Repositories[i], repo)) { +func (r *ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { + for _, remote := range r.remotes { + if ghrepo.IsSame(remote, repo) { return remote, nil } } diff --git a/context/remote_test.go b/context/remote_test.go index 98326b3aa..ab3f7e2e2 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -1,16 +1,13 @@ package context import ( - "bytes" "errors" "net/url" "reflect" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -69,163 +66,3 @@ func Test_translateRemotes(t *testing.T) { t.Errorf("got %q", result[0].RepoName()) } } - -func Test_resolvedRemotes_triangularSetup(t *testing.T) { - http := &httpmock.Registry{} - apiClient := api.NewClient(api.ReplaceTripper(http)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - &Remote{ - Remote: &git.Remote{Name: "fork"}, - Repo: ghrepo.New("MYSELF", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - { - Name: "REPO", - Owner: api.RepositoryOwner{Login: "MYSELF"}, - ViewerPermission: "ADMIN", - }, - }, - }, - apiClient: apiClient, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if baseRemote.Name != "origin" { - t.Errorf("got remote %q", baseRemote.Name) - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "fork" { - t.Errorf("got remote %q", headRemote.Name) - } -} - -func Test_resolvedRemotes_forkLookup(t *testing.T) { - http := &httpmock.Registry{} - apiClient := api.NewClient(api.ReplaceTripper(http)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - { "id": "FORKID", - "url": "https://github.com/FORKOWNER/REPO", - "name": "REPO", - "owner": { "login": "FORKOWNER" }, - "viewerPermission": "WRITE" - } - ] } } } } - `)) - - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - apiClient: apiClient, - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "FORKOWNER/REPO") - _, err = resolved.RemoteForRepo(headRepo) - if err == nil { - t.Fatal("expected to not find a matching remote") - } -} - -func Test_resolvedRemotes_clonedFork(t *testing.T) { - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "REPO", - Owner: api.RepositoryOwner{Login: "OWNER"}, - ViewerPermission: "ADMIN", - Parent: &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - }, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if baseRemote != nil || err == nil { - t.Error("did not expect any remote for base") - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "origin" { - t.Errorf("got remote %q", headRemote.Name) - } -} diff --git a/git/remote.go b/git/remote.go index e808c1492..77e550c37 100644 --- a/git/remote.go +++ b/git/remote.go @@ -1,6 +1,7 @@ package git import ( + "fmt" "net/url" "os/exec" "regexp" @@ -26,6 +27,7 @@ func NewRemote(name string, u string) *Remote { // Remote is a parsed git remote type Remote struct { Name string + Resolved string FetchURL *url.URL PushURL *url.URL } @@ -40,7 +42,30 @@ func Remotes() (RemoteSet, error) { if err != nil { return nil, err } - return parseRemotes(list), nil + remotes := parseRemotes(list) + + // this is affected by SetRemoteResolution + remoteCmd := exec.Command("git", "config", "--get-regexp", `^remote\..*\.gh-resolved$`) + output, _ := run.PrepareCmd(remoteCmd).Output() + for _, l := range outputLines(output) { + parts := strings.SplitN(l, " ", 2) + if len(parts) < 2 { + continue + } + rp := strings.SplitN(parts[0], ".", 3) + if len(rp) < 2 { + continue + } + name := rp[1] + for _, r := range remotes { + if r.Name == name { + r.Resolved = parts[1] + break + } + } + } + + return remotes, nil } func parseRemotes(gitRemotes []string) (remotes RemoteSet) { @@ -109,3 +134,8 @@ func AddRemote(name, u string) (*Remote, error) { PushURL: urlParsed, }, nil } + +func SetRemoteResolution(name, resolution string) error { + addCmd := exec.Command("git", "config", "--add", fmt.Sprintf("remote.%s.gh-resolved", name), resolution) + return run.PrepareCmd(addCmd).Run() +} diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index d14be560c..655fca9ba 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -127,8 +127,18 @@ func createRun(opts *CreateOptions) error { return err } - baseRepo, err := repoContext.BaseRepo() - if err != nil { + var baseRepo *api.Repository + if br, err := repoContext.BaseRepo(opts.IO); err == nil { + if r, ok := br.(*api.Repository); ok { + baseRepo = r + } else { + var err error + baseRepo, err = api.GitHubRepo(client, br) + if err != nil { + return err + } + } + } else { return fmt.Errorf("could not determine base repository: %w", err) } @@ -155,7 +165,7 @@ func createRun(opts *CreateOptions) error { // otherwise, determine the head repository with info obtained from the API if headRepo == nil { - if r, err := repoContext.HeadRepo(); err == nil { + if r, err := repoContext.HeadRepo(baseRepo); err == nil { headRepo = r } } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index e3257ed09..334ee418d 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -134,7 +134,7 @@ func resolvedBaseRepo(f *cmdutil.Factory) func() (ghrepo.Interface, error) { if err != nil { return nil, err } - baseRepo, err := repoContext.BaseRepo() + baseRepo, err := repoContext.BaseRepo(f.IOStreams) if err != nil { return nil, err } From 7a8db80420893d3201e1904ae0712697271fc100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 22:39:30 +0200 Subject: [PATCH 3/5] Prompt for push target during `pr create` We no longer guess the head repository using heuristics; instead, we present the user with the choice of pushable repositories and an additional option to create a new fork. The new `pr create --head` flag is available for the user to specify the head branch in `branch` or `owner:branch` format and completely skip any forking or auto-pushing checks. --- api/queries_repo.go | 29 +- context/context.go | 23 +- pkg/cmd/pr/create/create.go | 154 ++++-- pkg/cmd/pr/create/create_test.go | 848 +++++-------------------------- pkg/httpmock/legacy.go | 16 + 5 files changed, 269 insertions(+), 801 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index e9d8534a5..fdbd13e92 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -91,6 +91,8 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { query RepositoryInfo($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id + name + owner { login } hasIssuesEnabled description viewerPermission @@ -317,8 +319,8 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { }, nil } -// RepoFindFork finds a fork of repo affiliated with the viewer -func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { +// RepoFindForks finds forks of the repo that are affiliated with the viewer +func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Repository, error) { result := struct { Repository struct { Forks struct { @@ -330,12 +332,13 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { variables := map[string]interface{}{ "owner": repo.RepoOwner(), "repo": repo.RepoName(), + "limit": limit, } if err := client.GraphQL(repo.RepoHost(), ` - query RepositoryFindFork($owner: String!, $repo: String!) { + query RepositoryFindFork($owner: String!, $repo: String!, $limit: Int!) { repository(owner: $owner, name: $repo) { - forks(first: 1, affiliations: [OWNER, COLLABORATOR]) { + forks(first: $limit, affiliations: [OWNER, COLLABORATOR]) { nodes { id name @@ -350,14 +353,18 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { return nil, err } - forks := result.Repository.Forks.Nodes - // we check ViewerCanPush, even though we expect it to always be true per - // `affiliations` condition, to guard against versions of GitHub with a - // faulty `affiliations` implementation - if len(forks) > 0 && forks[0].ViewerCanPush() { - return InitRepoHostname(&forks[0], repo.RepoHost()), nil + var results []*Repository + for _, r := range result.Repository.Forks.Nodes { + // we check ViewerCanPush, even though we expect it to always be true per + // `affiliations` condition, to guard against versions of GitHub with a + // faulty `affiliations` implementation + if !r.ViewerCanPush() { + continue + } + results = append(results, InitRepoHostname(&r, repo.RepoHost())) } - return nil, &NotFoundError{errors.New("no fork found")} + + return results, nil } type RepoMetadataResult struct { diff --git a/context/context.go b/context/context.go index 746c866a2..babd51f55 100644 --- a/context/context.go +++ b/context/context.go @@ -139,7 +139,7 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, e return selectedRepo, err } -func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, error) { +func (r *ResolvedRemotes) HeadRepos() ([]*api.Repository, error) { if r.network == nil { err := resolveNetwork(r) if err != nil { @@ -147,28 +147,13 @@ func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, } } - // try to find a pushable fork among existing remotes - for _, repo := range r.network.Repositories { - if repo != nil && repo.Parent != nil && repo.ViewerCanPush() && ghrepo.IsSame(repo.Parent, baseRepo) { - return repo, nil - } - } - - // a fork might still exist on GitHub, so let's query for it - var notFound *api.NotFoundError - if repo, err := api.RepoFindFork(r.apiClient, baseRepo); err == nil { - return repo, nil - } else if !errors.As(err, ¬Found) { - return nil, err - } - - // fall back to any listed repository that has push access + var results []*api.Repository for _, repo := range r.network.Repositories { if repo != nil && repo.ViewerCanPush() { - return repo, nil + results = append(results, repo) } } - return nil, errors.New("none of the repositories have push access") + return results, nil } // RemoteForRepo finds the git remote that points to a repository diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 655fca9ba..3bffbb1d7 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -17,6 +18,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/githubtemplate" "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -40,6 +42,7 @@ type CreateOptions struct { Title string Body string BaseBranch string + HeadBranch string Reviewers []string Assignees []string @@ -60,13 +63,23 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "create", Short: "Create a pull request", + Long: heredoc.Doc(` + Create a pull request on GitHub. + + When the current branch isn't fully pushed to a git remote, a prompt will ask where + to push the branch and offer an option to fork the base repository. Use '--head' to + explicitly skip any forking or pushing behavior. + + A prompt will also ask for the title and the body of the pull request. Use '--title' + and '--body' to skip this, or use '--fill' to autofill these values from git commits. + `), Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" $ gh issue create --label "bug,help wanted" $ gh issue create --label bug --label "help wanted" $ gh pr create --reviewer monalisa,hubot $ gh pr create --project "Roadmap" - $ gh pr create --base develop + $ gh pr create --base develop --head monalisa:feature `), Args: cmdutil.NoArgsQuoteReminder, RunE: func(cmd *cobra.Command, args []string) error { @@ -96,9 +109,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl := cmd.Flags() fl.BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark pull request as a draft") - fl.StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") - fl.StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") - fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The branch into which you want your code merged") + fl.StringVarP(&opts.Title, "title", "t", "", "Title for the pull request") + fl.StringVarP(&opts.Body, "body", "b", "", "Body for the pull request") + fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged") + fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default: current branch)") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Do not prompt for title/body and just use commit info") fl.StringSliceVarP(&opts.Reviewers, "reviewer", "r", nil, "Request reviews from people by their `login`") @@ -132,6 +146,8 @@ func createRun(opts *CreateOptions) error { if r, ok := br.(*api.Repository); ok { baseRepo = r } else { + // TODO: if RepoNetwork is going to be requested anyway in `repoContext.HeadRepos()`, + // consider piggybacking on that result instead of performing a separate lookup var err error baseRepo, err = api.GitHubRepo(client, br) if err != nil { @@ -142,32 +158,106 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("could not determine base repository: %w", err) } - headBranch, err := opts.Branch() - if err != nil { - return fmt.Errorf("could not determine the current branch: %w", err) + isPushEnabled := false + headBranch := opts.HeadBranch + headBranchLabel := opts.HeadBranch + if headBranch == "" { + headBranch, err = opts.Branch() + if err != nil { + return fmt.Errorf("could not determine the current branch: %w", err) + } + headBranchLabel = headBranch + isPushEnabled = true + } else if idx := strings.IndexRune(headBranch, ':'); idx >= 0 { + headBranch = headBranch[idx+1:] + } + + if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { + fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) } var headRepo ghrepo.Interface var headRemote *context.Remote - // determine whether the head branch is already pushed to a remote - headBranchPushedTo := determineTrackingBranch(remotes, headBranch) - if headBranchPushedTo != nil { - for _, r := range remotes { - if r.Name != headBranchPushedTo.RemoteName { - continue + if isPushEnabled { + // determine whether the head branch is already pushed to a remote + if pushedTo := determineTrackingBranch(remotes, headBranch); pushedTo != nil { + isPushEnabled = false + for _, r := range remotes { + if r.Name != pushedTo.RemoteName { + continue + } + headRepo = r + headRemote = r + break } - headRepo = r - headRemote = r - break } } - // otherwise, determine the head repository with info obtained from the API - if headRepo == nil { - if r, err := repoContext.HeadRepo(baseRepo); err == nil { - headRepo = r + // otherwise, ask the user for the head repository using info obtained from the API + if headRepo == nil && isPushEnabled && opts.IO.CanPrompt() { + pushableRepos, err := repoContext.HeadRepos() + if err != nil { + return err } + + if len(pushableRepos) == 0 { + pushableRepos, err = api.RepoFindForks(client, baseRepo, 3) + if err != nil { + return err + } + } + + currentLogin, err := api.CurrentLoginName(client, baseRepo.RepoHost()) + if err != nil { + return err + } + + hasOwnFork := false + var pushOptions []string + for _, r := range pushableRepos { + pushOptions = append(pushOptions, ghrepo.FullName(r)) + if r.RepoOwner() == currentLogin { + hasOwnFork = true + } + } + + if !hasOwnFork { + pushOptions = append(pushOptions, "Create a fork of "+ghrepo.FullName(baseRepo)) + } + pushOptions = append(pushOptions, "Skip pushing the branch") + pushOptions = append(pushOptions, "Cancel") + + var selectedOption int + err = prompt.SurveyAskOne(&survey.Select{ + Message: fmt.Sprintf("Where should we push the '%s' branch?", headBranch), + Options: pushOptions, + }, &selectedOption) + if err != nil { + return err + } + + if selectedOption < len(pushableRepos) { + headRepo = pushableRepos[selectedOption] + if !ghrepo.IsSame(baseRepo, headRepo) { + headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) + } + } else if pushOptions[selectedOption] == "Skip pushing the branch" { + isPushEnabled = false + } else if pushOptions[selectedOption] == "Cancel" { + return cmdutil.SilentError + } else { + // "Create a fork of ..." + if baseRepo.IsPrivate { + return fmt.Errorf("cannot fork private repository %s", ghrepo.FullName(baseRepo)) + } + headBranchLabel = fmt.Sprintf("%s:%s", currentLogin, headBranch) + } + } + + if headRepo == nil && isPushEnabled && !opts.IO.CanPrompt() { + fmt.Fprintf(opts.IO.ErrOut, "aborted: you must first push the current branch to a remote, or use the --head flag") + return cmdutil.SilentError } baseBranch := opts.BaseBranch @@ -178,10 +268,6 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } - if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { - fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) - } - var milestoneTitles []string if opts.Milestone != "" { milestoneTitles = []string{opts.Milestone} @@ -211,10 +297,6 @@ func createRun(opts *CreateOptions) error { } if !opts.WebMode { - headBranchLabel := headBranch - if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) { - headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) - } existingPR, err := api.PullRequestForBranch(client, baseRepo, baseBranch, headBranchLabel) var notFound *api.NotFoundError if err != nil && !errors.As(err, ¬Found) { @@ -297,10 +379,7 @@ func createRun(opts *CreateOptions) error { didForkRepo := false // if a head repository could not be determined so far, automatically create // one by forking the base repository - if headRepo == nil { - if baseRepo.IsPrivate { - return fmt.Errorf("cannot fork private repository '%s'", ghrepo.FullName(baseRepo)) - } + if headRepo == nil && isPushEnabled { headRepo, err = api.ForkRepo(client, baseRepo) if err != nil { return fmt.Errorf("error forking repo: %w", err) @@ -308,12 +387,7 @@ func createRun(opts *CreateOptions) error { didForkRepo = true } - headBranchLabel := headBranch - if !ghrepo.IsSame(baseRepo, headRepo) { - headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) - } - - if headRemote == nil { + if headRemote == nil && headRepo != nil { headRemote, _ = repoContext.RemoteForRepo(headRepo) } @@ -324,7 +398,7 @@ func createRun(opts *CreateOptions) error { // // In either case, we want to add the head repo as a new git remote so we // can push to it. - if headRemote == nil { + if headRemote == nil && isPushEnabled { cfg, err := opts.Config() if err != nil { return err @@ -345,7 +419,7 @@ func createRun(opts *CreateOptions) error { } // automatically push the branch if it hasn't been pushed anywhere yet - if headBranchPushedTo == nil { + if isPushEnabled { pushTries := 0 maxPushTries := 3 for { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 1eecab69b..935675f5d 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -5,8 +5,6 @@ import ( "encoding/json" "io/ioutil" "net/http" - "os" - "path" "reflect" "strings" "testing" @@ -56,8 +54,11 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, remotes context.Remot } return context.Remotes{ { - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), + Remote: &git.Remote{ + Name: "origin", + Resolved: "base", + }, + Repo: ghrepo.New("OWNER", "REPO"), }, }, nil }, @@ -97,31 +98,23 @@ func TestPRCreate_nontty_web(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push cs.Stub("") // browser - output, err := runCommand(http, nil, "feature", false, `--web`) + output, err := runCommand(http, nil, "feature", false, `--web --head=feature`) require.NoError(t, err) eq(t, output.String(), "") eq(t, output.Stderr(), "") - eq(t, len(cs.Calls), 6) - eq(t, strings.Join(cs.Calls[4].Args, " "), "git push --set-upstream origin HEAD:feature") - browserCall := cs.Calls[5].Args + eq(t, len(cs.Calls), 3) + browserCall := cs.Calls[2].Args eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature?expand=1") } @@ -144,11 +137,7 @@ func TestPRCreate_nontty(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } @@ -162,16 +151,13 @@ func TestPRCreate_nontty(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - output, err := runCommand(http, nil, "feature", false, `-t "my title" -b "my body"`) + output, err := runCommand(http, nil, "feature", false, `-t "my title" -b "my body" -H feature`) require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -199,20 +185,30 @@ func TestPRCreate(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(input map[string]interface{}) { + assert.Equal(t, "REPOID", input["repositoryId"].(string)) + assert.Equal(t, "my title", input["title"].(string)) + assert.Equal(t, "my body", input["body"].(string)) + assert.Equal(t, "master", input["baseRefName"].(string)) + assert.Equal(t, "feature", input["headRefName"].(string)) + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -223,49 +219,51 @@ func TestPRCreate(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(0) + output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body"`) require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "my title") - eq(t, reqBody.Variables.Input.Body, "my body") - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) + assert.Equal(t, "\nCreating pull request for feature into master in OWNER/REPO\n\n", output.Stderr()) } -func TestPRCreate_nonLegacyTemplate(t *testing.T) { + +func TestPRCreate_createFork(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "monalisa"} } }`)) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + `)) + http.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StatusStringResponse(201, ` + { "node_id": "NODEID", + "name": "REPO", + "owner": {"login": "monalisa"} + } + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(input map[string]interface{}) { + assert.Equal(t, "REPOID", input["repositoryId"].(string)) + assert.Equal(t, "master", input["baseRefName"].(string)) + assert.Equal(t, "monalisa:feature", input["headRefName"].(string)) + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -274,8 +272,50 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Stub("") // git remote add cs.Stub("") // git push + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(1) + + output, err := runCommand(http, nil, "feature", true, `-t title -b body`) + require.NoError(t, err) + + assert.Equal(t, []string{"git", "remote", "add", "-f", "fork", "https://github.com/monalisa/REPO.git"}, cs.Calls[4].Args) + assert.Equal(t, []string{"git", "push", "--set-upstream", "fork", "HEAD:feature"}, cs.Calls[5].Args) + + assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) +} + +func TestPRCreate_nonLegacyTemplate(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.StubRepoInfoResponse("OWNER", "REPO", "master") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(input map[string]interface{}) { + assert.Equal(t, "my title", input["title"].(string)) + assert.Equal(t, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue", input["body"].(string)) + })) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + as, teardown := prompt.InitAskStubber() defer teardown() as.Stub([]*prompt.QuestionStub{ @@ -297,29 +337,9 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { }, }) - output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, `-t "my title"`, "./fixtures/repoWithNonLegacyPRTemplates") + output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, `-t "my title" -H feature`, "./fixtures/repoWithNonLegacyPRTemplates") require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "my title") - eq(t, reqBody.Variables.Input.Body, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue") - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } @@ -327,15 +347,7 @@ func TestPRCreate_metadata(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query RepositoryNetwork\b`), - httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register( - httpmock.GraphQL(`query RepositoryFindFork\b`), - httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.Register( httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` @@ -434,71 +446,20 @@ func TestPRCreate_metadata(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - output, err := runCommand(http, nil, "feature", true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) + output, err := runCommand(http, nil, "feature", true, `-t TITLE -b BODY -H feature -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) eq(t, err, nil) eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } -func TestPRCreate_withForking(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponseWithPermission("OWNER", "REPO", "READ") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "node_id": "NODEID", - "name": "REPO", - "owner": {"login": "myself"}, - "clone_url": "http://example.com", - "created_at": "2008-02-25T20:21:40Z" - } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git remote add - cs.Stub("") // git push - - output, err := runCommand(http, nil, "feature", true, `-t title -b body`) - require.NoError(t, err) - - eq(t, http.Requests[3].URL.Path, "/repos/OWNER/REPO/forks") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - func TestPRCreate_alreadyExists(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "url": "https://github.com/OWNER/REPO/pull/123", @@ -515,7 +476,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - _, err := runCommand(http, nil, "feature", true, ``) + _, err := runCommand(http, nil, "feature", true, `-H feature`) if err == nil { t.Fatal("error expected, got nil") } @@ -524,48 +485,15 @@ func TestPRCreate_alreadyExists(t *testing.T) { } } -func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "url": "https://github.com/OWNER/REPO/pull/123", - "headRefName": "feature", - "baseRefName": "master" } - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString("{}")) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git rev-parse - - _, err := runCommand(http, nil, "feature", true, `-BanotherBase -t"cool" -b"nah"`) - if err != nil { - t.Errorf("got unexpected error %q", err) - } -} - func TestPRCreate_web(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -577,6 +505,10 @@ func TestPRCreate_web(t *testing.T) { cs.Stub("") // git push cs.Stub("") // browser + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(0) + output, err := runCommand(http, nil, "feature", true, `--web`) require.NoError(t, err) @@ -589,552 +521,6 @@ func TestPRCreate_web(t *testing.T) { eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature?expand=1") } -func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub(" M git/git.go") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - - output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body"`) - eq(t, err, nil) - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - test.ExpectLines(t, output.Stderr(), `Warning: 1 uncommitted change`, `Creating pull request for.*feature.*into.*master.*in OWNER/REPO`) -} - -func TestPRCreate_cross_repo_same_branch(t *testing.T) { - remotes := context.Remotes{ - { - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - { - Remote: &git.Remote{Name: "fork"}, - Repo: ghrepo.New("MYSELF", "REPO"), - }, - } - - http := initFakeHTTP() - defer http.Verify(t) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID0", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "READ" - }, - "repo_001" : { - "parent": { - "id": "REPOID0", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "READ" - }, - "id": "REPOID1", - "name": "REPO", - "owner": {"login": "MYSELF"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "WRITE" - } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - - output, err := runCommand(http, remotes, "default", true, `-t "cross repo" -b "same branch"`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID0") - eq(t, reqBody.Variables.Input.Title, "cross repo") - eq(t, reqBody.Variables.Input.Body, "same branch") - eq(t, reqBody.Variables.Input.BaseRefName, "default") - eq(t, reqBody.Variables.Input.HeadRefName, "MYSELF:default") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - - // goal: only care that gql is formatted properly -} - -func TestPRCreate_survey_defaults_multicommit(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "cool_bug-fixes", true, ``) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "- commit 1\n- commit 0\n" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "cool bug fixes") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "cool_bug-fixes") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_defaults_monocommit(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(inputs map[string]interface{}) { - eq(t, inputs["repositoryId"], "REPOID") - eq(t, inputs["title"], "the sky above the port") - eq(t, inputs["body"], "was the color of a television, turned to a dead channel") - eq(t, inputs["baseRefName"], "master") - eq(t, inputs["headRefName"], "feature") - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - eq(t, err, nil) - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_defaults_monocommit_template(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(inputs map[string]interface{}) { - eq(t, inputs["repositoryId"], "REPOID") - eq(t, inputs["title"], "the sky above the port") - eq(t, inputs["body"], "was the color of a television\n\n... turned to a dead channel") - eq(t, inputs["baseRefName"], "master") - eq(t, inputs["headRefName"], "feature") - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - tmpdir, err := ioutil.TempDir("", "gh-cli") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpdir) - - templateFp := path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md") - _ = os.MkdirAll(path.Dir(templateFp), 0700) - _ = ioutil.WriteFile(templateFp, []byte("... turned to a dead channel"), 0700) - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television") // git show - cs.Stub(tmpdir) // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - eq(t, err, nil) - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_autofill_nontty(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - output, err := runCommand(http, nil, "feature", false, `-f`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID) - assert.Equal(t, "the sky above the port", reqBody.Variables.Input.Title) - assert.Equal(t, expectedBody, reqBody.Variables.Input.Body) - assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName) - assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName) - - assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) - - assert.Equal(t, "", output.Stderr()) -} - -func TestPRCreate_survey_autofill(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - output, err := runCommand(http, nil, "feature", true, `-f`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "the sky above the port") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_defaults_error_autofill(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - - _, err := runCommand(http, nil, "feature", true, "-f") - - eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") -} - -func TestPRCreate_defaults_error_web(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - - _, err := runCommand(http, nil, "feature", true, "-w") - - eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") -} - -func TestPRCreate_defaults_error_interactive(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Value: "social distancing", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 1, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - require.NoError(t, err) - - stderr := string(output.Stderr()) - eq(t, strings.Contains(stderr, "warning: could not compute title or body defaults: could not find any commits"), true) -} - func Test_determineTrackingBranch_empty(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index 0d42005d6..9b5d5afef 100644 --- a/pkg/httpmock/legacy.go +++ b/pkg/httpmock/legacy.go @@ -48,6 +48,22 @@ func (r *Registry) StubWithFixturePath(status int, fixturePath string) func() { } } +func (r *Registry) StubRepoInfoResponse(owner, repo, branch string) { + r.Register( + GraphQL(`query RepositoryInfo\b`), + StringResponse(fmt.Sprintf(` + { "data": { "repository": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "description": "", + "defaultBranchRef": {"name": "%s"}, + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `, repo, owner, branch))) +} + func (r *Registry) StubRepoResponse(owner, repo string) { r.StubRepoResponseWithPermission(owner, repo, "WRITE") } From f9239661f2083c4785e37a6ac543bf7782b40629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 14:48:53 +0200 Subject: [PATCH 4/5] Reuse the StubRepoInfoReponse test helper --- pkg/cmd/issue/create/create_test.go | 10 +--------- pkg/cmd/pr/checkout/checkout_test.go | 6 +----- pkg/cmd/repo/create/create_test.go | 9 +-------- pkg/cmd/repo/view/view_test.go | 4 +--- 4 files changed, 4 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index ffa5d3365..2216f5b1d 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -191,15 +191,7 @@ func TestIssueCreate_metadata(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "main") http.Register( httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), httpmock.StringResponse(` diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index c49fef7e0..ea9926018 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -198,11 +198,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { "maintainerCanModify": false } } } } `)) - http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` - { "data": { "repository": { - "defaultBranchRef": {"name": "master"} - } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") ranCommands := [][]string{} restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 1fd99c035..303119f36 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -322,14 +322,7 @@ func TestRepoCreate_template(t *testing.T) { } } } }`)) - reg.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(` - { "data": { - "repository": { - "id": "REPOID", - "description": "DESCRIPTION" - } } }`)) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") reg.Register( httpmock.GraphQL(`query UserCurrent\b`), diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index 626ff62ed..55e8ace6b 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -104,9 +104,7 @@ func Test_RepoView_Web(t *testing.T) { for _, tt := range tests { reg := &httpmock.Registry{} - reg.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(`{}`)) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") opts := &ViewOptions{ Web: true, From f99a55438fcd47af2a143e3974d02a08a396aaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 15:48:13 +0200 Subject: [PATCH 5/5] Remove `issue create` examples from `pr create` --- pkg/cmd/pr/create/create.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 3bffbb1d7..3259b5561 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -75,8 +75,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co `), Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" - $ gh issue create --label "bug,help wanted" - $ gh issue create --label bug --label "help wanted" $ gh pr create --reviewer monalisa,hubot $ gh pr create --project "Roadmap" $ gh pr create --base develop --head monalisa:feature