refactor: extract CLI tests from main.rs into integration tests

[unclesp1d3r]

Summary

src/main.rs is 1,647 lines, of which ~1,100 are colocated #[cfg(test)] tests. The CLI tests use low-level Unix fd manipulation (dup, dup2 via nix crate) for stdin mocking, which is fragile -- evidenced by the LLVM_PROFILE_FILE workaround that skips these tests under llvm-cov instrumentation.

Current Problems

1. File size: main.rs is 2.7x the 500-600 line guideline
2. Fragile fd manipulation: dup2_stdin, dup2_stdout, dup2_stderr operations are process-wide and not thread-safe (requires FD_MUTEX and --test-threads=1)
3. Coverage gaps: Several tests are skipped under llvm-cov due to fd interference
4. Coupling: Tests directly call internal functions (process_file, validate_arguments) rather than testing the binary interface

Proposal

1. Extract CLI integration tests into tests/cli_integration.rs using assert_cmd crate
2. Test the rmagic binary as a subprocess (natural process isolation)
3. Keep only unit tests for pure functions (e.g., output_format(), validate_arguments()) in main.rs
4. Use assert_cmd's write_stdin() for stdin testing (no fd manipulation needed)

Example

use assert_cmd::Command;

#[test]
fn test_builtin_elf_detection() {
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", "tests/fixtures/sample.elf"])
        .assert()
        .success()
        .stdout(predicates::str::contains("ELF"));
}

#[test]
fn test_stdin_input() {
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", "-"])
        .write_stdin(b"\x7fELF")
        .assert()
        .success()
        .stdout(predicates::str::contains("ELF"));
}

Acceptance Criteria

• main.rs under 600 lines
• CLI behavior tests moved to tests/cli_integration.rs
• Stdin tests work reliably under llvm-cov (no LLVM_PROFILE_FILE skip)
• nix dev-dependency removed (or only used where truly needed)
• FD_MUTEX and fd manipulation removed
• All existing test coverage preserved or improved

[traycerai]

Plan

Observations

The file:src/main.rs contains 1,647 lines with ~1,100 lines of #[cfg(test)] tests (lines 617-1645). These tests use low-level Unix fd manipulation (dup2_stdin, dup2_stdout, dup2_stderr from nix crate) with process-wide side effects requiring FD_MUTEX serialization and --test-threads=1. Several stdin tests skip execution under llvm-cov due to LLVM_PROFILE_FILE interference. The project already has integration test infrastructure in file:tests/cli_integration_tests.rs using std::process::Command, but doesn't yet use assert_cmd for cleaner subprocess testing.

Approach

Extract all CLI behavior tests from file:src/main.rs into a new file:tests/cli_integration.rs using assert_cmd crate for subprocess-based testing with natural process isolation. This eliminates fragile fd manipulation, removes the need for FD_MUTEX, and enables reliable execution under llvm-cov. Keep only unit tests for pure functions (output_format(), validate_arguments(), validate_input_file(), validate_magic_file()) in main.rs. The refactoring preserves all existing test coverage while reducing main.rs from 1,647 to under 600 lines and improving test reliability.

Implementation Steps

1. Add Required Dependencies

Update file:Cargo.toml dev-dependencies section:

[dev-dependencies]
assert_cmd = "2.0"
predicates = "3.0"
# Keep existing dev-dependencies
criterion = "0.8.2"
insta = { version = "1.46.3", features = ["json"] }
nix = { version = "0.31.1", features = ["fs"] }  # Can be removed after migration
proptest = "1.10.0"
regex = "1.12.3"
tempfile = "3.25.0"

2. Create CLI Integration Test File

Create file:tests/cli_integration.rs with the following structure:

Test Categories to Migrate:

Stdin Tests (from lines 1524-1644)

• test_stdin_detection → Test - argument recognition
• test_stdin_truncation_warning → Test stdin buffer truncation with write_stdin()
• test_stdin_no_false_truncation_warning → Verify no warning at exact buffer size
• test_stdin_empty_returns_data → Test empty stdin handling
• test_stdin_output_format → Verify stdin output format
• test_stdin_strict_mode_errors → Test strict mode with invalid stdin

