From 39ad6d7683a62ae45f814f1f5b68cd536ed35d50 Mon Sep 17 00:00:00 2001
From: Jason Frey <fryguy9@gmail.com>
Date: Sat, 4 Jan 2014 17:47:24 -0500
Subject: [PATCH] Extract #run into awesome_spawn gem.

---
 lib/linux_admin.rb                            |   1 -
 lib/linux_admin/command_result.rb             |  14 --
 lib/linux_admin/common.rb                     | 100 +-----------
 lib/linux_admin/exceptions.rb                 |  21 +--
 .../subscription_manager.rb                   |   4 +-
 linux_admin.gemspec                           |   1 +
 spec/command_result_spec.rb                   |  13 --
 spec/common_spec.rb                           | 151 +-----------------
 spec/deb_spec.rb                              |   4 +-
 spec/disk_spec.rb                             |   3 +-
 spec/mountable_spec.rb                        |  16 +-
 spec/rpm_spec.rb                              |  10 +-
 spec/subscription_manager_spec.rb             |   8 +-
 13 files changed, 37 insertions(+), 309 deletions(-)
 delete mode 100644 lib/linux_admin/command_result.rb
 delete mode 100644 spec/command_result_spec.rb

diff --git a/lib/linux_admin.rb b/lib/linux_admin.rb
index 049271d..764e3a5 100644
--- a/lib/linux_admin.rb
+++ b/lib/linux_admin.rb
@@ -5,7 +5,6 @@
 
 require 'linux_admin/common'
 require 'linux_admin/exceptions'
-require 'linux_admin/command_result'
 require 'linux_admin/package'
 require 'linux_admin/rpm'
 require 'linux_admin/deb'
diff --git a/lib/linux_admin/command_result.rb b/lib/linux_admin/command_result.rb
deleted file mode 100644
index bc0bee7..0000000
--- a/lib/linux_admin/command_result.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-class CommandResult
-  attr_reader :command_line, :output, :error, :exit_status
-
-  def initialize(command_line, output, error, exit_status)
-    @command_line = command_line
-    @output       = output
-    @error        = error
-    @exit_status  = exit_status
-  end
-
-  def inspect
-    "#{to_s.chop} @exit_status=#{@exit_status}>"
-  end
-end
diff --git a/lib/linux_admin/common.rb b/lib/linux_admin/common.rb
index 1651115..fc4905e 100644
--- a/lib/linux_admin/common.rb
+++ b/lib/linux_admin/common.rb
@@ -1,4 +1,4 @@
-require 'shellwords'
+require 'awesome_spawn'
 
 class LinuxAdmin
   module Common
@@ -7,105 +7,11 @@ def cmd(cmd)
     end
 
     def run(cmd, options = {})
-      params = options[:params] || options[:parameters]
-
-      launch_params = {}
-      launch_params[:chdir] = options[:chdir] if options[:chdir]
-
-      output        = ""
-      error         = ""
-      status        = nil
-      command_line  = build_cmd(cmd, params)
-
-      begin
-        output, error = launch(command_line, launch_params)
-        status = exitstatus
-      ensure
-        output ||= ""
-        error  ||= ""
-        self.exitstatus = nil
-      end
-    rescue Errno::ENOENT => err
-      raise NoSuchFileError.new(err.message) if NoSuchFileError.detected?(err.message)
-      raise
-    else
-      CommandResult.new(command_line, output, error, status)
+      AwesomeSpawn.run(cmd, options)
     end
 
     def run!(cmd, options = {})
