refactor Runner + allow stubbing on JRuby

lib/guard/rspec/rspec_process.rb

@@ -0,0 +1,65 @@
+require "guard/rspec/command"
+
+module Guard
+  class RSpec < Plugin
+    class RSpecProcess
+      class Failure < RuntimeError
+      end
+
+      attr_reader :results
+
+      def initialize(command, formatter_tmp_file)
+        @command = command
+        @formatter_tmp_file = formatter_tmp_file
+        @results = nil
+
+        @exit_code = _run
+        @results = _read_results
+      end
+
+      def all_green?
+        exit_code.zero?
+      end
+
+      private
+
+      def _run
+        _without_bundler_env do
+          exit_code = _really_run
+          unless [0, Command::FAILURE_EXIT_CODE].include?(exit_code)
+            fail Failure, "Failed: #{command.inspect} (exit code: #{exit_code})"
+          end
+          exit_code
+        end
+      end
+
+      def _really_run
+        pid = Kernel.spawn(command) # use spawn to stub in JRuby
+        result = Process.wait2(pid)
+        result.last.exitstatus
+      rescue Errno::ENOENT => ex
+        fail Failure, "Failed: #{command.inspect} (#{ex})"
+      end
+
+      def _read_results
+        Results.new(formatter_tmp_file)
+      ensure
+        File.delete(formatter_tmp_file) if File.exist?(formatter_tmp_file)
+      end
+
+      def _without_bundler_env
+        if defined?(::Bundler)
+          ::Bundler.with_clean_env { yield }
+        else
+          yield
+        end
+      end
+
+      private
+
+      attr_reader :command
+      attr_reader :exit_code
+      attr_reader :formatter_tmp_file
+    end
+  end
+end

lib/guard/rspec/runner.rb

@@ -2,11 +2,18 @@
 require "guard/rspec/command"
 require "guard/rspec/notifier"
 require "guard/rspec/results"
+require "guard/rspec/rspec_process"
 
 module Guard
   class RSpec < Plugin
     class Runner
-      # NOTE: must match with const in RspecFormatter!
+      class NoCmdOptionError < RuntimeError
+        def initialize
+          super "No cmd option specified, unable to run specs!"
+        end
+      end
+
+      # NOTE: must match with const in RSpecFormatter!
       TEMPORARY_FILE_PATH ||= "tmp/rspec_guard_result"
 
       attr_accessor :options, :inspector, :notifier
@@ -22,14 +29,16 @@ def run_all
         options = @options.merge(@options[:run_all])
         return true if paths.empty?
         Compat::UI.info(options[:message], reset: true)
-        _run(true, paths, options)
+        _run(paths, options)
       end
 
       def run(paths)
         paths = inspector.paths(paths)
         return true if paths.empty?
         Compat::UI.info("Running: #{paths.join(' ')}", reset: true)
-        _run(false, paths, options)
+        _run(paths, options) do |all_green|
+          run_all if options[:all_after_pass] && all_green
+        end
       end
 
       def reload
@@ -38,44 +47,28 @@ def reload
 
       private
 
-      def _run(all, paths, options)
-        return unless _cmd_option_present(options)
-        command = Command.new(paths, options)
-
-        _without_bundler_env { Kernel.system(command) }.tap do |result|
-          if _command_success?(result)
-            _process_run_result(result, all)
-          else
-            notifier.notify_failure
-          end
-        end
-      end
-
-      def _without_bundler_env
-        if defined?(::Bundler)
-          ::Bundler.with_clean_env { yield }
-        else
-          yield
-        end
-      end
-
-      def _cmd_option_present(options)
-        return true if options[:cmd]
-        Compat::UI.error("No cmd option specified, unable to run specs!")
+      def _run(paths, options, &block)
+        fail NoCmdOptionError unless options[:cmd]
+        _really_run(paths, options, &block)
+        true
+      rescue RSpecProcess::Failure, NoCmdOptionError => ex
+        Compat::UI.error(ex.to_s)
         notifier.notify_failure
         false
       end
 
-      def _command_success?(success)
-        return false if success.nil?
-        [Command::FAILURE_EXIT_CODE, 0].include?($CHILD_STATUS.exitstatus)
-      end
+      def _really_run(paths, options)
+        # TODO: add option to specify the file
+        file = _tmp_file(options[:chdir])
+
+        process = RSpecProcess.new(Command.new(paths, options), file)
+        results = process.results
 
-      def _command_output
-        formatter_tmp_file = _tmp_file(options[:chdir])
-        Results.new(formatter_tmp_file)
-      ensure
-        File.delete(formatter_tmp_file) if File.exist?(formatter_tmp_file)
+        inspector.failed(results.failed_paths)
+        notifier.notify(results.summary)
+        _open_launchy
+
+        yield process.all_green? if block_given?
       end
 
       def _open_launchy
