From 3e6f4fe5da5fce2c5ffca6b27f85276e0cf9dd39 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 11:55:12 +0500 Subject: [PATCH 1/3] test(ci): exercise the Windows self-replace for real, as its own check The existing update test swaps in a sentinel on every OS. On Windows, self-replace is not a rename (the running .exe is moved aside and a helper deletes the leftovers), so add a Windows-only test that swaps in a real binary and checks the replaced .exe launches, that it can update again over a file a self-replace put there, and that nothing is left in its directory afterwards. Run it as a dedicated check on both Windows architectures so a bump of self-replace or windows-sys (e.g. #97) fails by name. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01WeiJxQWhjhQvvURAjiDkti --- .github/workflows/ci.yml | 18 +++++++ tests/update_e2e.rs | 114 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 736ca6c..ba8d770 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,6 +129,24 @@ jobs: # A binary killed by a signal must not look like a success (#87). run: node --test npm/test-launcher.js + # `bugsee-cli update` replaces the running executable with the `self-replace` crate, whose Windows + # implementation (move the old .exe aside, drop the new one in, delete the leftovers from a helper + # process) is entirely different from the Unix rename. A dependency bump that touches it (the crate + # itself, or the `windows-sys` it is built on) must fail HERE, by name, not somewhere in the + # 500-test suite. Both Windows architectures, because the release ships both. + self-update-windows: + name: self-update (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [windows-latest, windows-11-arm] + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - run: cargo test --test update_e2e -- --nocapture + test: name: test (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/tests/update_e2e.rs b/tests/update_e2e.rs index ec315f6..b1d14f7 100644 --- a/tests/update_e2e.rs +++ b/tests/update_e2e.rs @@ -419,3 +419,117 @@ async fn update_readonly_dir_fails_safe_and_keeps_binary() { "a failed self-replace must leave the existing binary untouched" ); } + +/// Mount the three routes of a release: the per-major pointer, the archive and its SHA-256. +#[cfg(windows)] +async fn serve_release(newer: &str, triple: &str, artefact: &[u8]) -> MockServer { + let server = MockServer::start().await; + mount_pointer(&server, newer).await; + Mock::given(method("GET")) + .and(wm_path(format!( + "/v{newer}/bugsee-cli-{triple}.{}", + artifact_ext() + ))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(artefact.to_vec())) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(wm_path(format!( + "/v{newer}/bugsee-cli-{triple}.{}.sha256", + artifact_ext() + ))) + .respond_with( + ResponseTemplate::new(200).set_body_string(format!("{} x", sha256_hex(artefact))), + ) + .mount(&server) + .await; + server +} + +/// What is next to the binary, by file name. +#[cfg(windows)] +fn names_in(dir: &Path) -> Vec { + let mut names: Vec = std::fs::read_dir(dir) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names +} + +/// The Windows half of `self_replace`, which is not a rename: a running `.exe` cannot be +/// overwritten, so the crate moves the old binary aside, puts the new one in place and starts a +/// copy of itself that deletes the leftovers once this process has exited. +/// +/// The test above puts a sentinel in place, which proves the bytes land but not that what lands +/// is a program. This one swaps in a REAL binary (this build plus a few trailing bytes, which a PE +/// loader ignores) and then checks what a user depends on after an update: +/// - the replaced `.exe` launches, and reports the version; +/// - it can update again, over a file that was itself put there by a self-replace; +/// - nothing but the binary is left in its directory once the helper has finished. +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_litter() { + let triple = host_triple(); + let tmp = tempfile::tempdir().unwrap(); + let bindir = tmp.path().join("bin"); + std::fs::create_dir(&bindir).unwrap(); + let exe = copy_binary(&bindir); + let exe_name = exe.file_name().unwrap().to_string_lossy().into_owned(); + let real = std::fs::read(&exe).unwrap(); + let newer = format!("{}.99.0", current_major()); + + for round in 1..=2 { + let mut payload = real.clone(); + payload.extend_from_slice(format!("\0BUGSEE-TEST-OVERLAY-{round}\0").as_bytes()); + let artefacts = tmp.path().join(format!("release-{round}")); + std::fs::create_dir(&artefacts).unwrap(); + let archive = build_release_archive(&artefacts, triple, &payload); + let server = serve_release(&newer, triple, &archive).await; + + let (base, exe_for_run) = (server.uri(), exe.clone()); + tokio::task::spawn_blocking(move || { + assert_cmd::Command::new(&exe_for_run) + .env("BUGSEE_CLI_UPDATE_BASE_URL", &base) + .arg("update") + .assert() + .success(); + }) + .await + .unwrap(); + + assert_eq!( + std::fs::read(&exe).unwrap(), + payload, + "round {round}: the binary should be the downloaded release" + ); + + // It is a program, not just bytes: it starts and knows its version. + let exe_for_run = exe.clone(); + tokio::task::spawn_blocking(move || { + assert_cmd::Command::new(&exe_for_run) + .arg("--version") + .assert() + .success() + .stdout(predicates::str::contains(env!("CARGO_PKG_VERSION"))); + }) + .await + .unwrap(); + + // The helper that removes the old binary runs after the updater exits: give it time, then + // require that only the binary is left. + let mut left = names_in(&bindir); + for _ in 0..80 { + if left == [exe_name.clone()] { + break; + } + std::thread::sleep(std::time::Duration::from_millis(250)); + left = names_in(&bindir); + } + assert_eq!( + left, + [exe_name.clone()], + "round {round}: self-replace left files next to the binary" + ); + } +} From efad7b9f69740c62520c5a6410027b1056a248b4 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Tue, 3 Nov 2026 21:43:42 +0500 Subject: [PATCH 2/3] test(ci): watch the temp directory the self-replace helper actually cleans self-replace moves the old binary and its .__selfdelete__ helper into env::temp_dir(), not next to the binary, so checking only the binary's directory could never fail on CI. Give the updater a temp directory the test owns and require both it and the binary's directory to end clean. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01WeiJxQWhjhQvvURAjiDkti --- tests/update_e2e.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/update_e2e.rs b/tests/update_e2e.rs index b1d14f7..92f7db0 100644 --- a/tests/update_e2e.rs +++ b/tests/update_e2e.rs @@ -446,7 +446,7 @@ async fn serve_release(newer: &str, triple: &str, artefact: &[u8]) -> MockServer server } -/// What is next to the binary, by file name. +/// What is in a directory, by file name. #[cfg(windows)] fn names_in(dir: &Path) -> Vec { let mut names: Vec = std::fs::read_dir(dir) @@ -466,7 +466,10 @@ fn names_in(dir: &Path) -> Vec { /// loader ignores) and then checks what a user depends on after an update: /// - the replaced `.exe` launches, and reports the version; /// - it can update again, over a file that was itself put there by a self-replace; -/// - nothing but the binary is left in its directory once the helper has finished. +/// - nothing is left behind once the helper has finished. `self-replace` moves the old binary and +/// starts its `.__selfdelete__.exe` helper from `env::temp_dir()` (it only falls back to the +/// binary's directory when that is on another volume), so the updater is given a temp directory +/// the test owns and BOTH that and the binary's directory must end up clean. #[cfg(windows)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_litter() { @@ -486,11 +489,15 @@ async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_li std::fs::create_dir(&artefacts).unwrap(); let archive = build_release_archive(&artefacts, triple, &payload); let server = serve_release(&newer, triple, &archive).await; + let systemp = tmp.path().join(format!("systemp-{round}")); + std::fs::create_dir(&systemp).unwrap(); - let (base, exe_for_run) = (server.uri(), exe.clone()); + let (base, exe_for_run, temp_for_run) = (server.uri(), exe.clone(), systemp.clone()); tokio::task::spawn_blocking(move || { assert_cmd::Command::new(&exe_for_run) .env("BUGSEE_CLI_UPDATE_BASE_URL", &base) + .env("TMP", &temp_for_run) + .env("TEMP", &temp_for_run) .arg("update") .assert() .success(); @@ -517,19 +524,23 @@ async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_li .unwrap(); // The helper that removes the old binary runs after the updater exits: give it time, then - // require that only the binary is left. - let mut left = names_in(&bindir); + // require that only the binary is left, and that nothing is left in the temp directory. + let (mut left, mut in_temp) = (names_in(&bindir), names_in(&systemp)); for _ in 0..80 { - if left == [exe_name.clone()] { + if left == [exe_name.clone()] && in_temp.is_empty() { break; } std::thread::sleep(std::time::Duration::from_millis(250)); - left = names_in(&bindir); + (left, in_temp) = (names_in(&bindir), names_in(&systemp)); } assert_eq!( left, [exe_name.clone()], "round {round}: self-replace left files next to the binary" ); + assert!( + in_temp.is_empty(), + "round {round}: self-replace left files in the temp directory: {in_temp:?}" + ); } } From d0bedf9119857c4303aa61ccc69ac8b0ae7e0e7a Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Wed, 4 Nov 2026 13:33:53 +0500 Subject: [PATCH 3/3] test(ci): let the self-replace helper finish before the new binary starts, one test at a time The ARM64 job left both helper files in the watched temp directory after 20s once. Wait for the cleanup before launching the replaced binary, and run the dedicated job single-threaded so the helper's one delete attempt is not racing other work. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01WeiJxQWhjhQvvURAjiDkti --- .github/workflows/ci.yml | 4 +++- tests/update_e2e.rs | 26 ++++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba8d770..4f6e8df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -145,7 +145,9 @@ jobs: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - run: cargo test --test update_e2e -- --nocapture + # One test at a time: each self-replace starts a helper process that makes a single attempt + # to delete the leftovers, and the test watches for exactly that. + - run: cargo test --test update_e2e -- --nocapture --test-threads=1 test: name: test (${{ matrix.os }}) diff --git a/tests/update_e2e.rs b/tests/update_e2e.rs index 92f7db0..a7ba749 100644 --- a/tests/update_e2e.rs +++ b/tests/update_e2e.rs @@ -511,20 +511,10 @@ async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_li "round {round}: the binary should be the downloaded release" ); - // It is a program, not just bytes: it starts and knows its version. - let exe_for_run = exe.clone(); - tokio::task::spawn_blocking(move || { - assert_cmd::Command::new(&exe_for_run) - .arg("--version") - .assert() - .success() - .stdout(predicates::str::contains(env!("CARGO_PKG_VERSION"))); - }) - .await - .unwrap(); - // The helper that removes the old binary runs after the updater exits: give it time, then // require that only the binary is left, and that nothing is left in the temp directory. + // This comes BEFORE launching the new binary, so nothing else is touching the files while + // the helper makes its single attempt to delete them. let (mut left, mut in_temp) = (names_in(&bindir), names_in(&systemp)); for _ in 0..80 { if left == [exe_name.clone()] && in_temp.is_empty() { @@ -542,5 +532,17 @@ async fn update_on_windows_leaves_a_runnable_binary_that_updates_again_and_no_li in_temp.is_empty(), "round {round}: self-replace left files in the temp directory: {in_temp:?}" ); + + // It is a program, not just bytes: it starts and knows its version. + let exe_for_run = exe.clone(); + tokio::task::spawn_blocking(move || { + assert_cmd::Command::new(&exe_for_run) + .arg("--version") + .assert() + .success() + .stdout(predicates::str::contains(env!("CARGO_PKG_VERSION"))); + }) + .await + .unwrap(); } }