diff --git a/crates/fbuild-cli/tests/README.md b/crates/fbuild-cli/tests/README.md index 46fbaa8d8..b8641b390 100644 --- a/crates/fbuild-cli/tests/README.md +++ b/crates/fbuild-cli/tests/README.md @@ -8,6 +8,13 @@ and message contracts are covered end-to-end. HTTP daemon on an ephemeral loopback port, points the CLI at it via `FBUILD_DEV_MODE=1` + `FBUILD_DAEMON_PORT`, and asserts the CLI exits non-zero when the daemon returns a structured failure response. +- **`daemon_crash_recovery.rs`** -- regression for FastLED/fbuild#1228. + `#[ignore]`-gated (runs under `bash test --full`): spawns the real + `fbuild` + sibling `fbuild-daemon` binaries, kills the daemon uncleanly, + and asserts the very next CLI invocation respawns a fresh daemon and + reaches it instead of redialing the dead endpoint. Skips when a live dev + daemon owns the real `~/.fbuild/dev` root or no sibling daemon binary + exists. - **`ci_command.rs`** -- regression for FastLED/fbuild#242. Spawns the compiled `fbuild` binary and asserts that `ci --help` documents the PlatformIO-compatible flags (`--board`, `--lib`, `--project-conf`, diff --git a/crates/fbuild-cli/tests/daemon_crash_recovery.rs b/crates/fbuild-cli/tests/daemon_crash_recovery.rs new file mode 100644 index 000000000..f79f09f7b --- /dev/null +++ b/crates/fbuild-cli/tests/daemon_crash_recovery.rs @@ -0,0 +1,182 @@ +//! Real-process regression test for FastLED/fbuild#1228: after a daemon is +//! killed uncleanly (leaving stale port/pid/status records behind), the very +//! next CLI invocation must respawn a fresh daemon and reach it — the client +//! must never sit redialing the dead endpoint. +//! +//! This drives the REAL `fbuild` binary, which in turn spawns the REAL +//! sibling `fbuild-daemon` binary through the production spawn path +//! (`ensure_daemon_running` → sibling discovery → detached spawn). Isolation +//! notes, because the production spawn path rebuilds the daemon's +//! environment from the OS user baseline (`user_baseline_environment` +//! discards any test-provided `HOME`/`USERPROFILE`): +//! +//! - The daemon ALWAYS resolves the real `~/.fbuild/dev/` root. The test +//! therefore refuses to run (skips) when that root's `root-owner.lock` is +//! held — i.e. when a real dev daemon is alive on this machine — so it can +//! never displace or corrupt a daemon it does not own. +//! - Port and cache are still isolated: `FBUILD_DAEMON_PORT` (a free +//! ephemeral port) and `FBUILD_CACHE_DIR` (a tempdir) both survive the +//! spawn path's `FBUILD_*` propagation filter. +//! - `RUNNING_PROCESS_DISABLE=1` pins the legacy direct acquisition path so +//! the test deterministically exercises `ensure_direct_daemon_running` +//! (the path traced in #1228) rather than broker adoption. + +use std::path::PathBuf; +use std::process::Command; +use std::time::{Duration, Instant}; + +use fbuild_core::process_identity::{pid_is_alive, terminate_pid, wait_for_pid_exit}; +use fbuild_paths::daemon_ownership::RootOwnershipGuard; + +/// The real user home, the same way the spawned daemon will resolve it. +fn real_home() -> Option { + let key = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; + std::env::var_os(key).map(PathBuf::from) +} + +/// `/.fbuild/dev/daemon/root-owner.lock` — the dev-mode root +/// ownership lock the spawned daemon will contend for. Mirrors +/// `fbuild_paths` layout without mutating this process's env (env vars are +/// process-global and tests run multi-threaded). +fn dev_root_owner_lock(home: &std::path::Path) -> PathBuf { + home.join(".fbuild") + .join("dev") + .join("daemon") + .join("root-owner.lock") +} + +fn free_port() -> u16 { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind ephemeral port"); + listener.local_addr().expect("local addr").port() +} + +/// Run the fbuild CLI with the test's isolation env; returns (exit ok, stdout). +fn run_cli(args: &[&str], port: u16, cache_dir: &std::path::Path) -> (bool, String) { + let bin = env!("CARGO_BIN_EXE_fbuild"); + // allow-direct-spawn: test driver invoking the fbuild CLI binary under test. + let output = Command::new(bin) + .args(args) + .env("FBUILD_DEV_MODE", "1") + .env("FBUILD_DAEMON_PORT", port.to_string()) + .env("FBUILD_CACHE_DIR", cache_dir) + .env("RUNNING_PROCESS_DISABLE", "1") + .output() + .expect("spawn fbuild CLI"); + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + (output.status.success(), stdout) +} + +/// Parse ` PID: 12345` out of `fbuild daemon status` output. +fn parse_status_pid(status_stdout: &str) -> Option { + status_stdout.lines().find_map(|line| { + let rest = line.trim().strip_prefix("PID:")?; + rest.trim().parse().ok() + }) +} + +fn wait_for_health(port: u16, budget: Duration) -> bool { + let url = format!("http://127.0.0.1:{port}/health"); + let deadline = Instant::now() + budget; + while Instant::now() < deadline { + if let Ok(resp) = reqwest::blocking::get(&url) { + if resp.status().is_success() { + return true; + } + } + std::thread::sleep(Duration::from_millis(200)); + } + false +} + +#[test] +#[ignore = "spawns real fbuild + fbuild-daemon binaries (#1228)"] +fn client_recovers_after_daemon_is_killed_uncleanly() { + let Some(home) = real_home() else { + eprintln!("skip: no home directory resolvable"); + return; + }; + + // The production spawn path resolves the daemon binary as a sibling of + // the CLI. Under `cargo test --workspace` (and any full build) it exists; + // under an isolated `-p fbuild-cli` test run it may not — skip then. + let cli = PathBuf::from(env!("CARGO_BIN_EXE_fbuild")); + let daemon_name = if cfg!(windows) { + "fbuild-daemon.exe" + } else { + "fbuild-daemon" + }; + let sibling = cli.parent().map(|d| d.join(daemon_name)); + if !sibling.as_deref().is_some_and(std::path::Path::exists) { + eprintln!("skip: no sibling fbuild-daemon binary at {sibling:?} — build fbuild-daemon first"); + return; + } + + // Never contend with a real dev daemon: the spawned daemon will use the + // real ~/.fbuild/dev root (see module docs), so if something already owns + // it, running this test would try to displace a daemon we don't own. + let lock_path = dev_root_owner_lock(&home); + match RootOwnershipGuard::try_acquire_at(&lock_path) { + Ok(Some(guard)) => drop(guard), // free — safe to proceed + Ok(None) => { + eprintln!("skip: a live dev daemon holds {lock_path:?}"); + return; + } + Err(err) => { + eprintln!("skip: cannot probe {lock_path:?}: {err}"); + return; + } + } + + let cache_dir = tempfile::tempdir().expect("temp cache dir"); + let port = free_port(); + + // 1. Bring a daemon up through the production acquisition path. + let (ok, _) = run_cli(&["daemon", "restart"], port, cache_dir.path()); + assert!(ok, "initial `fbuild daemon restart` must succeed"); + assert!( + wait_for_health(port, Duration::from_secs(30)), + "daemon never became healthy on port {port}" + ); + let (ok, status) = run_cli(&["daemon", "status"], port, cache_dir.path()); + assert!(ok, "daemon status must succeed while daemon is up"); + let old_pid = parse_status_pid(&status) + .unwrap_or_else(|| panic!("no PID in daemon status output:\n{status}")); + assert!(pid_is_alive(old_pid), "freshly started daemon must be alive"); + + // 2. Crash it. This is the #1213/#1228 scenario: unclean death that + // leaves the port/pid/status records in place. + terminate_pid(old_pid); + assert!( + wait_for_pid_exit(old_pid, Duration::from_secs(15)), + "daemon (pid {old_pid}) did not die within 15s of terminate_pid" + ); + + // 3. The very next client invocation must recover: detect the dead + // endpoint, respawn, and reach the fresh daemon. `daemon restart` + // routes through the same `ensure_daemon_running` every build/deploy + // request uses. + let (ok, _) = run_cli(&["daemon", "restart"], port, cache_dir.path()); + assert!( + ok, + "client invocation after unclean daemon death must respawn and succeed (#1228)" + ); + assert!( + wait_for_health(port, Duration::from_secs(30)), + "respawned daemon never became healthy on port {port}" + ); + let (ok, status) = run_cli(&["daemon", "status"], port, cache_dir.path()); + assert!(ok, "daemon status must succeed after recovery"); + let new_pid = parse_status_pid(&status) + .unwrap_or_else(|| panic!("no PID in post-recovery status output:\n{status}")); + assert_ne!(new_pid, old_pid, "recovery must have spawned a NEW daemon"); + assert!(pid_is_alive(new_pid), "respawned daemon must be alive"); + + // 4. Cleanup: stop the daemon we spawned (also exercises the #1227 + // stale-record clearing) and confirm it exits. + let (ok, _) = run_cli(&["daemon", "stop"], port, cache_dir.path()); + assert!(ok, "daemon stop must succeed"); + assert!( + wait_for_pid_exit(new_pid, Duration::from_secs(15)), + "daemon (pid {new_pid}) did not exit within 15s of `daemon stop`" + ); +}