Merge pull request #2654 from opscode/jdm/lcm-parser-fixes

Fix bug where errored parsing from what-if output causes resource to be considered converged

5 files changed, 20 insertions, 20 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index f5d91c24a6..0868341849 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -124,6 +124,7 @@ class Chef
     class DuplicateDataBagItem < RuntimeError; end
 
     class PowershellCmdletException < RuntimeError; end
+    class LCMParser < RuntimeError; end
 
     class CannotDetermineHomebrewOwner < Package; end
 
diff --git a/lib/chef/util/dsc/lcm_output_parser.rb b/lib/chef/util/dsc/lcm_output_parser.rb
index f8f853a33a..754fde3e8b 100644
--- a/lib/chef/util/dsc/lcm_output_parser.rb
+++ b/lib/chef/util/dsc/lcm_output_parser.rb
@@ -18,6 +18,7 @@
 
 require 'chef/log'
 require 'chef/util/dsc/resource_info'
+require 'chef/exceptions'
 
 class Chef
   class Util
@@ -53,8 +54,7 @@ class Chef
           #   ]
           #
           def self.parse(lcm_output)
-            return [] unless lcm_output
-
+            lcm_output ||= ""
             current_resource = Hash.new
 
             resources = []
@@ -96,7 +96,11 @@ class Chef
               resources.push(current_resource)
             end
 
-            build_resource_info(resources)
+            if resources.length > 0
+              build_resource_info(resources)
+            else
+              raise Chef::Exceptions::LCMParser, "Could not parse:\n#{lcm_output}"
+            end
           end
 
           def self.parse_line(line)
diff --git a/lib/chef/util/dsc/local_configuration_manager.rb b/lib/chef/util/dsc/local_configuration_manager.rb
index f498a2bfea..c3149429f9 100644
--- a/lib/chef/util/dsc/local_configuration_manager.rb
+++ b/lib/chef/util/dsc/local_configuration_manager.rb
@@ -103,7 +103,7 @@ EOH
       Chef::Log.debug("DSC: DSC returned the following '-whatif' output from test operation:\n#{what_if_output}")
       begin
         Parser::parse(what_if_output)
-      rescue Chef::Util::DSC::LocalConfigurationManager::Parser => e
+      rescue Chef::Exceptions::LCMParser => e
         Chef::Log::warn("Could not parse LCM output: #{e}")
         [Chef::Util::DSC::ResourceInfo.new('Unknown DSC Resources', true, ['Unknown changes because LCM output was not parsable.'])]
       end
diff --git a/spec/unit/util/dsc/lcm_output_parser_spec.rb b/spec/unit/util/dsc/lcm_output_parser_spec.rb
index ba4f40c4f7..3d44e07885 100644
--- a/spec/unit/util/dsc/lcm_output_parser_spec.rb
+++ b/spec/unit/util/dsc/lcm_output_parser_spec.rb
@@ -20,24 +20,19 @@ require 'chef/util/dsc/lcm_output_parser'
 
 describe Chef::Util::DSC::LocalConfigurationManager::Parser do
   context 'empty input parameter' do
-    it 'returns an empty array for a 0 length string' do
-      expect(Chef::Util::DSC::LocalConfigurationManager::Parser::parse('')).to be_empty
+    it 'raises an exception when there are no valid lines' do
+      str = <<-EOF
+                
+      EOF
+      expect {Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str)}.to raise_error(Chef::Exceptions::LCMParser)
     end
 
-    it 'returns an empty array for a nil input' do
-      expect(Chef::Util::DSC::LocalConfigurationManager::Parser::parse('')).to be_empty
+    it 'raises an exception for a nil input' do
+      expect {Chef::Util::DSC::LocalConfigurationManager::Parser::parse(nil)}.to raise_error(Chef::Exceptions::LCMParser)
     end
   end
 
   context 'correctly formatted output from lcm' do
-    it 'returns an empty array for a log with no resources' do
-      str = <<EOF
-logtype: [machinename]: LCM:  [ Start  Set      ]
-logtype: [machinename]: LCM:  [ End    Set      ]
-EOF
-      expect(Chef::Util::DSC::LocalConfigurationManager::Parser::parse(str)).to be_empty
-    end
-
     it 'returns a single resource when only 1 logged with the correct name' do
       str = <<EOF
 logtype: [machinename]: LCM:  [ Start  Set      ]
diff --git a/spec/unit/util/dsc/local_configuration_manager_spec.rb b/spec/unit/util/dsc/local_configuration_manager_spec.rb
index 009c667c87..1281862e67 100644
--- a/spec/unit/util/dsc/local_configuration_manager_spec.rb
+++ b/spec/unit/util/dsc/local_configuration_manager_spec.rb
@@ -82,7 +82,7 @@ EOH
         end
 
         it 'should should return a (possibly empty) array of ResourceInfo instances' do
-          expect(Chef::Log).to receive(:warn)
+          expect(Chef::Log).to receive(:warn).at_least(:once)
           expect(lcm).to receive(:whatif_not_supported?).and_call_original
           test_configuration_result = nil
           expect {test_configuration_result = lcm.test_configuration('config')}.not_to raise_error
@@ -96,14 +96,14 @@ EOH
         let(:lcm_cmdlet_success) { false }
 
         it 'should log a warning if the message is formatted as expected when a resource import failure occurs' do
-          expect(Chef::Log).to receive(:warn)
+          expect(Chef::Log).to receive(:warn).at_least(:once)
           expect(lcm).to receive(:dsc_module_import_failure?).and_call_original
           test_configuration_result = nil
           expect {test_configuration_result = lcm.test_configuration('config')}.not_to raise_error
         end
 
         it 'should return a (possibly empty) array of ResourceInfo instances' do
-          expect(Chef::Log).to receive(:warn)
+          expect(Chef::Log).to receive(:warn).at_least(:once)
           test_configuration_result = nil
           expect {test_configuration_result = lcm.test_configuration('config')}.not_to raise_error
           expect(test_configuration_result.class).to be(Array)
@@ -116,7 +116,7 @@ EOH
         let(:lcm_cmdlet_success) { false }
 
         it 'should log a warning' do
-          expect(Chef::Log).to receive(:warn)
+          expect(Chef::Log).to receive(:warn).at_least(:once)
           expect(lcm).to receive(:dsc_module_import_failure?).and_call_original
           expect {lcm.test_configuration('config')}.not_to raise_error
         end