Skip to content

Commit cd22862

Browse files
authored
Fix flaky integration test (#4432)
- **PR Description** The integration test `stash/rename.go` occasionally fails for me locally. This PR fixes it. In 8b8343b we made a change to run newPtyTask from AfterLayout; this is needed so that the PTY gets the new, updated view size. However, this created a race condition for integration tests that select a line in a list view and then expect the main view to have certain content; sometimes that content gets rendered too late. I'm surprised that this didn't cause more tests to fail; right now I only know of one test that occasionally fails because of this, which is `stash/rename.go`. Fix this by moving the AfterLayout to inside newPtyTask, and do it only when we are actually using a PTY (we don't when no pager is configured, which is the case for integration tests). The diff is best viewed with "ignore whitespace" turned on.
2 parents c55062f + 10f29bc commit cd22862

File tree

2 files changed

+43
-40
lines changed

2 files changed

+43
-40
lines changed

pkg/gui/main_panels.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ func (gui *Gui) runTaskForView(view *gocui.View, task types.UpdateTask) error {
2020
return gui.newCmdTask(view, v.Cmd, v.Prefix)
2121

2222
case *types.RunPtyTask:
23-
gui.afterLayout(func() error {
24-
return gui.newPtyTask(view, v.Cmd, v.Prefix)
25-
})
26-
return nil
23+
return gui.newPtyTask(view, v.Cmd, v.Prefix)
2724
}
2825

2926
return nil

pkg/gui/pty.go

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -54,45 +54,51 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
5454
return gui.newCmdTask(view, cmd, prefix)
5555
}
5656

57-
cmdStr := strings.Join(cmd.Args, " ")
58-
59-
// This communicates to pagers that we're in a very simple
60-
// terminal that they should not expect to have much capabilities.
61-
// Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities.
62-
// Context: https://github.com/jesseduffield/lazygit/issues/3419
63-
cmd.Env = removeExistingTermEnvVars(cmd.Env)
64-
cmd.Env = append(cmd.Env, "TERM=dumb")
65-
66-
cmd.Env = append(cmd.Env, "GIT_PAGER="+pager)
67-
68-
manager := gui.getManager(view)
69-
70-
var ptmx *os.File
71-
start := func() (*exec.Cmd, io.Reader) {
72-
var err error
73-
ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view))
74-
if err != nil {
75-
gui.c.Log.Error(err)
57+
// Run the pty after layout so that it gets the correct size
58+
gui.afterLayout(func() error {
59+
// Need to get the width and the pager again because the layout might have
60+
// changed the size of the view
61+
width = view.InnerWidth()
62+
pager = gui.git.Config.GetPager(width)
63+
64+
cmdStr := strings.Join(cmd.Args, " ")
65+
66+
// This communicates to pagers that we're in a very simple
67+
// terminal that they should not expect to have much capabilities.
68+
// Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities.
69+
// Context: https://github.com/jesseduffield/lazygit/issues/3419
70+
cmd.Env = removeExistingTermEnvVars(cmd.Env)
71+
cmd.Env = append(cmd.Env, "TERM=dumb")
72+
73+
cmd.Env = append(cmd.Env, "GIT_PAGER="+pager)
74+
75+
manager := gui.getManager(view)
76+
77+
var ptmx *os.File
78+
start := func() (*exec.Cmd, io.Reader) {
79+
var err error
80+
ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view))
81+
if err != nil {
82+
gui.c.Log.Error(err)
83+
}
84+
85+
gui.Mutexes.PtyMutex.Lock()
86+
gui.viewPtmxMap[view.Name()] = ptmx
87+
gui.Mutexes.PtyMutex.Unlock()
88+
89+
return cmd, ptmx
7690
}
7791

78-
gui.Mutexes.PtyMutex.Lock()
79-
gui.viewPtmxMap[view.Name()] = ptmx
80-
gui.Mutexes.PtyMutex.Unlock()
81-
82-
return cmd, ptmx
83-
}
84-
85-
onClose := func() {
86-
gui.Mutexes.PtyMutex.Lock()
87-
ptmx.Close()
88-
delete(gui.viewPtmxMap, view.Name())
89-
gui.Mutexes.PtyMutex.Unlock()
90-
}
92+
onClose := func() {
93+
gui.Mutexes.PtyMutex.Lock()
94+
ptmx.Close()
95+
delete(gui.viewPtmxMap, view.Name())
96+
gui.Mutexes.PtyMutex.Unlock()
97+
}
9198

92-
linesToRead := gui.linesToReadFromCmdTask(view)
93-
if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
94-
return err
95-
}
99+
linesToRead := gui.linesToReadFromCmdTask(view)
100+
return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr)
101+
})
96102

97103
return nil
98104
}

0 commit comments

Comments
 (0)