diff --git a/CHANGELOG b/CHANGELOG index dc2c99e0..d9a1833e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Make the "no matching servers" error more sane [halorgium] + * Make sure the invoke task gives a sane error when the COMMAND value is omitted [halorgium] * Make sure variables are conditionally set in the deploy recipes, so as not to clobber values set elsewhere [Jamis Buck] diff --git a/lib/capistrano/configuration/connections.rb b/lib/capistrano/configuration/connections.rb index 839b606b..e04a6730 100644 --- a/lib/capistrano/configuration/connections.rb +++ b/lib/capistrano/configuration/connections.rb @@ -105,7 +105,7 @@ module Capistrano servers = find_servers_for_task(task, options) if servers.empty? - raise ScriptError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched" + raise Capistrano::NoMatchingServersError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched" end if task.continue_on_error? @@ -114,7 +114,7 @@ module Capistrano end else servers = find_servers(options) - raise ScriptError, "no servers found to match #{options.inspect}" if servers.empty? + raise Capistrano::NoMatchingServersError, "no servers found to match #{options.inspect}" if servers.empty? end servers = [servers.first] if options[:once] diff --git a/lib/capistrano/errors.rb b/lib/capistrano/errors.rb index 81ec77a0..dbdd321a 100644 --- a/lib/capistrano/errors.rb +++ b/lib/capistrano/errors.rb @@ -3,6 +3,7 @@ module Capistrano class CaptureError < Error; end class NoSuchTaskError < Error; end + class NoMatchingServersError < Error; end class RemoteError < Error attr_accessor :hosts diff --git a/lib/capistrano/shell.rb b/lib/capistrano/shell.rb index 9267a530..01a3bf15 100644 --- a/lib/capistrano/shell.rb +++ b/lib/capistrano/shell.rb @@ -160,7 +160,7 @@ HELP connect(task) configuration.execute_task(task) end - rescue Capistrano::NoSuchTaskError => error + rescue Capistrano::NoMatchingServersError, Capistrano::NoSuchTaskError => error warn "error: #{error.message}" end diff --git a/lib/capistrano/task_definition.rb b/lib/capistrano/task_definition.rb index 58273f94..a13a5aa6 100644 --- a/lib/capistrano/task_definition.rb +++ b/lib/capistrano/task_definition.rb @@ -3,14 +3,16 @@ require 'capistrano/server_definition' module Capistrano # Represents the definition of a single task. class TaskDefinition - attr_reader :name, :namespace, :options, :body + attr_reader :name, :namespace, :options, :body, :desc, :on_error def initialize(name, namespace, options={}, &block) @name, @namespace, @options = name, namespace, options + @desc = @options.delete(:desc) + @on_error = options.delete(:on_error) @body = block or raise ArgumentError, "a task requires a block" @servers = nil end - + # Returns the task's fully-qualified name, including the namespace def fully_qualified_name @fully_qualified_name ||= begin @@ -28,7 +30,7 @@ module Capistrano def description(rebuild=false) @description = nil if rebuild @description ||= begin - description = options[:desc] || "" + description = @desc || "" indentation = description[/\A\s+/] if indentation @@ -61,7 +63,7 @@ module Capistrano # Indicates whether the task wants to continue, even if a server has failed # previously def continue_on_error? - options[:on_error] == :continue + @on_error == :continue end end end \ No newline at end of file diff --git a/test/configuration/connections_test.rb b/test/configuration/connections_test.rb index 9b0724d2..12747e9d 100644 --- a/test/configuration/connections_test.rb +++ b/test/configuration/connections_test.rb @@ -138,13 +138,13 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase def test_execute_on_servers_without_current_task_should_raise_error_if_no_matching_servers @config.expects(:find_servers).with(:a => :b, :c => :d).returns([]) - assert_raises(ScriptError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } } + assert_raises(Capistrano::NoMatchingServersError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } } end def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_matching_servers_by_default @config.current_task = mock_task @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([]) - assert_raises(ScriptError) do + assert_raises(Capistrano::NoMatchingServersError) do @config.execute_on_servers do flunk "should not get here" end diff --git a/test/configuration/namespace_dsl_test.rb b/test/configuration/namespace_dsl_test.rb index 4d475ea2..fceffd4a 100644 --- a/test/configuration/namespace_dsl_test.rb +++ b/test/configuration/namespace_dsl_test.rb @@ -58,14 +58,14 @@ class ConfigurationNamespacesDSLTest < Test::Unit::TestCase @config.desc "A description" @config.task(:testing) { puts "foo" } @config.task(:another) { puts "bar" } - assert_equal "A description", @config.tasks[:testing].options[:desc] - assert_nil @config.tasks[:another].options[:desc] + assert_equal "A description", @config.tasks[:testing].desc + assert_nil @config.tasks[:another].desc end def test_pending_desc_should_apply_only_to_next_task_in_any_namespace @config.desc "A description" @config.namespace(:outer) { task(:testing) { puts "foo" } } - assert_equal "A description", @config.namespaces[:outer].tasks[:testing].options[:desc] + assert_equal "A description", @config.namespaces[:outer].tasks[:testing].desc end def test_defining_task_without_block_should_raise_error