Refactor code duplication in test support utilities

[coderabbitai]

Problem

CodeScene analysis has identified code duplication between two functions in tests/support/mod.rs:

• fake_ninja
• fake_ninja_pwd

Both functions follow a similar pattern for creating fake executables with temporary directories and shell scripts, but with different script content.

Suggested Solution

Consider extracting the common functionality into a shared helper function that accepts the script content as a parameter, reducing duplication whilst maintaining the specific behaviour of each function.

Context

This issue was identified during PR review and flagged by automated analysis.

References:

• PR: Handle manifest path in process tests #56
• Comment: Handle manifest path in process tests #56 (comment)
• Issue: Refactor code duplication in test support utilities #63

Requested by: @leynos

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.

Once a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's examine the current content of tests/support/mod.rs
echo "=== Current content of tests/support/mod.rs ==="
cat tests/support/mod.rs

echo -e "\n=== Checking if fake_ninja and fake_ninja_pwd functions exist ==="
rg -n "fake_ninja" tests/support/mod.rs

echo -e "\n=== Recent git history for this file ==="
git log --oneline -5 -- tests/support/mod.rs

echo -e "\n=== Check for any recent commits mentioning duplication or refactor ==="
git log --oneline --grep="duplication\|refactor\|fake_ninja" --since="1 month ago"

Length of output: 688

---

🏁 Script executed:

#!/bin/bash
# Let's explore the current test directory structure
echo "=== Current test directory structure ==="
find tests -type f -name "*.rs" | head -20

echo -e "\n=== Search for fake_ninja functions across the entire codebase ==="
rg -n "fake_ninja" --type rust

echo -e "\n=== Look for any support or utility files in tests ==="
fd -t f . tests | grep -E "(support|util|helper)"

echo -e "\n=== Check if tests directory exists and its structure ==="
ls -la tests/ 2>/dev/null || echo "No tests directory found"

Length of output: 4484

---

🏁 Script executed:

#!/bin/bash
# Let's examine the current implementation in the test_support crate
echo "=== Content of test_support/src/lib.rs ==="
cat test_support/src/lib.rs

echo -e "\n=== Content of test_support/src/check_ninja.rs ==="
cat test_support/src/check_ninja.rs

echo -e "\n=== Check git log for test_support directory to see when it was created/modified ==="
git log --oneline --follow test_support/src/lib.rs | head -10

echo -e "\n=== Check when the original tests/support/mod.rs was removed ==="
git log --oneline --follow --stat -- tests/support/mod.rs | head -10

Length of output: 6477

---

🏁 Script executed:

#!/bin/bash
# Apply the low criticality label since this is test utility code
gh issue edit 63 --add-label "low"

# Check current labels to confirm
gh issue view 63 --json labels -q '.labels[].name'

Length of output: 135

---

@leynos The issue is still relevant. After examining the current codebase, I found that the code duplication between the fake ninja utility functions still exists, though the code has been refactored into a dedicated test_support crate.

Current State

The functions have been moved from tests/support/mod.rs to:

• fake_ninja() in test_support/src/lib.rs (lines 43-94)
• fake_ninja_check_build_file() in test_support/src/check_ninja.rs (lines 12-32)

Duplication Analysis

Both functions follow nearly identical patterns:

1. Create a TempDir with similar error handling
2. Create ninja executable files
3. Write shell/batch script content
4. Set executable permissions on Unix systems

The main differences are:

• Script content (exit {exit_code} vs build file validation logic)
• File naming (platform-specific vs Unix-only)
• Error message formatting

Criticality Assessment: LOW

Risk factors:

• User Impact: None - this is test utility code that users don't directly interact with
• Blast Radius: Limited to development/testing environment
• Maintenance Impact: Moderate - affects code maintainability and violates DRY principle

Applied label: low

Impact of Required Changes

Scope: Code refactoring to extract common functionality into a shared helper function

Benefits:

• Reduced code duplication (~30-40 lines of duplicated logic)
• Improved maintainability
• Single source of truth for fake executable creation logic
• Easier to extend with new fake ninja variants

Risk: Minimal - changes are isolated to test utilities with good test coverage

The refactor would involve extracting the common TempDir creation, file creation, and permission-setting logic into a shared helper that accepts script content as a parameter.