Skip to content

Commit 5738a3b

Browse files
committed
Abort if the command for run fails and exit_on_failure? is true
According to the description of `exit_on_failure?`, the process should exit with status 1 if any error happens. I think that it should be also applied to `run`. Although we can pass `abort_on_failure` option for `run`, passing it every time seems a little redundant when we call `run` multiple times. This is the other reason for this change.
1 parent fd114d1 commit 5738a3b

File tree

3 files changed

+36
-16
lines changed

3 files changed

+36
-16
lines changed

lib/thor/actions.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ def run(command, config = {})
259259

260260
result = config[:capture] ? `#{command}` : system(command.to_s)
261261

262-
if config[:abort_on_failure]
262+
if config.fetch(:abort_on_failure, self.class.exit_on_failure?)
263263
success = config[:capture] ? $?.success? : result
264264
abort unless success
265265
end

lib/thor/base.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,11 @@ def handle_argument_error(command, error, args, arity) #:nodoc:
506506
raise InvocationError, msg
507507
end
508508

509+
# A flag that makes the process exit with status 1 if any error happens.
510+
def exit_on_failure?
511+
false
512+
end
513+
509514
protected
510515

511516
# Prints the class options per group. If an option does not belong to
@@ -641,11 +646,6 @@ def from_superclass(method, default = nil)
641646
end
642647
end
643648

644-
# A flag that makes the process exit with status 1 if any error happens.
645-
def exit_on_failure?
646-
false
647-
end
648-
649649
#
650650
# The basename of the program invoking the thor class.
651651
#

spec/actions_spec.rb

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -292,20 +292,40 @@ def file
292292
end
293293

294294
describe "aborting on failure" do
295-
it "aborts when abort_on_failure is given and command fails" do
296-
expect { action :run, "false", :abort_on_failure => true }.to raise_error(SystemExit)
297-
end
295+
context "exit_on_failure? is false" do
296+
before do
297+
allow(MyCounter).to receive(:exit_on_failure?).and_return(false)
298+
end
298299

299-
it "suceeds when abort_on_failure is given and command succeeds" do
300-
expect { action :run, "true", :abort_on_failure => true }.not_to raise_error
301-
end
300+
it "aborts when abort_on_failure is given and command fails" do
301+
expect { action :run, "false", :abort_on_failure => true }.to raise_error(SystemExit)
302+
end
303+
304+
it "suceeds when abort_on_failure is given and command succeeds" do
305+
expect { action :run, "true", :abort_on_failure => true }.not_to raise_error
306+
end
307+
308+
it "aborts when abort_on_failure is given, capture is given and command fails" do
309+
expect { action :run, "false", :abort_on_failure => true, :capture => true }.to raise_error(SystemExit)
310+
end
302311

303-
it "aborts when abort_on_failure is given, capture is given and command fails" do
304-
expect { action :run, "false", :abort_on_failure => true, :capture => true }.to raise_error(SystemExit)
312+
it "suceeds when abort_on_failure is given and command succeeds" do
313+
expect { action :run, "true", :abort_on_failure => true, :capture => true }.not_to raise_error
314+
end
305315
end
306316

307-
it "suceeds when abort_on_failure is given and command succeeds" do
308-
expect { action :run, "true", :abort_on_failure => true, :capture => true }.not_to raise_error
317+
context "exit_on_failure? is true" do
318+
before do
319+
allow(MyCounter).to receive(:exit_on_failure?).and_return(true)
320+
end
321+
322+
it "aborts when command fails even if abort_on_failure is not given" do
323+
expect { action :run, "false" }.to raise_error(SystemExit)
324+
end
325+
326+
it "does not abort when abort_on_failure is false even if the command fails" do
327+
expect { action :run, "false", :abort_on_failure => false }.not_to raise_error
328+
end
309329
end
310330
end
311331

0 commit comments

Comments
 (0)