From 53a8737c87b130b46d0cc7522382dfafa86df296 Mon Sep 17 00:00:00 2001 From: Ariel Deitcher Date: Mon, 9 May 2022 12:01:30 -0700 Subject: [PATCH 1/2] pr merge: add support for Merge Queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- api/client.go | 1 + api/queries_pr.go | 38 +-- pkg/cmd/pr/merge/merge.go | 179 +++++++----- pkg/cmd/pr/merge/merge_test.go | 512 +++++++++++++++++++++++++-------- 4 files changed, 526 insertions(+), 204 deletions(-) diff --git a/api/client.go b/api/client.go index e4cd392d4..3a2aa83ca 100644 --- a/api/client.go +++ b/api/client.go @@ -299,6 +299,7 @@ func (c Client) GraphQL(hostname string, query string, variables map[string]inte } req.Header.Set("Content-Type", "application/json; charset=utf-8") + req.Header.Set("GraphQL-Features", "merge_queue") resp, err := c.http.Do(req) if err != nil { diff --git a/api/queries_pr.go b/api/queries_pr.go index 6cda72919..4cbc1ffd1 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -17,24 +17,26 @@ type PullRequestAndTotalCount struct { } type PullRequest struct { - ID string - Number int - Title string - State string - Closed bool - URL string - BaseRefName string - HeadRefName string - Body string - Mergeable string - Additions int - Deletions int - ChangedFiles int - MergeStateStatus string - CreatedAt time.Time - UpdatedAt time.Time - ClosedAt *time.Time - MergedAt *time.Time + ID string + Number int + Title string + State string + Closed bool + URL string + BaseRefName string + HeadRefName string + Body string + Mergeable string + Additions int + Deletions int + ChangedFiles int + MergeStateStatus string + IsInMergeQueue bool + IsMergeQueueEnabled bool // Indicates whether the pull request's base ref has a merge queue enabled. + CreatedAt time.Time + UpdatedAt time.Time + ClosedAt *time.Time + MergedAt *time.Time MergeCommit *Commit PotentialMergeCommit *Commit diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 960fd426e..34360e2b1 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -47,9 +47,12 @@ type MergeOptions struct { UseAdmin bool IsDeleteBranchIndicated bool CanDeleteLocalBranch bool - InteractiveMode bool + MergeStrategyEmpty bool } +// ErrAlreadyInMergeQueue indicates that the pull request is already in a merge queue +var ErrAlreadyInMergeQueue = errors.New("already in merge queue") + func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Command { opts := &MergeOptions{ IO: f.IOStreams, @@ -74,6 +77,11 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm Without an argument, the pull request that belongs to the current branch is selected. + + When targeting a branch that requires a merge queue, no merge strategy is required. + If required checks have not yet passed, AutoMerge will be enabled. + If required checks have passed, the pull request will be added to the merge queue. + To bypass a merge queue and merge directly, pass the '--admin' flag. `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -101,10 +109,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm methodFlags++ } if methodFlags == 0 { - if !opts.IO.CanPrompt() { - return cmdutil.FlagErrorf("--merge, --rebase, or --squash required when not running interactively") - } - opts.InteractiveMode = true + opts.MergeStrategyEmpty = true } else if methodFlags > 1 { return cmdutil.FlagErrorf("only one of --merge, --rebase, or --squash can be enabled") } @@ -151,7 +156,12 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm if runF != nil { return runF(opts) } - return mergeRun(opts) + + err := mergeRun(opts) + if errors.Is(err, ErrAlreadyInMergeQueue) { + return nil + } + return err }, } @@ -170,18 +180,19 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm // mergeContext contains state and dependencies to merge a pull request. type mergeContext struct { - pr *api.PullRequest - baseRepo ghrepo.Interface - httpClient *http.Client - opts *MergeOptions - cs *iostreams.ColorScheme - isTerminal bool - merged bool - localBranchExists bool - autoMerge bool - crossRepoPR bool - deleteBranch bool - switchedToBranch string + pr *api.PullRequest + baseRepo ghrepo.Interface + httpClient *http.Client + opts *MergeOptions + cs *iostreams.ColorScheme + isTerminal bool + merged bool + localBranchExists bool + autoMerge bool + crossRepoPR bool + deleteBranch bool + switchedToBranch string + mergeQueueRequired bool } // Attempt to disable auto merge on the pull request. @@ -192,6 +203,16 @@ func (m *mergeContext) disableAutoMerge() error { return m.infof("%s Auto-merge disabled for pull request #%d\n", m.cs.SuccessIconWithColor(m.cs.Green), m.pr.Number) } +// Check if this pull request is in a merge queue +func (m *mergeContext) inMergeQueue() error { + // if the pull request is in a merge queue no further action is possible + if m.pr.IsInMergeQueue { + _ = m.warnf("%s Pull request #%d is already queued to merge\n", m.cs.WarningIcon(), m.pr.Number) + return ErrAlreadyInMergeQueue + } + return nil +} + // Warn if the pull request and the remote branch have diverged. func (m *mergeContext) warnIfDiverged() { if m.opts.SelectorArg != "" || len(m.pr.Commits.Nodes) == 0 { @@ -212,6 +233,11 @@ func (m *mergeContext) warnIfDiverged() { // Check if the current state of the pull request allows for merging func (m *mergeContext) canMerge() error { + if m.mergeQueueRequired { + // a pull request can always be added to the merge queue + return nil + } + reason := blockedReason(m.pr.MergeStateStatus, m.opts.UseAdmin) if reason == "" || m.autoMerge || m.merged { @@ -253,37 +279,50 @@ func (m *mergeContext) merge() error { setCommitBody: m.opts.BodySet, } - // get user input if not already given - if m.opts.InteractiveMode { - apiClient := api.NewClientFromHTTP(m.httpClient) - r, err := api.GitHubRepo(apiClient, m.baseRepo) - if err != nil { - return err + if m.shouldAddToMergeQueue() { + if !m.opts.MergeStrategyEmpty { + // only warn for now + _ = m.warnf("%s The merge strategy for %s is set by the merge queue\n", m.cs.Yellow("!"), m.pr.BaseRefName) } - - payload.method, err = mergeMethodSurvey(r) - if err != nil { - return err - } - - m.deleteBranch, err = deleteBranchSurvey(m.opts, m.crossRepoPR, m.localBranchExists) - if err != nil { - return err - } - - allowEditMsg := payload.method != PullRequestMergeMethodRebase - for { - action, err := confirmSurvey(allowEditMsg) - if err != nil { - return fmt.Errorf("unable to confirm: %w", err) + // auto merge will either enable auto merge or add to the merge queue + payload.auto = true + } else { + // get user input if not already given + if m.opts.MergeStrategyEmpty { + if !m.opts.IO.CanPrompt() { + return cmdutil.FlagErrorf("--merge, --rebase, or --squash required when not running interactively") } - submit, err := confirmSubmission(m.httpClient, m.opts, action, &payload) + apiClient := api.NewClientFromHTTP(m.httpClient) + r, err := api.GitHubRepo(apiClient, m.baseRepo) if err != nil { return err } - if submit { - break + + payload.method, err = mergeMethodSurvey(r) + if err != nil { + return err + } + + m.deleteBranch, err = deleteBranchSurvey(m.opts, m.crossRepoPR, m.localBranchExists) + if err != nil { + return err + } + + allowEditMsg := payload.method != PullRequestMergeMethodRebase + for { + action, err := confirmSurvey(allowEditMsg) + if err != nil { + return fmt.Errorf("unable to confirm: %w", err) + } + + submit, err := confirmSubmission(m.httpClient, m.opts, action, &payload) + if err != nil { + return err + } + if submit { + break + } } } } @@ -293,6 +332,11 @@ func (m *mergeContext) merge() error { return err } + if m.shouldAddToMergeQueue() { + _ = m.infof("%s Pull request #%d will be added to the merge queue for %s when ready\n", m.cs.SuccessIconWithColor(m.cs.Green), m.pr.Number, m.pr.BaseRefName) + return nil + } + if payload.auto { method := "" switch payload.method { @@ -322,7 +366,7 @@ func (m *mergeContext) deleteLocalBranch() error { if m.merged { // prompt for delete - if m.opts.InteractiveMode && !m.opts.IsDeleteBranchIndicated { + if m.opts.IO.CanPrompt() && !m.opts.IsDeleteBranchIndicated { err := prompt.SurveyAskOne(&survey.Confirm{ Message: fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", m.pr.Number), Default: false, @@ -346,17 +390,6 @@ func (m *mergeContext) deleteLocalBranch() error { // branch the command was run on is the same as the pull request branch if currentBranch == m.pr.HeadRefName { - // if the target branch of the PR is not known, set the current branch to the - // default branch of the repository - targetBranch := m.pr.BaseRefName - if targetBranch == "" { - apiClient := api.NewClientFromHTTP(m.httpClient) - targetBranch, err = api.RepoDefaultBranch(apiClient, m.baseRepo) - if err != nil { - return err - } - } - remotes, err := m.opts.Remotes() if err != nil { return err @@ -367,6 +400,7 @@ func (m *mergeContext) deleteLocalBranch() error { return err } + targetBranch := m.pr.BaseRefName if git.HasLocalBranch(targetBranch) { if err := git.CheckoutBranch(targetBranch); err != nil { return err @@ -415,6 +449,12 @@ func (m *mergeContext) deleteRemoteBranch() error { return m.infof("%s Deleted branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), branch) } +// Add the Pull Request to a merge queue +// Admins can bypass the queue and merge directly +func (m *mergeContext) shouldAddToMergeQueue() bool { + return m.mergeQueueRequired && !m.opts.UseAdmin +} + func (m *mergeContext) warnf(format string, args ...interface{}) error { _, err := fmt.Fprintf(m.opts.IO.ErrOut, format, args...) return err @@ -432,7 +472,7 @@ func (m *mergeContext) infof(format string, args ...interface{}) error { func NewMergeContext(opts *MergeOptions) (*mergeContext, error) { findOptions := shared.FindOptions{ Selector: opts.SelectorArg, - Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"}, + Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName", "baseRefName", "isInMergeQueue", "isMergeQueueEnabled"}, } pr, baseRepo, err := opts.Finder.Find(findOptions) if err != nil { @@ -445,17 +485,18 @@ func NewMergeContext(opts *MergeOptions) (*mergeContext, error) { } return &mergeContext{ - opts: opts, - pr: pr, - cs: opts.IO.ColorScheme(), - baseRepo: baseRepo, - isTerminal: opts.IO.IsStdoutTTY(), - httpClient: httpClient, - merged: pr.State == MergeStateStatusMerged, - deleteBranch: opts.DeleteBranch, - crossRepoPR: pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner(), - autoMerge: opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus), - localBranchExists: opts.CanDeleteLocalBranch && git.HasLocalBranch(pr.HeadRefName), + opts: opts, + pr: pr, + cs: opts.IO.ColorScheme(), + baseRepo: baseRepo, + isTerminal: opts.IO.IsStdoutTTY(), + httpClient: httpClient, + merged: pr.State == MergeStateStatusMerged, + deleteBranch: opts.DeleteBranch, + crossRepoPR: pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner(), + autoMerge: opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus), + localBranchExists: opts.CanDeleteLocalBranch && git.HasLocalBranch(pr.HeadRefName), + mergeQueueRequired: pr.IsMergeQueueEnabled, }, nil } @@ -466,6 +507,10 @@ func mergeRun(opts *MergeOptions) error { return err } + if err := ctx.inMergeQueue(); err != nil { + return err + } + // no further action is possible when disabling auto merge if opts.AutoMergeDisable { return ctx.disableAutoMerge() diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index b32695be1..035954fb9 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -52,7 +52,7 @@ func Test_NewCmdMerge(t *testing.T) { IsDeleteBranchIndicated: false, CanDeleteLocalBranch: true, MergeMethod: PullRequestMergeMethodMerge, - InteractiveMode: true, + MergeStrategyEmpty: true, Body: "", BodySet: false, }, @@ -67,7 +67,7 @@ func Test_NewCmdMerge(t *testing.T) { IsDeleteBranchIndicated: true, CanDeleteLocalBranch: true, MergeMethod: PullRequestMergeMethodMerge, - InteractiveMode: true, + MergeStrategyEmpty: true, Body: "", BodySet: false, }, @@ -82,7 +82,7 @@ func Test_NewCmdMerge(t *testing.T) { IsDeleteBranchIndicated: false, CanDeleteLocalBranch: true, MergeMethod: PullRequestMergeMethodMerge, - InteractiveMode: true, + MergeStrategyEmpty: true, Body: "a body from file", BodySet: true, }, @@ -98,7 +98,7 @@ func Test_NewCmdMerge(t *testing.T) { IsDeleteBranchIndicated: false, CanDeleteLocalBranch: true, MergeMethod: PullRequestMergeMethodMerge, - InteractiveMode: true, + MergeStrategyEmpty: true, Body: "this is on standard input", BodySet: true, }, @@ -113,7 +113,7 @@ func Test_NewCmdMerge(t *testing.T) { IsDeleteBranchIndicated: false, CanDeleteLocalBranch: true, MergeMethod: PullRequestMergeMethodMerge, - InteractiveMode: true, + MergeStrategyEmpty: true, Body: "cool", BodySet: true, }, @@ -130,12 +130,6 @@ func Test_NewCmdMerge(t *testing.T) { isTTY: true, wantErr: "argument required when using the --repo flag", }, - { - name: "insufficient flags in non-interactive mode", - args: "123", - isTTY: false, - wantErr: "--merge, --rebase, or --squash required when not running interactively", - }, { name: "multiple merge methods", args: "123 --merge --rebase", @@ -191,7 +185,7 @@ func Test_NewCmdMerge(t *testing.T) { assert.Equal(t, tt.want.DeleteBranch, opts.DeleteBranch) assert.Equal(t, tt.want.CanDeleteLocalBranch, opts.CanDeleteLocalBranch) assert.Equal(t, tt.want.MergeMethod, opts.MergeMethod) - assert.Equal(t, tt.want.InteractiveMode, opts.InteractiveMode) + assert.Equal(t, tt.want.MergeStrategyEmpty, opts.MergeStrategyEmpty) assert.Equal(t, tt.want.Body, opts.Body) assert.Equal(t, tt.want.BodySet, opts.BodySet) }) @@ -276,7 +270,7 @@ func TestPrMerge(t *testing.T) { Title: "The title of the PR", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -285,14 +279,14 @@ func TestPrMerge(t *testing.T) { assert.Equal(t, "THE-ID", 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 rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", true, "pr merge 1 --merge") + output, err := runCommand(http, "main", true, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -317,14 +311,14 @@ func TestPrMerge_blocked(t *testing.T) { Title: "The title of the PR", MergeStateStatus: "BLOCKED", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) cs, cmdTeardown := run.Stub() defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", true, "pr merge 1 --merge") + output, err := runCommand(http, "main", true, "pr merge 1 --merge") assert.EqualError(t, err, "SilentError") assert.Equal(t, "", output.String()) @@ -350,14 +344,14 @@ func TestPrMerge_dirty(t *testing.T) { BaseRefName: "trunk", HeadRefName: "feature", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) cs, cmdTeardown := run.Stub() defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", true, "pr merge 1 --merge") + output, err := runCommand(http, "main", true, "pr merge 1 --merge") assert.EqualError(t, err, "SilentError") assert.Equal(t, "", output.String()) @@ -382,7 +376,7 @@ func TestPrMerge_nontty(t *testing.T) { Title: "The title of the PR", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -398,7 +392,7 @@ func TestPrMerge_nontty(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", false, "pr merge 1 --merge") + output, err := runCommand(http, "main", false, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -420,7 +414,7 @@ func TestPrMerge_editMessage_nontty(t *testing.T) { Title: "The title of the PR", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -437,7 +431,7 @@ func TestPrMerge_editMessage_nontty(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", false, "pr merge 1 --merge -t mytitle -b mybody") + output, err := runCommand(http, "main", false, "pr merge 1 --merge -t mytitle -b mybody") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -459,7 +453,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) { Title: "The title of the PR", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -473,7 +467,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, "master", true, "pr merge 1 --merge -R OWNER/REPO") + output, err := runCommand(http, "main", true, "pr merge 1 --merge -R OWNER/REPO") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -497,9 +491,10 @@ func TestPrMerge_deleteBranch(t *testing.T) { State: "OPEN", Title: "Blueberries are a good fruit", HeadRefName: "blueberries", + BaseRefName: "main", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -516,8 +511,8 @@ func TestPrMerge_deleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") - cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/main`, 0, "") + cs.Register(`git checkout main`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") @@ -530,7 +525,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { 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 + ✓ Deleted branch blueberries and switched to branch main `), output.Stderr()) } @@ -549,7 +544,7 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) { MergeStateStatus: "CLEAN", BaseRefName: "fruit", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -599,7 +594,7 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) { MergeStateStatus: "CLEAN", BaseRefName: "fruit", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -648,7 +643,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { HeadRefName: "blueberries", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -668,7 +663,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") - output, err := runCommand(http, "master", true, `pr merge --merge --delete-branch blueberries`) + output, err := runCommand(http, "main", true, `pr merge --merge --delete-branch blueberries`) if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -690,11 +685,11 @@ func Test_nonDivergingPullRequest(t *testing.T) { Title: "Blueberries are a good fruit", State: "OPEN", MergeStateStatus: "CLEAN", + BaseRefName: "main", } stubCommit(pr, "COMMITSHA1") - prFinder := shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "master")) - prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"}) + shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "main")) http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), @@ -730,11 +725,11 @@ func Test_divergingPullRequestWarning(t *testing.T) { Title: "Blueberries are a good fruit", State: "OPEN", MergeStateStatus: "CLEAN", + BaseRefName: "main", } stubCommit(pr, "COMMITSHA1") - prFinder := shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "master")) - prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"}) + shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "main")) http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), @@ -774,7 +769,7 @@ func Test_pullRequestWithoutCommits(t *testing.T) { State: "OPEN", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -813,7 +808,7 @@ func TestPrMerge_rebase(t *testing.T) { State: "OPEN", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -829,7 +824,7 @@ func TestPrMerge_rebase(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", true, "pr merge 2 --rebase") + output, err := runCommand(http, "main", true, "pr merge 2 --rebase") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -854,7 +849,7 @@ func TestPrMerge_squash(t *testing.T) { State: "OPEN", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -870,7 +865,7 @@ func TestPrMerge_squash(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "master", true, "pr merge 3 --squash") + output, err := runCommand(http, "main", true, "pr merge 3 --squash") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -892,34 +887,31 @@ func TestPrMerge_alreadyMerged(t *testing.T) { Number: 4, State: "MERGED", HeadRefName: "blueberries", - BaseRefName: "master", + BaseRefName: "main", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") - cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/main`, 0, "") + cs.Register(`git checkout main`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) + as := prompt.NewAskStubber(t) + as.StubPrompt("Pull request #4 was already merged. Delete the branch locally?").AnswerWith(true) output, err := runCommand(http, "blueberries", true, "pr merge 4") assert.NoError(t, err) assert.Equal(t, "", output.String()) - assert.Equal(t, "✓ Deleted branch blueberries and switched to branch master\n", output.Stderr()) + assert.Equal(t, "✓ Deleted branch blueberries and switched to branch main\n", output.Stderr()) } -func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) { +func TestPrMerge_alreadyMerged_withMergeStrategy(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -932,7 +924,7 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) { HeadRepositoryOwner: api.Owner{Login: "OWNER"}, MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) cs, cmdTeardown := run.Stub() @@ -940,7 +932,7 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge") + output, err := runCommand(http, "blueberries", false, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -949,7 +941,41 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) { assert.Equal(t, "! Pull request #4 was already merged\n", output.Stderr()) } -func TestPrMerge_alreadyMerged_nonInteractive_crossRepo(t *testing.T) { +func TestPrMerge_alreadyMerged_withMergeStrategy_TTY(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "4", + &api.PullRequest{ + ID: "THE-ID", + Number: 4, + State: "MERGED", + HeadRepositoryOwner: api.Owner{Login: "OWNER"}, + MergeStateStatus: "CLEAN", + }, + baseRepo("OWNER", "REPO", "main"), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + cs.Register(`git branch -D `, 0, "") + + as := prompt.NewAskStubber(t) + as.StubPrompt("Pull request #4 was already merged. Delete the branch locally?").AnswerWith(true) + + output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge") + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Deleted branch \n", output.Stderr()) +} + +func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -962,7 +988,7 @@ func TestPrMerge_alreadyMerged_nonInteractive_crossRepo(t *testing.T) { HeadRepositoryOwner: api.Owner{Login: "monalisa"}, MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) cs, cmdTeardown := run.Stub() @@ -978,7 +1004,7 @@ func TestPrMerge_alreadyMerged_nonInteractive_crossRepo(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } -func TestPRMerge_interactive(t *testing.T) { +func TestPRMergeTTY(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -991,7 +1017,7 @@ func TestPRMerge_interactive(t *testing.T) { HeadRefName: "blueberries", MergeStateStatus: "CLEAN", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -1002,6 +1028,7 @@ func TestPRMerge_interactive(t *testing.T) { "rebaseMergeAllowed": true, "squashMergeAllowed": true } } }`)) + http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { @@ -1015,27 +1042,20 @@ func TestPRMerge_interactive(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) // Merge method survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) // Delete branch survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Submit") // Confirm submit survey + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerDefault() + as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Submit") output, err := runCommand(http, "blueberries", true, "") if err != nil { 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") + assert.Equal(t, "✓ Merged pull request #3 (It was the best of times)\n", output.Stderr()) } -func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { +func TestPRMergeTTY_withDeleteBranch(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -1047,8 +1067,9 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { Title: "It was the best of times", HeadRefName: "blueberries", MergeStateStatus: "CLEAN", + BaseRefName: "main", }, - baseRepo("OWNER", "REPO", "master"), + baseRepo("OWNER", "REPO", "main"), ) http.Register( @@ -1057,7 +1078,10 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { { "data": { "repository": { "mergeCommitAllowed": true, "rebaseMergeAllowed": true, - "squashMergeAllowed": true + "squashMergeAllowed": true, + "mergeQueue": { + "mergeMethod": "" + } } } }`)) http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), @@ -1073,20 +1097,15 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") - cs.Register(`git checkout master`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/main`, 0, "") + cs.Register(`git checkout main`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) // Merge method survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Submit") // Confirm submit survey + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Submit") output, err := runCommand(http, "blueberries", true, "-d") if err != nil { @@ -1096,17 +1115,19 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { 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 + ✓ Deleted branch blueberries and switched to branch main `), output.Stderr()) } -func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) { +func TestPRMergeTTY_squashEditCommitMsgAndSubject(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() + ios.SetStdinTTY(true) ios.SetStdoutTTY(true) ios.SetStderrTTY(true) tr := initFakeHTTP() defer tr.Verify(t) + tr.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -1141,20 +1162,12 @@ func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(2) // Merge method survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(false) // Delete branch survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Edit commit subject") // Confirm submit survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Edit commit message") // Confirm submit survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Submit") // Confirm submit survey + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerWith("Squash and merge") + as.StubPrompt("Delete the branch on GitHub?").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Edit commit message") + as.StubPrompt("What's next?").AnswerWith("Edit commit subject") + as.StubPrompt("What's next?").AnswerWith("Submit") err := mergeRun(&MergeOptions{ IO: ios, @@ -1162,8 +1175,8 @@ func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) { HttpClient: func() (*http.Client, error) { return &http.Client{Transport: tr}, nil }, - SelectorArg: "https://github.com/OWNER/REPO/pull/123", - InteractiveMode: true, + SelectorArg: "https://github.com/OWNER/REPO/pull/123", + MergeStrategyEmpty: true, Finder: shared.NewMockFinder( "https://github.com/OWNER/REPO/pull/123", &api.PullRequest{ID: "THE-ID", Number: 123, Title: "title", MergeStateStatus: "CLEAN"}, @@ -1176,7 +1189,34 @@ func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) { assert.Equal(t, "✓ Squashed and merged pull request #123 (title)\n", stderr.String()) } -func TestPRMerge_interactiveCancelled(t *testing.T) { +func TestPRMergeEmptyStrategyNonTTY(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + BaseRefName: "main", + }, + baseRepo("OWNER", "REPO", "main"), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", false, "pr merge 1") + assert.EqualError(t, err, "--merge, --rebase, or --squash required when not running interactively") + assert.Equal(t, "", output.String()) + assert.Equal(t, "", output.Stderr()) +} + +func TestPRTTY_cancelled(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -1200,16 +1240,10 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) // Merge method survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(true) // Delete branch survey - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne("Cancel") // Confirm submit survey + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerDefault() + as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Cancel") output, err := runCommand(http, "blueberries", true, "") if !errors.Is(err, cmdutil.CancelError) { @@ -1225,11 +1259,9 @@ func Test_mergeMethodSurvey(t *testing.T) { RebaseMergeAllowed: true, SquashMergeAllowed: true, } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt - as.StubOne(0) // Select first option which is rebase merge + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerWith("Rebase and merge") + method, err := mergeMethodSurvey(repo) assert.Nil(t, err) assert.Equal(t, PullRequestMergeMethodRebase, method) @@ -1345,6 +1377,248 @@ func TestMergeRun_disableAutoMerge(t *testing.T) { assert.Equal(t, "✓ Auto-merge disabled for pull request #123\n", stderr.String()) } +func TestPrInMergeQueue(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + IsInMergeQueue: true, + IsMergeQueueEnabled: true, + }, + baseRepo("OWNER", "REPO", "main"), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge 1") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "! Pull request #1 is already queued to merge\n", output.Stderr()) +} + +func TestPrAddToMergeQueueWithMergeMethod(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + IsInMergeQueue: false, + IsMergeQueueEnabled: true, + BaseRefName: "main", + }, + baseRepo("OWNER", "REPO", "main"), + ) + http.Register( + httpmock.GraphQL(`mutation PullRequestAutoMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + }), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge 1 --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + assert.Equal(t, "", output.String()) + assert.Equal(t, "! The merge strategy for main is set by the merge queue\n✓ Pull request #1 will be added to the merge queue for main when ready\n", output.Stderr()) +} + +func TestPrAddToMergeQueueClean(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + IsInMergeQueue: false, + IsMergeQueueEnabled: true, + BaseRefName: "main", + }, + baseRepo("OWNER", "REPO", "main"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestAutoMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + }), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge 1") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Pull request #1 will be added to the merge queue for main when ready\n", output.Stderr()) +} + +func TestPrAddToMergeQueueBlocked(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "BLOCKED", + IsInMergeQueue: false, + IsMergeQueueEnabled: true, + BaseRefName: "main", + }, + baseRepo("OWNER", "REPO", "main"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestAutoMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + }), + ) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge 1") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Pull request #1 will be added to the merge queue for main when ready\n", output.Stderr()) +} + +func TestPrAddToMergeQueueAdmin(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + IsInMergeQueue: false, + IsMergeQueueEnabled: true, + }, + baseRepo("OWNER", "REPO", "main"), + ) + + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "mergeCommitAllowed": true, + "rebaseMergeAllowed": true, + "squashMergeAllowed": true + } } }`)) + + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", 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 rev-parse --verify refs/heads/`, 0, "") + + as := prompt.NewAskStubber(t) + as.StubPrompt("What merge method would you like to use?").AnswerDefault() + as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() + as.StubPrompt("What's next?").AnswerDefault() + + output, err := runCommand(http, "blueberries", true, "pr merge 1 --admin") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Merged pull request #1 (The title of the PR)\n", output.Stderr()) +} + +func TestPrAddToMergeQueueAdminWithMergeStrategy(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "1", + &api.PullRequest{ + ID: "THE-ID", + Number: 1, + State: "OPEN", + Title: "The title of the PR", + MergeStateStatus: "CLEAN", + IsInMergeQueue: false, + }, + baseRepo("OWNER", "REPO", "main"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", 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 rev-parse --verify refs/heads/`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge 1 --admin --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Merged pull request #1 (The title of the PR)\n", output.Stderr()) +} + type testEditor struct{} func (e testEditor) Edit(filename, text string) (string, error) { From 4c6358dcbfee2b954c214470fe80615170efaa59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 7 Jun 2022 19:10:10 +0200 Subject: [PATCH 2/2] Guard PR merge queue queries behind feature detection --- .../featuredetection/feature_detection.go | 31 ++++++++++++++++++- .../feature_detection_test.go | 10 ++++++ pkg/cmd/pr/shared/finder.go | 13 ++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index 17254bb4f..45800a8d7 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -24,12 +24,14 @@ type PullRequestFeatures struct { ReviewDecision bool StatusCheckRollup bool BranchProtectionRule bool + MergeQueue bool } var allPullRequestFeatures = PullRequestFeatures{ ReviewDecision: true, StatusCheckRollup: true, BranchProtectionRule: true, + MergeQueue: true, } type RepositoryFeatures struct { @@ -74,7 +76,34 @@ func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) { return allPullRequestFeatures, nil } - return allPullRequestFeatures, nil + features := PullRequestFeatures{ + ReviewDecision: true, + StatusCheckRollup: true, + BranchProtectionRule: true, + } + + var featureDetection struct { + PullRequest struct { + Fields []struct { + Name string + } `graphql:"fields(includeDeprecated: true)"` + } `graphql:"PullRequest: __type(name: \"PullRequest\")"` + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(d.host), d.httpClient) + + err := gql.QueryNamed(context.Background(), "PullRequest_fields", &featureDetection, nil) + if err != nil { + return features, err + } + + for _, field := range featureDetection.PullRequest.Fields { + if field.Name == "isInMergeQueue" { + features.MergeQueue = true + } + } + + return features, nil } func (d *detector) RepositoryFeatures() (RepositoryFeatures, error) { diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index b7aa2673f..ae8695482 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -24,16 +24,26 @@ func TestPullRequestFeatures(t *testing.T) { ReviewDecision: true, StatusCheckRollup: true, BranchProtectionRule: true, + MergeQueue: true, }, wantErr: false, }, { name: "GHE", hostname: "git.my.org", + queryResponse: map[string]string{ + `query PullRequest_fields\b`: heredoc.Doc(` + { "data": { "PullRequest": { "fields": [ + {"name": "isInMergeQueue"}, + {"name": "isMergeQueueEnabled"} + ] } } } + `), + }, wantFeatures: PullRequestFeatures{ ReviewDecision: true, StatusCheckRollup: true, BranchProtectionRule: true, + MergeQueue: true, }, wantErr: false, }, diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index 79d001027..a3c6fa721 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/api" remotes "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" @@ -139,6 +140,18 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err numberFieldOnly := fields.Len() == 1 && fields.Contains("number") fields.Add("id") // for additional preload queries below + if fields.Contains("isInMergeQueue") || fields.Contains("isMergeQueueEnabled") { + detector := fd.NewDetector(httpClient, f.repo.RepoHost()) + prFeatures, err := detector.PullRequestFeatures() + if err != nil { + return nil, nil, err + } + if !prFeatures.MergeQueue { + fields.Remove("isInMergeQueue") + fields.Remove("isMergeQueueEnabled") + } + } + var pr *api.PullRequest if f.prNumber > 0 { if numberFieldOnly {