test: multi-perspective improvement rotation (20 new tests, exit code fix, docs)

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 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

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");

src/cli.rs

@@ -26,7 +26,16 @@ impl Default for CliArgs {
     }
 }
 
-pub fn parse_args(args: &[String]) -> Result<CliArgs, String> {
+/// 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<ParseOutcome, String> {
     let mut cli = CliArgs::default();
     let mut i = 0;
     while i < args.len() {
@@ -54,10 +63,13 @@ pub fn parse_args(args: &[String]) -> Result<CliArgs, String> {
             "--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<CliArgs, String> {
         }
         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<ParseOutcome, String>) -> 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,8 +153,62 @@ mod tests {
     #[test]
     fn test_version_flag() {
         let args: Vec<String> = vec!["--version"].into_iter().map(String::from).collect();
+        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<String> = vec!["--help"].into_iter().map(String::from).collect();
+        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<String> = vec!["-h"].into_iter().map(String::from).collect();
+        match parse_args(&args).unwrap() {
+            ParseOutcome::Info(msg) => assert!(msg.contains("Usage:")),
+            other => panic!("expected Info, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn test_unknown_flag() {
+        let args: Vec<String> = 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<String> = 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<String> = 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<String> = vec!["target1", "target2"]
+            .into_iter()
+            .map(String::from)
+            .collect();
         let err = parse_args(&args).unwrap_err();
-        assert!(err.starts_with("minibuild "));
+        assert!(err.contains("unexpected argument"));
     }
 
     #[test]
@@ -142,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);

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");

src/main.rs

@@ -12,7 +12,11 @@ use std::sync::{Arc, Mutex};
 fn main() {
     let args: Vec<String> = 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);
@@ -102,10 +106,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);
     }
 }

src/parser.rs

@@ -83,7 +83,9 @@ pub fn parse(input: &str) -> Result<BuildFile, String> {
                 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,95 @@ 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_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";