Unit tests for audit-mode in chef-solo.

* Audits are disabled by default.
* Also, updated spec file to use RSpec :let.

7 files changed, 85 insertions, 52 deletions

diff --git a/lib/chef/audit/audit_reporter.rb b/lib/chef/audit/audit_reporter.rb
index 407c2deeb0..596b06b285 100644
--- a/lib/chef/audit/audit_reporter.rb
+++ b/lib/chef/audit/audit_reporter.rb
@@ -117,8 +117,6 @@ class Chef
         run_data = audit_data.to_hash
 
         if error
-          # TODO: Rather than a single string we might want to format the exception here similar to
-          # lib/chef/resource_reporter.rb#83
           run_data[:error] = "#{error.class.to_s}: #{error.message}\n#{error.backtrace.join("\n")}"
         end
 
diff --git a/lib/chef/audit/runner.rb b/lib/chef/audit/runner.rb
index 1450ef7f61..4017593c55 100644
--- a/lib/chef/audit/runner.rb
+++ b/lib/chef/audit/runner.rb
@@ -108,8 +108,6 @@ class Chef
       # the output stream to be changed for a formatter once the formatter has
       # been added.
       def set_streams
-        # TODO: Do some testing to ensure these will output/output properly to
-        # a file.
         RSpec.configuration.output_stream = Chef::Config[:log_location]
         RSpec.configuration.error_stream = Chef::Config[:log_location]
       end
@@ -135,12 +133,9 @@ class Chef
         end
       end
 
-      # Set up the backend for Specinfra/Serverspec.
+      # Set up the backend for Specinfra/Serverspec.  :exec is the local system.
       def configure_specinfra
-        # TODO: We may need to be clever and adjust this based on operating
-        # system, or make it configurable. E.g., there is a PowerShell backend,
-        # as well as an SSH backend.
-        Specinfra.configuration.backend = :exec if Specinfra.configuration.backend != :exec
+        Specinfra.configuration.backend = :exec
       end
 
       # Iterates through the controls registered to this run_context, builds an
diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 634773cf80..77f63671d7 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -338,7 +338,8 @@ class Chef
       converge_exception
     end
 
-    # TODO don't want to change old API
+    # We don't want to change the old API on the `converge` method to have it perform
+    # saving.  So we wrap it in this method.
     def converge_and_save(run_context)
       converge_exception = converge(run_context)
       unless converge_exception
diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb
index 398c61fdc8..489888db8f 100644
--- a/lib/chef/formatters/doc.rb
+++ b/lib/chef/formatters/doc.rb
@@ -166,7 +166,7 @@ class Chef
       end
 
       def converge_failed(e)
-        # TODO do we want to do anything else in here?
+        # Currently a failed converge is handled the same way as a successful converge
         converge_complete
       end
 
@@ -183,7 +183,6 @@ class Chef
         puts_line ""
         puts_line "Audit phase exception:"
         indent
-        # TODO error_mapper ?
         puts_line "#{error.message}"
         error.backtrace.each do |l|
           puts_line l
diff --git a/spec/functional/audit/runner_spec.rb b/spec/functional/audit/runner_spec.rb
index 89c62ae2e8..aa35548f2f 100644
--- a/spec/functional/audit/runner_spec.rb
+++ b/spec/functional/audit/runner_spec.rb
@@ -20,6 +20,7 @@ require 'spec_helper'
 require 'spec/support/audit_helper'
 require 'chef/audit/runner'
 require 'rspec/support/spec/in_sub_process'
+require 'tempfile'
 
 ##
 # This functional test ensures that our runner can be setup to not interfere with existing RSpec
@@ -60,7 +61,7 @@ describe Chef::Audit::Runner do
       end
     end
 
-    context "there is a single successful control" do
+    shared_context "passing audit" do
       let(:audits) do
         should_pass = lambda do
           it "should pass" do
@@ -69,17 +70,9 @@ describe Chef::Audit::Runner do
         end
         { controls_name => Struct.new(:args, :block).new([controls_name], should_pass)}
       end
-
-      it "correctly runs" do
-        in_sub_process do
-          runner.run
-
-          expect(stdout.string).to match(/1 example, 0 failures/)
-        end
-      end
     end
 
