Simplify live_stream vs live_stdout vs live_stderr based on PR comments

4 files changed, 59 insertions, 48 deletions

diff --git a/lib/mixlib/shellout.rb b/lib/mixlib/shellout.rb
index a633819..6261c21 100644
--- a/lib/mixlib/shellout.rb
+++ b/lib/mixlib/shellout.rb
@@ -53,13 +53,13 @@ module Mixlib
     # to determine if the command was successful. Normally set via options to new
     attr_accessor :valid_exit_codes
 
-    # When live_stream is set, stdout and stderr of the subprocess will be
-    # copied to it as the subprocess is running. The stderr will also be copied,
-    # unless live_stderr_stream is set to nil or a different object. For example,
-    # if live_stream is set to STDOUT, the command's output will be echoed to STDOUT.
-    attr_accessor :live_stream
+    # When live_stdout is set, the stdout of the subprocess will be copied to it
+    # as the subprocess is running.
+    attr_accessor :live_stdout
 
-    attr_writer :live_stderr_stream
+    # When live_stderr is set, the stderr of the subprocess will be copied to it
+    # as the subprocess is running.
+    attr_accessor :live_stderr
 
     # ShellOut will push data from :input down the stdin of the subprocss.
     # Normally set via options passed to new.
@@ -150,7 +150,7 @@ module Mixlib
     #   cmd.run_command # etc.
     def initialize(*command_args)
       @stdout, @stderr = '', ''
-      @live_stream = nil
+      @live_stdout = @live_stderr = nil
       @input = nil
       @log_level = :debug
       @log_tag = nil
@@ -166,18 +166,16 @@ module Mixlib
       @command = command_args.size == 1 ? command_args.first : command_args
     end
 
-    # When live_stderr_stream is set, the stderr of the subprocess will be
-    # copied to it as the subprocess is running. For example, if live_stderr_stream is
-    # set to STDERR, the command's output will be echoed to STDERR. The default
-    # value is to match live_stream, so setting live_stream to STDOUT will also
-    # set live_stderr_stream to STDOUT. If you only want the stdout of the subprocess
-    # copied, then you should explicitly set live_stderr_stream to nil.
-    def live_stderr_stream
-      # We can't use ||= because it would override an explicit nil
-      unless defined?(@live_stderr_stream)
-        @live_stderr_stream = live_stream
-      end
-      @live_stderr_stream
+    # Returns the stream that both is being used by both live_stdout and live_stderr, or nil
+    def live_stream
+      live_stdout == live_stderr ? live_stdout : nil
+    end
+
+    # A shortcut for setting both live_stdout and live_stderr, so that both the
+    # stdout and stderr from the subprocess will be copied to the same stream as
+    # the subprocess is running.
+    def live_stream=(stream)
+      @live_stdout = @live_stderr = stream
     end
 
     # Set the umask that the subprocess will have. If given as a string, it
@@ -303,10 +301,11 @@ module Mixlib
         when 'returns'
           self.valid_exit_codes = Array(setting)
         when 'live_stream'
-          self.live_stream = setting
-        when 'live_stderr_stream'
-          puts "Setting to #{setting}"
-          self.live_stderr_stream = setting
+          self.live_stdout = self.live_stderr = setting
+        when 'live_stdout'
+          self.live_stdout = setting
+        when 'live_stderr'
+          self.live_stderr = setting
         when 'input'
           self.input = setting
         when 'logger'
diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb
index 249c3d4..be2e66a 100644
--- a/lib/mixlib/shellout/unix.rb
+++ b/lib/mixlib/shellout/unix.rb
@@ -278,7 +278,7 @@ module Mixlib
       def read_stdout_to_buffer
         while chunk = child_stdout.read_nonblock(READ_SIZE)
           @stdout << chunk
-          @live_stream << chunk if @live_stream
+          @live_stdout << chunk if @live_stdout
         end
       rescue Errno::EAGAIN
       rescue EOFError
@@ -288,7 +288,7 @@ module Mixlib
       def read_stderr_to_buffer
         while chunk = child_stderr.read_nonblock(READ_SIZE)
           @stderr << chunk
-          live_stderr_stream << chunk if live_stderr_stream
+          @live_stderr << chunk if @live_stderr
         end
       rescue Errno::EAGAIN
       rescue EOFError
diff --git a/lib/mixlib/shellout/windows.rb b/lib/mixlib/shellout/windows.rb
index 62adc37..2ae0256 100644
--- a/lib/mixlib/shellout/windows.rb
+++ b/lib/mixlib/shellout/windows.rb
@@ -167,7 +167,7 @@ module Mixlib
           begin
             next_chunk = stderr_read.readpartial(READ_SIZE)
             @stderr << next_chunk
