pauljd/craft-libs
1
0
Fork 0
mirror of https://github.com/storytold/craft-libs.git synced 2026-11-09 16:39:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

craft-update: self-update tests wait on handshakes instead of time

Browse Source 
The Windows helper tests hand step_after_wait a wait that reports when
the helper reaches it and blocks until the test has reaped the stand-in
launcher, so the old-file check no longer depends on a 500 ms sleep or a
2 s observation window. The test-only wait_watch hooks are removed. The
launcher-that-keeps-running test asks the real wait with no timeout
instead of bounding the elapsed time.

The Unix restart test puts a sleep script on the helper's PATH that
reports each pause through a named pipe, and the started command reports
whether the old process was still running. A thread that waits for the
helper to exit writes to the same pipe, so a helper that dies early fails
the test instead of hanging it.
This commit is contained in:
dotCooCoo
2026-11-07 20:34:29 -08:00
parent 10d1cbb4ab
commit 8ecf7fa15d
1 changed files with 119 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/update/src/self_update.rs
+119 -136
View File
@@ -1112,55 +1112,6 @@ fn check_helper(app: &App, me: &Path, paths: &Paths, exe: &Path, step: &Finish)
if allowed { Ok(()) } else { Err(refused()) }
}
/// Lets a Windows test observe both sides of `run_step`'s wait for the old launcher. Compiled only
/// for tests, so production behavior is unchanged.
#[cfg(all(test, windows))]
mod wait_watch {
use std::sync::mpsc::{self, Receiver, Sender};
use std::sync::{Mutex, PoisonError};
/// A test's two observations of one wait, keyed by the launcher's pid.
pub(super) struct Channels {
/// Sent just before `run_step` calls the wait.
pub(super) entered: Receiver<()>,
/// Sent right after the wait returns, before any extraction, and carries the wait's result.
pub(super) returned: Receiver<bool>,
}
struct Watch {
pid: u32,
entered: Option<Sender<()>>,
returned: Option<Sender<bool>>,
}
static WATCH: Mutex<Option<Watch>> = Mutex::new(None);
pub(super) fn watch(pid: u32) -> Channels {
let (entered, entered_rx) = mpsc::channel();
let (returned, returned_rx) = mpsc::channel();
*WATCH.lock().unwrap_or_else(PoisonError::into_inner) = Some(Watch { pid, entered: Some(entered), returned: Some(returned) });
Channels { entered: entered_rx, returned: returned_rx }
}
/// Notes that `run_step` is about to wait. Only the first call for a watched pid is kept.
pub(super) fn entered(pid: u32) {
let mut watch = WATCH.lock().unwrap_or_else(PoisonError::into_inner);
let sender = watch.as_mut().filter(|watch| watch.pid == pid).and_then(|watch| watch.entered.take());
if let Some(sender) = sender {
let _ = sender.send(());
}
}
/// Notes the wait's result as soon as it returns; the step hasn't run yet.
pub(super) fn returned(pid: u32, waited: bool) {
let mut watch = WATCH.lock().unwrap_or_else(PoisonError::into_inner);
let sender = watch.as_mut().filter(|watch| watch.pid == pid).and_then(|watch| watch.returned.take());
if let Some(sender) = sender {
let _ = sender.send(waited);
}
}
}
/// [`step_after_wait`], waiting up to `wait` for `pid` to exit.
#[cfg(all(windows, test))]
fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: std::time::Duration) -> Result<(), String> {
@@ -1180,12 +1131,7 @@ struct Stepped {
/// `reference`.
#[cfg(windows)]
fn step_after_wait(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: &dyn Fn(u32) -> bool) -> Stepped {
#[cfg(test)]
wait_watch::entered(pid);
// The test hook needs the result, so it is stored rather than consumed by the branch.
let waited = wait(pid);
#[cfg(test)]
wait_watch::returned(pid, waited);
let result = if waited {
match step {
Finish::Msi(msi) => {
@@ -1364,18 +1310,23 @@ mod tests {
let tmp = dir.path().join("cache").join("launcher-update");
let step = Finish::Portable { zip: zip.clone(), tmp: tmp.clone() };
let mut old = std::process::Command::new("ping").args(["-n", "3", "127.0.0.1"]).stdout(std::process::Stdio::null()).spawn().unwrap();
let mut old = BlockedLauncher::start();
let exe = folder.join("artcraft-launcher.exe");
let helper = {
let (exe, step, pid) = (exe.clone(), step.clone(), old.id());
std::thread::spawn(move || {
run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &exe, &step, std::time::Duration::from_secs(30))
})
};
std::thread::sleep(std::time::Duration::from_millis(500));
assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running");
old.wait().unwrap();
helper.join().unwrap().unwrap();
std::thread::scope(|scope| {
// Inside the scope, so a failed assertion drops the gate and releases the helper
// before the scope joins it.
let (gate, wait) = WaitGate::new();
let (helper_exe, pid) = (exe.clone(), old.id());
let helper = scope.spawn(move || step_after_wait(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, &wait));
gate.entered();
old.assert_running("while the helper was waiting");
assert_eq!(std::fs::read(&exe).unwrap(), b"0.1.0", "replaced while the old launcher was still running");
old.finish().expect("the stand-in launcher couldn't be reaped");
gate.release();
let stepped = helper.join().unwrap();
assert!(stepped.exited, "the wait reported that the launcher was still running although it had exited");
stepped.result.unwrap();
});
assert_eq!(std::fs::read(&exe).unwrap(), signed_exe);
assert_eq!(std::fs::read_to_string(folder.join("ArtCraftLauncherData").join("settings.json")).unwrap(), "{}");
assert!(!folder.join("artcraft-launcher-0.2.0-windows-x64-portable").exists(), "the zip's folder must not be nested in the launcher's");
@@ -1434,20 +1385,52 @@ mod tests {
}
}
/// A wait for [`step_after_wait`] that the test controls: it reports when the helper has
/// reached its wait, blocks until the test releases it, and then asks the real
/// [`process::wait_for_exit`] whether the launcher has exited, without waiting.
#[cfg(windows)]
struct WaitGate {
entered: std::sync::mpsc::Receiver<()>,
release: std::sync::mpsc::Sender<()>,
}
#[cfg(windows)]
impl WaitGate {
/// The gate and the wait to hand to the helper. The wait holds the only sender of
/// `entered`, so a helper that ends without reaching its wait fails [`WaitGate::entered`]
/// instead of blocking it, and dropping the gate releases the wait.
fn new() -> (Self, impl Fn(u32) -> bool + Send) {
let (entered_tx, entered) = std::sync::mpsc::channel();
let (release, released) = std::sync::mpsc::channel::<()>();
let released = std::sync::Mutex::new(released);
let wait = move |pid| {
let _ = entered_tx.send(());
let _ = released.lock().map(|released| released.recv());
process::wait_for_exit(pid, std::time::Duration::ZERO)
};
(Self { entered, release }, wait)
}
/// Blocks until the helper has reached its wait.
fn entered(&self) {
self.entered.recv().expect("the helper never reached its wait");
}
/// Lets the helper's wait return.
fn release(&self) {
self.release.send(()).expect("the helper stopped before its wait was released");
}
}
/// A launcher without `portable.txt` is a standalone installation, and the helper still
/// replaces its own executable after it exits. The test holds a stand-in launcher alive
/// through a pipe and observes both sides of the helper's wait: the first assertion runs only
/// after the helper announces it is about to wait. A wait that returns while the stand-in is alive
/// fails the test before any extraction can replace the executable.
///
/// The "still blocked" check is a bounded observation, not proof: a helper descheduled for the
/// whole window could hide a missing wait until the release below. A premature timeout longer
/// than this observation window is also outside this test's coverage.
/// through a pipe and holds the helper inside its wait until the stand-in has exited, so the
/// executable must still be the old one while the helper waits, and the real
/// [`process::wait_for_exit`] must report the live stand-in as running and the exited one as gone.
#[cfg(windows)]
#[test]
fn a_standalone_update_replaces_the_launcher_after_it_exits() {
use std::io::Write;
use std::sync::mpsc::RecvTimeoutError;
let dir = tempfile::tempdir().unwrap();
let folder = dir.path().join("ArtCraft Launcher");
std::fs::create_dir_all(folder.join("ArtCraftLauncherData")).unwrap();
@@ -1468,49 +1451,23 @@ mod tests {
let tmp = dir.path().join("cache").join(PORTABLE_STAGING);
let step = Finish::Portable { zip: zip.clone(), tmp: tmp.clone() };
// Timeouts only bound the test; each failure names the observation that never arrived.
let entered_timeout = std::time::Duration::from_secs(10);
let blocked_window = std::time::Duration::from_secs(2);
let returned_timeout = std::time::Duration::from_secs(30);
let mut old = BlockedLauncher::start();
let pid = old.id();
std::thread::scope(|scope| {
let mut old = BlockedLauncher::start();
let pid = old.id();
let watch = wait_watch::watch(pid);
// Inside the scope, so a failed assertion drops the gate and releases the helper
// before the scope joins it.
let (gate, wait) = WaitGate::new();
let helper_exe = exe.clone();
let step = step.clone();
let helper = scope.spawn(move || {
run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, std::time::Duration::from_secs(30))
});
match watch.entered.recv_timeout(entered_timeout) {
Ok(()) => {}
Err(RecvTimeoutError::Timeout) => {
panic!("the helper didn't reach its wait within {entered_timeout:?} (helper already finished: {})", helper.is_finished())
}
Err(RecvTimeoutError::Disconnected) => panic!("the entered channel disconnected before the helper reached its wait"),
}
old.assert_running("before the first replacement check");
assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running");
// A correct wait can't return while the stand-in holds the pipe.
match watch.returned.recv_timeout(blocked_window) {
Err(RecvTimeoutError::Timeout) => {}
Ok(waited) => panic!("run_step left its wait while the old launcher was still running (waited: {waited})"),
Err(RecvTimeoutError::Disconnected) => panic!("the returned channel disconnected while the old launcher was still running"),
}
old.assert_running("after the wait was expected to stay blocked");
assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running");
let helper = scope.spawn(move || step_after_wait(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, &wait));
gate.entered();
old.assert_running("while the helper was waiting");
assert_eq!(std::fs::read(&exe).unwrap(), b"0.1.0", "replaced while the old launcher was still running");
assert!(!process::wait_for_exit(pid, std::time::Duration::ZERO), "the wait reported the running launcher as exited");
old.finish().expect("the stand-in launcher couldn't be reaped");
match watch.returned.recv_timeout(returned_timeout) {
Ok(waited) => assert!(waited, "the wait reported that the launcher was still running although it had exited"),
Err(RecvTimeoutError::Timeout) => panic!(
"the helper didn't leave its wait within {returned_timeout:?} after the launcher exited (helper already finished: {})",
helper.is_finished()
),
Err(RecvTimeoutError::Disconnected) => panic!("the returned channel disconnected before reporting the wait's result"),
}
helper.join().unwrap().unwrap();
gate.release();
let stepped = helper.join().unwrap();
assert!(stepped.exited, "the wait reported that the launcher was still running although it had exited");
stepped.result.unwrap();
});
assert_eq!(std::fs::read(&exe).unwrap(), signed_exe);
assert_eq!(std::fs::read_to_string(folder.join("ArtCraftLauncherData").join("settings.json")).unwrap(), "{}");
@@ -1730,19 +1687,11 @@ mod tests {
let msi = dir.path().join("artcraft-launcher-0.2.0-windows-x64.msi");
std::fs::write(&msi, "not a package").unwrap();
let running = std::process::id();
let started = std::time::Instant::now();
// The test process itself never exits within the wait, so the package is never run.
let error = run_step(
&crate::suite::LAUNCHER,
&system_program("cmd.exe"),
running,
&exe,
&Finish::Msi(msi.clone()),
std::time::Duration::from_millis(300),
)
.unwrap_err();
// The test process itself is still running when the wait asks, so the package is never run.
let error =
run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), running, &exe, &Finish::Msi(msi.clone()), std::time::Duration::ZERO)
.unwrap_err();
assert!(error.contains("still running"), "{error}");
assert!(started.elapsed() < std::time::Duration::from_secs(30));
assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0");
assert!(!msi.exists());
}
@@ -1977,22 +1926,56 @@ mod tests {
}
}
/// The helper waits for the old launcher to exit before it starts the new one.
/// The helper waits for the old launcher to exit before it starts the new one. The helper's
/// `sleep` is a script on `PATH` that reports each pause through a named pipe and pauses until
/// the test releases it, and the started command reports whether the old launcher was still
/// running. The test holds both pipes open for reading and writing, so no open blocks, and a
/// thread that waits for the helper to exit writes `exited`, so every read gets a line.
#[cfg(unix)]
#[test]
fn the_restart_waits_for_the_old_process_to_exit() {
use std::io::{BufRead, Write};
use std::os::unix::fs::PermissionsExt;
let dir = tempfile::tempdir().unwrap();
let marker = dir.path().join("started");
let mut old = std::process::Command::new("sleep").arg("1").spawn().unwrap();
relaunch_after_exit(&crate::suite::LAUNCHER, old.id(), &[std::ffi::OsStr::new("touch"), marker.as_os_str()]).unwrap();
std::thread::sleep(std::time::Duration::from_millis(400));
assert!(!marker.exists(), "started while the old launcher was still running");
old.wait().unwrap();
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while !marker.exists() && std::time::Instant::now() < deadline {
std::thread::sleep(std::time::Duration::from_millis(50));
let (events, release) = (dir.path().join("events"), dir.path().join("release"));
for fifo in [&events, &release] {
assert!(std::process::Command::new("mkfifo").arg(fifo).status().unwrap().success());
}
assert!(marker.exists(), "never started after the old launcher exited");
let bin = dir.path().join("bin");
std::fs::create_dir(&bin).unwrap();
let sleep = bin.join("sleep");
std::fs::write(&sleep, "#!/bin/sh\nprintf 'waiting\\n' > \"$CRAFT_TEST_EVENTS\"\nread _ < \"$CRAFT_TEST_RELEASE\"\n").unwrap();
std::fs::set_permissions(&sleep, std::fs::Permissions::from_mode(0o755)).unwrap();
let mut path = std::ffi::OsString::from(bin.as_os_str());
if let Some(rest) = std::env::var_os("PATH") {
path.push(":");
path.push(rest);
}
let mut old = std::process::Command::new("cat").stdin(std::process::Stdio::piped()).stdout(std::process::Stdio::null()).spawn().unwrap();
let pid = old.id().to_string();
let started = r#"if kill -0 "$1" 2>/dev/null; then s=alive; else s=gone; fi; printf 'started %s\n' "$s" > "$CRAFT_TEST_EVENTS""#;
let command = [std::ffi::OsStr::new("/bin/sh"), "-c".as_ref(), started.as_ref(), "sh".as_ref(), pid.as_ref()];
let open = |fifo: &Path| std::fs::OpenOptions::new().read(true).write(true).open(fifo).unwrap();
let (events_pipe, mut release_pipe) = (open(&events), open(&release));
let mut exited = events_pipe.try_clone().unwrap();
let mut lines = std::io::BufReader::new(events_pipe).lines();
let mut next = move || lines.next().unwrap().unwrap();
let mut helper = helper_command(&crate::suite::LAUNCHER, old.id(), &command);
helper.env("PATH", path).env("CRAFT_TEST_EVENTS", &events).env("CRAFT_TEST_RELEASE", &release);
let mut helper = helper.stdin(std::process::Stdio::null()).stdout(std::process::Stdio::null()).spawn().unwrap();
std::thread::spawn(move || {
let _ = helper.wait();
let _ = exited.write_all(b"exited\n");
});
assert_eq!(next(), "waiting", "the helper didn't wait while the old launcher was still running");
drop(old.stdin.take());
old.wait().unwrap();
release_pipe.write_all(b"go\n").unwrap();
assert_eq!(next(), "started gone", "didn't start once the old launcher had exited");
assert_eq!(next(), "exited");
}
/// A real disk image: the new `ArtCraft Launcher.app` replaces the old one in place.