Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[Feature] Add the possibility to pause/unpause by sending OS signal #254

Merged
merged 1 commit into from

4 participants

@steakknife

USR1/USR2 signals and doc.

Future suggestion: print guard's pid when is verbose enabled to help users identify multiple guard processes.

@thibaudgg
Owner

Thanks @steakknife, can you please add some specs :)

@steakknife

Summary: Humour taken seriously may lead to awesome. :)

Although other pulls have tested signal handler code on each other, I'll see what's possible just for the sport of it.

Let me know if you find specs or helpers for rspec, rails, etc. that check signal handling.

@netzpirat
Owner

You can simply send a signal in the spec with Process.kill:

Process.kill :USR1, Process.pid
Barry Allard This implements #241.
Added a signal handler spec which may not be correct.
5c60354
@steakknife

This is probably wrong in 20 different ways, but here's a start.

@netzpirat netzpirat merged commit 5c60354 into guard:master
@netzpirat
Owner

Thanks for adding the specs, however they failed and the signal handling was not working at all. I had to move the method call to #add_signal_handlers outside of the listener thread to make it work.

In addition I merged your separate signals_spec.rb into the listener_spec.rb to follow the common lib/spec pattern and slim down the spec, since you tested a way too extensively: You don't need to test for combination of method call sequences, just each program execution path within the method (two signals, two conditions). The internal state of the listener is a simple @pause instance boolean and is not a state machine :P

Thanks for your effort.

@steakknife

I've never heard a complaint about too much testing unless test time slowed down production deployments.

Often, the problem is lack of testing.

Be careful what you wish for.

@netzpirat
Owner

There are also other aspects about tests: They have to be maintained by someone and they have to test the right thing.

Your #add_signal_handlers has four execution paths: two signal blocks and in each an if condition. This should result in four contexts to be specified, but your signal spec has 12 different contexts. So you're testing three times to much as necessary. When focusing on the important logic of the method, I actually see only two contexts, because the purpose of the two if is only informational.

In short, the added method does the following: SIGUSR1 => pause and SIGUSR2 => run. As I mentioned in my previous comment, you're testing different sequences of possible method calls. Why do you want to ensure that a second USR1 would result in something other than the first call? Do you want to test if the if statement really behaves the same on subsequent calls? The signal handlers are idempotent, you do not have to test the Ruby language constructs.

I really don't understand your reaction. It was you who said: "this is probably wrong in 20 different ways", and when I give you a honest feedback, you want to teach me in a childish way with your comment here and in #263. I really have no problem when someone teaches me, but it should be well-considered and constructive.

I also had the choice of simply not merge you changes, because it doesn't work as advertised and the provided specs are failing, but I decided to support your intention to add signal handlers, even though I don't need them for my personal workflow. And what do I get for spending my time to support you? A response in a disparaging way.

So long, and thanks for all the fish.

@rymai
Owner

@steakknife If you'd known @netzpirat personally, you wouldn't have answered him like that.

Please talk to him with the respect he deserves.

Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 14, 2012
  1. This implements #241.

    Barry Allard authored
    Added a signal handler spec which may not be correct.
