diff --git a/command/issue.go b/command/issue.go index f5a574e5d..192ac593c 100644 --- a/command/issue.go +++ b/command/issue.go @@ -12,8 +12,10 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/githubtemplate" + "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -297,28 +299,28 @@ func issueStatus(cmd *cobra.Command, args []string) error { fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") - printHeader(out, "Issues assigned to you") + shared.PrintHeader(out, "Issues assigned to you") if issuePayload.Assigned.TotalCount > 0 { printIssues(out, " ", issuePayload.Assigned.TotalCount, issuePayload.Assigned.Issues) } else { message := " There are no issues assigned to you" - printMessage(out, message) + shared.PrintMessage(out, message) } fmt.Fprintln(out) - printHeader(out, "Issues mentioning you") + shared.PrintHeader(out, "Issues mentioning you") if issuePayload.Mentioned.TotalCount > 0 { printIssues(out, " ", issuePayload.Mentioned.TotalCount, issuePayload.Mentioned.Issues) } else { - printMessage(out, " There are no issues mentioning you") + shared.PrintMessage(out, " There are no issues mentioning you") } fmt.Fprintln(out) - printHeader(out, "Issues opened by you") + shared.PrintHeader(out, "Issues opened by you") if issuePayload.Authored.TotalCount > 0 { printIssues(out, " ", issuePayload.Authored.TotalCount, issuePayload.Authored.Issues) } else { - printMessage(out, " There are no issues opened by you") + shared.PrintMessage(out, " There are no issues opened by you") } fmt.Fprintln(out) @@ -356,7 +358,7 @@ func issueView(cmd *cobra.Command, args []string) error { } func issueStateTitleWithColor(state string) string { - colorFunc := colorFuncForState(state) + colorFunc := shared.ColorFuncForState(state) return colorFunc(strings.Title(strings.ToLower(state))) } @@ -708,11 +710,11 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue) } now := time.Now() ago := now.Sub(issue.UpdatedAt) - table.AddField(issueNum, nil, colorFuncForState(issue.State)) + table.AddField(issueNum, nil, shared.ColorFuncForState(issue.State)) if !table.IsTTY() { table.AddField(issue.State, nil, nil) } - table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil) + table.AddField(text.ReplaceExcessiveWhitespace(issue.Title), nil, nil) table.AddField(labels, nil, utils.Gray) if table.IsTTY() { table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray) diff --git a/command/pr.go b/command/pr.go index 2090b4dc1..49604d3b4 100644 --- a/command/pr.go +++ b/command/pr.go @@ -1,22 +1,14 @@ package command import ( - "errors" "fmt" - "io" - "regexp" - "sort" "strconv" - "strings" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" - "github.com/cli/cli/context" - "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -28,14 +20,8 @@ func init() { RootCmd.AddCommand(prCmd) prCmd.AddCommand(prCreateCmd) - prCmd.AddCommand(prStatusCmd) prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) - prCmd.AddCommand(prMergeCmd) - prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local and remote branch after merge") - prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch") - prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") - prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") prCmd.AddCommand(prReadyCmd) prCmd.AddCommand(prListCmd) @@ -45,9 +31,6 @@ func init() { prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels") prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - - prCmd.AddCommand(prViewCmd) - prViewCmd.Flags().BoolP("web", "w", false, "Open a pull request in the browser") } var prCmd = &cobra.Command{ @@ -78,23 +61,6 @@ var prListCmd = &cobra.Command{ `), RunE: prList, } -var prStatusCmd = &cobra.Command{ - Use: "status", - Short: "Show status of relevant pull requests", - Args: cmdutil.NoArgsQuoteReminder, - RunE: prStatus, -} -var prViewCmd = &cobra.Command{ - Use: "view [ | | ]", - Short: "View a pull request", - Long: `Display the title, body, and other information about a pull request. - -Without an argument, the pull request that belongs to the current branch -is displayed. - -With '--web', open the pull request in a web browser instead.`, - RunE: prView, -} var prCloseCmd = &cobra.Command{ Use: "close { | | }", Short: "Close a pull request", @@ -107,18 +73,6 @@ var prReopenCmd = &cobra.Command{ Args: cobra.ExactArgs(1), RunE: prReopen, } -var prMergeCmd = &cobra.Command{ - Use: "merge [ | | ]", - Short: "Merge a pull request", - Long: heredoc.Doc(` - Merge a pull request on GitHub. - - By default, the head branch of the pull request will get deleted on both remote and local repositories. - To retain the branch, use '--delete-branch=false'. - `), - Args: cobra.MaximumNArgs(1), - RunE: prMerge, -} var prReadyCmd = &cobra.Command{ Use: "ready [ | | ]", Short: "Mark a pull request as ready for review", @@ -126,72 +80,6 @@ var prReadyCmd = &cobra.Command{ RunE: prReady, } -func prStatus(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return err - } - - repoOverride, _ := cmd.Flags().GetString("repo") - currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) - - if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) { - return fmt.Errorf("could not query for pull request for current branch: %w", err) - } - - // the `@me` macro is available because the API lookup is ElasticSearch-based - currentUser := "@me" - prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) - if err != nil { - return err - } - - out := colorableOut(cmd) - - fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) - fmt.Fprintln(out, "") - - printHeader(out, "Current branch") - currentPR := prPayload.CurrentPR - currentBranch, _ := ctx.Branch() - if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { - currentPR = nil - } - if currentPR != nil { - printPrs(out, 1, *currentPR) - } else if currentPRHeadRef == "" { - printMessage(out, " There is no current branch") - } else { - printMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) - } - fmt.Fprintln(out) - - printHeader(out, "Created by you") - if prPayload.ViewerCreated.TotalCount > 0 { - printPrs(out, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) - } else { - printMessage(out, " You have no open pull requests") - } - fmt.Fprintln(out) - - printHeader(out, "Requesting a code review from you") - if prPayload.ReviewRequested.TotalCount > 0 { - printPrs(out, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) - } else { - printMessage(out, " You have no pull requests to review") - } - fmt.Fprintln(out) - - return nil -} - func prList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -301,8 +189,8 @@ func prList(cmd *cobra.Command, args []string) error { if table.IsTTY() { prNum = "#" + prNum } - table.AddField(prNum, nil, colorFuncForPR(pr)) - table.AddField(replaceExcessiveWhitespace(pr.Title), nil, nil) + table.AddField(prNum, nil, shared.ColorFuncForPR(pr)) + table.AddField(text.ReplaceExcessiveWhitespace(pr.Title), nil, nil) table.AddField(pr.HeadLabel(), nil, utils.Cyan) if !table.IsTTY() { table.AddField(prStateWithDraft(&pr), nil, nil) @@ -317,69 +205,6 @@ func prList(cmd *cobra.Command, args []string) error { return nil } -func prStateTitleWithColor(pr api.PullRequest) string { - prStateColorFunc := colorFuncForPR(pr) - if pr.State == "OPEN" && pr.IsDraft { - return prStateColorFunc(strings.Title(strings.ToLower("Draft"))) - } - return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) -} - -func colorFuncForPR(pr api.PullRequest) func(string) string { - if pr.State == "OPEN" && pr.IsDraft { - return utils.Gray - } - return colorFuncForState(pr.State) -} - -// colorFuncForState returns a color function for a PR/Issue state -func colorFuncForState(state string) func(string) string { - switch state { - case "OPEN": - return utils.Green - case "CLOSED": - return utils.Red - case "MERGED": - return utils.Magenta - default: - return nil - } -} - -func prView(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - web, err := cmd.Flags().GetBool("web") - if err != nil { - return err - } - - pr, _, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - openURL := pr.URL - - if web { - if connectedToTerminal(cmd) { - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) - } - return utils.OpenInBrowser(openURL) - } - - if connectedToTerminal(cmd) { - out := colorableOut(cmd) - return printHumanPrPreview(out, pr) - } - - return printRawPrPreview(cmd.OutOrStdout(), pr) -} - func prClose(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -442,200 +267,6 @@ func prReopen(cmd *cobra.Command, args []string) error { return nil } -func prMerge(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - - if pr.Mergeable == "CONFLICTING" { - err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) - return err - } else if pr.Mergeable == "UNKNOWN" { - err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) - return err - } else if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) - return err - } - - var mergeMethod api.PullRequestMergeMethod - deleteBranch, err := cmd.Flags().GetBool("delete-branch") - if err != nil { - return err - } - - deleteLocalBranch := !cmd.Flags().Changed("repo") - crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() - - // Ensure only one merge method is specified - enabledFlagCount := 0 - isInteractive := false - if b, _ := cmd.Flags().GetBool("merge"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodMerge - } - if b, _ := cmd.Flags().GetBool("rebase"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodRebase - } - if b, _ := cmd.Flags().GetBool("squash"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodSquash - } - - if enabledFlagCount == 0 { - if !connectedToTerminal(cmd) { - return errors.New("--merge, --rebase, or --squash required when not attached to a tty") - } - isInteractive = true - } else if enabledFlagCount > 1 { - return errors.New("expected exactly one of --merge, --rebase, or --squash to be true") - } - - if isInteractive { - mergeMethod, deleteBranch, err = prInteractiveMerge(deleteLocalBranch, crossRepoPR) - if err != nil { - return nil - } - } - - var action string - if mergeMethod == api.PullRequestMergeMethodRebase { - action = "Rebased and merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase) - } else if mergeMethod == api.PullRequestMergeMethodSquash { - action = "Squashed and merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash) - } else if mergeMethod == api.PullRequestMergeMethodMerge { - action = "Merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) - } else { - err = fmt.Errorf("unknown merge method (%d) used", mergeMethod) - return err - } - - if err != nil { - return fmt.Errorf("API call failed: %w", err) - } - - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) - } - - if deleteBranch { - branchSwitchString := "" - - if deleteLocalBranch && !crossRepoPR { - currentBranch, err := ctx.Branch() - if err != nil { - return err - } - - var branchToSwitchTo string - if currentBranch == pr.HeadRefName { - branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) - if err != nil { - return err - } - err = git.CheckoutBranch(branchToSwitchTo) - if err != nil { - return err - } - } - - localBranchExists := git.HasLocalBranch(pr.HeadRefName) - if localBranchExists { - err = git.DeleteLocalBranch(pr.HeadRefName) - if err != nil { - err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err) - return err - } - } - - if branchToSwitchTo != "" { - branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) - } - } - - if !crossRepoPR { - err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName) - var httpErr api.HTTPError - // The ref might have already been deleted by GitHub - if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) { - err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) - return err - } - } - - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) - } - } - - return nil -} - -func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) { - mergeMethodQuestion := &survey.Question{ - Name: "mergeMethod", - Prompt: &survey.Select{ - Message: "What merge method would you like to use?", - Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, - Default: "Create a merge commit", - }, - } - - qs := []*survey.Question{mergeMethodQuestion} - - if !crossRepoPR { - var message string - if deleteLocalBranch { - message = "Delete the branch locally and on GitHub?" - } else { - message = "Delete the branch on GitHub?" - } - - deleteBranchQuestion := &survey.Question{ - Name: "deleteBranch", - Prompt: &survey.Confirm{ - Message: message, - Default: true, - }, - } - qs = append(qs, deleteBranchQuestion) - } - - answers := struct { - MergeMethod int - DeleteBranch bool - }{} - - err := prompt.SurveyAsk(qs, &answers) - if err != nil { - return 0, false, fmt.Errorf("could not prompt: %w", err) - } - - var mergeMethod api.PullRequestMergeMethod - switch answers.MergeMethod { - case 0: - mergeMethod = api.PullRequestMergeMethodMerge - case 1: - mergeMethod = api.PullRequestMergeMethodRebase - case 2: - mergeMethod = api.PullRequestMergeMethodSquash - } - - deleteBranch := answers.DeleteBranch - return mergeMethod, deleteBranch, nil -} - func prStateWithDraft(pr *api.PullRequest) string { if pr.IsDraft && pr.State == "OPEN" { return "DRAFT" @@ -644,78 +275,6 @@ func prStateWithDraft(pr *api.PullRequest) string { return pr.State } -func printRawPrPreview(out io.Writer, pr *api.PullRequest) error { - reviewers := prReviewerList(*pr) - assignees := prAssigneeList(*pr) - labels := prLabelList(*pr) - projects := prProjectList(*pr) - - fmt.Fprintf(out, "title:\t%s\n", pr.Title) - fmt.Fprintf(out, "state:\t%s\n", prStateWithDraft(pr)) - fmt.Fprintf(out, "author:\t%s\n", pr.Author.Login) - fmt.Fprintf(out, "labels:\t%s\n", labels) - fmt.Fprintf(out, "assignees:\t%s\n", assignees) - fmt.Fprintf(out, "reviewers:\t%s\n", reviewers) - fmt.Fprintf(out, "projects:\t%s\n", projects) - fmt.Fprintf(out, "milestone:\t%s\n", pr.Milestone.Title) - - fmt.Fprintln(out, "--") - fmt.Fprintln(out, pr.Body) - - return nil -} - -func printHumanPrPreview(out io.Writer, pr *api.PullRequest) error { - // Header (Title and State) - fmt.Fprintln(out, utils.Bold(pr.Title)) - fmt.Fprintf(out, "%s", prStateTitleWithColor(*pr)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf( - " • %s wants to merge %s into %s from %s", - pr.Author.Login, - utils.Pluralize(pr.Commits.TotalCount, "commit"), - pr.BaseRefName, - pr.HeadRefName, - ))) - fmt.Fprintln(out) - - // Metadata - if reviewers := prReviewerList(*pr); reviewers != "" { - fmt.Fprint(out, utils.Bold("Reviewers: ")) - fmt.Fprintln(out, reviewers) - } - if assignees := prAssigneeList(*pr); assignees != "" { - fmt.Fprint(out, utils.Bold("Assignees: ")) - fmt.Fprintln(out, assignees) - } - if labels := prLabelList(*pr); labels != "" { - fmt.Fprint(out, utils.Bold("Labels: ")) - fmt.Fprintln(out, labels) - } - if projects := prProjectList(*pr); projects != "" { - fmt.Fprint(out, utils.Bold("Projects: ")) - fmt.Fprintln(out, projects) - } - if pr.Milestone.Title != "" { - fmt.Fprint(out, utils.Bold("Milestone: ")) - fmt.Fprintln(out, pr.Milestone.Title) - } - - // Body - if pr.Body != "" { - fmt.Fprintln(out) - md, err := utils.RenderMarkdown(pr.Body) - if err != nil { - return err - } - fmt.Fprintln(out, md) - } - fmt.Fprintln(out) - - // Footer - fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL) - return nil -} - func prReady(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -745,300 +304,3 @@ func prReady(cmd *cobra.Command, args []string) error { return nil } - -// Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/ -const ( - requestedReviewState = "REQUESTED" // This is our own state for review request - approvedReviewState = "APPROVED" - changesRequestedReviewState = "CHANGES_REQUESTED" - commentedReviewState = "COMMENTED" - dismissedReviewState = "DISMISSED" - pendingReviewState = "PENDING" -) - -type reviewerState struct { - Name string - State string -} - -// colorFuncForReviewerState returns a color function for a reviewer state -func colorFuncForReviewerState(state string) func(string) string { - switch state { - case requestedReviewState: - return utils.Yellow - case approvedReviewState: - return utils.Green - case changesRequestedReviewState: - return utils.Red - case commentedReviewState: - return func(str string) string { return str } // Do nothing - default: - return nil - } -} - -// formattedReviewerState formats a reviewerState with state color -func formattedReviewerState(reviewer *reviewerState) string { - state := reviewer.State - if state == dismissedReviewState { - // Show "DISMISSED" review as "COMMENTED", since "dimissed" only makes - // sense when displayed in an events timeline but not in the final tally. - state = commentedReviewState - } - stateColorFunc := colorFuncForReviewerState(state) - return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(state)), "_", " "))) -} - -// prReviewerList generates a reviewer list with their last state -func prReviewerList(pr api.PullRequest) string { - reviewerStates := parseReviewers(pr) - reviewers := make([]string, 0, len(reviewerStates)) - - sortReviewerStates(reviewerStates) - - for _, reviewer := range reviewerStates { - reviewers = append(reviewers, formattedReviewerState(reviewer)) - } - - reviewerList := strings.Join(reviewers, ", ") - - return reviewerList -} - -// Ref. https://developer.github.com/v4/union/requestedreviewer/ -const teamTypeName = "Team" - -const ghostName = "ghost" - -// parseReviewers parses given Reviews and ReviewRequests -func parseReviewers(pr api.PullRequest) []*reviewerState { - reviewerStates := make(map[string]*reviewerState) - - for _, review := range pr.Reviews.Nodes { - if review.Author.Login != pr.Author.Login { - name := review.Author.Login - if name == "" { - name = ghostName - } - reviewerStates[name] = &reviewerState{ - Name: name, - State: review.State, - } - } - } - - // Overwrite reviewer's state if a review request for the same reviewer exists. - for _, reviewRequest := range pr.ReviewRequests.Nodes { - name := reviewRequest.RequestedReviewer.Login - if reviewRequest.RequestedReviewer.TypeName == teamTypeName { - name = reviewRequest.RequestedReviewer.Name - } - reviewerStates[name] = &reviewerState{ - Name: name, - State: requestedReviewState, - } - } - - // Convert map to slice for ease of sort - result := make([]*reviewerState, 0, len(reviewerStates)) - for _, reviewer := range reviewerStates { - if reviewer.State == pendingReviewState { - continue - } - result = append(result, reviewer) - } - - return result -} - -// sortReviewerStates puts completed reviews before review requests and sorts names alphabetically -func sortReviewerStates(reviewerStates []*reviewerState) { - sort.Slice(reviewerStates, func(i, j int) bool { - if reviewerStates[i].State == requestedReviewState && - reviewerStates[j].State != requestedReviewState { - return false - } - if reviewerStates[j].State == requestedReviewState && - reviewerStates[i].State != requestedReviewState { - return true - } - - return reviewerStates[i].Name < reviewerStates[j].Name - }) -} - -func prAssigneeList(pr api.PullRequest) string { - if len(pr.Assignees.Nodes) == 0 { - return "" - } - - AssigneeNames := make([]string, 0, len(pr.Assignees.Nodes)) - for _, assignee := range pr.Assignees.Nodes { - AssigneeNames = append(AssigneeNames, assignee.Login) - } - - list := strings.Join(AssigneeNames, ", ") - if pr.Assignees.TotalCount > len(pr.Assignees.Nodes) { - list += ", …" - } - return list -} - -func prLabelList(pr api.PullRequest) string { - if len(pr.Labels.Nodes) == 0 { - return "" - } - - labelNames := make([]string, 0, len(pr.Labels.Nodes)) - for _, label := range pr.Labels.Nodes { - labelNames = append(labelNames, label.Name) - } - - list := strings.Join(labelNames, ", ") - if pr.Labels.TotalCount > len(pr.Labels.Nodes) { - list += ", …" - } - return list -} - -func prProjectList(pr api.PullRequest) string { - if len(pr.ProjectCards.Nodes) == 0 { - return "" - } - - projectNames := make([]string, 0, len(pr.ProjectCards.Nodes)) - for _, project := range pr.ProjectCards.Nodes { - colName := project.Column.Name - if colName == "" { - colName = "Awaiting triage" - } - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) - } - - list := strings.Join(projectNames, ", ") - if pr.ProjectCards.TotalCount > len(pr.ProjectCards.Nodes) { - list += ", …" - } - return list -} - -func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) { - prHeadRef, err = ctx.Branch() - if err != nil { - return - } - branchConfig := git.ReadBranchConfig(prHeadRef) - - // the branch is configured to merge a special PR head ref - prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) - if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil { - prNumber, _ = strconv.Atoi(m[1]) - return - } - - var branchOwner string - if branchConfig.RemoteURL != nil { - // the branch merges from a remote specified by URL - if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil { - branchOwner = r.RepoOwner() - } - } else if branchConfig.RemoteName != "" { - // the branch merges from a remote specified by name - rem, _ := ctx.Remotes() - if r, err := rem.FindByName(branchConfig.RemoteName); err == nil { - branchOwner = r.RepoOwner() - } - } - - if branchOwner != "" { - if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { - prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") - } - // prepend `OWNER:` if this branch is pushed to a fork - if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) { - prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef) - } - } - - return -} - -func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { - for _, pr := range prs { - prNumber := fmt.Sprintf("#%d", pr.Number) - - prStateColorFunc := utils.Green - if pr.IsDraft { - prStateColorFunc = utils.Gray - } else if pr.State == "MERGED" { - prStateColorFunc = utils.Magenta - } else if pr.State == "CLOSED" { - prStateColorFunc = utils.Red - } - - fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) - - checks := pr.ChecksStatus() - reviews := pr.ReviewStatus() - - if pr.State == "OPEN" { - reviewStatus := reviews.ChangesRequested || reviews.Approved || reviews.ReviewRequired - if checks.Total > 0 || reviewStatus { - // show checks & reviews on their own line - fmt.Fprintf(w, "\n ") - } - - if checks.Total > 0 { - var summary string - if checks.Failing > 0 { - if checks.Failing == checks.Total { - summary = utils.Red("× All checks failing") - } else { - summary = utils.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) - } - } else if checks.Pending > 0 { - summary = utils.Yellow("- Checks pending") - } else if checks.Passing == checks.Total { - summary = utils.Green("✓ Checks passing") - } - fmt.Fprint(w, summary) - } - - if checks.Total > 0 && reviewStatus { - // add padding between checks & reviews - fmt.Fprint(w, " ") - } - - if reviews.ChangesRequested { - fmt.Fprint(w, utils.Red("+ Changes requested")) - } else if reviews.ReviewRequired { - fmt.Fprint(w, utils.Yellow("- Review required")) - } else if reviews.Approved { - fmt.Fprint(w, utils.Green("✓ Approved")) - } - } else { - fmt.Fprintf(w, " - %s", prStateTitleWithColor(pr)) - } - - fmt.Fprint(w, "\n") - } - remaining := totalCount - len(prs) - if remaining > 0 { - fmt.Fprintf(w, utils.Gray(" And %d more\n"), remaining) - } -} - -func printHeader(w io.Writer, s string) { - fmt.Fprintln(w, utils.Bold(s)) -} - -func printMessage(w io.Writer, s string) { - fmt.Fprintln(w, utils.Gray(s)) -} - -func replaceExcessiveWhitespace(s string) string { - s = strings.TrimSpace(s) - s = regexp.MustCompile(`\r?\n`).ReplaceAllString(s, " ") - s = regexp.MustCompile(`\s{2,}`).ReplaceAllString(s, " ") - return s -} diff --git a/command/pr_test.go b/command/pr_test.go index f11939b1b..f02a2e9f2 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -8,12 +8,9 @@ import ( "strings" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/httpmock" - "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/test" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" ) @@ -24,276 +21,6 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -func TestPRStatus(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatus.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedPrs := []*regexp.Regexp{ - regexp.MustCompile(`#8.*\[strawberries\]`), - regexp.MustCompile(`#9.*\[apples\]`), - regexp.MustCompile(`#10.*\[blueberries\]`), - regexp.MustCompile(`#11.*\[figs\]`), - } - - for _, r := range expectedPrs { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/", r) - } - } -} - -func TestPRStatus_fork(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusFork.json")) - - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{Out: []byte(`branch.blueberries.remote origin -branch.blueberries.merge refs/heads/blueberries`)} - default: - panic("not implemented") - } - })() - - output, err := RunCommand("pr status") - if err != nil { - t.Fatalf("error running command `pr status`: %v", err) - } - - branchRE := regexp.MustCompile(`#10.*\[OWNER:blueberries\]`) - if !branchRE.MatchString(output.String()) { - t.Errorf("did not match current branch:\n%v", output.String()) - } -} - -func TestPRStatus_reviewsAndChecks(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusChecks.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := []string{ - "✓ Checks passing + Changes requested", - "- Checks pending ✓ Approved", - "× 1/3 checks failing - Review required", - } - - for _, line := range expected { - if !strings.Contains(output.String(), line) { - t.Errorf("output did not contain %q: %q", line, output.String()) - } - } -} - -func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } - - unexpectedLines := []*regexp.Regexp{ - regexp.MustCompile(`#9 Blueberries are a good fruit \[blueberries\] - Merged`), - regexp.MustCompile(`#8 Blueberries are probably a good fruit \[blueberries\] - Closed`), - } - for _, r := range unexpectedLines { - if r.MatchString(output.String()) { - t.Errorf("output unexpectedly match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) - - output, err := RunCommand("pr status -R OWNER/REPO") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\]`) - if expectedLine.MatchString(output.String()) { - t.Errorf("output not expected to match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Closed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosed.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Closed`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Merged(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMerged.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Merged`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_blankSlate(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := ` -Relevant pull requests in OWNER/REPO - -Current branch - There is no pull request associated with [blueberries] - -Created by you - You have no open pull requests - -Requesting a code review from you - You have no pull requests to review - -` - if output.String() != expected { - t.Errorf("expected %q, got %q", expected, output.String()) - } -} - -func TestPRStatus_detachedHead(t *testing.T) { - initBlankContext("", "OWNER/REPO", "") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := ` -Relevant pull requests in OWNER/REPO - -Current branch - There is no current branch - -Created by you - You have no open pull requests - -Requesting a code review from you - You have no pull requests to review - -` - if output.String() != expected { - t.Errorf("expected %q, got %q", expected, output.String()) - } -} - func TestPRList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") defer stubTerminal(true)() @@ -484,594 +211,6 @@ func TestPRList_web(t *testing.T) { eq(t, url, expectedURL) } -func TestPRView_Preview_nontty(t *testing.T) { - defer stubTerminal(false)() - tests := map[string]struct { - ownerRepo string - args string - fixture string - expectedOutputs []string - }{ - "Open PR without metadata": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreview.json", - expectedOutputs: []string{ - `title:\tBlueberries are from a fork\n`, - `state:\tOPEN\n`, - `author:\tnobody\n`, - `labels:\t\n`, - `assignees:\t\n`, - `reviewers:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - `blueberries taste good`, - }, - }, - "Open PR with metadata by number": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewWithMetadataByNumber.json", - expectedOutputs: []string{ - `title:\tBlueberries are from a fork\n`, - `reviewers:\t2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`, - `assignees:\tmarseilles, monaco\n`, - `labels:\tone, two, three, four, five\n`, - `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, - `milestone:\tuluru\n`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Open PR with reviewers by number": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json", - expectedOutputs: []string{ - `title:\tBlueberries are from a fork\n`, - `state:\tOPEN\n`, - `author:\tnobody\n`, - `labels:\t\n`, - `assignees:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Open PR with metadata by branch": { - ownerRepo: "master", - args: "pr view blueberries", - fixture: "../test/fixtures/prViewPreviewWithMetadataByBranch.json", - expectedOutputs: []string{ - `title:\tBlueberries are a good fruit`, - `state:\tOPEN`, - `author:\tnobody`, - `assignees:\tmarseilles, monaco\n`, - `labels:\tone, two, three, four, five\n`, - `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, - `milestone:\tuluru\n`, - `blueberries taste good`, - }, - }, - "Open PR for the current branch": { - ownerRepo: "blueberries", - args: "pr view", - fixture: "../test/fixtures/prView.json", - expectedOutputs: []string{ - `title:\tBlueberries are a good fruit`, - `state:\tOPEN`, - `author:\tnobody`, - `assignees:\t\n`, - `labels:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Open PR wth empty body for the current branch": { - ownerRepo: "blueberries", - args: "pr view", - fixture: "../test/fixtures/prView_EmptyBody.json", - expectedOutputs: []string{ - `title:\tBlueberries are a good fruit`, - `state:\tOPEN`, - `author:\tnobody`, - `assignees:\t\n`, - `labels:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - }, - }, - "Closed PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewClosedState.json", - expectedOutputs: []string{ - `state:\tCLOSED\n`, - `author:\tnobody\n`, - `labels:\t\n`, - `assignees:\t\n`, - `reviewers:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Merged PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewMergedState.json", - expectedOutputs: []string{ - `state:\tMERGED\n`, - `author:\tnobody\n`, - `labels:\t\n`, - `assignees:\t\n`, - `reviewers:\t\n`, - `projects:\t\n`, - `milestone:\t\n`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Draft PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewDraftState.json", - expectedOutputs: []string{ - `title:\tBlueberries are from a fork\n`, - `state:\tDRAFT\n`, - `author:\tnobody\n`, - `labels:`, - `assignees:`, - `projects:`, - `milestone:`, - `\*\*blueberries taste good\*\*`, - }, - }, - "Draft PR by branch": { - ownerRepo: "master", - args: "pr view blueberries", - fixture: "../test/fixtures/prViewPreviewDraftStatebyBranch.json", - expectedOutputs: []string{ - `title:\tBlueberries are a good fruit\n`, - `state:\tDRAFT\n`, - `author:\tnobody\n`, - `labels:`, - `assignees:`, - `projects:`, - `milestone:`, - `\*\*blueberries taste good\*\*`, - }, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - initBlankContext("", "OWNER/REPO", tc.ownerRepo) - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) - - output, err := RunCommand(tc.args) - if err != nil { - t.Errorf("error running command `%v`: %v", tc.args, err) - } - - eq(t, output.Stderr(), "") - - test.ExpectLines(t, output.String(), tc.expectedOutputs...) - }) - } -} - -func TestPRView_Preview(t *testing.T) { - defer stubTerminal(true)() - tests := map[string]struct { - ownerRepo string - args string - fixture string - expectedOutputs []string - }{ - "Open PR without metadata": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreview.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Open.*nobody wants to merge 12 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, - }, - }, - "Open PR with metadata by number": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewWithMetadataByNumber.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Open.*nobody wants to merge 12 commits into master from blueberries`, - `Reviewers:.*2 \(.*Approved.*\), 3 \(Commented\), 1 \(.*Requested.*\)\n`, - `Assignees:.*marseilles, monaco\n`, - `Labels:.*one, two, three, four, five\n`, - `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, - `Milestone:.*uluru\n`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, - }, - }, - "Open PR with reviewers by number": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, - }, - }, - "Open PR with metadata by branch": { - ownerRepo: "master", - args: "pr view blueberries", - fixture: "../test/fixtures/prViewPreviewWithMetadataByBranch.json", - expectedOutputs: []string{ - `Blueberries are a good fruit`, - `Open.*nobody wants to merge 8 commits into master from blueberries`, - `Assignees:.*marseilles, monaco\n`, - `Labels:.*one, two, three, four, five\n`, - `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, - `Milestone:.*uluru\n`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10\n`, - }, - }, - "Open PR for the current branch": { - ownerRepo: "blueberries", - args: "pr view", - fixture: "../test/fixtures/prView.json", - expectedOutputs: []string{ - `Blueberries are a good fruit`, - `Open.*nobody wants to merge 8 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, - }, - }, - "Open PR wth empty body for the current branch": { - ownerRepo: "blueberries", - args: "pr view", - fixture: "../test/fixtures/prView_EmptyBody.json", - expectedOutputs: []string{ - `Blueberries are a good fruit`, - `Open.*nobody wants to merge 8 commits into master from blueberries`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, - }, - }, - "Closed PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewClosedState.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Closed.*nobody wants to merge 12 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, - }, - }, - "Merged PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewMergedState.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Merged.*nobody wants to merge 12 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, - }, - }, - "Draft PR": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewDraftState.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Draft.*nobody wants to merge 12 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, - }, - }, - "Draft PR by branch": { - ownerRepo: "master", - args: "pr view blueberries", - fixture: "../test/fixtures/prViewPreviewDraftStatebyBranch.json", - expectedOutputs: []string{ - `Blueberries are a good fruit`, - `Draft.*nobody wants to merge 8 commits into master from blueberries`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, - }, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - initBlankContext("", "OWNER/REPO", tc.ownerRepo) - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) - - output, err := RunCommand(tc.args) - if err != nil { - t.Errorf("error running command `%v`: %v", tc.args, err) - } - - eq(t, output.Stderr(), "") - - test.ExpectLines(t, output.String(), tc.expectedOutputs...) - }) - } -} - -func TestPRView_web_currentBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView.json")) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{} - default: - seenCmd = cmd - return &test.OutputStub{} - } - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - if url != "https://github.com/OWNER/REPO/pull/10" { - t.Errorf("got: %q", url) - } -} - -func TestPRView_web_noResultsForBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("../test/fixtures/prView_NoActiveBranch.json")) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{} - default: - seenCmd = cmd - return &test.OutputStub{} - } - }) - defer restoreCmd() - - _, err := RunCommand("pr view -w") - if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { - t.Errorf("error running command `pr view`: %v", err) - } - - if seenCmd != nil { - t.Fatalf("unexpected command: %v", seenCmd.Args) - } -} - -func TestPRView_web_numberArg(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequest": { - "url": "https://github.com/OWNER/REPO/pull/23" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w 23") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/pull/23") -} - -func TestPRView_web_numberArgWithHash(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequest": { - "url": "https://github.com/OWNER/REPO/pull/23" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w \"#23\"") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/pull/23") -} - -func TestPRView_web_urlArg(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequest": { - "url": "https://github.com/OWNER/REPO/pull/23" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w https://github.com/OWNER/REPO/pull/23/files") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/pull/23") -} - -func TestPRView_web_branchArg(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", - "isCrossRepository": false, - "url": "https://github.com/OWNER/REPO/pull/23" } - ] } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w blueberries") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/pull/23") -} - -func TestPRView_web_branchWithOwnerArg(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "headRefName": "blueberries", - "isCrossRepository": true, - "headRepositoryOwner": { "login": "hubot" }, - "url": "https://github.com/hubot/REPO/pull/23" } - ] } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr view -w hubot:blueberries") - if err != nil { - t.Errorf("error running command `pr view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/hubot/REPO/pull/23") -} - -func TestReplaceExcessiveWhitespace(t *testing.T) { - eq(t, replaceExcessiveWhitespace("hello\ngoodbye"), "hello goodbye") - eq(t, replaceExcessiveWhitespace(" hello goodbye "), "hello goodbye") - eq(t, replaceExcessiveWhitespace("hello goodbye"), "hello goodbye") - eq(t, replaceExcessiveWhitespace(" hello \n goodbye "), "hello goodbye") -} - -func TestPrStateTitleWithColor(t *testing.T) { - tests := map[string]struct { - pr api.PullRequest - want string - }{ - "Format OPEN state": {pr: api.PullRequest{State: "OPEN", IsDraft: false}, want: "Open"}, - "Format OPEN state for Draft PR": {pr: api.PullRequest{State: "OPEN", IsDraft: true}, want: "Draft"}, - "Format CLOSED state": {pr: api.PullRequest{State: "CLOSED", IsDraft: false}, want: "Closed"}, - "Format MERGED state": {pr: api.PullRequest{State: "MERGED", IsDraft: false}, want: "Merged"}, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - got := prStateTitleWithColor(tc.pr) - diff := cmp.Diff(tc.want, got) - if diff != "" { - t.Fatalf(diff) - } - }) - } -} - func TestPrClose(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() @@ -1197,470 +336,6 @@ func TestPRReopen_alreadyMerged(t *testing.T) { } } -func TestPrMerge(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 1, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - 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)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") - - output, err := RunCommand("pr merge 1 --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`)) - 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)) - })) - http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(`{ - "data": { - "repository": { - "defaultBranchRef": {"name": "master"} - } - } - }`)) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") - - output, err := RunCommand("pr merge 1 --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "", output.Stderr()) -} - -func TestPrMerge_nontty_insufficient_flags(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`)) - - output, err := RunCommand("pr merge 1") - if err == nil { - t.Fatal("expected error") - } - - assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a tty", err.Error()) - assert.Equal(t, "", output.String()) -} - -func TestPrMerge_withRepoFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.GraphQLQuery(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`, func(_ string, params map[string]interface{}) { - assert.Equal(t, "stinky", params["owner"].(string)) - assert.Equal(t, "boi", params["repo"].(string)) - })) - 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)) - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - eq(t, len(cs.Calls), 0) - - output, err := RunCommand("pr merge 1 --merge -R stinky/boi") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_deleteBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "PR_10", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git checkout master - cs.Stub("") // git rev-parse --verify blueberries` - cs.Stub("") // git branch -d - cs.Stub("") // git push origin --delete blueberries - - output, err := RunCommand(`pr merge --merge --delete-branch`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) -} - -func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "another-branch") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "PR_10", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - // We don't expect the default branch to be checked out, just that blueberries is deleted - cs.Stub("") // git rev-parse --verify blueberries - cs.Stub("") // git branch -d blueberries - cs.Stub("") // git push origin --delete blueberries - - output, err := RunCommand(`pr merge --merge --delete-branch blueberries`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) -} - -func TestPrMerge_noPrNumberGiven(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "PR_10", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_rebase(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 2, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - 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, "REBASE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 2 --rebase") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_squash(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 3, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - 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, "SQUASH", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 3 --squash") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3", `Deleted branch.*blueberries`) -} - -func TestPrMerge_alreadyMerged(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"} - } } }`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 4") - if err == nil { - t.Fatalf("expected an error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`) - - if !r.MatchString(err.Error()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRMerge_interactive(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes": [{ - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"}, - "id": "THE-ID", - "number": 3 - }] } } } }`)) - 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)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git push origin --delete blueberries - cs.Stub("") // git branch -d - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "mergeMethod", - Value: 0, - }, - { - Name: "deleteBranch", - Value: true, - }, - }) - - output, err := RunCommand(`pr merge`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) -} - -func TestPrMerge_multipleMergeMethods(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - - _, err := RunCommand("pr merge 1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} - -func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - - _, err := RunCommand("pr merge 1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} - func TestPRReady(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 218b3a5eb..45de50f8e 100644 --- a/command/root.go +++ b/command/root.go @@ -28,7 +28,10 @@ import ( gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout" prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" + prMergeCmd "github.com/cli/cli/pkg/cmd/pr/merge" prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review" + prStatusCmd "github.com/cli/cli/pkg/cmd/pr/status" + prViewCmd "github.com/cli/cli/pkg/cmd/pr/view" repoCmd "github.com/cli/cli/pkg/cmd/repo" repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create" @@ -180,6 +183,9 @@ func init() { prCmd.AddCommand(prReviewCmd.NewCmdReview(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prDiffCmd.NewCmdDiff(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prViewCmd.NewCmdView(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prMergeCmd.NewCmdMerge(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prStatusCmd.NewCmdStatus(&repoResolvingCmdFactory, nil)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } @@ -280,6 +286,9 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, setAccept bool) *htt opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), + // antiope-preview: Checks + // FIXME: avoid setting this header for `api` command + api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) { if token := os.Getenv("GITHUB_TOKEN"); token != "" { return fmt.Sprintf("token %s", token), nil diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go new file mode 100644 index 000000000..00aa25695 --- /dev/null +++ b/pkg/cmd/pr/merge/merge.go @@ -0,0 +1,272 @@ +package merge + +import ( + "errors" + "fmt" + "net/http" + + "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type MergeOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + SelectorArg string + DeleteBranch bool + DeleteLocalBranch bool + MergeMethod api.PullRequestMergeMethod + InteractiveMode bool +} + +func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Command { + opts := &MergeOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + var ( + flagMerge bool + flagSquash bool + flagRebase bool + ) + + cmd := &cobra.Command{ + Use: "merge [ | | ]", + Short: "Merge a pull request", + Long: heredoc.Doc(` + Merge a pull request on GitHub. + + By default, the head branch of the pull request will get deleted on both remote and local repositories. + To retain the branch, use '--delete-branch=false'. + `), + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + methodFlags := 0 + if flagMerge { + opts.MergeMethod = api.PullRequestMergeMethodMerge + methodFlags++ + } + if flagRebase { + opts.MergeMethod = api.PullRequestMergeMethodRebase + methodFlags++ + } + if flagSquash { + opts.MergeMethod = api.PullRequestMergeMethodSquash + methodFlags++ + } + if methodFlags == 0 { + if !opts.IO.IsStdoutTTY() || !opts.IO.IsStdinTTY() { + return &cmdutil.FlagError{Err: errors.New("--merge, --rebase, or --squash required when not attached to a terminal")} + } + opts.InteractiveMode = true + } else if methodFlags > 1 { + return &cmdutil.FlagError{Err: errors.New("only one of --merge, --rebase, or --squash can be enabled")} + } + + opts.DeleteLocalBranch = !cmd.Flags().Changed("repo") + + if runF != nil { + return runF(opts) + } + return mergeRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", true, "Delete the local and remote branch after merge") + cmd.Flags().BoolVarP(&flagMerge, "merge", "m", false, "Merge the commits with the base branch") + cmd.Flags().BoolVarP(&flagRebase, "rebase", "r", false, "Rebase the commits onto the base branch") + cmd.Flags().BoolVarP(&flagSquash, "squash", "s", false, "Squash the commits into one commit and merge it into the base branch") + + return cmd +} + +func mergeRun(opts *MergeOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return err + } + + if pr.Mergeable == "CONFLICTING" { + err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) + return err + } else if pr.Mergeable == "UNKNOWN" { + err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) + return err + } else if pr.State == "MERGED" { + err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) + return err + } + + mergeMethod := opts.MergeMethod + deleteBranch := opts.DeleteBranch + crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() + + if opts.InteractiveMode { + mergeMethod, deleteBranch, err = prInteractiveMerge(opts.DeleteLocalBranch, crossRepoPR) + if err != nil { + return nil + } + } + + var action string + if mergeMethod == api.PullRequestMergeMethodRebase { + action = "Rebased and merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase) + } else if mergeMethod == api.PullRequestMergeMethodSquash { + action = "Squashed and merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash) + } else if mergeMethod == api.PullRequestMergeMethodMerge { + action = "Merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) + } else { + err = fmt.Errorf("unknown merge method (%d) used", mergeMethod) + return err + } + + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + isTerminal := opts.IO.IsStdoutTTY() + + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) + } + + if deleteBranch { + branchSwitchString := "" + + if opts.DeleteLocalBranch && !crossRepoPR { + currentBranch, err := opts.Branch() + if err != nil { + return err + } + + var branchToSwitchTo string + if currentBranch == pr.HeadRefName { + branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + err = git.CheckoutBranch(branchToSwitchTo) + if err != nil { + return err + } + } + + localBranchExists := git.HasLocalBranch(pr.HeadRefName) + if localBranchExists { + err = git.DeleteLocalBranch(pr.HeadRefName) + if err != nil { + err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err) + return err + } + } + + if branchToSwitchTo != "" { + branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) + } + } + + if !crossRepoPR { + err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName) + var httpErr api.HTTPError + // The ref might have already been deleted by GitHub + if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) { + err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) + return err + } + } + + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) + } + } + + return nil +} + +func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) { + mergeMethodQuestion := &survey.Question{ + Name: "mergeMethod", + Prompt: &survey.Select{ + Message: "What merge method would you like to use?", + Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, + Default: "Create a merge commit", + }, + } + + qs := []*survey.Question{mergeMethodQuestion} + + if !crossRepoPR { + var message string + if deleteLocalBranch { + message = "Delete the branch locally and on GitHub?" + } else { + message = "Delete the branch on GitHub?" + } + + deleteBranchQuestion := &survey.Question{ + Name: "deleteBranch", + Prompt: &survey.Confirm{ + Message: message, + Default: true, + }, + } + qs = append(qs, deleteBranchQuestion) + } + + answers := struct { + MergeMethod int + DeleteBranch bool + }{} + + err := prompt.SurveyAsk(qs, &answers) + if err != nil { + return 0, false, fmt.Errorf("could not prompt: %w", err) + } + + var mergeMethod api.PullRequestMergeMethod + switch answers.MergeMethod { + case 0: + mergeMethod = api.PullRequestMergeMethodMerge + case 1: + mergeMethod = api.PullRequestMergeMethodRebase + case 2: + mergeMethod = api.PullRequestMergeMethodSquash + } + + deleteBranch := answers.DeleteBranch + return mergeMethod, deleteBranch, nil +} diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go new file mode 100644 index 000000000..b479e36a9 --- /dev/null +++ b/pkg/cmd/pr/merge/merge_test.go @@ -0,0 +1,505 @@ +package merge + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "strings" + "testing" + + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return api.InitRepoHostname(&api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "OWNER"}, + DefaultBranchRef: api.BranchRef{Name: "master"}, + }, "github.com"), nil + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + return branch, nil + }, + } + + cmd := NewCmdMerge(factory, nil) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + cli = strings.TrimPrefix(cli, "pr merge") + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func initFakeHTTP() *httpmock.Registry { + return &httpmock.Registry{} +} + +func TestPrMerge(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") + + output, err := runCommand(http, "master", true, "pr merge 1 --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_nontty(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") + + output, err := runCommand(http, "master", false, "pr merge 1 --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "", output.Stderr()) +} + +func TestPrMerge_nontty_insufficient_flags(t *testing.T) { + output, err := runCommand(nil, "master", false, "pr merge 1") + if err == nil { + t.Fatal("expected error") + } + + assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a terminal", err.Error()) + assert.Equal(t, "", output.String()) +} + +func TestPrMerge_withRepoFlag(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + output, err := runCommand(http, "master", true, "pr merge 1 --merge -R OWNER/REPO") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, 0, len(cs.Calls)) + + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_deleteBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git checkout master + cs.Stub("") // git rev-parse --verify blueberries` + cs.Stub("") // git branch -d + cs.Stub("") // git push origin --delete blueberries + + output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) +} + +func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + // We don't expect the default branch to be checked out, just that blueberries is deleted + cs.Stub("") // git rev-parse --verify blueberries + cs.Stub("") // git branch -d blueberries + cs.Stub("") // git push origin --delete blueberries + + output, err := runCommand(http, "master", true, `pr merge --merge --delete-branch blueberries`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) +} + +func TestPrMerge_noPrNumberGiven(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_rebase(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 2, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + 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, "REBASE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 2 --rebase") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_squash(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 3, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + 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, "SQUASH", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 3 --squash") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3", `Deleted branch.*blueberries`) +} + +func TestPrMerge_alreadyMerged(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"} + } } }`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 4") + if err == nil { + t.Fatalf("expected an error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`) + + if !r.MatchString(err.Error()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRMerge_interactive(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "THE-ID", + "number": 3 + }] } } } }`)) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git push origin --delete blueberries + cs.Stub("") // git branch -d + + as, surveyTeardown := prompt.InitAskStubber() + defer surveyTeardown() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "mergeMethod", + Value: 0, + }, + { + Name: "deleteBranch", + Value: true, + }, + }) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) +} + +func TestPrMerge_multipleMergeMethods(t *testing.T) { + _, err := runCommand(nil, "master", true, "1 --merge --squash") + if err == nil { + t.Fatal("expected error running `pr merge` with multiple merge methods") + } +} + +func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) { + _, err := runCommand(nil, "master", false, "1 --merge --squash") + if err == nil { + t.Fatal("expected error running `pr merge` with multiple merge methods") + } +} diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go new file mode 100644 index 000000000..f43b7fa67 --- /dev/null +++ b/pkg/cmd/pr/shared/display.go @@ -0,0 +1,47 @@ +package shared + +import ( + "fmt" + "io" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/utils" +) + +func StateTitleWithColor(pr api.PullRequest) string { + prStateColorFunc := ColorFuncForPR(pr) + if pr.State == "OPEN" && pr.IsDraft { + return prStateColorFunc(strings.Title(strings.ToLower("Draft"))) + } + return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) +} + +func ColorFuncForPR(pr api.PullRequest) func(string) string { + if pr.State == "OPEN" && pr.IsDraft { + return utils.Gray + } + return ColorFuncForState(pr.State) +} + +// ColorFuncForState returns a color function for a PR/Issue state +func ColorFuncForState(state string) func(string) string { + switch state { + case "OPEN": + return utils.Green + case "CLOSED": + return utils.Red + case "MERGED": + return utils.Magenta + default: + return nil + } +} + +func PrintHeader(w io.Writer, s string) { + fmt.Fprintln(w, utils.Bold(s)) +} + +func PrintMessage(w io.Writer, s string) { + fmt.Fprintln(w, utils.Gray(s)) +} diff --git a/test/fixtures/prStatus.json b/pkg/cmd/pr/status/fixtures/prStatus.json similarity index 100% rename from test/fixtures/prStatus.json rename to pkg/cmd/pr/status/fixtures/prStatus.json diff --git a/test/fixtures/prStatusChecks.json b/pkg/cmd/pr/status/fixtures/prStatusChecks.json similarity index 100% rename from test/fixtures/prStatusChecks.json rename to pkg/cmd/pr/status/fixtures/prStatusChecks.json diff --git a/test/fixtures/prStatusCurrentBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranch.json diff --git a/test/fixtures/prStatusCurrentBranchClosed.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosed.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchClosed.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosed.json diff --git a/test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json diff --git a/test/fixtures/prStatusCurrentBranchMerged.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMerged.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchMerged.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMerged.json diff --git a/test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go new file mode 100644 index 000000000..8708dae65 --- /dev/null +++ b/pkg/cmd/pr/status/status.go @@ -0,0 +1,238 @@ +package status + +import ( + "errors" + "fmt" + "io" + "net/http" + "regexp" + "strconv" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type StatusOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + HasRepoOverride bool +} + +func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { + opts := &StatusOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + cmd := &cobra.Command{ + Use: "status", + Short: "Show status of relevant pull requests", + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + opts.HasRepoOverride = cmd.Flags().Changed("repo") + + if runF != nil { + return runF(opts) + } + return statusRun(opts) + }, + } + + return cmd +} + +func statusRun(opts *StatusOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + var currentBranch string + var currentPRNumber int + var currentPRHeadRef string + + if !opts.HasRepoOverride { + currentBranch, err = opts.Branch() + if err != nil && !errors.Is(err, git.ErrNotOnAnyBranch) { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + + remotes, _ := opts.Remotes() + currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(baseRepo, currentBranch, remotes) + if err != nil { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + } + + // the `@me` macro is available because the API lookup is ElasticSearch-based + currentUser := "@me" + prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) + if err != nil { + return err + } + + out := opts.IO.Out + + fmt.Fprintln(out, "") + fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintln(out, "") + + shared.PrintHeader(out, "Current branch") + currentPR := prPayload.CurrentPR + if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { + currentPR = nil + } + if currentPR != nil { + printPrs(out, 1, *currentPR) + } else if currentPRHeadRef == "" { + shared.PrintMessage(out, " There is no current branch") + } else { + shared.PrintMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) + } + fmt.Fprintln(out) + + shared.PrintHeader(out, "Created by you") + if prPayload.ViewerCreated.TotalCount > 0 { + printPrs(out, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) + } else { + shared.PrintMessage(out, " You have no open pull requests") + } + fmt.Fprintln(out) + + shared.PrintHeader(out, "Requesting a code review from you") + if prPayload.ReviewRequested.TotalCount > 0 { + printPrs(out, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) + } else { + shared.PrintMessage(out, " You have no pull requests to review") + } + fmt.Fprintln(out) + + return nil +} + +func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem context.Remotes) (prNumber int, selector string, err error) { + selector = prHeadRef + branchConfig := git.ReadBranchConfig(prHeadRef) + + // the branch is configured to merge a special PR head ref + prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) + if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil { + prNumber, _ = strconv.Atoi(m[1]) + return + } + + var branchOwner string + if branchConfig.RemoteURL != nil { + // the branch merges from a remote specified by URL + if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil { + branchOwner = r.RepoOwner() + } + } else if branchConfig.RemoteName != "" { + // the branch merges from a remote specified by name + if r, err := rem.FindByName(branchConfig.RemoteName); err == nil { + branchOwner = r.RepoOwner() + } + } + + if branchOwner != "" { + if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { + selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") + } + // prepend `OWNER:` if this branch is pushed to a fork + if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) { + selector = fmt.Sprintf("%s:%s", branchOwner, prHeadRef) + } + } + + return +} + +func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { + for _, pr := range prs { + prNumber := fmt.Sprintf("#%d", pr.Number) + + prStateColorFunc := utils.Green + if pr.IsDraft { + prStateColorFunc = utils.Gray + } else if pr.State == "MERGED" { + prStateColorFunc = utils.Magenta + } else if pr.State == "CLOSED" { + prStateColorFunc = utils.Red + } + + fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, text.ReplaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) + + checks := pr.ChecksStatus() + reviews := pr.ReviewStatus() + + if pr.State == "OPEN" { + reviewStatus := reviews.ChangesRequested || reviews.Approved || reviews.ReviewRequired + if checks.Total > 0 || reviewStatus { + // show checks & reviews on their own line + fmt.Fprintf(w, "\n ") + } + + if checks.Total > 0 { + var summary string + if checks.Failing > 0 { + if checks.Failing == checks.Total { + summary = utils.Red("× All checks failing") + } else { + summary = utils.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) + } + } else if checks.Pending > 0 { + summary = utils.Yellow("- Checks pending") + } else if checks.Passing == checks.Total { + summary = utils.Green("✓ Checks passing") + } + fmt.Fprint(w, summary) + } + + if checks.Total > 0 && reviewStatus { + // add padding between checks & reviews + fmt.Fprint(w, " ") + } + + if reviews.ChangesRequested { + fmt.Fprint(w, utils.Red("+ Changes requested")) + } else if reviews.ReviewRequired { + fmt.Fprint(w, utils.Yellow("- Review required")) + } else if reviews.Approved { + fmt.Fprint(w, utils.Green("✓ Approved")) + } + } else { + fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(pr)) + } + + fmt.Fprint(w, "\n") + } + remaining := totalCount - len(prs) + if remaining > 0 { + fmt.Fprintf(w, utils.Gray(" And %d more\n"), remaining) + } +} diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go new file mode 100644 index 000000000..bc4f4e4f9 --- /dev/null +++ b/pkg/cmd/pr/status/status_test.go @@ -0,0 +1,310 @@ +package status + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "strings" + "testing" + + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + if branch == "" { + return "", git.ErrNotOnAnyBranch + } + return branch, nil + }, + } + + cmd := NewCmdStatus(factory, nil) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func initFakeHTTP() *httpmock.Registry { + return &httpmock.Registry{} +} + +func TestPRStatus(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatus.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedPrs := []*regexp.Regexp{ + regexp.MustCompile(`#8.*\[strawberries\]`), + regexp.MustCompile(`#9.*\[apples\]`), + regexp.MustCompile(`#10.*\[blueberries\]`), + regexp.MustCompile(`#11.*\[figs\]`), + } + + for _, r := range expectedPrs { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/", r) + } + } +} + +func TestPRStatus_reviewsAndChecks(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusChecks.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := []string{ + "✓ Checks passing + Changes requested", + "- Checks pending ✓ Approved", + "× 1/3 checks failing - Review required", + } + + for _, line := range expected { + if !strings.Contains(output.String(), line) { + t.Errorf("output did not contain %q: %q", line, output.String()) + } + } +} + +func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } + + unexpectedLines := []*regexp.Regexp{ + regexp.MustCompile(`#9 Blueberries are a good fruit \[blueberries\] - Merged`), + regexp.MustCompile(`#8 Blueberries are probably a good fruit \[blueberries\] - Closed`), + } + for _, r := range unexpectedLines { + if r.MatchString(output.String()) { + t.Errorf("output unexpectedly match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + +func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "-R OWNER/REPO") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\]`) + if expectedLine.MatchString(output.String()) { + t.Errorf("output not expected to match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Closed(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosed.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Closed`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Merged(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchMerged.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Merged`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_blankSlate(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no pull request associated with [blueberries] + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} + +func TestPRStatus_detachedHead(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) + + output, err := runCommand(http, "", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no current branch + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} diff --git a/test/fixtures/prView.json b/pkg/cmd/pr/view/fixtures/prView.json similarity index 100% rename from test/fixtures/prView.json rename to pkg/cmd/pr/view/fixtures/prView.json diff --git a/test/fixtures/prViewPreview.json b/pkg/cmd/pr/view/fixtures/prViewPreview.json similarity index 100% rename from test/fixtures/prViewPreview.json rename to pkg/cmd/pr/view/fixtures/prViewPreview.json diff --git a/test/fixtures/prViewPreviewClosedState.json b/pkg/cmd/pr/view/fixtures/prViewPreviewClosedState.json similarity index 100% rename from test/fixtures/prViewPreviewClosedState.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewClosedState.json diff --git a/test/fixtures/prViewPreviewDraftState.json b/pkg/cmd/pr/view/fixtures/prViewPreviewDraftState.json similarity index 100% rename from test/fixtures/prViewPreviewDraftState.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewDraftState.json diff --git a/test/fixtures/prViewPreviewDraftStatebyBranch.json b/pkg/cmd/pr/view/fixtures/prViewPreviewDraftStatebyBranch.json similarity index 100% rename from test/fixtures/prViewPreviewDraftStatebyBranch.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewDraftStatebyBranch.json diff --git a/test/fixtures/prViewPreviewMergedState.json b/pkg/cmd/pr/view/fixtures/prViewPreviewMergedState.json similarity index 100% rename from test/fixtures/prViewPreviewMergedState.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewMergedState.json diff --git a/test/fixtures/prViewPreviewWithMetadataByBranch.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByBranch.json similarity index 100% rename from test/fixtures/prViewPreviewWithMetadataByBranch.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByBranch.json diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json similarity index 100% rename from test/fixtures/prViewPreviewWithMetadataByNumber.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json diff --git a/test/fixtures/prViewPreviewWithReviewersByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json similarity index 100% rename from test/fixtures/prViewPreviewWithReviewersByNumber.json rename to pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json diff --git a/test/fixtures/prView_EmptyBody.json b/pkg/cmd/pr/view/fixtures/prView_EmptyBody.json similarity index 100% rename from test/fixtures/prView_EmptyBody.json rename to pkg/cmd/pr/view/fixtures/prView_EmptyBody.json diff --git a/test/fixtures/prView_NoActiveBranch.json b/pkg/cmd/pr/view/fixtures/prView_NoActiveBranch.json similarity index 100% rename from test/fixtures/prView_NoActiveBranch.json rename to pkg/cmd/pr/view/fixtures/prView_NoActiveBranch.json diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go new file mode 100644 index 000000000..b93bfcd1d --- /dev/null +++ b/pkg/cmd/pr/view/view.go @@ -0,0 +1,355 @@ +package view + +import ( + "fmt" + "io" + "net/http" + "sort" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ViewOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + SelectorArg string + BrowserMode bool +} + +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + cmd := &cobra.Command{ + Use: "view [ | | ]", + Short: "View a pull request", + Long: heredoc.Doc(` + Display the title, body, and other information about a pull request. + + Without an argument, the pull request that belongs to the current branch + is displayed. + + With '--web', open the pull request in a web browser instead. + `), + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.BrowserMode, "web", "w", false, "Open a pull request in the browser") + + return cmd +} + +func viewRun(opts *ViewOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, _, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return err + } + + openURL := pr.URL + connectedToTerminal := opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() + + if opts.BrowserMode { + if connectedToTerminal { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", openURL) + } + return utils.OpenInBrowser(openURL) + } + + if connectedToTerminal { + return printHumanPrPreview(opts.IO.Out, pr) + } + return printRawPrPreview(opts.IO.Out, pr) +} + +func printRawPrPreview(out io.Writer, pr *api.PullRequest) error { + reviewers := prReviewerList(*pr) + assignees := prAssigneeList(*pr) + labels := prLabelList(*pr) + projects := prProjectList(*pr) + + fmt.Fprintf(out, "title:\t%s\n", pr.Title) + fmt.Fprintf(out, "state:\t%s\n", prStateWithDraft(pr)) + fmt.Fprintf(out, "author:\t%s\n", pr.Author.Login) + fmt.Fprintf(out, "labels:\t%s\n", labels) + fmt.Fprintf(out, "assignees:\t%s\n", assignees) + fmt.Fprintf(out, "reviewers:\t%s\n", reviewers) + fmt.Fprintf(out, "projects:\t%s\n", projects) + fmt.Fprintf(out, "milestone:\t%s\n", pr.Milestone.Title) + + fmt.Fprintln(out, "--") + fmt.Fprintln(out, pr.Body) + + return nil +} + +func printHumanPrPreview(out io.Writer, pr *api.PullRequest) error { + // Header (Title and State) + fmt.Fprintln(out, utils.Bold(pr.Title)) + fmt.Fprintf(out, "%s", shared.StateTitleWithColor(*pr)) + fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + " • %s wants to merge %s into %s from %s", + pr.Author.Login, + utils.Pluralize(pr.Commits.TotalCount, "commit"), + pr.BaseRefName, + pr.HeadRefName, + ))) + fmt.Fprintln(out) + + // Metadata + if reviewers := prReviewerList(*pr); reviewers != "" { + fmt.Fprint(out, utils.Bold("Reviewers: ")) + fmt.Fprintln(out, reviewers) + } + if assignees := prAssigneeList(*pr); assignees != "" { + fmt.Fprint(out, utils.Bold("Assignees: ")) + fmt.Fprintln(out, assignees) + } + if labels := prLabelList(*pr); labels != "" { + fmt.Fprint(out, utils.Bold("Labels: ")) + fmt.Fprintln(out, labels) + } + if projects := prProjectList(*pr); projects != "" { + fmt.Fprint(out, utils.Bold("Projects: ")) + fmt.Fprintln(out, projects) + } + if pr.Milestone.Title != "" { + fmt.Fprint(out, utils.Bold("Milestone: ")) + fmt.Fprintln(out, pr.Milestone.Title) + } + + // Body + if pr.Body != "" { + fmt.Fprintln(out) + md, err := utils.RenderMarkdown(pr.Body) + if err != nil { + return err + } + fmt.Fprintln(out, md) + } + fmt.Fprintln(out) + + // Footer + fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL) + return nil +} + +// Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/ +const ( + requestedReviewState = "REQUESTED" // This is our own state for review request + approvedReviewState = "APPROVED" + changesRequestedReviewState = "CHANGES_REQUESTED" + commentedReviewState = "COMMENTED" + dismissedReviewState = "DISMISSED" + pendingReviewState = "PENDING" +) + +type reviewerState struct { + Name string + State string +} + +// colorFuncForReviewerState returns a color function for a reviewer state +func colorFuncForReviewerState(state string) func(string) string { + switch state { + case requestedReviewState: + return utils.Yellow + case approvedReviewState: + return utils.Green + case changesRequestedReviewState: + return utils.Red + case commentedReviewState: + return func(str string) string { return str } // Do nothing + default: + return nil + } +} + +// formattedReviewerState formats a reviewerState with state color +func formattedReviewerState(reviewer *reviewerState) string { + state := reviewer.State + if state == dismissedReviewState { + // Show "DISMISSED" review as "COMMENTED", since "dimissed" only makes + // sense when displayed in an events timeline but not in the final tally. + state = commentedReviewState + } + stateColorFunc := colorFuncForReviewerState(state) + return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(state)), "_", " "))) +} + +// prReviewerList generates a reviewer list with their last state +func prReviewerList(pr api.PullRequest) string { + reviewerStates := parseReviewers(pr) + reviewers := make([]string, 0, len(reviewerStates)) + + sortReviewerStates(reviewerStates) + + for _, reviewer := range reviewerStates { + reviewers = append(reviewers, formattedReviewerState(reviewer)) + } + + reviewerList := strings.Join(reviewers, ", ") + + return reviewerList +} + +// Ref. https://developer.github.com/v4/union/requestedreviewer/ +const teamTypeName = "Team" + +const ghostName = "ghost" + +// parseReviewers parses given Reviews and ReviewRequests +func parseReviewers(pr api.PullRequest) []*reviewerState { + reviewerStates := make(map[string]*reviewerState) + + for _, review := range pr.Reviews.Nodes { + if review.Author.Login != pr.Author.Login { + name := review.Author.Login + if name == "" { + name = ghostName + } + reviewerStates[name] = &reviewerState{ + Name: name, + State: review.State, + } + } + } + + // Overwrite reviewer's state if a review request for the same reviewer exists. + for _, reviewRequest := range pr.ReviewRequests.Nodes { + name := reviewRequest.RequestedReviewer.Login + if reviewRequest.RequestedReviewer.TypeName == teamTypeName { + name = reviewRequest.RequestedReviewer.Name + } + reviewerStates[name] = &reviewerState{ + Name: name, + State: requestedReviewState, + } + } + + // Convert map to slice for ease of sort + result := make([]*reviewerState, 0, len(reviewerStates)) + for _, reviewer := range reviewerStates { + if reviewer.State == pendingReviewState { + continue + } + result = append(result, reviewer) + } + + return result +} + +// sortReviewerStates puts completed reviews before review requests and sorts names alphabetically +func sortReviewerStates(reviewerStates []*reviewerState) { + sort.Slice(reviewerStates, func(i, j int) bool { + if reviewerStates[i].State == requestedReviewState && + reviewerStates[j].State != requestedReviewState { + return false + } + if reviewerStates[j].State == requestedReviewState && + reviewerStates[i].State != requestedReviewState { + return true + } + + return reviewerStates[i].Name < reviewerStates[j].Name + }) +} + +func prAssigneeList(pr api.PullRequest) string { + if len(pr.Assignees.Nodes) == 0 { + return "" + } + + AssigneeNames := make([]string, 0, len(pr.Assignees.Nodes)) + for _, assignee := range pr.Assignees.Nodes { + AssigneeNames = append(AssigneeNames, assignee.Login) + } + + list := strings.Join(AssigneeNames, ", ") + if pr.Assignees.TotalCount > len(pr.Assignees.Nodes) { + list += ", …" + } + return list +} + +func prLabelList(pr api.PullRequest) string { + if len(pr.Labels.Nodes) == 0 { + return "" + } + + labelNames := make([]string, 0, len(pr.Labels.Nodes)) + for _, label := range pr.Labels.Nodes { + labelNames = append(labelNames, label.Name) + } + + list := strings.Join(labelNames, ", ") + if pr.Labels.TotalCount > len(pr.Labels.Nodes) { + list += ", …" + } + return list +} + +func prProjectList(pr api.PullRequest) string { + if len(pr.ProjectCards.Nodes) == 0 { + return "" + } + + projectNames := make([]string, 0, len(pr.ProjectCards.Nodes)) + for _, project := range pr.ProjectCards.Nodes { + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) + } + + list := strings.Join(projectNames, ", ") + if pr.ProjectCards.TotalCount > len(pr.ProjectCards.Nodes) { + list += ", …" + } + return list +} + +func prStateWithDraft(pr *api.PullRequest) string { + if pr.IsDraft && pr.State == "OPEN" { + return "DRAFT" + } + + return pr.State +} diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go new file mode 100644 index 000000000..fa540fa62 --- /dev/null +++ b/pkg/cmd/pr/view/view_test.go @@ -0,0 +1,620 @@ +package view + +import ( + "bytes" + "io/ioutil" + "net/http" + "os/exec" + "reflect" + "strings" + "testing" + + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + +func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + return branch, nil + }, + } + + cmd := NewCmdView(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestPRView_Preview_nontty(t *testing.T) { + tests := map[string]struct { + branch string + args string + fixture string + expectedOutputs []string + }{ + "Open PR without metadata": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreview.json", + expectedOutputs: []string{ + `title:\tBlueberries are from a fork\n`, + `state:\tOPEN\n`, + `author:\tnobody\n`, + `labels:\t\n`, + `assignees:\t\n`, + `reviewers:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + `blueberries taste good`, + }, + }, + "Open PR with metadata by number": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewWithMetadataByNumber.json", + expectedOutputs: []string{ + `title:\tBlueberries are from a fork\n`, + `reviewers:\t2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`, + `assignees:\tmarseilles, monaco\n`, + `labels:\tone, two, three, four, five\n`, + `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, + `milestone:\tuluru\n`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Open PR with reviewers by number": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewWithReviewersByNumber.json", + expectedOutputs: []string{ + `title:\tBlueberries are from a fork\n`, + `state:\tOPEN\n`, + `author:\tnobody\n`, + `labels:\t\n`, + `assignees:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Open PR with metadata by branch": { + branch: "master", + args: "blueberries", + fixture: "./fixtures/prViewPreviewWithMetadataByBranch.json", + expectedOutputs: []string{ + `title:\tBlueberries are a good fruit`, + `state:\tOPEN`, + `author:\tnobody`, + `assignees:\tmarseilles, monaco\n`, + `labels:\tone, two, three, four, five\n`, + `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `milestone:\tuluru\n`, + `blueberries taste good`, + }, + }, + "Open PR for the current branch": { + branch: "blueberries", + args: "", + fixture: "./fixtures/prView.json", + expectedOutputs: []string{ + `title:\tBlueberries are a good fruit`, + `state:\tOPEN`, + `author:\tnobody`, + `assignees:\t\n`, + `labels:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Open PR wth empty body for the current branch": { + branch: "blueberries", + args: "", + fixture: "./fixtures/prView_EmptyBody.json", + expectedOutputs: []string{ + `title:\tBlueberries are a good fruit`, + `state:\tOPEN`, + `author:\tnobody`, + `assignees:\t\n`, + `labels:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + }, + }, + "Closed PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewClosedState.json", + expectedOutputs: []string{ + `state:\tCLOSED\n`, + `author:\tnobody\n`, + `labels:\t\n`, + `assignees:\t\n`, + `reviewers:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Merged PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewMergedState.json", + expectedOutputs: []string{ + `state:\tMERGED\n`, + `author:\tnobody\n`, + `labels:\t\n`, + `assignees:\t\n`, + `reviewers:\t\n`, + `projects:\t\n`, + `milestone:\t\n`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Draft PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewDraftState.json", + expectedOutputs: []string{ + `title:\tBlueberries are from a fork\n`, + `state:\tDRAFT\n`, + `author:\tnobody\n`, + `labels:`, + `assignees:`, + `projects:`, + `milestone:`, + `\*\*blueberries taste good\*\*`, + }, + }, + "Draft PR by branch": { + branch: "master", + args: "blueberries", + fixture: "./fixtures/prViewPreviewDraftStatebyBranch.json", + expectedOutputs: []string{ + `title:\tBlueberries are a good fruit\n`, + `state:\tDRAFT\n`, + `author:\tnobody\n`, + `labels:`, + `assignees:`, + `projects:`, + `milestone:`, + `\*\*blueberries taste good\*\*`, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) + + output, err := runCommand(http, tc.branch, false, tc.args) + if err != nil { + t.Errorf("error running command `%v`: %v", tc.args, err) + } + + eq(t, output.Stderr(), "") + + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } +} + +func TestPRView_Preview(t *testing.T) { + tests := map[string]struct { + branch string + args string + fixture string + expectedOutputs []string + }{ + "Open PR without metadata": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreview.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Open.*nobody wants to merge 12 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "Open PR with metadata by number": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewWithMetadataByNumber.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Open.*nobody wants to merge 12 commits into master from blueberries`, + `Reviewers:.*2 \(.*Approved.*\), 3 \(Commented\), 1 \(.*Requested.*\)\n`, + `Assignees:.*marseilles, monaco\n`, + `Labels:.*one, two, three, four, five\n`, + `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, + `Milestone:.*uluru\n`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, + }, + }, + "Open PR with reviewers by number": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewWithReviewersByNumber.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, + }, + }, + "Open PR with metadata by branch": { + branch: "master", + args: "blueberries", + fixture: "./fixtures/prViewPreviewWithMetadataByBranch.json", + expectedOutputs: []string{ + `Blueberries are a good fruit`, + `Open.*nobody wants to merge 8 commits into master from blueberries`, + `Assignees:.*marseilles, monaco\n`, + `Labels:.*one, two, three, four, five\n`, + `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Milestone:.*uluru\n`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10\n`, + }, + }, + "Open PR for the current branch": { + branch: "blueberries", + args: "", + fixture: "./fixtures/prView.json", + expectedOutputs: []string{ + `Blueberries are a good fruit`, + `Open.*nobody wants to merge 8 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, + }, + }, + "Open PR wth empty body for the current branch": { + branch: "blueberries", + args: "", + fixture: "./fixtures/prView_EmptyBody.json", + expectedOutputs: []string{ + `Blueberries are a good fruit`, + `Open.*nobody wants to merge 8 commits into master from blueberries`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, + }, + }, + "Closed PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewClosedState.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Closed.*nobody wants to merge 12 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "Merged PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewMergedState.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Merged.*nobody wants to merge 12 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "Draft PR": { + branch: "master", + args: "12", + fixture: "./fixtures/prViewPreviewDraftState.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Draft.*nobody wants to merge 12 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "Draft PR by branch": { + branch: "master", + args: "blueberries", + fixture: "./fixtures/prViewPreviewDraftStatebyBranch.json", + expectedOutputs: []string{ + `Blueberries are a good fruit`, + `Draft.*nobody wants to merge 8 commits into master from blueberries`, + `blueberries taste good`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) + + output, err := runCommand(http, tc.branch, true, tc.args) + if err != nil { + t.Errorf("error running command `%v`: %v", tc.args, err) + } + + eq(t, output.Stderr(), "") + + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } +} + +func TestPRView_web_currentBranch(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("./fixtures/prView.json")) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + switch strings.Join(cmd.Args, " ") { + case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: + return &test.OutputStub{} + default: + seenCmd = cmd + return &test.OutputStub{} + } + }) + defer restoreCmd() + + output, err := runCommand(http, "blueberries", true, "-w") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/10 in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + if url != "https://github.com/OWNER/REPO/pull/10" { + t.Errorf("got: %q", url) + } +} + +func TestPRView_web_noResultsForBranch(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.FileResponse("./fixtures/prView_NoActiveBranch.json")) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + switch strings.Join(cmd.Args, " ") { + case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: + return &test.OutputStub{} + default: + seenCmd = cmd + return &test.OutputStub{} + } + }) + defer restoreCmd() + + _, err := runCommand(http, "blueberries", true, "-w") + if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { + t.Errorf("error running command `pr view`: %v", err) + } + + if seenCmd != nil { + t.Fatalf("unexpected command: %v", seenCmd.Args) + } +} + +func TestPRView_web_numberArg(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "url": "https://github.com/OWNER/REPO/pull/23" + } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, "master", true, "-w 23") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/pull/23") +} + +func TestPRView_web_numberArgWithHash(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "url": "https://github.com/OWNER/REPO/pull/23" + } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, "master", true, `-w "#23"`) + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/pull/23") +} + +func TestPRView_web_urlArg(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "url": "https://github.com/OWNER/REPO/pull/23" + } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, "master", true, "-w https://github.com/OWNER/REPO/pull/23/files") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/pull/23") +} + +func TestPRView_web_branchArg(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "headRefName": "blueberries", + "isCrossRepository": false, + "url": "https://github.com/OWNER/REPO/pull/23" } + ] } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, "master", true, "-w blueberries") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/pull/23") +} + +func TestPRView_web_branchWithOwnerArg(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "headRefName": "blueberries", + "isCrossRepository": true, + "headRepositoryOwner": { "login": "hubot" }, + "url": "https://github.com/hubot/REPO/pull/23" } + ] } } } } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, "master", true, "-w hubot:blueberries") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/hubot/REPO/pull/23") +} diff --git a/pkg/text/sanitize.go b/pkg/text/sanitize.go new file mode 100644 index 000000000..16bb902dc --- /dev/null +++ b/pkg/text/sanitize.go @@ -0,0 +1,12 @@ +package text + +import ( + "regexp" + "strings" +) + +var ws = regexp.MustCompile(`\s+`) + +func ReplaceExcessiveWhitespace(s string) string { + return ws.ReplaceAllString(strings.TrimSpace(s), " ") +} diff --git a/pkg/text/sanitize_test.go b/pkg/text/sanitize_test.go new file mode 100644 index 000000000..1c03362d9 --- /dev/null +++ b/pkg/text/sanitize_test.go @@ -0,0 +1,29 @@ +package text + +import "testing" + +func TestReplaceExcessiveWhitespace(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "no replacements", + input: "one two three", + want: "one two three", + }, + { + name: "whitespace b-gone", + input: "\n one\n\t two three\r\n ", + want: "one two three", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ReplaceExcessiveWhitespace(tt.input); got != tt.want { + t.Errorf("ReplaceExcessiveWhitespace() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/test/fixtures/prStatusFork.json b/test/fixtures/prStatusFork.json deleted file mode 100644 index c9a7a5b3a..000000000 --- a/test/fixtures/prStatusFork.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "data": { - "repository": { - "pullRequests": { - "totalCount": 1, - "edges": [ - { - "node": { - "number": 10, - "title": "Blueberries are a good fruit", - "state": "OPEN", - "url": "https://github.com/PARENT/REPO/pull/10", - "headRefName": "blueberries", - "isDraft": false, - "headRepositoryOwner": { - "login": "OWNER" - }, - "isCrossRepository": true - } - } - ] - } - }, - "viewerCreated": { - "totalCount": 0, - "edges": [] - }, - "reviewRequested": { - "totalCount": 0, - "edges": [] - } - } -}