Skip to content

Commit d3fd6cb

Browse files
justin808claude
andcommitted
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 <noreply@anthropic.com>
1 parent 777bee2 commit d3fd6cb

File tree

2 files changed

+148
-18
lines changed

2 files changed

+148
-18
lines changed

lib/react_on_rails/dev/pack_generator.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,10 @@ def handle_rake_error(error, _silent)
105105
error_msg += "\n#{error.backtrace.join("\n")}" if ENV["DEBUG"]
106106

107107
# Always write to stderr, even in silent mode
108-
warn error_msg
108+
# Use STDERR constant instead of warn/$stderr to bypass capture_output redirection
109+
# rubocop:disable Style/StderrPuts, Style/GlobalStdStream
110+
STDERR.puts error_msg
111+
# rubocop:enable Style/StderrPuts, Style/GlobalStdStream
109112
end
110113

111114
def run_via_bundle_exec(silent: false)

spec/react_on_rails/dev/pack_generator_spec.rb

Lines changed: 144 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,157 @@
55

66
RSpec.describe ReactOnRails::Dev::PackGenerator do
77
describe ".generate" do
8-
it "runs pack generation successfully in verbose mode" do
9-
allow(described_class).to receive(:system)
10-
.with("bundle", "exec", "rake", "react_on_rails:generate_packs")
11-
.and_return(true)
8+
context "when in Bundler context with Rails available" do
9+
let(:mock_task) { instance_double(Rake::Task) }
10+
let(:mock_rails_app) do
11+
# rubocop:disable RSpec/VerifiedDoubles
12+
double("Rails.application").tap do |app|
13+
allow(app).to receive(:load_tasks)
14+
allow(app).to receive(:respond_to?).with(:load_tasks).and_return(true)
15+
end
16+
# rubocop:enable RSpec/VerifiedDoubles
17+
end
1218

13-
expect { described_class.generate(verbose: true) }
14-
.to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
19+
before do
20+
# Setup Bundler context
21+
stub_const("Bundler", Module.new)
22+
allow(ENV).to receive(:[]).and_call_original
23+
allow(ENV).to receive(:[]).with("BUNDLE_GEMFILE").and_return("/path/to/Gemfile")
24+
25+
# Setup Rails availability
26+
app = mock_rails_app
27+
rails_module = Module.new do
28+
define_singleton_method(:application) { app }
29+
define_singleton_method(:respond_to?) { |method, *| method == :application }
30+
end
31+
stub_const("Rails", rails_module)
32+
33+
# Mock Rake::Task at the boundary
34+
allow(Rake::Task).to receive(:task_defined?).with("react_on_rails:generate_packs").and_return(false)
35+
allow(Rake::Task).to receive(:[]).with("react_on_rails:generate_packs").and_return(mock_task)
36+
allow(mock_task).to receive(:reenable)
37+
allow(mock_task).to receive(:invoke)
38+
end
39+
40+
it "runs pack generation successfully in verbose mode using direct rake execution" do
41+
expect { described_class.generate(verbose: true) }
42+
.to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
43+
44+
expect(mock_task).to have_received(:invoke)
45+
expect(mock_rails_app).to have_received(:load_tasks)
46+
end
47+
48+
it "runs pack generation successfully in quiet mode using direct rake execution" do
49+
expect { described_class.generate(verbose: false) }
50+
.to output(/📦 Generating packs\.\.\. ✅/).to_stdout_from_any_process
51+
52+
expect(mock_task).to have_received(:invoke)
53+
end
54+
55+
it "exits with error when pack generation fails" do
56+
allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Task failed"))
57+
58+
# Mock STDERR.puts to capture output
59+
error_output = []
60+
# rubocop:disable Style/GlobalStdStream
61+
allow(STDERR).to receive(:puts) { |msg| error_output << msg }
62+
# rubocop:enable Style/GlobalStdStream
63+
64+
expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
65+
expect(error_output.join("\n")).to match(/Error generating packs: Task failed/)
66+
end
67+
68+
it "outputs errors to stderr even in silent mode" do
69+
allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Silent mode error"))
70+
71+
# Mock STDERR.puts to capture output
72+
error_output = []
73+
# rubocop:disable Style/GlobalStdStream
74+
allow(STDERR).to receive(:puts) { |msg| error_output << msg }
75+
# rubocop:enable Style/GlobalStdStream
76+
77+
expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
78+
expect(error_output.join("\n")).to match(/Error generating packs: Silent mode error/)
79+
end
80+
81+
it "includes backtrace in error output when DEBUG env is set" do
82+
allow(ENV).to receive(:[]).with("DEBUG").and_return("true")
83+
allow(mock_task).to receive(:invoke).and_raise(StandardError.new("Debug error"))
84+
85+
# Mock STDERR.puts to capture output
86+
error_output = []
87+
# rubocop:disable Style/GlobalStdStream
88+
allow(STDERR).to receive(:puts) { |msg| error_output << msg }
89+
# rubocop:enable Style/GlobalStdStream
90+
91+
expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
92+
expect(error_output.join("\n")).to match(/Error generating packs: Debug error.*pack_generator_spec\.rb/m)
93+
end
94+
95+
it "suppresses stdout in silent mode" do
96+
# Mock task to produce output
97+
allow(mock_task).to receive(:invoke) do
98+
puts "This should be suppressed"
99+
end
100+
101+
expect { described_class.generate(verbose: false) }
102+
.not_to output(/This should be suppressed/).to_stdout_from_any_process
103+
end
15104
end
16105

