From 6c4e7ee9729ccfd65ac03073a37bd110a61be432 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Mon, 3 Jul 2023 14:16:43 +1000 Subject: [PATCH] Add busy count for integration tests Integration tests need to be notified when Lazygit is idle so they can progress to the next assertion / user action. --- pkg/gui/background.go | 2 +- pkg/gui/context/suggestions_context.go | 2 +- .../controllers/helpers/app_status_helper.go | 5 +- pkg/gui/controllers/helpers/refresh_helper.go | 4 +- .../controllers/local_commits_controller.go | 2 +- pkg/gui/controllers/sub_commits_controller.go | 3 +- pkg/gui/file_watching.go | 5 +- pkg/gui/gui.go | 28 +++-- pkg/gui/gui_common.go | 4 + pkg/gui/gui_driver.go | 11 +- pkg/gui/layout.go | 2 + pkg/gui/popup/popup_handler.go | 6 +- pkg/gui/tasks_adapter.go | 8 +- pkg/gui/test_mode.go | 22 +++- pkg/gui/types/common.go | 3 + .../components/assertion_helper.go | 2 + pkg/integration/tests/commit/reword.go | 1 + .../tests/diff/diff_and_apply_patch.go | 2 +- .../squash_fixups_above_first_commit.go | 2 +- pkg/tasks/async_handler.go | 9 +- pkg/tasks/async_handler_test.go | 5 +- pkg/tasks/tasks.go | 105 ++++++++++++------ pkg/tasks/tasks_test.go | 29 ++++- 23 files changed, 184 insertions(+), 78 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 14ee70187..218bd8ce7 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -79,7 +79,7 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru if self.pauseBackgroundThreads { continue } - _ = function() + self.gui.c.OnWorker(func() { _ = function() }) case <-stop: return } diff --git a/pkg/gui/context/suggestions_context.go b/pkg/gui/context/suggestions_context.go index 58b2205a4..d8b650642 100644 --- a/pkg/gui/context/suggestions_context.go +++ b/pkg/gui/context/suggestions_context.go @@ -30,7 +30,7 @@ func NewSuggestionsContext( c *ContextCommon, ) *SuggestionsContext { state := &SuggestionsContextState{ - AsyncHandler: tasks.NewAsyncHandler(), + AsyncHandler: tasks.NewAsyncHandler(c.OnWorker), } getModel := func() []*types.Suggestion { return state.Suggestions diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index f125ebf7b..dcac41e48 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -4,7 +4,6 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/gui/status" - "github.com/jesseduffield/lazygit/pkg/utils" ) type AppStatusHelper struct { @@ -28,7 +27,7 @@ func (self *AppStatusHelper) Toast(message string) { // withWaitingStatus wraps a function and shows a waiting status while the function is still executing func (self *AppStatusHelper) WithWaitingStatus(message string, f func() error) { - go utils.Safe(func() { + self.c.OnWorker(func() { self.statusMgr().WithWaitingStatus(message, func() { self.renderAppStatus() @@ -50,7 +49,7 @@ func (self *AppStatusHelper) GetStatusString() string { } func (self *AppStatusHelper) renderAppStatus() { - go utils.Safe(func() { + self.c.OnWorker(func() { ticker := time.NewTicker(time.Millisecond * 50) defer ticker.Stop() for range ticker.C { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index f0827dc41..67e6dc909 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -90,7 +90,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { wg.Add(1) func() { if options.Mode == types.ASYNC { - go utils.Safe(f) + self.c.OnWorker(f) } else { f() } @@ -206,7 +206,7 @@ func getModeName(mode types.RefreshMode) string { func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { switch self.c.State().GetRepoState().GetStartupStage() { case types.INITIAL: - go utils.Safe(func() { + self.c.OnWorker(func() { _ = self.refreshReflogCommits() self.refreshBranches() self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 49abe02ff..95c791a21 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -816,7 +816,7 @@ func (self *LocalCommitsController) GetOnFocus() func(types.OnFocusOpts) error { context := self.context() if context.GetSelectedLineIdx() > COMMIT_THRESHOLD && context.GetLimitCommits() { context.SetLimitCommits(false) - go utils.Safe(func() { + self.c.OnWorker(func() { if err := self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}}); err != nil { _ = self.c.Error(err) } diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index 485d49820..f6c519406 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -3,7 +3,6 @@ package controllers import ( "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" - "github.com/jesseduffield/lazygit/pkg/utils" ) type SubCommitsController struct { @@ -60,7 +59,7 @@ func (self *SubCommitsController) GetOnFocus() func(types.OnFocusOpts) error { context := self.context() if context.GetSelectedLineIdx() > COMMIT_THRESHOLD && context.GetLimitCommits() { context.SetLimitCommits(false) - go utils.Safe(func() { + self.c.OnWorker(func() { if err := self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUB_COMMITS}}); err != nil { _ = self.c.Error(err) } diff --git a/pkg/gui/file_watching.go b/pkg/gui/file_watching.go index ff04353e2..2c8addf8f 100644 --- a/pkg/gui/file_watching.go +++ b/pkg/gui/file_watching.go @@ -120,7 +120,10 @@ func (gui *Gui) WatchFilesForChanges() { } // only refresh if we're not already if !gui.IsRefreshingFiles { - _ = gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}) + gui.c.OnUIThread(func() error { + // TODO: find out if refresh needs to be run on the UI thread + return gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}) + }) } // watch for errors diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 106aee7a9..92deee1a0 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -130,6 +130,8 @@ type Gui struct { c *helpers.HelperCommon helpers *helpers.Helpers + + integrationTest integrationTypes.IntegrationTest } type StateAccessor struct { @@ -472,6 +474,7 @@ func NewGui( func(message string, f func() error) { gui.helpers.AppStatus.WithWaitingStatus(message, f) }, func(message string) { gui.helpers.AppStatus.Toast(message) }, func() string { return gui.Views.Confirmation.TextArea.GetContent() }, + func(f func()) { gui.c.OnWorker(f) }, ) guiCommon := &guiCommon{gui: gui, IPopupHandler: gui.PopupHandler} @@ -620,7 +623,8 @@ func (gui *Gui) Run(startArgs appTypes.StartArgs) error { gui.c.Log.Info("starting main loop") - gui.handleTestMode(startArgs.IntegrationTest) + // setting here so we can use it in layout.go + gui.integrationTest = startArgs.IntegrationTest return gui.g.MainLoop() } @@ -779,16 +783,15 @@ func (gui *Gui) showInitialPopups(tasks []func(chan struct{}) error) { gui.waitForIntro.Add(len(tasks)) done := make(chan struct{}) - go utils.Safe(func() { + gui.c.OnWorker(func() { for _, task := range tasks { - task := task - go utils.Safe(func() { - if err := task(done); err != nil { - _ = gui.c.Error(err) - } - }) + if err := task(done); err != nil { + _ = gui.c.Error(err) + } + gui.g.DecrementBusyCount() <-done + gui.g.IncrementBusyCount() gui.waitForIntro.Done() } }) @@ -796,9 +799,10 @@ func (gui *Gui) showInitialPopups(tasks []func(chan struct{}) error) { func (gui *Gui) showIntroPopupMessage(done chan struct{}) error { onConfirm := func() error { - done <- struct{}{} gui.c.GetAppState().StartupPopupVersion = StartupPopupVersion - return gui.c.SaveAppState() + err := gui.c.SaveAppState() + done <- struct{}{} + return err } return gui.c.Confirm(types.ConfirmOpts{ @@ -828,6 +832,10 @@ func (gui *Gui) onUIThread(f func() error) { }) } +func (gui *Gui) onWorker(f func()) { + gui.g.OnWorker(f) +} + func (gui *Gui) getWindowDimensions(informationStr string, appStatus string) map[string]boxlayout.Dimensions { return gui.helpers.WindowArrangement.GetWindowDimensions(informationStr, appStatus) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 8fc7732fc..779f1ddad 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -136,6 +136,10 @@ func (self *guiCommon) OnUIThread(f func() error) { self.gui.onUIThread(f) } +func (self *guiCommon) OnWorker(f func()) { + self.gui.onWorker(f) +} + func (self *guiCommon) RenderToMainViews(opts types.RefreshMainOpts) error { return self.gui.refreshMainViews(opts) } diff --git a/pkg/gui/gui_driver.go b/pkg/gui/gui_driver.go index 824c9ed33..630da5b0b 100644 --- a/pkg/gui/gui_driver.go +++ b/pkg/gui/gui_driver.go @@ -18,7 +18,8 @@ import ( // this gives our integration test a way of interacting with the gui for sending keypresses // and reading state. type GuiDriver struct { - gui *Gui + gui *Gui + isIdleChan chan struct{} } var _ integrationTypes.GuiDriver = &GuiDriver{} @@ -40,6 +41,9 @@ func (self *GuiDriver) PressKey(keyStr string) { tcell.NewEventKey(tcellKey, r, tcell.ModNone), 0, ) + + // wait until lazygit is idle (i.e. all processing is done) before continuing + <-self.isIdleChan } func (self *GuiDriver) Keys() config.KeybindingConfig { @@ -71,7 +75,10 @@ func (self *GuiDriver) Fail(message string) { self.gui.g.Close() // need to give the gui time to close time.Sleep(time.Millisecond * 100) - fmt.Fprintln(os.Stderr, fullMessage) + _, err := fmt.Fprintln(os.Stderr, fullMessage) + if err != nil { + panic("Test failed. Failed writing to stderr") + } panic("Test failed") } diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index ed10fda92..919186aa5 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -114,6 +114,8 @@ func (gui *Gui) layout(g *gocui.Gui) error { return err } + gui.handleTestMode() + gui.ViewsSetup = true } diff --git a/pkg/gui/popup/popup_handler.go b/pkg/gui/popup/popup_handler.go index 633e91a55..b32da3ef5 100644 --- a/pkg/gui/popup/popup_handler.go +++ b/pkg/gui/popup/popup_handler.go @@ -9,7 +9,6 @@ import ( gctx "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" - "github.com/jesseduffield/lazygit/pkg/utils" "github.com/sasha-s/go-deadlock" ) @@ -25,6 +24,7 @@ type PopupHandler struct { withWaitingStatusFn func(message string, f func() error) toastFn func(message string) getPromptInputFn func() string + onWorker func(func()) } var _ types.IPopupHandler = &PopupHandler{} @@ -39,6 +39,7 @@ func NewPopupHandler( withWaitingStatusFn func(message string, f func() error), toastFn func(message string), getPromptInputFn func() string, + onWorker func(func()), ) *PopupHandler { return &PopupHandler{ Common: common, @@ -51,6 +52,7 @@ func NewPopupHandler( withWaitingStatusFn: withWaitingStatusFn, toastFn: toastFn, getPromptInputFn: getPromptInputFn, + onWorker: onWorker, } } @@ -141,7 +143,7 @@ func (self *PopupHandler) WithLoaderPanel(message string, f func() error) error return nil } - go utils.Safe(func() { + self.onWorker(func() { if err := f(); err != nil { self.Log.Error(err) } diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 2bd7be4f5..89bccdb7d 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -48,7 +48,7 @@ func (gui *Gui) newStringTask(view *gocui.View, str string) error { func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error { manager := gui.getManager(view) - f := func(stop chan struct{}) error { + f := func(tasks.TaskOpts) error { gui.c.SetViewContent(view, str) return nil } @@ -65,7 +65,7 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error { func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX int, originY int) error { manager := gui.getManager(view) - f := func(stop chan struct{}) error { + f := func(tasks.TaskOpts) error { gui.c.SetViewContent(view, str) _ = view.SetOrigin(originX, originY) return nil @@ -81,7 +81,7 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) error { manager := gui.getManager(view) - f := func(stop chan struct{}) error { + f := func(tasks.TaskOpts) error { gui.c.ResetViewOrigin(view) gui.c.SetViewContent(view, str) return nil @@ -130,6 +130,8 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { func() { _ = view.SetOrigin(0, 0) }, + gui.c.GocuiGui().IncrementBusyCount, + gui.c.GocuiGui().DecrementBusyCount, ) gui.viewBufferManagerMap[view.Name()] = manager } diff --git a/pkg/gui/test_mode.go b/pkg/gui/test_mode.go index be46041db..e24e922a9 100644 --- a/pkg/gui/test_mode.go +++ b/pkg/gui/test_mode.go @@ -7,29 +7,39 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/integration/components" - integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types" "github.com/jesseduffield/lazygit/pkg/utils" ) type IntegrationTest interface { - Run(guiAdapter *GuiDriver) + Run(*GuiDriver) } -func (gui *Gui) handleTestMode(test integrationTypes.IntegrationTest) { +func (gui *Gui) handleTestMode() { + test := gui.integrationTest if os.Getenv(components.SANDBOX_ENV_VAR) == "true" { return } if test != nil { - go func() { - time.Sleep(time.Millisecond * 100) + isIdleChan := make(chan struct{}) - test.Run(&GuiDriver{gui: gui}) + gui.c.GocuiGui().AddIdleListener(isIdleChan) + + waitUntilIdle := func() { + <-isIdleChan + } + + go func() { + waitUntilIdle() + + test.Run(&GuiDriver{gui: gui, isIdleChan: isIdleChan}) gui.g.Update(func(*gocui.Gui) error { return gocui.ErrQuit }) + waitUntilIdle() + time.Sleep(time.Second * 1) log.Fatal("gocui should have already exited") diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 09ab040f2..184288c0a 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -77,6 +77,9 @@ type IGuiCommon interface { // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. // All controller handlers are executed on the UI thread. OnUIThread(f func() error) + // Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact + // that lazygit is still busy. + OnWorker(f func()) // returns the gocui Gui struct. There is a good chance you don't actually want to use // this struct and instead want to use another method above diff --git a/pkg/integration/components/assertion_helper.go b/pkg/integration/components/assertion_helper.go index 48cc14741..f7e435a1f 100644 --- a/pkg/integration/components/assertion_helper.go +++ b/pkg/integration/components/assertion_helper.go @@ -13,6 +13,8 @@ type assertionHelper struct { // milliseconds we'll wait when an assertion fails. func retryWaitTimes() []int { + return []int{0} + if os.Getenv("LONG_WAIT_BEFORE_FAIL") == "true" { // CI has limited hardware, may be throttled, runs tests in parallel, etc, so we // give it more leeway compared to when we're running things locally. diff --git a/pkg/integration/tests/commit/reword.go b/pkg/integration/tests/commit/reword.go index 48941b7d2..21727c494 100644 --- a/pkg/integration/tests/commit/reword.go +++ b/pkg/integration/tests/commit/reword.go @@ -61,6 +61,7 @@ var Reword = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Lines( Contains(wipCommitMessage), + Contains(commitMessage), ) }, }) diff --git a/pkg/integration/tests/diff/diff_and_apply_patch.go b/pkg/integration/tests/diff/diff_and_apply_patch.go index caf2338b4..c0c95cc17 100644 --- a/pkg/integration/tests/diff/diff_and_apply_patch.go +++ b/pkg/integration/tests/diff/diff_and_apply_patch.go @@ -62,7 +62,7 @@ var DiffAndApplyPatch = NewIntegrationTest(NewIntegrationTestArgs{ Tap(func() { t.ExpectPopup().Menu().Title(Equals("Diffing")).Select(Contains("Exit diff mode")).Confirm() - t.Views().Information().Content(DoesNotContain("Building patch")) + t.Views().Information().Content(Contains("Building patch")) }). Press(keys.Universal.CreatePatchOptionsMenu) diff --git a/pkg/integration/tests/interactive_rebase/squash_fixups_above_first_commit.go b/pkg/integration/tests/interactive_rebase/squash_fixups_above_first_commit.go index 8c9fed0a4..4e5fe28f6 100644 --- a/pkg/integration/tests/interactive_rebase/squash_fixups_above_first_commit.go +++ b/pkg/integration/tests/interactive_rebase/squash_fixups_above_first_commit.go @@ -30,7 +30,7 @@ var SquashFixupsAboveFirstCommit = NewIntegrationTest(NewIntegrationTestArgs{ Content(Contains("Are you sure you want to create a fixup! commit for commit")). Confirm() }). - NavigateToLine(Contains("commit 01")). + NavigateToLine(Contains("commit 01").DoesNotContain("fixup!")). Press(keys.Commits.SquashAboveCommits). Tap(func() { t.ExpectPopup().Confirmation(). diff --git a/pkg/tasks/async_handler.go b/pkg/tasks/async_handler.go index c277a1184..6cf1a4044 100644 --- a/pkg/tasks/async_handler.go +++ b/pkg/tasks/async_handler.go @@ -1,7 +1,6 @@ package tasks import ( - "github.com/jesseduffield/lazygit/pkg/utils" "github.com/sasha-s/go-deadlock" ) @@ -18,11 +17,13 @@ type AsyncHandler struct { lastId int mutex deadlock.Mutex onReject func() + onWorker func(func()) } -func NewAsyncHandler() *AsyncHandler { +func NewAsyncHandler(onWorker func(func())) *AsyncHandler { return &AsyncHandler{ - mutex: deadlock.Mutex{}, + mutex: deadlock.Mutex{}, + onWorker: onWorker, } } @@ -32,7 +33,7 @@ func (self *AsyncHandler) Do(f func() func()) { id := self.currentId self.mutex.Unlock() - go utils.Safe(func() { + self.onWorker(func() { after := f() self.handle(after, id) }) diff --git a/pkg/tasks/async_handler_test.go b/pkg/tasks/async_handler_test.go index b6edbec20..70e678b94 100644 --- a/pkg/tasks/async_handler_test.go +++ b/pkg/tasks/async_handler_test.go @@ -12,7 +12,10 @@ func TestAsyncHandler(t *testing.T) { wg := sync.WaitGroup{} wg.Add(2) - handler := NewAsyncHandler() + onWorker := func(f func()) { + go f() + } + handler := NewAsyncHandler(onWorker) handler.onReject = func() { wg.Done() } diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index ad227fc65..58784da5f 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -48,6 +48,10 @@ type ViewBufferManager struct { refreshView func() onEndOfInput func() + // see docs/dev/Busy.md + incrementBusyCount func() + decrementBusyCount func() + // if the user flicks through a heap of items, with each one // spawning a process to render something to the main view, // it can slow things down quite a bit. In these situations we @@ -76,15 +80,19 @@ func NewViewBufferManager( refreshView func(), onEndOfInput func(), onNewKey func(), + incrementBusyCount func(), + decrementBusyCount func(), ) *ViewBufferManager { return &ViewBufferManager{ - Log: log, - writer: writer, - beforeStart: beforeStart, - refreshView: refreshView, - onEndOfInput: onEndOfInput, - readLines: make(chan LinesToRead, 1024), - onNewKey: onNewKey, + Log: log, + writer: writer, + beforeStart: beforeStart, + refreshView: refreshView, + onEndOfInput: onEndOfInput, + readLines: make(chan LinesToRead, 1024), + onNewKey: onNewKey, + incrementBusyCount: incrementBusyCount, + decrementBusyCount: decrementBusyCount, } } @@ -94,13 +102,22 @@ func (self *ViewBufferManager) ReadLines(n int) { }) } -// note: onDone may be called twice -func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDone func()) func(chan struct{}) error { - return func(stop chan struct{}) error { - var once sync.Once - var onDoneWrapper func() - if onDone != nil { - onDoneWrapper = func() { once.Do(onDone) } +func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { + return func(opts TaskOpts) error { + var onDoneOnce sync.Once + var onFirstPageShownOnce sync.Once + + onFirstPageShown := func() { + onFirstPageShownOnce.Do(func() { + opts.InitialContentLoaded() + }) + } + + onDone := func() { + if onDoneFn != nil { + onDoneOnce.Do(onDoneFn) + } + onFirstPageShown() } if self.throttle { @@ -109,7 +126,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p } select { - case <-stop: + case <-opts.Stop: + onDone() return nil default: } @@ -119,7 +137,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p timeToStart := time.Since(startTime) go utils.Safe(func() { - <-stop + <-opts.Stop // we use the time it took to start the program as a way of checking if things // are running slow at the moment. This is admittedly a crude estimate, but // the point is that we only want to throttle when things are running slow @@ -132,9 +150,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p } // for pty's we need to call onDone here so that cmd.Wait() doesn't block forever - if onDoneWrapper != nil { - onDoneWrapper() - } + onDone() }) loadingMutex := deadlock.Mutex{} @@ -153,7 +169,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p ticker := time.NewTicker(time.Millisecond * 200) defer ticker.Stop() select { - case <-stop: + case <-opts.Stop: return case <-ticker.C: loadingMutex.Lock() @@ -182,12 +198,12 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p outer: for { select { - case <-stop: + case <-opts.Stop: break outer case linesToRead := <-self.readLines: for i := 0; i < linesToRead.Total; i++ { select { - case <-stop: + case <-opts.Stop: break outer default: } @@ -219,6 +235,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p } } refreshViewIfStale() + onFirstPageShown() } } @@ -231,10 +248,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p } } - // calling onDoneWrapper here again in case the program ended on its own accord - if onDoneWrapper != nil { - onDoneWrapper() - } + // calling this here again in case the program ended on its own accord + onDone() close(done) }) @@ -272,8 +287,30 @@ func (self *ViewBufferManager) Close() { // 1) command based, where the manager can be asked to read more lines, but the command can be killed // 2) string based, where the manager can also be asked to read more lines -func (self *ViewBufferManager) NewTask(f func(stop chan struct{}) error, key string) error { +type TaskOpts struct { + // Channel that tells the task to stop, because another task wants to run. + Stop chan struct{} + + // Only for tasks which are long-running, where we read more lines sporadically. + // We use this to keep track of when a user's action is complete (i.e. all views + // have been refreshed to display the results of their action) + InitialContentLoaded func() +} + +func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error { + self.incrementBusyCount() + + var decrementCounterOnce sync.Once + + decrementCounter := func() { + decrementCounterOnce.Do(func() { + self.decrementBusyCount() + }) + } + go utils.Safe(func() { + defer decrementCounter() + self.taskIDMutex.Lock() self.newTaskID++ taskID := self.newTaskID @@ -286,9 +323,9 @@ func (self *ViewBufferManager) NewTask(f func(stop chan struct{}) error, key str self.taskIDMutex.Unlock() self.waitingMutex.Lock() - defer self.waitingMutex.Unlock() if taskID < self.newTaskID { + self.waitingMutex.Unlock() return } @@ -307,13 +344,13 @@ func (self *ViewBufferManager) NewTask(f func(stop chan struct{}) error, key str self.stopCurrentTask = func() { once.Do(onStop) } - go utils.Safe(func() { - if err := f(stop); err != nil { - self.Log.Error(err) // might need an onError callback - } + self.waitingMutex.Unlock() - close(notifyStopped) - }) + if err := f(TaskOpts{Stop: stop, InitialContentLoaded: decrementCounter}); err != nil { + self.Log.Error(err) // might need an onError callback + } + + close(notifyStopped) }) return nil diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index ef13f6bf6..e42d13e0b 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -19,6 +19,11 @@ func getCounter() (func(), func() int) { return func() { counter++ }, func() int { return counter } } +func getIncDecCounter(initialValue int) (func(), func(), func() int) { + counter := initialValue + return func() { counter++ }, func() { counter-- }, func() int { return counter } +} + func TestNewCmdTaskInstantStop(t *testing.T) { writer := bytes.NewBuffer(nil) beforeStart, getBeforeStartCallCount := getCounter() @@ -26,6 +31,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { onEndOfInput, getOnEndOfInputCallCount := getCounter() onNewKey, getOnNewKeyCallCount := getCounter() onDone, getOnDoneCallCount := getCounter() + incBusyCount, decBusyCount, getBusyCount := getIncDecCounter(1) manager := NewViewBufferManager( utils.NewDummyLog(), @@ -34,6 +40,8 @@ func TestNewCmdTaskInstantStop(t *testing.T) { refreshView, onEndOfInput, onNewKey, + incBusyCount, + decBusyCount, ) stop := make(chan struct{}) @@ -49,7 +57,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1}, onDone) - _ = fn(stop) + _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: decBusyCount}) callCountExpectations := []struct { expected int @@ -68,6 +76,10 @@ func TestNewCmdTaskInstantStop(t *testing.T) { } } + if getBusyCount() != 0 { + t.Errorf("expected busy count to be 0, got %d", getBusyCount()) + } + expectedContent := "" actualContent := writer.String() if actualContent != expectedContent { @@ -82,6 +94,7 @@ func TestNewCmdTask(t *testing.T) { onEndOfInput, getOnEndOfInputCallCount := getCounter() onNewKey, getOnNewKeyCallCount := getCounter() onDone, getOnDoneCallCount := getCounter() + incBusyCount, decBusyCount, getBusyCount := getIncDecCounter(1) manager := NewViewBufferManager( utils.NewDummyLog(), @@ -90,6 +103,8 @@ func TestNewCmdTask(t *testing.T) { refreshView, onEndOfInput, onNewKey, + incBusyCount, + decBusyCount, ) stop := make(chan struct{}) @@ -109,7 +124,7 @@ func TestNewCmdTask(t *testing.T) { close(stop) wg.Done() }() - _ = fn(stop) + _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: decBusyCount}) wg.Wait() @@ -130,6 +145,10 @@ func TestNewCmdTask(t *testing.T) { } } + if getBusyCount() != 0 { + t.Errorf("expected busy count to be 0, got %d", getBusyCount()) + } + expectedContent := "prefix\ntest\n" actualContent := writer.String() if actualContent != expectedContent { @@ -208,6 +227,8 @@ func TestNewCmdTaskRefresh(t *testing.T) { lineCountsOnRefresh = append(lineCountsOnRefresh, strings.Count(writer.String(), "\n")) } + decBusyCount := func() {} + manager := NewViewBufferManager( utils.NewDummyLog(), writer, @@ -215,6 +236,8 @@ func TestNewCmdTaskRefresh(t *testing.T) { refreshView, func() {}, func() {}, + func() {}, + decBusyCount, ) stop := make(chan struct{}) @@ -234,7 +257,7 @@ func TestNewCmdTaskRefresh(t *testing.T) { close(stop) wg.Done() }() - _ = fn(stop) + _ = fn(TaskOpts{Stop: stop, InitialContentLoaded: decBusyCount}) wg.Wait()