hive-agent: pin what a zero means for each turn-loop knob

turn.rs has no tests. The knobs read env through two helpers and the
difference between them is load-bearing: env_u64 keeps a parsed 0, so
HIVE_TURN_IDLE_SECS=0 disables the watchdog and
HIVE_AUTO_RESET_WATERMARK_TOKENS=0 disables auto-reset; env_u64_positive
discards it, so a 0 sleep or cache TTL falls back to the default. Calling
the wrong one is a one-word edit that changes whether 0 turns a feature
off or does nothing.

Splitting the parse and zero-rejection halves out of the env lookup makes
both testable — std::env is process-global, so the lookup itself is not
safely settable from a threaded test runner, and the env-to-knob mapping
stays uncovered for that reason.

No behaviour change: each of the four knobs already agreed with its own
doc comment.
This commit is contained in:
atlas 2026-09-02 13:26:12 +02:00 committed by mara
commit 31a98f511d

View file

@ -200,15 +200,29 @@ pub enum TurnError {
/// Parse an env var as `u64`, ignoring absent / blank / unparseable values.
/// Returns the raw value including `0` (several knobs use `0` as "disable").
fn env_u64(name: &str) -> Option<u64> {
std::env::var(name)
.ok()
.and_then(|s| s.trim().parse::<u64>().ok())
parse_u64(std::env::var(name).ok().as_deref())
}
/// The parsing half of [`env_u64`], split from the lookup so it can be
/// exercised without setting a process-global variable.
fn parse_u64(raw: Option<&str>) -> Option<u64> {
raw.and_then(|s| s.trim().parse::<u64>().ok())
}
/// Like [`env_u64`] but also rejects `0`, falling back to `default` — for
/// knobs where `0` is meaningless rather than a "disable" sentinel.
fn env_u64_positive(name: &str, default: u64) -> u64 {
env_u64(name).filter(|&v| v > 0).unwrap_or(default)
nonzero_or(env_u64(name), default)
}
/// The `0`-rejecting half of [`env_u64_positive`].
///
/// Which of the two a knob uses is the whole distinction: a knob reading
/// [`env_u64`] treats `0` as "disable" and keeps it, one reading
/// [`env_u64_positive`] treats `0` as nonsense and falls back. Picking the
/// wrong one silently changes when the loop sleeps, compacts or resets.
fn nonzero_or(parsed: Option<u64>, default: u64) -> u64 {
parsed.filter(|&v| v > 0).unwrap_or(default)
}
/// How long to sleep after a rate-limit before re-entering the serve loop.
@ -726,3 +740,52 @@ fn archive_session(bus: &Bus) {
}
}
}
#[cfg(test)]
mod tests {
use super::{nonzero_or, parse_u64};
#[test]
fn an_absent_or_blank_value_is_not_a_number() {
assert_eq!(parse_u64(None), None);
assert_eq!(parse_u64(Some("")), None);
assert_eq!(parse_u64(Some(" ")), None);
}
#[test]
fn surrounding_whitespace_is_trimmed() {
assert_eq!(parse_u64(Some("12")), Some(12));
assert_eq!(parse_u64(Some(" 12\n")), Some(12));
}
#[test]
fn a_value_that_is_not_a_u64_is_rejected_rather_than_coerced() {
for raw in ["abc", "1.5", "-1", "12s", "0x10"] {
assert_eq!(parse_u64(Some(raw)), None, "for {raw}");
}
// One past u64::MAX: rejected, not wrapped or saturated.
assert_eq!(parse_u64(Some("18446744073709551616")), None);
assert_eq!(parse_u64(Some("18446744073709551615")), Some(u64::MAX));
}
#[test]
fn nonzero_or_falls_back_when_there_is_no_value() {
assert_eq!(nonzero_or(None, 9), 9);
assert_eq!(nonzero_or(Some(5), 9), 5);
}
/// The distinction the two helpers exist for. A knob reading the raw
/// parse keeps `0` and treats it as "disable"; one reading
/// `nonzero_or` discards it and uses its default. Swapping which
/// helper a knob calls is a one-word edit that silently changes
/// whether `0` turns the feature off or does nothing at all.
#[test]
fn zero_survives_parsing_but_is_rejected_as_a_positive_knob() {
assert_eq!(parse_u64(Some("0")), Some(0), "kept: 0 can mean disable");
assert_eq!(
nonzero_or(parse_u64(Some("0")), 7),
7,
"rejected: 0 is not a duration"
);
}
}