17-
it "runs pack generation successfully in quiet mode" do
18-
allow(described_class).to receive(:system)
19-
.with("bundle", "exec", "rake", "react_on_rails:generate_packs", out: File::NULL, err: File::NULL)
20-
.and_return(true)
106+
context "when not in Bundler context" do
107+
before do
108+
# Ensure we're not in Bundler context
109+
hide_const("Bundler") if defined?(Bundler)
110+
end
111+
112+
it "runs pack generation successfully in verbose mode using bundle exec" do
113+
command = "bundle exec rake react_on_rails:generate_packs"
114+
allow(described_class).to receive(:system).with(command).and_return(true)
115+
116+
expect { described_class.generate(verbose: true) }
117+
.to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
118+
119+
expect(described_class).to have_received(:system).with(command)
120+
end
121+
122+
it "runs pack generation successfully in quiet mode using bundle exec" do
123+
command = "bundle exec rake react_on_rails:generate_packs > /dev/null 2>&1"
124+
allow(described_class).to receive(:system).with(command).and_return(true)
21125

22-
expect { described_class.generate(verbose: false) }
23-
.to output(/📦 Generating packs\.\.\. ✅/).to_stdout_from_any_process
126+
expect { described_class.generate(verbose: false) }
127+
.to output(/📦 Generating packs\.\.\. ✅/).to_stdout_from_any_process
128+
129+
expect(described_class).to have_received(:system).with(command)
130+
end
131+
132+
it "exits with error when pack generation fails" do
133+
command = "bundle exec rake react_on_rails:generate_packs > /dev/null 2>&1"
134+
allow(described_class).to receive(:system).with(command).and_return(false)
135+
136+
expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
137+
end
24138
end
25139

26-
it "exits with error when pack generation fails" do
27-
allow(described_class).to receive(:system)
28-
.with("bundle", "exec", "rake", "react_on_rails:generate_packs", out: File::NULL, err: File::NULL)
29-
.and_return(false)
140+
context "when Rails is not available" do
141+
before do
142+
stub_const("Bundler", Module.new)
143+
allow(ENV).to receive(:[]).and_call_original
144+
allow(ENV).to receive(:[]).with("BUNDLE_GEMFILE").and_return("/path/to/Gemfile")
145+
146+
# Rails not available
147+
hide_const("Rails") if defined?(Rails)
148+
end
149+
150+
it "falls back to bundle exec when Rails is not defined" do
151+
command = "bundle exec rake react_on_rails:generate_packs"
152+
allow(described_class).to receive(:system).with(command).and_return(true)
153+
154+
expect { described_class.generate(verbose: true) }
155+
.to output(/📦 Generating React on Rails packs.../).to_stdout_from_any_process
30156

31-
expect { described_class.generate(verbose: false) }.to raise_error(SystemExit)
157+
expect(described_class).to have_received(:system).with(command)
158+
end
32159
end
33160
end
34161
end

0 commit comments

Comments
 (0)