Add an App-level event harness for felis-client window lifecycle tests #150

New issue

Open

opened 2026-09-06 02:22:22 +09:00 by natsukium · 0 comments

natsukium commented

2026-09-06 02:22:22 +09:00

Owner

What was found

felis-client's App cannot be built in a unit test: it holds an
EventLoopProxy<AppEvent>, which only a winit EventLoop produces, and
building one needs a display server. Every event handler also takes
&ActiveEventLoop. So the whole window state machine
(crates/felis-client/src/event_handler.rs,
crates/felis-client/src/app_methods.rs) is reachable only through free
functions and the state enums beside it; there is no test that drives an
ordered sequence of AppEvents into a window.

Three behaviors #22 landed have no regression test for exactly this
reason:

the shell-exit ordering criterion ("the final queued grid state is
applied before the window closes"): the pump forwards Grid frames
and SessionExited in order and App::user_event applies them in
that order, but nothing pins it. The test that claimed to
(the_rows_queued_ahead_of_the_exit_are_the_ones_the_window_closes_on)
only exercised ShadowScreen::apply and was removed rather than left
overclaiming.
an outstanding Region::Request cleared by begin_reconnect and by
continue_exit_ladder, asserted through App::is_busy().
a transport close absorbed as ClosedOutcome::AwaitSwitch starting
the reconnect ladder once the in-flight landing fails.

Why it is out of scope for #22

#22's deliverable is the lifecycle behavior itself. A harness that can
drive App means either a headless winit event loop in CI (a new CI
dependency and a per-platform question) or extracting the handlers from
the winit types — a refactor of the densest module in the client, with
its own design. Neither fits inside the issue's budget, and both would
land under the release freeze for reasons unrelated to the freeze.

What the follow-up should do

Pick one of the two shapes above, then port the three cases listed
above onto it: a stub pump delivering RowDelta, RowDelta,
SessionExited; a pipe request in flight through both the
transport-loss and shell-exit paths, asserting is_busy() clears; and a
close during an in-flight switch whose landing then fails, asserting the
reconnect ladder starts.

Split from #22.

## What was found `felis-client`'s `App` cannot be built in a unit test: it holds an `EventLoopProxy<AppEvent>`, which only a winit `EventLoop` produces, and building one needs a display server. Every event handler also takes `&ActiveEventLoop`. So the whole window state machine (`crates/felis-client/src/event_handler.rs`, `crates/felis-client/src/app_methods.rs`) is reachable only through free functions and the state enums beside it; there is no test that drives an ordered sequence of `AppEvent`s into a window. Three behaviors #22 landed have no regression test for exactly this reason: - the shell-exit ordering criterion ("the final queued grid state is applied before the window closes"): the pump forwards `Grid` frames and `SessionExited` in order and `App::user_event` applies them in that order, but nothing pins it. The test that claimed to (`the_rows_queued_ahead_of_the_exit_are_the_ones_the_window_closes_on`) only exercised `ShadowScreen::apply` and was removed rather than left overclaiming. - an outstanding `Region::Request` cleared by `begin_reconnect` and by `continue_exit_ladder`, asserted through `App::is_busy()`. - a transport close absorbed as `ClosedOutcome::AwaitSwitch` starting the reconnect ladder once the in-flight landing fails. ## Why it is out of scope for #22 #22's deliverable is the lifecycle behavior itself. A harness that can drive `App` means either a headless winit event loop in CI (a new CI dependency and a per-platform question) or extracting the handlers from the winit types — a refactor of the densest module in the client, with its own design. Neither fits inside the issue's budget, and both would land under the release freeze for reasons unrelated to the freeze. ## What the follow-up should do Pick one of the two shapes above, then port the three cases listed above onto it: a stub pump delivering `RowDelta`, `RowDelta`, `SessionExited`; a pipe request in flight through both the transport-loss and shell-exit paths, asserting `is_busy()` clears; and a close during an in-flight switch whose landing then fails, asserting the reconnect ladder starts. Split from #22.

natsukium added the

priority/P2

label

2026-09-06 20:10:36 +09:00