@@ -85,20 +78,6 @@ def _open_launchy
         ::Launchy.open(options[:launchy]) if pn.exist?
       end
 
-      def _run_all_after_pass
-        return unless options[:all_after_pass]
-        run_all
-      end
-
-      def _process_run_result(result, all)
-        results = _command_output
-        inspector.failed(results.failed_paths)
-        notifier.notify(results.summary)
-        _open_launchy
-
-        _run_all_after_pass if !all && result
-      end
-
       def _tmp_file(chdir)
         chdir ? File.join(chdir, TEMPORARY_FILE_PATH) : TEMPORARY_FILE_PATH
       end

spec/lib/guard/rspec/rspec_process_spec.rb

@@ -0,0 +1,77 @@
+require "guard/compat/test/helper"
+
+require "guard/rspec/rspec_process"
+
+RSpec.describe Guard::RSpec::RSpecProcess do
+  before do
+    allow(Kernel).to receive(:spawn) do |*args|
+      fail "Not stubbed: Kernel.spawn(#{args.map(&:inspect) * ','})"
+    end
+  end
+
+  let(:results) { instance_double(Guard::RSpec::Results) }
+
+  let(:cmd) { "foo" }
+  let(:file) { "foobar.txt" }
+  let(:pid) { 1234 }
+  let(:exit_code) { 0 }
+  let(:status) { instance_double(Process::Status, exitstatus: exit_code) }
+  let(:wait_result) { [pid, status] }
+
+  subject do
+    described_class.new(cmd, file)
+  end
+
+  before do
+    allow(Guard::RSpec::Results).to receive(:new).
+      with("foobar.txt").and_return(results)
+  end
+
+  context "with an non-existing command" do
+    before do
+      allow(Kernel).to receive(:spawn).
+        and_raise(Errno::ENOENT, "No such file or directory - foo")
+    end
+
+    it "fails" do
+      expect { subject }.
+        to raise_error(Guard::RSpec::RSpecProcess::Failure, /Failed: /)
+    end
+  end
+
+  context "with an existing command" do
+    before do
+      allow(Kernel).to receive(:spawn).with(cmd).and_return(pid)
+      allow(Process).to receive(:wait2).with(pid).and_return(wait_result)
+    end
+
+    context "with an unknown failure" do
+      let(:exit_code) { 100 }
+
+      it "fails" do
+        expect { subject }.
+          to raise_error(Guard::RSpec::RSpecProcess::Failure, /Failed: /)
+      end
+    end
+
+    context "with the failure code for normal test failures" do
+      let(:exit_code) { Guard::RSpec::Command::FAILURE_EXIT_CODE }
+
+      it "fails" do
+        expect { subject }.to_not raise_error
+      end
+
+      it { is_expected.to_not be_all_green }
+    end
+
+    context "with no failures" do
+      it "spawns and waits" do
+        expect(Kernel).to receive(:spawn).with(cmd).and_return(pid)
+        expect(Process).to receive(:wait2).with(pid).and_return(wait_result)
+        subject
+      end
+
+      it { is_expected.to be_all_green }
+    end
+  end
+end

spec/lib/guard/rspec/runner_spec.rb

@@ -10,22 +10,23 @@
   let(:inspector) { instance_double(Guard::RSpec::Inspectors::SimpleInspector) }
   let(:notifier) { instance_double(Guard::RSpec::Notifier) }
   let(:results) { instance_double(Guard::RSpec::Results) }
+  let(:process) { instance_double(Guard::RSpec::RSpecProcess) }
 
   before do
     allow(Guard::Compat::UI).to receive(:info)
-    allow(Kernel).to receive(:system) { true }
+    allow(Guard::Compat::UI).to receive(:error)
     allow(Guard::RSpec::Inspectors::Factory).to receive(:create) { inspector }
     allow(Guard::RSpec::Notifier).to receive(:new) { notifier }
     allow(Guard::RSpec::Command).to receive(:new) { "rspec" }
     allow(notifier).to receive(:notify)
     allow(notifier).to receive(:notify_failure)
 
-    allow(Guard::RSpec::Results).to receive(:new).
-      with("tmp/rspec_guard_result").and_return(results)
     allow(results).to receive(:summary).and_return("Summary")
     allow(results).to receive(:failed_paths).and_return([])
 
-    $CHILD_STATUS = double("exitstatus", exitstatus: 0)
+    allow(Guard::RSpec::RSpecProcess).to receive(:new).and_return(process)
+    allow(process).to receive(:all_green?).and_return(true)
+    allow(process).to receive(:results).and_return(results)
   end
 
   describe ".initialize" do
@@ -217,7 +218,9 @@
     end
 
     it "notifies failure" do
-      allow(Kernel).to receive(:system) { nil }
+      allow(process).to receive(:all_green?).
+        and_raise(Guard::RSpec::RSpecProcess::Failure, /Failed: /)
+
       expect(notifier).to receive(:notify_failure)
       runner.run(paths)
     end