Repository navigation
Fix live merged threads at the start - #1050
Conversation
The live TUI starts in merged mode, so its table should show every thread Signed-off-by: Kirill Ignatev <kignatev@bloomberg.net>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1050 +/- ##
==========================================
- Coverage 92.31% 92.27% -0.04%
==========================================
Files 103 103
Lines 13363 13363
Branches 483 483
==========================================
- Hits 12336 12331 -5
- Misses 1027 1032 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The table only showed the first thread until merging was toggled. Signed-off-by: Kirill Ignatev <kignatev@bloomberg.net>
89811c4 to
579c0fd
Compare
godlygeek
left a comment
There was a problem hiding this comment.
LGTM, except I renamed the news entry to use the PR number instead of just calling it news/live-merged-threads.bugfix.rst - that way the changelog will include a link to the PR. I try to link to the PR whenever there's no issue to link to.
While reviewing this, I noticed that pilot.press("") doesn't actually seem to be defined to do nothing - but it's also an existing pattern we used elsewhere in the tests, so I think I'll send a separate PR to fix that everywhere where we've done it.
Also, I'm going to squash this one rather than rebase - I don't want a commit on main with a failing test, since it makes bisecting harder, so I'm going to squash the test and fix into one commit. Just an FYI, since we were talking about when to squash vs when to rebase yesterday 🙂
Closes: #1045
Default
AllocationTable.merge_threadstoTrue, matching the screen.Before:
Starting in Live mode shows only 1 thread despite claiming "All threads" at the top unless you press "m" twice.
After:
Live mode correctly merges all threads immediately after start.
Notes
This does not look intentional.
Added test that fails without this change.