Complete refactoring of call_count patterns in compiler_spec.rb

[justin808]

Summary

PR #854 refactored some tests in spec/shakapacker/compiler_spec.rb to use explicit have_received/not_to have_received assertions instead of indirect call_count counter patterns. However, several tests still use the anti-pattern and should be refactored for consistency.

Remaining tests to refactor

The following tests still use call_count patterns:

1. Line ~112: "handles hook with both stdout and stderr"
2. Line ~168: "handles hook commands with spaces in paths using Shellwords"
3. Line ~194: "supports hooks with additional arguments" (if exists)
4. Line ~241: "passes configured environment variables to precompile_hook" (if exists)
5. Line ~274: "merges hook environment variables with Rails.application.config_for" (if exists)

Why this matters

• Consistency: All tests should follow the same pattern
• Self-documenting: Explicit assertions make test intent clearer
• Robustness: Tests won't break if unrelated implementation details change
• Better failure messages: "expected not to receive X" vs "expected 1, got 2"

Additional considerations

Mock setup patterns

The codebase now has two patterns:

• Tests that need to differentiate calls: Use conditional blocks
• Tests where hooks aren't called: Use simple and_return

This is reasonable but worth documenting.

Skip test assertion

The "skips precompile_hook when SHAKAPACKER_SKIP_PRECOMPILE_HOOK=true" test uses:

expect(Open3).not_to have_received(:capture3).with(anything, hook_command, anything)
expect(Open3).to have_received(:capture3).once

This could theoretically pass if the hook was called with different arguments. Consider making the mock setup more explicit.

Related

• PR Improve test assertions using explicit have_received checks #854: Initial refactoring of call_count patterns
• See CLAUDE.md testing guidelines for the preferred pattern

🤖 Generated with Claude Code

[coderabbitai]

Duplicate Detection

ℹ️ This issue may be similar to shakacode/react_on_rails#2157.

Related Issues

• Improve test assertions: Replace ambiguous call_count patterns with explicit have_received checks react_on_rails#2157: Improve test assertions: Replace ambiguous call_count patterns with explicit have_received checks

Suggested Issue Assignees

• justin808

Actions

• 📝 Generate an implementation plan for this issue

---

_{ℹ️ Note: Issue enrichment is currently in early access.}

Disable automatic issue enrichment

_{To disable automatic issue enrichment, add the following to your .coderabbit.yaml:}

issue_enrichment:
  auto_enrich:
    enabled: false