From e17bb77c519e913d0e822861ff205185df7de73f Mon Sep 17 00:00:00 2001 From: hanafish <1106510024@qq.com> Date: Fri, 7 Aug 2026 18:12:27 +0800 Subject: [PATCH 1/2] fix(test-runner): make runs cancellable, deadlock-free, and bounded --- src-tauri/crates/test-runner/Cargo.toml | 7 + src-tauri/crates/test-runner/src/capture.rs | 122 +++++++ src-tauri/crates/test-runner/src/commands.rs | 130 +++++-- src-tauri/crates/test-runner/src/lib.rs | 1 + src-tauri/crates/test-runner/src/runner.rs | 316 +++++++++++++----- .../test-runner/src/tests/commands_tests.rs | 134 ++++++++ src-tauri/crates/test-runner/src/tests/mod.rs | 1 + .../test-runner/src/tests/runner_tests.rs | 244 ++++++++++++++ src-tauri/crates/test-runner/src/types.rs | 18 + src/hooks/testRunner/useTestRunner.ts | 7 +- .../registration/actions/testActions.zod.ts | 10 +- src/services/test/TestService.ts | 57 +++- src/services/test/__tests__/TEST_CASES.md | 49 +++ .../test/__tests__/testRunLifecycle.test.ts | 117 +++++++ src/services/test/testRunLifecycle.ts | 59 ++++ src/types/testing/types.ts | 6 + 16 files changed, 1130 insertions(+), 148 deletions(-) create mode 100644 src-tauri/crates/test-runner/src/capture.rs create mode 100644 src-tauri/crates/test-runner/src/tests/commands_tests.rs create mode 100644 src-tauri/crates/test-runner/src/tests/runner_tests.rs create mode 100644 src/services/test/__tests__/TEST_CASES.md create mode 100644 src/services/test/__tests__/testRunLifecycle.test.ts create mode 100644 src/services/test/testRunLifecycle.ts diff --git a/src-tauri/crates/test-runner/Cargo.toml b/src-tauri/crates/test-runner/Cargo.toml index 6ec08fa8b..acb6b032e 100644 --- a/src-tauri/crates/test-runner/Cargo.toml +++ b/src-tauri/crates/test-runner/Cargo.toml @@ -21,8 +21,15 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1" tauri = { workspace = true } tokio = { workspace = true } +tokio-util = { workspace = true } # CancellationToken for run cancellation tracing = "0.1" uuid = { version = "1", features = ["v4"] } +[target.'cfg(unix)'.dependencies] +libc = "0.2" # process-group signalling on cancellation + +[target.'cfg(windows)'.dependencies] +app_platform = { path = "../app-platform" } # CREATE_NO_WINDOW for spawned test processes / taskkill + [dev-dependencies] app_utils = { path = "../app-utils", features = ["testing"] } diff --git a/src-tauri/crates/test-runner/src/capture.rs b/src-tauri/crates/test-runner/src/capture.rs new file mode 100644 index 000000000..f0da1e4c8 --- /dev/null +++ b/src-tauri/crates/test-runner/src/capture.rs @@ -0,0 +1,122 @@ +//! Bounded tail capture for child-process output streams. +//! +//! Test processes can emit arbitrarily large output (watch modes, verbose +//! reporters, runaway logging). Captured output must stay within a fixed +//! budget so a single run can never grow app memory without bound: once the +//! budget is exceeded the *oldest* bytes are dropped, keeping the tail — +//! summaries and failures print last, so the tail is the useful part for +//! both parsing and error reporting. + +use std::collections::VecDeque; + +use tokio::io::{AsyncRead, AsyncReadExt}; + +/// Output captured from one stream (stdout or stderr) of a test process. +#[derive(Debug)] +pub(crate) struct CapturedOutput { + /// Tail of the stream, lossily decoded as UTF-8. At most `max_bytes` + /// long (a byte or two shorter when truncation split a code point). + pub text: String, + /// True when the stream produced more than `max_bytes` and the head + /// was dropped. Parsers that need the full document (JSON reporters) + /// cannot succeed on truncated output. + pub truncated: bool, + /// Total bytes the stream produced, including dropped bytes. + pub total_bytes: u64, +} + +/// Read `reader` to EOF, retaining at most `max_bytes` of the tail. +/// +/// Reads in fixed-size chunks (never buffers a whole line), so a single +/// line larger than the budget — e.g. one giant JSON document — still +/// respects the bound. +pub(crate) async fn capture_stream( + mut reader: R, + max_bytes: usize, +) -> CapturedOutput { + let mut tail: VecDeque = VecDeque::new(); + let mut total_bytes: u64 = 0; + let mut chunk = [0u8; 8192]; + + loop { + match reader.read(&mut chunk).await { + Ok(0) => break, + Ok(n) => { + total_bytes += n as u64; + tail.extend(&chunk[..n]); + if tail.len() > max_bytes { + let excess = tail.len() - max_bytes; + tail.drain(..excess); + } + } + // Pipe errors (e.g. the child was killed mid-write) end the + // capture; whatever arrived so far is still returned. + Err(_) => break, + } + } + + let truncated = total_bytes > tail.len() as u64; + let bytes: Vec = tail.into(); + CapturedOutput { + text: String::from_utf8_lossy(&bytes).into_owned(), + truncated, + total_bytes, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn small_output_is_kept_verbatim() { + let captured = capture_stream(&b"hello\nworld\n"[..], 1024).await; + assert_eq!(captured.text, "hello\nworld\n"); + assert!(!captured.truncated); + assert_eq!(captured.total_bytes, 12); + } + + #[tokio::test] + async fn oversized_output_keeps_only_the_tail() { + let input: Vec = (0..10_000u32) + .flat_map(|i| format!("line-{i}\n").into_bytes()) + .collect(); + let captured = capture_stream(&input[..], 1000).await; + assert!(captured.truncated); + assert_eq!(captured.total_bytes, input.len() as u64); + assert!(captured.text.len() <= 1000); + // The tail must contain the *last* line, not the first. + assert!(captured.text.contains("line-9999")); + assert!(!captured.text.contains("line-0\n")); + } + + #[tokio::test] + async fn single_line_larger_than_budget_is_bounded() { + let input = vec![b'a'; 1_000_000]; + let captured = capture_stream(&input[..], 4096).await; + assert!(captured.truncated); + assert_eq!(captured.total_bytes, 1_000_000); + assert_eq!(captured.text.len(), 4096); + } + + #[tokio::test] + async fn truncation_mid_code_point_is_lossy_not_fatal() { + // 4-byte emoji repeated; a byte-oriented cut can land mid-sequence. + // Each orphaned lead-in byte decodes to one 3-byte U+FFFD, so the + // text may exceed the byte budget by a few replacement chars — the + // point is that decoding stays sane, not byte-exact. + let input: Vec = "😀".repeat(1000).into_bytes(); + let captured = capture_stream(&input[..], 10).await; + assert!(captured.truncated); + assert!(captured.text.len() <= 10 + 3 * "\u{FFFD}".len()); + assert!(captured.text.contains('😀')); + } + + #[tokio::test] + async fn empty_stream_yields_empty_capture() { + let captured = capture_stream(&b""[..], 1024).await; + assert_eq!(captured.text, ""); + assert!(!captured.truncated); + assert_eq!(captured.total_bytes, 0); + } +} diff --git a/src-tauri/crates/test-runner/src/commands.rs b/src-tauri/crates/test-runner/src/commands.rs index 5b7eb91da..0efdad30e 100644 --- a/src-tauri/crates/test-runner/src/commands.rs +++ b/src-tauri/crates/test-runner/src/commands.rs @@ -7,22 +7,63 @@ use crate::types::*; use std::collections::HashMap; use std::path::PathBuf; -use std::sync::Arc; -use tauri::{AppHandle, State}; -use tokio::sync::Mutex; - -/// State for tracking running test processes +use std::sync::Mutex; +use tauri::{AppHandle, Emitter, State}; +use tokio_util::sync::CancellationToken; + +/// Registry of in-flight test runs, keyed by the canonical `run_id`. +/// +/// `run_tests` mints one `run_id` per command invocation, registers it here +/// *before* any event is emitted, and hands the same id to the runner — so +/// the id the frontend sees on `run_started` is always a valid key for +/// `stop_tests`. Entries are removed when the run future completes (or is +/// dropped), so the map never retains terminal runs. pub struct TestRunnerState { - /// Map of run_id to cancellation flag - running: Arc>>, + running: Mutex>, } impl TestRunnerState { pub fn new() -> Self { Self { - running: Arc::new(Mutex::new(HashMap::new())), + running: Mutex::new(HashMap::new()), } } + + /// Register a new run and return the token the runner must observe. + fn begin(&self, run_id: &str) -> CancellationToken { + let token = CancellationToken::new(); + self.lock().insert(run_id.to_string(), token.clone()); + token + } + + /// Remove a run from the registry once its future settles. + fn finish(&self, run_id: &str) { + self.lock().remove(run_id); + } + + /// Signal cancellation for `run_id`. Returns `true` when an active run + /// was found and signalled, `false` when it already finished — a benign + /// race for callers, not an error. + fn request_stop(&self, run_id: &str) -> bool { + match self.lock().get(run_id) { + Some(token) => { + token.cancel(); + true + } + None => false, + } + } + + fn lock(&self) -> std::sync::MutexGuard<'_, HashMap> { + self.running + .lock() + .expect("test runner state lock poisoned") + } + + #[cfg(test)] + fn active_runs(&self) -> usize { + self.lock().len() + } } impl Default for TestRunnerState { @@ -31,6 +72,19 @@ impl Default for TestRunnerState { } } +/// Deregisters a run when the `run_tests` future settles — including when +/// Tauri drops the future because the invoking webview went away. +struct RunGuard<'a> { + state: &'a TestRunnerState, + run_id: &'a str, +} + +impl Drop for RunGuard<'_> { + fn drop(&mut self) { + self.state.finish(self.run_id); + } +} + /// Detect test framework in a project #[tauri::command] pub async fn detect_test_framework(workspace_path: String) -> Result { @@ -122,36 +176,40 @@ pub async fn run_tests( return Err("No test framework detected in project".to_string()); } - // Mark as running + // Mint the canonical run id and register it before the runner emits + // anything, so a stop request for the id seen on `run_started` always + // finds this entry. let run_id = uuid::Uuid::new_v4().to_string(); - { - let mut running = state.running.lock().await; - running.insert(run_id.clone(), false); - } - - // Run tests - let result = runner::run_tests(app, &path, detected_framework, test_ids).await; - - // Remove from running - { - let mut running = state.running.lock().await; - running.remove(&run_id); - } - - result + let cancel = state.begin(&run_id); + let _guard = RunGuard { + state: state.inner(), + run_id: &run_id, + }; + + let emit = move |event: TestEvent| { + let _ = app.emit("test-event", event); + }; + + runner::run_tests( + run_id.clone(), + &path, + detected_framework, + test_ids, + cancel, + &emit, + ) + .await } -/// Stop a running test +/// Signal cancellation for a running test run. +/// +/// Returns `true` when an active run was signalled; the terminated run then +/// reports itself via a `run_cancelled` event. Returns `false` when the run +/// had already finished — callers should treat that as "nothing to stop", +/// not as a failure. #[tauri::command] -pub async fn stop_tests(run_id: String, state: State<'_, TestRunnerState>) -> Result<(), String> { - let mut running = state.running.lock().await; - - if let Some(cancelled) = running.get_mut(&run_id) { - *cancelled = true; - Ok(()) - } else { - Err(format!("No running test with id: {}", run_id)) - } +pub async fn stop_tests(run_id: String, state: State<'_, TestRunnerState>) -> Result { + Ok(state.request_stop(&run_id)) } /// Get test patterns for a framework (useful for frontend filtering) @@ -162,3 +220,7 @@ pub fn get_test_patterns(framework: TestFramework) -> Vec { .map(|s| s.to_string()) .collect() } + +#[cfg(test)] +#[path = "tests/commands_tests.rs"] +mod tests; diff --git a/src-tauri/crates/test-runner/src/lib.rs b/src-tauri/crates/test-runner/src/lib.rs index 47a5cf76f..09e52103e 100644 --- a/src-tauri/crates/test-runner/src/lib.rs +++ b/src-tauri/crates/test-runner/src/lib.rs @@ -11,6 +11,7 @@ //! - Python: Pytest //! - Rust: `cargo test` +pub(crate) mod capture; pub mod commands; pub mod detection; pub mod discovery; diff --git a/src-tauri/crates/test-runner/src/runner.rs b/src-tauri/crates/test-runner/src/runner.rs index 55458a248..bda840226 100644 --- a/src-tauri/crates/test-runner/src/runner.rs +++ b/src-tauri/crates/test-runner/src/runner.rs @@ -1,3 +1,4 @@ +use crate::capture::{capture_stream, CapturedOutput}; use crate::detection::get_test_command; use crate::types::*; use regex::Regex; @@ -5,29 +6,50 @@ use regex::Regex; * Test Runner * * Executes tests and parses output from various test frameworks. - * Emits streaming events to the frontend via Tauri events. + * Emits streaming events through the provided sink (the Tauri command + * layer forwards them to the frontend as `test-event`). */ use std::path::Path; use std::process::Stdio; -use tauri::{AppHandle, Emitter}; -use tokio::io::{AsyncBufReadExt, BufReader}; -use tokio::process::Command; - -/// Run tests and stream results +use tokio::process::{Child, Command}; +use tokio_util::sync::CancellationToken; + +/// Per-stream capture budget. Big enough for the one-document JSON +/// reporters (Vitest/Jest/Mocha) on large suites; a stream beyond this is +/// tail-truncated so a runaway test process cannot grow app memory +/// without bound. +pub(crate) const MAX_CAPTURED_STREAM_BYTES: usize = 16 * 1024 * 1024; + +/// Courtesy interval between SIGTERM and SIGKILL on cancellation. Kept +/// short: stopping tests should feel immediate, and test processes have no +/// state worth a long graceful shutdown. +const TERMINATE_GRACE: std::time::Duration = std::time::Duration::from_millis(300); + +/// Event sink used by the runner. The command layer forwards each event to +/// the webview; tests can collect them directly. +pub type EventSink<'a> = &'a (dyn Fn(TestEvent) + Send + Sync); + +/// Run tests and stream results. +/// +/// `run_id` is the canonical identifier minted by the command layer: it is +/// stamped on every emitted event and on the returned summary, and it is +/// the key `stop_tests` uses to signal `cancel`. pub async fn run_tests( - app: AppHandle, + run_id: String, workspace_path: &Path, framework: TestFramework, test_ids: Option>, + cancel: CancellationToken, + emit: EventSink<'_>, ) -> Result { tracing::info!( framework = ?framework, project = %workspace_path.display(), test_ids = ?test_ids, + run_id = %run_id, "[TestRunner] Running tests" ); - let run_id = uuid::Uuid::new_v4().to_string(); let started_at = chrono::Utc::now().to_rfc3339(); let start_time = std::time::Instant::now(); @@ -69,85 +91,50 @@ pub async fn run_tests( } } - // Emit run started - let _ = app.emit( - "test-event", - TestEvent::RunStarted { - run_id: run_id.clone(), - total_tests: 0, - }, - ); + emit(TestEvent::RunStarted { + run_id: run_id.clone(), + total_tests: 0, + }); - // Spawn process tracing::info!( command = %cmd, args = ?args, cwd = %workspace_path.display(), "[TestRunner] Spawning process" ); - let mut child = Command::new(cmd) - .args(&args) - .current_dir(workspace_path) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|e| { - let error_msg = format!("Failed to spawn test process: {}", e); - tracing::error!(error = %error_msg, "[TestRunner] Failed to spawn process"); - error_msg - })?; - - tracing::info!("[TestRunner] Process spawned successfully, reading output"); - let stdout = child.stdout.take().ok_or("Failed to capture stdout")?; - let stderr = child.stderr.take().ok_or("Failed to capture stderr")?; - - // Read stdout and stderr - let stdout_reader = BufReader::new(stdout); - let stderr_reader = BufReader::new(stderr); - - let mut stdout_lines = stdout_reader.lines(); - let mut stderr_lines = stderr_reader.lines(); - - let mut stdout_output = String::new(); - let mut stderr_output = String::new(); - - // Read stdout - while let Ok(Some(line)) = stdout_lines.next_line().await { - stdout_output.push_str(&line); - stdout_output.push('\n'); - } - - // Read stderr - while let Ok(Some(line)) = stderr_lines.next_line().await { - stderr_output.push_str(&line); - stderr_output.push('\n'); - } - - // Wait for process - let status = child - .wait() - .await - .map_err(|e| format!("Test process failed: {}", e))?; + let capture = run_command_capture( + cmd, + &args, + workspace_path, + &cancel, + MAX_CAPTURED_STREAM_BYTES, + ) + .await + .map_err(|error| { + tracing::error!(error = %error, "[TestRunner] Failed to run test process"); + emit(TestEvent::Error { + message: error.clone(), + }); + error + })?; let duration_ms = start_time.elapsed().as_millis() as u64; let finished_at = chrono::Utc::now().to_rfc3339(); tracing::info!( - exit_code = ?status.code(), - stdout_bytes = stdout_output.len(), - stderr_bytes = stderr_output.len(), + exit_code = ?capture.status.and_then(|status| status.code()), + cancelled = capture.cancelled, + stdout_bytes = capture.stdout.total_bytes, + stderr_bytes = capture.stderr.total_bytes, + stdout_truncated = capture.stdout.truncated, + stderr_truncated = capture.stderr.truncated, "[TestRunner] Process finished" ); - if !stdout_output.is_empty() { - tracing::debug!(stdout = %stdout_output, "[TestRunner] Captured stdout"); - } - if !stderr_output.is_empty() { - tracing::debug!(stderr = %stderr_output, "[TestRunner] Captured stderr"); - } - // Parse output based on framework - let results = parse_test_output(&stdout_output, &stderr_output, &framework); + // Parse output based on framework. On cancellation or truncation this + // yields whatever completed tests are still visible in the tail. + let results = parse_test_output(&capture.stdout.text, &capture.stderr.text, &framework); tracing::info!( result_count = results.len(), @@ -168,22 +155,22 @@ pub async fn run_tests( } // Emit individual test result - let _ = app.emit( - "test-event", - TestEvent::TestFinished { - result: result.clone(), - }, - ); + emit(TestEvent::TestFinished { + result: result.clone(), + }); } - // If no results parsed but command failed, treat as error - if results.is_empty() && !status.success() { - let _ = app.emit( - "test-event", - TestEvent::Error { - message: format!("Test command failed: {}", stderr_output), - }, - ); + let succeeded = capture.status.map(|s| s.success()).unwrap_or(false); + + // If no results parsed but command failed (and was not cancelled by the + // user), surface the failure. + if results.is_empty() && !succeeded && !capture.cancelled { + emit(TestEvent::Error { + message: format!( + "Test command failed: {}", + tail_of(&capture.stderr.text, 4096) + ), + }); } let summary = TestRunSummary { @@ -197,19 +184,162 @@ pub async fn run_tests( results, started_at, finished_at: Some(finished_at), + cancelled: capture.cancelled, }; - // Emit run finished - let _ = app.emit( - "test-event", - TestEvent::RunFinished { + if capture.cancelled { + emit(TestEvent::RunCancelled { + run_id: run_id.clone(), + }); + } else { + emit(TestEvent::RunFinished { summary: summary.clone(), - }, - ); + }); + } Ok(summary) } +/// Outcome of executing one test command to completion or cancellation. +#[derive(Debug)] +pub(crate) struct CommandCapture { + /// Exit status. `None` only when the post-kill reap failed. + pub status: Option, + /// True when the run ended because `cancel` fired. + pub cancelled: bool, + pub stdout: CapturedOutput, + pub stderr: CapturedOutput, +} + +/// Spawn `cmd` and capture both output streams concurrently (bounded), until +/// the process exits or `cancel` fires — in which case the whole process +/// tree is terminated. +/// +/// Concurrent consumption is load-bearing: reading the streams sequentially +/// deadlocks once the unread pipe's buffer fills while the child blocks +/// writing to it. +pub(crate) async fn run_command_capture( + cmd: &str, + args: &[String], + cwd: &Path, + cancel: &CancellationToken, + max_stream_bytes: usize, +) -> Result { + let mut command = Command::new(cmd); + command + .args(args) + .current_dir(cwd) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // Safety net: if this future is dropped (e.g. the invoking webview + // reloads), the child must not outlive it. + .kill_on_drop(true); + + // Own process group / hidden console so cancellation can terminate the + // whole tree (npx → node → workers) and not just the wrapper. + #[cfg(unix)] + command.process_group(0); + #[cfg(windows)] + command.creation_flags(app_platform::CREATE_NO_WINDOW); + + let mut child = command + .spawn() + .map_err(|e| format!("Failed to spawn test process: {}", e))?; + + let stdout = child.stdout.take().ok_or("Failed to capture stdout")?; + let stderr = child.stderr.take().ok_or("Failed to capture stderr")?; + + let stdout_task = tokio::spawn(capture_stream(stdout, max_stream_bytes)); + let stderr_task = tokio::spawn(capture_stream(stderr, max_stream_bytes)); + + let mut cancelled = false; + let status = tokio::select! { + status = child.wait() => { + Some(status.map_err(|e| format!("Test process failed: {}", e))?) + } + _ = cancel.cancelled() => { + cancelled = true; + terminate_child_tree(&mut child).await + } + }; + + // The child is gone (exited or killed), so both pipes hit EOF and the + // capture tasks finish draining promptly. + let stdout = stdout_task + .await + .map_err(|e| format!("stdout capture task failed: {}", e))?; + let stderr = stderr_task + .await + .map_err(|e| format!("stderr capture task failed: {}", e))?; + + Ok(CommandCapture { + status, + cancelled, + stdout, + stderr, + }) +} + +/// Terminate the child and every process in its group/tree, then reap it. +/// +/// Unix: SIGTERM to the process group (the child is its own group leader), +/// a short grace, then SIGKILL to the group. The group signals are sent +/// while the leader is still unreaped, so the pgid cannot have been +/// recycled. Windows: `taskkill /T /F` walks the tree by parent PID. +async fn terminate_child_tree(child: &mut Child) -> Option { + let pid = child.id(); + + #[cfg(unix)] + { + if let Some(pid) = pid { + signal_process_group(pid, libc::SIGTERM); + tokio::time::sleep(TERMINATE_GRACE).await; + signal_process_group(pid, libc::SIGKILL); + } else { + // Already reaped elsewhere; nothing to signal. + let _ = child.start_kill(); + } + child.wait().await.ok() + } + + #[cfg(windows)] + { + if let Some(pid) = pid { + let mut taskkill = Command::new("taskkill"); + taskkill.args(["/PID", &pid.to_string(), "/T", "/F"]); + // Suppress the console window `taskkill` would otherwise flash. + taskkill.creation_flags(app_platform::CREATE_NO_WINDOW); + let _ = taskkill.output().await; + } + let _ = child.start_kill(); + child.wait().await.ok() + } +} + +/// Send `signal` to the process group led by `pid`. The child was spawned +/// with `process_group(0)`, so its PID is also its PGID. +#[cfg(unix)] +fn signal_process_group(pid: u32, signal: libc::c_int) { + // SAFETY: `libc::kill` is an FFI call with no Rust-side invariants; a + // stale PID simply yields `ESRCH`, which is harmless. + unsafe { + libc::kill(-(pid as libc::pid_t), signal); + } +} + +/// Last `max_bytes` of `s`, adjusted forward to a UTF-8 boundary. +fn tail_of(s: &str, max_bytes: usize) -> &str { + if s.len() <= max_bytes { + return s; + } + let mut start = s.len() - max_bytes; + while !s.is_char_boundary(start) { + start += 1; + } + &s[start..] +} + /// Parse test output based on framework fn parse_test_output(stdout: &str, stderr: &str, framework: &TestFramework) -> Vec { match framework { @@ -641,3 +771,7 @@ fn parse_mocha_json(json: &serde_json::Value) -> Vec { results } + +#[cfg(test)] +#[path = "tests/runner_tests.rs"] +mod tests; diff --git a/src-tauri/crates/test-runner/src/tests/commands_tests.rs b/src-tauri/crates/test-runner/src/tests/commands_tests.rs new file mode 100644 index 000000000..87ded79fe --- /dev/null +++ b/src-tauri/crates/test-runner/src/tests/commands_tests.rs @@ -0,0 +1,134 @@ +//! Tests for the run registry (`TestRunnerState`) and the wire shape of +//! `TestEvent` — the contract the frontend `TestEvent` union relies on. + +use super::*; + +#[test] +fn begin_stop_finish_lifecycle() { + let state = TestRunnerState::new(); + + let token = state.begin("run-1"); + assert_eq!(state.active_runs(), 1); + assert!(!token.is_cancelled()); + + assert!(state.request_stop("run-1"), "active run must be stoppable"); + assert!(token.is_cancelled(), "stop must signal the runner's token"); + + state.finish("run-1"); + assert_eq!(state.active_runs(), 0); + assert!( + !state.request_stop("run-1"), + "stopping a finished run reports false, not an error" + ); +} + +#[test] +fn parallel_runs_are_isolated() { + let state = TestRunnerState::new(); + let token_a = state.begin("run-a"); + let token_b = state.begin("run-b"); + + assert!(state.request_stop("run-a")); + assert!(token_a.is_cancelled()); + assert!( + !token_b.is_cancelled(), + "stopping run-a must not cancel run-b" + ); + + state.finish("run-a"); + assert_eq!(state.active_runs(), 1); + state.finish("run-b"); + assert_eq!(state.active_runs(), 0); +} + +#[test] +fn run_guard_deregisters_on_drop() { + let state = TestRunnerState::new(); + let _token = state.begin("run-1"); + { + let _guard = RunGuard { + state: &state, + run_id: "run-1", + }; + assert_eq!(state.active_runs(), 1); + } + // Guard dropped (as it would be if Tauri dropped the command future): + // the registry must not retain the terminal run. + assert_eq!(state.active_runs(), 0); +} + +#[test] +fn repeated_finish_is_idempotent() { + let state = TestRunnerState::new(); + let _token = state.begin("run-1"); + state.finish("run-1"); + state.finish("run-1"); + assert_eq!(state.active_runs(), 0); +} + +/// The frontend switch reads `data.runId` (camelCase). This pins the wire +/// format so a serde attribute regression cannot silently send `run_id` +/// again (which made the frontend see `undefined` run ids). +#[test] +fn test_event_wire_format_uses_camel_case_fields() { + let started = serde_json::to_value(TestEvent::RunStarted { + run_id: "r1".into(), + total_tests: 5, + }) + .expect("serialize"); + assert_eq!(started["type"], "run_started"); + assert_eq!(started["runId"], "r1"); + assert_eq!(started["totalTests"], 5); + assert!(started.get("run_id").is_none()); + + let cancelled = serde_json::to_value(TestEvent::RunCancelled { + run_id: "r1".into(), + }) + .expect("serialize"); + assert_eq!(cancelled["type"], "run_cancelled"); + assert_eq!(cancelled["runId"], "r1"); + + let test_started = serde_json::to_value(TestEvent::TestStarted { + test_id: "t1".into(), + name: "adds".into(), + }) + .expect("serialize"); + assert_eq!(test_started["testId"], "t1"); +} + +#[test] +fn summary_wire_format_includes_cancelled_flag() { + let summary = TestRunSummary { + run_id: "r1".into(), + framework: TestFramework::Vitest, + total: 0, + passed: 0, + failed: 0, + skipped: 0, + duration_ms: 12, + results: vec![], + started_at: "2026-08-07T00:00:00Z".into(), + finished_at: None, + cancelled: true, + }; + let value = serde_json::to_value(&summary).expect("serialize"); + assert_eq!(value["cancelled"], true); + assert_eq!(value["runId"], "r1"); + assert_eq!(value["durationMs"], 12); + + // Old persisted/serialized summaries without the flag still deserialize. + let legacy = serde_json::json!({ + "runId": "r0", + "framework": "vitest", + "total": 0, + "passed": 0, + "failed": 0, + "skipped": 0, + "durationMs": 1, + "results": [], + "startedAt": "2026-08-07T00:00:00Z", + "finishedAt": null + }); + let parsed: TestRunSummary = serde_json::from_value(legacy).expect("deserialize legacy"); + assert!(!parsed.cancelled); +} diff --git a/src-tauri/crates/test-runner/src/tests/mod.rs b/src-tauri/crates/test-runner/src/tests/mod.rs index 1bd3c4d10..f6b5515fa 100644 --- a/src-tauri/crates/test-runner/src/tests/mod.rs +++ b/src-tauri/crates/test-runner/src/tests/mod.rs @@ -1,4 +1,5 @@ const SOURCE_FILES: &[(&str, &str)] = &[ + ("capture.rs", include_str!("../capture.rs")), ("commands.rs", include_str!("../commands.rs")), ("discovery.rs", include_str!("../discovery.rs")), ("runner.rs", include_str!("../runner.rs")), diff --git a/src-tauri/crates/test-runner/src/tests/runner_tests.rs b/src-tauri/crates/test-runner/src/tests/runner_tests.rs new file mode 100644 index 000000000..be362f432 --- /dev/null +++ b/src-tauri/crates/test-runner/src/tests/runner_tests.rs @@ -0,0 +1,244 @@ +//! Lifecycle tests for the command-execution core of the runner: +//! concurrent bounded capture, cancellation, and process-tree termination. +//! +//! Shell-based cases are Unix-only; the capture/cancellation logic they +//! exercise is platform-independent (only the kill mechanics differ). + +use super::*; +use std::time::Duration; +use tokio_util::sync::CancellationToken; + +fn no_cancel() -> CancellationToken { + CancellationToken::new() +} + +#[cfg(unix)] +fn sh_args(script: &str) -> Vec { + vec!["-c".to_string(), script.to_string()] +} + +/// A process is (still) alive when `kill(pid, 0)` succeeds. +#[cfg(unix)] +fn process_alive(pid: i32) -> bool { + // SAFETY: signal 0 performs error checking only; it never signals. + unsafe { libc::kill(pid, 0) == 0 } +} + +#[cfg(unix)] +#[tokio::test] +async fn completed_run_captures_status_and_both_streams() { + let dir = std::env::temp_dir(); + let capture = tokio::time::timeout( + Duration::from_secs(10), + run_command_capture( + "sh", + &sh_args("echo out-line; echo err-line >&2; exit 3"), + &dir, + &no_cancel(), + MAX_CAPTURED_STREAM_BYTES, + ), + ) + .await + .expect("run must not hang") + .expect("run must succeed"); + + assert!(!capture.cancelled); + assert_eq!(capture.status.and_then(|s| s.code()), Some(3)); + assert!(capture.stdout.text.contains("out-line")); + assert!(capture.stderr.text.contains("err-line")); + assert!(!capture.stdout.truncated); + assert!(!capture.stderr.truncated); +} + +/// Regression: the old implementation read stdout to EOF before touching +/// stderr, so a child that filled the stderr pipe first deadlocked forever. +#[cfg(unix)] +#[tokio::test] +async fn large_stderr_before_stdout_does_not_deadlock_and_is_bounded() { + let dir = std::env::temp_dir(); + // ~2 MB to stderr first (far beyond the ~64 KiB pipe buffer), then stdout. + let script = "yes err | head -c 2000000 >&2; yes out | head -c 2000000"; + let capture = tokio::time::timeout( + Duration::from_secs(30), + run_command_capture("sh", &sh_args(script), &dir, &no_cancel(), 64 * 1024), + ) + .await + .expect("concurrent capture must not deadlock") + .expect("run must succeed"); + + assert!(!capture.cancelled); + assert_eq!(capture.stderr.total_bytes, 2_000_000); + assert_eq!(capture.stdout.total_bytes, 2_000_000); + assert!(capture.stderr.truncated); + assert!(capture.stdout.truncated); + assert!(capture.stderr.text.len() <= 64 * 1024); + assert!(capture.stdout.text.len() <= 64 * 1024); +} + +/// Cancellation must terminate the whole process group — including a +/// grandchild the direct child spawned — and report `cancelled`. +#[cfg(unix)] +#[tokio::test] +async fn cancel_kills_process_group_including_grandchildren() { + let dir = std::env::temp_dir(); + let cancel = CancellationToken::new(); + + // The child prints its background grandchild's PID, then waits on it. + let script = "sleep 300 & echo $!; wait $!"; + + let cancel_trigger = cancel.clone(); + let trigger = tokio::spawn(async move { + // Give the shell time to spawn and print the grandchild PID. + tokio::time::sleep(Duration::from_millis(300)).await; + cancel_trigger.cancel(); + }); + + let started = std::time::Instant::now(); + let capture = tokio::time::timeout( + Duration::from_secs(10), + run_command_capture( + "sh", + &sh_args(script), + &dir, + &cancel, + MAX_CAPTURED_STREAM_BYTES, + ), + ) + .await + .expect("cancellation must resolve promptly") + .expect("run must succeed"); + trigger.await.expect("trigger task"); + + assert!(capture.cancelled, "outcome must be marked cancelled"); + assert!( + started.elapsed() < Duration::from_secs(8), + "cancel must not wait for the 300s sleep" + ); + + let grandchild_pid: i32 = capture + .stdout + .text + .lines() + .next() + .expect("grandchild pid line") + .trim() + .parse() + .expect("grandchild pid parses"); + + // SIGKILL delivery is asynchronous; poll briefly. + let mut alive = process_alive(grandchild_pid); + for _ in 0..40 { + if !alive { + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + alive = process_alive(grandchild_pid); + } + assert!( + !alive, + "grandchild {grandchild_pid} must be dead after group termination" + ); +} + +/// A token cancelled before the run starts must still terminate promptly. +#[cfg(unix)] +#[tokio::test] +async fn pre_cancelled_token_stops_run_immediately() { + let dir = std::env::temp_dir(); + let cancel = CancellationToken::new(); + cancel.cancel(); + + let capture = tokio::time::timeout( + Duration::from_secs(10), + run_command_capture( + "sh", + &sh_args("sleep 300"), + &dir, + &cancel, + MAX_CAPTURED_STREAM_BYTES, + ), + ) + .await + .expect("pre-cancelled run must resolve promptly") + .expect("run must succeed"); + + assert!(capture.cancelled); +} + +#[tokio::test] +async fn spawn_failure_surfaces_as_error() { + let dir = std::env::temp_dir(); + let result = run_command_capture( + "definitely-not-a-real-binary-orgii", + &[], + &dir, + &no_cancel(), + MAX_CAPTURED_STREAM_BYTES, + ) + .await; + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Failed to spawn test process")); +} + +#[test] +fn tail_of_respects_char_boundaries() { + assert_eq!(tail_of("hello", 10), "hello"); + assert_eq!(tail_of("hello", 3), "llo"); + // "😀" is 4 bytes; a 5-byte tail of "a😀😀" would split the first emoji, + // so the boundary moves forward past it. + assert_eq!(tail_of("a😀😀", 5), "😀"); + assert_eq!(tail_of("", 5), ""); +} + +/// Full-pipeline check of event ordering and the canonical run id: every +/// event and the returned summary must carry the id the caller minted. +#[cfg(unix)] +#[tokio::test] +async fn cancelled_run_emits_run_started_then_run_cancelled_with_same_id() { + use std::sync::{Arc, Mutex}; + + // Minimal directory satisfying the Cargo framework preconditions; the + // run is cancelled immediately, so no real project is needed. + let (_tempdir, root) = app_utils::testing::temp_dir_with_files(&[( + "Cargo.toml", + "[package]\nname = \"x\"\nversion = \"0.0.0\"\n", + )]); + let cancel = CancellationToken::new(); + cancel.cancel(); // cancel immediately: no dependence on real test output + + let events: Arc>> = Arc::new(Mutex::new(Vec::new())); + let sink_events = events.clone(); + let emit = move |event: TestEvent| { + sink_events.lock().expect("events lock").push(event); + }; + + let summary = run_tests( + "run-under-test".to_string(), + &root, + TestFramework::Cargo, + None, + cancel, + &emit, + ) + .await + .expect("cancelled run still returns a summary"); + + assert_eq!(summary.run_id, "run-under-test"); + assert!(summary.cancelled); + + let events = events.lock().expect("events lock"); + match events.first() { + Some(TestEvent::RunStarted { run_id, .. }) => assert_eq!(run_id, "run-under-test"), + other => panic!("first event must be RunStarted, got {other:?}"), + } + match events.last() { + Some(TestEvent::RunCancelled { run_id }) => assert_eq!(run_id, "run-under-test"), + other => panic!("last event must be RunCancelled, got {other:?}"), + } + assert!( + !events + .iter() + .any(|event| matches!(event, TestEvent::RunFinished { .. })), + "a cancelled run must not also report RunFinished" + ); +} diff --git a/src-tauri/crates/test-runner/src/types.rs b/src-tauri/crates/test-runner/src/types.rs index c0af57a95..ed1adc6e0 100644 --- a/src-tauri/crates/test-runner/src/types.rs +++ b/src-tauri/crates/test-runner/src/types.rs @@ -86,16 +86,34 @@ pub struct TestRunSummary { pub results: Vec, pub started_at: String, pub finished_at: Option, + /// True when the run was stopped before the test process finished on + /// its own. `results` then only cover whatever completed before the + /// process was terminated. + #[serde(default)] + pub cancelled: bool, } /// Events emitted during test run (for streaming to frontend) +/// +/// Variant *tags* are snake_case (`run_started`), but the *fields* must be +/// camelCase to match the frontend `TestEvent` union in +/// `src/types/testing/types.ts` — hence the per-variant `rename_all`. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] pub enum TestEvent { + #[serde(rename_all = "camelCase")] RunStarted { run_id: String, total_tests: u32 }, + #[serde(rename_all = "camelCase")] TestStarted { test_id: String, name: String }, + #[serde(rename_all = "camelCase")] TestFinished { result: TestResult }, + #[serde(rename_all = "camelCase")] RunFinished { summary: TestRunSummary }, + /// The run was stopped via `stop_tests` and the test process (tree) has + /// been terminated. + #[serde(rename_all = "camelCase")] + RunCancelled { run_id: String }, + #[serde(rename_all = "camelCase")] Error { message: String }, } diff --git a/src/hooks/testRunner/useTestRunner.ts b/src/hooks/testRunner/useTestRunner.ts index 98299e21f..23b1757ad 100644 --- a/src/hooks/testRunner/useTestRunner.ts +++ b/src/hooks/testRunner/useTestRunner.ts @@ -54,7 +54,8 @@ export interface UseTestRunnerReturn { runTests: (testIds?: string[]) => Promise; runTest: (testId: string) => Promise; runAllTests: () => Promise; - stopTests: () => Promise; + /** Resolves true when an active run was signalled to stop. */ + stopTests: () => Promise; clearResults: () => void; // Standardized actions sub-object for dispatcher integration @@ -64,7 +65,7 @@ export interface UseTestRunnerReturn { runAll: () => Promise; runFile: (filePath: string) => Promise; runTests: (testIds?: string[]) => Promise; - stop: () => Promise; + stop: () => Promise; clear: () => void; }; } @@ -113,7 +114,7 @@ export function useTestRunner({ return TestService.runAll(repoPath); }, [repoPath]); - const stopTests = useCallback(async (): Promise => { + const stopTests = useCallback(async (): Promise => { return TestService.stop(); }, []); diff --git a/src/modules/WorkStation/ActionSystem/registration/actions/testActions.zod.ts b/src/modules/WorkStation/ActionSystem/registration/actions/testActions.zod.ts index 5f50ea7d4..62f152321 100644 --- a/src/modules/WorkStation/ActionSystem/registration/actions/testActions.zod.ts +++ b/src/modules/WorkStation/ActionSystem/registration/actions/testActions.zod.ts @@ -70,8 +70,14 @@ export function createTestZodActions(repoPath: string) { examples: ["stop tests", "cancel tests"], }, async () => { - await TestService.stop(); - return { success: true, message: "Tests stopped" }; + const stopped = await TestService.stop(); + if (!stopped) { + return { success: false, message: "No running test run to stop" }; + } + return { + success: true, + message: "Stop requested; the run will report as cancelled", + }; } ); diff --git a/src/services/test/TestService.ts b/src/services/test/TestService.ts index 975dd093d..5f321b2dd 100644 --- a/src/services/test/TestService.ts +++ b/src/services/test/TestService.ts @@ -11,6 +11,7 @@ import { createLogger } from "@src/hooks/logger"; import { clearResultsAtom, + currentRunAtom, lastRunSummaryAtom, setCurrentRunAtom, setDiscoveringAtom, @@ -32,6 +33,8 @@ import { listenTauri, } from "@src/util/platform/tauri/init"; +import { nextRunState, stopTargetRunId } from "./testRunLifecycle"; + const log = createLogger("TestService"); // ============================================ @@ -62,12 +65,18 @@ async function initializeEventListener(): Promise { switch (data.type) { case "run_started": - store.set(setCurrentRunAtom, { - runId: data.runId, - status: "running", - progress: 0, - }); + case "run_finished": + case "run_cancelled": { + const current = store.get(currentRunAtom); + const next = nextRunState(current, data); + if (next !== current) { + store.set(setCurrentRunAtom, next); + } + if (data.type === "run_finished") { + store.set(lastRunSummaryAtom, data.summary); + } break; + } case "test_started": store.set(updateTestResultAtom, { @@ -80,15 +89,6 @@ async function initializeEventListener(): Promise { store.set(updateTestResultAtom, data.result); break; - case "run_finished": - store.set(setCurrentRunAtom, { - runId: data.summary.runId, - status: "completed", - progress: 100, - }); - store.set(lastRunSummaryAtom, data.summary); - break; - case "error": log.error("[TestService] Test error:", data.message); break; @@ -234,11 +234,32 @@ export const TestService = { }, /** - * Stop running tests + * Stop the currently running test run. + * + * Signals the backend, which terminates the whole test process tree; the + * run then reports itself as cancelled via a `run_cancelled` event (this + * method does not mutate run state locally — the backend confirmation is + * the source of truth). + * + * Returns true when an active run was signalled, false when there was + * nothing to stop (or the run had already finished). */ - async stop(): Promise { - // TODO: Implement when backend supports cancellation - getStore().set(setCurrentRunAtom, null); + async stop(): Promise { + if (!isTauriReady()) { + return false; + } + + const runId = stopTargetRunId(getStore().get(currentRunAtom)); + if (!runId) { + return false; + } + + try { + return await invokeTauri("stop_tests", { runId }); + } catch (error) { + log.error("[TestService] Failed to stop tests:", error); + return false; + } }, /** diff --git a/src/services/test/__tests__/TEST_CASES.md b/src/services/test/__tests__/TEST_CASES.md new file mode 100644 index 000000000..69d56b8dc --- /dev/null +++ b/src/services/test/__tests__/TEST_CASES.md @@ -0,0 +1,49 @@ +# Test Cases: TestService run lifecycle (run / stop / cancel) + +## Preconditions + +- A workspace with a detectable test framework (e.g. Vitest) is open. +- The Testing tab (`EditorPrimarySidebar → TestingTab`) is visible. +- Tauri backend is running (`test_runner` crate commands registered). + +## Happy Path + +| # | Steps | Expected Result | +|---|----|-----| +| 1 | Click "Run All Tests" | Run starts; toolbar toggles to "Stop Tests"; results stream in as tests finish | +| 2 | Let the run finish | Run state becomes `completed`; summary (passed/failed counts) shown; toolbar back to "Run All Tests" | +| 3 | Click "Run All Tests", then click "Stop Tests" mid-run | Test process tree terminates within ~1s; run state becomes `cancelled` (not `completed`); toolbar returns to "Run All Tests" | +| 4 | Run again after a stop | New run starts normally with a fresh run id | + +## Edge Cases + +| # | Scenario | Steps | Expected Result | +|---|----|----|-----| +| 1 | Stop with no run | Invoke the `TEST_STOP` action while idle | Action reports `success: false`, "No running test run to stop"; no state change | +| 2 | Stop races run completion | Stop right as the run finishes | Backend returns `false` (nothing to signal); run stays `completed`; no error surfaced | +| 3 | Huge test output | Suite logging tens of MB (or a single giant JSON line) | Run completes; memory bounded (16 MiB tail per stream); no hang even when stderr floods before stdout | +| 4 | Test process spawns children | Framework wrapper (npx → node → workers) | Stop kills the whole process tree, not just the wrapper (`ps` shows no orphaned workers) | +| 5 | Parallel runs (two windows) | Start runs in two windows, stop one | Only the stopped run reports `cancelled`; the other completes untouched | +| 6 | Rapid repeated stop clicks | Click Stop multiple times quickly | First click signals; later clicks are no-ops (`false`); no duplicate cancel events | + +## Error / Degraded States + +| # | Scenario | Steps | Expected Result | +|---|----|----|-----| +| 1 | Command fails with no parseable results | Break the test config, run | `error` event with the tail of stderr (bounded), run reports finished with 0 results | +| 2 | Spawn failure | Framework binary missing | Run promise rejects with "Failed to spawn test process"; error event emitted | +| 3 | Webview reload mid-run | Reload the app while tests run | Child process killed (kill_on_drop); registry entry removed (no leak) | + +## Accessibility + +- [ ] Run/Stop toolbar control keyboard-reachable (existing `IconButton` in TestingTab) +- [ ] Status changes reflected in the tree (existing status icons) + +## Acceptance Criteria + +- [ ] `stop()` resolves true only after the backend confirms an active run was signalled +- [ ] Cancelled runs end in `cancelled` state, never `completed` +- [ ] `run_started` events carry a real `runId` (camelCase wire format) +- [ ] A stopped run's process group is fully terminated (no orphaned test processes) +- [ ] Captured output per stream never exceeds the 16 MiB budget +- [ ] Stale parallel-run terminal events do not clobber the tracked run diff --git a/src/services/test/__tests__/testRunLifecycle.test.ts b/src/services/test/__tests__/testRunLifecycle.test.ts new file mode 100644 index 000000000..eabbd2bbe --- /dev/null +++ b/src/services/test/__tests__/testRunLifecycle.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from "vitest"; + +import type { TestEvent, TestRunState, TestRunSummary } from "@src/types/testing"; + +import { nextRunState, stopTargetRunId } from "../testRunLifecycle"; + +function createRunState(overrides?: Partial): TestRunState { + return { runId: "run-1", status: "running", progress: 0, ...overrides }; +} + +function createSummary(overrides?: Partial): TestRunSummary { + return { + runId: "run-1", + framework: "vitest", + total: 1, + passed: 1, + failed: 0, + skipped: 0, + durationMs: 10, + results: [], + startedAt: "2026-08-07T00:00:00Z", + ...overrides, + }; +} + +describe("nextRunState", () => { + it("starts tracking a run on run_started", () => { + const next = nextRunState(null, { + type: "run_started", + runId: "run-1", + totalTests: 3, + }); + expect(next).toEqual({ runId: "run-1", status: "running", progress: 0 }); + }); + + it("a newly started run supersedes the tracked one", () => { + const next = nextRunState(createRunState(), { + type: "run_started", + runId: "run-2", + totalTests: 0, + }); + expect(next).toEqual({ runId: "run-2", status: "running", progress: 0 }); + }); + + it("completes the tracked run on run_finished", () => { + const next = nextRunState(createRunState(), { + type: "run_finished", + summary: createSummary(), + }); + expect(next).toEqual({ runId: "run-1", status: "completed", progress: 100 }); + }); + + it("ignores run_finished from a stale parallel run", () => { + const current = createRunState({ runId: "run-2" }); + const next = nextRunState(current, { + type: "run_finished", + summary: createSummary({ runId: "run-1" }), + }); + expect(next).toBe(current); + }); + + it("marks the tracked run cancelled on run_cancelled", () => { + const next = nextRunState(createRunState({ progress: 40 }), { + type: "run_cancelled", + runId: "run-1", + }); + expect(next).toEqual({ runId: "run-1", status: "cancelled", progress: 40 }); + }); + + it("ignores run_cancelled from a stale parallel run", () => { + const current = createRunState({ runId: "run-2" }); + const next = nextRunState(current, { + type: "run_cancelled", + runId: "run-1", + }); + expect(next).toBe(current); + }); + + it("applies terminal events even when no run is tracked", () => { + expect( + nextRunState(null, { type: "run_finished", summary: createSummary() }) + ).toEqual({ runId: "run-1", status: "completed", progress: 100 }); + expect( + nextRunState(null, { type: "run_cancelled", runId: "run-1" }) + ).toEqual({ runId: "run-1", status: "cancelled", progress: 0 }); + }); + + it("leaves run state untouched for per-test and error events", () => { + const current = createRunState(); + const events: TestEvent[] = [ + { type: "test_started", testId: "t1", name: "adds" }, + { + type: "test_finished", + result: { testId: "t1", status: "passed" }, + }, + { type: "error", message: "boom" }, + ]; + for (const event of events) { + expect(nextRunState(current, event)).toBe(current); + } + }); +}); + +describe("stopTargetRunId", () => { + it("targets the tracked running run", () => { + expect(stopTargetRunId(createRunState())).toBe("run-1"); + }); + + it("returns null when nothing is tracked", () => { + expect(stopTargetRunId(null)).toBeNull(); + }); + + it("returns null for already-terminal runs", () => { + expect(stopTargetRunId(createRunState({ status: "completed" }))).toBeNull(); + expect(stopTargetRunId(createRunState({ status: "cancelled" }))).toBeNull(); + }); +}); diff --git a/src/services/test/testRunLifecycle.ts b/src/services/test/testRunLifecycle.ts new file mode 100644 index 000000000..9b96b36c8 --- /dev/null +++ b/src/services/test/testRunLifecycle.ts @@ -0,0 +1,59 @@ +/** + * Pure state transitions for the shared current-run state. + * + * Extracted from TestService so the run lifecycle (started → completed / + * cancelled, stale-run guards, stop targeting) is unit-testable without + * Tauri or the Jotai store. + */ +import type { TestEvent, TestRunState } from "@src/types/testing"; + +/** + * Next current-run state after a backend test event. + * + * Terminal events (`run_finished` / `run_cancelled`) only apply to the run + * currently being tracked: with parallel runs, a stale run finishing must + * not clobber the state of the run the UI is following. + */ +export function nextRunState( + current: TestRunState | null, + event: TestEvent +): TestRunState | null { + switch (event.type) { + case "run_started": + return { runId: event.runId, status: "running", progress: 0 }; + + case "run_finished": { + if (current && current.runId !== event.summary.runId) { + return current; + } + return { + runId: event.summary.runId, + status: "completed", + progress: 100, + }; + } + + case "run_cancelled": { + if (current && current.runId !== event.runId) { + return current; + } + return { + runId: event.runId, + status: "cancelled", + progress: current?.progress ?? 0, + }; + } + + default: + return current; + } +} + +/** + * The run a stop request should target, or null when there is nothing + * running — stopping an idle or already-terminal run is a no-op, not an + * error. + */ +export function stopTargetRunId(current: TestRunState | null): string | null { + return current && current.status === "running" ? current.runId : null; +} diff --git a/src/types/testing/types.ts b/src/types/testing/types.ts index e9681ea91..d10413c01 100644 --- a/src/types/testing/types.ts +++ b/src/types/testing/types.ts @@ -70,6 +70,11 @@ export interface TestRunSummary { results: TestResult[]; startedAt: string; finishedAt?: string; + /** + * True when the run was stopped before the test process finished on its + * own; `results` then only cover tests that completed before termination. + */ + cancelled?: boolean; } /** Discovery result from backend */ @@ -88,6 +93,7 @@ export type TestEvent = | { type: "test_started"; testId: string; name: string } | { type: "test_finished"; result: TestResult } | { type: "run_finished"; summary: TestRunSummary } + | { type: "run_cancelled"; runId: string } | { type: "error"; message: string }; // ============================================ From e0aaa48a175da27e36328adbc67145ae6ea761a5 Mon Sep 17 00:00:00 2001 From: Harry19081 <20519290+Harry19081@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:12:44 +0800 Subject: [PATCH 2/2] fix(ci): satisfy lifecycle frontend checks --- .../PropertyField/PropertyFieldEditable.tsx | 2 +- src/services/test/__tests__/testRunLifecycle.test.ts | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/components/PropertyField/PropertyFieldEditable.tsx b/src/components/PropertyField/PropertyFieldEditable.tsx index c286465dd..7cfc099d3 100644 --- a/src/components/PropertyField/PropertyFieldEditable.tsx +++ b/src/components/PropertyField/PropertyFieldEditable.tsx @@ -54,7 +54,7 @@ export const FieldRow: React.FC = ({ value, valueClassName = "", isSelected, - isActive, + isActive = false, showChevron = true, usePencil = false, suffix, diff --git a/src/services/test/__tests__/testRunLifecycle.test.ts b/src/services/test/__tests__/testRunLifecycle.test.ts index eabbd2bbe..c1549c468 100644 --- a/src/services/test/__tests__/testRunLifecycle.test.ts +++ b/src/services/test/__tests__/testRunLifecycle.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import type { TestEvent, TestRunState, TestRunSummary } from "@src/types/testing"; +import type { + TestEvent, + TestRunState, + TestRunSummary, +} from "@src/types/testing"; import { nextRunState, stopTargetRunId } from "../testRunLifecycle"; @@ -47,7 +51,11 @@ describe("nextRunState", () => { type: "run_finished", summary: createSummary(), }); - expect(next).toEqual({ runId: "run-1", status: "completed", progress: 100 }); + expect(next).toEqual({ + runId: "run-1", + status: "completed", + progress: 100, + }); }); it("ignores run_finished from a stale parallel run", () => {