-
-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Partially fix flaky tests #308
Conversation
- This has the benefit of creating snapshots for everything, allowing for later diffing
I'll |
- This makes snapshots more human-inspectable
df091f3
to
9245bc7
Compare
- `pause_by_space` - `rearranged_by_tab`
I realised the problem is not with any single test, but with the actual program termination logic, which explains why the random failures are all over the place. TLDR, race condition. Bandwhich has one worker thread to handle inputs (a.k.a. terminal events), and one to print to terminal. Currently they are unsynchronized on program exit, which in tests can cause fewer/more draw events than expected, and in deployment can cause garbage to be printed to terminal (which I've personally seen a few times). Difficult to diagnose, but should be an easy fix. |
Actually I'll just go manually trigger the tests. See https://github.com/imsnif/bandwhich/actions/workflows/ci.yaml?query=branch%3Aflaky-test-fix. |
a5e9484
to
3e2e652
Compare
Aright. After thinking more about the current implementation, I think the best way forward is to rewrite Lines 182 to 194 in 5d2ee96
Lines 240 to 256 in 5d2ee96
I will merge this PR now without closing #303. This work warrants a separate PR. |
This is a partial fix, and hence does not close (!! no GitHub, don't close it) #303. A full refactor of
main
is necessary to get rid of the race condition entirely.Problems found and fixed