-      params = options[:params] || options[:parameters]
-      command_result = run(cmd, options)
-
-      if command_result.exit_status != 0
-        message = "#{cmd} exit code: #{command_result.exit_status}"
-        raise CommandResultError.new(message, command_result)
-      end
-
-      command_result
-    end
-
-    private
-
-    def sanitize(params)
-      return [] if params.blank?
-      params.collect do |k, v|
-        v = case v
-            when Array;    v.collect {|i| i.to_s.shellescape}
-            when NilClass; v
-            else           v.to_s.shellescape
-            end
-        [k, v]
-      end
-    end
-
-    def assemble_params(sanitized_params)
-      sanitized_params.collect do |pair|
-        pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
-        pair.flatten.compact.join(pair_joiner)
-      end.join(" ")
-    end
-
-    def build_cmd(cmd, params = nil)
-      return cmd.to_s if params.blank?
-      "#{cmd} #{assemble_params(sanitize(params))}"
-    end
-
-    # IO pipes have a maximum size of 64k before blocking,
-    # so we need to read and write synchronously.
-    # http://stackoverflow.com/questions/13829830/ruby-process-spawn-stdout-pipe-buffer-size-limit/13846146#13846146
-    THREAD_SYNC_KEY = "LinuxAdmin-exitstatus"
-
-    def launch(cmd, spawn_options = {})
-      out_r, out_w = IO.pipe
-      err_r, err_w = IO.pipe
-      pid = Kernel.spawn(cmd, {:err => err_w, :out => out_w}.merge(spawn_options))
-      wait_for_process(pid, out_w, err_w)
-      wait_for_pipes(out_r, err_r)
-    end
-
-    def wait_for_process(pid, out_w, err_w)
-      self.exitstatus = :not_done
-      Thread.new(Thread.current) do |parent_thread|
-        _, status = Process.wait2(pid)
-        out_w.close
-        err_w.close
-        parent_thread[THREAD_SYNC_KEY] = status.exitstatus
-      end
-    end
-
-    def wait_for_pipes(out_r, err_r)
-      out = out_r.read
-      err = err_r.read
-      sleep(0.1) while exitstatus == :not_done
-      return out, err
-    end
-
-    def exitstatus
-      Thread.current[THREAD_SYNC_KEY]
-    end
-
-    def exitstatus=(value)
-      Thread.current[THREAD_SYNC_KEY] = value
+      AwesomeSpawn.run!(cmd, options)
     end
   end
 end
diff --git a/lib/linux_admin/exceptions.rb b/lib/linux_admin/exceptions.rb
index df46e7a..a620c98 100644
--- a/lib/linux_admin/exceptions.rb
+++ b/lib/linux_admin/exceptions.rb
@@ -1,24 +1,5 @@
-class CommandResultError < StandardError
-  attr_reader :result
-
-  def initialize(message, result)
-    super(message)
-    @result = result
-  end
-end
-
 class LinuxAdmin
-  class NoSuchFileError < Errno::ENOENT
-    def initialize(message)
-      super(message.split("No such file or directory -").last.split(" ").first)
-    end
-
-    def self.detected?(message)
-      message.start_with?("No such file or directory -")
-    end
-  end
-
-  class CredentialError < CommandResultError
+  class CredentialError < AwesomeSpawn::CommandResultError
     def initialize(result)
       super("Invalid username or password", result)
     end
diff --git a/lib/linux_admin/registration_system/subscription_manager.rb b/lib/linux_admin/registration_system/subscription_manager.rb
index 5754991..26b4705 100644
--- a/lib/linux_admin/registration_system/subscription_manager.rb
+++ b/lib/linux_admin/registration_system/subscription_manager.rb
@@ -4,7 +4,7 @@ class LinuxAdmin
   class SubscriptionManager < RegistrationSystem
     def run!(cmd, options = {})
       super(cmd, options)
-    rescue CommandResultError => err
+    rescue AwesomeSpawn::CommandResultError => err
       raise CredentialError.new(err.result) if err.result.error.downcase.include?("invalid username or password")
       raise
     end
@@ -140,4 +140,4 @@ def proxy_params(options)
       config
     end
   end
-end
\ No newline at end of file
+end
diff --git a/linux_admin.gemspec b/linux_admin.gemspec
index 097a046..6f95457 100644
--- a/linux_admin.gemspec
+++ b/linux_admin.gemspec
@@ -34,5 +34,6 @@ registration, updates, etc.
   spec.add_dependency "activesupport",        "> 3.2"
   spec.add_dependency "inifile",              "~> 2.0.2"
   spec.add_dependency "more_core_extensions", "~> 1.1.2"
+  spec.add_dependency "awesome_spawn",        "~> 1.1.0"
   spec.add_dependency "nokogiri"
 end
diff --git a/spec/command_result_spec.rb b/spec/command_result_spec.rb
deleted file mode 100644
index 6a68b0d..0000000
--- a/spec/command_result_spec.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-require 'spec_helper'
-
-describe CommandResult do
-  context "#inspect" do
-    it "will not display sensitive information" do
-      command_result = described_class.new("aaa", "bbb", "ccc", 0).inspect
-
-      expect(command_result.include?("aaa")).to be_false
-      expect(command_result.include?("bbb")).to be_false
-      expect(command_result.include?("ccc")).to be_false
-    end
-  end
-end
diff --git a/spec/common_spec.rb b/spec/common_spec.rb
index ed2f152..fcfc1ad 100644
--- a/spec/common_spec.rb
+++ b/spec/common_spec.rb
@@ -11,23 +11,9 @@ class TestClass
     Object.send(:remove_const, :TestClass)
   end
 
