From cae2aeb74fc09db6a304f4191b597c928853d978 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:55:21 +0800 Subject: [PATCH] test(ui): run the window and pane gpui tests on Windows too (#791) Windows tty7-app tests go 1466 -> 1638 with no assertion weakened. Also holds the SCM graph idle test's daemon end open: the moved handle closed the socket right after writing Cwd, which on Windows (loopback TcpStream with unread data) is an abortive close, so settle_graph would time out and the test would silently skip every assertion. --- src/terminal/view.rs | 83 ++++++++++++++++++++++++++++++------------ src/ui/app.rs | 19 ++++------ src/ui/diff_overlay.rs | 20 ++++++++-- src/ui/file_tree.rs | 6 +-- src/ui/scm/detail.rs | 16 +++++++- src/ui/scm/graph.rs | 17 +++++---- src/ui/scm/panel.rs | 15 +++++++- src/ui/switcher.rs | 2 +- src/ui/tree_sync.rs | 2 - 9 files changed, 125 insertions(+), 55 deletions(-) diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 800f1713..bda9a686 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -98,7 +98,7 @@ pub fn declare_displayed(cx: &App, panes: impl IntoIterator Option { cx.try_global::()? .0 @@ -9132,22 +9132,42 @@ mod tests { } } +/// A connected pair of [`crate::daemon::transport::Stream`]s, one for each end +/// of a pane's link to its daemon. +/// +/// The client half is what a pane really reads and writes; the daemon half is +/// the test's, to speak protocol into. +/// +/// This is the one thing a pane harness needs that Unix and Windows spell +/// differently — `socketpair` there, a loopback connect here — and every gpui +/// test in this crate is portable once it goes through this instead of naming +/// `UnixStream` itself. +#[cfg(test)] +pub(crate) fn test_stream_pair() -> ( + crate::daemon::transport::Stream, + crate::daemon::transport::Stream, +) { + #[cfg(unix)] + { + std::os::unix::net::UnixStream::pair().unwrap() + } + #[cfg(windows)] + { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let client_side = std::net::TcpStream::connect(addr).unwrap(); + let (daemon_side, _) = listener.accept().unwrap(); + (client_side, daemon_side) + } +} + #[cfg(test)] pub(crate) fn quiet_test_pane( pane_id: u64, window: &mut Window, cx: &mut gpui::App, ) -> (gpui::Entity, crate::daemon::transport::Stream) { - #[cfg(unix)] - let (client_side, daemon_side) = std::os::unix::net::UnixStream::pair().unwrap(); - #[cfg(windows)] - let (client_side, daemon_side) = { - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - let client_side = std::net::TcpStream::connect(addr).unwrap(); - let (daemon_side, _) = listener.accept().unwrap(); - (client_side, daemon_side) - }; + let (client_side, daemon_side) = test_stream_pair(); let terminal = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)) .expect("quiet test terminal"); let view = cx.new(|cx| TerminalView::with_terminal(terminal, pane_id, window, cx)); @@ -9200,20 +9220,20 @@ pub(crate) fn quiet_test_ssh_pane_with( (view, stream) } -#[cfg(all(test, unix))] +#[cfg(test)] mod gpui_tests { use super::*; use crate::daemon::protocol::{ClientMsg, DaemonMsg}; + use crate::daemon::transport::Stream; use gpui::{Entity, TestAppContext, point}; - use std::os::unix::net::UnixStream; - fn harness(cx: &mut TestAppContext) -> (gpui::WindowHandle, UnixStream) { + fn harness(cx: &mut TestAppContext) -> (gpui::WindowHandle, Stream) { // Building a view reads the config. Whether that hit the real user // directory used to come down to which test happened to pin the // scratch dir first. crate::core::config::pin_test_config_dir(); cx.executor().allow_parking(); - let (client_side, daemon_side) = UnixStream::pair().unwrap(); + let (client_side, daemon_side) = super::test_stream_pair(); cx.update(|cx| { gpui_component::init(cx); cx.set_global(Config::default()); @@ -9238,11 +9258,11 @@ mod gpui_tests { ) -> ( gpui::WindowHandle, Entity, - UnixStream, + Stream, ) { crate::core::config::pin_test_config_dir(); cx.executor().allow_parking(); - let (client_side, daemon_side) = UnixStream::pair().unwrap(); + let (client_side, daemon_side) = super::test_stream_pair(); cx.update(|cx| { gpui_component::init(cx); cx.set_global(Config::default()); @@ -9268,7 +9288,7 @@ mod gpui_tests { fn prompt_ready( window: &gpui::WindowHandle, cx: &mut TestAppContext, - daemon: &mut UnixStream, + daemon: &mut Stream, ) { DaemonMsg::Prompt { active: true, @@ -9292,7 +9312,7 @@ mod gpui_tests { fn alt_screen_ready( window: &gpui::WindowHandle, cx: &mut TestAppContext, - daemon: &mut UnixStream, + daemon: &mut Stream, ) { DaemonMsg::Output(b"\x1b[?1049h".to_vec()) .encode(daemon) @@ -9320,7 +9340,7 @@ mod gpui_tests { .encode(&mut daemon) .unwrap(); - let report = |status: AgentStatus, daemon: &mut UnixStream| { + let report = |status: AgentStatus, daemon: &mut Stream| { DaemonMsg::AgentStatus(Some(AgentSessionState { status, message: None, @@ -9427,7 +9447,7 @@ mod gpui_tests { status: crate::core::cli_agent::AgentStatus, pane: &gpui::Entity, cx: &mut TestAppContext, - daemon: &mut UnixStream, + daemon: &mut Stream, ) { use crate::core::cli_agent::AgentSessionState; @@ -9877,6 +9897,21 @@ mod gpui_tests { /// must not have made the promise. Otherwise those paths sit "not answered /// yet" for the life of the pane — no underline, and a click that says /// nothing, which is the silence this whole path exists to remove. + /// + /// Was unix-only because the path it prints is: `Path::new("/etc/hosts")` + /// is not absolute on Windows, so `FileCandidate::paths` measured it from + /// the roots rather than letting it stand alone — and a workspace that + /// never connected has no roots, so nothing was ever wanted. Which was + /// itself the divergence: a Windows tty7 looking at a *remote* Linux pane + /// never probed the POSIX paths that pane printed. + /// + /// #795 settled that. `paths` now asks the pane's own + /// [`super::search::PathStyle`] rather than this machine's, and a remote + /// pane that has not reported a cwd is read as `Posix`, so `/etc/hosts` + /// stands alone on every client. The gate is only still here because + /// nothing has run this test on Windows yet; lifting it belongs in a + /// change that can show it green, not in a merge. + #[cfg(unix)] #[gpui::test] fn a_probe_with_no_host_to_ask_stays_wanted(cx: &mut TestAppContext) { let (window, mut daemon) = harness(cx); @@ -10068,7 +10103,7 @@ mod gpui_tests { .unwrap(); } - fn next_input(daemon: &mut UnixStream) -> Vec { + fn next_input(daemon: &mut Stream) -> Vec { loop { match ClientMsg::read(daemon).expect("client socket stays open") { ClientMsg::Input(bytes) => return bytes, @@ -10100,7 +10135,7 @@ mod gpui_tests { } } - fn next_input_until_timeout(daemon: &mut UnixStream) -> Option> { + fn next_input_until_timeout(daemon: &mut Stream) -> Option> { use std::io::ErrorKind; daemon @@ -12845,7 +12880,7 @@ mod gpui_tests { } assert_eq!(seen, "before", "the pre-drop screen is what we relink over"); - let (new_client, mut new_daemon) = UnixStream::pair().unwrap(); + let (new_client, mut new_daemon) = super::test_stream_pair(); window .update(cx, |view, _, cx| { view.adopt_relink( diff --git a/src/ui/app.rs b/src/ui/app.rs index 8c73c6fb..6a76284b 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -9491,14 +9491,13 @@ pub(crate) mod test_window { } /// A window carrying `n` quiet tabs, active on the first. - #[cfg(unix)] pub(crate) fn harness_with_tabs( cx: &mut TestAppContext, n: usize, ) -> ( Entity, VisualTestContext, - Vec, + Vec, ) { use crate::terminal::view::quiet_test_pane; use crate::ui::pane::{Pane, PaneSlot}; @@ -9525,13 +9524,12 @@ pub(crate) mod test_window { (app, vcx, streams) } - #[cfg(unix)] pub(crate) fn harness_with_pane( cx: &mut TestAppContext, ) -> ( Entity, VisualTestContext, - std::os::unix::net::UnixStream, + crate::daemon::transport::Stream, ) { use crate::terminal::view::quiet_test_pane; use crate::ui::pane::{Pane, PaneSlot}; @@ -9580,7 +9578,6 @@ pub(crate) mod test_window { /// another frame 250ms later. So the sleep below is load-bearing too, and /// a round that drew nothing is not on its own enough to stop on — a burst /// still open is a frame already owed. - #[cfg(unix)] pub(crate) fn quiesce(vcx: &mut VisualTestContext, cwd: Option<&std::path::Path>) { use crate::terminal::git_data::ScmData; use crate::terminal::git_status::GitStatusCache; @@ -9692,7 +9689,7 @@ mod cursor_blink_gpui_tests { } } -#[cfg(all(test, unix))] +#[cfg(test)] mod ssh_rebuild_gpui_tests { use super::test_window::harness_with_pane; use crate::core::session::{ @@ -10129,9 +10126,7 @@ mod shell_menu_gpui_tests { } } -// `harness_with_tabs` hands back the panes' `UnixStream`s, so it exists only -// on unix — same as `ssh_rebuild_gpui_tests` below it. -#[cfg(all(test, unix))] +#[cfg(test)] mod rename_gpui_tests { use gpui::TestAppContext; @@ -10215,7 +10210,7 @@ mod rename_gpui_tests { // everything else hidden; a pane nobody has declared — or whose id nobody // registered — must err toward displayed, because the failure direction that // matters is a visible pane that stops repainting. -#[cfg(all(test, unix))] +#[cfg(test)] mod displayed_gpui_tests { use gpui::TestAppContext; @@ -10316,7 +10311,7 @@ mod displayed_gpui_tests { // Zoom is a tab's view state: it rides with the tab across a switch, while a // layout change (drag, split, close) still clears it. -#[cfg(all(test, unix))] +#[cfg(test)] mod zoom_gpui_tests { use gpui::TestAppContext; @@ -10444,7 +10439,7 @@ mod zoom_gpui_tests { // test config dir and nothing is listening on it — so every forward request // fails. That is exactly the case these are about: what the panel and the form // are left holding when the far side does not answer. -#[cfg(all(test, unix))] +#[cfg(test)] mod managed_forward_gpui_tests { use gpui::TestAppContext; use gpui_component::input::InputState; diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index 8d5eb30c..4a3e595c 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -2737,7 +2737,7 @@ mod tests { } } -#[cfg(all(test, unix))] +#[cfg(test)] mod overlay_gpui_tests { use super::*; use crate::ui::app::test_window; @@ -3015,8 +3015,22 @@ mod overlay_gpui_tests { /// terminal, an editor, a worktree command — and the cached branch is a branch /// the repository has left. That is what the stale entry below stands for. /// -/// Unix-gated like every other window harness in this tree: `harness_with_tabs` -/// hands back a `std::os::unix::net::UnixStream` for the pane. +/// Unix-only, and not for the harness: on Windows the root this test seeds +/// the cache with is not the root the probe lands with, so `scm_epoch` never +/// agrees with the landing snapshot and the overlay re-probes on every frame +/// — `load` reaches `Ready` and `loading` goes straight back to `true`, which +/// is the exact spin this test exists to catch. +/// +/// Not the slash direction — `Path` compares by component, so `C:/x` and +/// `C:\x` are already equal. It is the prefix, and since #796 it is this +/// test's own: the product keys a repository by one spelling now +/// (`Host::canonicalize` drops the `\\?\` extended-length prefix and +/// `core::git::git_path` re-spells what git prints), while the seed below +/// still comes straight from `std::fs::canonicalize` and so carries +/// `\\?\C:\Users\—` — a `VerbatimDisk` prefix where everything it is +/// compared against is now `Disk`. Seeding through +/// `tty7_core::core::path_spelling` should lift this, as a change that can +/// show it green rather than a drive-by. #[cfg(all(test, unix))] mod render_idle_gpui_tests { use super::*; diff --git a/src/ui/file_tree.rs b/src/ui/file_tree.rs index f5408ec8..0d379823 100644 --- a/src/ui/file_tree.rs +++ b/src/ui/file_tree.rs @@ -2931,7 +2931,7 @@ mod tests { } } -#[cfg(all(test, unix))] +#[cfg(test)] mod render_idle_gpui_tests { use super::*; use crate::daemon::protocol::DaemonMsg; @@ -2959,7 +2959,7 @@ mod render_idle_gpui_tests { ) -> ( Entity, VisualTestContext, - std::os::unix::net::UnixStream, + crate::daemon::transport::Stream, ) { let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); DaemonMsg::Cwd(root.to_path_buf()) @@ -3558,7 +3558,7 @@ mod render_idle_gpui_tests { /// What these cannot reach is the hit test — whether the row under the cursor /// is the one that gets the drop is decided by gpui's hitbox stack, and there /// is no headless way to put a cursor over a row. -#[cfg(all(test, unix))] +#[cfg(test)] mod drop_gpui_tests { use super::render_idle_gpui_tests::{files_panel_on, rows, scratch, serial, settle}; use super::*; diff --git a/src/ui/scm/detail.rs b/src/ui/scm/detail.rs index d1ffc073..043e2747 100644 --- a/src/ui/scm/detail.rs +++ b/src/ui/scm/detail.rs @@ -974,6 +974,20 @@ mod tests { /// here — a missing global, a theme token, a slice through the middle of a /// character — goes wrong during layout and paint, so these arm the render /// probe and insist something was actually drawn. +/// +/// Still unix-only, and for a reason worth naming rather than a harness one: +/// on Windows the root the panel settles on is not the root this module hands +/// it, so the panel never settles on the directory it is already showing. +/// +/// The forward slashes `git rev-parse --show-toplevel` prints are not what +/// breaks it — `Path` compares by component, so `C:/x` and `C:\x` are equal. +/// The prefix is, and since #796 it is this module's own: the product keys a +/// repository by one spelling now, while `scratch` below still hands the pane +/// `std::fs::canonicalize`'s `\\?\C:\Users\—`, a `VerbatimDisk` prefix where +/// every root it is compared against is `Disk`. Taking the gate off needs +/// that helper to spell its answer the way +/// `tty7_core::core::path_spelling` does, in a change that can show these +/// green rather than a drive-by. #[cfg(all(test, unix))] mod detail_gpui_tests { use super::*; @@ -1043,7 +1057,7 @@ mod detail_gpui_tests { ) -> ( Entity, VisualTestContext, - std::os::unix::net::UnixStream, + crate::daemon::transport::Stream, ) { let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); DaemonMsg::Cwd(root.to_path_buf()) diff --git a/src/ui/scm/graph.rs b/src/ui/scm/graph.rs index 553c9a61..c0e4ea87 100644 --- a/src/ui/scm/graph.rs +++ b/src/ui/scm/graph.rs @@ -2186,11 +2186,7 @@ mod tests { /// way to know is to settle the window and count frames. Same shape as the file /// tree's own idle tests, including the serial lock: the render probe is /// thread-local and two of these at once would count each other's frames. -/// -/// `unix` for the same reason `panel.rs`, `detail.rs` and `file_tree.rs` gate -/// theirs: a real pane means `test_window::harness_with_pane`, and that harness -/// hands back a `std::os::unix::net::UnixStream`. -#[cfg(all(test, unix))] +#[cfg(test)] mod render_idle_gpui_tests { use super::*; use crate::ui::app::{render_probe, test_window}; @@ -2290,9 +2286,16 @@ mod render_idle_gpui_tests { )); } - let (app, mut vcx, _pane) = test_window::harness_with_pane(cx); + // The daemon end is held for the life of the test, the way every other + // panel harness holds it. `&mut { _pane }` dropped it on the spot: on + // Unix a closed `socketpair` half still delivers the `Cwd` written a + // moment earlier, but on Windows the link is a loopback `TcpStream`, + // and closing one with the pane's own `Resize` sitting unread on it is + // an abortive close — the `Cwd` goes with the connection, and the test + // spends its 30s deadline waiting for a cwd that was thrown away. + let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); crate::daemon::protocol::DaemonMsg::Cwd(root.clone()) - .encode(&mut { _pane }) + .encode(&mut pane) .expect("the pane's socket takes the cwd"); app.update_in(&mut vcx, |app, _, cx| { app.right_panel_visible = true; diff --git a/src/ui/scm/panel.rs b/src/ui/scm/panel.rs index fc591cdf..f828597d 100644 --- a/src/ui/scm/panel.rs +++ b/src/ui/scm/panel.rs @@ -2894,7 +2894,7 @@ mod tests { /// `default_global`, which fires the global observers whether or not anything /// changed, and it is called every frame. A watcher that notified on every one /// of those would request a frame from inside a frame and never stop. -#[cfg(all(test, unix))] +#[cfg(test)] mod render_idle_gpui_tests { use super::*; use crate::daemon::protocol::DaemonMsg; @@ -2933,7 +2933,7 @@ mod render_idle_gpui_tests { ) -> ( Entity, VisualTestContext, - std::os::unix::net::UnixStream, + crate::daemon::transport::Stream, ) { let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); DaemonMsg::Cwd(root.to_path_buf()) @@ -2980,6 +2980,17 @@ mod render_idle_gpui_tests { render_probe::draws() } + /// The only test in this module that waits on `repo.root`, and so the only + /// one Windows cannot run: since #796 that root is keyed by one spelling + /// and carries a `Disk` prefix, while the pane's cwd came out of `scratch` + /// above — `std::fs::canonicalize`, so `\\?\C:\Users\—` and a + /// `VerbatimDisk` prefix — and the equality below never holds between the + /// two. The slashes are the red herring; `Path` compares by component, so + /// `C:/x` and `C:\x` are equal. Spelling `scratch`'s answer the way + /// `tty7_core::core::path_spelling` does should lift this, in a change + /// that can show it green. Its sibling keys off the pane's cwd instead, + /// and runs everywhere. + #[cfg(unix)] #[gpui::test] fn a_settled_source_control_panel_reaches_render_idle(cx: &mut TestAppContext) { let _serial = serial(); diff --git a/src/ui/switcher.rs b/src/ui/switcher.rs index e94f451d..9f318889 100644 --- a/src/ui/switcher.rs +++ b/src/ui/switcher.rs @@ -3720,7 +3720,7 @@ mod tests { } } -#[cfg(all(test, unix))] +#[cfg(test)] mod gpui_tests { use gpui::{Modifiers, TestAppContext}; diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index 5ea14edc..ba193b4e 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -3686,7 +3686,6 @@ mod tests { /// the window shows one, and the one it could not put up must not come out /// of a `Full` diff as `TabClose` — that op deleted from the machine exactly /// the tabs a restart had failed to bring back, panes and all (#672). - #[cfg(unix)] #[gpui::test] fn the_next_sync_leaves_a_tab_the_rebuild_could_not_put_up_on_the_machine( cx: &mut gpui::TestAppContext, @@ -3775,7 +3774,6 @@ mod tests { /// straight after clears the queue, so by the time a test can look the /// ops are gone either way — while `informed` outliving the arrival is /// both durable and the thing that made them possible. - #[cfg(unix)] #[gpui::test] fn arriving_at_a_workspace_does_not_prune_what_is_already_in_it(cx: &mut gpui::TestAppContext) { let (app, mut vcx, _pane_stream) = crate::ui::app::test_window::harness_with_pane(cx);