feat(pr): merge with the repository's defaults, ready, gh flags and ✓ lines for pr mutations

pr merge takes the method from the repository's default merge style instead of always merging (D22), accepts gh's --merge, --squash and --rebase next to -s/--method, refuses methods the repository disables, and adds -d/--delete-branch (defaulting to the repository setting), --auto, -t, -b, -F and -e. On a terminal it asks for the method and branch deletion with the defaults preselected, without a picker when the current branch has a pull request. It reports "✓ Squashed and merged pull request alice/x#3 (title)", the deleted remote branch, and switches off and deletes the local head branch; --json returns the merged pull request. Merged, closed and draft pull requests are reported before anything is asked. -s without a value points at --squash. pr checkout names the local branch after the head branch when that name is free or already at the head, falling back to OWNER/BRANCH for forks and pr/NUMBER (D13), fetches and switches quietly, reports "✓ Checked out #1 into feat/one", and has the alias co. pr clean reports what it deleted and that a remote branch was already gone. pr create asks for the title and body, then "What's next?": submit, submit as a draft, add metadata or change the branches, without the AGit question up front or a final confirmation. It pushes an unpushed head with --set-upstream --quiet ("✓ Pushed feat/one to origin"), prints "✓ Created pull request alice/x#9 (title)" and the URL, and adds --fill, --web, the alias new, the gh shorthands, and milestones by title. New tea pr ready [--undo]. pr close and reopen take -c/--comment, close takes -d/--delete-branch, and both say when the pull request already is in that state. pr edit reports "✓ Edited", takes the singular gh reviewer flags (the plurals still work), milestones by title and --remove-milestone. pr diff is colored and paged on a terminal and byte-exact when piped, with --name-only, --color and --web. Every pull request argument accepts #N, URLs and branch names and defaults to the current branch's pull request. Signed-off-by: bircni <bircni@icloud.com>
2026-10-26 11:01:25 +01:00 · 2026-10-11 23:55:11 +02:00
1 parent 2c5e005bcf
commit 4895aa089d
@@ -5,69 +5,197 @@ package app

 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"testing"

+	"gitea.dev/tea/internal/clierr"
 	"gitea.dev/tea/internal/terminal"
 	"github.com/stretchr/testify/require"
 )

