Merge pull request #370 from guard/add_thorough_debugs_and_warnings

add thorough warnings to help users report issues

lib/guard/rspec/rspec_process.rb

@@ -26,6 +26,10 @@ def all_green?
       def _run
         _without_bundler_env do
           exit_code = _really_run
+
+          msg = "Guard::RSpec: RSpec command %s exited with: %s"
+          Compat::UI.debug(format(msg, command, exit_code.inspect))
+
           unless [0, Command::FAILURE_EXIT_CODE].include?(exit_code)
             msg = "Failed: %s (exit code: %d)"
             raise Failure, format(msg, command.inspect, exit_code)
@@ -36,6 +40,11 @@ def _run
 
       def _really_run
         env = { "GUARD_RSPEC_RESULTS_FILE" => formatter_tmp_file }
+
+        _warn_unless_absolute_path(formatter_tmp_file)
+
+        Compat::UI.debug("Guard::RSpec: results file: #{formatter_tmp_file}")
+
         pid = Kernel.spawn(env, command) # use spawn to stub in JRuby
         result = ::Process.wait2(pid)
         result.last.exitstatus
@@ -45,6 +54,13 @@ def _really_run
 
       def _read_results
         Results.new(formatter_tmp_file)
+      rescue Errno::ENOENT
+        msg = "Guard::RSpec cannot open results file: %s. This is likely a bug"\
+          "so please report this at"\
+          " http://github.com/guard/guard-rspec/issues/new along with as much"\
+          "information as possible to reproduce this issue."
+        Compat::UI.error(format(msg, formatter_tmp_file.inspect))
+        raise
       ensure
         File.delete(formatter_tmp_file) if File.exist?(formatter_tmp_file)
       end
@@ -57,6 +73,14 @@ def _without_bundler_env
         end
       end
 
+      def _warn_unless_absolute_path(formatter_tmp_file)
+        return if Pathname(formatter_tmp_file).absolute?
+
+        msg = "Guard::RSpec: The results file %s is not an absolute path."\
+          " Please provide an absolute path to avoid issues."
+        Compat::UI.warning(format(msg, formatter_tmp_file.inspect))
+      end
+
       attr_reader :command
       attr_reader :exit_code
       attr_reader :formatter_tmp_file

lib/guard/rspec/runner.rb

@@ -15,9 +15,6 @@ def initialize
         end
       end
 
-      # NOTE: must match with const in RSpecFormatter!
-      TEMPORARY_FILE_PATH ||= "tmp/rspec_guard_result".freeze
-
       attr_accessor :options, :inspector, :notifier
 
       def initialize(options = {})
@@ -85,9 +82,18 @@ def _open_launchy
       end
 
       def _results_file(results_file, chdir)
-        results_file ||= RSpecDefaults::TEMPORARY_FILE_PATH
+        results_file ||= File.expand_path(RSpecDefaults::TEMPORARY_FILE_PATH)
+        return results_file unless Pathname(results_file).relative?
+        results_file = File.join(chdir, results_file) if chdir
         return results_file unless Pathname(results_file).relative?
-        chdir ? File.join(chdir, results_file) : results_file
+
+        unless Pathname(results_file).absolute?
+          msg = "Guard::RSpec: The results file %s is not an absolute path."\
+            " Please provide an absolute path to avoid issues."
+          Compat::UI.warning(format(msg, results_file.inspect))
+        end
+
+        File.expand_path(results_file)
       end
     end
   end

lib/guard/rspec_formatter.rb

@@ -9,6 +9,8 @@
 
 require_relative "rspec_formatter_results_path"
 
+require "guard/ui"
+
 module Guard
   class RSpecFormatter < ::RSpec::Core::Formatters::BaseFormatter
     UNSUPPORTED_PATTERN =
@@ -115,6 +117,10 @@ def write_summary(duration, total, failures, pending)
 
     def _write(&block)
       file = RSpecFormatterResultsPath.new.path
+      if Guard.const_defined?(:Compat)
+        msg = "Guard::RSpec: using results file: #{file.inspect}"
+        Guard::Compat::UI.debug(format(msg, file))
+      end
       FileUtils.mkdir_p(File.dirname(file))
       File.open(file, "w", &block)
     end

spec/lib/guard/rspec/rspec_process_spec.rb

@@ -12,7 +12,7 @@
   let(:results) { instance_double(Guard::RSpec::Results) }
 
   let(:cmd) { "foo" }
-  let(:file) { "foobar.txt" }
+  let(:file) { "/tmp/foobar.txt" }
   let(:pid) { 1234 }
   let(:exit_code) { 0 }
   let(:status) { instance_double(Process::Status, exitstatus: exit_code) }
@@ -28,6 +28,8 @@
 
     allow(Guard::RSpec::Results).to receive(:new).
       with(file).and_return(results)
+
+    allow(Guard::Compat::UI).to receive(:debug)
   end
 
   context "with an non-existing command" do
