Improve test assertions using explicit have_received checks (#854)

## Summary

- Replace indirect `call_count` counter patterns with explicit RSpec spy
assertions
- Use `have_received(:capture3).with(hook_command)` to verify hook was
called
- Use `not_to have_received` to explicitly verify hook was skipped
- Simplify mock setup by using direct return values where possible

## Why this matters

The previous tests used indirect assertions like `expect(call_count).to
eq(1)` which:
- Don't explicitly prove what was or wasn't called
- Are brittle to implementation changes
- Require understanding the counter logic to interpret

The new assertions like `expect(Open3).not_to
have_received(:capture3).with(anything, hook_command, anything)` are:
- Self-documenting - explicitly states "hook was NOT called"
- More robust to implementation changes
- Produce clearer failure messages

## Test plan

- [ ] CI tests pass
- [ ] Verify test output is meaningful when assertions fail

Based on discussion:
https://github.com/shakacode/shakapacker/discussions/new?category=ideas

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Simplified and hardened precompile hook tests using direct spy
assertions, ensuring consistent verification across configured,
unconfigured, skipped, and edge-case scenarios (spaces or env
assignments) and verifying webpack runs independently of the hook.
* **Documentation**
* Updated testing guidance to recommend explicit spy assertions with
examples and warn against indirect call-count patterns.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: justin808

CLAUDE.md

@@ -12,6 +12,10 @@
 - Run corresponding RSpec tests when changing source files
 - For example, when changing `lib/shakapacker/foo.rb`, run `spec/shakapacker/foo_spec.rb`
 - Run the full test suite with `bundle exec rspec` before pushing
+- **Use explicit RSpec spy assertions** - prefer `have_received`/`not_to have_received` over indirect counter patterns
+  - Good: `expect(Open3).to have_received(:capture3).with(anything, hook_command, anything)`
+  - Good: `expect(Open3).not_to have_received(:capture3).with(anything, hook_command, anything)`
+  - Avoid: `call_count += 1` followed by `expect(call_count).to eq(1)`
 
 ## Code Style
 

spec/shakapacker/compiler_spec.rb

@@ -63,26 +63,20 @@
 
       hook_status = OpenStruct.new(success?: true, exitstatus: 0)
       webpack_status = OpenStruct.new(success?: true)
+      hook_command = "bin/test-hook"
 
-      # Mock to track hook call and allow subsequent webpack call
-      call_count = 0
       allow(Open3).to receive(:capture3) do |env, *args|
-        call_count += 1
-        if call_count == 1
-          # First call: hook with executable as first arg
-          expect(args[0]).to eq("bin/test-hook")
+        if args[0] == hook_command
           ["Hook output", "", hook_status]
         else
-          # Second call: webpack
           ["", "", webpack_status]
         end
       end
 
-      # Temporarily stub config to return hook
-      allow(Shakapacker.config).to receive(:precompile_hook).and_return("bin/test-hook")
+      allow(Shakapacker.config).to receive(:precompile_hook).and_return(hook_command)
 
       expect(Shakapacker.compiler.compile).to be true
-      expect(call_count).to eq(2) # Both hook and webpack were called
+      expect(Open3).to have_received(:capture3).with(hash_including, hook_command, hash_including).once
     end
 
     it "does not run precompile_hook when not configured" do
@@ -91,17 +85,14 @@
       allow(Shakapacker.compiler).to receive(:strategy).and_return(mocked_strategy)
 
       webpack_status = OpenStruct.new(success?: true)
-      call_count = 0
-      allow(Open3).to receive(:capture3) do |*args|
-        call_count += 1
-        ["", "", webpack_status]
-      end
+      allow(Open3).to receive(:capture3).and_return(["", "", webpack_status])
 
       # Config returns nil for precompile_hook (default)
       expect(Shakapacker.config.precompile_hook).to be_nil
 
       expect(Shakapacker.compiler.compile).to be true
-      expect(call_count).to eq(1) # Only webpack was called
+      # Verify webpack was called once, and no hook command was invoked
+      expect(Open3).to have_received(:capture3).once
     end
 
     it "raises error when precompile_hook fails" do
@@ -316,21 +307,21 @@
       allow(Shakapacker.compiler).to receive(:strategy).and_return(mocked_strategy)
 
       webpack_status = OpenStruct.new(success?: true)
-      call_count = 0
-      allow(Open3).to receive(:capture3) do |*args|
-        call_count += 1
-        ["", "", webpack_status]
-      end
+      hook_command = "bin/test-hook"
+      allow(Open3).to receive(:capture3).and_return(["", "", webpack_status])
 
       # Hook is configured
-      allow(Shakapacker.config).to receive(:precompile_hook).and_return("bin/test-hook")
+      allow(Shakapacker.config).to receive(:precompile_hook).and_return(hook_command)
 
       # But skip flag is set
       allow(ENV).to receive(:[]).and_call_original
       allow(ENV).to receive(:[]).with("SHAKAPACKER_SKIP_PRECOMPILE_HOOK").and_return("true")
 
       expect(Shakapacker.compiler.compile).to be true
-      expect(call_count).to eq(1) # Only webpack was called, hook was skipped
+      # Explicitly verify hook was NOT called
+      expect(Open3).not_to have_received(:capture3).with(anything, hook_command, anything)
+      # Verify webpack was still called
+      expect(Open3).to have_received(:capture3).once
     end
 
     it "runs precompile_hook when SHAKAPACKER_SKIP_PRECOMPILE_HOOK is not set" do
@@ -340,26 +331,25 @@
 
       hook_status = OpenStruct.new(success?: true, exitstatus: 0)
       webpack_status = OpenStruct.new(success?: true)
+      hook_command = "bin/test-hook"
 
-      call_count = 0
       allow(Open3).to receive(:capture3) do |env, *args|
-        call_count += 1
-        if call_count == 1
-          expect(args[0]).to eq("bin/test-hook")
+        if args[0] == hook_command
           ["Hook output", "", hook_status]
         else
           ["", "", webpack_status]
         end
       end
 
-      allow(Shakapacker.config).to receive(:precompile_hook).and_return("bin/test-hook")
+      allow(Shakapacker.config).to receive(:precompile_hook).and_return(hook_command)
 
       # Skip flag is not set
       allow(ENV).to receive(:[]).and_call_original
       allow(ENV).to receive(:[]).with("SHAKAPACKER_SKIP_PRECOMPILE_HOOK").and_return(nil)
 
       expect(Shakapacker.compiler.compile).to be true
-      expect(call_count).to eq(2) # Both hook and webpack were called
+      # Explicitly verify hook was called
+      expect(Open3).to have_received(:capture3).with(hash_including, hook_command, hash_including).once
     end
 
     it "runs precompile_hook when SHAKAPACKER_SKIP_PRECOMPILE_HOOK is false" do
@@ -369,26 +359,25 @@
 
       hook_status = OpenStruct.new(success?: true, exitstatus: 0)
       webpack_status = OpenStruct.new(success?: true)
+      hook_command = "bin/test-hook"
 
-      call_count = 0
       allow(Open3).to receive(:capture3) do |env, *args|
-        call_count += 1
-        if call_count == 1
-          expect(args[0]).to eq("bin/test-hook")
+        if args[0] == hook_command
           ["Hook output", "", hook_status]
         else
           ["", "", webpack_status]
         end
       end
 
-      allow(Shakapacker.config).to receive(:precompile_hook).and_return("bin/test-hook")
+      allow(Shakapacker.config).to receive(:precompile_hook).and_return(hook_command)
 
       # Skip flag is set to "false" (not "true")
       allow(ENV).to receive(:[]).and_call_original
       allow(ENV).to receive(:[]).with("SHAKAPACKER_SKIP_PRECOMPILE_HOOK").and_return("false")
 
       expect(Shakapacker.compiler.compile).to be true
-      expect(call_count).to eq(2) # Both hook and webpack were called
+      # Explicitly verify hook was called
+      expect(Open3).to have_received(:capture3).with(hash_including, hook_command, hash_including).once
     end
   end
 end