Skip to content

Commit 68b7338

Browse files
stefanhallerstefanhaller
committed
Add tests
I can't do my usual "add the tests first, with EXPECTED/ACTUAL sections that document the bug" method here, because the tests would hang without the bug being fixed. We need two different tests here: one where a cherry-picked commit simply becomes empty "by itself", because the change is already present on the destination branch (this was only a problem with git versions older than 2.45), and the other where the cherry-pick stops with conflicts, and the user resolves them such that no changes are left, and then continues the cherry-pick. This would still fail even with git 2.45 or later. The fix is the same for both cases though. The tests show that the selection behavior after skipping an empty cherry-picked commit is not ideal, but since that's only a minor cosmetic problem we don't bother fixing it here.
1 parent c272d0b commit 68b7338

File tree

3 files changed

+193
-0
lines changed

3 files changed

+193
-0
lines changed

‎pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package cherry_pick
2+
3+
import (
4+
"github.com/jesseduffield/lazygit/pkg/config"
5+
. "github.com/jesseduffield/lazygit/pkg/integration/components"
6+
)
7+
8+
var CherryPickCommitThatBecomesEmpty = NewIntegrationTest(NewIntegrationTestArgs{
9+
Description: "Cherry-pick a commit that becomes empty at the destination",
10+
ExtraCmdArgs: []string{},
11+
Skip: false,
12+
SetupConfig: func(config *config.AppConfig) {},
13+
SetupRepo: func(shell *Shell) {
14+
shell.
15+
EmptyCommit("base").
16+
CreateFileAndAdd("file1", "change 1\n").
17+
CreateFileAndAdd("file2", "change 2\n").
18+
Commit("two changes in one commit").
19+
NewBranchFrom("branch", "HEAD^").
20+
CreateFileAndAdd("file1", "change 1\n").
21+
Commit("single change").
22+
CreateFileAndAdd("file3", "change 3\n").
23+
Commit("unrelated change").
24+
Checkout("master")
25+
},
26+
Run: func(t *TestDriver, keys config.KeybindingConfig) {
27+
t.Views().Branches().
28+
Focus().
29+
Lines(
30+
Contains("master").IsSelected(),
31+
Contains("branch"),
32+
).
33+
SelectNextItem().
34+
PressEnter()
35+
36+
t.Views().SubCommits().
37+
IsFocused().
38+
Lines(
39+
Contains("unrelated change").IsSelected(),
40+
Contains("single change"),
41+
Contains("base"),
42+
).
43+
Press(keys.Universal.RangeSelectDown).
44+
Press(keys.Commits.CherryPickCopy).
45+
Tap(func() {
46+
t.Views().Information().Content(Contains("2 commits copied"))
47+
})
48+
49+
t.Views().Commits().
50+
Focus().
51+
Lines(
52+
Contains("two changes in one commit").IsSelected(),
53+
Contains("base"),
54+
).
55+
Press(keys.Commits.PasteCommits).
56+
Tap(func() {
57+
t.ExpectPopup().Alert().
58+
Title(Equals("Cherry-pick")).
59+
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
60+
Confirm()
61+
})
62+
63+
if t.Git().Version().IsAtLeast(2, 45, 0) {
64+
t.Views().Commits().
65+
Lines(
66+
Contains("unrelated change"),
67+
Contains("single change"),
68+
Contains("two changes in one commit").IsSelected(),
69+
Contains("base"),
70+
).
71+
SelectPreviousItem()
72+
73+
// Cherry-picked commit is empty
74+
t.Views().Main().Content(DoesNotContain("diff --git"))
75+
} else {
76+
t.Views().Commits().
77+
// We have a bug with how the selection is updated in this case; normally you would
78+
// expect the "two changes in one commit" commit to be selected because it was
79+
// selected before pasting, and we try to maintain that selection. This is broken
80+
// for two reasons:
81+
// 1. We increment the selected line index after pasting by the number of pasted
82+
// commits; this is wrong because we skipped the commit that became empty. So
83+
// according to this bug, the "base" commit should be selected.
84+
// 2. We only update the selected line index after pasting if the currently selected
85+
// commit is not a rebase TODO commit, on the assumption that if it is, we are in a
86+
// rebase and the cherry-picked commits end up below the selection. In this case,
87+
// however, we still think we are cherry-picking because the final refresh after the
88+
// CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet;
89+
// so the "unrelated change" still has a "pick" action.
90+
//
91+
// Since this only happens for older git versions, we don't bother fixing it.
92+
Lines(
93+
Contains("unrelated change").IsSelected(),
94+
Contains("two changes in one commit"),
95+
Contains("base"),
96+
)
97+
}
98+
},
99+
})