-  let(:params) do
-    {
-      "--user"  => "bob",
-      "--pass"  => "P@$sw0^& |<>/-+*d%",
-      "--db"    => nil,
-      "--desc=" => "Some Description",
-      nil       => ["pkg1", "some pkg"]
-    }
-  end
-
-  let (:modified_params) do
-    params.to_a + [123, 456].collect {|pool| ["--pool", pool]}
-  end
-
   subject { TestClass }
 
-  context ".cmd" do
+  context "#cmd" do
     it "looks up local command from id" do
       d = double(LinuxAdmin::Distro)
       d.class::COMMANDS = {:sh => '/bin/sh'}
@@ -36,136 +22,13 @@ class TestClass
     end
   end
 
-  shared_examples_for "run" do
-    context "paramater and command handling" do
-      before do
-        subject.stub(:exitstatus => 0)
-      end
-
-      it "sanitizes crazy params" do
-        subject.should_receive(:launch).once.with("true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description pkg1 some\\ pkg --pool 123 --pool 456", {})
-        subject.send(run_method, "true", :params => modified_params)
-      end
-
-      it "sanitizes fixnum array params" do
-        subject.should_receive(:launch).once.with("true 1", {})
-        subject.send(run_method, "true", :params => {nil => [1]})
-      end
-
-      it "sanitizes Pathname option value" do
-        require 'pathname'
-        subject.should_receive(:launch).once.with("true /usr/bin/ruby", {})
-        subject.send(run_method, "true", :params => {nil => [Pathname.new("/usr/bin/ruby")]})
-      end
-
-      it "sanitizes Pathname option" do
-        require 'pathname'
-        subject.should_receive(:launch).once.with("true /usr/bin/ruby", {})
-        subject.send(run_method, "true", :params => {Pathname.new("/usr/bin/ruby") => nil})
-      end
-
-      it "as empty hash" do
-        subject.should_receive(:launch).once.with("true", {})
-        subject.send(run_method, "true", :params => {})
-      end
-
-      it "as nil" do
-        subject.should_receive(:launch).once.with("true", {})
-        subject.send(run_method, "true", :params => nil)
-      end
-
-      it "won't modify caller params" do
-        orig_params = params.dup
-        subject.stub(:launch)
-        subject.send(run_method, "true", :params => params)
-        expect(orig_params).to eq(params)
-      end
-
-      it "Pathname command" do
-        subject.should_receive(:launch).once.with("/usr/bin/ruby", {})
-        subject.send(run_method, Pathname.new("/usr/bin/ruby"), {})
-      end
-
-      it "Pathname command with params" do
-        subject.should_receive(:launch).once.with("/usr/bin/ruby -v", {})
-        subject.send(run_method, Pathname.new("/usr/bin/ruby"), :params => {"-v" => nil})
-      end
-
-      it "supports spawn's chdir option" do
-        subject.should_receive(:launch).once.with("true", {:chdir => ".."})
-        subject.send(run_method, "true", :chdir => "..")
-      end
-    end
-
-    context "with real execution" do
-      before do
-        Kernel.stub(:spawn).and_call_original
-      end
-
-      it "command ok exit ok" do
-        expect(subject.send(run_method, "true")).to be_kind_of CommandResult
-      end
-
-      it "command ok exit bad" do
-        if run_method == "run!"
-          error = nil
-
-          # raise_error with do/end block notation is broken in rspec-expectations 2.14.x
-          # and has been fixed in master but not yet released.
-          # See: https://github.com/rspec/rspec-expectations/commit/b0df827f4c12870aa4df2f20a817a8b01721a6af
-          expect {subject.send(run_method, "false")}.to raise_error {|e| error = e }
-          expect(error).to be_kind_of CommandResultError
-          expect(error.result).to be_kind_of CommandResult
-        else
-          expect {subject.send(run_method, "false")}.to_not raise_error
-        end
-      end
-
-      it "command bad" do
-        expect {subject.send(run_method, "XXXXX --user=bob")}.to raise_error(LinuxAdmin::NoSuchFileError, "No such file or directory - XXXXX")
-      end
-
-      context "#exit_status" do
-        it "command ok exit ok" do
-          expect(subject.send(run_method, "true").exit_status).to eq(0)
-        end
-
-        it "command ok exit bad" do
-          expect(subject.send(run_method, "false").exit_status).to eq(1) if run_method == "run"
-        end
-      end
-
-      context "#output" do
-        it "command ok exit ok" do
-          expect(subject.send(run_method, "echo \"Hello World\"").output).to eq("Hello World\n")
-        end
-
-        it "command ok exit bad" do
-          expect(subject.send(run_method, "echo 'bad' && false").output).to eq("bad\n") if run_method == "run"
-        end
-      end
-
-      context "#error" do
-        it "command ok exit ok" do
-          expect(subject.send(run_method, "echo \"Hello World\" >&2").error).to eq("Hello World\n")
-        end
-
-        it "command ok exit bad" do
-          expect(subject.send(run_method, "echo 'bad' >&2 && false").error).to eq("bad\n") if run_method == "run"
-        end
-      end
-    end
+  it "#run" do
+    AwesomeSpawn.should_receive(:run).with("echo", nil => "test")
+    subject.run("echo", nil => "test")
   end
 
