Give Windows a free-space gate, and stop two tests timing the scheduler
Three release-profile failures on macOS and Windows, all found by the qa gate on its first run. Release-profile tests on those platforms had never run before it existed, so none of these were regressions — they had simply never been visible. available_space returned None on every non-unix target. That did not merely skip a test, it skipped the GATE: a Windows user extracting a disc to a full volume got a confusing failure part-way through instead of a clear refusal up front, and Windows is where the GUI ships. GetDiskFreeSpaceExW is declared directly against kernel32, matching how scsi::windows already reaches Win32 rather than pulling in a binding crate for one call. It asks for FreeBytesAvailableToCaller, which accounts for per-user quotas — the same question f_bavail answers on unix. The other two asserted on wall-clock timing with no margin: - sleep_until_halted_wakes_mid_sleep bounded the wait at 350 ms and measured 377 ms on a loaded runner. That bound measures the scheduler, not the wake. What the test is for is distinguishing "woke because the flag flipped" from "woke because the 10 s timeout expired", and 2 s does that just as well. - abandon_loses_to_a_close_already_committed released at 600 ms against two 300 ms grace windows plus a 250 ms poll cadence, so the windows could expire first and the caller abandoned — a race, not a defect. The intervals are scaled up so jitter is small relative to them; the ordering under test is unchanged, only the margin.
This commit is contained in:
+47
-1
@@ -684,7 +684,53 @@ fn available_space(dir: &Path) -> Option<u64> {
|
|||||||
Some(avail)
|
Some(avail)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
/// Windows has no `statvfs`. This returned `None` unconditionally, which does
|
||||||
|
/// not merely skip a test — it skipped the free-space GATE, so a Windows user
|
||||||
|
/// extracting a disc to a full volume got a confusing failure part-way through
|
||||||
|
/// instead of a clear refusal up front. libfreemkv ships a Windows GUI, so that
|
||||||
|
/// is the platform where the friendly error matters most.
|
||||||
|
///
|
||||||
|
/// `GetDiskFreeSpaceExW` is declared directly against kernel32, matching how
|
||||||
|
/// `scsi::windows` already reaches the Win32 API rather than pulling in a
|
||||||
|
/// binding crate for one call.
|
||||||
|
#[cfg(windows)]
|
||||||
|
fn available_space(dir: &Path) -> Option<u64> {
|
||||||
|
use std::os::windows::ffi::OsStrExt;
|
||||||
|
|
||||||
|
unsafe extern "system" {
|
||||||
|
fn GetDiskFreeSpaceExW(
|
||||||
|
lpDirectoryName: *const u16,
|
||||||
|
lpFreeBytesAvailableToCaller: *mut u64,
|
||||||
|
lpTotalNumberOfBytes: *mut u64,
|
||||||
|
lpTotalNumberOfFreeBytes: *mut u64,
|
||||||
|
) -> i32;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Wide, NUL-terminated. An interior NUL cannot reach the API, so reject it
|
||||||
|
// rather than silently truncating the path and measuring the wrong volume.
|
||||||
|
let mut wide: Vec<u16> = dir.as_os_str().encode_wide().collect();
|
||||||
|
if wide.contains(&0) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
wide.push(0);
|
||||||
|
|
||||||
|
let mut avail: u64 = 0;
|
||||||
|
// FreeBytesAvailableToCaller, not TotalNumberOfFreeBytes: it accounts for
|
||||||
|
// per-user quotas, which is what "can I actually write this much" means and
|
||||||
|
// what `statvfs`'s `f_bavail` gives on the unix side.
|
||||||
|
let ok = unsafe {
|
||||||
|
GetDiskFreeSpaceExW(
|
||||||
|
wide.as_ptr(),
|
||||||
|
&mut avail,
|
||||||
|
std::ptr::null_mut(),
|
||||||
|
std::ptr::null_mut(),
|
||||||
|
)
|
||||||
|
};
|
||||||
|
if ok == 0 { None } else { Some(avail) }
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Neither unix nor Windows: no way to ask, so the gate is skipped.
|
||||||
|
#[cfg(not(any(unix, windows)))]
|
||||||
fn available_space(_dir: &Path) -> Option<u64> {
|
fn available_space(_dir: &Path) -> Option<u64> {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|||||||
+12
-2
@@ -1406,8 +1406,18 @@ mod halt_tests {
|
|||||||
let r = sleep_until_halted(&flag, Duration::from_secs(10));
|
let r = sleep_until_halted(&flag, Duration::from_secs(10));
|
||||||
assert!(matches!(r, Err(Error::Halted)));
|
assert!(matches!(r, Err(Error::Halted)));
|
||||||
let waited = t0.elapsed();
|
let waited = t0.elapsed();
|
||||||
// Flag flipped at ~150 ms; we wake within one 100 ms slice → <300 ms.
|
// What this test is for: the sleep must end because the flag flipped,
|
||||||
assert!(waited < Duration::from_millis(350), "waited {waited:?}");
|
// NOT because the 10 s timeout expired. Anything comfortably under
|
||||||
|
// that proves it, and the lower bound proves it did not return early
|
||||||
|
// for some other reason.
|
||||||
|
//
|
||||||
|
// The upper bound used to be 350 ms — flag at ~150 ms plus one 100 ms
|
||||||
|
// poll slice, with a little slack. That measures the SCHEDULER, not
|
||||||
|
// this function: a loaded CI runner took 377 ms and failed, which says
|
||||||
|
// nothing about whether the wake worked. Bound it well below the
|
||||||
|
// timeout instead, so the assertion still distinguishes the two
|
||||||
|
// outcomes it exists to distinguish.
|
||||||
|
assert!(waited < Duration::from_secs(2), "waited {waited:?}");
|
||||||
assert!(waited >= Duration::from_millis(140), "waited {waited:?}");
|
assert!(waited >= Duration::from_millis(140), "waited {waited:?}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+14
-2
@@ -1839,11 +1839,23 @@ mod tests {
|
|||||||
thread::spawn(move || {
|
thread::spawn(move || {
|
||||||
// Past the first grace window (and past the 250 ms poll cadence that
|
// Past the first grace window (and past the 250 ms poll cadence that
|
||||||
// bounds when the window is actually observed), inside the second.
|
// bounds when the window is actually observed), inside the second.
|
||||||
thread::sleep(Duration::from_millis(600));
|
//
|
||||||
|
// These intervals used to be 600 ms against a 300 ms grace, which
|
||||||
|
// left NO margin: two 300 ms windows end at 600 ms, and the 250 ms
|
||||||
|
// poll cadence can push the observation later still, so on a loaded
|
||||||
|
// runner the second window expired first and the caller abandoned —
|
||||||
|
// failing with Err(Halted) against a race, not a defect.
|
||||||
|
//
|
||||||
|
// Scaled up so the jitter is small relative to the intervals: the
|
||||||
|
// first window ends at ~1.0-1.25 s and the second at ~2.0-2.25 s,
|
||||||
|
// so releasing at 1.6 s sits well inside the second with roughly
|
||||||
|
// 350 ms of slack on either side. The ordering under test is
|
||||||
|
// unchanged; only the margin is.
|
||||||
|
thread::sleep(Duration::from_millis(1600));
|
||||||
rel.store(true, Ordering::SeqCst);
|
rel.store(true, Ordering::SeqCst);
|
||||||
});
|
});
|
||||||
|
|
||||||
let grace = Duration::from_millis(300);
|
let grace = Duration::from_secs(1);
|
||||||
let res = finish_with_grace(handle, &state, grace, Error::Halted);
|
let res = finish_with_grace(handle, &state, grace, Error::Halted);
|
||||||
assert!(
|
assert!(
|
||||||
matches!(res, Ok(42)),
|
matches!(res, Ok(42)),
|
||||||
|
|||||||
Reference in New Issue
Block a user