-type mergeFormFunc func(context.Context, []terminal.MergePullChoice) (int64, error)
-
-func (f mergeFormFunc) SelectPull(ctx context.Context, choices []terminal.MergePullChoice) (int64, error) {
-	return f(ctx, choices)
+// mergeForm answers the merge form: pick selects among pull requests, options returns the merge options.
+type mergeForm struct {
+	pick    func([]terminal.MergePullChoice) int64
+	options func(terminal.MergeFormChoices) terminal.MergeFormValues
+	asked   []terminal.MergeFormChoices
 }

-func TestPullMergeInteractiveSelectsMatchingBranch(t *testing.T) {
-	merges := 0
-	server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+func (f *mergeForm) SelectPull(_ context.Context, choices []terminal.MergePullChoice) (int64, error) {
+	return f.pick(choices), nil
+}
+
+func (f *mergeForm) MergeOptions(_ context.Context, choices terminal.MergeFormChoices) (terminal.MergeFormValues, error) {
+	f.asked = append(f.asked, choices)
+	return f.options(choices), nil
+}
+
+// mergeRepository allows merge, rebase and squash merges in alice/project and squashes and deletes
+// branches by default.
+const mergeRepository = `{"full_name":"alice/project","allow_merge_commits":true,"allow_rebase":true,"allow_squash_merge":true,"default_merge_style":"squash","default_delete_branch_after_merge":true}`
+
+// mergeServer serves pull request 3, "Add login" from feature into main, and records merges into
+// payload. After a merge the pull request reads as merged with its head branch deleted.
+func mergeServer(t *testing.T, pull string, payload *map[string]any) http.Handler {
+	t.Helper()
+	merged := false
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch {
-		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/pulls":
-			require.Equal(t, "open", r.URL.Query().Get("state"))
+		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project":
+			_, _ = w.Write([]byte(mergeRepository))
+		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/pulls/3" && merged:
+			_, _ = w.Write([]byte(`{"number":3,"title":"Add login","state":"closed","merged":true,"base":{"ref":"main"},"head":{"ref":"refs/pull/3/head","label":"feature","sha":"abc123"}}`))
+		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/pulls/3":
+			_, _ = w.Write([]byte(pull))
+		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/alice/project/pulls/3/merge":
+			merged = true
+			require.NoError(t, json.NewDecoder(r.Body).Decode(payload))
+		default:
+			t.Errorf("unexpected %s %s", r.Method, r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+		}
+	})
+}
+
+const openPull = `{"number":3,"title":"Add login","state":"open","base":{"ref":"main"},"head":{"ref":"feature","sha":"abc123","repo":{"full_name":"alice/project","permissions":{"push":true}}}}`
+
+func TestPullMergeUsesTheRepositoryDefaults(t *testing.T) {
+	var payload map[string]any
+	server := newAuthenticatedServer(mergeServer(t, openPull, &payload))
+	defer server.Close()
+	deps, out, diagnostics := serverDeps(t, server.URL)
+	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge", "3", "-R", "alice/project", "--yes"}))
+	require.Equal(t, "squash", payload["Do"])
+	require.Equal(t, true, payload["delete_branch_after_merge"])
+	require.Empty(t, out.String())
+	require.Empty(t, diagnostics.String())
+}
+
+func TestPullMergeAsksForTheMethodAndBranchDeletion(t *testing.T) {
+	var payload map[string]any
+	server := newAuthenticatedServer(mergeServer(t, openPull, &payload))
+	defer server.Close()
+	form := &mergeForm{options: func(choices terminal.MergeFormChoices) terminal.MergeFormValues {
+		return terminal.MergeFormValues{Method: "rebase", DeleteBranch: false}
+	}}
+	prompt := &askingPrompter{}
+	deps, _, diagnostics := serverDeps(t, server.URL)
+	deps.MergeForm, deps.Prompter = form, prompt
+	deps.Streams.InTTY, deps.Streams.ErrTTY, deps.Streams.OutTTY = true, true, true
+	deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"})
+	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge", "3", "-R", "alice/project"}))
+	require.Equal(t, []terminal.MergeFormChoices{{
+		Header:  `Merge #3 "Add login" (feature → main) in alice/project`,
+		Methods: []string{"merge", "rebase", "squash"}, Method: "squash", DeleteBranch: true, Branch: "feature",
+	}}, form.asked)
+	require.Empty(t, prompt.confirmations, "the form is the confirmation")
+	require.Equal(t, "rebase", payload["Do"])
+	require.Equal(t, false, payload["delete_branch_after_merge"])
+	require.Equal(t, "✓ Rebased and merged pull request alice/project#3 (Add login)\n", diagnostics.String())
+
+	diagnostics.Reset()
+	payload = nil
+	server2 := newAuthenticatedServer(mergeServer(t, openPull, &payload))
+	defer server2.Close()
+	deps.Getenv = serverEnv(server2.URL, map[string]string{"NO_COLOR": "1"})
+	prompt.accept = true
+	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge", "3", "-R", "alice/project", "--squash", "-d"}))
+	require.Len(t, form.asked, 1, "flags answer the form")
+	require.Equal(t, []string{`Merge #3 "Add login" (feature → main) in alice/project?`}, prompt.confirmations)
+	require.Equal(t, []bool{true}, prompt.defaults)
+	require.Equal(t, "✓ Squashed and merged pull request alice/project#3 (Add login)\n✓ Deleted remote branch feature\n", diagnostics.String())
+}
+
+func TestPullMergeRefusals(t *testing.T) {
+	for _, tc := range []struct {
+		name, pull string
+		args       []string
+		want       string
+		code       int
+	}{
+		{"draft", `{"number":3,"title":"WIP: x","state":"open","draft":true}`, nil, "pull request alice/project#3 (WIP: x) is a draft; mark it ready with: tea pr ready 3", clierr.Failure},
+		{"closed", `{"number":3,"title":"Old","state":"closed"}`, nil, "pull request alice/project#3 (Old) is closed; reopen it with: tea pr reopen 3", clierr.Failure},
+		{"disallowed method", openPull, []string{"--method", "fast-forward-only"}, "fast-forward-only merges are disabled in alice/project; use one of: merge, rebase, squash", clierr.Usage},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var payload map[string]any
+			server := newAuthenticatedServer(mergeServer(t, tc.pull, &payload))
+			defer server.Close()
+			deps, _, _ := serverDeps(t, server.URL)
+			err := Execute(context.Background(), deps, append([]string{"pr", "merge", "3", "-R", "alice/project", "--yes"}, tc.args...))
+			require.EqualError(t, err, tc.want)
+			require.Equal(t, tc.code, clierr.ExitCode(err))
+			require.Nil(t, payload)
+		})
+	}
+}
+
+func TestPullMergeAlreadyMergedIsANoOp(t *testing.T) {
+	var payload map[string]any
+	server := newAuthenticatedServer(mergeServer(t, `{"number":3,"title":"Done","state":"closed","merged":true}`, &payload))
+	defer server.Close()
+	deps, _, diagnostics := serverDeps(t, server.URL)
+	deps.Streams.OutTTY = true
+	deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"})
+	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge", "3", "-R", "alice/project"}))
+	require.Equal(t, "! Pull request alice/project#3 (Done) is already merged\n", diagnostics.String())
+	require.Nil(t, payload)
+}
+
+func TestPullMergeMethodFlags(t *testing.T) {
+	for _, tc := range []struct {
+		args []string
+		want string
+	}{
+		{[]string{"pr", "merge", "3", "--squash", "--rebase"}, "choose one merge method: --merge, --squash, --rebase or --method"},
+		{[]string{"pr", "merge", "3", "--squash", "-s", "merge"}, "choose one merge method"},
+		{[]string{"pr", "merge", "3", "-s"}, "-s takes a method (merge, rebase, rebase-merge, squash or fast-forward-only); for gh-style use --squash"},
+		{[]string{"pr", "merge", "3", "-s", "bogus"}, "--method must be merge, rebase, rebase-merge, squash, or fast-forward-only"},
+	} {
+		t.Run(tc.args[len(tc.args)-1], func(t *testing.T) {
+			requireUsageBeforeRequests(t, tc.want, tc.args...)
+		})
+	}
+}
+
+func TestPullMergePicksAPullRequestForTheBranch(t *testing.T) {
+	var payload map[string]any
+	pulls := mergeServer(t, openPull, &payload)
+	server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/api/v1/repos/alice/project/pulls" {
 			if r.URL.Query().Get("page") != "1" {
 				_, _ = w.Write([]byte(`[]`))
 				return
 			}
 			_, _ = w.Write([]byte(`[
-				{"number":3,"title":"Other","state":"open","base":{"ref":"main"},"head":{"ref":"other","label":"alice:other"}},
-				{"number":7,"title":"Feature","state":"open","base":{"ref":"main"},"head":{"ref":"feature","label":"alice:feature"}},
-				{"number":9,"title":"Into feature","state":"open","base":{"ref":"feature"},"head":{"ref":"topic","label":"alice:topic"}}
+				{"number":3,"title":"Add login","state":"open","base":{"ref":"main"},"head":{"ref":"feature"}},
+				{"number":9,"title":"Other","state":"open","base":{"ref":"main"},"head":{"ref":"other"}}
 			]`))
-		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/pulls/7":
-			_, _ = w.Write([]byte(`{"number":7,"head":{"ref":"feature","sha":"abc123"}}`))
-		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/alice/project/pulls/7/merge":
-			merges++
-			w.WriteHeader(http.StatusOK)
-		default:
-			t.Errorf("unexpected %s %s", r.Method, r.URL.Path)
-			w.WriteHeader(http.StatusNotFound)
+			return
 		}
