From 566ef19f99e5312f39646c1addf78f06c7e15f31 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 13:59:57 -0700 Subject: [PATCH 1/6] test: add 17 tests for error paths and edge cases QA Engineer perspective (MPI iteration 1): - cli: help flag, unknown flag, missing --file value, invalid --jobs, duplicate target - parser: indented line outside rule, empty rule name (bare 'rule'), trailing whitespace-only rule name, unknown top-level/rule directive, expand_vars edge cases (bare $ at end, unclosed brace), empty env key - graph: self-cycle detection (a -> a) - cache: empty/malformed deserialization Also fixed parser to reject 'rule' with no name (previously unreachable error path because trim() consumed trailing whitespace before strip_prefix). Signed-off-by: Sebastien Tardif --- src/cache.rs | 16 +++++++++++++ src/cli.rs | 48 +++++++++++++++++++++++++++++++++++++++ src/graph.rs | 8 +++++++ src/parser.rs | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index bf238a1..f71ae6e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -235,6 +235,22 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn test_deserialize_empty() { + let cache = BuildCache::deserialize(""); + assert!(cache.entries.is_empty()); + } + + #[test] + fn test_deserialize_malformed_lines() { + // Lines that don't match any prefix should be silently skipped + let content = "RULE test\n IN abc not_a_number\n garbage line\n"; + let cache = BuildCache::deserialize(content); + assert_eq!(cache.entries.len(), 1); + // The malformed IN line should be skipped (non-numeric timestamp) + assert!(cache.entries["test"].input_hashes.is_empty()); + } + #[test] fn test_clean() { let dir = std::env::temp_dir().join("minibuild_test_clean"); diff --git a/src/cli.rs b/src/cli.rs index f6294e1..5e210b4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -136,6 +136,54 @@ mod tests { assert!(err.starts_with("minibuild ")); } + #[test] + fn test_help_flag() { + let args: Vec = vec!["--help"].into_iter().map(String::from).collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("Usage:")); + } + + #[test] + fn test_help_short_flag() { + let args: Vec = vec!["-h"].into_iter().map(String::from).collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("Usage:")); + } + + #[test] + fn test_unknown_flag() { + let args: Vec = vec!["--unknown"].into_iter().map(String::from).collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("unknown flag")); + } + + #[test] + fn test_file_missing_value() { + let args: Vec = vec!["--file"].into_iter().map(String::from).collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("requires a value")); + } + + #[test] + fn test_jobs_invalid_value() { + let args: Vec = vec!["--jobs", "abc"] + .into_iter() + .map(String::from) + .collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("invalid --jobs value")); + } + + #[test] + fn test_duplicate_target() { + let args: Vec = vec!["target1", "target2"] + .into_iter() + .map(String::from) + .collect(); + let err = parse_args(&args).unwrap_err(); + assert!(err.contains("unexpected argument")); + } + #[test] fn test_short_flags() { let args: Vec = vec!["-f", "my.build", "-j", "2", "-n", "-v"] diff --git a/src/graph.rs b/src/graph.rs index e0734c4..218f9a6 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -268,6 +268,14 @@ rule e\n run echo e\n", assert!(!reachable.contains("e")); } + #[test] + fn test_self_cycle() { + let bf = make_buildfile("rule a\n deps a\n run echo a\n"); + let result = build_graph(&bf); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("circular dependency")); + } + #[test] fn test_reachable_from_unknown() { let bf = make_buildfile("rule a\n run echo a\n"); diff --git a/src/parser.rs b/src/parser.rs index 703d369..bf9816f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -83,7 +83,9 @@ pub fn parse(input: &str) -> Result { rules.insert(r.name.clone(), r); } - if let Some(rest) = line.strip_prefix("rule ") { + if line == "rule" { + return Err(format!("line {line_num}: rule has no name")); + } else if let Some(rest) = line.strip_prefix("rule ") { let name = rest.trim().to_string(); if name.is_empty() { return Err(format!("line {line_num}: rule has no name")); @@ -279,6 +281,64 @@ rule link assert_eq!(expand_vars("$MISSING test", &env), " test"); } + #[test] + fn test_parse_indented_line_outside_rule() { + let input = " run echo orphan\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("indented line outside of a rule block")); + } + + #[test] + fn test_parse_empty_rule_name() { + // "rule" alone (no name following) should error + let input = "rule\n run echo test\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("rule has no name")); + } + + #[test] + fn test_parse_rule_trailing_whitespace_only() { + // "rule " (only whitespace after prefix) should also error + let input = "rule \n run echo test\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("rule has no name")); + } + + #[test] + fn test_parse_unknown_top_level_directive() { + let input = "unknown_directive something\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("unexpected top-level directive")); + } + + #[test] + fn test_parse_unknown_rule_directive() { + let input = "rule a\n badkey value\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("unknown rule directive")); + } + + #[test] + fn test_expand_vars_bare_dollar_at_end() { + let env = HashMap::new(); + // Bare $ at end of string should pass through as literal $ + assert_eq!(expand_vars("hello$", &env), "hello$"); + } + + #[test] + fn test_expand_vars_unclosed_brace() { + let env = HashMap::new(); + // ${VAR without closing brace should pass through as literal + assert_eq!(expand_vars("${UNCLOSED", &env), "${UNCLOSED"); + } + + #[test] + fn test_parse_env_empty_key() { + let input = "env = value\nrule a\n run echo a\n"; + let err = parse(input).unwrap_err(); + assert!(err.contains("empty key")); + } + #[test] fn test_parse_env_missing_equals() { let input = "env BROKEN\nrule a\n run echo a\n"; From 523bc2c83accc2048eff957e4edae20633bd1272 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 14:01:33 -0700 Subject: [PATCH 2/6] refactor: use RuleResult failure data in main instead of matches!() Developer perspective (MPI iteration 2): Collect failures by destructuring RuleResult::Failed(name, reason) instead of discarding the fields with matches!(). The executor's eprintln already reports failures to stderr, so this prepares the data for any future structured exit reporting without changing behavior. Signed-off-by: Sebastien Tardif --- src/main.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index a3431a9..77c1021 100644 --- a/src/main.rs +++ b/src/main.rs @@ -102,10 +102,14 @@ fn main() { } // Exit with failure if any rule failed - let has_failure = results + let failures: Vec<_> = results .iter() - .any(|r| matches!(r, executor::RuleResult::Failed(_, _))); - if has_failure { + .filter_map(|r| match r { + executor::RuleResult::Failed(name, reason) => Some((name, reason)), + _ => None, + }) + .collect(); + if !failures.is_empty() { process::exit(1); } } From 857e2dcb94e7795c71de2588564e805553ecf88b Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 14:02:03 -0700 Subject: [PATCH 3/6] docs: add --version flag to CLI usage, update test count to 53 End User perspective (MPI iteration 3): - Missing --version/-V flag in README CLI Usage section - Test count was stale at 36 (now 53 after QA iteration) Signed-off-by: Sebastien Tardif --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 490ccc5..4e297da 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ Options: --clean Remove the build cache and rebuild everything --dry-run, -n Print what would be executed without running anything --verbose, -v Show detailed execution info + --version, -V Show version --help, -h Show help ``` @@ -155,7 +156,7 @@ src/ ## Tests -The project includes 36 tests covering: +The project includes 53 tests covering: - **Diamond dependencies** — A depends on B and C, both depend on D - **Large graphs** — 120-rule chains and 110-leaf fan-out graphs to stress the scheduler From edc5d442b91c2fe80f23315783679c57e3efb2b7 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 14:04:42 -0700 Subject: [PATCH 4/6] fix: --help and --version now print to stdout and exit 0 Spec/Contract Compliance perspective (MPI iteration 8): Previously --help and --version used the Err path, printing to stderr and exiting with code 1. Standard CLI convention is informational flags print to stdout and exit 0 (only real errors exit non-zero). Introduce ParseOutcome enum to distinguish Info (--help, --version) from Run (normal build) at the type level. Tests updated accordingly. Signed-off-by: Sebastien Tardif --- src/cli.rs | 53 ++++++++++++++++++++++++++++++++++++++++------------- src/main.rs | 6 +++++- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 5e210b4..497bcea 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -26,7 +26,16 @@ impl Default for CliArgs { } } -pub fn parse_args(args: &[String]) -> Result { +/// Outcome of CLI argument parsing. +#[derive(Debug)] +pub enum ParseOutcome { + /// Successfully parsed arguments; proceed with build. + Run(CliArgs), + /// Informational output (--help, --version); print to stdout and exit 0. + Info(String), +} + +pub fn parse_args(args: &[String]) -> Result { let mut cli = CliArgs::default(); let mut i = 0; while i < args.len() { @@ -54,10 +63,13 @@ pub fn parse_args(args: &[String]) -> Result { "--dry-run" | "-n" => cli.dry_run = true, "--verbose" | "-v" => cli.verbose = true, "--version" | "-V" => { - return Err(format!("minibuild {}", env!("CARGO_PKG_VERSION"))); + return Ok(ParseOutcome::Info(format!( + "minibuild {}", + env!("CARGO_PKG_VERSION") + ))); } "--help" | "-h" => { - return Err(usage()); + return Ok(ParseOutcome::Info(usage())); } s if s.starts_with('-') => { return Err(format!("unknown flag: {s}")); @@ -71,7 +83,7 @@ pub fn parse_args(args: &[String]) -> Result { } i += 1; } - Ok(cli) + Ok(ParseOutcome::Run(cli)) } fn usage() -> String { @@ -91,9 +103,18 @@ fn usage() -> String { mod tests { use super::*; + /// Extract CliArgs from a ParseOutcome::Run, panicking on Info or Err. + fn unwrap_run(result: Result) -> CliArgs { + match result { + Ok(ParseOutcome::Run(cli)) => cli, + Ok(ParseOutcome::Info(msg)) => panic!("expected Run, got Info: {msg}"), + Err(e) => panic!("expected Run, got Err: {e}"), + } + } + #[test] fn test_defaults() { - let cli = parse_args(&[]).unwrap(); + let cli = unwrap_run(parse_args(&[])); assert_eq!(cli.file, "Buildfile"); assert!(cli.jobs >= 1); assert!(cli.target.is_none()); @@ -114,7 +135,7 @@ mod tests { .into_iter() .map(String::from) .collect(); - let cli = parse_args(&args).unwrap(); + let cli = unwrap_run(parse_args(&args)); assert_eq!(cli.file, "build.mb"); assert_eq!(cli.jobs, 8); assert!(cli.clean); @@ -132,22 +153,28 @@ mod tests { #[test] fn test_version_flag() { let args: Vec = vec!["--version"].into_iter().map(String::from).collect(); - let err = parse_args(&args).unwrap_err(); - assert!(err.starts_with("minibuild ")); + match parse_args(&args).unwrap() { + ParseOutcome::Info(msg) => assert!(msg.starts_with("minibuild ")), + other => panic!("expected Info, got {other:?}"), + } } #[test] fn test_help_flag() { let args: Vec = vec!["--help"].into_iter().map(String::from).collect(); - let err = parse_args(&args).unwrap_err(); - assert!(err.contains("Usage:")); + match parse_args(&args).unwrap() { + ParseOutcome::Info(msg) => assert!(msg.contains("Usage:")), + other => panic!("expected Info, got {other:?}"), + } } #[test] fn test_help_short_flag() { let args: Vec = vec!["-h"].into_iter().map(String::from).collect(); - let err = parse_args(&args).unwrap_err(); - assert!(err.contains("Usage:")); + match parse_args(&args).unwrap() { + ParseOutcome::Info(msg) => assert!(msg.contains("Usage:")), + other => panic!("expected Info, got {other:?}"), + } } #[test] @@ -190,7 +217,7 @@ mod tests { .into_iter() .map(String::from) .collect(); - let cli = parse_args(&args).unwrap(); + let cli = unwrap_run(parse_args(&args)); assert_eq!(cli.file, "my.build"); assert_eq!(cli.jobs, 2); assert!(cli.dry_run); diff --git a/src/main.rs b/src/main.rs index 77c1021..3f92539 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,11 @@ use std::sync::{Arc, Mutex}; fn main() { let args: Vec = std::env::args().skip(1).collect(); let cli = match cli::parse_args(&args) { - Ok(c) => c, + Ok(cli::ParseOutcome::Run(c)) => c, + Ok(cli::ParseOutcome::Info(msg)) => { + println!("{}", msg); + return; + } Err(msg) => { eprintln!("{}", msg); process::exit(1); From 41ab5f39cab8f1b944b972ff55592b6b0183d370 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 14:06:16 -0700 Subject: [PATCH 5/6] test: add adversarial edge case tests for parser Adversarial Tester perspective (MPI iteration 11): - Double dollar ($$X) expansion behavior documented via test - Unicode rule names (cafe with accent) work correctly - Large dependency lists (50 deps) handled without issues Signed-off-by: Sebastien Tardif --- src/parser.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index bf9816f..2c3070a 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -339,6 +339,37 @@ rule link assert!(err.contains("empty key")); } + #[test] + fn test_expand_vars_double_dollar() { + let mut env = HashMap::new(); + env.insert("X".to_string(), "val".to_string()); + // First $ sees next char $ (not alphanumeric), passes through as literal $. + // Second $ sees X, expands to "val". Result: "$val" + let result = expand_vars("$$X", &env); + assert_eq!(result, "$val"); + } + + #[test] + fn test_parse_unicode_rule_name() { + let input = "rule caf\u{00e9}\n run echo hello\n"; + let bf = parse(input).unwrap(); + assert!(bf.rules.contains_key("caf\u{00e9}")); + } + + #[test] + fn test_parse_many_deps() { + let mut input = String::from("rule top\n deps"); + for i in 0..50 { + input.push_str(&format!(" dep{i}")); + } + input.push('\n'); + for i in 0..50 { + input.push_str(&format!("rule dep{i}\n run echo {i}\n")); + } + let bf = parse(&input).unwrap(); + assert_eq!(bf.rules["top"].deps.len(), 50); + } + #[test] fn test_parse_env_missing_equals() { let input = "env BROKEN\nrule a\n run echo a\n"; From 149a41b910fec9d08669d0fa6202bc088bca5fc6 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 2 Jul 2026 14:09:00 -0700 Subject: [PATCH 6/6] docs: update test count to 56 after adversarial tests Post-cycle gate B: documentation accuracy audit. Signed-off-by: Sebastien Tardif --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4e297da..20f4125 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,7 @@ src/ ## Tests -The project includes 53 tests covering: +The project includes 56 tests covering: - **Diamond dependencies** — A depends on B and C, both depend on D - **Large graphs** — 120-rule chains and 110-leaf fan-out graphs to stress the scheduler