Cleanup

* (More) consistently use `.dig` to retrieve facts
* Supply missing facts in tests

lib/puppet/functions/simplib/host_is_me.rb

@@ -28,8 +28,8 @@ def hostlist_contains_me(hosts)
     scope = closure_scope
 
     host_identifiers = [
-      scope['facts']['networking']['fqdn'],
-      scope['facts']['networking']['hostname'],
+      scope['facts'].dig('networking', 'fqdn'),
+      scope['facts'].dig('networking', 'hostname'),
       'localhost',
       'localhost.localdomain',
     ]

lib/puppet/functions/simplib/ip_to_cron.rb

@@ -53,7 +53,7 @@
   def ip_to_cron(occurs = 1, max_value = 59, algorithm = 'ip_mod', ip = nil)
     if ip.nil?
       scope = closure_scope
-      ipaddr = scope['facts']['networking']['ip']
+      ipaddr = scope['facts'].dig('networking', 'ip')
     else
       ipaddr = ip.dup
     end

lib/puppet/functions/simplib/join_mount_opts.rb

@@ -34,7 +34,7 @@ def join_mount_opts(system_mount_opts, new_mount_opts)
 
     mount_options = {}
     scope = closure_scope
-    selinux_current_mode = scope['facts']['os'].dig('selinux', 'current_mode')
+    selinux_current_mode = scope['facts'].dig('os', 'selinux', 'current_mode')
 
     if !selinux_current_mode || (selinux_current_mode == 'disabled')
       # SELinux is off, get rid of selinux related items in the options

lib/puppet/provider/reboot_notify/notify.rb

@@ -124,7 +124,7 @@ def self.post_resource_eval
       # If the number of seconds between the time that the record was written
       # and the current time is greater than the system uptime then we should
       # remove the record
-      (current_time - v['updated']) > Facter.value(:system_uptime)['seconds']
+      (current_time - v['updated']) > Facter.value(:system_uptime)&.dig('seconds')
     end
 
     unless records.empty?

spec/classes/reboot_notify_spec.rb

@@ -2,8 +2,10 @@
 
 describe 'simplib::reboot_notify' do
   context 'on supported operating systems' do
-    on_supported_os.each_key do |os|
+    on_supported_os.each do |os, os_facts|
       context "on #{os}" do
+        let(:facts) { os_facts }
+
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to create_reboot_notify('__simplib_control__').with_log_level('notice') }
         it { is_expected.to create_reboot_notify('__simplib_control__').with_control_only(true) }

spec/functions/ldap/domain_to_dn_spec.rb

@@ -1,31 +1,35 @@
 require 'spec_helper'
 
 describe 'simplib::ldap::domain_to_dn' do
-  on_supported_os.each_value do |os_facts|
-    context 'with a regular domain' do
-      let(:facts) do
-        os_facts[:networking][:domain] = 'test.domain'
-        os_facts
-      end
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
 
-      it { is_expected.to run.and_return('DC=test,DC=domain') }
-    end
+      context 'with a regular domain' do
+        let(:facts) do
+          os_facts[:networking][:domain] = 'test.domain'
+          os_facts
+        end
 
-    context 'with a short domain' do
-      let(:facts) do
-        os_facts[:networking][:domain] = 'domain'
-        os_facts
+        it { is_expected.to run.and_return('DC=test,DC=domain') }
       end
 
-      it { is_expected.to run.and_return('DC=domain') }
-    end
+      context 'with a short domain' do
+        let(:facts) do
+          os_facts[:networking][:domain] = 'domain'
+          os_facts
+        end
 
-    context 'when passed a domain' do
-      it { is_expected.to run.with_params('test.domain').and_return('DC=test,DC=domain') }
-    end
+        it { is_expected.to run.and_return('DC=domain') }
+      end
+
+      context 'when passed a domain' do
+        it { is_expected.to run.with_params('test.domain').and_return('DC=test,DC=domain') }
+      end
 
-    context 'when told to downcase the attributes' do
-      it { is_expected.to run.with_params('test.domain', true).and_return('dc=test,dc=domain') }
+      context 'when told to downcase the attributes' do
+        it { is_expected.to run.with_params('test.domain', true).and_return('dc=test,dc=domain') }
+      end
     end
   end
 end

spec/functions/simplib/dlookup_spec.rb

@@ -2,70 +2,75 @@
 
 # This just gives us a hook so that we can call the function later on
 describe 'simplib::stages', type: :class do
-  let(:pre_condition) do
-    <<~END
-      define mydef::test (
-        $attribute = simplib::dlookup('mydef::test', 'attribute', $title, { 'default_value' => 'lucille2' })
-      ) {
-        notify { "$title says": message => $attribute }
-      }
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+      let(:pre_condition) do
+        <<~END
+          define mydef::test (
+            $attribute = simplib::dlookup('mydef::test', 'attribute', $title, { 'default_value' => 'lucille2' })
+          ) {
+            notify { "$title says": message => $attribute }
+          }
 
-      define mydef::othertest (
-        $attribute = simplib::dlookup('mydef::test', 'attribute', { 'default_value' => 'lucille2' })
-      ) {
-        notify { "other $title says": message => $attribute }
-      }
+          define mydef::othertest (
+            $attribute = simplib::dlookup('mydef::test', 'attribute', { 'default_value' => 'lucille2' })
+          ) {
+            notify { "other $title says": message => $attribute }
+          }
 
-      mydef::test { 'gob': }
-      mydef::test { 'tobias': }
-      mydef::test { 'michael': attribute => 'bananastand' }
+          mydef::test { 'gob': }
+          mydef::test { 'tobias': }
+          mydef::test { 'michael': attribute => 'bananastand' }
 
-      mydef::othertest { 'gob': }
-      mydef::othertest { 'tobias': }
-      mydef::othertest { 'michael': attribute => 'bananastand' }
-    END
-  end
+          mydef::othertest { 'gob': }
+          mydef::othertest { 'tobias': }
+          mydef::othertest { 'michael': attribute => 'bananastand' }
+        END
+      end
 
-  let(:gob) { catalogue.resource('Mydef::Test[gob]') }
-  let(:tobias) { catalogue.resource('Mydef::Test[tobias]') }
-  let(:michael) { catalogue.resource('Mydef::Test[michael]') }
+      let(:gob) { catalogue.resource('Mydef::Test[gob]') }
+      let(:tobias) { catalogue.resource('Mydef::Test[tobias]') }
+      let(:michael) { catalogue.resource('Mydef::Test[michael]') }
 
-  let(:gob_other) { catalogue.resource('Mydef::Othertest[gob]') }
-  let(:tobias_other) { catalogue.resource('Mydef::Othertest[tobias]') }
-  let(:michael_other) { catalogue.resource('Mydef::Othertest[michael]') }
+      let(:gob_other) { catalogue.resource('Mydef::Othertest[gob]') }
+      let(:tobias_other) { catalogue.resource('Mydef::Othertest[tobias]') }
+      let(:michael_other) { catalogue.resource('Mydef::Othertest[michael]') }
 
-  it { is_expected.to compile.with_all_deps }
+      it { is_expected.to compile.with_all_deps }
 
-  context 'no overrides' do
-    it { expect(gob[:attribute]).to eq('lucille2') }
-    it { expect(tobias[:attribute]).to eq('lucille2') }
-    it { expect(michael[:attribute]).to eq('bananastand') }
-    it { expect(gob_other[:attribute]).to eq('lucille2') }
-    it { expect(tobias_other[:attribute]).to eq('lucille2') }
-    it { expect(michael_other[:attribute]).to eq('bananastand') }
-  end
+      context 'no overrides' do
+        it { expect(gob[:attribute]).to eq('lucille2') }
+        it { expect(tobias[:attribute]).to eq('lucille2') }
+        it { expect(michael[:attribute]).to eq('bananastand') }
+        it { expect(gob_other[:attribute]).to eq('lucille2') }
+        it { expect(tobias_other[:attribute]).to eq('lucille2') }
+        it { expect(michael_other[:attribute]).to eq('bananastand') }
+      end
 
-  context 'overrides' do
-    let(:facts) do
-      {
-        cache_bust: Time.now.to_s,
-        hieradata: 'simplib_dlookup_overrides',
-      }
-    end
+      context 'overrides' do
+        let(:facts) do
+          os_facts.merge(
+            cache_bust: Time.now.to_s,
+            hieradata: 'simplib_dlookup_overrides',
+          )
+        end
 
-    let(:hieradata) { 'simplib_dlookup_overrides' }
+        let(:hieradata) { 'simplib_dlookup_overrides' }
 
-    context 'with global overrides' do
-      it { expect(gob[:attribute]).to eq('illusions') }
-      it { expect(gob_other[:attribute]).to eq('illusions') }
-      it { expect(tobias_other[:attribute]).to eq('illusions') }
-    end
-    context 'with specific overrides' do
-      it { expect(tobias[:attribute]).to eq('blueman') }
-    end
-    context 'with a static value' do
-      it { expect(michael[:attribute]).to eq('bananastand') }
-      it { expect(michael_other[:attribute]).to eq('bananastand') }
+        context 'with global overrides' do
+          it { expect(gob[:attribute]).to eq('illusions') }
+          it { expect(gob_other[:attribute]).to eq('illusions') }
+          it { expect(tobias_other[:attribute]).to eq('illusions') }
+        end
+        context 'with specific overrides' do
+          it { expect(tobias[:attribute]).to eq('blueman') }
+        end
+        context 'with a static value' do
+          it { expect(michael[:attribute]).to eq('bananastand') }
+          it { expect(michael_other[:attribute]).to eq('bananastand') }
+        end
+      end
     end
   end
 end

spec/type_aliases/cron/hour_entry_spec.rb

@@ -1,30 +1,36 @@
 require 'spec_helper'
 
 describe 'Simplib::Cron::Hour_entry' do
-  context 'with valid parameters' do
-    it { is_expected.to allow_value('22') }
-    it { is_expected.to allow_value('*') }
-    it { is_expected.to allow_value('*/5') }
-    it { is_expected.to allow_value('2/5') }
-    it { is_expected.to allow_value(22) }
-    it { is_expected.to allow_value('23,20') }
-    it { is_expected.to allow_value('20-23') }
-    it { is_expected.to allow_value('0-23/2') }
-  end
-  context 'with invalid parameters' do
-    it { is_expected.not_to allow_value('one') }
-    it { is_expected.not_to allow_value('-2') }
-    it { is_expected.not_to allow_value('/3') }
-    it { is_expected.not_to allow_value('24') }
-    it { is_expected.not_to allow_value('13/*') }
-    it { is_expected.not_to allow_value('13-/15') }
-  end
-  context 'with silly things' do
-    it { is_expected.not_to allow_value([]) }
-    it { is_expected.not_to allow_value('.') }
-    it { is_expected.not_to allow_value('') }
-    it { is_expected.not_to allow_value('1  ') }
-    it { is_expected.not_to allow_value('5 1') }
-    it { is_expected.not_to allow_value(:undef) }
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+
+      context 'with valid parameters' do
+        it { is_expected.to allow_value('22') }
+        it { is_expected.to allow_value('*') }
+        it { is_expected.to allow_value('*/5') }
+        it { is_expected.to allow_value('2/5') }
+        it { is_expected.to allow_value(22) }
+        it { is_expected.to allow_value('23,20') }
+        it { is_expected.to allow_value('20-23') }
+        it { is_expected.to allow_value('0-23/2') }
+      end
+      context 'with invalid parameters' do
+        it { is_expected.not_to allow_value('one') }
+        it { is_expected.not_to allow_value('-2') }
+        it { is_expected.not_to allow_value('/3') }
+        it { is_expected.not_to allow_value('24') }
+        it { is_expected.not_to allow_value('13/*') }
+        it { is_expected.not_to allow_value('13-/15') }
+      end
+      context 'with silly things' do
+        it { is_expected.not_to allow_value([]) }
+        it { is_expected.not_to allow_value('.') }
+        it { is_expected.not_to allow_value('') }
+        it { is_expected.not_to allow_value('1  ') }
+        it { is_expected.not_to allow_value('5 1') }
+        it { is_expected.not_to allow_value(:undef) }
+      end
+    end
   end
 end

spec/type_aliases/cron/hour_spec.rb

@@ -1,20 +1,26 @@
 require 'spec_helper'
 
 describe 'Simplib::Cron::Hour' do
-  context 'with valid parameters' do
-    it { is_expected.to allow_value([22]) }
-    it { is_expected.to allow_value(['22']) }
-    it { is_expected.to allow_value(['20-23', '10-14/2', 3, 5, '19']) }
-    it { is_expected.to allow_value(22) }
-    it { is_expected.to allow_value('22') }
-    it { is_expected.to allow_value('20-23') }
-    it { is_expected.to allow_value('*') }
-    it { is_expected.to allow_value('*/5') }
-    it { is_expected.to allow_value('0-23/2') }
-  end
-  context 'with invalid parameters' do
-    it { is_expected.not_to allow_value(['20-23', '10-14/2', 3, 24, 5, '19']) }
-    it { is_expected.not_to allow_value(['0,1,12-19,5']) }
-    it { is_expected.not_to allow_value(["'0','1','12-19','5'"]) }
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+
+      context 'with valid parameters' do
+        it { is_expected.to allow_value([22]) }
+        it { is_expected.to allow_value(['22']) }
+        it { is_expected.to allow_value(['20-23', '10-14/2', 3, 5, '19']) }
+        it { is_expected.to allow_value(22) }
+        it { is_expected.to allow_value('22') }
+        it { is_expected.to allow_value('20-23') }
+        it { is_expected.to allow_value('*') }
+        it { is_expected.to allow_value('*/5') }
+        it { is_expected.to allow_value('0-23/2') }
+      end
+      context 'with invalid parameters' do
+        it { is_expected.not_to allow_value(['20-23', '10-14/2', 3, 24, 5, '19']) }
+        it { is_expected.not_to allow_value(['0,1,12-19,5']) }
+        it { is_expected.not_to allow_value(["'0','1','12-19','5'"]) }
+      end
+    end
   end
 end