Fix critical error handling bug and improve test suite

This commit addresses critical issues identified in code review:

1. **Fix error handling in silent mode** (CRITICAL)
   - Changed from warn/\$stderr to STDERR.puts constant
   - Bypasses capture_output redirection to ensure errors always appear
   - Added RuboCop disable pragmas with detailed comments explaining why

2. **Refactor test suite to mock at boundaries**
   - Removed mocking of private methods (run_rake_task_directly)
   - Now mocks Rake::Task and system calls directly
   - Tests verify actual behavior, not implementation details
   - More resilient to refactoring

3. **Add comprehensive error handling tests**
   - Test that errors appear in stderr even in silent mode
   - Test backtrace inclusion when DEBUG env is set
   - Test stdout suppression in silent mode
   - Test Rails fallback when not available

4. **Improve test coverage and quality**
   - 10 examples, all passing
   - Tests now verify error output reaches STDERR correctly
   - Added context for Rails not being available

All tests pass (10 examples, 0 failures)
RuboCop passes with no offenses

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

Co-Authored-By: Claude <[email protected]>

Author: justin808

lib/react_on_rails/dev/pack_generator.rb

@@ -106,7 +106,10 @@ def handle_rake_error(error, _silent)
           error_msg += "\n#{error.backtrace.join("\n")}" if ENV["DEBUG"]
 
           # Always write to stderr, even in silent mode
-          warn error_msg
+          # Use STDERR constant instead of warn/$stderr to bypass capture_output redirection
+          # rubocop:disable Style/StderrPuts, Style/GlobalStdStream
+          STDERR.puts error_msg
+          # rubocop:enable Style/StderrPuts, Style/GlobalStdStream
         end
 
         def run_via_bundle_exec(silent: false)

spec/react_on_rails/dev/pack_generator_spec.rb

@@ -6,40 +6,107 @@
 RSpec.describe ReactOnRails::Dev::PackGenerator do
   describe ".generate" do
     context "when in Bundler context with Rails available" do
+      let(:mock_task) { instance_double(Rake::Task) }
+      let(:mock_rails_app) do
+        # rubocop:disable RSpec/VerifiedDoubles
+        double("Rails.application").tap do |app|
+          allow(app).to receive(:load_tasks)
+          allow(app).to receive(:respond_to?).with(:load_tasks).and_return(true)
+        end
+        # rubocop:enable RSpec/VerifiedDoubles
+      end
+
       before do
+        # Setup Bundler context
         stub_const("Bundler", Module.new)
         allow(ENV).to receive(:[]).and_call_original
         allow(ENV).to receive(:[]).with("BUNDLE_GEMFILE").and_return("/path/to/Gemfile")
-        allow(described_class).to receive(:rails_available?).and_return(true)
+
+        # Setup Rails availability
+        app = mock_rails_app
+        rails_module = Module.new do
+          define_singleton_method(:application) { app }
+          define_singleton_method(:respond_to?) { |method, *| method == :application }
+        end
+        stub_const("Rails", rails_module)
+
+        # Mock Rake::Task at the boundary
+        allow(Rake::Task).to receive(:task_defined?).with("react_on_rails:generate_packs").and_return(false)
+        allow(Rake::Task).to receive(:[]).with("react_on_rails:generate_packs").and_return(mock_task)
+        allow(mock_task).to receive(:reenable)
+        allow(mock_task).to receive(:invoke)
       end
 
       it "runs pack generation successfully in verbose mode using direct rake execution" do
-        allow(described_class).to receive(:run_rake_task_directly).and_return(true)
-
         expect { described_class.generate(verbose: true) }
           .to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
-        expect(described_class).to have_received(:run_rake_task_directly)
+
+        expect(mock_task).to have_received(:invoke)
+        expect(mock_rails_app).to have_received(:load_tasks)
       end
 
       it "runs pack generation successfully in quiet mode using direct rake execution" do