+		pulls.ServeHTTP(w, r)
 	}))
 	defer server.Close()
-	calls := 0
-	prompt := &scriptedPrompter{accept: true}
+	form := &mergeForm{
+		pick: func(choices []terminal.MergePullChoice) int64 {
+			require.Equal(t, []terminal.MergePullChoice{{Number: 3, Title: "Add login", Head: "feature"}, {Number: 9, Title: "Other", Head: "other"}}, choices)
+			return 3
+		},
+		options: func(choices terminal.MergeFormChoices) terminal.MergeFormValues {
+			return terminal.MergeFormValues{Method: choices.Method, DeleteBranch: choices.DeleteBranch}
+		},
+	}
 	deps, _, _ := serverDeps(t, server.URL)
-	deps.MergeForm = mergeFormFunc(func(_ context.Context, choices []terminal.MergePullChoice) (int64, error) {
-		calls++
-		require.Len(t, choices, 2)
-		require.EqualValues(t, 7, choices[0].Number)
-		require.EqualValues(t, 9, choices[1].Number)
-		return 7, nil
-	})
-	deps.Prompter = prompt
+	deps.Getenv = serverEnv(server.URL, map[string]string{"TEA_REPO": "alice/project"})
+	deps.MergeForm, deps.Prompter = form, &askingPrompter{}
 	deps.Streams.InTTY, deps.Streams.ErrTTY = true, true
-	deps.Git = &fakeGit{results: map[string]string{"symbolic-ref --short HEAD": "feature"}}
-	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge", "-R", "alice/project", "--interactive", "--yes"}))
-	require.Equal(t, 1, calls)
-	require.Equal(t, 1, merges)
+	deps.Git = &fakeGit{results: map[string]string{"symbolic-ref --short HEAD": "main"}}
+	require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge"}))
+	require.Equal(t, "squash", payload["Do"])
 }

 func TestPullMergeRequiresNumberWithoutInteractive(t *testing.T) {
 	deps, _, _ := testDeps(t)
-	deps.MergeForm = mergeFormFunc(func(context.Context, []terminal.MergePullChoice) (int64, error) {
-		t.Fatal("merge form should not run")
-		return 0, nil
-	})
 	err := Execute(context.Background(), deps, []string{"pr", "merge", "-R", "alice/project", "--no-input"})
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "pull request number")