Permalink
Browse files

Cleanup interactor start/stop.

This removes unnecessary start/stop thread creation
for the interactor in the Runner and also some duplicate
interactor creation on Guard startup.

Before it behaved like:

1. Guard run some task
2. Runner loops through every Guard
3. Runner stops and start before each task the interactor

So when you had 5 plugins configured, it created/killed
7 threads on start, and 5 on each subsequent task.

The refactoring removes the responsibility of starting and
stopping the interactor from the Runner class. This means
that every class that makes use of the Runner must ensure
that the interactor is properly stopped and started by using
`Guard.within_preserved_state`.
  • Loading branch information...
1 parent ec16a74 commit 56ef54eff4173b12a911115e5240ffb98905791c @netzpirat netzpirat committed Jun 18, 2012
Showing with 55 additions and 47 deletions.
  1. +20 −13 lib/guard.rb
  2. +10 −3 lib/guard/interactor.rb
  3. +15 −17 lib/guard/runner.rb
  4. +4 −0 spec/guard/interactor_spec.rb
  5. +0 −5 spec/guard/runner_spec.rb
  6. +6 −9 spec/guard_spec.rb
View
@@ -104,7 +104,10 @@ def setup_signal_traps
def setup_listener
listener_callback = lambda do |modified, added, removed|
Dsl.reevaluate_guardfile if Watcher.match_guardfile?(modified)
- runner.run_on_changes(modified, added, removed)
+
+ ::Guard.within_preserved_state do
+ runner.run_on_changes(modified, added, removed)
+ end
end
listener_options = { :relative_paths => true }
@@ -126,7 +129,6 @@ def setup_notifier
def setup_interactor
unless options[:no_interactions]
@interactor = Interactor.fabricate
- @interactor.start if @interactor
end
end
@@ -151,40 +153,45 @@ def start(options = {})
setup(options)
UI.info "Guard is now watching at '#{ @watchdir }'"
- interactor.start if interactor
-
- runner.run(:start)
+ within_preserved_state do
+ runner.run(:start)
+ end
+
listener.start
end
# Stop Guard listening to file changes
#
def stop
+ listener.stop
interactor.stop if interactor
runner.run(:stop)
UI.info 'Bye bye...', :reset => true
- listener.stop
end
# Reload Guardfile and all Guards currently enabled.
#
# @param [Hash] scopes an hash with a guard or a group scope
#
def reload(scopes)
- UI.clear
- UI.action_with_scopes('Reload', scopes)
- Dsl.reevaluate_guardfile if scopes.empty?
- runner.run(:reload, scopes)
+ within_preserved_state do
+ UI.clear
+ UI.action_with_scopes('Reload', scopes)
+ Dsl.reevaluate_guardfile if scopes.empty?
+ runner.run(:reload, scopes)
+ end
end
# Trigger `run_all` on all Guards currently enabled.
#
# @param [Hash] scopes an hash with a guard or a group scope
#
def run_all(scopes)
- UI.clear
- UI.action_with_scopes('Run', scopes)
- runner.run(:run_all, scopes)
+ within_preserved_state do
+ UI.clear
+ UI.action_with_scopes('Run', scopes)
+ runner.run(:run_all, scopes)
+ end
end
# Pause Guard listening to file changes.
@@ -4,7 +4,8 @@ module Guard
autoload :CoollineInteractor, 'guard/interactors/coolline'
autoload :SimpleInteractor, 'guard/interactors/simple'
autoload :DslDescriber, 'guard/dsl_describer'
-
+ autoload :UI, 'guard/ui'
+
# The interactor triggers specific action from input
# read by a interactor implementation.
#
@@ -106,13 +107,17 @@ def self.available?(silent = false)
#
def start
return if ENV['GUARD_ENV'] == 'test'
+
+ ::Guard::UI.debug 'Start interactor'
@thread = Thread.new { read_line } if !@thread || !@thread.alive?
end
# Kill interactor thread if not current
#
def stop
- return if ENV['GUARD_ENV'] == 'test'
+ return if !@thread || ENV['GUARD_ENV'] == 'test'
+
+ ::Guard::UI.debug 'Stop interactor'
unless Thread.current == @thread
@thread.kill
end
@@ -147,7 +152,9 @@ def process_input(line)
when :reload
::Guard.reload(scopes)
when :change
- ::Guard.runner.run_on_changes(rest, [], [])
+ ::Guard.within_preserved_state do
+ ::Guard.runner.run_on_changes(rest, [], [])
+ end
when :run_all
::Guard.run_all(scopes)
when :notification
View
@@ -84,26 +84,24 @@ def run_on_changes(modified, added, removed)
# @raise [:task_has_failed] when task has failed
#
def run_supervised_task(guard, task, *args)
- ::Guard.within_preserved_state do
- begin
- catch Runner.stopping_symbol_for(guard) do
- guard.hook("#{ task }_begin", *args)
- result = guard.send(task, *args)
- guard.hook("#{ task }_end", result)
- result
- end
+ begin
+ catch Runner.stopping_symbol_for(guard) do
+ guard.hook("#{ task }_begin", *args)
+ result = guard.send(task, *args)
+ guard.hook("#{ task }_end", result)
+ result
+ end
- rescue NoMethodError
- # Do nothing
- rescue Exception => ex
- UI.error("#{ guard.class.name } failed to achieve its <#{ task.to_s }>, exception was:" +
- "\n#{ ex.class }: #{ ex.message }\n#{ ex.backtrace.join("\n") }")
+ rescue NoMethodError
+ # Do nothing
+ rescue Exception => ex
+ UI.error("#{ guard.class.name } failed to achieve its <#{ task.to_s }>, exception was:" +
+ "\n#{ ex.class }: #{ ex.message }\n#{ ex.backtrace.join("\n") }")
- ::Guard.guards.delete guard
- UI.info("\n#{ guard.class.name } has just been fired")
+ ::Guard.guards.delete guard
+ UI.info("\n#{ guard.class.name } has just been fired")
- ex
- end
+ ex
end
end
@@ -99,6 +99,10 @@
end
describe '#process_input' do
+ before do
+ ::Guard.stub(:within_preserved_state).and_yield
+ end
+
it 'shows the help on help action' do
subject.should_receive(:extract_scopes_and_action).with('help').and_return [{ }, :help, []]
subject.should_receive(:help)
@@ -236,11 +236,6 @@ class ::Guard::Bar2 < ::Guard::Guard; end
subject.run_supervised_task(foo_guard, :my_task)
end
- it 'runs the task within a preserved state' do
- guard_module.should_receive(:within_preserved_state)
- subject.run_supervised_task(foo_guard, :my_task)
- end
-
context 'with a task that succeeds' do
context 'without any arguments' do
before do
View
@@ -50,7 +50,7 @@
end
it "displays an error message when no guard are defined in Guardfile" do
- described_class::UI.should_receive(:error)
+ described_class::UI.should_receive(:error).with('No guards found in Guardfile, please add at least one.')
subject
end
@@ -66,6 +66,8 @@
end
describe ".setup_signal_traps" do
+ before { ::Guard::Dsl.stub(:evaluate_guardfile) }
+
unless windows?
context 'when receiving SIGUSR1' do
context 'when Guard is running' do
@@ -459,9 +461,9 @@ class Guard::FooBaz < Guard::Guard; end
describe ".start" do
before do
described_class.stub(:setup)
- described_class.stub(:interactor => mock('interactor', :start => true))
described_class.stub(:listener => mock('listener', :start => true))
described_class.stub(:runner => mock('runner', :run => true))
+ described_class.stub(:within_preserved_state).and_yield
end
it "setup Guard" do
@@ -477,12 +479,6 @@ class Guard::FooBaz < Guard::Guard; end
described_class.start
end
- it "tells the interactor to start" do
- described_class.interactor.should_receive(:start)
-
- described_class.start
- end
-
it "tell the runner to run the :start task" do
described_class.runner.should_receive(:run).with(:start)
@@ -588,7 +584,8 @@ class Guard::FooBaz < Guard::Guard; end
describe '.within_preserved_state' do
subject { ::Guard.setup }
-
+ before { Guard.interactor = ::Guard::Interactor.new }
+
it 'disables the interactor before running the block and then re-enables it when done' do
subject.interactor.should_receive(:stop)
subject.interactor.should_receive(:start)

1 comment on commit 56ef54e

@rymai
Member
rymai commented on 56ef54e Jun 18, 2012

Awesome! I actually began to code a refactor like this at the time of the listen branch integration but abandoned it because there was too much conflicts with already committed code...

That's a great refactor, thanks!

Please sign in to comment.