Use assert_cmd::Command::cargo_bin("rmagic") with .write_stdin() for all stdin tests - no fd manipulation needed.

Multiple File Tests (from lines 1408-1514)

• test_args_multiple_files → Verify multiple file argument parsing
• test_args_strict_with_multiple_files → Test strict mode with multiple files
• test_args_multiple_files_with_magic_file → Test custom magic file with multiple inputs

Test sequential processing and output order using subprocess execution.

Builtin Flag Tests (from lines 1428-1470)

• test_use_builtin_flag_parsing → Test --use-builtin flag
• test_use_builtin_with_strict → Combine builtin with strict mode
• test_use_builtin_with_json → Combine builtin with JSON output
• test_use_builtin_with_magic_file → Test builtin flag with custom magic file
• test_use_builtin_default_false → Verify default behavior

Use assert_cmd with .args() to test flag combinations.

Error Handling Integration Tests

• Test file not found errors (exit code 3)
• Test permission denied errors (exit code 3)
• Test invalid input errors (exit code 2)
• Test parse errors (exit code 4)
• Test evaluation errors (exit code 1)
• Test timeout errors (exit code 5)

Use .assert().failure().code(expected_code) for exit code validation.

Test Structure Pattern:

use assert_cmd::Command;
use predicates::prelude::*;
use tempfile::TempDir;

#[test]
fn test_builtin_elf_detection() {
    let temp_dir = TempDir::new().unwrap();
    let test_file = temp_dir.path().join("test.elf");
    std::fs::write(&test_file, b"\x7fELF\x02\x01\x01\x00").unwrap();
    
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", test_file.to_str().unwrap()])
        .assert()
        .success()
        .stdout(predicate::str::contains("ELF"));
}

#[test]
fn test_stdin_input() {
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", "-"])
        .write_stdin(b"\x7fELF")
        .assert()
        .success()
        .stdout(predicate::str::contains("ELF"));
}

3. Keep Unit Tests in main.rs

Retain only pure function unit tests in file:src/main.rs (lines to keep):

Argument Parsing Tests (lines 800-910):

• test_basic_file_argument
• test_json_output_flag
• test_text_output_flag
• test_magic_file_argument
• test_all_arguments_combined
• test_json_text_conflict
• test_missing_file_argument
• test_complex_file_paths
• test_magic_file_with_spaces

Output Format Tests (lines 873-888):

• test_output_format_default
• test_output_format_json
• test_output_format_text_explicit

Magic File Path Tests (lines 913-980):

• test_get_magic_file_path_custom
• test_get_magic_file_path_default
• test_default_magic_file_path

Validation Function Tests (lines 1040-1229):

• test_validate_arguments_empty_files
• test_validate_arguments_empty_magic_file
• test_validate_arguments_valid
• test_validate_input_file_not_found
• test_validate_input_file_directory
• test_validate_input_file_valid
• test_validate_magic_file_not_found
• test_validate_magic_file_directory
• test_validate_magic_file_empty
• test_validate_magic_file_whitespace_only
• test_validate_magic_file_valid

Magic File Search Order Tests (lines 1234-1405):

• test_magic_file_search_order_text_first
• test_magic_file_search_order_magdir_priority
• test_magic_file_candidates_exact_sequence
• test_magic_file_search_selects_first_existing
• test_magic_file_search_binary_fallback

These tests don't require subprocess execution and test pure logic.

4. Remove FD Manipulation Code

Delete from file:src/main.rs:

• Lines 623-634: FD_MUTEX static and imports
• Lines 636-706: capture_stdout() and capture_stderr() functions
• Lines 708-763: with_mocked_stdin() and with_invalid_stdin() functions
• Lines 765-798: resolve_magic_file_for_stdin_tests() with LLVM_PROFILE_FILE check

