Merge pull request #321 from guard/e2-fix_results_file_handling

improve results file handling

Author: e2

lib/guard/rspec/results.rb

@@ -0,0 +1,23 @@
+module Guard
+  class RSpec < Plugin
+    class Results
+      class InvalidData < RuntimeError
+      end
+
+      attr_reader :summary
+      attr_reader :failed_paths
+
+      def initialize(filename)
+        lines = File.readlines(filename)
+        if lines.empty? || lines.first.empty?
+          dump = lines.inspect
+          fail InvalidData, "Invalid results in: #{filename},"\
+            " lines:\n#{dump}\n"
+        end
+
+        @summary = lines.first.chomp
+        @failed_paths = lines[1..11].map(&:chomp).compact
+      end
+    end
+  end
+end

lib/guard/rspec/runner.rb

@@ -1,6 +1,7 @@
 require "guard/rspec/inspectors/factory"
 require "guard/rspec/command"
 require "guard/rspec/notifier"
+require "guard/rspec/results"
 
 module Guard
   class RSpec < Plugin
@@ -41,8 +42,12 @@ def _run(all, paths, options)
         return unless _cmd_option_present(options)
         command = Command.new(paths, options)
 
-        _without_bundler_env { Kernel.system(command) }.tap do |success|
-          _process_run_result(success, all)
+        _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
 
@@ -68,13 +73,7 @@ def _command_success?(success)
 
       def _command_output
         formatter_tmp_file = _tmp_file(options[:chdir])
-        lines = File.readlines(formatter_tmp_file)
-        summary = lines.first.strip
-        failed_paths = lines[1..11].map(&:strip).compact
-
-        [summary, failed_paths]
-      rescue
-        [nil, nil]
+        Results.new(formatter_tmp_file)
       ensure
         File.delete(formatter_tmp_file) if File.exist?(formatter_tmp_file)
       end
@@ -92,19 +91,9 @@ def _run_all_after_pass
       end
 
       def _process_run_result(result, all)
-        unless _command_success?(result)
-          notifier.notify_failure
-          return
-        end
-
-        summary, failed_paths = _command_output
-        unless summary && failed_paths
-          notifier.notify_failure
-          return
-        end
-
-        inspector.failed(failed_paths)
-        notifier.notify(summary)
+        results = _command_output
+        inspector.failed(results.failed_paths)
+        notifier.notify(results.summary)
         _open_launchy
 
         _run_all_after_pass if !all && result

spec/lib/guard/rspec/command_spec.rb

@@ -35,7 +35,8 @@
 
       before do
         allow(RSpec::Core::ConfigurationOptions).to receive(:new) do
-          double(options: { formatters: formatters })
+          instance_double(RSpec::Core::ConfigurationOptions,
+                          options: { formatters: formatters })
         end
       end
 

spec/lib/guard/rspec/results_spec.rb

@@ -0,0 +1,64 @@
+require "guard/compat/test/helper"
+
+require "guard/rspec/results"
+
+RSpec.describe Guard::RSpec::Results do
+  subject do
+    described_class.new("foo/bar.txt")
+  end
+
+  before do
+    allow(File).to receive(:readlines).with("foo/bar.txt").and_return(data)
+  end
+
+  context "with valid data" do
+    let(:data) do
+      [
+        "5 examples, 2 failures (3 pending)\n",
+        "foo1/bar1_spec.rb\n",
+        "foo1/bar2_spec.rb\n",
+      ]
+    end
+
+    describe "#summary" do
+      it "sets a summary" do
+        expect(subject.summary).to eq("5 examples, 2 failures (3 pending)")
+      end
+    end
+
+    describe "#failures" do
+      it "sets a list of failures" do
+        expect(subject.failed_paths).
+          to eq(%w(foo1/bar1_spec.rb foo1/bar2_spec.rb))
+      end
+    end
+  end
+
+  context "with no data" do
+    let(:data) do
+      []
+    end
+
+    it "crashes" do
+      expect do
+        subject.load
+      end.to raise_error(
+        Guard::RSpec::Results::InvalidData,
+        "Invalid results in: foo/bar.txt, lines:\n[]\n")
+    end
+  end
+
+  context "with invalid data" do
+    let(:data) do
+      [""]
+    end
+
+    it "crashes" do
+      expect do
+        subject.load
+      end.to raise_error(
+        Guard::RSpec::Results::InvalidData,
+        "Invalid results in: foo/bar.txt, lines:\n[\"\"]\n")
+    end
+  end
+end

spec/lib/guard/rspec/runner_spec.rb

@@ -7,8 +7,9 @@
 RSpec.describe Guard::RSpec::Runner do
   let(:options) { { cmd: "rspec" } }
   let(:runner) { Guard::RSpec::Runner.new(options) }
-  let(:inspector) { double(Guard::RSpec::Inspectors::SimpleInspector) }
-  let(:notifier) { double(Guard::RSpec::Notifier) }
+  let(:inspector) { instance_double(Guard::RSpec::Inspectors::SimpleInspector) }
+  let(:notifier) { instance_double(Guard::RSpec::Notifier) }
+  let(:results) { instance_double(Guard::RSpec::Results) }
 
   before do
     allow(Guard::Compat::UI).to receive(:info)
@@ -19,6 +20,11 @@
     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)
   end
 
@@ -68,7 +74,6 @@
 
     before do
       allow(inspector).to receive(:failed)
-      allow(File).to receive(:readlines).and_return([])
     end
 
     it "builds commands with spec paths" do
@@ -136,10 +141,7 @@
   describe "#run" do
     let(:paths) { %w(spec_path1 spec_path2) }
     before do
-      tmp_file = "tmp/rspec_guard_result"
-      allow(File).to receive(:readlines).with(tmp_file) { ["Summary\n"] }
       allow(inspector).to receive(:paths) { paths }
-      allow(inspector).to receive(:clear_paths) { true }
       allow(inspector).to receive(:failed)
     end
 
@@ -195,15 +197,12 @@
 
     context "with failed paths" do
       before do
-        allow(File).to receive(:readlines).
-          with("tmp/rspec_guard_result") do
-          [
-            "Summary\n",
-            "./failed_spec.rb:123\n",
-            "./other/failed_spec.rb:77\n"
-          ]
-        end
+        allow(results).to receive(:failed_paths).and_return([
+          "./failed_spec.rb:123",
+          "./other/failed_spec.rb:77"
+        ])
       end
+
       it "notifies inspector about failed paths" do
         expect(inspector).to receive(:failed).
           with(["./failed_spec.rb:123", "./other/failed_spec.rb:77"])

spec/lib/guard/rspec_spec.rb

@@ -5,7 +5,7 @@
   let(:default_options) { Guard::RSpec::Options::DEFAULTS }
   let(:options) { {} }
   let(:plugin) { Guard::RSpec.new(options) }
-  let(:runner) { double(Guard::RSpec::Runner) }
+  let(:runner) { instance_double(Guard::RSpec::Runner) }
 
   before do
     allow(Guard::Compat::UI).to receive(:info)

spec/spec_helper.rb

@@ -8,6 +8,14 @@
 rspec_version = ::RSpec::Version::STRING.to_f
 old_rspec = (rspec_version < 3)
 
+if old_rspec
+  class RSpec::Core::ExampleGroup
+    def instance_double(*args)
+      double(*args)
+    end
+  end
+end
+
 # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
 RSpec.configure do |config|
   config.expect_with :rspec do |expectations|