From 9481920101c7f8e29875e6db2642fe2b317564f9 Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Tue, 18 Sep 2018 20:47:40 +0200 Subject: [PATCH 1/3] commands/git : add test to GetLog --- pkg/commands/git_test.go | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 790a34690..ba2696e5d 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -1553,6 +1553,48 @@ func TestGitCommandGetCommits(t *testing.T) { } } +func TestGitCommandGetLog(t *testing.T) { + type scenario struct { + testName string + command func(string, ...string) *exec.Cmd + test func(string) + } + + scenarios := []scenario{ + { + "Retrieves logs", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"log", "--oneline", "-30"}, args) + + return exec.Command("echo", "6f0b32f commands/git : add GetCommits tests refactor\n9d9d775 circle : remove new line") + }, + func(output string) { + assert.EqualValues(t, "6f0b32f commands/git : add GetCommits tests refactor\n9d9d775 circle : remove new line\n", output) + }, + }, + { + "An error occurred when retrieving logs", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"log", "--oneline", "-30"}, args) + return exec.Command("test") + }, + func(output string) { + assert.Empty(t, output) + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.OSCommand.command = s.command + s.test(gitCmd.GetLog()) + }) + } +} + func TestGitCommandDiff(t *testing.T) { gitCommand := newDummyGitCommand() assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh")) From bdeb78c9a04963847ffd90c2277b3a274e3ba309 Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Tue, 18 Sep 2018 20:53:32 +0200 Subject: [PATCH 2/3] commands/git : returns an error instead of panicing --- pkg/commands/git.go | 8 ++------ pkg/commands/git_test.go | 13 +++++++++++++ pkg/gui/commits_panel.go | 5 ++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index c37ca9b47..24dcc6894 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -499,12 +499,8 @@ func (c *GitCommand) Ignore(filename string) error { } // Show shows the diff of a commit -func (c *GitCommand) Show(sha string) string { - result, err := c.OSCommand.RunCommandWithOutput("git show --color " + sha) - if err != nil { - panic(err) - } - return result +func (c *GitCommand) Show(sha string) (string, error) { + return c.OSCommand.RunCommandWithOutput(fmt.Sprintf("git show --color %s", sha)) } // Diff returns the diff of a file diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index ba2696e5d..eb17103fc 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -1421,6 +1421,19 @@ func TestGitCommandRemoveFile(t *testing.T) { } } +func TestGitCommandShow(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.OSCommand.command = func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"show", "--color", "456abcde"}, args) + + return exec.Command("echo") + } + + _, err := gitCmd.Show("456abcde") + assert.NoError(t, err) +} + func TestGitCommandCheckout(t *testing.T) { type scenario struct { testName string diff --git a/pkg/gui/commits_panel.go b/pkg/gui/commits_panel.go index 7c09559ff..91bb334ff 100644 --- a/pkg/gui/commits_panel.go +++ b/pkg/gui/commits_panel.go @@ -68,7 +68,10 @@ func (gui *Gui) handleCommitSelect(g *gocui.Gui, v *gocui.View) error { } return gui.renderString(g, "main", gui.Tr.SLocalize("NoCommitsThisBranch")) } - commitText := gui.GitCommand.Show(commit.Sha) + commitText, err := gui.GitCommand.Show(commit.Sha) + if err != nil { + return err + } return gui.renderString(g, "main", commitText) } From 360b7c1def1f5945f30d89e393bdd4233f0c3179 Mon Sep 17 00:00:00 2001 From: Anthony HAMON Date: Tue, 18 Sep 2018 21:09:51 +0200 Subject: [PATCH 3/3] commands/git : refactor test to Diff, refactor function --- pkg/commands/git.go | 4 +- pkg/commands/git_test.go | 130 +++++++++++++----------------------- test/repos/lots_of_diffs.sh | 35 ---------- 3 files changed, 50 insertions(+), 119 deletions(-) delete mode 100755 test/repos/lots_of_diffs.sh diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 24dcc6894..d9effbef5 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -506,15 +506,15 @@ func (c *GitCommand) Show(sha string) (string, error) { // Diff returns the diff of a file func (c *GitCommand) Diff(file *File) string { cachedArg := "" + trackedArg := "--" fileName := c.OSCommand.Quote(file.Name) if file.HasStagedChanges && !file.HasUnstagedChanges { cachedArg = "--cached" } - trackedArg := "--" if !file.Tracked && !file.HasStagedChanges { trackedArg = "--no-index /dev/null" } - command := fmt.Sprintf("%s %s %s %s", "git diff --color ", cachedArg, trackedArg, fileName) + command := fmt.Sprintf("git diff --color %s %s %s", cachedArg, trackedArg, fileName) // for now we assume an error means the file was deleted s, _ := c.OSCommand.RunCommandWithOutput(command) diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index eb17103fc..e5fd539b9 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/i18n" - "github.com/jesseduffield/lazygit/pkg/test" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" gogit "gopkg.in/src-d/go-git.v4" @@ -1609,96 +1608,63 @@ func TestGitCommandGetLog(t *testing.T) { } func TestGitCommandDiff(t *testing.T) { - gitCommand := newDummyGitCommand() - assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh")) + type scenario struct { + testName string + command func(string, ...string) *exec.Cmd + file *File + } - files := []*File{ + scenarios := []scenario{ { - Name: "deleted_staged", - HasStagedChanges: false, - HasUnstagedChanges: true, - Tracked: true, - Deleted: true, - HasMergeConflicts: false, - DisplayString: " D deleted_staged", + "Default case", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"diff", "--color", "--", "test.txt"}, args) + + return exec.Command("echo") + }, + &File{ + Name: "test.txt", + HasStagedChanges: false, + Tracked: true, + }, }, { - Name: "file with space staged", - HasStagedChanges: true, - HasUnstagedChanges: false, - Tracked: false, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "A \"file with space staged\"", + "All changes staged", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"diff", "--color", "--cached", "--", "test.txt"}, args) + + return exec.Command("echo") + }, + &File{ + Name: "test.txt", + HasStagedChanges: true, + HasUnstagedChanges: false, + Tracked: true, + }, }, { - Name: "file with space unstaged", - HasStagedChanges: false, - HasUnstagedChanges: true, - Tracked: false, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "?? file with space unstaged", - }, - { - Name: "modified_unstaged", - HasStagedChanges: true, - HasUnstagedChanges: false, - Tracked: true, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "M modified_unstaged", - }, - { - Name: "modified_staged", - HasStagedChanges: false, - HasUnstagedChanges: true, - Tracked: true, - Deleted: false, - HasMergeConflicts: false, - DisplayString: " M modified_staged", - }, - { - Name: "renamed_before -> renamed_after", - HasStagedChanges: true, - HasUnstagedChanges: false, - Tracked: true, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "R renamed_before -> renamed_after", - }, - { - Name: "untracked_unstaged", - HasStagedChanges: false, - HasUnstagedChanges: true, - Tracked: false, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "?? untracked_unstaged", - }, - { - Name: "untracked_staged", - HasStagedChanges: true, - HasUnstagedChanges: false, - Tracked: false, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "A untracked_staged", - }, - { - Name: "master", - HasStagedChanges: false, - HasUnstagedChanges: true, - Tracked: false, - Deleted: false, - HasMergeConflicts: false, - DisplayString: "?? master", + "File not tracked and file has no staged changes", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"diff", "--color", "--no-index", "/dev/null", "test.txt"}, args) + + return exec.Command("echo") + }, + &File{ + Name: "test.txt", + HasStagedChanges: false, + Tracked: false, + }, }, } - for _, file := range files { - t.Run(file.Name, func(t *testing.T) { - assert.NotContains(t, gitCommand.Diff(file), "error") + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.OSCommand.command = s.command + gitCmd.Diff(s.file) }) } } diff --git a/test/repos/lots_of_diffs.sh b/test/repos/lots_of_diffs.sh deleted file mode 100755 index 08c743f35..000000000 --- a/test/repos/lots_of_diffs.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/bin/bash -set -ex; rm -rf repo; mkdir repo; cd repo - -git init -git config user.email "test@example.com" -git config user.name "Lazygit Tester" - - -echo "deleted" > deleted_staged -echo "deleted_unstaged" > deleted_unstaged -echo "modified_staged" > modified_staged -echo "modified_unstaged" > modified_unstaged -echo "renamed" > renamed_before - -git add . -git commit -m "files to delete" -rm deleted_staged -rm deleted_unstaged - -rm renamed_before -echo "renamed" > renamed_after -echo "more" >> modified_staged -echo "more" >> modified_unstaged -echo "untracked_staged" > untracked_staged -echo "untracked_unstaged" > untracked_unstaged -echo "blah" > "file with space staged" -echo "blah" > "file with space unstaged" -echo "same name as branch" > master - -git add deleted_staged -git add modified_staged -git add untracked_staged -git add "file with space staged" -git add renamed_before -git add renamed_after \ No newline at end of file