Skip to content

Fix a few flaky tests - #5756

Merged
stefanhaller merged 3 commits into
masterfrom
event-ordering-fixes
Jul 2, 2026
Merged

Fix a few flaky tests#5756
stefanhaller merged 3 commits into
masterfrom
event-ordering-fixes

Conversation

@stefanhaller

Copy link
Copy Markdown
Collaborator

These started out as a few intermittently-failing integration tests, but the root cause was real: a handful of places where UI updates (rendering, model updates, view content) could be applied in a different order than they were issued, because the order depended on goroutine scheduling rather than being deterministic. In rare cases that could make the main view briefly show stale or wrong content — most likely when moving quickly between items. This was never observed in normal usage, but caused some integration tests to fail occasionally (both locally and on CI).

Each commit fixes one of these orderings; see the commit messages for details.

stefanhaller and others added 3 commits July 2, 2026 17:24
NewTask was incrementing newTaskID and reading taskID inside the
spawned goroutine, so for two NewTask calls in quick succession the
assignment was determined by goroutine scheduling order rather than
call order. When the goroutines reordered, the first NewTask call
could end up with the higher taskID and "win" the staleness check,
superseding the second call's task even though the caller intended
the second to be the latest.

Worse, the staleness check ran after onNewKey, so a goroutine destined
to bail as stale would still reset the view buffer first, potentially
wiping the winning task's already-written output.

Take newTaskID++ synchronously in NewTask so taskIDs follow call order,
and move the first staleness check ahead of onNewKey so a stale task
doesn't side-effect the view before exiting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy the outgoing view's content into the target view (the flicker-
prevention step) before queuing the render task, rather than after. The
task writes the fresh content from a worker goroutine, so with the old
order the worker write races the UI-thread copy, and the copy can land
last and clobber the fresh content with stale output.

This is only needed while view writes happen concurrently. Once view
writes are serialized on the UI thread and the view write-mutex goes
away, the synchronous copy always precedes the FIFO-queued write
regardless of order, so the reorder becomes unnecessary. No code comment
is added for it, since that comment would be obsoleted by that work and
likely left behind.
Update spawned a goroutine per call that then sent on the user-events
channel, so multiple Update calls from the same goroutine could be
reordered by the scheduler — the doc comment even admitted "the order in
which the user events will be handled is not guaranteed." That
non-determinism is a latent source of flaky rendering: code that queues a
model update and then a render in source order could see them run in the
opposite order.

Send on the channel directly instead, so same-goroutine calls arrive in
source order. The send is non-blocking and panics on a full channel
rather than blocking (a blocked send from the UI goroutine would deadlock
against itself) or silently reordering; the buffer is sized generously so
this is unreachable in normal use. UpdateAsync is now identical to Update
and unused, so it's removed along with the shared updateAsyncAux helper.
@stefanhaller stefanhaller added the maintenance For refactorings, CI changes, tests, version bumping, etc label Jul 2, 2026
@stefanhaller
stefanhaller enabled auto-merge July 2, 2026 15:48
@stefanhaller
stefanhaller merged commit 3d912f0 into master Jul 2, 2026
14 checks passed
@stefanhaller
stefanhaller deleted the event-ordering-fixes branch July 2, 2026 15:50
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jul 9, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.62.2` → `v0.63.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary>

### [`v0.63.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.63.0)

[Compare Source](jesseduffield/lazygit@v0.62.2...v0.63.0)

<!-- Release notes generated using configuration in .github/release.yml at v0.63.0 -->

#### What's Changed

##### Enhancements 🔥