5. Update Cargo.toml

After migration is complete and verified, optionally remove or reduce nix dependency:

# Remove if no longer needed:
# nix = { version = "0.31.1", features = ["fs"] }

# Or keep only if used elsewhere in tests

6. Verification Steps

Test Coverage Verification:

# Run all tests to ensure nothing breaks
cargo test

# Run specific test suites
cargo test --test cli_integration
cargo test --lib

# Verify coverage works without LLVM_PROFILE_FILE workaround
cargo llvm-cov --html

# Verify parallel execution works (no FD_MUTEX needed)
cargo test -- --test-threads=8

Line Count Verification:

# Check main.rs is under 600 lines
wc -l src/main.rs

# Should show approximately:
# - 250 lines: imports, structs, main logic
# - 300 lines: remaining unit tests
# - Total: ~550 lines

Test Migration Checklist:

• All stdin tests migrated and passing
• All multiple file tests migrated and passing
• All builtin flag tests migrated and passing
• All error handling tests migrated and passing
• All strict mode tests migrated and passing
• Unit tests still pass in main.rs
• No LLVM_PROFILE_FILE checks remain
• No FD_MUTEX usage remains
• cargo test passes with default thread count
• cargo llvm-cov runs successfully
• main.rs under 600 lines

7. Documentation Updates

Update file:docs/src/testing.md:

• Remove references to fd manipulation and FD_MUTEX
• Add section on CLI integration tests using assert_cmd
• Update test count statistics
• Document new test organization (unit tests in src/, integration in tests/)

Test Migration Mapping

graph TD
    A[main.rs Tests<br/>~1100 lines] --> B[cli_integration.rs<br/>CLI Behavior Tests]
    A --> C[main.rs Unit Tests<br/>~300 lines]
    
    B --> D[Stdin Tests<br/>assert_cmd + write_stdin]
    B --> E[Multiple Files<br/>subprocess execution]
    B --> F[Builtin Flags<br/>flag combinations]
    B --> G[Error Handling<br/>exit code validation]
    
    C --> H[Arg Parsing<br/>clap::Parser tests]
    C --> I[Validation<br/>pure function tests]
    C --> J[Magic File Search<br/>path resolution tests]
    
    style A fill:#ffcccc
    style B fill:#ccffcc
    style C fill:#ccccff

Loading

Import In IDE

🤖 Prompt for AI Agents

## Observations

The `file:src/main.rs` contains 1,647 lines with ~1,100 lines of `#[cfg(test)]` tests (lines 617-1645). These tests use low-level Unix fd manipulation (`dup2_stdin`, `dup2_stdout`, `dup2_stderr` from `nix` crate) with process-wide side effects requiring `FD_MUTEX` serialization and `--test-threads=1`. Several stdin tests skip execution under llvm-cov due to `LLVM_PROFILE_FILE` interference. The project already has integration test infrastructure in `file:tests/cli_integration_tests.rs` using `std::process::Command`, but doesn't yet use `assert_cmd` for cleaner subprocess testing.

## Approach

Extract all CLI behavior tests from `file:src/main.rs` into a new `file:tests/cli_integration.rs` using `assert_cmd` crate for subprocess-based testing with natural process isolation. This eliminates fragile fd manipulation, removes the need for `FD_MUTEX`, and enables reliable execution under llvm-cov. Keep only unit tests for pure functions (`output_format()`, `validate_arguments()`, `validate_input_file()`, `validate_magic_file()`) in main.rs. The refactoring preserves all existing test coverage while reducing main.rs from 1,647 to under 600 lines and improving test reliability.

## Implementation Steps

### 1. Add Required Dependencies

Update `file:Cargo.toml` dev-dependencies section:

```toml
[dev-dependencies]
assert_cmd = "2.0"
predicates = "3.0"
# Keep existing dev-dependencies
criterion = "0.8.2"
insta = { version = "1.46.3", features = ["json"] }
nix = { version = "0.31.1", features = ["fs"] }  # Can be removed after migration
proptest = "1.10.0"
regex = "1.12.3"
tempfile = "3.25.0"
```

