Improve tests structure

[codcod]

Test Suite Review (High-Level)

Key issues (typical in growing test suites):

• Large integration test files mixing setup, execution, assertions.
• Repeated ad-hoc temp directory + config + git repo setup.
• Lack of clear separation: unit vs integration vs E2E.
• Some removed tests (CLI argument validation) now leave coverage gaps (inventory shows them as still Automated).

Recommendations (Minimal Change Mindset)

1. Structure
   • Split tests by domain: tests/config.rs, tests/clone.rs, tests/run_command.rs, tests/run_recipe.rs, tests/metadata.rs, tests/plugins.rs, tests/pr.rs, tests/init.rs.
   • Keep unit tests co-located with modules under src/, but move cross-cutting helpers to src/test_support/ (only compiled under test via #[cfg(test)]).
2. Fixtures / Helpers (reduce duplication)
   • Temp workspace creation (mk_temp_workspace()).
   • Config builder (ConfigBuilder -> write_to(path)).
   • Git repo initializer (init_bare_remote(), init_local_repo()).
   • CLI runner (run_cli(args: &[&str]) -> Output { status, stdout, stderr }).
   • Metadata assertions (assert_command_metadata(path, expected_fields), assert_recipe_metadata(...)).
   • Exit code description mapper unit-tested independently.
   • Sanitization test helper (assert_sanitized(input, expected)).
3. Parameterization
   • Use rstest or table-driven loops for repeated variations (exit code mapping, tag filtering cases).
4. Duplicate Removal
   • Merge similar “run echo” cases differing only by repo count.
   • Consolidate recipe multi-step + metadata validation into one test with sub-assertions.
5. Faster Unit Tests
   • Extract pure logic (sanitization, exit code mapping, tag filtering, directory name generation) into small functions with focused unit tests.
6. Coverage Plan (target 80%+)
   • Add missing unit tests for: timestamp format, signal >128 mapping, command vs recipe exclusivity, zero-step recipe validation, permissions failure (mock using tempdir chmod).
7. E2E Tier
   • Add thin smoke tests invoking binary for: init + clone + run + metadata flow; plugin discovery; PR mock path.
8. CI Optimization
   • Stage execution: Unit (fast), Integration (normal), E2E (nightly). Use cargo nextest if adopted.

Proposed Helper Skeletons

// Common test support (integration/E2E)
use std::{path::PathBuf, process::Command, fs, time::SystemTime};
use tempfile::TempDir;

pub struct Workspace {
    pub root: TempDir,
    pub config_path: PathBuf,
}

impl Workspace {
    pub fn new() -> Self {
        let root = tempfile::tempdir().unwrap();
        let config_path = root.path().join("config.yaml");
        Self { root, config_path }
    }
    pub fn write_config(&self, yaml: &str) {
        fs::write(&self.config_path, yaml).unwrap();
    }
    pub fn path(&self) -> &std::path::Path { self.root.path() }
}

pub struct CliOutput {
    pub status: i32,
    pub stdout: String,
    pub stderr: String,
}

pub fn run_cli(args: &[&str], cwd: Option<&std::path::Path>) -> CliOutput {
    let mut cmd = Command::new("cargo");
    cmd.args(["run", "--quiet", "--"]);
    cmd.args(args);
    if let Some(dir) = cwd { cmd.current_dir(dir); }
    let out = cmd.output().unwrap();
    CliOutput {
        status: out.status.code().unwrap_or(-1),
        stdout: String::from_utf8_lossy(&out.stdout).into(),
        stderr: String::from_utf8_lossy(&out.stderr).into(),
    }
}

pub fn read_metadata(dir: &std::path::Path) -> serde_json::Value {
    let data = fs::read(dir.join("metadata.json")).unwrap();
    serde_json::from_slice(&data).unwrap()
}

pub fn assert_command_metadata(json: &serde_json::Value) {
    let obj = json.as_object().unwrap();
    assert!(obj.get("command").is_some());
    assert!(obj.get("recipe").is_none());
    assert!(obj.get("exit_code").is_some());
    assert!(obj.get("exit_code_description").is_some());
}

pub fn assert_recipe_metadata(json: &serde_json::Value) {
    let obj = json.as_object().unwrap();
    assert!(obj.get("recipe").is_some());
    assert!(obj.get("command").is_none());
    assert!(obj.get("recipe_steps").is_some());
}

pub fn timestamp_format_ok(ts: &str) -> bool {
    // Simple shape check YYYY-MM-DD HH:MM:SS
    let parts: Vec<_> = ts.split(' ').collect();
    if parts.len() != 2 { return false; }
    let date = parts[0];
    let time = parts[1];
    date.len() == 10
        && time.len() == 8
        && date.chars().nth(4) == Some('-')
        && date.chars().nth(7) == Some('-')
        && time.chars().nth(2) == Some(':')
        && time.chars().nth(5) == Some(':')
}

// Minimal git init helper
pub fn init_git_repo(path: &std::path::Path) {
    fs::create_dir_all(path).unwrap();
    Command::new("git").arg("init").arg(path).output().unwrap();
}

// Unit-test only helpers (#[cfg(test)])
#[cfg(test)]
pub fn map_exit_code(code: i32) -> &'static str {
    match code {
        0 => "success",
        1 => "general error",
        2 => "misuse of shell builtins",
        126 => "command invoked cannot execute",
        127 => "command not found",
        130 => "script terminated by Control-C",
        c if c > 128 => "terminated by signal",
        _ => "error",
    }
}

