mirror of
https://github.com/github/gh-stack.git
synced 2026-09-14 20:26:28 +08:00
Emily Wise
36b278467b
Expose --atomic for multi-ref pushes through the shared Git push infrastructure. Keep push non-atomic by default and preserve sync's existing atomic default with --atomic=false available as an opt-out. Document the command behavior, audit link and submit push paths, and add unit and integration coverage for atomic rejection semantics.
443 lines
11 KiB
Go
443 lines
11 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_Atomic(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, _, _ := config.NewTestConfig()
|
|
cfg.GitHubClientOverride = &github.MockClient{}
|
|
cmd := PushCmd(cfg)
|
|
cmd.SetArgs([]string{"--atomic"})
|
|
cmd.SetOut(io.Discard)
|
|
cmd.SetErr(io.Discard)
|
|
|
|
require.NoError(t, cmd.Execute())
|
|
require.Len(t, pushCalls, 1)
|
|
assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
|
|
assert.True(t, pushCalls[0].force)
|
|
assert.True(t, pushCalls[0].atomic)
|
|
}
|
|
|
|
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")
|
|
}
|