### 2. Create CLI Integration Test File

Create `file:tests/cli_integration.rs` with the following structure:

**Test Categories to Migrate:**

#### Stdin Tests (from lines 1524-1644)
- `test_stdin_detection` → Test `-` argument recognition
- `test_stdin_truncation_warning` → Test stdin buffer truncation with `write_stdin()`
- `test_stdin_no_false_truncation_warning` → Verify no warning at exact buffer size
- `test_stdin_empty_returns_data` → Test empty stdin handling
- `test_stdin_output_format` → Verify stdin output format
- `test_stdin_strict_mode_errors` → Test strict mode with invalid stdin

Use `assert_cmd::Command::cargo_bin("rmagic")` with `.write_stdin()` for all stdin tests - no fd manipulation needed.

#### Multiple File Tests (from lines 1408-1514)
- `test_args_multiple_files` → Verify multiple file argument parsing
- `test_args_strict_with_multiple_files` → Test strict mode with multiple files
- `test_args_multiple_files_with_magic_file` → Test custom magic file with multiple inputs

Test sequential processing and output order using subprocess execution.

#### Builtin Flag Tests (from lines 1428-1470)
- `test_use_builtin_flag_parsing` → Test `--use-builtin` flag
- `test_use_builtin_with_strict` → Combine builtin with strict mode
- `test_use_builtin_with_json` → Combine builtin with JSON output
- `test_use_builtin_with_magic_file` → Test builtin flag with custom magic file
- `test_use_builtin_default_false` → Verify default behavior

Use `assert_cmd` with `.args()` to test flag combinations.

#### Error Handling Integration Tests
- Test file not found errors (exit code 3)
- Test permission denied errors (exit code 3)
- Test invalid input errors (exit code 2)
- Test parse errors (exit code 4)
- Test evaluation errors (exit code 1)
- Test timeout errors (exit code 5)

Use `.assert().failure().code(expected_code)` for exit code validation.

**Test Structure Pattern:**

```rust
use assert_cmd::Command;
use predicates::prelude::*;
use tempfile::TempDir;

#[test]
fn test_builtin_elf_detection() {
    let temp_dir = TempDir::new().unwrap();
    let test_file = temp_dir.path().join("test.elf");
    std::fs::write(&test_file, b"\x7fELF\x02\x01\x01\x00").unwrap();
    
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", test_file.to_str().unwrap()])
        .assert()
        .success()
        .stdout(predicate::str::contains("ELF"));
}

#[test]
fn test_stdin_input() {
    Command::cargo_bin("rmagic")
        .unwrap()
        .args(["--use-builtin", "-"])
        .write_stdin(b"\x7fELF")
        .assert()
        .success()
        .stdout(predicate::str::contains("ELF"));
}
```

### 3. Keep Unit Tests in main.rs

Retain only pure function unit tests in `file:src/main.rs` (lines to keep):

**Argument Parsing Tests (lines 800-910):**
- `test_basic_file_argument`
- `test_json_output_flag`
- `test_text_output_flag`
- `test_magic_file_argument`
- `test_all_arguments_combined`
- `test_json_text_conflict`
- `test_missing_file_argument`
- `test_complex_file_paths`
- `test_magic_file_with_spaces`

**Output Format Tests (lines 873-888):**
- `test_output_format_default`
- `test_output_format_json`
- `test_output_format_text_explicit`

**Magic File Path Tests (lines 913-980):**
- `test_get_magic_file_path_custom`
- `test_get_magic_file_path_default`
- `test_default_magic_file_path`

**Validation Function Tests (lines 1040-1229):**
- `test_validate_arguments_empty_files`
- `test_validate_arguments_empty_magic_file`
- `test_validate_arguments_valid`
- `test_validate_input_file_not_found`
- `test_validate_input_file_directory`
- `test_validate_input_file_valid`
- `test_validate_magic_file_not_found`
- `test_validate_magic_file_directory`
- `test_validate_magic_file_empty`
- `test_validate_magic_file_whitespace_only`
- `test_validate_magic_file_valid`

