Fix daemonization broken in #219

lib/shoryuken/cli.rb

@@ -31,13 +31,13 @@ def run(args)
 
       options = parse_cli_args(args)
 
-      daemonize
-      write_pid
-
-      load_celluloid
+      daemonize(options)
+      write_pid(options)
 
       EnvironmentLoader.load(options)
 
+      load_celluloid
+
       require 'shoryuken/launcher'
       @launcher = Shoryuken::Launcher.new
 
@@ -64,21 +64,27 @@ def run(args)
     private
 
     def load_celluloid
-      raise "Celluloid cannot be required until here, or it will break Shoryuken's daemonization" if defined?(::Celluloid) && Shoryuken.options[:daemon]
-
-      # Celluloid can't be loaded until after we've daemonized
-      # because it spins up threads and creates locks which get
-      # into a very bad state if forked.
       require 'celluloid/autostart'
       Celluloid.logger = (Shoryuken.options[:verbose] ? Shoryuken.logger : nil)
 
       require 'shoryuken/manager'
     end
 
-    def daemonize
-      return unless Shoryuken.options[:daemon]
+    def celluloid_loaded?
+      defined?(::Celluloid)
+    end
+
+    def daemonize(options)
+      return unless options[:daemon]
 
-      fail ArgumentError, "You really should set a logfile if you're going to daemonize" unless Shoryuken.options[:logfile]
+      fail ArgumentError, "You really should set a logfile if you're going to daemonize" unless options[:logfile]
+
+      if celluloid_loaded?
+        # Celluloid can't be loaded until after we've daemonized
+        # because it spins up threads and creates locks which get
+        # into a very bad state if forked.
+        fail RuntimeError, "Celluloid cannot be required until here, or it will break Shoryuken's daemonization"
+      end
 
       files_to_reopen = []
       ObjectSpace.each_object(File) do |file|
@@ -96,16 +102,16 @@ def daemonize
       end
 
       [$stdout, $stderr].each do |io|
-        File.open(Shoryuken.options[:logfile], 'ab') do |f|
+        File.open(options[:logfile], 'ab') do |f|
           io.reopen(f)
         end
         io.sync = true
       end
       $stdin.reopen('/dev/null')
     end
 
-    def write_pid
-      if (path = Shoryuken.options[:pidfile])
+    def write_pid(options)
+      if (path = options[:pidfile])
         File.open(path, 'w') do |f|
           f.puts Process.pid
         end

spec/shoryuken/cli_spec.rb

@@ -0,0 +1,72 @@
+require 'spec_helper'
+require 'shoryuken/cli'
+require 'shoryuken/launcher'
+
+RSpec.describe Shoryuken::CLI do
+
+  let(:cli) { Shoryuken::CLI.instance }
+
+  before(:each) do
+    # make sure we do not bail
+    allow(cli).to receive(:exit)
+
+    # make sure we do not mess with standard streams
+    allow_any_instance_of(IO).to receive(:reopen)
+  end
+
+  describe '#run' do
+
+    let(:launcher) { instance_double('Shoryuken::Launcher') }
+
+    before(:each) do
+      allow(Shoryuken::Launcher).to receive(:new).and_return(launcher)
+      allow(launcher).to receive(:run).and_raise(Interrupt)
+      allow(launcher).to receive(:stop)
+    end
+
+    it 'does not raise' do
+      expect{ cli.run([]) }.to_not raise_error
+    end
+
+    it 'daemonizes with --daemon --logfile' do
+      expect(cli).to receive(:celluloid_loaded?).and_return(false)
+      expect(Process).to receive(:daemon)
+      cli.run(['--daemon', '--logfile', '/dev/null'])
+    end
+
+    it 'does NOT daemonize with --daemon --logfile' do
+      expect(Process).to_not receive(:daemon)
+      cli.run(['--logfile', '/dev/null'])
+    end
+
+    it 'writes PID file with --pidfile' do
+      pidfile = instance_double('File')
+      expect(File).to receive(:open).with('/dev/null', 'w').and_yield(pidfile)
+      expect(pidfile).to receive(:puts).with(Process.pid)
+      cli.run(['--pidfile', '/dev/null'])
+    end
+  end
+
+  describe '#daemonize' do
+
+    before(:each) do
+      allow(cli).to receive(:celluloid_loaded?).and_return(false)
+    end
+
+    it 'raises if logfile is not set' do
+      expect{ cli.send(:daemonize, { daemon: true }) }.to raise_error(ArgumentError)
+    end
+
+    it 'raises if Celluloid is already loaded' do
+      expect(cli).to receive(:celluloid_loaded?).and_return(true)
+      args = { daemon: true, logfile: '/dev/null' }
+      expect{ cli.send(:daemonize, args) }.to raise_error(RuntimeError)
+    end
+
+    it 'calls Process.daemon' do
+      args = { daemon: true, logfile: '/dev/null' }
+      expect(Process).to receive(:daemon).with(true, true)
+      cli.send(:daemonize, args)
+    end
+  end
+end