-    context "there is a single failing control" do
+    shared_context "failing audit" do
       let(:audits) do
         should_fail = lambda do
           it "should fail" do
@@ -88,7 +81,21 @@ describe Chef::Audit::Runner do
         end
         { controls_name => Struct.new(:args, :block).new([controls_name], should_fail)}
       end
+    end
+
+    context "there is a single successful control" do
+      include_context "passing audit"
+      it "correctly runs" do
+        in_sub_process do
+          runner.run
+
+          expect(stdout.string).to match(/1 example, 0 failures/)
+        end
+      end
+    end
 
+    context "there is a single failing control" do
+      include_context "failing audit"
       it "correctly runs" do
         in_sub_process do
           runner.run
@@ -100,6 +107,30 @@ describe Chef::Audit::Runner do
       end
     end
 
+    describe "log location is a file" do
+      let(:tmpfile) { Tempfile.new("audit") }
+      before do
+        Chef::Config[:log_location] = tmpfile.path
+      end
+
+      after do
+        tmpfile.close
+        tmpfile.unlink
+      end
+
+      include_context "failing audit"
+      it "correctly runs" do
+        in_sub_process do
+          runner.run
+
+          contents = tmpfile.read
+          expect(contents).to match(/Failure\/Error: expect\(2 - 1\)\.to eq\(0\)/)
+          expect(contents).to match(/1 example, 1 failure/)
+          expect(contents).to match(/# controls_name should fail/)
+        end
+      end
+    end
+
   end
 
 end
diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb
index 3554b78c13..33af9bc5c1 100644
--- a/spec/unit/application/client_spec.rb
+++ b/spec/unit/application/client_spec.rb
@@ -74,7 +74,7 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
 
       it "should not terminate" do
         expect(Chef::Application).not_to receive(:fatal!)
-        @app.reconfigure
+        app.reconfigure
       end
     end
 
diff --git a/spec/unit/application/solo_spec.rb b/spec/unit/application/solo_spec.rb
index 80f0bead8b..2a07ff38ad 100644
--- a/spec/unit/application/solo_spec.rb
+++ b/spec/unit/application/solo_spec.rb
@@ -18,13 +18,16 @@
 require 'spec_helper'
 
 describe Chef::Application::Solo do
+
+  let(:app) { Chef::Application::Solo.new }
+
   before do
     allow(Kernel).to receive(:trap).and_return(:ok)
-    @app = Chef::Application::Solo.new
-    allow(@app).to receive(:configure_opt_parser).and_return(true)
-    allow(@app).to receive(:configure_chef).and_return(true)
-    allow(@app).to receive(:configure_logging).and_return(true)
-    allow(@app).to receive(:trap)
+    allow(app).to receive(:configure_opt_parser).and_return(true)
+    allow(app).to receive(:configure_chef).and_return(true)
+    allow(app).to receive(:configure_logging).and_return(true)
+    allow(app).to receive(:trap)
+
     Chef::Config[:recipe_url] = false
     Chef::Config[:json_attribs] = false
     Chef::Config[:solo] = true
@@ -32,10 +35,15 @@ describe Chef::Application::Solo do
 
   describe "configuring the application" do
     it "should set solo mode to true" do
-      @app.reconfigure
+      app.reconfigure
       expect(Chef::Config[:solo]).to be_truthy
     end
 
+    it "should set audit-mode to :disabled" do
+      app.reconfigure
+      expect(Chef::Config[:audit_mode]).to be :disabled
+    end
+
     describe "when configured to not fork the client process" do
       before do
         Chef::Config[:client_fork] = false
@@ -56,7 +64,7 @@ Configuration settings:
   interval  = 600 seconds
 Enable chef-client interval runs by setting `:client_fork = true` in your config file or adding `--fork` to your command line options."
           )
-          @app.reconfigure
+          app.reconfigure
         end
       end
     end
@@ -68,7 +76,7 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
 
       it "should set the interval to 1800" do
         Chef::Config[:interval] = nil
-        @app.reconfigure
+        app.reconfigure
         expect(Chef::Config[:interval]).to eq(1800)
       end
     end
