From db91eb06cd3cb3f15ad3b1dfdf91230036ced71d Mon Sep 17 00:00:00 2001
From: Thibault Jouan <tj@a13.fr>
Date: Mon, 21 Jul 2014 12:45:57 +0000
Subject: [PATCH] Forward standard error stream from remote execution

---
 features/action_sh.feature                       | 10 ++++++++++
 features/steps/output_steps.rb                   |  4 ++++
 lib/producer/core/action.rb                      |  2 +-
 lib/producer/core/actions/shell_command.rb       |  2 +-
 lib/producer/core/cli.rb                         |  2 +-
 lib/producer/core/env.rb                         | 15 ++++++++-------
 lib/producer/core/remote.rb                      |  6 +++++-
 lib/producer/core/testing/mock_remote.rb         | 11 ++++++++---
 spec/producer/core/actions/shell_command_spec.rb |  9 +++++++++
 spec/producer/core/cli_spec.rb                   |  8 +++++++-
 spec/producer/core/env_spec.rb                   | 13 +++++++++++++
 spec/producer/core/remote_spec.rb                | 10 ++++++++++
 spec/support/shared_action.rb                    |  6 ++++++
 spec/support/test_env_helpers.rb                 | 12 ++++++++++--
 14 files changed, 93 insertions(+), 17 deletions(-)