@@ -73,6 +75,47 @@
       end
 
       it { is_expected.to be_all_green }
+
+      context "with a relative results file path" do
+        before do
+          allow(Guard::Compat::UI).to receive(:warning)
+        end
+        let(:file) { "foobar.txt" }
+
+        it "shows a warning" do
+          expect(Guard::Compat::UI).to receive(:warning).
+            with(/is not an absolute path/)
+          subject
+        end
+      end
+
+      context "with a missing results file" do
+        before do
+          allow(Guard::Compat::UI).to receive(:error)
+        end
+        before do
+          allow(Guard::RSpec::Results).to receive(:new).
+            with(file).and_raise(Errno::ENOENT, "foobar.txt")
+        end
+
+        it "shows a message" do
+          expect(Guard::Compat::UI).to receive(:error).
+            with(/cannot open results file/)
+
+          begin
+            subject
+          rescue Errno::ENOENT
+            nil
+          end
+        end
+
+        it "fails with an error" do
+          expect { subject }.to raise_error(
+            Errno::ENOENT,
+            /No such file or directory - foobar/
+          )
+        end
+      end
     end
   end
 end

spec/lib/guard/rspec/runner_spec.rb

@@ -14,6 +14,7 @@
 
   before do
     allow(Guard::Compat::UI).to receive(:info)
+    allow(Guard::Compat::UI).to receive(:warning)
     allow(Guard::Compat::UI).to receive(:error)
     allow(Guard::RSpec::Inspectors::Factory).to receive(:create) { inspector }
     allow(Guard::RSpec::Notifier).to receive(:new) { notifier }
@@ -214,7 +215,7 @@
         let(:chdir_options) { {} }
 
         context "when the path is relative" do
-          let(:results_file) { "foobar.txt" }
+          let(:results_file) { File.join(Dir.pwd, "foobar.txt") }
           it "uses the given file" do
             expect(Guard::RSpec::RSpecProcess).to receive(:new).
               with(anything, results_file).and_return(process)
@@ -233,24 +234,60 @@
       end
 
       context "with chdir option" do
-        let(:chdir_options) { { chdir: "moduleA" } }
-
-        context "when the path is relative" do
-          let(:results_file) { "foobar.txt" }
+        context "when chdir option is absolute" do
+          let(:chdir_options) { { chdir: "/foo/bar/moduleA" } }
+
+          context "when the path is relative" do
+            let(:results_file) { "foobar.txt" }
+
+            it "uses a path relative to chdir" do
+              expected = "/foo/bar/moduleA/foobar.txt"
+              expect(Guard::RSpec::RSpecProcess).to receive(:new).
+                with(anything, expected).and_return(process)
+              runner.run(paths)
+            end
+          end
 
-          it "uses a path relative to chdir" do
-            expect(Guard::RSpec::RSpecProcess).to receive(:new).
-              with(anything, "moduleA/foobar.txt").and_return(process)
-            runner.run(paths)
+          context "when the path is absolute" do
+            let(:results_file) { "/foo/foobar.txt" }
+            it "uses the full given path anyway" do
+              expect(Guard::RSpec::RSpecProcess).to receive(:new).
+                with(anything, results_file).and_return(process)
+              runner.run(paths)
+            end
           end
         end
 
-        context "when the path is absolute" do
-          let(:results_file) { "/foo/foobar.txt" }
-          it "uses the full given path anyway" do
-            expect(Guard::RSpec::RSpecProcess).to receive(:new).
-              with(anything, results_file).and_return(process)
-            runner.run(paths)
+        context "when chdir option is relative" do
+          let(:chdir_options) { { chdir: "moduleA" } }
+          before do
+            allow(Guard::Compat::UI).to receive(:warning)
+          end
+
+          context "when the path is relative" do
+            let(:results_file) { "foobar.txt" }
+
+            it "uses a path relative to chdir" do
+              expected = File.join(Dir.pwd, "moduleA/foobar.txt")
+              expect(Guard::RSpec::RSpecProcess).to receive(:new).
+                with(anything, expected).and_return(process)
+              runner.run(paths)
+            end
+
+            it "shows a warning" do
+              expect(Guard::Compat::UI).to receive(:warning).
+                with(/is not an absolute path/)
+              runner.run(paths)
+            end
+          end
+
+          context "when the path is absolute" do
+            let(:results_file) { "/foo/foobar.txt" }
+            it "uses the full given path anyway" do
+              expect(Guard::RSpec::RSpecProcess).to receive(:new).
+                with(anything, results_file).and_return(process)
+              runner.run(paths)
+            end
           end
         end
       end
@@ -260,7 +297,7 @@
       let(:options) { { cmd: "rspec" } }
       it "uses the default" do
         expect(Guard::RSpec::RSpecProcess).to receive(:new).
-          with(anything, "tmp/rspec_guard_result").and_return(process)
+          with(anything, %r{/tmp/rspec_guard_result$}).and_return(process)
         runner.run(paths)
       end
     end