mirror of
https://github.com/github/gh-stack.git
synced 2026-09-14 20:26:28 +08:00
Sameen Karim
4c05e58b83
* Save selected remote to gh-stack.remote git config Users with multiple git remotes are prompted to choose a remote on every gh stack operation, which is tedious. This adds the ability to persist that choice so it only needs to be made once. When a user interactively selects a remote (because multiple remotes exist and none is configured as a push default), they are now shown a Y/n follow-up prompt offering to save that remote for all future gh stack operations. If accepted, the choice is written to the local git config key `gh-stack.remote`, and instructions for changing or clearing it are printed. The saved remote is checked in `ResolveRemote` after the standard git push config keys (branch.<name>.pushRemote, remote.pushDefault, branch.<name>.remote) but before falling back to listing all remotes. This means per-branch git push configuration still takes precedence, and the --remote flag on individual commands continues to override everything. All commands that resolve a remote (push, submit, sync, rebase, checkout, link, modify, trunk) go through the shared `pickRemote` helper, so they all benefit automatically. Changes: - Add GetSavedRemote, SaveRemote, ClearRemote to the git Ops interface, defaultOps implementation, public wrappers, and MockOps - Check gh-stack.remote in ResolveRemote's priority chain - Move pickRemote from push.go to utils.go as a shared helper - Add save-remote confirmation prompt after interactive remote selection - Add unit tests for pickRemote save/decline/skip/override flows - Add integration tests for ResolveRemote with saved remote and precedence, and for the SaveRemote/GetSavedRemote/ClearRemote lifecycle * add error message for save failure Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
407 lines
10 KiB
Go
407 lines
10 KiB
Go
package cmd
|
|
|
|
import (
|
|
"fmt"
|
|
"io"
|
|
"testing"
|
|
|
|
"github.com/github/gh-stack/internal/config"
|
|
"github.com/github/gh-stack/internal/git"
|
|
"github.com/github/gh-stack/internal/github"
|
|
"github.com/github/gh-stack/internal/stack"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// newPushMock creates a MockOps pre-configured for push tests.
|
|
func newPushMock(tmpDir string, currentBranch string) *git.MockOps {
|
|
return &git.MockOps{
|
|
GitDirFn: func() (string, error) { return tmpDir, nil },
|
|
CurrentBranchFn: func() (string, error) { return currentBranch, nil },
|
|
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
|
|
PushFn: func(string, []string, bool, bool) error { return nil },
|
|
}
|
|
}
|
|
|
|
func TestPush_PushesAllBranches(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1"},
|
|
{Branch: "b2"},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
var pushCalls []pushCall
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
|
|
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
|
|
return nil
|
|
}
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
errOut, _ := io.ReadAll(errR)
|
|
output := string(errOut)
|
|
|
|
assert.NoError(t, err)
|
|
require.Len(t, pushCalls, 1)
|
|
assert.Equal(t, "origin", pushCalls[0].remote)
|
|
assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
|
|
assert.True(t, pushCalls[0].force)
|
|
assert.False(t, pushCalls[0].atomic)
|
|
assert.Contains(t, output, "Pushed 2 branches")
|
|
assert.Contains(t, output, "gh stack submit", "should hint about submit when branches have no PRs")
|
|
}
|
|
|
|
func TestPush_NoSubmitHintWhenPRsExist(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
|
|
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{
|
|
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
|
|
switch number {
|
|
case 10:
|
|
return &github.PullRequest{Number: 10, State: "OPEN", HeadRefName: "b1"}, nil
|
|
case 11:
|
|
return &github.PullRequest{Number: 11, State: "OPEN", HeadRefName: "b2"}, nil
|
|
}
|
|
return nil, nil
|
|
},
|
|
}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
errOut, _ := io.ReadAll(errR)
|
|
output := string(errOut)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Contains(t, output, "Pushed 2 branches")
|
|
assert.NotContains(t, output, "gh stack submit", "should not hint about submit when all branches have PRs")
|
|
}
|
|
|
|
func TestPush_SkipsMergedBranches(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
|
|
{Branch: "b2"},
|
|
{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3, Merged: true}},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
var pushCalls []pushCall
|
|
|
|
mock := newPushMock(tmpDir, "b2")
|
|
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
|
|
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
|
|
return nil
|
|
}
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
_, _ = io.ReadAll(errR)
|
|
|
|
assert.NoError(t, err)
|
|
require.Len(t, pushCalls, 1)
|
|
assert.Equal(t, []string{"b2"}, pushCalls[0].branches)
|
|
}
|
|
|
|
func TestPush_PushFailure(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1"},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
mock.PushFn = func(string, []string, bool, bool) error {
|
|
return fmt.Errorf("remote rejected")
|
|
}
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
errOut, _ := io.ReadAll(errR)
|
|
output := string(errOut)
|
|
|
|
assert.ErrorIs(t, err, ErrSilent)
|
|
assert.Contains(t, output, "failed to push")
|
|
}
|
|
|
|
func TestPush_FetchesBeforePush(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1"},
|
|
{Branch: "b2"},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
var callOrder []string
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
mock.FetchBranchesFn = func(remote string, branches []string) error {
|
|
callOrder = append(callOrder, "fetch")
|
|
assert.Equal(t, "origin", remote)
|
|
assert.Equal(t, []string{"b1", "b2"}, branches)
|
|
return nil
|
|
}
|
|
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
|
|
callOrder = append(callOrder, "push")
|
|
return nil
|
|
}
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
_, _ = io.ReadAll(errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, []string{"fetch", "push"}, callOrder, "fetch must happen before push")
|
|
}
|
|
|
|
func TestPush_FetchFailureIsNonFatal(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1"},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
pushCalled := false
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
mock.FetchBranchesFn = func(string, []string) error {
|
|
return fmt.Errorf("network error")
|
|
}
|
|
mock.PushFn = func(string, []string, bool, bool) error {
|
|
pushCalled = true
|
|
return nil
|
|
}
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
errOut, _ := io.ReadAll(errR)
|
|
|
|
assert.NoError(t, err, "fetch failure should not abort push")
|
|
assert.True(t, pushCalled, "push should proceed after fetch failure")
|
|
assert.NotContains(t, string(errOut), "Failed to fetch", "fetch failure should be silent")
|
|
}
|
|
|
|
func TestPush_DoesNotCreatePRs(t *testing.T) {
|
|
s := stack.Stack{
|
|
Trunk: stack.BranchRef{Branch: "main"},
|
|
Branches: []stack.BranchRef{
|
|
{Branch: "b1"},
|
|
{Branch: "b2"},
|
|
},
|
|
}
|
|
|
|
tmpDir := t.TempDir()
|
|
writeStackFile(t, tmpDir, s)
|
|
|
|
mock := newPushMock(tmpDir, "b1")
|
|
|
|
restore := git.SetOps(mock)
|
|
defer restore()
|
|
|
|
createPRCalled := false
|
|
cfg, _, errR := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{
|
|
CreatePRFn: func(string, string, string, string, bool) (*github.PullRequest, error) {
|
|
createPRCalled = true
|
|
return nil, nil
|
|
},
|
|
}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
err := cmd.Execute()
|
|
|
|
cfg.Err.Close()
|
|
_, _ = io.ReadAll(errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.False(t, createPRCalled, "push should not create PRs")
|
|
}
|
|
|
|
func TestPickRemote_SavesWhenConfirmed(t *testing.T) {
|
|
savedRemote := ""
|
|
restore := git.SetOps(&git.MockOps{
|
|
ResolveRemoteFn: func(string) (string, error) {
|
|
return "", &git.ErrMultipleRemotes{Remotes: []string{"origin", "upstream"}}
|
|
},
|
|
SaveRemoteFn: func(r string) error {
|
|
savedRemote = r
|
|
return nil
|
|
},
|
|
})
|
|
defer restore()
|
|
|
|
cfg, outR, errR := config.NewTestConfig()
|
|
cfg.ForceInteractive = true
|
|
cfg.SelectFn = func(prompt, defaultValue string, options []string) (int, error) {
|
|
return 1, nil // select "upstream"
|
|
}
|
|
cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) {
|
|
assert.Contains(t, prompt, "upstream")
|
|
assert.True(t, defaultValue)
|
|
return true, nil
|
|
}
|
|
|
|
remote, err := pickRemote(cfg, "my-branch", "")
|
|
output := collectOutput(cfg, outR, errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, "upstream", remote)
|
|
assert.Equal(t, "upstream", savedRemote)
|
|
assert.Contains(t, output, "Saved")
|
|
assert.Contains(t, output, "git config gh-stack.remote")
|
|
assert.Contains(t, output, "git config --unset gh-stack.remote")
|
|
}
|
|
|
|
func TestPickRemote_SkipsSaveWhenDeclined(t *testing.T) {
|
|
saveCalled := false
|
|
restore := git.SetOps(&git.MockOps{
|
|
ResolveRemoteFn: func(string) (string, error) {
|
|
return "", &git.ErrMultipleRemotes{Remotes: []string{"origin", "upstream"}}
|
|
},
|
|
SaveRemoteFn: func(string) error {
|
|
saveCalled = true
|
|
return nil
|
|
},
|
|
})
|
|
defer restore()
|
|
|
|
cfg, outR, errR := config.NewTestConfig()
|
|
cfg.ForceInteractive = true
|
|
cfg.SelectFn = func(prompt, defaultValue string, options []string) (int, error) {
|
|
return 0, nil // select "origin"
|
|
}
|
|
cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) {
|
|
return false, nil
|
|
}
|
|
|
|
remote, err := pickRemote(cfg, "my-branch", "")
|
|
output := collectOutput(cfg, outR, errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, "origin", remote)
|
|
assert.False(t, saveCalled, "SaveRemote should not be called when user declines")
|
|
assert.NotContains(t, output, "Saved")
|
|
}
|
|
|
|
func TestPickRemote_SkipsPromptWhenSingleRemote(t *testing.T) {
|
|
restore := git.SetOps(&git.MockOps{
|
|
ResolveRemoteFn: func(string) (string, error) {
|
|
return "origin", nil
|
|
},
|
|
})
|
|
defer restore()
|
|
|
|
cfg, outR, errR := config.NewTestConfig()
|
|
|
|
remote, err := pickRemote(cfg, "my-branch", "")
|
|
collectOutput(cfg, outR, errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, "origin", remote)
|
|
}
|
|
|
|
func TestPickRemote_OverrideTakesPrecedence(t *testing.T) {
|
|
resolveCalled := false
|
|
restore := git.SetOps(&git.MockOps{
|
|
ResolveRemoteFn: func(string) (string, error) {
|
|
resolveCalled = true
|
|
return "", fmt.Errorf("should not be called")
|
|
},
|
|
})
|
|
defer restore()
|
|
|
|
cfg, outR, errR := config.NewTestConfig()
|
|
|
|
remote, err := pickRemote(cfg, "my-branch", "custom")
|
|
collectOutput(cfg, outR, errR)
|
|
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, "custom", remote)
|
|
assert.False(t, resolveCalled, "ResolveRemote should not be called when override is provided")
|
|
}
|