fix reflog failing to properly refresh

3 files changed, 168 insertions, 3 deletions

diff --git a/pkg/commands/loaders/commits_test.go b/pkg/commands/loaders/commits_test.go
index 9eda80708..23406abcc 100644
--- a/pkg/commands/loaders/commits_test.go
+++ b/pkg/commands/loaders/commits_test.go
@@ -41,7 +41,7 @@ d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield||65f910ebd852
 func TestGetCommits(t *testing.T) {
 	type scenario struct {
 		testName          string
-		runner            oscommands.ICmdObjRunner
+		runner            *oscommands.FakeCmdObjRunner
 		expectedCommits   []*models.Commit
 		expectedError     error
 		rebaseMode        enums.RebaseMode
@@ -208,6 +208,8 @@ func TestGetCommits(t *testing.T) {
 
 			assert.Equal(t, scenario.expectedCommits, commits)
 			assert.Equal(t, scenario.expectedError, err)
+
+			scenario.runner.CheckForMissingCalls()
 		})
 	}
 }
diff --git a/pkg/commands/loaders/reflog_commits.go b/pkg/commands/loaders/reflog_commits.go
index f2cb01aaa..dc1a4ac15 100644
--- a/pkg/commands/loaders/reflog_commits.go
+++ b/pkg/commands/loaders/reflog_commits.go
@@ -32,7 +32,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit
 		filterPathArg = fmt.Sprintf(" --follow -- %s", self.cmd.Quote(filterPath))
 	}
 