-            @live_stderr_stream << next_chunk if @live_stderr_stream
+            @live_stderr << next_chunk if @live_stderr
           rescue EOFError
             stderr_read.close
             open_streams.delete(stderr_read)
diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb
index 72d550e..ff493c2 100644
--- a/spec/mixlib/shellout_spec.rb
+++ b/spec/mixlib/shellout_spec.rb
@@ -191,16 +191,20 @@ describe Mixlib::ShellOut do
           shell_cmd.live_stream = stream
         end
 
-        it "should set the live stream" do
+        it "live stream should return the stream used for live stdout and live stderr" do
           shell_cmd.live_stream.should eql(stream)
         end
 
+        it "should set the live stdout stream" do
+          shell_cmd.live_stderr.should eql(stream)
+        end
+
         it "should set the live stderr stream" do
-          shell_cmd.live_stderr_stream.should eql(stream)
+          shell_cmd.live_stderr.should eql(stream)
         end
       end
 
-      context 'when setting a live stream and live stderr stream separately' do
+      context 'when setting the live stdout and live stderr streams separately' do
         let(:accessor) { :live_stream }
         let(:stream) { StringIO.new }
         let(:value) { stream }
@@ -208,35 +212,43 @@ describe Mixlib::ShellOut do
         let(:stderr_stream) { StringIO.new }
 
         before(:each) do
-          shell_cmd.live_stream = stdout_stream
-          shell_cmd.live_stderr_stream = stderr_stream
+          shell_cmd.live_stdout = stdout_stream
+          shell_cmd.live_stderr = stderr_stream
         end
 
-        it "should set the live stream" do
-          shell_cmd.live_stream.should eql(stdout_stream)
+        it "live_stream should return nil" do
+          shell_cmd.live_stream.should be_nil
         end
 
-        it "should set the live stream" do
-          shell_cmd.live_stderr_stream.should eql(stderr_stream)
+        it "should set the live stdout" do
+          shell_cmd.live_stdout.should eql(stdout_stream)
+        end
+
+        it "should set the live stderr" do
+          shell_cmd.live_stderr.should eql(stderr_stream)
         end
       end
 
-      context 'when setting a live stream and explicitly disabling live stderr stream' do
+      context 'when setting a live stream and then overriding the live stderr' do
         let(:accessor) { :live_stream }
         let(:value) { stream }
         let(:stream) { StringIO.new }
 
         before(:each) do
-          shell_cmd.live_stream = stream
-          shell_cmd.live_stderr_stream = nil
+          shell_cmd.live_stdout = stream
+          shell_cmd.live_stderr = nil
         end
 
-        it "should set the live stream" do
-          shell_cmd.live_stream.should eql(stream)
+        it "should return nil" do
+          should be_nil
         end
 
-        it "should set the live stderr stream" do
-          shell_cmd.live_stderr_stream.should eql(nil)
+        it "should set the live stdout" do
+          shell_cmd.live_stdout.should eql(stream)
+        end
+
+        it "should set the live stderr" do
+          shell_cmd.live_stderr.should eql(nil)
         end
       end
 
@@ -505,32 +517,32 @@ describe Mixlib::ShellOut do
         stream.string.should include("hello#{LINE_ENDING}")
       end
 
-      context "with default stderr stream" do
+      context "with default live stderr" do
         it "should copy the child's stderr to the live stream" do
           shell_cmd.run_command
           stream.string.should include("world#{LINE_ENDING}")
         end
       end
 
-      context "without an stderr stream" do
+      context "without live stderr" do
         it "should not copy the child's stderr to the live stream" do
-          shell_cmd.live_stderr_stream = nil
+          shell_cmd.live_stderr = nil
           shell_cmd.run_command
           stream.string.should_not include("world#{LINE_ENDING}")
         end
       end
 
-      context "with a separate stderr stream" do
+      context "with a separate live stderr" do
         let(:stderr_stream) { StringIO.new }
 
         it "should not copy the child's stderr to the live stream" do
-          shell_cmd.live_stderr_stream = stderr_stream
+          shell_cmd.live_stderr = stderr_stream
           shell_cmd.run_command
           stream.string.should_not include("world#{LINE_ENDING}")
         end
 
         it "should copy the child's stderr to the live stderr stream" do
-          shell_cmd.live_stderr_stream = stderr_stream
+          shell_cmd.live_stderr = stderr_stream
           shell_cmd.run_command
           stderr_stream.string.should include("world#{LINE_ENDING}")
         end