Skip to content

Commit ba7ebd5

Browse files
yjuglJohanLorenzo
authored andcommitted
Fix cargo test when running in a terminal
Make existing tests deterministically use the no-tty path. Previously, running cargo test in a terminal would block on reading stdin unless </dev/null was used. This was caused by tests reaching the interactive path for missing parameters. Use this as an opportunity to define some more tests, though none that requires true interaction with stdin.
1 parent a1294de commit ba7ebd5

1 file changed

Lines changed: 44 additions & 7 deletions

File tree

‎src/commands/execute.rs‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ fn build_parameter_map(
137137
metadata: &QueryMetadata,
138138
cli_params: &[(String, serde_json::Value)],
139139
interactive: bool,
140+
has_tty: bool,
140141
) -> Result<Option<HashMap<String, serde_json::Value>>> {
141142
if metadata.options.parameters.is_empty() {
142143
return Ok(None);
@@ -154,8 +155,6 @@ fn build_parameter_map(
154155
param_map.insert(name.clone(), coerced);
155156
}
156157

157-
let has_tty = std::io::stdin().is_terminal();
158-
159158
for param in &metadata.options.parameters {
160159
if !param_map.contains_key(&param.name) {
161160
if interactive && has_tty {
@@ -290,7 +289,8 @@ pub async fn execute(
290289
.map(|arg| parse_parameter_arg(arg))
291290
.collect::<Result<Vec<_>>>()?;
292291

293-
let parameters = build_parameter_map(&metadata, &cli_params, interactive)?;
292+
let has_tty = std::io::stdin().is_terminal();
293+
let parameters = build_parameter_map(&metadata, &cli_params, interactive, has_tty)?;
294294

295295
if let Some(ref params) = parameters {
296296
eprintln!("Parameters:");
@@ -584,7 +584,7 @@ mod tests {
584584
fn test_build_parameter_map_coerces_text_param() {
585585
let metadata = make_metadata_with_param("days", None);
586586
let cli_params = vec![("days".to_string(), serde_json::json!(90))];
587-
let result = build_parameter_map(&metadata, &cli_params, false)
587+
let result = build_parameter_map(&metadata, &cli_params, false, false)
588588
.unwrap()
589589
.unwrap();
590590
assert_eq!(result["days"], serde_json::Value::String("90".to_string()));
@@ -593,24 +593,61 @@ mod tests {
593593
#[test]
594594
fn test_build_parameter_map_interactive_no_tty_uses_default() {
595595
let metadata = make_metadata_with_param("p", Some(serde_json::json!("default_val")));
596-
let result = build_parameter_map(&metadata, &[], true).unwrap();
596+
let result = build_parameter_map(&metadata, &[], true, false).unwrap();
597597
let map = result.unwrap();
598598
assert_eq!(map["p"], serde_json::json!("default_val"));
599599
}
600600

601601
#[test]
602602
fn test_build_parameter_map_interactive_no_tty_no_default_bails() {
603603
let metadata = make_metadata_with_param("p", None);
604-
let err = build_parameter_map(&metadata, &[], true).unwrap_err();
604+
let err = build_parameter_map(&metadata, &[], true, false).unwrap_err();
605605
assert!(err.to_string().contains("--param p="));
606606
}
607607

608608
#[test]
609609
fn test_build_parameter_map_interactive_no_tty_cli_param_overrides() {
610610
let metadata = make_metadata_with_param("p", None);
611611
let cli_params = vec![("p".to_string(), serde_json::json!("provided"))];
612-
let result = build_parameter_map(&metadata, &cli_params, true).unwrap();
612+
let result = build_parameter_map(&metadata, &cli_params, true, false).unwrap();
613+
let map = result.unwrap();
614+
assert_eq!(map["p"], serde_json::json!("provided"));
615+
}
616+
617+
#[test]
618+
fn test_build_parameter_map_interactive_tty_cli_param_skips_prompt() {
619+
let metadata = make_metadata_with_param("p", None);
620+
let cli_params = vec![("p".to_string(), serde_json::json!("provided"))];
621+
// interactive + has_tty would prompt for a missing param (blocking on
622+
// stdin); supplying it via CLI must satisfy it without prompting.
623+
let result = build_parameter_map(&metadata, &cli_params, true, true).unwrap();
613624
let map = result.unwrap();
614625
assert_eq!(map["p"], serde_json::json!("provided"));
615626
}
627+
628+
#[test]
629+
fn test_build_parameter_map_interactive_tty_cli_param_coerces() {
630+
let metadata = make_metadata_with_param("days", None);
631+
let cli_params = vec![("days".to_string(), serde_json::json!(90))];
632+
let result = build_parameter_map(&metadata, &cli_params, true, true)
633+
.unwrap()
634+
.unwrap();
635+
assert_eq!(result["days"], serde_json::Value::String("90".to_string()));
636+
}
637+
638+
#[test]
639+
fn test_build_parameter_map_non_interactive_tty_uses_default() {
640+
let metadata = make_metadata_with_param("p", Some(serde_json::json!("default_val")));
641+
// has_tty alone must not trigger prompting when not interactive.
642+
let result = build_parameter_map(&metadata, &[], false, true).unwrap();
643+
let map = result.unwrap();
644+
assert_eq!(map["p"], serde_json::json!("default_val"));
645+
}
646+
647+
#[test]
648+
fn test_build_parameter_map_non_interactive_tty_no_default_bails() {
649+
let metadata = make_metadata_with_param("p", None);
650+
let err = build_parameter_map(&metadata, &[], false, true).unwrap_err();
651+
assert!(err.to_string().contains("Missing required parameter"));
652+
}
616653
}

0 commit comments

Comments
 (0)