Skip to content

Commit dd85244

Browse files
justin808claude
andcommitted
Add comprehensive test coverage and edge case handling for terminate_processes
Improvements based on PR feedback: 1. Test Coverage: - Add 7 new test cases for terminate_processes method - Cover ESRCH, EPERM, ArgumentError, RangeError scenarios - Test mixed success/error cases 2. Edge Case Handling: - Handle ArgumentError (invalid signal) - Handle RangeError (invalid PID) - Consistent with file_manager.rb patterns 3. Return Value Consistency: - All rescue branches now explicitly return nil - Consistent behavior across all error types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1453829 commit dd85244

File tree

2 files changed

+63
-2
lines changed

2 files changed

+63
-2
lines changed

lib/react_on_rails/dev/server_manager.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,13 @@ def find_process_pids(pattern)
7070
def terminate_processes(pids)
7171
pids.each do |pid|
7272
Process.kill("TERM", pid)
73-
rescue Errno::ESRCH
74-
# Process already stopped - this is fine
73+
rescue Errno::ESRCH, ArgumentError, RangeError
74+
# Process already stopped, or invalid signal/PID - silently skip
7575
nil
7676
rescue Errno::EPERM
7777
# Permission denied - warn the user
7878
puts " ⚠️ Process #{pid} - permission denied (process owned by another user)"
79+
nil
7980
end
8081
end
8182

spec/react_on_rails/dev/server_manager_spec.rb

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,66 @@ def mock_system_calls
203203
end
204204
end
205205

206+
describe ".terminate_processes" do
207+
it "successfully kills processes" do
208+
pids = [1234, 5678]
209+
expect(Process).to receive(:kill).with("TERM", 1234)
210+
expect(Process).to receive(:kill).with("TERM", 5678)
211+
212+
described_class.terminate_processes(pids)
213+
end
214+
215+
it "handles ESRCH (process not found) silently" do
216+
pids = [1234]
217+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(Errno::ESRCH)
218+
219+
# Should not raise an error and should not output anything
220+
expect { described_class.terminate_processes(pids) }.not_to raise_error
221+
end
222+
223+
it "handles EPERM (permission denied) with warning" do
224+
pids = [1234]
225+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(Errno::EPERM)
226+
227+
# Should not raise an error but should output a warning
228+
expect { described_class.terminate_processes(pids) }.to output(/permission denied/).to_stdout_from_any_process
229+
end
230+
231+
it "handles mixed success and ESRCH" do
232+
pids = [1234, 5678]
233+
expect(Process).to receive(:kill).with("TERM", 1234)
234+
allow(Process).to receive(:kill).with("TERM", 5678).and_raise(Errno::ESRCH)
235+
236+
expect { described_class.terminate_processes(pids) }.not_to raise_error
237+
end
238+
239+
it "handles mixed success and EPERM" do
240+
pids = [1234, 5678]
241+
expect(Process).to receive(:kill).with("TERM", 1234)
242+
allow(Process).to receive(:kill).with("TERM", 5678).and_raise(Errno::EPERM)
243+
244+
expect do
245+
described_class.terminate_processes(pids)
246+
end.to output(/5678.*permission denied/).to_stdout_from_any_process
247+
end
248+
249+
it "handles ArgumentError (invalid signal)" do
250+
pids = [1234]
251+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(ArgumentError)
252+
253+
# Should not raise an error
254+
expect { described_class.terminate_processes(pids) }.not_to raise_error
255+
end
256+
257+
it "handles RangeError (invalid PID)" do
258+
pids = [999_999_999_999]
259+
allow(Process).to receive(:kill).with("TERM", 999_999_999_999).and_raise(RangeError)
260+
261+
# Should not raise an error
262+
expect { described_class.terminate_processes(pids) }.not_to raise_error
263+
end
264+
end
265+
206266
describe ".show_help" do
207267
it "displays help information" do
208268
expect { described_class.show_help }.to output(%r{Usage: bin/dev \[command\]}).to_stdout_from_any_process

0 commit comments

Comments
 (0)