From 1e1d85dbfd5ecb56e8a4bfc29767614284924794 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 19 Sep 2017 13:50:46 -0400 Subject: [PATCH] Fix incorrect use of Process::exit. This fixes open issue #244. Unlike the C library function exit(), Ruby's version takes a boolean, not an integer. Added a spec to check the actual shell-level exit value of a Thor script, and fixed calls to Process::exit. --- lib/thor/base.rb | 4 ++-- spec/fixtures/exit_status.thor | 19 +++++++++++++++++++ spec/script_exit_status_spec.rb | 29 +++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/exit_status.thor create mode 100644 spec/script_exit_status_spec.rb diff --git a/lib/thor/base.rb b/lib/thor/base.rb index cdbf39ea8..b0de5b2c0 100644 --- a/lib/thor/base.rb +++ b/lib/thor/base.rb @@ -466,13 +466,13 @@ def start(given_args = ARGV, config = {}) dispatch(nil, given_args.dup, nil, config) rescue Thor::Error => e config[:debug] || ENV["THOR_DEBUG"] == "1" ? (raise e) : config[:shell].error(e.message) - exit(1) if exit_on_failure? + exit(false) if exit_on_failure? rescue Errno::EPIPE # This happens if a thor command is piped to something like `head`, # which closes the pipe when it's done reading. This will also # mean that if the pipe is closed, further unnecessary # computation will not occur. - exit(0) + exit(true) end # Allows to use private methods from parent in child classes as commands. diff --git a/spec/fixtures/exit_status.thor b/spec/fixtures/exit_status.thor new file mode 100644 index 000000000..5fcebbaed --- /dev/null +++ b/spec/fixtures/exit_status.thor @@ -0,0 +1,19 @@ +require "thor" + +class ExitStatus < Thor + def self.exit_on_failure? + true + end + + desc "error", "exit with a planned error" + def error + raise Thor::Error.new("planned error") + end + + desc "ok", "exit with no error" + def ok + end +end + +ExitStatus.start(ARGV) + diff --git a/spec/script_exit_status_spec.rb b/spec/script_exit_status_spec.rb new file mode 100644 index 000000000..cbb6ded46 --- /dev/null +++ b/spec/script_exit_status_spec.rb @@ -0,0 +1,29 @@ +describe "when the Thor class's exit_with_failure? method returns true" do + def thor_command(command) + gem_dir= File.expand_path("#{File.dirname(__FILE__)}/..") + lib_path= "#{gem_dir}/lib" + script_path= "#{gem_dir}/spec/fixtures/exit_status.thor" + ruby_lib= ENV['RUBYLIB'].nil? ? lib_path : "#{lib_path}:#{ENV['RUBYLIB']}" + + full_command= "ruby #{script_path} #{command}" + r,w= IO.pipe + pid= spawn({'RUBYLIB' => ruby_lib}, + full_command, + {:out => w, :err => [:child, :out]}) + w.close + + junk, exit_status= Process.wait2(pid) + junk= r.read + r.close + + exit_status.exitstatus + end + + it "a command that raises a Thor::Error exits with a status of 1" do + expect(thor_command("error")).to eq(1) + end + + it "a command that does not raise a Thor::Error exits with a status of 0" do + expect(thor_command("ok")).to eq(0) + end +end