Merge pull request #277 from antham/add-tests-part-3

Add tests to pkg/commands/git - Part 3

pkg/commands/git.go

@@ -7,7 +7,6 @@ import (
 	"os/exec"
 	"strings"
 
-	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/i18n"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/sirupsen/logrus"
@@ -59,11 +58,13 @@ func setupRepositoryAndWorktree(openGitRepository func(string) (*gogit.Repositor
 
 // GitCommand is our main git interface
 type GitCommand struct {
-	Log       *logrus.Entry
-	OSCommand *OSCommand
-	Worktree  *gogit.Worktree
-	Repo      *gogit.Repository
-	Tr        *i18n.Localizer
+	Log                *logrus.Entry
+	OSCommand          *OSCommand
+	Worktree           *gogit.Worktree
+	Repo               *gogit.Repository
+	Tr                 *i18n.Localizer
+	getGlobalGitConfig func(string) (string, error)
+	getLocalGitConfig  func(string) (string, error)
 }
 
 // NewGitCommand it runs git commands
@@ -92,11 +93,13 @@ func NewGitCommand(log *logrus.Entry, osCommand *OSCommand, tr *i18n.Localizer)
 	}
 
 	return &GitCommand{
-		Log:       log,
-		OSCommand: osCommand,
-		Tr:        tr,
-		Worktree:  worktree,
-		Repo:      repo,
+		Log:                log,
+		OSCommand:          osCommand,
+		Tr:                 tr,
+		Worktree:           worktree,
+		Repo:               repo,
+		getGlobalGitConfig: gitconfig.Global,
+		getLocalGitConfig:  gitconfig.Local,
 	}, nil
 }
 
@@ -274,40 +277,41 @@ func (c *GitCommand) AbortMerge() error {
 	return c.OSCommand.RunCommand("git merge --abort")
 }
 
-// UsingGpg tells us whether the user has gpg enabled so that we can know
+// usingGpg tells us whether the user has gpg enabled so that we can know
 // whether we need to run a subprocess to allow them to enter their password
-func (c *GitCommand) UsingGpg() bool {
-	gpgsign, _ := gitconfig.Global("commit.gpgsign")
+func (c *GitCommand) usingGpg() bool {
+	gpgsign, _ := c.getLocalGitConfig("commit.gpgsign")
 	if gpgsign == "" {
-		gpgsign, _ = gitconfig.Local("commit.gpgsign")
+		gpgsign, _ = c.getGlobalGitConfig("commit.gpgsign")
 	}
-	if gpgsign == "" {
-		return false
-	}
-	return true
+	value := strings.ToLower(gpgsign)
+
+	return value == "true" || value == "1" || value == "yes" || value == "on"
 }
 
-// Commit commit to git
-func (c *GitCommand) Commit(g *gocui.Gui, message string) (*exec.Cmd, error) {
-	command := "git commit -m " + c.OSCommand.Quote(message)
-	if c.UsingGpg() {
+// Commit commits to git
+func (c *GitCommand) Commit(message string) (*exec.Cmd, error) {
+	command := fmt.Sprintf("git commit -m %s", c.OSCommand.Quote(message))
+	if c.usingGpg() {
 		return c.OSCommand.PrepareSubProcess(c.OSCommand.Platform.shell, c.OSCommand.Platform.shellArg, command), nil
 	}
+
 	return nil, c.OSCommand.RunCommand(command)
 }
 
-// Pull pull from repo
+// Pull pulls from repo
 func (c *GitCommand) Pull() error {
 	return c.OSCommand.RunCommand("git pull --no-edit")
 }
 
-// Push push to a branch
+// Push pushes to a branch
 func (c *GitCommand) Push(branchName string, force bool) error {
 	forceFlag := ""
 	if force {
 		forceFlag = "--force-with-lease "
 	}
-	return c.OSCommand.RunCommand("git push " + forceFlag + "-u origin " + branchName)
+
+	return c.OSCommand.RunCommand(fmt.Sprintf("git push %s -u origin %s", forceFlag, branchName))
 }
 
 // SquashPreviousTwoCommits squashes a commit down to the one below it

pkg/commands/git_test.go

@@ -56,9 +56,11 @@ func newDummyLog() *logrus.Entry {
 
 func newDummyGitCommand() *GitCommand {
 	return &GitCommand{
-		Log:       newDummyLog(),
-		OSCommand: newDummyOSCommand(),
-		Tr:        i18n.NewLocalizer(newDummyLog()),
+		Log:                newDummyLog(),
+		OSCommand:          newDummyOSCommand(),
+		Tr:                 i18n.NewLocalizer(newDummyLog()),
+		getGlobalGitConfig: func(string) (string, error) { return "", nil },
+		getLocalGitConfig:  func(string) (string, error) { return "", nil },
 	}
 }
 
@@ -730,6 +732,227 @@ func TestGitCommandMerge(t *testing.T) {
 	assert.NoError(t, gitCmd.Merge("test"))
 }
 
+func TestGitCommandUsingGpg(t *testing.T) {
+	type scenario struct {
+		testName           string
+		getLocalGitConfig  func(string) (string, error)
+		getGlobalGitConfig func(string) (string, error)
+		test               func(bool)
+	}
+
+	scenarios := []scenario{
+		{
+			"Option global and local config commit.gpgsign is not set",
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(gpgEnabled bool) {
+				assert.False(t, gpgEnabled)
+			},
+		},
+		{
+			"Option global config commit.gpgsign is not set, fallback on local config",
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(string) (string, error) {
+				return "true", nil
+			},
+			func(gpgEnabled bool) {
+				assert.True(t, gpgEnabled)
+			},
+		},
+		{
+			"Option commit.gpgsign is true",
+			func(string) (string, error) {
+				return "True", nil
+			},
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(gpgEnabled bool) {
+				assert.True(t, gpgEnabled)
+			},
+		},
+		{
+			"Option commit.gpgsign is on",
+			func(string) (string, error) {
+				return "ON", nil
+			},
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(gpgEnabled bool) {
+				assert.True(t, gpgEnabled)
+			},
+		},
+		{
+			"Option commit.gpgsign is yes",
+			func(string) (string, error) {
+				return "YeS", nil
+			},
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(gpgEnabled bool) {
+				assert.True(t, gpgEnabled)
+			},
+		},
+		{
+			"Option commit.gpgsign is 1",
+			func(string) (string, error) {
+				return "1", nil
+			},
+			func(string) (string, error) {
+				return "", nil
+			},
+			func(gpgEnabled bool) {
+				assert.True(t, gpgEnabled)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.testName, func(t *testing.T) {
+			gitCmd := newDummyGitCommand()
+			gitCmd.getGlobalGitConfig = s.getGlobalGitConfig
+			gitCmd.getLocalGitConfig = s.getLocalGitConfig
+			s.test(gitCmd.usingGpg())
+		})
+	}
+}
+
+func TestGitCommandCommit(t *testing.T) {
+	type scenario struct {
+		testName           string
+		command            func(string, ...string) *exec.Cmd
+		getGlobalGitConfig func(string) (string, error)
+		test               func(*exec.Cmd, error)
+	}
+
+	scenarios := []scenario{
+		{
+			"Commit using gpg",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "bash", cmd)
+				assert.EqualValues(t, []string{"-c", `git commit -m 'test'`}, args)
+
+				return exec.Command("echo")
+			},
+			func(string) (string, error) {
+				return "true", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NotNil(t, cmd)
+				assert.Nil(t, err)
+			},
+		},
+		{
+			"Commit without using gpg",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "git", cmd)
+				assert.EqualValues(t, []string{"commit", "-m", "test"}, args)
+
+				return exec.Command("echo")
+			},
+			func(string) (string, error) {
+				return "false", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.Nil(t, cmd)
+				assert.Nil(t, err)
+			},
+		},
+		{
+			"Commit without using gpg with an error",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "git", cmd)
+				assert.EqualValues(t, []string{"commit", "-m", "test"}, args)
+
+				return exec.Command("exit", "1")
+			},
+			func(string) (string, error) {
+				return "false", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.Nil(t, cmd)
+				assert.Error(t, err)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.testName, func(t *testing.T) {
+			gitCmd := newDummyGitCommand()
+			gitCmd.getGlobalGitConfig = s.getGlobalGitConfig
+			gitCmd.OSCommand.command = s.command
+			s.test(gitCmd.Commit("test"))
+		})
+	}
+}
+
+func TestGitCommandPush(t *testing.T) {
+	type scenario struct {
+		testName  string
+		command   func(string, ...string) *exec.Cmd
+		forcePush bool
+		test      func(error)
+	}
+
+	scenarios := []scenario{
+		{
+			"Push with force disabled",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "git", cmd)
+				assert.EqualValues(t, []string{"push", "-u", "origin", "test"}, args)
+
+				return exec.Command("echo")
+			},
+			false,
+			func(err error) {
+				assert.Nil(t, err)
+			},
+		},
+		{
+			"Push with force enable",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "git", cmd)
+				assert.EqualValues(t, []string{"push", "--force-with-lease", "-u", "origin", "test"}, args)
+
+				return exec.Command("echo")
+			},
+			true,
+			func(err error) {
+				assert.Nil(t, err)
+			},
+		},
+		{
+			"Push with an error occurring",
+			func(cmd string, args ...string) *exec.Cmd {
+				assert.EqualValues(t, "git", cmd)
+				assert.EqualValues(t, []string{"push", "-u", "origin", "test"}, args)
+
+				return exec.Command("exit", "1")
+			},
+			false,
+			func(err error) {
+				assert.Error(t, err)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.testName, func(t *testing.T) {
+			gitCmd := newDummyGitCommand()
+			gitCmd.OSCommand.command = s.command
+			s.test(gitCmd.Push("test", s.forcePush))
+		})
+	}
+}
+
 func TestGitCommandDiff(t *testing.T) {
 	gitCommand := newDummyGitCommand()
 	assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh"))

pkg/gui/commit_message_panel.go

@@ -12,7 +12,7 @@ func (gui *Gui) handleCommitConfirm(g *gocui.Gui, v *gocui.View) error {
 	if message == "" {
 		return gui.createErrorPanel(g, gui.Tr.SLocalize("CommitWithoutMessageErr"))
 	}
-	sub, err := gui.GitCommand.Commit(g, message)
+	sub, err := gui.GitCommand.Commit(message)
 	if err != nil {
 		// TODO need to find a way to send through this error
 		if err != gui.Errors.ErrSubProcess {