Add tests and revise logging

2 files changed, 131 insertions, 19 deletions

diff --git a/lib/ohai/mixin/command.rb b/lib/ohai/mixin/command.rb
index a27dff21..6823ccf2 100644
--- a/lib/ohai/mixin/command.rb
+++ b/lib/ohai/mixin/command.rb
@@ -28,27 +28,21 @@ require "mixlib/shellout"
 module Ohai
   module Mixin
     module Command
+      # DISCLAIMER: Logging only works in the context of a plugin!!
       # accept a command and any of the mixlib-shellout options
       def shell_out(cmd, **options)
         # unless specified by the caller timeout after 30 seconds
-        options[:timeout] = 30 unless options[:timeout]
-        m = Mixlib::ShellOut.new(cmd, options)
+        options[:timeout] ||= 30
+        so = Mixlib::ShellOut.new(cmd, options)
         begin
-          m.run_command
-
-          # we should really fail here on non-0, but historically we haven't so we can't now
-          if m.exitstatus == 0
-            Ohai::Log.debug("Plugin #{self.name} successfully ran command #{cmd}")
-          else
-            Ohai::Log.debug("Plugin #{self.name} command #{cmd} returned status #{m.exitstatus}")
-          end
-
-          m
+          so.run_command
+          Ohai::Log.debug("Plugin #{self.name} ran '#{cmd}' and returned #{so.exitstatus}")
+          so
         rescue Errno::ENOENT => e
-          Ohai::Log.debug("Plugin #{self.name} failed to run command #{cmd}")
+          Ohai::Log.debug("Plugin #{self.name} ran '#{cmd}' and failed #{e.inspect}")
           raise Ohai::Exceptions::Exec, e
         rescue Mixlib::ShellOut::CommandTimeout => e
-          Ohai::Log.debug("Plugin #{self.name} timed out after #{options[:timeout]}s running command #{cmd}")
+          Ohai::Log.debug("Plugin #{self.name} ran '#{cmd}' and timed out after #{options[:timeout]} seconds")
           raise Ohai::Exceptions::Exec, e
         end
       end
diff --git a/spec/unit/mixin/command_spec.rb b/spec/unit/mixin/command_spec.rb
index 97180d06..7a326820 100644
--- a/spec/unit/mixin/command_spec.rb
+++ b/spec/unit/mixin/command_spec.rb
@@ -30,7 +30,7 @@ describe Ohai::Mixin::Command, "popen4" do
   end
 
   it "should respect locale when specified explicitly" do
-    Ohai::Mixin::Command.popen4("echo $LC_ALL", :environment => { "LC_ALL" => "es" }) do |pid, stdin, stdout, stderr|
+    Ohai::Mixin::Command.popen4("echo $LC_ALL", :environment => {  "LC_ALL" => "es"  }) do |pid, stdin, stdout, stderr|
       stdin.close
       expect(stdout.read.strip).to eq("es")
     end
@@ -52,14 +52,14 @@ describe Ohai::Mixin::Command, "popen4" do
 
       it "should force encode the string to UTF-8" do
         extend Ohai::Mixin::Command
-        snowy = run_command(:command => ("echo '" + ("☃" * 8096) + "'"))[1]
+        snowy = run_command(:command => ("echo "" + ("☃" * 8096) + """))[1]
         expect(snowy.encoding).to eq(Encoding::UTF_8)
       end
     end
 
     it "should force encode the string to UTF-8" do
       extend Ohai::Mixin::Command
-      snowy = run_command(:command => ("echo '" + ("☃" * 8096) + "'"))[1]
+      snowy = run_command(:command => ("echo "" + ("☃" * 8096) + """))[1]
       expect(snowy.encoding).to eq(Encoding::UTF_8)
     end
   end
@@ -72,7 +72,7 @@ describe Ohai::Mixin::Command, "popen4" do
     created_procs = 0
     100.times do
       begin
-        Ohai::Mixin::Command.popen4("/bin/this-is-not-a-real-command") { |p, i, o, e| nil }
+        Ohai::Mixin::Command.popen4("/bin/this-is-not-a-real-command") {  |p, i, o, e| nil  }
       rescue Ohai::Exceptions::Exec
         created_procs += 1
       end
