client: drive the window state machines through a seam so they can be tested without winit #233

New issue

Closed

opened 2026-10-23 10:27:14 +09:00 by natsukium · 0 comments

natsukium commented

2026-10-23 10:27:14 +09:00

Owner

Problem

The window-level state machines in felis-client have no tests. App (crates/felis-client/src/main.rs, with app_methods.rs and event_handler.rs, ~5.6k lines) owns session switching, parked reattach/retarget pushes, pipe/run visits and their return, the exit ladder, and the search bar, and it reaches them only through a live winit event loop:

App holds Arc<Window> (title, cursor icon, fullscreen, decorations, attention, theme, request_redraw) and EventLoopProxy<AppEvent> (the IPC reader's wake-up), and its methods call both directly.
winit input types (ModifiersState, Key, Ime, MouseScrollDelta, PhysicalPosition) flow through the input handlers rather than stopping at the adapter.

The 105 tests in tests.rs therefore cover pure helpers only (click streak, wheel math, search labels). Bugs in the interplay (a pipe chord refused on a re-pointed session, the scrollback view dropped on a bare modifier) were fixed without a regression test at the App level, because none can be written without a window.

The same spread makes a winit major upgrade (0.31 reshapes ApplicationHandler and the event types) touch most of these 5.6k lines.

Proposal

Confine winit to a thin adapter so App can be driven by a fake in tests:

Input as felis events. Translate winit keyboard, IME, mouse and wheel events into a felis-owned input enum at the edge (winit_keys.rs, input.rs, keymap_adapter.rs already map keys to felis_client_core types); handlers take that enum.
Window effects behind a seam. Route every Window and EventLoopProxy call through one host interface (set title / cursor icon / fullscreen / decorations, request redraw, request attention, wake the loop). The winit build implements it over the real window; tests implement it as a recorder.
First test. Pick one interplay that is untested today, e.g. a reattach push arriving mid-switch is parked and applied on landing, and pin it through the fake host. Extract only as much as that test needs, then widen.

Behavior does not change. App stays in felis-client; whether the seam is a trait, a generic, or a cfg(test) swap is decided by the first test, preferring the form that keeps App non-generic if it suffices.

Out of scope

Moving App out of felis-client into another crate.
Platform-specific paths the fake cannot model (Windows IME, macOS window chrome); they stay covered by smoke and manual runs.

Notes

Doing this together with the winit 0.31 bump avoids paying the churn twice. Expect conflicts with in-flight app_methods.rs work (#224).

## Problem The window-level state machines in `felis-client` have no tests. `App` (`crates/felis-client/src/main.rs`, with `app_methods.rs` and `event_handler.rs`, ~5.6k lines) owns session switching, parked reattach/retarget pushes, pipe/run visits and their return, the exit ladder, and the search bar, and it reaches them only through a live winit event loop: - `App` holds `Arc<Window>` (title, cursor icon, fullscreen, decorations, attention, theme, `request_redraw`) and `EventLoopProxy<AppEvent>` (the IPC reader's wake-up), and its methods call both directly. - winit input types (`ModifiersState`, `Key`, `Ime`, `MouseScrollDelta`, `PhysicalPosition`) flow through the input handlers rather than stopping at the adapter. The 105 tests in `tests.rs` therefore cover pure helpers only (click streak, wheel math, search labels). Bugs in the interplay (a pipe chord refused on a re-pointed session, the scrollback view dropped on a bare modifier) were fixed without a regression test at the `App` level, because none can be written without a window. The same spread makes a winit major upgrade (0.31 reshapes `ApplicationHandler` and the event types) touch most of these 5.6k lines. ## Proposal Confine winit to a thin adapter so `App` can be driven by a fake in tests: 1. **Input as felis events.** Translate winit keyboard, IME, mouse and wheel events into a felis-owned input enum at the edge (`winit_keys.rs`, `input.rs`, `keymap_adapter.rs` already map keys to `felis_client_core` types); handlers take that enum. 2. **Window effects behind a seam.** Route every `Window` and `EventLoopProxy` call through one host interface (set title / cursor icon / fullscreen / decorations, request redraw, request attention, wake the loop). The winit build implements it over the real window; tests implement it as a recorder. 3. **First test.** Pick one interplay that is untested today, e.g. a reattach push arriving mid-switch is parked and applied on landing, and pin it through the fake host. Extract only as much as that test needs, then widen. Behavior does not change. `App` stays in `felis-client`; whether the seam is a trait, a generic, or a `cfg(test)` swap is decided by the first test, preferring the form that keeps `App` non-generic if it suffices. ## Out of scope - Moving `App` out of `felis-client` into another crate. - Platform-specific paths the fake cannot model (Windows IME, macOS window chrome); they stay covered by smoke and manual runs. ## Notes Doing this together with the winit 0.31 bump avoids paying the churn twice. Expect conflicts with in-flight `app_methods.rs` work (#224).

natsukium added the

Area/client

Kind/Testing

labels

2026-10-23 10:27:15 +09:00

natsukium referenced this issue from a pull request that will close it,

2026-10-23 19:40:58 +09:00

client: drive App through felis-owned seams so its state machines are testable without winit #234

natsukium closed this issue

2026-10-23 22:47:30 +09:00