From 7bf398db1d0a13c173d30cb22c9c8e323d95bd52 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 Oct 2026 15:09:48 +0100 Subject: [PATCH] Add tests demonstrating bugs when moving a patch to an earlier commit When moving a custom patch to an earlier commit, or into a new commit before the original one, lazygit applies the patch to the earlier commit and then replays the original commit unchanged. It relies on git noticing during the replay that the patch's changes are already there. This doesn't always work: - If the patch contains only some of several adjacent added lines, replaying the original commit stops with a conflict. - If git can place the patch's added lines in two positions because they repeat the line after them, the replay may put them in the other position than the patch did. Git then keeps both copies, and the file ends up containing the patch's lines twice. There's no conflict, so this goes unnoticed. When moving into a new commit before the original one, the conflict of the first case doesn't show the usual "Conflicts!" popup; lazygit focuses the commits panel instead. Co-Authored-By: Claude Opus 5.5 --- ...ly_placed_added_lines_to_earlier_commit.go | 72 ++++++++++++++++++ ...placed_added_lines_to_new_commit_before.go | 71 ++++++++++++++++++ ..._adjacent_added_lines_to_earlier_commit.go | 75 +++++++++++++++++++ ...jacent_added_lines_to_new_commit_before.go | 74 ++++++++++++++++++ ...patch_from_added_file_to_earlier_commit.go | 10 +++ pkg/integration/tests/test_list.go | 4 + 6 files changed, 306 insertions(+) create mode 100644 pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go create mode 100644 pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go create mode 100644 pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go create mode 100644 pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..7c2ec2c07 --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go @@ -0,0 +1,72 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to an earlier commit, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + /* EXPECTED: + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ACTUAL: */ + Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..e10cf853d --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go @@ -0,0 +1,71 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to a new commit before it, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + /* EXPECTED: + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ACTUAL: */ + Equals("first line\nextra line\n}\nadded line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..db30538cc --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go @@ -0,0 +1,75 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to an earlier commit", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + /* EXPECTED: + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }). + SelectPreviousItem(). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }) + ACTUAL: */ + t.Common().AcknowledgeConflicts() + + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU").Contains("file1"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..56c3ad038 --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go @@ -0,0 +1,74 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to a new commit before it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + /* EXPECTED: + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }). + SelectNextItem(). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }) + ACTUAL: */ + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("<-- CONFLICT --- commit to move from"), + Contains("─── Commits"), + Contains("new commit"), + Contains("first commit"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go index 1cad9e646..a726af2c5 100644 --- a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go @@ -44,6 +44,16 @@ var MovePatchFromAddedFileToEarlierCommit = NewIntegrationTest(NewIntegrationTes t.Views().Commits().Focus().SelectNextItem() t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + /* EXPECTED: + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ). + PressEnter() + ACTUAL: */ // The source commit tries to add a file that the moved line now causes to exist. t.Common().AcknowledgeConflicts() diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index acbe5b445..6e902bacb 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -449,9 +449,13 @@ var tests = []*components.IntegrationTest{ main_view.KeepSelectedRangeWhenChangingContextSize, main_view.KeepSelectionAfterMovingPatchOut, main_view.KeepSelectionVisibleWhenDiffShrinks, + main_view.MoveAmbiguouslyPlacedAddedLinesToEarlierCommit, + main_view.MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore, main_view.MoveMultiFileRangeToIndex, main_view.MoveOnWhenThePatchMarksRewrapTheDiff, + main_view.MovePartOfAdjacentAddedLinesToEarlierCommit, main_view.MovePartOfAdjacentAddedLinesToIndex, + main_view.MovePartOfAdjacentAddedLinesToNewCommitBefore, main_view.MovePartialPatchToIndex, main_view.MovePartialPatchToLaterCommit, main_view.MovePartialPatchToNewCommit,