Skip to content

Commit

Permalink
fix double int without breaking stdin
Browse files Browse the repository at this point in the history
  • Loading branch information
grosser committed Feb 6, 2023
1 parent b0d2e56 commit 9c048b1
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Fixed

- Avoid double sending int while also not breaking debugging [#891](https://github.com/grosser/parallel_tests/pull/891)

## 4.1.0 - 2023-01-14

### Fixed
Expand Down
20 changes: 17 additions & 3 deletions lib/parallel_tests/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,23 @@ def handle_interrupt
@graceful_shutdown_attempted ||= false
Kernel.exit if @graceful_shutdown_attempted

# The Pid class's synchronize method can't be called directly from a trap
# Using Thread workaround https://github.com/ddollar/foreman/issues/332
Thread.new { ParallelTests.stop_all_processes }
# In a shell, all sub-processes also get an interrupt, so they shut themselves down.
# In a background process this does not happen and we need to do it ourselves.
# We cannot always send the interrupt since then the sub-processes would get interrupted twice when in foreground
# and that messes with interrupt handling.
#
# (can simulate detached with `(bundle exec parallel_rspec test/a_spec.rb -n 2 &)`)
# also the integration test "passes on int signal to child processes" is detached.
#
# On windows getpgid does not work so we resort to always killing which is the smaller bug.
#
# The ParallelTests::Pids `synchronize` method can't be called directly from a trap,
# using Thread workaround https://github.com/ddollar/foreman/issues/332
Thread.new do
if Gem.win_platform? || ((child_pid = ParallelTests.pids.all.first) && Process.getpgid(child_pid) != Process.pid)
ParallelTests.stop_all_processes
end
end

@graceful_shutdown_attempted = true
end
Expand Down
5 changes: 2 additions & 3 deletions lib/parallel_tests/test/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,10 @@ def print_command(command, env)
end

def execute_command_and_capture_output(env, cmd, options)
pid = nil

popen_options = { pgroup: true }
popen_options = {} # do not add `pgroup: true`, it will break `binding.irb` inside the test
popen_options[:err] = [:child, :out] if options[:combine_stderr]

pid = nil
output = IO.popen(env, cmd, popen_options) do |io|
pid = io.pid
ParallelTests.pids.add(pid)
Expand Down
5 changes: 4 additions & 1 deletion spec/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,10 @@ class A < Spinach::FeatureSteps
# Process.kill on Windows doesn't work as expected. It kills all process group instead of just one process.
it "passes on int signal to child processes", unless: Gem.win_platform? do
timeout = 2
write "spec/test_spec.rb", "sleep #{timeout}; describe { specify { 'Should not get here' }; specify { p 'Should not get here either'} }"
write(
"spec/test_spec.rb",
"sleep #{timeout}; describe { specify { p 'Should not get here' }; specify { p 'Should not get here either'} }"
)
pid = nil
Thread.new { sleep timeout - 0.5; Process.kill("INT", pid) }
result = run_tests(["spec"], processes: 2, type: 'rspec', fail: true) { |io| pid = io.pid }
Expand Down

0 comments on commit 9c048b1

Please sign in to comment.