From: Daniele Sluijters <daenney@users.noreply.github.com>
Date: Tue, 20 Jan 2015 17:39:46 +0000 (-0500)
Subject: Merge pull request #403 from WolverineFan/fix_apt_updates_facts
X-Git-Tag: 1.8.0~22
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6b23f3cb78301f4ae7b039e7254ce00a675eb211;hp=6d60659e703293344884f0f41ec010bfc855f5a9;p=puppet-modules%2Fpuppetlabs-apt.git

Merge pull request #403 from WolverineFan/fix_apt_updates_facts

Fix apt_has_updates fact not parsing apt-check output correctly
---

diff --git a/lib/facter/apt_updates.rb b/lib/facter/apt_updates.rb
index 77e667e..014782e 100644
--- a/lib/facter/apt_updates.rb
+++ b/lib/facter/apt_updates.rb
@@ -2,18 +2,23 @@ apt_package_updates = nil
 Facter.add("apt_has_updates") do
   confine :osfamily => 'Debian'
   if File.executable?("/usr/lib/update-notifier/apt-check")
-    apt_package_updates = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check 2>/dev/null').split(';')
+    apt_check_result = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check 2>&1')
+    if not apt_check_result.nil? and apt_check_result =~ /^\d+;\d+$/
+      apt_package_updates = apt_check_result.split(';')
+    end
   end
 
   setcode do
-    apt_package_updates != ['0', '0'] unless apt_package_updates.nil?
+    if not apt_package_updates.nil? and apt_package_updates.length == 2
+      apt_package_updates != ['0', '0']
+    end
   end
 end
 
 Facter.add("apt_package_updates") do
   confine :apt_has_updates => true
   setcode do
-    packages = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check -p 2>/dev/null').split("\n")
+    packages = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check -p 2>&1').split("\n")
     if Facter.version < '2.0.0'
       packages.join(',')
     else
diff --git a/spec/unit/facter/apt_has_updates_spec.rb b/spec/unit/facter/apt_has_updates_spec.rb
index 691fb32..f8a3f20 100644
--- a/spec/unit/facter/apt_has_updates_spec.rb
+++ b/spec/unit/facter/apt_has_updates_spec.rb
@@ -6,27 +6,49 @@ describe 'apt_has_updates fact' do
 
   describe 'on non-Debian distro' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'RedHat'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'RedHat'
     }
     it { should be_nil }
   end
 
   describe 'on Debian based distro missing update-notifier-common' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'Debian'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
       File.stubs(:executable?) # Stub all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns false
     }
     it { should be_nil }
   end
 
+  describe 'on Debian based distro with broken packages' do
+    before {
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
+      File.stubs(:executable?) # Stub all other calls
+      Facter::Util::Resolution.stubs(:exec) # Catch all other calls
+      File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "E: Error: BrokenCount > 0"
+    }
+    it { should be_nil }
+  end
+
+  describe 'on Debian based distro with unknown error with semicolons' do
+    before {
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
+      File.stubs(:executable?) # Stub all other calls
+      Facter::Util::Resolution.stubs(:exec) # Catch all other calls
+      File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "E: Unknown Error: 'This error contains something that could be parsed like 4;3' (10)"
+    }
+    it { should be_nil }
+  end
+
   describe 'on Debian based distro' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'Debian'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "4;3"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "4;3"
     }
     it { should be true }
   end
diff --git a/spec/unit/facter/apt_package_updates_spec.rb b/spec/unit/facter/apt_package_updates_spec.rb
index c0f54f2..5c7a624 100644
--- a/spec/unit/facter/apt_package_updates_spec.rb
+++ b/spec/unit/facter/apt_package_updates_spec.rb
@@ -17,8 +17,8 @@ describe 'apt_package_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "1;2"
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check -p 2>/dev/null').returns "puppet-common\nlinux-generic\nlinux-image-generic"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "1;2"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check -p 2>&1').returns "puppet-common\nlinux-generic\nlinux-image-generic"
     }
     it {
       if Facter.version < '2.0.0'
diff --git a/spec/unit/facter/apt_security_updates_spec.rb b/spec/unit/facter/apt_security_updates_spec.rb
index a5d26c7..4bc760f 100644
--- a/spec/unit/facter/apt_security_updates_spec.rb
+++ b/spec/unit/facter/apt_security_updates_spec.rb
@@ -17,7 +17,7 @@ describe 'apt_security_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "14;7"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "14;7"
     }
     it { should == 7 }
   end
diff --git a/spec/unit/facter/apt_updates_spec.rb b/spec/unit/facter/apt_updates_spec.rb
index 538466f..7e9b77f 100644
--- a/spec/unit/facter/apt_updates_spec.rb
+++ b/spec/unit/facter/apt_updates_spec.rb
@@ -17,7 +17,7 @@ describe 'apt_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "14;7"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "14;7"
     }
     it { should == 14 }
   end