Skip to content

Commit ca2cbdb

Browse files
committed
test(args): regression coverage for BoolishValueParser on env bools
Two new tests in cli_global_args.rs: - bool_env_vars_accept_one_and_yes: SOCKET_OFFLINE=1, SOCKET_GLOBAL=yes, SOCKET_JSON=on, etc. all parse as `true`. Pins the BoolishValueParser wiring we just added — without it clap fails with `error: invalid value '1' for '--offline'` (the bug that prompted the fix). - bool_env_vars_reject_zero_and_falsey: SOCKET_OFFLINE=0, SOCKET_DEBUG=false, SOCKET_TELEMETRY_DISABLED=no, SOCKET_JSON=off all parse as `false`. Guards against an over-eager parser flipping a bool on any non-empty string. Both new tests + the existing env_vars_populate_global_args are now `#[serial]` — env-var state is process-global, so parallel test runs race the cleanup. Without serialization the negative-case test flakes by inheriting `SOCKET_OFFLINE=1` from the positive case. Assisted-by: Claude Code:opus-4-7
1 parent 3835974 commit ca2cbdb

1 file changed

Lines changed: 103 additions & 0 deletions

File tree

crates/socket-patch-cli/tests/cli_global_args.rs

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ fn reserved_short_forms_are_not_assigned() {
175175
///
176176
/// Combined into one test to avoid env-var races between parallel tests.
177177
#[test]
178+
#[serial_test::serial]
178179
fn env_vars_populate_global_args() {
179180
// Save then clear any env vars we set, then verify clap picks them up.
180181
let pairs = [
@@ -247,3 +248,105 @@ fn env_vars_populate_global_args() {
247248
}
248249
}
249250
}
251+
252+
/// Regression: bool env vars accept "1"/"yes" (the conventional truthy
253+
/// strings), not just clap's strict "true"/"false". Before
254+
/// BoolishValueParser was wired onto every bool with env, setting
255+
/// SOCKET_OFFLINE=1 (or SOCKET_DEBUG=1) crashed clap with
256+
/// `error: invalid value '1' for '--offline'`, taking down every
257+
/// downstream CLI run that follows the conventional shell idiom.
258+
///
259+
/// `#[serial]` because env-var state is process-global; without it
260+
/// these tests race each other (and the existing
261+
/// `env_vars_populate_global_args`) when cargo runs them in
262+
/// parallel.
263+
#[test]
264+
#[serial_test::serial]
265+
fn bool_env_vars_accept_one_and_yes() {
266+
// (env var name, value to set)
267+
let cases: &[(&str, &str)] = &[
268+
("SOCKET_OFFLINE", "1"),
269+
("SOCKET_GLOBAL", "yes"),
270+
("SOCKET_JSON", "on"),
271+
("SOCKET_VERBOSE", "1"),
272+
("SOCKET_SILENT", "y"),
273+
("SOCKET_DRY_RUN", "1"),
274+
("SOCKET_YES", "yes"),
275+
("SOCKET_BREAK_LOCK", "1"),
276+
("SOCKET_DEBUG", "1"),
277+
("SOCKET_TELEMETRY_DISABLED", "1"),
278+
];
279+
280+
let saved: Vec<(String, Option<String>)> = cases
281+
.iter()
282+
.map(|(k, _)| (k.to_string(), std::env::var(k).ok()))
283+
.collect();
284+
for (k, v) in cases {
285+
std::env::set_var(k, v);
286+
}
287+
288+
let cli = Cli::try_parse_from(["socket-patch", "list"]).expect("parse");
289+
if let socket_patch_cli::Commands::List(args) = cli.command {
290+
assert!(args.common.offline, "SOCKET_OFFLINE=1 must parse as true");
291+
assert!(args.common.global, "SOCKET_GLOBAL=yes must parse as true");
292+
assert!(args.common.json, "SOCKET_JSON=on must parse as true");
293+
assert!(args.common.verbose, "SOCKET_VERBOSE=1 must parse as true");
294+
assert!(args.common.silent, "SOCKET_SILENT=y must parse as true");
295+
assert!(args.common.dry_run, "SOCKET_DRY_RUN=1 must parse as true");
296+
assert!(args.common.yes, "SOCKET_YES=yes must parse as true");
297+
assert!(args.common.break_lock, "SOCKET_BREAK_LOCK=1 must parse as true");
298+
assert!(args.common.debug, "SOCKET_DEBUG=1 must parse as true");
299+
assert!(
300+
args.common.no_telemetry,
301+
"SOCKET_TELEMETRY_DISABLED=1 must parse as true"
302+
);
303+
} else {
304+
panic!("expected List");
305+
}
306+
307+
for (k, orig) in saved {
308+
match orig {
309+
Some(v) => std::env::set_var(&k, v),
310+
None => std::env::remove_var(&k),
311+
}
312+
}
313+
}
314+
315+
/// Defensive: "0", "false", "no", "off", and empty string must NOT
316+
/// engage a bool. Otherwise an operator unsetting via SOCKET_OFFLINE=0
317+
/// would still get airgap mode (and various subtler shell idioms).
318+
#[test]
319+
#[serial_test::serial]
320+
fn bool_env_vars_reject_zero_and_falsey() {
321+
let cases: &[(&str, &str)] = &[
322+
("SOCKET_OFFLINE", "0"),
323+
("SOCKET_DEBUG", "false"),
324+
("SOCKET_TELEMETRY_DISABLED", "no"),
325+
("SOCKET_JSON", "off"),
326+
];
327+
328+
let saved: Vec<(String, Option<String>)> = cases
329+
.iter()
330+
.map(|(k, _)| (k.to_string(), std::env::var(k).ok()))
331+
.collect();
332+
for (k, v) in cases {
333+
std::env::set_var(k, v);
334+
}
335+
336+
let cli = Cli::try_parse_from(["socket-patch", "list"]).expect("parse");
337+
if let socket_patch_cli::Commands::List(args) = cli.command {
338+
assert!(!args.common.offline);
339+
assert!(!args.common.debug);
340+
assert!(!args.common.no_telemetry);
341+
assert!(!args.common.json);
342+
} else {
343+
panic!("expected List");
344+
}
345+
346+
for (k, orig) in saved {
347+
match orig {
348+
Some(v) => std::env::set_var(&k, v),
349+
None => std::env::remove_var(&k),
350+
}
351+
}
352+
}

0 commit comments

Comments
 (0)