#[cfg(test)]
pub fn sanitize_name(input: &str) -> String {
    let mut s = input.chars()
        .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
        .collect::<String>();
    if s.len() > 50 {
        s.truncate(50);
    }
    s
}

New / Adjusted Tests (Examples)

use tests::support::{Workspace, run_cli};

#[test]
fn error_when_both_command_and_recipe() {
    let ws = Workspace::new();
    ws.write_config(r#"
repositories:
  - name: repo1
    url: https://example.com/dummy.git
recipes:
  build:
    steps:
      - echo building
"#);
    let out = run_cli(&[
        "run",
        "--config", ws.config_path.to_str().unwrap(),
        "--recipe", "build",
        "echo hello"
    ], Some(ws.path()));
    assert_ne!(out.status, 0);
    assert!(out.stderr.contains("Cannot specify both command and --recipe"));
}

#[test]
fn error_when_missing_command_and_recipe() {
    let ws = Workspace::new();
    ws.write_config(r#"
repositories:
  - name: repo1
    url: https://example.com/dummy.git
"#);
    let out = run_cli(&[
        "run",
        "--config", ws.config_path.to_str().unwrap()
    ], Some(ws.path()));
    assert_ne!(out.status, 0);
    assert!(out.stderr.contains("Either --recipe or a command must be provided"));
}

Updated Test Inventory (Added missing exclusivity test, updated statuses)

// ...existing code...
### 8. Error Handling

### 8.1 Missing command and recipe (CLI validation)

- Description: Error when neither provided.
- Status: Gap (previous test removed; needs reimplementation)

### 8.7 Both command and recipe provided

- Description: Supplying both should yield exclusivity error.
- Expected: Non-zero exit; stderr contains "Cannot specify both command and --recipe"; no run directory.
- Type: E2E
- Status: Gap (newly added; not currently automated)
// ...existing code...

### 18.8 Error Handling

| Case | Type | Rationale | Status |
|------|------|-----------|---------|
|8.1 Missing command & recipe| E2E | Full CLI validation | Gap |
|8.2 Nonexistent binary (plugin)| Integration | External failure path | Partial |
|8.3 Mutual exclusivity metadata| Integration | Schema correctness | Automated |
|8.4 Command not found 127| Integration | Real process exit | Automated |
|8.5 Script cannot execute 126| Integration | Permission/exec failure | Gap |
|8.6 Interrupted 130| Integration | Signal handling | Gap |
|8.7 Both command & recipe provided| E2E | Exclusivity validation error | Gap |
|Signal >128 mapping (edge)| Unit | Mapping function correctness | Gap |
// ...existing code...

Coverage Focus (Next Increment)

Add fast unit tests for:

• map_exit_code (including >128, unknown negative)
• sanitize_name (unsafe chars, truncation, UTF-8)
• timestamp format (extract formatting into function)
• tag filtering combinations (already present—confirm no duplicate logic)
• command vs recipe exclusivity (re-add removed tests)

Implementation Order (Low-Risk)

1. Add support modules (tests/support/, src/test_support/).
2. Reintroduce exclusivity tests (above).
3. Add unit tests for exit code mapping & sanitization.
4. Add timestamp format unit test (extract function if not already).
5. Add permission failure test (chmod 0 on script path before execution).
6. Measure coverage (cargo tarpaulin --skip-clean); target deltas.

CI Suggestion

Stage:

• cargo test --lib (unit)
• cargo test --tests (integration)
• (optional nightly) cargo test -- --ignored (E2E/perf)
  Add tarpaulin coverage gate (>=80%).