Add a way to disable adding interrupts.

[ioquatix]

In code which needs to test fork and other process related functions, it should be possible to disable the interrupt handler, which interferes with code which has it's own signal handlers.

https://github.com/rspec/rspec-core/blob/d730c023b0765ffeaa5becb80f17e1fc02d2f9d1/lib/rspec/core/runner.rb#L65

Suggest something like config.disable_signal_handlers! or something similar.

[pirj]

@ioquatix can you please submit a minimal reproduction case that fails with trap_interrupt, but is passing without it?

[JonRowe]

Hm, they carry over to children? I mean we trap our INT for Ctrl C, without it theres no way to hard interrupt us.

[ioquatix]

Yes it carries to children.

[ioquatix]

Here is a working example:

#!/usr/bin/env rspec

RSpec.describe "fork" do
  it "handles interrupt correctly" do
    pid = fork do
      trap(:INT, :DEFAULT)
      sleep
    end

    Process.kill(:INT, pid)
		
    sleep(1)
		
    result = Process.wait(pid, Process::WNOHANG)
    expect(result).to_not be_nil
  end
end

[JonRowe]

edit: deleted comment was duplicate code, removed for clarity

Thats less than ideal... but you might be waiting forever with no way to quit if you disabled our handler, I'm not adverse to adding a way to suppress it however...

[JonRowe]

Is it possible to remove them? I vaguely remember issues with puma in a similar vein, I'm wondering if we can provide a method instead, wrap a test inside a "no trap" handler which has a backup timeout kill before restoring the old one?

[ioquatix]

If I was building RSpec, I'd want it to be minimally invasive on program state.

Even minitest gets this quite wrong IMHO: minitest/minitest#429

As signals are part of user code state, even if it's not traditionally something that needs to be tested, it does affect the behaviour of user-facing interfaces.

So, my suggestion would be to find a different solution. If the user presses Ctrl-C, the main thread will raise an Interrupt exception by default. I'd suggest you catch this instead.

So:

def run_tests
end

def run_all
  begin
     run_tests
  rescue Interrupt
    # exit and cancel current test
  end
end

[ioquatix]

Also, another point, pressing Ctrl-C, I expect a backtrace, but right now it gets swallowed up and often I never see it if the test has actually failed due to some blocking operation - presssing Ctrl-C to cancel the tests doesn't print any output.

[JonRowe]

The expected behaviour is that the first Ctrl-C prints a message but carries on waiting for the current test to finish. The second Ctrl-C causes a hard exit. We do this because it allows cleanup to run for tests that aren't blocked, so in the case of the majority of tests they get a clean exit, but we also allow overriding that as a, no now, type deal.

[ioquatix]

If the test will never finish, pressing Ctrl-C a second time exits with no backtrace. Is this intentional?

[JonRowe]

Not intentional, just a side effect of implementation, a second Ctrl-C causes us to exit, there is no backtrace as this is an intended Ruby exit, not a crash. I don't think we could display a stacktrace if we even wanted to...

[ioquatix]

I think you should make this behaviour optional and I'd probably immediately disable it in my template Rspec configuration, because I find it so annoying when debugging that exceptions are swallowed and backtraces are not printed.

[JonRowe]

As I said, open to improving this if you want to work on something better, but current behaviour needs to be preserved by default :)

[ioquatix]

Okay, I'll make a PR.