- Add direnv support by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5660](jesseduffield/lazygit#5660)
- Improve cycling through multiple pagers by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5678](jesseduffield/lazygit#5678)
- Detect external repo changes via background polling by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5662](jesseduffield/lazygit#5662)
- Make the side panels configurable by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5702](jesseduffield/lazygit#5702)
- Add a global keybinding for editing the config file by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5728](jesseduffield/lazygit#5728)
- Improve resolving non-textual and submodule merge conflicts by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5735](jesseduffield/lazygit#5735)
- Auto-dismiss the continue-rebase prompt when it becomes stale by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5758](jesseduffield/lazygit#5758)
- Support custom pagers and passphrase prompts on Windows by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5740](jesseduffield/lazygit#5740)
- Make creating worktrees simpler and less error-prone by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5741](jesseduffield/lazygit#5741)
- Improve deleting worktrees and their branches by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5748](jesseduffield/lazygit#5748)
- Allow overriding the platform used for default keybindings by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5671](jesseduffield/lazygit#5671)
- Add `gui.shrinkSidePanelsToContent` option by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5754](jesseduffield/lazygit#5754)
- Show renamed files in the custom patch builder by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5759](jesseduffield/lazygit#5759)

##### Fixes 🔧

- Fix unstaging a submodule with dirty content by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5666](jesseduffield/lazygit#5666)
- Fix coloring of wrapped delta lines by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5711](jesseduffield/lazygit#5711)
- Fix Files Panel artefacts during rebase commands by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5661](jesseduffield/lazygit#5661)
- Keep selected commits stable across refreshes by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5717](jesseduffield/lazygit#5717)
- Fix quoting of shell commands on Windows by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5704](jesseduffield/lazygit#5704)
- Silently consume unrecognized or malformed escape sequences by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5738](jesseduffield/lazygit#5738)
- Don't include common ancestor when picking "both" for a conflict in diff3 style by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5747](jesseduffield/lazygit#5747)

##### Maintenance ⚙️

- Some fixes to our infrastructure by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5705](jesseduffield/lazygit#5705)
- Restructure the `just` recipes for running integration tests by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5720](jesseduffield/lazygit#5720)
- Fix flaky TestNewCmdTaskInstantStop test by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5743](jesseduffield/lazygit#5743)
- Bump golang.org/x/sync from 0.20.0 to 0.21.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5684](jesseduffield/lazygit#5684)
- Bump golang.org/x/sys from 0.45.0 to 0.46.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5685](jesseduffield/lazygit#5685)
- Bump github.com/sahilm/fuzzy from 0.1.2 to 0.1.3 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5706](jesseduffield/lazygit#5706)
- Bump actions/cache from 5 to 6 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5722](jesseduffield/lazygit#5722)
- Bump actions/checkout from 6 to 7 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5723](jesseduffield/lazygit#5723)
- Bump goreleaser/goreleaser-action from 7.2.2 to 7.2.3 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5745](jesseduffield/lazygit#5745)
- Bump golangci/golangci-lint-action from 9.2.0 to 9.3.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5746](jesseduffield/lazygit#5746)
- Bump golang.org/x/net from 0.47.0 to 0.55.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;5752](jesseduffield/lazygit#5752)
- Pin gofumpt version to 0.9.2 by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5753](jesseduffield/lazygit#5753)
- Fix a few flaky tests by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5756](jesseduffield/lazygit#5756)

##### I18n 🌎

- Update translations from Crowdin by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5760](jesseduffield/lazygit#5760)

##### Performance Improvements 📊

- Prevent staging from becoming slower over time by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;5712](jesseduffield/lazygit#5712)

**Full Changelog**: <jesseduffield/lazygit@v0.62.2...v0.63.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTYuMCIsInVwZGF0ZWRJblZlciI6IjQzLjI1Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
stefanhaller added a commit that referenced this pull request Jul 15, 2026
In #5756 we changed the userEvents channel to a fixed 256-slot channel
with a non-blocking send that panicked when the channel was full. It
turns out that this panic can happen in real use:

- Toggling a directory of several hundred files into a custom patch
(reliably): the operation runs on a worker behind a waiting status,
whose spinner enqueues a content-only render on every tick, and over the
long operation these outrun the UI loop and overflow the buffer.
- Editing the config in an editor that suspends lazygit: the editor
subprocess runs on the UI thread, so the loop drains nothing for the
whole editing session, and the full refresh fired on resume fans out
across every scope at once — a burst of updates that overflows before
the just-resumed loop catches up.
- Any time the UI thread blocks for a long time, the periodic refreshes
keep enqueuing and eventually overflow.

The 256-slot buffer was chosen deliberately, with the panic as a "should
never happen" guard, to preserve two properties: FIFO ordering of
same-goroutine Update calls (an earlier goroutine-per-Update design
reordered them), and no self-deadlock (a blocking send from the UI
thread would block against the loop that drains it). But a fixed channel
can only offer those by crashing on overflow.

Replace it with an unbounded, order-preserving queue: a mutex-guarded
slice plus a buffered(1) doorbell channel that wakes the main loop's
select. Enqueuing appends and rings the doorbell; the loop drains the
slice to empty on each wake. This keeps FIFO order and never blocks the
caller, so there is no self-deadlock and no overflow to panic on — under
a stall the queue just grows and then drains.

Fixes #5772.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance For refactorings, CI changes, tests, version bumping, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant