refactor sync test

pkg/commands/sync.go

@@ -4,18 +4,18 @@ import (
 	"fmt"
 
 	"github.com/go-errors/errors"
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 )
 
 // Push pushes to a branch
 type PushOpts struct {
-	Force                   bool
-	UpstreamRemote          string
-	UpstreamBranch          string
-	SetUpstream             bool
-	PromptUserForCredential func(string) string
+	Force          bool
+	UpstreamRemote string
+	UpstreamBranch string
+	SetUpstream    bool
 }
 
-func (c *GitCommand) Push(opts PushOpts) error {
+func (c *GitCommand) PushCmdObj(opts PushOpts) (oscommands.ICmdObj, error) {
 	cmdStr := "git push"
 
 	if opts.Force {
@@ -32,13 +32,22 @@ func (c *GitCommand) Push(opts PushOpts) error {
 
 	if opts.UpstreamBranch != "" {
 		if opts.UpstreamRemote == "" {
-			return errors.New(c.Tr.MustSpecifyOriginError)
+			return nil, errors.New(c.Tr.MustSpecifyOriginError)
 		}
 		cmdStr += " " + c.OSCommand.Quote(opts.UpstreamBranch)
 	}
 
 	cmdObj := c.Cmd.New(cmdStr)
-	return c.DetectUnamePass(cmdObj, opts.PromptUserForCredential)
+	return cmdObj, nil
+}
+
+func (c *GitCommand) Push(opts PushOpts, promptUserForCredential func(string) string) error {
+	cmdObj, err := c.PushCmdObj(opts)
+	if err != nil {
+		return err
+	}
+
+	return c.DetectUnamePass(cmdObj, promptUserForCredential)
 }
 
 type FetchOptions struct {

pkg/commands/sync_test.go

@@ -1,164 +1,93 @@
 package commands
 
 import (
-	"os/exec"
 	"testing"
 
-	"github.com/jesseduffield/lazygit/pkg/secureexec"
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/stretchr/testify/assert"
 )
 
-// TestGitCommandPush is a function.
 func TestGitCommandPush(t *testing.T) {
 	type scenario struct {
 		testName string
-		command  func(string, ...string) *exec.Cmd
 		opts     PushOpts
-		test     func(error)
-	}
-
-	prompt := func(passOrUname string) string {
-		return "\n"
+		test     func(oscommands.ICmdObj, error)
 	}
 
 	scenarios := []scenario{
 		{
-			"Push with force disabled",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push"}, args)
-
-				return secureexec.Command("echo")
-			},
-			PushOpts{Force: false, PromptUserForCredential: prompt},
-			func(err error) {
+			testName: "Push with force disabled",
+			opts:     PushOpts{Force: false},
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), "git push")
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Push with force enabled",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push", "--force-with-lease"}, args)
-
-				return secureexec.Command("echo")
-			},
-			PushOpts{Force: true, PromptUserForCredential: prompt},
-			func(err error) {
+			testName: "Push with force enabled",
+			opts:     PushOpts{Force: true},
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), "git push --force-with-lease")
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Push with an error occurring",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push"}, args)
-				return secureexec.Command("test")
+			testName: "Push with force disabled, upstream supplied",
+			opts: PushOpts{
+				Force:          false,
+				UpstreamRemote: "origin",
+				UpstreamBranch: "master",
 			},
-			PushOpts{Force: false, PromptUserForCredential: prompt},
-			func(err error) {
-				assert.Error(t, err)
-			},
-		},
-		{
-			"Push with force disabled, upstream supplied",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push", "origin", "master"}, args)
-
-				return secureexec.Command("echo")
-			},
-			PushOpts{
-				Force:                   false,
-				UpstreamRemote:          "origin",
-				UpstreamBranch:          "master",
-				PromptUserForCredential: prompt,
-			},
-			func(err error) {
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), `git push "origin" "master"`)
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Push with force disabled, setting upstream",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push", "--set-upstream", "origin", "master"}, args)
-
-				return secureexec.Command("echo")
+			testName: "Push with force disabled, setting upstream",
+			opts: PushOpts{
+				Force:          false,
+				UpstreamRemote: "origin",
+				UpstreamBranch: "master",
+				SetUpstream:    true,
 			},