@@ -85,44 +93,46 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
       end
 
       it "reads the JSON attributes from the specified source" do
-        @app.reconfigure
-        expect(@app.chef_client_json).to eq(json_attribs)
+        app.reconfigure
+        expect(app.chef_client_json).to eq(json_attribs)
       end
     end
 
     describe "when the recipe_url configuration option is specified" do
+      let(:tarfile) { StringIO.new("remote_tarball_content") }
+      let(:target_file) { StringIO.new }
+
       before do
         Chef::Config[:cookbook_path] = "#{Dir.tmpdir}/chef-solo/cookbooks"
         Chef::Config[:recipe_url] = "http://junglist.gen.nz/recipes.tgz"
+
         allow(FileUtils).to receive(:rm_rf).and_return(true)
         allow(FileUtils).to receive(:mkdir_p).and_return(true)
-        @tarfile = StringIO.new("remote_tarball_content")
-        allow(@app).to receive(:open).with("http://junglist.gen.nz/recipes.tgz").and_yield(@tarfile)
 
-        @target_file = StringIO.new
-        allow(File).to receive(:open).with("#{Dir.tmpdir}/chef-solo/recipes.tgz", "wb").and_yield(@target_file)
+        allow(app).to receive(:open).with("http://junglist.gen.nz/recipes.tgz").and_yield(tarfile)
+        allow(File).to receive(:open).with("#{Dir.tmpdir}/chef-solo/recipes.tgz", "wb").and_yield(target_file)
 
         allow(Chef::Mixin::Command).to receive(:run_command).and_return(true)
       end
 
       it "should create the recipes path based on the parent of the cookbook path" do
         expect(FileUtils).to receive(:mkdir_p).with("#{Dir.tmpdir}/chef-solo").and_return(true)
-        @app.reconfigure
+        app.reconfigure
       end
 
       it "should download the recipes" do
-        expect(@app).to receive(:open).with("http://junglist.gen.nz/recipes.tgz").and_yield(@tarfile)
-        @app.reconfigure
+        expect(app).to receive(:open).with("http://junglist.gen.nz/recipes.tgz").and_yield(tarfile)
+        app.reconfigure
       end
 
       it "should write the recipes to the target path" do
-        @app.reconfigure
-        expect(@target_file.string).to eq("remote_tarball_content")
+        app.reconfigure
+        expect(target_file.string).to eq("remote_tarball_content")
       end
 
       it "should untar the target file to the parent of the cookbook path" do
         expect(Chef::Mixin::Command).to receive(:run_command).with({:command => "tar zxvf #{Dir.tmpdir}/chef-solo/recipes.tgz -C #{Dir.tmpdir}/chef-solo"}).and_return(true)
-        @app.reconfigure
+        app.reconfigure
       end
     end
   end
@@ -142,9 +152,9 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
     end
 
     it "should fetch the recipe_url first" do
-      expect(@app).to receive(:fetch_recipe_tarball).ordered
+      expect(app).to receive(:fetch_recipe_tarball).ordered
       expect(Chef::ConfigFetcher).to receive(:new).ordered.and_return(config_fetcher)
-      @app.reconfigure
+      app.reconfigure
     end
   end
 
@@ -153,18 +163,17 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
       Chef::Config[:solo] = true
 
       allow(Chef::Daemon).to receive(:change_privilege)
-      @chef_client = double("Chef::Client")
-      allow(Chef::Client).to receive(:new).and_return(@chef_client)
-      @app = Chef::Application::Solo.new
+      chef_client = double("Chef::Client")
+      allow(Chef::Client).to receive(:new).and_return(chef_client)
       # this is all stuff the reconfigure method needs
-      allow(@app).to receive(:configure_opt_parser).and_return(true)
-      allow(@app).to receive(:configure_chef).and_return(true)
-      allow(@app).to receive(:configure_logging).and_return(true)
+      allow(app).to receive(:configure_opt_parser).and_return(true)
+      allow(app).to receive(:configure_chef).and_return(true)
+      allow(app).to receive(:configure_logging).and_return(true)
     end
 
     it "should change privileges" do
       expect(Chef::Daemon).to receive(:change_privilege).and_return(true)
-      @app.setup_application
+      app.setup_application
     end
   end