Fix a chat-mode toggle crash and the flaky TUI tests#539
Merged
Conversation
Enter in the catalog search installs the first visible match by design, so the two focus-return tests were starting a genuine download task for the fake test model on every run. Locally the worker dies within the leak guard's grace; on a loaded runner it lives for minutes and starves whichever TUI test shares the xdist worker next, which is where most of the moving-victim CI failures came from. Stub _enqueue_download around the submit so the selection path stays real but no worker is spawned.
The panel's interval timer can slip extra ticks in between the test's explicit ones on a slow host. With a counter-keyed fake, one such tick consumed the good-stats answer while its labels snapshot predated set_devices, rendering the default GPU0 label, and the test's own tick then drew the raising branch. Answer by the test's current phase instead, so any number of extra ticks is idempotent.
ChatModeToggle.on_mount refreshes through query_one, and on a slow mount the composed pills are not attached yet, so the refresh raises NoMatches inside the message handler and takes the whole screen down (seen as test_initial_view_switches_to_catalog failing on CI). Keep the references compose creates and use them in _refresh; attributes carry no mount-order dependency. Also settle the catalog LeaveUp focus test on wait_for_scheduled_animations: a bare pause reads focus mid-flight.
Sabotage-verified: fails against the query_one version of _refresh, passes with the compose-held references.
tobocop2
changed the title
tobocop2
changed the title
The suite runs under a 100 percent coverage gate and the None guard's return was the one statement nothing executed: every mounted toggle has its pills by the time anything refreshes it. Exercise the pre-compose state directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The chat-mode toggle could crash the screen while mounting: its refresh queries the pills in the DOM, and when it ran before they finished attaching, the NoMatches took the screen down.
Beyond that, CI's red was flaky tests. The catalog search focus tests started a real model download every run (never stubbed), and on a loaded runner that worker starves the other TUI tests on its xdist worker, failing a different one each time. The GPU panel test keyed its fake probe on call count, so an extra timer tick drew the wrong answer.
Solution
The toggle keeps the pill references its compose creates and refreshes through them, so mount order no longer matters; a regression test pins it. The search tests stub the download enqueue, the GPU panel fake answers by phase, and the LeaveUp focus test waits for animations to settle before reading focus.