Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handle interrupts #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better handle interrupts #46

wants to merge 1 commit into from

Conversation

Bo98
Copy link

@Bo98 Bo98 commented Dec 17, 2023

Currently if you hit Ctrl+C, the underlying rspec tests might continue to still run. This behaviours seemed to consistently happen in my environment (executed via bundle exec).

This PR adopts logic similar to parallel_tests that fixes this: https://github.com/grosser/parallel_tests/blob/6e9be6f529b64e5f9c8e4439754bca0b2ef3f2b1/lib/parallel_tests/cli.rb#L31-L54

An extra consideration for turbo_tests was that the Ctrl+C might happen before the FIFO file was written to, causing the read thread to block indefinitely. A fix for this is to make the process exit monitoring thread write to the FIFO file (if not already written to) to signal the other thread to stop.

@ilyazub ilyazub self-requested a review January 15, 2024 19:03
Copy link
Collaborator

@ilyazub ilyazub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bo98 Thanks for your work and my apologies for not reviewing your PRs sooner.

Let's also handle the race condition in handle_interrupt when the PID file was removed.

child_pid = wait_thr.pid
pgid = Process.respond_to?(:getpgid) ? Process.getpgid(child_pid) : 0
Process.kill(:INT, child_pid) if Process.pid != pgid
rescue Errno::ESRCH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also handle race condition when the PID file was removed.

$ bundle exec turbo_tests
Using recorded test runtime
3 processes for 3 specs, ~ 1 specs per process

Randomized with seed 27875

......

Finished in 3.36 seconds (files took 3.12 seconds to load)
6 examples, 0 failures


Randomized with seed 27875

#<Thread:0x0000000001b5c2b8 $HOME/Workspace/turbo_tests/lib/turbo_tests/runner.rb:224 run> terminated with exception (report_on_exception is true):
Traceback (most recent call last):
	1: from $HOME/Workspace/turbo_tests/lib/turbo_tests/runner.rb:232:in `block in start_subprocess'
$HOME/Workspace/turbo_tests/lib/turbo_tests/runner.rb:232:in `write': No such file or directory @ rb_sysopen - tmp/test-pipes/subprocess-2 (Errno::ENOENT)
bundler: failed to load command: turbo_tests ($HOME/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests)
Traceback (most recent call last):
	1: from $HOME/Workspace/turbo_tests/lib/turbo_tests/runner.rb:232:in `block in start_subprocess'
$HOME/Workspace/turbo_tests/lib/turbo_tests/runner.rb:232:in `write': No such file or directory @ rb_sysopen - tmp/test-pipes/subprocess-2 (Errno::ENOENT)
Suggested change
rescue Errno::ESRCH
rescue Errno::ESRCH, Errno::ENOENT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants