From: Daniele Sluijters <daenney@users.noreply.github.com>
Date: Thu, 19 Mar 2015 09:16:15 +0000 (+0100)
Subject: Merge pull request #472 from mhaskel/acceptance_test_fixes
X-Git-Tag: 2.0.0~16
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ff51f714aba075f1bb8605471f8c34ee0654da5c;hp=f5c677556435afb8dc13d7617e45b14f7344217d;p=puppet-modules%2Fpuppetlabs-apt.git

Merge pull request #472 from mhaskel/acceptance_test_fixes

Acceptance test fixes
---

diff --git a/.travis.yml b/.travis.yml
index ec6f08d..8b374e9 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,6 @@
 ---
 language: ruby
+sudo: false
 bundler_args: --without system_tests
 script: "bundle exec rake validate && bundle exec rake lint && bundle exec rake spec SPEC_OPTS='--format documentation'"
 matrix:
diff --git a/manifests/init.pp b/manifests/init.pp
index dbf0332..453df31 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -14,9 +14,6 @@ class apt(
   if $update['frequency'] {
     validate_re($update['frequency'], $frequency_options)
   }
-  if $update['always'] {
-    validate_bool($update['always'])
-  }
   if $update['timeout'] {
     unless is_integer($update['timeout']) {
       fail('timeout value for update must be an integer')
@@ -84,7 +81,7 @@ class apt(
     true  => absent,
   }
 
-  if $_update['always'] {
+  if $_update['frequency'] == 'always' {
     Exec <| title=='apt_update' |> {
       refreshonly => false,
     }
diff --git a/manifests/params.pp b/manifests/params.pp
index f4489cd..cf6547a 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -57,7 +57,6 @@ class apt::params {
   }
 
   $update_defaults = {
-    'always'    => false,
     'frequency' => 'reluctantly',
     'timeout'   => undef,
     'tries'     => undef,
diff --git a/manifests/update.pp b/manifests/update.pp
index a19e90a..86f0682 100644
--- a/manifests/update.pp
+++ b/manifests/update.pp
@@ -4,51 +4,45 @@ class apt::update {
   #on the first run, but if it's not run in awhile something is likely borked
   #with apt and we'd want to know about it.
 
-  if $::apt::_update['always'] == false {
-    #if always_apt_update is true there's no point in parsing this logic.
-
-    case $::apt::_update['frequency'] {
-      'always': {
-        $_kick_apt = true
-      }
-      'daily': {
-        #compare current date with the apt_update_last_success fact to determine
-        #if we should kick apt_update.
-        $daily_threshold = (strftime('%s') - 86400)
-        if $::apt_update_last_success {
-          if $::apt_update_last_success < $daily_threshold {
-            $_kick_apt = true
-          } else {
-            $_kick_apt = false
-          }
-        } else {
-          #if apt-get update has not successfully run, we should kick apt_update
+  case $::apt::_update['frequency'] {
+    'always': {
+      $_kick_apt = true
+    }
+    'daily': {
+      #compare current date with the apt_update_last_success fact to determine
+      #if we should kick apt_update.
+      $daily_threshold = (strftime('%s') - 86400)
+      if $::apt_update_last_success {
+        if $::apt_update_last_success < $daily_threshold {
           $_kick_apt = true
+        } else {
+          $_kick_apt = false
         }
+      } else {
+        #if apt-get update has not successfully run, we should kick apt_update
+        $_kick_apt = true
       }
-      'weekly':{
-        #compare current date with the apt_update_last_success fact to determine
-        #if we should kick apt_update.
-        $weekly_threshold = (strftime('%s') - 604800)
-        if $::apt_update_last_success {
-          if ( $::apt_update_last_success < $weekly_threshold ) {
-            $_kick_apt = true
-          } else {
-            $_kick_apt = false
-          }
-        } else {
-          #if apt-get update has not successfully run, we should kick apt_update
+    }
+    'weekly':{
+      #compare current date with the apt_update_last_success fact to determine
+      #if we should kick apt_update.
+      $weekly_threshold = (strftime('%s') - 604800)
+      if $::apt_update_last_success {
+        if ( $::apt_update_last_success < $weekly_threshold ) {
           $_kick_apt = true
+        } else {
+          $_kick_apt = false
         }
-      }
-      default: {
-        #catches 'recluctantly', and any other value (which should not occur).
-        #do nothing.
-        $_kick_apt = false
+      } else {
+        #if apt-get update has not successfully run, we should kick apt_update
+        $_kick_apt = true
       }
     }
-  } else {
-    $_kick_apt = false
+    default: {
+      #catches 'recluctantly', and any other value (which should not occur).
+      #do nothing.
+      $_kick_apt = false
+    }
   }
 
   if $_kick_apt {
diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb
index 856007b..2131c8a 100644
--- a/spec/classes/apt_spec.rb
+++ b/spec/classes/apt_spec.rb
@@ -96,7 +96,7 @@ describe 'apt' do
   context 'lots of non-defaults' do
     let :params do
       {
-        :update => { 'always' => true, 'timeout' => 1, 'tries' => 3 },
+        :update => { 'frequency' => 'always', 'timeout' => 1, 'tries' => 3 },
         :purge  => { 'sources.list' => false, 'sources.list.d' => false,
                      'preferences' => false, 'preferences.d' => false, },
       }
diff --git a/spec/classes/apt_update_spec.rb b/spec/classes/apt_update_spec.rb
index 06f76de..8fba4a0 100644
--- a/spec/classes/apt_update_spec.rb
+++ b/spec/classes/apt_update_spec.rb
@@ -2,108 +2,74 @@
 require 'spec_helper'
 
 describe 'apt::update', :type => :class do
-  context "when update['always']=true" do
-    #This should completely disable all of this logic. These tests are to guarantee that we don't somehow magically change the behavior.
-    let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
-    let (:pre_condition) { "class{'::apt': update => {'always' => true},}" }
-    it 'should trigger an apt-get update run' do
-      #set the apt_update exec's refreshonly attribute to false
-      is_expected.to contain_exec('apt_update').with({'refreshonly' => false })
-    end
-    ['always','daily','weekly','reluctantly'].each do |update_frequency|
-      context "when apt::update['frequency'] has the value of #{update_frequency}" do
-        { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
-          context "and $::apt_update_last_success indicates #{desc}" do
-            let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
-            let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
-            it 'should trigger an apt-get update run' do
-              # set the apt_update exec's refreshonly attribute to false
-              is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
-            end
-          end
-          context 'when $::apt_update_last_success is nil' do
-            let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
-            let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
-            it 'should trigger an apt-get update run' do
-              #set the apt_update exec\'s refreshonly attribute to false
-              is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
-            end
-          end
-        end
-      end
-    end
-  end
-
-  context "when apt::update['always']=false" do
-    context "and apt::update['frequency']='always'" do
-      { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
-        context "and $::apt_update_last_success indicates #{desc}" do
-          let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
-          let (:pre_condition) { "class{'::apt': update => {'always' => false, 'frequency' => 'always' },}" }
-          it 'should trigger an apt-get update run' do
-            #set the apt_update exec's refreshonly attribute to false
-            is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
-          end
-        end
-      end
-      context 'when $::apt_update_last_success is nil' do
-        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
-        let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'always' },}" }
+  context "and apt::update['frequency']='always'" do
+    { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
+      context "and $::apt_update_last_success indicates #{desc}" do
+        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
+        let (:pre_condition) { "class{'::apt': update => {'frequency' => 'always' },}" }
         it 'should trigger an apt-get update run' do
-          #set the apt_update exec\'s refreshonly attribute to false
+          #set the apt_update exec's refreshonly attribute to false
           is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
         end
       end
     end
-    context "and apt::update['frequency']='reluctantly'" do
-      {'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
-        context "and $::apt_update_last_success indicates #{desc}" do
-          let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy'} }
-          let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
-          it 'should not trigger an apt-get update run' do
-            #don't change the apt_update exec's refreshonly attribute. (it should be true)
-            is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
-          end
-        end
+    context 'when $::apt_update_last_success is nil' do
+      let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
+      let (:pre_condition) { "class{ '::apt': update => {'frequency' => 'always' },}" }
+      it 'should trigger an apt-get update run' do
+        #set the apt_update exec\'s refreshonly attribute to false
+        is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
       end
-      context 'when $::apt_update_last_success is nil' do
-        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
-        let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
+    end
+  end
+  context "and apt::update['frequency']='reluctantly'" do
+    {'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
+      context "and $::apt_update_last_success indicates #{desc}" do
+        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy'} }
+        let (:pre_condition) { "class{ '::apt': update => {'frequency' => 'reluctantly' },}" }
         it 'should not trigger an apt-get update run' do
           #don't change the apt_update exec's refreshonly attribute. (it should be true)
           is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
         end
       end
     end
-    ['daily','weekly'].each do |update_frequency|
-      context "and apt::update['frequency'] has the value of #{update_frequency}" do
-        { 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
-          context "and $::apt_update_last_success indicates #{desc}" do
-            let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
-            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
-            it 'should trigger an apt-get update run' do
-              #set the apt_update exec\'s refreshonly attribute to false
-              is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
-            end
-          end
-        end
-        context 'when the $::apt_update_last_success fact has a recent value' do
-          let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => Time.now.to_i } }
-            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
-          it 'should not trigger an apt-get update run' do
-            #don't change the apt_update exec\'s refreshonly attribute. (it should be true)
-            is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
-          end
-        end
-        context 'when $::apt_update_last_success is nil' do
-          let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => nil } }
-            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
+    context 'when $::apt_update_last_success is nil' do
+      let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
+      let (:pre_condition) { "class{ '::apt': update => {'frequency' => 'reluctantly' },}" }
+      it 'should not trigger an apt-get update run' do
+        #don't change the apt_update exec's refreshonly attribute. (it should be true)
+        is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
+      end
+    end
+  end
+  ['daily','weekly'].each do |update_frequency|
+    context "and apt::update['frequency'] has the value of #{update_frequency}" do
+      { 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
+        context "and $::apt_update_last_success indicates #{desc}" do
+          let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
+          let (:pre_condition) { "class{ '::apt': update => {'frequency' => '#{update_frequency}',} }" }
           it 'should trigger an apt-get update run' do
             #set the apt_update exec\'s refreshonly attribute to false
             is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
           end
         end
       end
+      context 'when the $::apt_update_last_success fact has a recent value' do
+        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => Time.now.to_i } }
+        let (:pre_condition) { "class{ '::apt': update => {'frequency' => '#{update_frequency}',} }" }
+        it 'should not trigger an apt-get update run' do
+          #don't change the apt_update exec\'s refreshonly attribute. (it should be true)
+          is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
+        end
+      end
+      context 'when $::apt_update_last_success is nil' do
+        let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => nil } }
+        let (:pre_condition) { "class{ '::apt': update => {'frequency' => '#{update_frequency}',} }" }
+        it 'should trigger an apt-get update run' do
+          #set the apt_update exec\'s refreshonly attribute to false
+          is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
+        end
+      end
     end
   end
 end