diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b740f0..c40322b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index 33ad63e0..efd5fe3d 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -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 diff --git a/lib/parallel_tests/test/runner.rb b/lib/parallel_tests/test/runner.rb index 38990d39..47651012 100644 --- a/lib/parallel_tests/test/runner.rb +++ b/lib/parallel_tests/test/runner.rb @@ -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) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index f3e54439..19ea69d2 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -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 }