More tests, rearranged some behavior

* Callbacks::Executor::Executable no longer aborts when an
  unexecutable hook is detected. Instead, it's basically a no-op.
* Callbacks::Distributor::ViabilityFilter now warns on the
  detection of an unexecutable hook, but still passes the hook
  along for execution. This is to make things jibe with the old
  outside test suite, and it's actually a bit closer to how we
  want to handle things in the long run (detecting hook problems
  before they ever get a chance to execute).
* With the exception of (more or less) dumb data clump types,
  the entirety of the Callbacks namespace is tested to at least
  98% coverage in the inside suite.
* Deprecated some outside test examples, as they don't work well
  with the new hook implementation, and the outside suite is to
  have a full rewrite in the near future.

lib/engineyard-serverside/callbacks/distributor.rb

@@ -9,7 +9,7 @@ module Distributor
         def self.distribute(runner, hooks)
           ViabilityFilter.
             new.
-            call({:candidates => hooks}).
+            call({:candidates => hooks, :shell => runner.shell}).
             and_then {|callback_name|
               Remote.distribute(runner, callback_name)
             }

lib/engineyard-serverside/callbacks/distributor/viability_filter.rb

@@ -39,7 +39,13 @@ def check_executable_candidates(input = {})
               select {|hook| hook.flavor == :executable}
 
             hooks.each do |hook|
-              input[:viable].push(hook) if hook.path.executable?
+              if hook.path.executable?
+                input[:viable].push(hook)
+              else
+                input[:shell].warning(
+                  "Skipping possible deploy hook #{hook} because it is not executable."
+                )
+              end
             end
 
             Success(input)

lib/engineyard-serverside/callbacks/executor/executable.rb

@@ -19,7 +19,7 @@ def handle_failure(payload = {})
             case payload[:reason]
 
             when :not_executable
-              abort "*** [Error] Hook is not executable: #{hook_path} ***\n"
+              true
             when :execution_failed
               abort "*** [Error] Hook failed to exit cleanly: #{hook_path} ***\n"
             else

spec-inside/engineyard-serverside/callbacks/distributor/viability_filter_spec.rb

@@ -11,6 +11,7 @@ module Callbacks
       module Distributor
 
         describe ViabilityFilter do
+          let(:shell) {Object.new}
           let(:ruby_hook) {Object.new}
           let(:executable_hook) {Object.new}
           let(:executable_path) {Object.new}
@@ -28,6 +29,8 @@ module Distributor
 
             allow(executable_hook).to receive(:path).and_return(executable_path)
             allow(executable_path).to receive(:executable?).and_return(false)
+
+            allow(shell).to receive(:warning)
           end
 
           it 'is a Railway' do
@@ -48,8 +51,14 @@ module Distributor
           end
 
           describe '#normalize_input' do
-            let(:candidates) {[hooks]}
-            let(:input) {{:candidates => candidates}}
+            let(:candidates) {hooks}
+
+            let(:input) {
+              {
+                :candidates => candidates,
+                :shell => shell
+              }
+            }
 
             let(:result) {filter.normalize_input(input)}
 
@@ -74,6 +83,7 @@ module Distributor
             let(:input) {
               {
                 :candidates => hooks,
+                :shell => shell,
                 :viable => []
               }
             }
@@ -97,6 +107,7 @@ module Distributor
             let(:input) {
               {
                 :candidates => hooks,
+                :shell => shell,
                 :viable => []
               }
             }
@@ -126,6 +137,16 @@ module Distributor
                 allow(executable_path).to receive(:executable?).and_return(false)
               end
 
+              it 'warns the user that the hook is being skipped' do
+                expect(shell).
+                  to receive(:warning).
+                  with(
+                    "Skipping possible deploy hook #{executable_hook} because it is not executable."
+                  )
+
+                result
+              end
+
               it 'omits that hook from the viable array' do
                 expect(result.value[:viable]).not_to include(executable_hook)
               end
@@ -138,6 +159,7 @@ module Distributor
             let(:input) {
               {
                 :candidates => hooks,
+                :shell => shell,
                 :viable => viable
               }
             }

spec-inside/engineyard-serverside/callbacks/distributor_spec.rb

@@ -13,14 +13,20 @@ module Callbacks
 
         describe '.distribute' do
           let(:runner) {Object.new}
+          let(:shell) {Object.new}
           let(:hook_1) {Object.new}
           let(:hook_2) {Object.new}
           let(:matches) {[hook_2, hook_1]}
           let(:filter) {Object.new}
           let(:hook_name) {:some_hook}
-          let(:filter_args) {{:candidates => matches}}
           let(:failure) {Result::Failure.new({})}
           let(:success) {Result::Success.new(hook_name)}
+          let(:filter_args) {
+            {
+              :candidates => matches,
+              :shell => shell
+            }
+          }
 
           let(:result) {described_class.distribute(runner, matches)}
 
@@ -33,12 +39,14 @@ module Callbacks
 
             allow(described_class::Remote).to receive(:distribute)
 
-            allow(filter).to receive(:call)
+            allow(filter).to receive(:call).and_return(failure)
+
+            allow(runner).to receive(:shell).and_return(shell)
           end
 
           it 'filters the hooks for viability' do
-            expect(filter).
-              to receive(:call).
+            puts "failure == '#{failure}'"
+            expect(filter).to receive(:call).
               with(filter_args).
               and_return(failure)
 
@@ -49,7 +57,7 @@ module Callbacks
             before(:each) do
               allow(filter).
                 to receive(:call).
-                with({:candidates => matches}).
+                with(filter_args).
                 and_return(Result::Success.new(hook_name))
             end
 

spec-inside/engineyard-serverside/callbacks/executor/executable_spec.rb

@@ -27,12 +27,14 @@ module Executor
           let(:config_json) {'a big ol hash'}
           let(:verbose) {false}
           let(:execution_success) {true}
+          let(:hook_string) {'hooky'}
 
           let(:executor) {described_class.new(config, shell, hook)}
 
           before(:each) do
             allow(hook).to receive(:path).and_return(hook_path)
             allow(hook).to receive(:short_name).and_return(short_name)
+            allow(hook).to receive(:to_s).and_return(hook_string)
             allow(hook).
               to receive(:respond_to?).
               with(:service_name).
@@ -344,14 +346,8 @@ module Executor
             context 'when the hook is not actually executable' do
               let(:reason) {:not_executable}
 
-              it 'aborts with a message about the bad hook' do
-                expect(executor).
-                  to receive(:abort).
-                  with(
-                    "*** [Error] Hook is not executable: #{hook_path} ***\n"
-                  )
-
-                result
+              it 'does nothing and returns true' do
+                expect(result).to eql(true)
               end
             end