diff --git a/features/action_sh.feature b/features/action_sh.feature
index 79352bf..eceb3c1 100644
--- a/features/action_sh.feature
+++ b/features/action_sh.feature
@@ -11,6 +11,16 @@ Feature: `sh' task action
     When I successfully execute the recipe on remote target
     Then the output must contain exactly "hello from remote\n"
 
+  Scenario: forwards error ouput
+    Given a recipe with:
+      """
+      task :sh_action do
+        sh '\echo error from remote >&2'
+      end
+      """
+    When I successfully execute the recipe on remote target
+    Then the error output must contain exactly "error from remote\n"
+
   Scenario: aborts on failed command execution
     Given a recipe with:
       """
diff --git a/features/steps/output_steps.rb b/features/steps/output_steps.rb
index 3ad5a72..690dd57 100644
--- a/features/steps/output_steps.rb
+++ b/features/steps/output_steps.rb
@@ -21,3 +21,7 @@ end
 Then /^the output must contain exactly:$/ do |content|
   assert_exact_output content, all_output
 end
+
+Then /^the error output must contain exactly "([^"]+)"$/ do |content|
+  assert_exact_output content, all_stderr
+end
diff --git a/lib/producer/core/action.rb b/lib/producer/core/action.rb
index 7fbb6ad..f8167c8 100644
--- a/lib/producer/core/action.rb
+++ b/lib/producer/core/action.rb
@@ -2,7 +2,7 @@ module Producer
   module Core
     class Action
       extend Forwardable
-      def_delegators :@env, :input, :output, :remote
+      def_delegators :@env, :input, :output, :error_output, :remote
       def_delegators :remote, :fs
 
       attr_reader :env, :arguments
diff --git a/lib/producer/core/actions/shell_command.rb b/lib/producer/core/actions/shell_command.rb
index 3ff2599..92eb265 100644
--- a/lib/producer/core/actions/shell_command.rb
+++ b/lib/producer/core/actions/shell_command.rb
@@ -7,7 +7,7 @@ module Producer
         end
 
         def apply
-          remote.execute(arguments.first, output)
+          remote.execute(arguments.first, output, error_output)
         end
       end
     end
diff --git a/lib/producer/core/cli.rb b/lib/producer/core/cli.rb
index bbad10a..121c846 100644
--- a/lib/producer/core/cli.rb
+++ b/lib/producer/core/cli.rb
@@ -31,7 +31,7 @@ module Producer
         @arguments  = args
         @stdin      = stdin
         @stdout     = stdout
-        @env        = Env.new(input: stdin, output: stdout)
+        @env        = Env.new(input: stdin, output: stdout, error_output: stderr)
       end
 
       def parse_arguments!
diff --git a/lib/producer/core/env.rb b/lib/producer/core/env.rb
index cb7acf7..e4bed73 100644
--- a/lib/producer/core/env.rb
+++ b/lib/producer/core/env.rb
@@ -1,15 +1,16 @@
 module Producer
   module Core
     class Env
-      attr_reader   :input, :output, :registry, :logger
+      attr_reader   :input, :output, :error_output, :registry, :logger
       attr_accessor :target, :verbose, :dry_run
 
-      def initialize(input: $stdin, output: $stdout, remote: nil, registry: {})
-        @verbose  = @dry_run = false
-        @input    = input
-        @output   = output
-        @remote   = remote
-        @registry = registry
+      def initialize(input: $stdin, output: $stdout, error_output: $stderr, remote: nil, registry: {})
+        @verbose      = @dry_run = false
+        @input        = input
+        @output       = output
+        @error_output = error_output
+        @remote       = remote
+        @registry     = registry
       end
 
       def remote
diff --git a/lib/producer/core/remote.rb b/lib/producer/core/remote.rb
index 5c7b16d..1b39c94 100644
--- a/lib/producer/core/remote.rb
+++ b/lib/producer/core/remote.rb
@@ -24,13 +24,17 @@ module Producer
         @fs ||= Remote::FS.new(session.sftp.connect)
       end
 
-      def execute(command, output = '')
+      def execute(command, output = '', error_output = '')
         channel = session.open_channel do |channel|
           channel.exec command do |ch, success|
             ch.on_data do |c, data|
               output << data
             end
 
+            ch.on_extended_data do |c, type, data|
+              error_output << data
+            end
+
             ch.on_request 'exit-status' do |c, data|
               exit_status = data.read_long
               fail RemoteCommandExecutionError, command if exit_status != 0
diff --git a/lib/producer/core/testing/mock_remote.rb b/lib/producer/core/testing/mock_remote.rb
index 9348098..453b85e 100644
--- a/lib/producer/core/testing/mock_remote.rb
+++ b/lib/producer/core/testing/mock_remote.rb
@@ -6,13 +6,18 @@ module Producer
           fail 'no session for mock remote!'
         end
 
-        def execute(command, output = '')
-          tokens = command.split
+        def execute(command, output = '', error_output = '')
+          tokens = command.gsub(/\d?>.*/, '').split
           program = tokens.shift
 
           case program
           when 'echo'
-            output << tokens.join(' ') << "\n"
+            out = tokens.join(' ') << "\n"
+            if command =~ />&2\z/
+              error_output << out
+            else
+              output << out
+            end
           when 'true'
             output << ''
           when 'false'
diff --git a/spec/producer/core/actions/shell_command_spec.rb b/spec/producer/core/actions/shell_command_spec.rb
index dbef67d..933bd2e 100644
--- a/spec/producer/core/actions/shell_command_spec.rb
+++ b/spec/producer/core/actions/shell_command_spec.rb
@@ -19,6 +19,15 @@ module Producer::Core
           sh.apply
           expect(output).to eq "#{command_args}\n"
         end
+
+        context 'when content is written to standard error' do
+          let(:command) { "echo #{command_args} >&2" }
+
+          it 'writes errors to given error stream' do
+            sh.apply
+            expect(error_output).to eq "#{command_args}\n"
+          end
+        end
       end
     end
   end
diff --git a/spec/producer/core/cli_spec.rb b/spec/producer/core/cli_spec.rb
index f15544f..7281d95 100644
--- a/spec/producer/core/cli_spec.rb
+++ b/spec/producer/core/cli_spec.rb
@@ -12,7 +12,9 @@ module Producer::Core
     let(:stdout)      { StringIO.new }
     let(:stderr)      { StringIO.new }
 
-    subject(:cli)     { CLI.new(arguments, stdin: stdin, stdout: stdout) }
+    subject(:cli)     { described_class.new(
+                          arguments,
+                          stdin: stdin, stdout: stdout, stderr: stderr) }
 
     describe '.run!' do
       let(:cli) { double('cli').as_null_object }
@@ -118,6 +120,10 @@ module Producer::Core
       it 'assigns CLI stdout as the env output' do
         expect(cli.env.output).to be stdout
       end
+
+      it 'assigns CLI stderr as the env error output' do
+        expect(cli.env.error_output).to be stderr
+      end
     end
 
     describe '#parse_arguments!' do
diff --git a/spec/producer/core/env_spec.rb b/spec/producer/core/env_spec.rb
index 78ea585..f55ab28 100644
--- a/spec/producer/core/env_spec.rb
+++ b/spec/producer/core/env_spec.rb
@@ -10,6 +10,10 @@ module Producer::Core
         expect(env.input).to be $stdin
       end
 
+      it 'assigns $stderr as the default error output' do
+        expect(env.error_output).to be $stderr
+      end
+
       it 'assigns no default target' do
         expect(env.target).not_to be
       end
@@ -51,6 +55,15 @@ module Producer::Core
         end
       end
 
+      context 'when error output is given as argument' do
+        let(:error_output)  { StringIO.new }
+        subject(:env)       { described_class.new(error_output: error_output) }
+
+        it 'assigns the given error output' do
+          expect(env.error_output).to be error_output
+        end
+      end
+
       context 'when remote is given as argument' do
         let(:remote)  { double 'remote' }
         subject(:env) { described_class.new(remote: remote) }
diff --git a/spec/producer/core/remote_spec.rb b/spec/producer/core/remote_spec.rb
index 15db48a..393c8a6 100644
--- a/spec/producer/core/remote_spec.rb
+++ b/spec/producer/core/remote_spec.rb
@@ -106,6 +106,16 @@ module Producer::Core
         expect(output.string).to eq arguments
       end
 
+      it 'writes command error output to provided error output' do
+        error_output = StringIO.new
+        story_with_new_channel do |ch|
+          ch.sends_exec command
+          ch.gets_extended_data arguments
+        end
+        remote.execute command, output, error_output
+        expect(error_output.string).to eq arguments
+      end
+
       context 'when command execution fails' do
         before do
           story_with_new_channel do |ch|
diff --git a/spec/support/shared_action.rb b/spec/support/shared_action.rb
index d6d8539..be65106 100644
--- a/spec/support/shared_action.rb
+++ b/spec/support/shared_action.rb
@@ -29,6 +29,12 @@ module Producer::Core
       end
     end
 
+    describe '#error_output' do
+      it 'returns env error output' do
+        expect(action.error_output).to be env.error_output
+      end
+    end
+
     describe '#remote' do
       it 'returns env remote' do
         expect(action.remote).to be env.remote
diff --git a/spec/support/test_env_helpers.rb b/spec/support/test_env_helpers.rb
index 6429aa5..4b32efa 100644
--- a/spec/support/test_env_helpers.rb
+++ b/spec/support/test_env_helpers.rb
@@ -9,6 +9,10 @@ module TestEnvHelpers
     env.output.string
   end
 
+  def error_output
+    env.error_output.string
+  end
+
   def remote_fs
     env.remote.fs
   end
@@ -17,14 +21,18 @@ module TestEnvHelpers
     opts = { expected_from: caller.first }
     RSpec::Mocks
       .expect_message(env.remote, :execute, opts)
-      .with(command, env.output)
+      .with(command, env.output, env.error_output)
   end
 
 
   private
 
   def build_env
-    Producer::Core::Env.new(output: StringIO.new, remote: build_remote)
+    Producer::Core::Env.new(
+      output:       StringIO.new,
+      error_output: StringIO.new,
+      remote:       build_remote
+    )
   end
 
   def build_remote