diff --git a/.yoi/tickets/00001KSKBP9YG/item.md b/.yoi/tickets/00001KSKBP9YG/item.md index f262ef79..892e7e23 100644 --- a/.yoi/tickets/00001KSKBP9YG/item.md +++ b/.yoi/tickets/00001KSKBP9YG/item.md @@ -2,7 +2,7 @@ title: "E2E テストハーネス" state: 'inprogress' created_at: "2026-05-27T00:00:02Z" -updated_at: '2026-06-13T14:38:03Z' +updated_at: '2026-06-13T15:00:29Z' queued_by: 'yoi ticket' queued_at: '2026-06-13T14:17:34Z' --- diff --git a/.yoi/tickets/00001KSKBP9YG/thread.md b/.yoi/tickets/00001KSKBP9YG/thread.md index aecd50dd..0aa60548 100644 --- a/.yoi/tickets/00001KSKBP9YG/thread.md +++ b/.yoi/tickets/00001KSKBP9YG/thread.md @@ -218,4 +218,51 @@ Remaining gaps / risks: - Fixture setup blocks Companion/Orchestrator role auto-spawn by placing intentionally invalid isolated test pod metadata in the fixture data dir; this avoids leaking live background Pods but should be replaced by a cleaner fixture strategy if the Panel gains an explicit test fixture mode. +--- + + + +## Implementation report + +Follow-up implementation update for review-required changes: + +Mouse E2E false-positive prevention: +- `PanelHarness` now tracks the PTY output for terminal mouse mode enable/disable escape sequences. +- It requires both normal mouse tracking (`ESC[?1000h`, with disable `ESC[?1000l`) and SGR mouse mode (`ESC[?1006h`, with disable `ESC[?1006l`) to have been observed as currently enabled. +- Added `PanelHarness::expect_mouse_capture_enabled()` and the Panel mouse E2E calls it before clicking. +- `PanelHarness::click(...)` also fails with `MouseCaptureNotEnabled` if mouse capture has not been observed, so raw SGR mouse injection alone cannot make the test pass. + +Production binary contamination reduction: +- Added explicit feature gates: + - `tui/e2e-test` + - `yoi/e2e-test = ["tui/e2e-test"]` +- The `YOI_TUI_TEST_EVENTS` JSONL event path and the background-task hold seam are compiled only under `tui/e2e-test`. +- Normal builds still type-check a no-op facade so call sites stay simple, but env-var observability/hold behavior is compiled out unless the feature is enabled. +- E2E binary build command is now: `cargo build -p yoi --features e2e-test`. + +Quit latency barrier strengthening: +- Added a feature-gated test-only seam `YOI_TUI_TEST_HOLD_BACKGROUND_TASK=reload` that holds the Panel reload task pending in the E2E binary until quit aborts it. +- Added `PanelHarness::expect_background_task_pending("reload")`, which verifies that a `background_task_started` event exists and no matching `background_task_finished` or `background_task_aborted` event has superseded it. +- The quit latency test now uses the hold seam and asserts pending reload work immediately before sending Ctrl+C through the PTY. + +Harness/artifact adjustment: +- E2E fixture artifacts now persist under workspace `target/e2e-artifacts/...` instead of package-local temporary dirs, so failure artifacts remain inspectable. + +Validation after update: +- `cargo fmt --check` — passed. +- `cargo build -p yoi --features e2e-test` — passed. +- `YOI_E2E_BIN=/home/hare/Projects/yoi/.worktree/e2e-harness/target/debug/yoi cargo test -p yoi-e2e --features e2e --test panel -- --nocapture` — passed (2 tests). +- `git diff --check` — passed. +- `cargo check -p tui --all-targets` — passed. +- `cargo check -p yoi --all-targets` — passed. +- `cargo check -p tui --all-targets --features e2e-test` — passed. +- `cargo check -p yoi --all-targets --features e2e-test` — passed. +- `cargo check -p yoi-e2e --all-targets --features e2e` — passed. + +Remaining gaps / risks: +- The E2E harness remains Unix PTY based. +- The screen artifact remains raw PTY output, not a parsed terminal snapshot. +- The test-only pending-task hold intentionally changes timing only for the `e2e-test` binary; it is not compiled into normal builds. + + --- diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index 14911805..48112528 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -4,6 +4,10 @@ version = "0.1.0" edition.workspace = true license.workspace = true +[features] +default = [] +e2e-test = [] + [dependencies] client = { workspace = true } protocol = { workspace = true } diff --git a/crates/tui/src/e2e_observer.rs b/crates/tui/src/e2e_observer.rs index 91a81a8e..8c2749cf 100644 --- a/crates/tui/src/e2e_observer.rs +++ b/crates/tui/src/e2e_observer.rs @@ -1,57 +1,89 @@ -use std::fs::{File, OpenOptions}; -use std::io::Write; -use std::path::PathBuf; -use std::sync::{Mutex, OnceLock}; -use std::time::{SystemTime, UNIX_EPOCH}; +#[cfg(feature = "e2e-test")] +mod imp { + use std::fs::{File, OpenOptions}; + use std::io::Write; + use std::path::PathBuf; + use std::sync::{Mutex, OnceLock}; + use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use serde::Serialize; + use serde::Serialize; -const EVENT_PATH_ENV: &str = "YOI_TUI_TEST_EVENTS"; + const EVENT_PATH_ENV: &str = "YOI_TUI_TEST_EVENTS"; + const HOLD_BACKGROUND_TASK_ENV: &str = "YOI_TUI_TEST_HOLD_BACKGROUND_TASK"; -static EVENT_WRITER: OnceLock>> = OnceLock::new(); + static EVENT_WRITER: OnceLock>> = OnceLock::new(); -#[derive(Serialize)] -struct EventEnvelope<'a, T> { - ts_ms: u128, - surface: &'a str, - event: &'a str, - data: T, -} + #[derive(Serialize)] + struct EventEnvelope<'a, T> { + ts_ms: u128, + surface: &'a str, + event: &'a str, + data: T, + } -pub(crate) fn emit(surface: &'static str, event: &'static str, data: T) -where - T: Serialize, -{ - let Some(writer) = EVENT_WRITER.get_or_init(open_event_writer).as_ref() else { - return; - }; - let Ok(mut writer) = writer.lock() else { - return; - }; - let envelope = EventEnvelope { - ts_ms: SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_millis()) - .unwrap_or_default(), - surface, - event, - data, - }; - if serde_json::to_writer(&mut *writer, &envelope).is_ok() { - let _ = writer.write_all(b"\n"); - let _ = writer.flush(); + pub(crate) fn emit(surface: &'static str, event: &'static str, data: T) + where + T: Serialize, + { + let Some(writer) = EVENT_WRITER.get_or_init(open_event_writer).as_ref() else { + return; + }; + let Ok(mut writer) = writer.lock() else { + return; + }; + let envelope = EventEnvelope { + ts_ms: SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_millis()) + .unwrap_or_default(), + surface, + event, + data, + }; + if serde_json::to_writer(&mut *writer, &envelope).is_ok() { + let _ = writer.write_all(b"\n"); + let _ = writer.flush(); + } + } + + pub(crate) async fn hold_background_task_if_requested(task: &'static str) { + let requested = std::env::var(HOLD_BACKGROUND_TASK_ENV).unwrap_or_default(); + if !requested + .split(',') + .map(str::trim) + .any(|requested| requested == task) + { + return; + } + emit( + "panel", + "background_task_hold_started", + serde_json::json!({ "task": task }), + ); + loop { + tokio::time::sleep(Duration::from_millis(25)).await; + } + } + + fn open_event_writer() -> Option> { + let path = std::env::var_os(EVENT_PATH_ENV).map(PathBuf::from)?; + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + OpenOptions::new() + .create(true) + .append(true) + .open(path) + .ok() + .map(Mutex::new) } } -fn open_event_writer() -> Option> { - let path = std::env::var_os(EVENT_PATH_ENV).map(PathBuf::from)?; - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - OpenOptions::new() - .create(true) - .append(true) - .open(path) - .ok() - .map(Mutex::new) -} +#[cfg(feature = "e2e-test")] +pub(crate) use imp::{emit, hold_background_task_if_requested}; + +#[cfg(not(feature = "e2e-test"))] +pub(crate) fn emit(_surface: &'static str, _event: &'static str, _data: T) {} + +#[cfg(not(feature = "e2e-test"))] +pub(crate) async fn hold_background_task_if_requested(_task: &'static str) {} diff --git a/crates/tui/src/multi_pod.rs b/crates/tui/src/multi_pod.rs index 43e39d2e..8175fb5e 100644 --- a/crates/tui/src/multi_pod.rs +++ b/crates/tui/src/multi_pod.rs @@ -264,6 +264,7 @@ impl PendingReload { }), ); self.handle = Some(tokio::spawn(async move { + crate::e2e_observer::hold_background_task_if_requested("reload").await; load_multi_pod_snapshot(None, lifecycle_mode).await })); true diff --git a/crates/yoi/Cargo.toml b/crates/yoi/Cargo.toml index 21f265e6..0121f6a2 100644 --- a/crates/yoi/Cargo.toml +++ b/crates/yoi/Cargo.toml @@ -4,6 +4,10 @@ version = "0.1.0" edition.workspace = true license.workspace = true +[features] +default = [] +e2e-test = ["tui/e2e-test"] + [dependencies] project-record = { workspace = true } chrono = { version = "0.4", default-features = false, features = ["clock"] } diff --git a/tests/e2e/src/lib.rs b/tests/e2e/src/lib.rs index 2fb65e82..5c26b6e0 100644 --- a/tests/e2e/src/lib.rs +++ b/tests/e2e/src/lib.rs @@ -9,16 +9,17 @@ use std::io::{self, Read, Write}; use std::os::fd::{AsRawFd, FromRawFd}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; use serde_json::Value; -use tempfile::TempDir; const DEFAULT_WAIT: Duration = Duration::from_secs(5); const DEFAULT_EXIT_WAIT: Duration = Duration::from_millis(1500); +static FIXTURE_COUNTER: AtomicU64 = AtomicU64::new(0); pub type Result = std::result::Result; @@ -37,6 +38,9 @@ pub enum HarnessError { artifacts: PanelArtifacts, }, MissingBinary(PathBuf), + MouseCaptureNotEnabled { + artifacts: PanelArtifacts, + }, Protocol(String), } @@ -62,9 +66,14 @@ impl std::fmt::Display for HarnessError { ), Self::MissingBinary(path) => write!( f, - "missing yoi binary {}; run `cargo build -p yoi` or set YOI_E2E_BIN", + "missing yoi binary {}; run `cargo build -p yoi --features e2e-test` or set YOI_E2E_BIN", path.display() ), + Self::MouseCaptureNotEnabled { artifacts } => write!( + f, + "terminal mouse capture was not observed before mouse input; artifacts at {}", + artifacts.dir.display() + ), Self::Protocol(message) => write!(f, "protocol error: {message}"), } } @@ -93,6 +102,7 @@ pub struct PanelHarnessConfig { pub xdg_state_home: PathBuf, pub xdg_config_home: PathBuf, pub terminal_size: (u16, u16), + pub hold_background_task: Option, pub artifacts_dir: PathBuf, } @@ -190,6 +200,7 @@ impl PanelHarness { "columns": config.terminal_size.0, "rows": config.terminal_size.1, }, + "hold_background_task": config.hold_background_task, }))?, )?; @@ -212,6 +223,9 @@ impl PanelHarness { .stdin(Stdio::from(slave_for_stdin)) .stdout(Stdio::from(slave_for_stdout)) .stderr(Stdio::from(slave)); + if let Some(task) = &config.hold_background_task { + command.env("YOI_TUI_TEST_HOLD_BACKGROUND_TASK", task); + } let child = command.spawn()?; let output = Arc::new(Mutex::new(Vec::new())); @@ -298,7 +312,58 @@ impl PanelHarness { serde_json::from_value(event.data).map_err(HarnessError::from) } + pub fn expect_mouse_capture_enabled(&mut self) -> Result<()> { + let start = Instant::now(); + loop { + if self.mouse_capture_enabled() { + return Ok(()); + } + if start.elapsed() >= DEFAULT_WAIT { + self.flush_output_artifact()?; + return Err(HarnessError::MouseCaptureNotEnabled { + artifacts: self.artifacts.clone(), + }); + } + if let Some(status) = self.child.try_wait()? { + self.flush_output_artifact()?; + return Err(HarnessError::Protocol(format!( + "process exited with {status} before mouse capture was enabled" + ))); + } + thread::sleep(Duration::from_millis(20)); + } + } + + pub fn expect_background_task_pending(&mut self, task: &str) -> Result<()> { + let start = Instant::now(); + loop { + if background_task_is_pending(&self.events()?, task) { + return Ok(()); + } + if start.elapsed() >= DEFAULT_WAIT { + self.flush_output_artifact()?; + return Err(HarnessError::Timeout { + what: format!("background task {task:?} pending"), + artifacts: self.artifacts.clone(), + }); + } + if let Some(status) = self.child.try_wait()? { + self.flush_output_artifact()?; + return Err(HarnessError::Protocol(format!( + "process exited with {status} before background task {task:?} was pending" + ))); + } + thread::sleep(Duration::from_millis(20)); + } + } + pub fn click(&mut self, row: &RenderedPanelRow) -> Result<()> { + if !self.mouse_capture_enabled() { + self.flush_output_artifact()?; + return Err(HarnessError::MouseCaptureNotEnabled { + artifacts: self.artifacts.clone(), + }); + } let x = row.rect.x.saturating_add(1); let y = row.rect.y; self.write_input( @@ -332,9 +397,7 @@ impl PanelHarness { loop { if let Some(status) = self.child.try_wait()? { self.flush_output_artifact()?; - if let Some(reader) = self.reader.take() { - let _ = reader.join(); - } + let _ = self.reader.take(); return Ok(status); } if start.elapsed() >= timeout { @@ -394,6 +457,13 @@ impl PanelHarness { Ok(()) } + fn mouse_capture_enabled(&self) -> bool { + self.output + .lock() + .map(|output| output_has_enabled_mouse_capture(&output)) + .unwrap_or(false) + } + fn flush_output_artifact(&self) -> Result<()> { if let Ok(output) = self.output.lock() { fs::write(&self.artifacts.output_log, &*output)?; @@ -409,15 +479,13 @@ impl Drop for PanelHarness { let _ = self.child.wait(); } let _ = self.flush_output_artifact(); - if let Some(reader) = self.reader.take() { - let _ = reader.join(); - } + let _ = self.reader.take(); } } #[derive(Debug)] pub struct FixtureWorkspace { - _temp: TempDir, + pub root: PathBuf, pub workspace: PathBuf, pub home: PathBuf, pub xdg_data_home: PathBuf, @@ -428,8 +496,22 @@ pub struct FixtureWorkspace { impl FixtureWorkspace { pub fn new(binary: &Path) -> Result { - let temp = tempfile::Builder::new().prefix("yoi-e2e-").tempdir()?; - let root = temp.path(); + let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .ok_or_else(|| { + HarnessError::Protocol("could not resolve workspace root for artifacts".to_owned()) + })? + .to_path_buf(); + let root = workspace_root + .join("target") + .join("e2e-artifacts") + .join(format!( + "{}-{}-{}", + std::process::id(), + now_ms(), + FIXTURE_COUNTER.fetch_add(1, Ordering::Relaxed) + )); let workspace = root.join("workspace"); let home = root.join("home"); let xdg_data_home = root.join("data"); @@ -485,7 +567,7 @@ impl FixtureWorkspace { "Planning E2E Ticket", )?; Ok(Self { - _temp: temp, + root, workspace, home, xdg_data_home, @@ -504,9 +586,20 @@ impl FixtureWorkspace { xdg_state_home: self.xdg_state_home.clone(), xdg_config_home: self.xdg_config_home.clone(), terminal_size: (100, 32), + hold_background_task: None, artifacts_dir: self.artifacts_dir.clone(), } } + + pub fn panel_config_holding_background_task( + &self, + binary: PathBuf, + task: impl Into, + ) -> PanelHarnessConfig { + let mut config = self.panel_config(binary); + config.hold_background_task = Some(task.into()); + config + } } pub fn yoi_binary() -> PathBuf { @@ -628,6 +721,45 @@ fn write_blocking_pod_metadata(data_home: &Path, pod_name: &str) -> Result<()> { Ok(()) } +fn output_has_enabled_mouse_capture(output: &[u8]) -> bool { + mouse_mode_enabled(output, b"\x1b[?1000h", b"\x1b[?1000l") + && mouse_mode_enabled(output, b"\x1b[?1006h", b"\x1b[?1006l") +} + +fn mouse_mode_enabled(output: &[u8], enable: &[u8], disable: &[u8]) -> bool { + let last_enable = last_subsequence_index(output, enable); + let last_disable = last_subsequence_index(output, disable); + match (last_enable, last_disable) { + (Some(enable), Some(disable)) => enable > disable, + (Some(_), None) => true, + _ => false, + } +} + +fn last_subsequence_index(haystack: &[u8], needle: &[u8]) -> Option { + if needle.is_empty() || haystack.len() < needle.len() { + return None; + } + haystack + .windows(needle.len()) + .rposition(|window| window == needle) +} + +fn background_task_is_pending(events: &[HarnessEvent], task: &str) -> bool { + let mut pending = false; + for event in events { + if event.data.get("task").and_then(Value::as_str) != Some(task) { + continue; + } + match event.event.as_str() { + "background_task_started" => pending = true, + "background_task_finished" | "background_task_aborted" => pending = false, + _ => {} + } + } + pending +} + fn now_ms() -> u128 { SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/tests/e2e/tests/panel.rs b/tests/e2e/tests/panel.rs index 0bce20b6..87a9638c 100644 --- a/tests/e2e/tests/panel.rs +++ b/tests/e2e/tests/panel.rs @@ -8,6 +8,7 @@ fn panel_mouse_click_selects_row_without_dispatching_action() -> yoi_e2e::Result let fixture = FixtureWorkspace::new(&binary)?; let mut panel = PanelHarness::spawn(fixture.panel_config(binary))?; + panel.expect_mouse_capture_enabled()?; let rows = panel.wait_for_rows(2)?; let selected = rows.selected.clone(); let target = rows @@ -40,19 +41,13 @@ fn panel_mouse_click_selects_row_without_dispatching_action() -> yoi_e2e::Result fn panel_ctrl_c_exits_promptly_after_background_barrier() -> yoi_e2e::Result<()> { let binary = yoi_binary(); let fixture = FixtureWorkspace::new(&binary)?; - let mut panel = PanelHarness::spawn(fixture.panel_config(binary))?; + let mut panel = + PanelHarness::spawn(fixture.panel_config_holding_background_task(binary, "reload"))?; panel.wait_for("panel_ready", Duration::from_secs(5), |event| { event.event == "panel_ready" })?; - assert!( - panel - .events()? - .iter() - .any(|event| event.event == "background_task_started"), - "background task barrier was not observed; artifacts at {}", - panel.artifacts().dir.display() - ); + panel.expect_background_task_pending("reload")?; let started = std::time::Instant::now(); panel.press(KeyPress::CtrlC)?;