-			PushOpts{
-				Force:                   false,
-				UpstreamRemote:          "origin",
-				UpstreamBranch:          "master",
-				PromptUserForCredential: prompt,
-				SetUpstream:             true,
-			},
-			func(err error) {
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), `git push --set-upstream "origin" "master"`)
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Push with force enabled, setting upstream",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push", "--force-with-lease", "--set-upstream", "origin", "master"}, args)
-
-				return secureexec.Command("echo")
+			testName: "Push with force enabled, setting upstream",
+			opts: PushOpts{
+				Force:          true,
+				UpstreamRemote: "origin",
+				UpstreamBranch: "master",
+				SetUpstream:    true,
 			},
-			PushOpts{
-				Force:                   true,
-				UpstreamRemote:          "origin",
-				UpstreamBranch:          "master",
-				PromptUserForCredential: prompt,
-				SetUpstream:             true,
-			},
-			func(err error) {
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), `git push --force-with-lease --set-upstream "origin" "master"`)
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Push with remote branch but no origin",
-			func(cmd string, args ...string) *exec.Cmd {
-				return nil
+			testName: "Push with remote branch but no origin",
+			opts: PushOpts{
+				Force:          true,
+				UpstreamRemote: "",
+				UpstreamBranch: "master",
+				SetUpstream:    true,
 			},
-			PushOpts{
-				Force:                   true,
-				UpstreamRemote:          "",
-				UpstreamBranch:          "master",
-				PromptUserForCredential: prompt,
-				SetUpstream:             true,
-			},
-			func(err error) {
+			test: func(cmdObj oscommands.ICmdObj, err error) {
 				assert.Error(t, err)
 				assert.EqualValues(t, "Must specify a remote if specifying a branch", err.Error())
 			},
 		},
-		{
-			"Push with force disabled, upstream supplied",
-			func(cmd string, args ...string) *exec.Cmd {
-				assert.EqualValues(t, "git", cmd)
-				assert.EqualValues(t, []string{"push", "origin", "master"}, args)
-
-				return secureexec.Command("echo")
-			},
-			PushOpts{
-				Force:                   false,
-				UpstreamRemote:          "origin",
-				UpstreamBranch:          "master",
-				PromptUserForCredential: prompt,
-			},
-			func(err error) {
-				assert.NoError(t, err)
-			},
-		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.testName, func(t *testing.T) {
-			gitCmd := NewDummyGitCommand()
-			gitCmd.OSCommand.Command = s.command
-			err := gitCmd.Push(s.opts)
-			s.test(err)
+			gitCmd := NewDummyGitCommandWithRunner(oscommands.NewFakeRunner(t))
+			s.test(gitCmd.PushCmdObj(s.opts))
 		})
 	}
 }

pkg/gui/files_panel.go

@@ -736,12 +736,11 @@ func (gui *Gui) push(opts pushOpts) error {
 	}
 	go utils.Safe(func() {
 		err := gui.GitCommand.WithSpan(gui.Tr.Spans.Push).Push(commands.PushOpts{
-			Force:                   opts.force,
-			UpstreamRemote:          opts.upstreamRemote,
-			UpstreamBranch:          opts.upstreamBranch,
-			SetUpstream:             opts.setUpstream,
-			PromptUserForCredential: gui.promptUserForCredential,
-		})
+			Force:          opts.force,
+			UpstreamRemote: opts.upstreamRemote,
+			UpstreamBranch: opts.upstreamBranch,
+			SetUpstream:    opts.setUpstream,
+		}, gui.promptUserForCredential)
 
 		if err != nil && !opts.force && strings.Contains(err.Error(), "Updates were rejected") {
 			forcePushDisabled := gui.UserConfig.Git.DisableForcePushing