From 5c204b28136d3f13baec64868e20a19685253fbb Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Mon, 10 Sep 2018 21:57:08 +0200 Subject: [PATCH 1/8] commands/git: rewrite UsingGpg, add tests --- pkg/commands/git.go | 41 ++++++++-------- pkg/commands/git_test.go | 101 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index f7a80dc63..2945e1e92 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -59,11 +59,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 +94,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,23 +278,22 @@ 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.getGlobalGitConfig("commit.gpgsign") if gpgsign == "" { - gpgsign, _ = gitconfig.Local("commit.gpgsign") + gpgsign, _ = c.getLocalGitConfig("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() { + if c.usingGpg() { return c.OSCommand.PrepareSubProcess(c.OSCommand.Platform.shell, c.OSCommand.Platform.shellArg, command), nil } return nil, c.OSCommand.RunCommand(command) diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 7bc7a8910..5d451174d 100644 --- a/pkg/commands/git_test.go +++ b/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,99 @@ func TestGitCommandMerge(t *testing.T) { assert.NoError(t, gitCmd.Merge("test")) } +func TestGitCommandUsingGpg(t *testing.T) { + type scenario struct { + testName string + getGlobalGitConfig func(string) (string, error) + getLocalGitConfig 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 TestGitCommandDiff(t *testing.T) { gitCommand := newDummyGitCommand() assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh")) From d23577168f46f249e2af629ed23f15ddfd2cef6a Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Mon, 10 Sep 2018 22:01:52 +0200 Subject: [PATCH 2/8] commands/git : remove dependency on gocui --- pkg/commands/git.go | 5 ++--- pkg/gui/commit_message_panel.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 2945e1e92..18f698e30 100644 --- a/pkg/commands/git.go +++ b/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" @@ -290,8 +289,8 @@ func (c *GitCommand) usingGpg() bool { 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) { +// Commit commits to git +func (c *GitCommand) Commit(message string) (*exec.Cmd, error) { command := "git commit -m " + c.OSCommand.Quote(message) if c.usingGpg() { return c.OSCommand.PrepareSubProcess(c.OSCommand.Platform.shell, c.OSCommand.Platform.shellArg, command), nil diff --git a/pkg/gui/commit_message_panel.go b/pkg/gui/commit_message_panel.go index 99d649102..e23c47da0 100644 --- a/pkg/gui/commit_message_panel.go +++ b/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 { From 415aad600c9ec5db052e4e99c556a9a4f38df8cf Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Mon, 10 Sep 2018 22:25:02 +0200 Subject: [PATCH 3/8] commands/git : add test to Commit func, refactor --- pkg/commands/git.go | 3 +- pkg/commands/git_test.go | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 18f698e30..edaf65ced 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -291,10 +291,11 @@ func (c *GitCommand) usingGpg() bool { // Commit commits to git func (c *GitCommand) Commit(message string) (*exec.Cmd, error) { - command := "git commit -m " + c.OSCommand.Quote(message) + 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) } diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 5d451174d..fbc953ec2 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -825,6 +825,75 @@ func TestGitCommandUsingGpg(t *testing.T) { } } +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 TestGitCommandDiff(t *testing.T) { gitCommand := newDummyGitCommand() assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh")) From ccbc5e569c27b4b7a2160f07a63b6ba0cc3ae17a Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Mon, 10 Sep 2018 22:38:08 +0200 Subject: [PATCH 4/8] commands/git : add test to Push func, refactor --- pkg/commands/git.go | 6 ++-- pkg/commands/git_test.go | 59 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index edaf65ced..a770c03ee 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -304,13 +304,15 @@ 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 diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index fbc953ec2..c90ce324c 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -894,6 +894,65 @@ func TestGitCommandCommit(t *testing.T) { } } +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")) From 0aba49af2ba7a3425d409809ed649cba7a9d6e50 Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Mon, 10 Sep 2018 22:38:25 +0200 Subject: [PATCH 5/8] commands/git : fix typo --- pkg/commands/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index a770c03ee..4eb62a7fd 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -299,7 +299,7 @@ func (c *GitCommand) Commit(message string) (*exec.Cmd, error) { 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") } From f03544f392edb2e7be016f9c68d21d259da96acd Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Tue, 11 Sep 2018 22:20:59 +0200 Subject: [PATCH 6/8] commands/git : fix test --- pkg/commands/git_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index c90ce324c..18d90b765 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -838,7 +838,7 @@ func TestGitCommandCommit(t *testing.T) { "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) + assert.EqualValues(t, []string{"-c", `git commit -m 'test'`}, args) return exec.Command("echo") }, From 9bad0337fe0c52d0fe6c91d0135201a4570b0c7e Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Wed, 12 Sep 2018 07:50:49 +0200 Subject: [PATCH 7/8] commands/git : swap global/local get config --- pkg/commands/git.go | 4 ++-- pkg/commands/git_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 4eb62a7fd..ecbd702c1 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -280,9 +280,9 @@ func (c *GitCommand) AbortMerge() error { // 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, _ := c.getGlobalGitConfig("commit.gpgsign") + gpgsign, _ := c.getLocalGitConfig("commit.gpgsign") if gpgsign == "" { - gpgsign, _ = c.getLocalGitConfig("commit.gpgsign") + gpgsign, _ = c.getGlobalGitConfig("commit.gpgsign") } value := strings.ToLower(gpgsign) diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 18d90b765..bda3ea225 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -735,8 +735,8 @@ func TestGitCommandMerge(t *testing.T) { func TestGitCommandUsingGpg(t *testing.T) { type scenario struct { testName string - getGlobalGitConfig func(string) (string, error) getLocalGitConfig func(string) (string, error) + getGlobalGitConfig func(string) (string, error) test func(bool) } From 97e0a6dc45ebd949205f013dada9dd12a214428e Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Wed, 12 Sep 2018 07:51:14 +0200 Subject: [PATCH 8/8] commands/git : remove extra space --- pkg/commands/git.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index ecbd702c1..624588fc2 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -307,7 +307,6 @@ func (c *GitCommand) Pull() error { // Push pushes to a branch func (c *GitCommand) Push(branchName string, force bool) error { forceFlag := "" - if force { forceFlag = "--force-with-lease " }