-  context ".run" do
-    include_examples "run" do
-      let(:run_method) {"run"}
-    end
-  end
-
-  context ".run!" do
-    include_examples "run" do
-      let(:run_method) {"run!"}
-    end
+  it "#run!" do
+    AwesomeSpawn.should_receive(:run!).with("echo", nil => "test")
+    subject.run!("echo", nil => "test")
   end
 end
diff --git a/spec/deb_spec.rb b/spec/deb_spec.rb
index 6bae531..b90bc5a 100644
--- a/spec/deb_spec.rb
+++ b/spec/deb_spec.rb
@@ -39,9 +39,9 @@
 Supported: 9m
 Task: kubuntu-desktop, kubuntu-full, kubuntu-active, kubuntu-active-desktop, kubuntu-active-full, kubuntu-active, edubuntu-desktop-gnome, ubuntustudio-font-meta
 EOS
-      described_class.should_receive(:run).
+      described_class.should_receive(:run!).
                       with(described_class::APT_CACHE_CMD, :params => ["show", "ruby"]).
-                      and_return(CommandResult.new("", data, "", 0))
+                      and_return(double(:output => data))
       metadata = described_class.info("ruby")
       metadata['package'].should == 'ruby'
       metadata['priority'].should == 'optional'
diff --git a/spec/disk_spec.rb b/spec/disk_spec.rb
index 4908948..c9826b1 100644
--- a/spec/disk_spec.rb
+++ b/spec/disk_spec.rb
@@ -54,8 +54,7 @@
 
     it "returns [] on non-zero parted rc" do
       disk = LinuxAdmin::Disk.new :path => '/dev/hda'
-      disk.stub(:exitstatus => 1)
-      disk.stub(:launch)
+      disk.should_receive(:run).and_return(double(:output => "", :exit_status => 1))
       disk.partitions.should == []
     end
 
diff --git a/spec/mountable_spec.rb b/spec/mountable_spec.rb
index 1ce03a3..9619132 100644
--- a/spec/mountable_spec.rb
+++ b/spec/mountable_spec.rb
@@ -27,20 +27,20 @@ def path
 
   describe "#mount_point_exists?" do
     it "uses mount" do
-      TestMountable.should_receive(:run!).with(TestMountable.cmd(:mount)).and_return(CommandResult.new("", "", "", 0))
+      TestMountable.should_receive(:run!).with(TestMountable.cmd(:mount)).and_return(double(:output => ""))
       TestMountable.mount_point_exists?('/mnt/usb')
     end
 
     context "disk mounted at specified location" do
       it "returns true" do
-        TestMountable.should_receive(:run!).and_return(CommandResult.new("", @mount_out1, "", 0))
+        TestMountable.should_receive(:run!).and_return(double(:output => @mount_out1))
         TestMountable.mount_point_exists?('/mnt/usb').should be_true
       end
     end
 
     context "no disk mounted at specified location" do
       it "returns false" do
-        TestMountable.should_receive(:run!).and_return(CommandResult.new("", @mount_out2, "", 0))
+        TestMountable.should_receive(:run!).and_return(double(:output => @mount_out2))
         TestMountable.mount_point_exists?('/mnt/usb').should be_false
       end
     end
@@ -48,20 +48,20 @@ def path
 
   describe "#mount_point_available?" do
     it "uses mount" do
