Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add thorough warnings to help users report issues #370

Merged
merged 1 commit into from
May 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions lib/guard/rspec/rspec_process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
16 changes: 11 additions & 5 deletions lib/guard/rspec/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {})
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/guard/rspec_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

require_relative "rspec_formatter_results_path"

require "guard/ui"

module Guard
class RSpecFormatter < ::RSpec::Core::Formatters::BaseFormatter
UNSUPPORTED_PATTERN =
Expand Down Expand Up @@ -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
Expand Down
45 changes: 44 additions & 1 deletion spec/lib/guard/rspec/rspec_process_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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
Expand Down Expand Up @@ -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
69 changes: 53 additions & 16 deletions spec/lib/guard/rspec/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down