-	cmdObj := self.cmd.New(fmt.Sprintf(`git log -g --abbrev=20 --format="%%h %%ct %%gs" %s`, filterPathArg)).DontLog()
+	cmdObj := self.cmd.New(fmt.Sprintf(`git log -g --abbrev=20 --format="%%h %%ct %%gs"%s`, filterPathArg)).DontLog()
 	onlyObtainedNewReflogCommits := false
 	err := cmdObj.RunAndProcessLines(func(line string) (bool, error) {
 		fields := strings.SplitN(line, " ", 3)
@@ -49,7 +49,11 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit
 			Status:        "reflog",
 		}
 
-		if lastReflogCommit != nil && commit.Sha == lastReflogCommit.Sha && commit.UnixTimestamp == lastReflogCommit.UnixTimestamp {
+		// note that the unix timestamp here is the timestamp of the COMMIT, not the reflog entry itself,
+		// so two consequetive reflog entries may have both the same SHA and therefore same timestamp.
+		// We use the reflog message to disambiguate, and fingers crossed that we never see the same of those
+		// twice in a row. Reason being that it would mean we'd be erroneously exiting early.
+		if lastReflogCommit != nil && commit.Sha == lastReflogCommit.Sha && commit.UnixTimestamp == lastReflogCommit.UnixTimestamp && commit.Name == lastReflogCommit.Name {
 			onlyObtainedNewReflogCommits = true
 			// after this point we already have these reflogs loaded so we'll simply return the new ones
 			return true, nil
diff --git a/pkg/commands/loaders/reflog_commits_test.go b/pkg/commands/loaders/reflog_commits_test.go
new file mode 100644
index 000000000..0e00ca3e5
--- /dev/null
+++ b/pkg/commands/loaders/reflog_commits_test.go
@@ -0,0 +1,159 @@
+package loaders
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/sanity-io/litter"
+	"github.com/stretchr/testify/assert"
+)
+
+const reflogOutput = `c3c4b66b64c97ffeecde 1643150483 checkout: moving from A to B
+c3c4b66b64c97ffeecde 1643150483 checkout: moving from B to A
+c3c4b66b64c97ffeecde 1643150483 checkout: moving from A to B
+c3c4b66b64c97ffeecde 1643150483 checkout: moving from master to A
+f4ddf2f0d4be4ccc7efa 1643149435 checkout: moving from A to master
+`
+
+func TestGetReflogCommits(t *testing.T) {
+	type scenario struct {
+		testName                string
+		runner                  *oscommands.FakeCmdObjRunner
+		lastReflogCommit        *models.Commit
+		filterPath              string
+		expectedCommits         []*models.Commit
+		expectedOnlyObtainedNew bool
+		expectedError           error
+	}
+
+	scenarios := []scenario{
+		{
+			testName: "no reflog entries",
+			runner: oscommands.NewFakeRunner(t).
+				Expect(`git log -g --abbrev=20 --format="%h %ct %gs"`, "", nil),
+
+			lastReflogCommit:        nil,
+			expectedCommits:         []*models.Commit{},
+			expectedOnlyObtainedNew: false,
+			expectedError:           nil,
+		},
+		{
+			testName: "some reflog entries",
+			runner: oscommands.NewFakeRunner(t).
+				Expect(`git log -g --abbrev=20 --format="%h %ct %gs"`, reflogOutput, nil),
+
+			lastReflogCommit: nil,
+			expectedCommits: []*models.Commit{
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from A to B",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from B to A",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from A to B",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from master to A",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+				{
+					Sha:           "f4ddf2f0d4be4ccc7efa",
+					Name:          "checkout: moving from A to master",
+					Status:        "reflog",
+					UnixTimestamp: 1643149435,
+				},
+			},
+			expectedOnlyObtainedNew: false,
+			expectedError:           nil,
+		},
+		{
+			testName: "some reflog entries where last commit is given",
+			runner: oscommands.NewFakeRunner(t).
+				Expect(`git log -g --abbrev=20 --format="%h %ct %gs"`, reflogOutput, nil),
+
+			lastReflogCommit: &models.Commit{
+				Sha:           "c3c4b66b64c97ffeecde",
+				Name:          "checkout: moving from B to A",
+				Status:        "reflog",
+				UnixTimestamp: 1643150483,
+			},
+			expectedCommits: []*models.Commit{
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from A to B",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+			},
+			expectedOnlyObtainedNew: true,
+			expectedError:           nil,
+		},
+		{
+			testName: "when passing filterPath",
+			runner: oscommands.NewFakeRunner(t).
+				Expect(`git log -g --abbrev=20 --format="%h %ct %gs" --follow -- "path"`, reflogOutput, nil),
+
+			lastReflogCommit: &models.Commit{
+				Sha:           "c3c4b66b64c97ffeecde",
+				Name:          "checkout: moving from B to A",
+				Status:        "reflog",
+				UnixTimestamp: 1643150483,
+			},
+			filterPath: "path",
+			expectedCommits: []*models.Commit{
+				{
+					Sha:           "c3c4b66b64c97ffeecde",
+					Name:          "checkout: moving from A to B",
+					Status:        "reflog",
+					UnixTimestamp: 1643150483,
+				},
+			},
+			expectedOnlyObtainedNew: true,
+			expectedError:           nil,
+		},
+		{
+			testName: "when command returns error",
+			runner: oscommands.NewFakeRunner(t).
+				Expect(`git log -g --abbrev=20 --format="%h %ct %gs"`, "", errors.New("haha")),
+
+			lastReflogCommit:        nil,
+			filterPath:              "",
+			expectedCommits:         nil,
+			expectedOnlyObtainedNew: false,
+			expectedError:           errors.New("haha"),
+		},
+	}
+
+	for _, scenario := range scenarios {
+		scenario := scenario
+		t.Run(scenario.testName, func(t *testing.T) {
+			builder := &ReflogCommitLoader{
+				Common: utils.NewDummyCommon(),
+				cmd:    oscommands.NewDummyCmdObjBuilder(scenario.runner),
+			}
+
+			commits, onlyObtainednew, err := builder.GetReflogCommits(scenario.lastReflogCommit, scenario.filterPath)
+			assert.Equal(t, scenario.expectedOnlyObtainedNew, onlyObtainednew)
+			assert.Equal(t, scenario.expectedError, err)
+			t.Logf("actual commits: \n%s", litter.Sdump(commits))
+			assert.Equal(t, scenario.expectedCommits, commits)
+
+			scenario.runner.CheckForMissingCalls()
+		})
+	}
+}