@@ -80,10 +80,128 @@ describe Ohai::Mixin::Command, "popen4" do
     expect(created_procs).to eq(100)
     reaped_procs = 0
     begin
-      loop { Process.wait(-1); reaped_procs += 1 }
+      loop {  Process.wait(-1); reaped_procs += 1  }
     rescue Errno::ECHILD
     end
     expect(reaped_procs).to eq(0)
   end
+end
+
+describe Ohai::Mixin::Command, "shell_out" do
+  let(:cmd) {  'sparkle-dream --version'  }
+
+  let(:shell_out) {  double("Mixlib::ShellOut")  }
+
+  let(:plugin_name) {  :OSSparkleDream  }
+
+  before(:each) do
+    allow(Ohai::Mixin::Command).to receive(:name).and_return(plugin_name)
+  end
+
+  describe "when the command runs" do
+    it "logs the command and exitstatus" do
+      expect(Mixlib::ShellOut).
+        to receive(:new).
+        with(cmd, { timeout: 30 }).
+        and_return(shell_out)
+
+      expect(shell_out).
+        to receive(:run_command)
+
+      expect(shell_out).
+        to receive(:exitstatus).
+        and_return(256)
+
+      expect(Ohai::Log).to receive(:debug).
+        with("Plugin OSSparkleDream ran 'sparkle-dream --version' and returned 256")
+
+      Ohai::Mixin::Command.shell_out(cmd)
+    end
+  end
+
+  describe "when the command does not exist" do
+    it "logs the command and error message" do
+      expect(Mixlib::ShellOut).
+        to receive(:new).
+        with(cmd, { timeout: 30 }).
+        and_return(shell_out)
+
+      expect(shell_out).
+        to receive(:run_command).
+        and_raise(Errno::ENOENT, "sparkle-dream")
+
+      expect(Ohai::Log).
+        to receive(:debug).
+        with("Plugin OSSparkleDream ran 'sparkle-dream --version' and failed " \
+             "#<Errno::ENOENT: No such file or directory - sparkle-dream>")
+
+      expect{ Ohai::Mixin::Command.shell_out(cmd) }.
+        to raise_error(Ohai::Exceptions::Exec)
+    end
+  end
 
+  describe "when the command times out" do
+    it "logs the command an timeout error message" do
+      expect(Mixlib::ShellOut).
+        to receive(:new).
+        with(cmd, { timeout: 30 }).
+        and_return(shell_out)
+
+      expect(shell_out).
+        to receive(:run_command).
+        and_raise(Mixlib::ShellOut::CommandTimeout)
+
+      expect(Ohai::Log).
+        to receive(:debug).
+        with("Plugin OSSparkleDream ran 'sparkle-dream --version' and timed " \
+             "out after 30 seconds")
+
+      expect{ Ohai::Mixin::Command.shell_out(cmd) }.
+        to raise_error(Ohai::Exceptions::Exec)
+    end
+  end
+
+  describe "when a timeout option is provided" do
+    let(:options) {  {timeout: 10}  }
+
+    it "runs the command with the provided timeout" do
+      expect(Mixlib::ShellOut).
+        to receive(:new).
+        with(cmd, options).
+        and_return(shell_out)
+
+      expect(shell_out).
+        to receive(:run_command)
+
+      expect(shell_out).
+        to receive(:exitstatus).
+        and_return(256)
+
+      expect(Ohai::Log).to receive(:debug).
+        with("Plugin OSSparkleDream ran 'sparkle-dream --version' and returned 256")
+
+      Ohai::Mixin::Command.shell_out(cmd, options)
+    end
+
+    describe "when the command times out" do
+      it "logs the command an timeout error message" do
+        expect(Mixlib::ShellOut).
+          to receive(:new).
+          with(cmd, options).
+          and_return(shell_out)
+
+        expect(shell_out).
+          to receive(:run_command).
+          and_raise(Mixlib::ShellOut::CommandTimeout)
+
+        expect(Ohai::Log).
+          to receive(:debug).
+          with("Plugin OSSparkleDream ran 'sparkle-dream --version' and timed " \
+               "out after 10 seconds")
+
+        expect{ Ohai::Mixin::Command.shell_out(cmd, options) }.
+          to raise_error(Ohai::Exceptions::Exec)
+      end
+    end
+  end
 end