-        allow(described_class).to receive(:run_rake_task_directly).and_return(true)
-
         expect { described_class.generate(verbose: false) }
           .to output(/📦 Generating packs\.\.\. ✅/).to_stdout_from_any_process
-        expect(described_class).to have_received(:run_rake_task_directly)
+
+        expect(mock_task).to have_received(:invoke)
       end
 
       it "exits with error when pack generation fails" do
-        allow(described_class).to receive(:run_rake_task_directly).and_return(false)
+        allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Task failed"))
+
+        # Mock STDERR.puts to capture output
+        error_output = []
+        # rubocop:disable Style/GlobalStdStream
+        allow(STDERR).to receive(:puts) { |msg| error_output << msg }
+        # rubocop:enable Style/GlobalStdStream
 
         expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
+        expect(error_output.join("\n")).to match(/Error generating packs: Task failed/)
+      end
+
+      it "outputs errors to stderr even in silent mode" do
+        allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Silent mode error"))
+
+        # Mock STDERR.puts to capture output
+        error_output = []
+        # rubocop:disable Style/GlobalStdStream
+        allow(STDERR).to receive(:puts) { |msg| error_output << msg }
+        # rubocop:enable Style/GlobalStdStream
+
+        expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
+        expect(error_output.join("\n")).to match(/Error generating packs: Silent mode error/)
+      end
+
+      it "includes backtrace in error output when DEBUG env is set" do
+        allow(ENV).to receive(:[]).with("DEBUG").and_return("true")
+        allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Debug error"))
+
+        # Mock STDERR.puts to capture output
+        error_output = []
+        # rubocop:disable Style/GlobalStdStream
+        allow(STDERR).to receive(:puts) { |msg| error_output << msg }
+        # rubocop:enable Style/GlobalStdStream
+
+        expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
+        expect(error_output.join("\n")).to match(/Error generating packs: Debug error.*pack_generator_spec\.rb/m)
+      end
+
+      it "suppresses stdout in silent mode" do
+        # Mock task to produce output
+        allow(mock_task).to receive(:invoke) do
+          puts "This should be suppressed"
+        end
+
+        expect { described_class.generate(verbose: false) }
+          .not_to output(/This should be suppressed/).to_stdout_from_any_process
       end
     end
 
     context "when not in Bundler context" do
       before do
-        stub_const("Bundler", nil) unless defined?(Bundler)
-        allow(described_class).to receive(:should_run_directly?).and_return(false)
+        # Ensure we're not in Bundler context
+        hide_const("Bundler") if defined?(Bundler)
       end
 
       it "runs pack generation successfully in verbose mode using bundle exec" do
@@ -48,6 +115,8 @@
 
         expect { described_class.generate(verbose: true) }
           .to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
+
+        expect(described_class).to have_received(:system).with(command)
       end
 
       it "runs pack generation successfully in quiet mode using bundle exec" do
@@ -56,6 +125,8 @@
 
         expect { described_class.generate(verbose: false) }
           .to output(/📦 Generating packs\.\.\. ✅/).to_stdout_from_any_process
+
+        expect(described_class).to have_received(:system).with(command)
       end
 
       it "exits with error when pack generation fails" do
@@ -65,5 +136,26 @@
         expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
       end
     end
+
+    context "when Rails is not available" do
+      before do
+        stub_const("Bundler", Module.new)
+        allow(ENV).to receive(:[]).and_call_original
+        allow(ENV).to receive(:[]).with("BUNDLE_GEMFILE").and_return("/path/to/Gemfile")
+
+        # Rails not available
+        hide_const("Rails") if defined?(Rails)
+      end
+
+      it "falls back to bundle exec when Rails is not defined" do
+        command = "bundle exec rake react_on_rails:generate_packs"
+        allow(described_class).to receive(:system).with(command).and_return(true)
+
+        expect { described_class.generate(verbose: true) }
+          .to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
+
+        expect(described_class).to have_received(:system).with(command)
+      end
+    end
   end
 end