From 74b759b4537dd222be34928a0b51df27ccfe0d43 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sun, 6 Sep 2026 23:21:34 -0700 Subject: [PATCH 1/2] test: make changelog contract robust to rollover; bump restart budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix A: smoke_demo_and_changelog_contract asserted KV/export/routing claims only against the `## [Unreleased]` slice of the embedded mcp CHANGELOG. PR #307 (the rc.3 changelog rollover) moved every bullet into `## [1.0.0-rc.3]`, emptying Unreleased and failing the test. Because #307 was docs-only, CI's `paths-ignore: **/*.md` skipped the Rust suite, so the break surfaced only on a later code PR. Assert instead against the current release window (`## [Unreleased]` + the most-recent dated section) via a new `current_release_window` helper, so a future rollover can't silently rebreak it. The window is bounded to the newest dated section so a stale token in an ancient entry can't satisfy a deleted-claim check. Fix B: engine_recovers_after_hyperd_killed and hyperd_monitor_detects_killed_hyperd_and_restarts time out on ubuntu-latest under CI load — a 5s monitor tick plus cold hyperd spawn against a 12s readiness budget. Replace the three magic `12`s with a named RESTART_READINESS_BUDGET_SECS = 45 and document the derivation. Budget bump only; the macOS-only ignore is unchanged, so Linux/Windows crash-recovery coverage from #279/#286 stays. See #305. --- hyperdb-mcp/tests/daemon_tests.rs | 56 +++++++++++++++++++++++++------ hyperdb-mcp/tests/readme_tests.rs | 45 +++++++++++++++++++++---- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/hyperdb-mcp/tests/daemon_tests.rs b/hyperdb-mcp/tests/daemon_tests.rs index 498d9cf..077a94b 100644 --- a/hyperdb-mcp/tests/daemon_tests.rs +++ b/hyperdb-mcp/tests/daemon_tests.rs @@ -25,6 +25,25 @@ static ENV_LOCK: Mutex<()> = Mutex::new(()); const ENGINE_REPORT_CHILD_ENV: &str = "HYPERDB_MCP_ENGINE_REPORT_CHILD"; const ENGINE_REPORT_TEST_NAME: &str = "report_hyperd_error_targets_discovered_health_port"; +/// Seconds a crash-recovery test waits for the daemon to bring a fresh `hyperd` +/// back to a reachable state after the running engine is killed. +/// +/// Derivation: the liveness monitor polls on a 5 s tick +/// (`daemon::run::HYPERD_POLL_INTERVAL`), so a kill landing just after a tick +/// costs up to ~5 s just to be *noticed*. Recovery then has to drop the dead +/// process, cold-spawn a new `hyperd`, rebind, and republish STATUS. On a +/// loaded shared CI runner (`ubuntu-latest`) a cold engine spawn alone can eat +/// several seconds, so the previous 12 s budget (~5 s detection + ~7 s slack) +/// tripped as a *false* timeout even though recovery was progressing — these +/// tests pass 4/4 locally and the third restart test passes on the same CI run. +/// 45 s keeps ~40 s of headroom past the detection tick, comfortably absorbing a +/// slow runner while still bounding a genuinely wedged daemon (which would never +/// republish and would fail at the deadline regardless). This is a budget bump, +/// not a coverage change: the tests still run on Linux/Windows. See issue #305 +/// for the standing daemon-test timing decision (which now spans Linux, not just +/// the macOS-ignored set). +const RESTART_READINESS_BUDGET_SECS: u64 = 45; + fn acquire_env_lock() -> std::sync::MutexGuard<'static, ()> { ENV_LOCK .lock() @@ -1608,11 +1627,17 @@ fn hyperd_monitor_detects_killed_hyperd_and_restarts() { // the next 5s tick and restart hyperd. kill_pid(pid_before); - // Wait up to 12 seconds for the monitor to fire and restart hyperd. - // (5s monitor tick + spawn time + slack.) - let new_endpoint = - wait_for_live_hyperd_after_kill(daemon.info.health_port, &daemon.info.hyperd_endpoint, 12) - .expect("daemon should restart hyperd within 12s"); + // Wait for the monitor to fire and restart hyperd. The budget is generous + // (5s monitor tick + cold spawn + slack) so a loaded CI runner doesn't trip + // a false timeout; see RESTART_READINESS_BUDGET_SECS. + let new_endpoint = wait_for_live_hyperd_after_kill( + daemon.info.health_port, + &daemon.info.hyperd_endpoint, + RESTART_READINESS_BUDGET_SECS, + ) + .unwrap_or_else(|| { + panic!("daemon should restart hyperd within {RESTART_READINESS_BUDGET_SECS}s") + }); // The new endpoint must be reachable. Don't assert it differs from the old — // port reuse is permitted by the OS. @@ -1644,9 +1669,14 @@ fn client_report_triggers_restart_after_kill() { let response = health::send_command(daemon.info.health_port, "REPORT_HYPERD_ERROR").unwrap(); assert_eq!(response.trim(), "OK"); - let new_endpoint = - wait_for_live_hyperd_after_kill(daemon.info.health_port, &daemon.info.hyperd_endpoint, 12) - .expect("daemon should restart hyperd within 12s after report"); + let new_endpoint = wait_for_live_hyperd_after_kill( + daemon.info.health_port, + &daemon.info.hyperd_endpoint, + RESTART_READINESS_BUDGET_SECS, + ) + .unwrap_or_else(|| { + panic!("daemon should restart hyperd within {RESTART_READINESS_BUDGET_SECS}s after report") + }); let probe = std::net::TcpStream::connect_timeout( &new_endpoint.parse().expect("valid endpoint"), @@ -1699,8 +1729,14 @@ fn engine_recovers_after_hyperd_killed() { // Wait for daemon-side restart. The daemon persists `daemon.json` before // it flips what STATUS serves, so a live endpoint from STATUS means the // discovery file `Engine::new` reads below is already committed. - wait_for_live_hyperd_after_kill(daemon.info.health_port, &daemon.info.hyperd_endpoint, 12) - .expect("daemon should restart hyperd within 12s"); + wait_for_live_hyperd_after_kill( + daemon.info.health_port, + &daemon.info.hyperd_endpoint, + RESTART_READINESS_BUDGET_SECS, + ) + .unwrap_or_else(|| { + panic!("daemon should restart hyperd within {RESTART_READINESS_BUDGET_SECS}s") + }); // Engine #2: post-restart. This mirrors what `with_engine` does after a // ConnectionLost — drop the old engine (already done above) and create a diff --git a/hyperdb-mcp/tests/readme_tests.rs b/hyperdb-mcp/tests/readme_tests.rs index 6347df0..7cbfd33 100644 --- a/hyperdb-mcp/tests/readme_tests.rs +++ b/hyperdb-mcp/tests/readme_tests.rs @@ -35,6 +35,36 @@ fn contains_any(text: &str, alternatives: &[&str]) -> bool { .any(|candidate| text.contains(candidate)) } +/// The changelog's *current release window*: the `## [Unreleased]` section plus +/// the single most-recent dated release section, concatenated. +/// +/// When a release ships, a `docs:` rollover (see PR #307) moves every bullet +/// out of `## [Unreleased]` into a fresh dated `## [x.y.z]` section. Asserting +/// documentation claims against the `## [Unreleased]` slice alone therefore +/// breaks silently the moment that rollover lands — which is exactly what +/// happened to `smoke_demo_and_changelog_contract`. Searching this window +/// instead keeps the contract's intent ("these claims are documented for the +/// current release candidate") stable across the rollover: before a release the +/// claims live under `## [Unreleased]`, after it they live under the newest +/// dated section, and the window spans both. Only the *most-recent* dated +/// section is included — older historical sections are deliberately excluded so +/// a stale token in an ancient entry cannot satisfy a check for a claim that was +/// actually deleted. +/// +/// `changelog_lower` must already be lowercased (the headings are matched +/// case-insensitively by lowercasing the whole document at the call site). +fn current_release_window(changelog_lower: &str) -> String { + let unreleased = markdown_section(changelog_lower, "## [unreleased]", "\n## ["); + // Everything after the `## [Unreleased]` heading begins with the newest + // dated section's header; `markdown_section` then bounds it to that one + // section (up to the following `## [` heading). + let after_unreleased = changelog_lower + .split_once("## [unreleased]") + .map_or(changelog_lower, |(_, rest)| rest); + let latest_release = markdown_section(after_unreleased, "## [", "\n## ["); + format!("{unreleased}\n{latest_release}") +} + fn markdown_bullet(text: &str, needle: &str) -> String { let mut lines = Vec::new(); let mut capturing = false; @@ -767,8 +797,9 @@ fn public_docs_database_and_read_only_contract() { fn smoke_demo_and_changelog_contract() { let smoke = SMOKE_TESTS.to_lowercase(); let demo = DEMO.to_lowercase(); - let unreleased = - markdown_section(&CHANGELOG.to_lowercase(), "## [unreleased]", "\n## [").to_owned(); + // Assert against the current release window (`## [Unreleased]` + the newest + // dated section) so the release rollover (#307) can't silently rebreak this. + let release_window = current_release_window(&CHANGELOG.to_lowercase()); let batch_section = markdown_section( &smoke, "## 2. create / read / overwrite (upsert)", @@ -932,14 +963,14 @@ fn smoke_demo_and_changelog_contract() { } for heading in ["### added", "### fixed", "### changed"] { - let section = markdown_section(&unreleased, heading, "\n### "); + let section = markdown_section(&release_window, heading, "\n### "); if section.is_empty() || !section .lines() .any(|line| line.trim_start().starts_with("- ")) { failures.push(format!( - "crate ## [Unreleased] must contain at least one bullet under {heading}" + "crate current release window must contain at least one bullet under {heading}" )); } } @@ -954,14 +985,14 @@ fn smoke_demo_and_changelog_contract() { "show_legend", "y_scale", ] { - if !unreleased.contains(token) { + if !release_window.contains(token) { failures.push(format!( - "crate ## [Unreleased] does not account for {token:?}" + "crate current release window does not account for {token:?}" )); } } - let hyper_export = markdown_bullet(&unreleased, "hyper-format export"); + let hyper_export = markdown_bullet(&release_window, "hyper-format export"); if !(hyper_export.contains("source") && contains_any( &hyper_export, From 329c9f23efb1f0acd8e87c3e1a8fdd97afb18d39 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sun, 6 Sep 2026 23:37:03 -0700 Subject: [PATCH 2/2] test: gate RESTART_READINESS_BUDGET_SECS to cfg(unix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The constant's only use sites are the three `#[cfg(unix)]` restart tests and their Unix-only helpers, so on Windows it compiled but was never referenced and `clippy -D warnings` (windows-latest) rejected it as dead code. Gate the definition with `#[cfg(unix)]` so it exists exactly where it's used — the honest fix, not `#[allow(dead_code)]`. --- hyperdb-mcp/tests/daemon_tests.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hyperdb-mcp/tests/daemon_tests.rs b/hyperdb-mcp/tests/daemon_tests.rs index 077a94b..45fb464 100644 --- a/hyperdb-mcp/tests/daemon_tests.rs +++ b/hyperdb-mcp/tests/daemon_tests.rs @@ -39,9 +39,17 @@ const ENGINE_REPORT_TEST_NAME: &str = "report_hyperd_error_targets_discovered_he /// 45 s keeps ~40 s of headroom past the detection tick, comfortably absorbing a /// slow runner while still bounding a genuinely wedged daemon (which would never /// republish and would fail at the deadline regardless). This is a budget bump, -/// not a coverage change: the tests still run on Linux/Windows. See issue #305 +/// not a coverage change: the restart tests still run on Linux. See issue #305 /// for the standing daemon-test timing decision (which now spans Linux, not just /// the macOS-ignored set). +/// +/// `#[cfg(unix)]` because its only use sites are the `#[cfg(unix)]` restart +/// tests (`hyperd_monitor_detects_killed_hyperd_and_restarts`, +/// `client_report_triggers_restart_after_kill`, +/// `engine_recovers_after_hyperd_killed`) and their Unix-only helpers. Without +/// the gate the constant is dead code on Windows, which `clippy -D warnings` +/// rejects. +#[cfg(unix)] const RESTART_READINESS_BUDGET_SECS: u64 = 45; fn acquire_env_lock() -> std::sync::MutexGuard<'static, ()> {