**Magic File Search Order Tests (lines 1234-1405):**
- `test_magic_file_search_order_text_first`
- `test_magic_file_search_order_magdir_priority`
- `test_magic_file_candidates_exact_sequence`
- `test_magic_file_search_selects_first_existing`
- `test_magic_file_search_binary_fallback`

These tests don't require subprocess execution and test pure logic.

### 4. Remove FD Manipulation Code

Delete from `file:src/main.rs`:

- Lines 623-634: `FD_MUTEX` static and imports
- Lines 636-706: `capture_stdout()` and `capture_stderr()` functions
- Lines 708-763: `with_mocked_stdin()` and `with_invalid_stdin()` functions
- Lines 765-798: `resolve_magic_file_for_stdin_tests()` with `LLVM_PROFILE_FILE` check

### 5. Update Cargo.toml

After migration is complete and verified, optionally remove or reduce `nix` dependency:

```toml
# Remove if no longer needed:
# nix = { version = "0.31.1", features = ["fs"] }

# Or keep only if used elsewhere in tests
```

### 6. Verification Steps

**Test Coverage Verification:**

```bash
# Run all tests to ensure nothing breaks
cargo test

# Run specific test suites
cargo test --test cli_integration
cargo test --lib

# Verify coverage works without LLVM_PROFILE_FILE workaround
cargo llvm-cov --html

# Verify parallel execution works (no FD_MUTEX needed)
cargo test -- --test-threads=8
```

**Line Count Verification:**

```bash
# Check main.rs is under 600 lines
wc -l src/main.rs

# Should show approximately:
# - 250 lines: imports, structs, main logic
# - 300 lines: remaining unit tests
# - Total: ~550 lines
```

**Test Migration Checklist:**

- [ ] All stdin tests migrated and passing
- [ ] All multiple file tests migrated and passing
- [ ] All builtin flag tests migrated and passing
- [ ] All error handling tests migrated and passing
- [ ] All strict mode tests migrated and passing
- [ ] Unit tests still pass in main.rs
- [ ] No `LLVM_PROFILE_FILE` checks remain
- [ ] No `FD_MUTEX` usage remains
- [ ] `cargo test` passes with default thread count
- [ ] `cargo llvm-cov` runs successfully
- [ ] main.rs under 600 lines

### 7. Documentation Updates

Update `file:docs/src/testing.md`:

- Remove references to fd manipulation and `FD_MUTEX`
- Add section on CLI integration tests using `assert_cmd`
- Update test count statistics
- Document new test organization (unit tests in src/, integration in tests/)

## Test Migration Mapping

```mermaid
graph TD
    A[main.rs Tests<br/>~1100 lines] --> B[cli_integration.rs<br/>CLI Behavior Tests]
    A --> C[main.rs Unit Tests<br/>~300 lines]
    
    B --> D[Stdin Tests<br/>assert_cmd + write_stdin]
    B --> E[Multiple Files<br/>subprocess execution]
    B --> F[Builtin Flags<br/>flag combinations]
    B --> G[Error Handling<br/>exit code validation]
    
    C --> H[Arg Parsing<br/>clap::Parser tests]
    C --> I[Validation<br/>pure function tests]
    C --> J[Magic File Search<br/>path resolution tests]
    
    style A fill:#ffcccc
    style B fill:#ccffcc
    style C fill:#ccccff
```

---

Execution Information

Branch: main
Commit: 36adac5

---

💡 Tips

Supported Commands (Inside Comments)

• Use @traycerai generate to iterate on the previous version of the implementation plan.

Supported Commands (Inside Description)

• Add @traycerai ignore anywhere in the ticket description to prevent this ticket from being processed.
• Add @traycerai branch:<branch-name> anywhere in the ticket description to specify the target branch for the implementation plan.

Community

• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.