-      TestMountable.should_receive(:run!).with(TestMountable.cmd(:mount)).and_return(CommandResult.new("", "", "", 0))
+      TestMountable.should_receive(:run!).with(TestMountable.cmd(:mount)).and_return(double(:output => ""))
       TestMountable.mount_point_available?('/mnt/usb')
     end
 
     context "disk mounted at specified location" do
       it "returns false" do
-        TestMountable.should_receive(:run!).and_return(CommandResult.new("", @mount_out1, "", 0))
+        TestMountable.should_receive(:run!).and_return(double(:output => @mount_out1))
         TestMountable.mount_point_available?('/mnt/usb').should be_false
       end
     end
 
     context "no disk mounted at specified location" do
       it "returns true" do
-        TestMountable.should_receive(:run!).and_return(CommandResult.new("", @mount_out2, "", 0))
+        TestMountable.should_receive(:run!).and_return(double(:output => @mount_out2))
         TestMountable.mount_point_available?('/mnt/usb').should be_true
       end
     end
@@ -85,8 +85,8 @@ def path
   describe "#mount" do
     it "sets mount point" do
       # ignore actual mount cmds
-      @mountable.should_receive(:run!).and_return(CommandResult.new("", "", "", ""))
-      TestMountable.should_receive(:run!).and_return(CommandResult.new("", "", "", ""))
+      @mountable.should_receive(:run!).and_return(double(:output => ""))
+      TestMountable.should_receive(:run!).and_return(double(:output => ""))
 
       @mountable.mount('/mnt/sda2').should == '/mnt/sda2'
       @mountable.mount_point.should == '/mnt/sda2'
diff --git a/spec/rpm_spec.rb b/spec/rpm_spec.rb
index cb008f4..10485e1 100644
--- a/spec/rpm_spec.rb
+++ b/spec/rpm_spec.rb
@@ -28,8 +28,8 @@
   end
 
   it ".import_key" do
-    described_class.should_receive(:run).with("rpm", {:params=>{"--import"=>"abc"}}).and_return(CommandResult.new("", "", "", 0))
-    expect(described_class.import_key("abc")).to be_true
+    described_class.should_receive(:run!).with("rpm", {:params => {"--import" => "abc"}})
+    expect { described_class.import_key("abc") }.to_not raise_error
   end
 
   describe "#info" do
@@ -59,9 +59,9 @@
 files and to do system management tasks (as in Perl).  It is simple,
 straight-forward, and extensible.
 EOS
-      described_class.should_receive(:run).
+      described_class.should_receive(:run!).
                       with(described_class::RPM_CMD, :params => {"-qi" => "ruby"}).
-                      and_return(CommandResult.new("", data, "", 0))
+                      and_return(double(:output => data))
       metadata = described_class.info("ruby")
       metadata['name'].should == 'ruby'
       metadata['version'].should == '2.0.0.247'
@@ -79,7 +79,7 @@
   end
 
   it ".upgrade" do
-    described_class.should_receive(:run).with("rpm -U", {:params=>{nil=>"abc"}}).and_return(CommandResult.new("", "", "", 0))
+    described_class.should_receive(:run).with("rpm -U", {:params=>{nil=>"abc"}}).and_return(double(:exit_status => 0))
     expect(described_class.upgrade("abc")).to be_true
   end
 end
diff --git a/spec/subscription_manager_spec.rb b/spec/subscription_manager_spec.rb
index 6868aa0..16cc325 100644
--- a/spec/subscription_manager_spec.rb
+++ b/spec/subscription_manager_spec.rb
@@ -140,7 +140,13 @@
 
     it "with invalid credentials" do
       run_options = ["subscription-manager orgs", {:params=>{"--username="=>"BadUser", "--password="=>"BadPass"}}]
-      described_class.any_instance.should_receive(:run).once.with(*run_options).and_return(CommandResult.new("", "", "Invalid username or password. To create a login, please visit https://www.redhat.com/wapps/ugc/register.html", 255))
+      error = AwesomeSpawn::CommandResultError.new("",
+        double(
+          :error       => "Invalid username or password. To create a login, please visit https://www.redhat.com/wapps/ugc/register.html",
+          :exit_status => 255
+        )
+      )
+      AwesomeSpawn.should_receive(:run!).once.with(*run_options).and_raise(error)
       expect { described_class.new.organizations({:username=>"BadUser", :password=>"BadPass"}) }.to raise_error(LinuxAdmin::CredentialError)
     end
   end