This page is out of date. Refresh to see the latest.
View
12 README.md
@@ -339,6 +339,18 @@ $ guard start -I
$ guard start --no-vendor
```
+### Pause watching (all but Windows)
+
+```bash
+$ kill -USR1 <guard_pid>
+```
+
+### Continue watching (all but Windows)
+
+```bash
+$ kill -USR2 <guard_pid>
+```
+
### List
You can list the available Guards with the `list` task:
View
14 lib/guard/listener.rb
@@ -82,6 +82,7 @@ def start_reactor
return if ENV["GUARD_ENV"] == 'test'
Thread.new do
+ add_signal_handlers unless self.windows?
loop do
if @changed_files != [] && !@paused
changed_files = @changed_files.dup
@@ -138,6 +139,19 @@ def update_last_event
@last_event = Time.now
end
+ # Add signals handlers (works only on posix-compatible systems).
+ # (Yes, they're not atomic. Be nice to them. :)
+ def add_signal_handlers
+ Signal.trap("USR1") do
+ UI.info "Paused Guard on signal USR1" unless @paused
+ pause
+ end
+ Signal.trap("USR2") do
+ UI.info "Continued Guard on signal USR2" if @paused
+ run
+ end
+ end
+
# Get the modified files.
#
# If the `:watch_all_modifications` option is true, then moved and
View
197 spec/guard/listeners/signals_spec.rb
@@ -0,0 +1,197 @@
+require 'spec_helper'
+require 'guard/listener'
+
+describe Guard::Listener do
+
+ if windows?
+ STDERR.puts "ERROR guard listener signal testing not run because windows"
+ else # ! windows
+
+ describe 'when #initialize a new Listener' do
+ let(:guard) { Guard::Listener.new }
+ let(:ui) { Guard::UI }
+
+ before { ENV['GUARD_ENV'] = 'test_signals' }
+ after { ENV['GUARD_ENV'] = 'test' if ENV['GUARD_ENV'] == 'test_signals' }
+
+ # ---- USR* signals while running
+
+ context 'on an USR1 signal' do
+ before { Process.kill :USR1, Process.pid }
+
+ it '#pause once' do
+ guard.should_receive(:pause).once
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR2 signal' do
+ before { Process.kill :USR2, Process.pid }
+
+ it 'does nothing' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on duplicate USR1 signals' do
+ before do
+ Process.kill :USR1, Process.pid
+ Process.kill :USR1, Process.pid
+ end
+
+ it '#pause once' do
+ guard.should_receive(:pause).once
+ guard.should_receive(:run).any_number_of_times
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on duplicate USR2 signals' do
+ before do
+ Process.kill :USR2, Process.pid
+ Process.kill :USR2, Process.pid
+ end
+
+ it 'does nothing' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR1 and then USR2 signal' do
+ before do
+ Process.kill :USR1, Process.pid
+ Process.kill :USR2, Process.pid
+ end
+
+ it '#pause and then #run' do
+ guard.should_receive(:pause).once
+ guard.should_receive(:run).once
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR2 and then USR1 signal' do
+ before do
+ Process.kill :USR2, Process.pid
+ Process.kill :USR1, Process.pid
+ end
+
+ it '#run once' do
+ guard.should_receive(:pause).once
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+
+ # ---- USR* signals while started and then paused
+
+ context 'when #pause' do
+ before { guard.stub(:pause) }
+
+ context 'on an USR1 signal' do
+ before { Process.kill :USR1, Process.pid }
+
+ it 'does nothing' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR2 signal' do
+ before { Process.kill :USR2, Process.pid }
+
+ it '#pause once' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_receive(:run).once
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on duplicate USR1 signals' do
+ before do
+ Process.kill :USR1, Process.pid
+ Process.kill :USR1, Process.pid
+ end
+
+ it 'does nothing' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_not_receive(:run).any_number_of_times
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_not_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on duplicate USR2 signals' do
+ before do
+ Process.kill :USR2, Process.pid
+ Process.kill :USR2, Process.pid
+ end
+
+ it '#run once' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_receive(:run).once
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR1 and then USR2 signal' do
+ before do
+ Process.kill :USR1, Process.pid
+ Process.kill :USR2, Process.pid
+ end
+
+ it '#run once' do
+ guard.should_not_receive(:pause).any_number_of_times
+ guard.should_receive(:run).once
+
+ ui.should_not_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+
+ context 'on an USR2 and then USR1 signal' do
+ before do
+ Process.kill :USR2, Process.pid
+ Process.kill :USR1, Process.pid
+ end
+
+ it '#run and then #pause' do
+ guard.should_not_receive(:pause).once
+ guard.should_receive(:run).once
+
+ ui.should_receive(:info).with("Paused Guard on signal USR1")
+ ui.should_receive(:info).with("Continued Guard on signal USR2")
+ end
+ end
+ end # when #pause
+
+ end # describe when #start
+ end # !windows
+end # describe Guard::Listener
Something went wrong with that request. Please try again.