‎pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package cherry_pick
2+
3+
import (
4+
"github.com/jesseduffield/lazygit/pkg/config"
5+
. "github.com/jesseduffield/lazygit/pkg/integration/components"
6+
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
7+
)
8+
9+
var CherryPickConflictsEmptyCommitAfterResolving = NewIntegrationTest(NewIntegrationTestArgs{
10+
Description: "Cherry pick commits with conflicts, resolve them so that the commit becomes empty",
11+
ExtraCmdArgs: []string{},
12+
Skip: false,
13+
SetupConfig: func(config *config.AppConfig) {
14+
config.GetUserConfig().Git.LocalBranchSortOrder = "recency"
15+
},
16+
SetupRepo: func(shell *Shell) {
17+
shared.MergeConflictsSetup(shell)
18+
},
19+
Run: func(t *TestDriver, keys config.KeybindingConfig) {
20+
t.Views().Branches().
21+
Focus().
22+
Lines(
23+
Contains("first-change-branch"),
24+
Contains("second-change-branch"),
25+
Contains("original-branch"),
26+
).
27+
SelectNextItem().
28+
PressEnter()
29+
30+
t.Views().SubCommits().
31+
IsFocused().
32+
TopLines(
33+
Contains("second-change-branch unrelated change"),
34+
Contains("second change"),
35+
).
36+
Press(keys.Universal.RangeSelectDown).
37+
Press(keys.Commits.CherryPickCopy)
38+
39+
t.Views().Information().Content(Contains("2 commits copied"))
40+
41+
t.Views().Commits().
42+
Focus().
43+
TopLines(
44+
Contains("first change").IsSelected(),
45+
).
46+
Press(keys.Commits.PasteCommits)
47+
48+
t.ExpectPopup().Alert().
49+
Title(Equals("Cherry-pick")).
50+
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
51+
Confirm()
52+
53+
t.Common().AcknowledgeConflicts()
54+
55+
t.Views().Files().
56+
IsFocused().
57+
SelectedLine(Contains("file")).
58+
Press(keys.Universal.Remove)
59+
60+
t.ExpectPopup().Menu().
61+
Title(Equals("Discard changes")).
62+
Select(Contains("Discard all changes")).
63+
Confirm()
64+
65+
t.Common().ContinueOnConflictsResolved("cherry-pick")
66+
67+
t.Views().Files().IsEmpty()
68+
69+
t.Views().Commits().
70+
Focus().
71+
TopLines(
72+
// We have a bug with how the selection is updated in this case; normally you would
73+
// expect the "first change" commit to be selected because it was selected before
74+
// pasting, and we try to maintain that selection. This is broken for two reasons:
75+
// 1. We increment the selected line index after pasting by the number of pasted
76+
// commits; this is wrong because we skipped the commit that became empty. So
77+
// according to this bug, the "original" commit should be selected.
78+
// 2. We only update the selected line index after pasting if the currently selected
79+
// commit is not a rebase TODO commit, on the assumption that if it is, we are in a
80+
// rebase and the cherry-picked commits end up below the selection. In this case,
81+
// however, we still think we are cherry-picking because the final refresh after the
82+
// CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet;
83+
// so the "second-change-branch unrelated change" still has a "pick" action.
84+
//
85+
// We don't bother fixing it for now because it's a pretty niche case, and the
86+
// nature of the problem is only cosmetic.
87+
Contains("second-change-branch unrelated change").IsSelected(),
88+
Contains("first change"),
89+
Contains("original"),
90+
)
91+
},
92+
})

‎pkg/integration/tests/test_list.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ var tests = []*components.IntegrationTest{
8686
branch.Suggestions,
8787
branch.UnsetUpstream,
8888
cherry_pick.CherryPick,
89+
cherry_pick.CherryPickCommitThatBecomesEmpty,
8990
cherry_pick.CherryPickConflicts,
91+
cherry_pick.CherryPickConflictsEmptyCommitAfterResolving,
9092
cherry_pick.CherryPickDuringRebase,
9193
cherry_pick.CherryPickMerge,
9294
cherry_pick.CherryPickRange,

0 commit comments

Comments
 (0)