mirror of
https://github.com/block/buzz.git
synced 2026-11-01 23:11:58 +00:00
test(desktop): isolate login-shell probe measurements (#7570)
Pinky, acting on Wes’s behalf. ## Summary Fix a pre-existing false-failure mechanism in the login-shell discovery tests, independently of the thread-window work. - Run the two process-global-counter assertions in separate, bounded libtest processes. The global counter still sees all of each test’s auth/worker threads; no production discovery or cache behavior changes. - Require exact test selection, successful child exit, and a completion receipt after the assertion body. Reject counter resets outside isolation. - Keep the cold/repeated cheap-discovery zero assertions and forced-discovery positive control unchanged. Add parent-probe exclusion and own-worker controls. Three test-only files, +103/-4. Base `00209076c7a10d9e4a475466c313e8ebecf041f5`; head `07f8e86c58a0aa5378537a3daa4610361fdbad55`. ## Reproduction and causal checks On the main-based source, cheap discovery alone counted 0. Running the existing missing-binary version test on another thread inside the measurement interval made the repeated-cheap assertion fail with 1. This reproduces interference, not the exact scheduling/caller behind the original full-suite failure. The counter measures `run_in_login_shell` invocations, not exact OS process counts. Mutation checks: - Removing isolation fails the measurement guard. - Removing the real production `record()` call fails the original forced positive control (0). - A reviewer-identified early-return mutation initially survived; the final helper owns the assertion closure instead. Deleting that invocation now fails the external worker assertion (0 vs 1). Independent read-only review by Brain found no remaining blocking finding at the final head. ## Validation at the final head - **Full `just ci`: passed.** Desktop Tauri: 3,175 passed, 19 ignored; desktop JS: 6,488 passed; mobile: 2,098 passed. Remaining Rust unit, formatting/lint/static, and desktop/web build lanes passed. An initial five-minute tool timeout interrupted compilation; the subsequent complete run used a longer deadline, not altered gates. - **Normal pre-commit and pre-push hooks: passed.** Push includes default + mesh Tauri Clippy, complete Tauri workspace tests, size and branch-scope checks. No bypass. - **Real local relay, following `TESTING.md`: passed.** Built release `buzz-relay`, `buzz`, and `buzz-admin` from this head. Fresh isolated Postgres/Redis, additive migrations, dedicated loopback ports, disposable NIP-98 identity; health, create channel, signed root/reply send, exact-ID get, and thread retrieval passed. Relay and both owned containers stopped afterward. Dev auth-token mode follows the guide; not a production-auth or thread-window/native UI acceptance claim. ## Scope and limits This is process/cache isolation, not a filesystem sandbox. The existing bounded-command helper bounds child runtime/output; its Unix process-group behavior does not guarantee ownership of nested probe groups. Existing individual probe deadlines remain unchanged. No UI change, screenshots, thread-window commits, feature enablement, merge, or deployment included. Originating conversation: buzz://message?channel=0b73400d-6922-4fe3-9b97-454da04d011b&id=984528c883faa8829971d19394870f0f2d543d5c1950565fd55ffddc1b8d8022 Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> Co-authored-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -13,9 +13,96 @@ pub(crate) fn record() {
|
||||
}
|
||||
|
||||
pub(crate) fn reset() {
|
||||
#[cfg(unix)]
|
||||
assert!(
|
||||
is_isolated_process(),
|
||||
"counter measurements must run in an isolated test process"
|
||||
);
|
||||
COUNT.store(0, Ordering::SeqCst);
|
||||
}
|
||||
|
||||
pub(crate) fn count() -> usize {
|
||||
COUNT.load(Ordering::SeqCst)
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
const CHILD_TEST: &str = "BUZZ_LOGIN_SHELL_PROBE_TEST";
|
||||
|
||||
#[cfg(unix)]
|
||||
fn is_isolated_process() -> bool {
|
||||
std::thread::current()
|
||||
.name()
|
||||
.is_some_and(|name| std::env::var(CHILD_TEST).as_deref() == Ok(name))
|
||||
}
|
||||
|
||||
/// Run a counter-owning test body alone in a fresh libtest process.
|
||||
///
|
||||
/// The counter deliberately stays process-wide: discovery's auth workers must
|
||||
/// count too. A voluntary PATH lock cannot exclude unrelated, unlocked probe
|
||||
/// callers in the full suite. Process isolation excludes those callers without
|
||||
/// changing production code or requiring new workers to inherit test context.
|
||||
/// Call while holding the PATH lock so the child inherits stable environment.
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn run_in_isolated_process(test: impl FnOnce()) {
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
|
||||
let thread = std::thread::current();
|
||||
let name = thread.name().expect("libtest names its test threads");
|
||||
let completed = format!("completed isolated login-shell probe test: {name}");
|
||||
if is_isolated_process() {
|
||||
test();
|
||||
// Receipt only after the assertion body returns, never on entry.
|
||||
println!("{completed}");
|
||||
return;
|
||||
}
|
||||
|
||||
let mut command = Command::new(std::env::current_exe().expect("test executable"));
|
||||
command
|
||||
.args(["--exact", name, "--nocapture", "--test-threads=1"])
|
||||
.env(CHILD_TEST, name);
|
||||
let output = super::bounded_command::output_with_timeout(command, Duration::from_secs(300))
|
||||
.expect("isolated probe test must finish within five minutes and the output cap");
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
output.status.success() && stdout.lines().any(|line| line.contains(&completed)),
|
||||
"isolated probe test {name} failed or did not run: {}\n{stdout}\n{stderr}",
|
||||
output.status
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn isolated_counter_excludes_parent_probes_but_counts_own_workers() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let probe_on_worker = || {
|
||||
std::thread::spawn(|| {
|
||||
// A real production probe, independent of the shared PATH cache.
|
||||
super::find_via_login_shell("buzz-absent-probe-isolation-xyzzy")
|
||||
})
|
||||
.join()
|
||||
.expect("probe worker must finish")
|
||||
};
|
||||
if !is_isolated_process() {
|
||||
assert!(probe_on_worker().is_none());
|
||||
assert!(count() >= 1, "parent worker must reach the real probe");
|
||||
}
|
||||
run_in_isolated_process(|| {
|
||||
// Before reset: unrelated parent probes must not enter this process.
|
||||
assert_eq!(count(), 0, "the child must start with its own counter");
|
||||
reset();
|
||||
assert!(probe_on_worker().is_none());
|
||||
});
|
||||
if is_isolated_process() {
|
||||
// Outside the closure so omitting its invocation cannot pass this test.
|
||||
assert_eq!(count(), 1, "the child's assertion body and worker must run");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
#[should_panic(expected = "counter measurements must run in an isolated test process")]
|
||||
fn reset_rejects_unisolated_measurement() {
|
||||
reset();
|
||||
}
|
||||
|
||||
@@ -108,6 +108,14 @@ fn forced_discovery_probes_auth_but_cheap_discovery_reuses_cached_status() {
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn cheap_discovery_reports_absent_before_any_forced_probe() {
|
||||
let _path_guard = crate::managed_agents::lock_path_mutex();
|
||||
super::super::login_shell_spawn_probe::run_in_isolated_process(
|
||||
cheap_discovery_reports_absent_before_any_forced_probe_body,
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn cheap_discovery_reports_absent_before_any_forced_probe_body() {
|
||||
use crate::managed_agents::custom_harnesses::registry_test_lock;
|
||||
use crate::managed_agents::discovery::{
|
||||
clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe,
|
||||
@@ -115,7 +123,6 @@ fn cheap_discovery_reports_absent_before_any_forced_probe() {
|
||||
use crate::managed_agents::{AcpAvailabilityStatus, AuthStatus};
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let _path_guard = crate::managed_agents::lock_path_mutex();
|
||||
let _registry_guard = registry_test_lock();
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -126,6 +126,14 @@ fn resolve_command_prefers_buzz_managed_npm_shim_over_path() {
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn cheap_discovery_never_spawns_login_shell_even_when_cold() {
|
||||
let _path_guard = crate::managed_agents::lock_path_mutex();
|
||||
super::super::login_shell_spawn_probe::run_in_isolated_process(
|
||||
cheap_discovery_never_spawns_login_shell_even_when_cold_body,
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn cheap_discovery_never_spawns_login_shell_even_when_cold_body() {
|
||||
use crate::managed_agents::custom_harnesses::registry_test_lock;
|
||||
use crate::managed_agents::discovery::{
|
||||
clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe,
|
||||
@@ -133,9 +141,6 @@ fn cheap_discovery_never_spawns_login_shell_even_when_cold() {
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
// Serialize with every other test that spawns a login shell: the spawn
|
||||
// counter and the PATH/login-shell caches are process-global.
|
||||
let _path_guard = crate::managed_agents::lock_path_mutex();
|
||||
let _registry = registry_test_lock();
|
||||
|
||||
// A custom harness whose command cannot resolve anywhere, so the resolver
|
||||
|
||||
Reference in New Issue
Block a user