Fix ruby validation errors

Only capture the exec for `radiusd -v`, as facter etc. runs `uname -s` and whatever else. Mock Facter.value for things in os_facts, so we return the value from os_facts rather than the actual value from the system running the tests.

lib/facter/freeradius_version.rb

 # Grab the FreeRADIUS version from the output of radiusd -v
-
 module Facter::Util::FreeradiusVersion
   class << self
     def version_string
       # Set path to binary for our platform
       dist = Facter.value(:osfamily)
-      case dist
-      when /RedHat/
-        binary = 'radiusd'
-      when /Debian/
-        binary = 'freeradius'
+      binary = case dist
+               when %r{RedHat}
+                 'radiusd'
+               when %r{Debian}
+                 'freeradius'
                else
-        binary = 'radiusd'
+                 'radiusd'
                end
 
       Facter::Core::Execution.exec("#{binary} -v")
@@ -23,8 +22,8 @@ end
 Facter.add(:freeradius_version) do
   setcode do
     version_string = Facter::Util::FreeradiusVersion.version_string
-    if !version_string.nil?
-      version = version_string.split(/\n/)[0].match(/FreeRADIUS Version (\d+\.\d+\.\d+)/)[1].to_s
+    unless version_string.nil?
+      version = version_string.split(%r{\n})[0].match(%r{FreeRADIUS Version (\d+\.\d+\.\d+)})[1].to_s
     end
     version
   end
@@ -34,8 +33,8 @@ end
 Facter.add(:freeradius_maj_version) do
   setcode do
     version_string = Facter::Util::FreeradiusVersion.version_string
-    if !version_string.nil?
-      majver = version_string.split(/\n/)[0].match(/FreeRADIUS Version (\d+)\.\d+\.\d+/)[1].to_s
+    unless version_string.nil?
+      majver = version_string.split(%r{\n})[0].match(%r{FreeRADIUS Version (\d+)\.\d+\.\d+})[1].to_s
     end
     majver
   end

spec/classes/freeradius_spec.rb

@@ -59,7 +59,7 @@ describe 'freeradius' do
           '/etc/raddb/clients.d',
           '/etc/raddb/listen.d',
           '/etc/raddb/sites-enabled',
-          '/etc/raddb/instantiate'
+          '/etc/raddb/instantiate',
         ].each do |file|
           is_expected.to contain_file(file)
             .with(
@@ -93,7 +93,7 @@ describe 'freeradius' do
           .with(
             'content' => 'policy {',
             'order'   => '10',
-            'target'  => '/etc/raddb/policy.conf'
+            'target'  => '/etc/raddb/policy.conf',
           )
       end
 
@@ -102,7 +102,7 @@ describe 'freeradius' do
           .with(
             'content' => '}',
             'order'   => '99',
-            'target'  => '/etc/raddb/policy.conf'
+            'target'  => '/etc/raddb/policy.conf',
           )
       end
 
@@ -123,7 +123,7 @@ describe 'freeradius' do
           .with(
             'content' => "# Proxy config\n",
             'order'   => '05',
-            'target'  => '/etc/raddb/proxy.conf'
+            'target'  => '/etc/raddb/proxy.conf',
           )
       end
 
@@ -143,7 +143,7 @@ describe 'freeradius' do
         is_expected.to contain_concat__fragment('attr-default')
           .with(
             'order'   => '10',
-            'target'  => '/etc/raddb/mods-available/attr_filter'
+            'target'  => '/etc/raddb/mods-available/attr_filter',
           )
       end
 
@@ -163,7 +163,7 @@ describe 'freeradius' do
           .with(
             'order'  => '10',
             'source' => 'puppet:///modules/freeradius/dictionary.header',
-            'target' => '/etc/raddb/dictionary'
+            'target' => '/etc/raddb/dictionary',
           )
       end
 
@@ -172,7 +172,7 @@ describe 'freeradius' do
           .with(
             'order'  => '90',
             'source' => 'puppet:///modules/freeradius/dictionary.footer',
-            'target' => '/etc/raddb/dictionary'
+            'target' => '/etc/raddb/dictionary',
           )
       end
 
@@ -180,7 +180,7 @@ describe 'freeradius' do
         is_expected.to contain_package('freeradius')
           .with(
             'ensure' => 'installed',
-            'name'   => 'freeradius'
+            'name'   => 'freeradius',
           )
       end
 
@@ -233,23 +233,23 @@ describe 'freeradius' do
 
       it do
         is_expected.to contain_freeradius__module('always')
-          .with(      )
+          .with_preserve(true)
       end
 
       it do
         is_expected.to contain_freeradius__module('detail')
-          .with(      )
+          .with_preserve(true)
       end
 
       it do
         is_expected.to contain_freeradius__module('detail.log')
-          .with(      )
+          .with_preserve(true)
       end
 
       it do
         [
           '/var/log/radius',
-          '/var/log/radius/radacct'
+          '/var/log/radius/radacct',
         ].each do |file|
           is_expected.to contain_file(file)
             .with(
@@ -266,7 +266,7 @@ describe 'freeradius' do
           .with(
             'group'   => 'radiusd',
             'owner'   => 'radiusd',
-            'seltype' => 'radiusd_log_t'
+            'seltype' => 'radiusd_log_t',
           )
           .that_requires('Package[freeradius]')
           .that_requires('User[radiusd]')
@@ -283,7 +283,7 @@ describe 'freeradius' do
             'postrotate'    => 'kill -HUP `cat /var/run/radiusd/radiusd.pid`',
             'rotate'        => '7',
             'rotate_every'  => 'day',
-            'sharedscripts' => 'true'
+            'sharedscripts' => 'true',
           )
       end
 
@@ -297,7 +297,7 @@ describe 'freeradius' do
             'postrotate'    => 'kill -HUP `cat /var/run/radiusd/radiusd.pid`',
             'rotate'        => '1',
             'rotate_every'  => 'week',
-            'sharedscripts' => 'true'
+            'sharedscripts' => 'true',
           )
       end
 
@@ -311,18 +311,16 @@ describe 'freeradius' do
             'postrotate'    => 'kill -HUP `cat /var/run/radiusd/radiusd.pid`',
             'rotate'        => '26',
             'rotate_every'  => 'week',
-            'sharedscripts' => 'true'
+            'sharedscripts' => 'true',
           )
       end
 
       it do
         [
           '/etc/raddb/certs/dh',
-          '/etc/raddb/certs/random'
+          '/etc/raddb/certs/random',
         ].each do |file|
           is_expected.to contain_file(file)
-            .with(
-            )
             .that_requires('Exec[dh]')
             .that_requires('Exec[random]')
         end
@@ -355,7 +353,7 @@ describe 'freeradius' do
             'logoutput'   => 'on_failure',
             'path'        => ['/bin/', '/sbin/', '/usr/bin/', '/usr/sbin/'],
             'refreshonly' => 'true',
-            'returns'     => '0'
+            'returns'     => '0',
           )
       end
 
@@ -387,7 +385,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-mysql')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -402,7 +400,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-postgresql')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -417,7 +415,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-perl')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -432,7 +430,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-utils')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -447,7 +445,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-ldap')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -462,7 +460,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-dhcp')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -477,7 +475,7 @@ describe 'freeradius' do
         it do
           is_expected.to contain_package('freeradius-krb5')
             .with(
-              'ensure' => 'installed'
+              'ensure' => 'installed',
             )
         end
       end
@@ -493,7 +491,7 @@ describe 'freeradius' do
           is_expected.to contain_package('wpa_supplicant')
             .with(
               'ensure' => 'installed',
-              'name'   => 'wpa_supplicant'
+              'name'   => 'wpa_supplicant',
             )
         end
       end

spec/defines/attr_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::attr' do
 
   let(:params) do
     {
-      :source => 'puppet:///modules/test/path/to/file',
+      source: 'puppet:///modules/test/path/to/file',
     }
   end
 

spec/defines/client_spec.rb

@@ -7,9 +7,9 @@ describe 'freeradius::client' do
 
   let(:params) do
     {
-      :shortname => 'test_short',
-      :secret => 'secret_value',
-      :ip => '1.2.3.4',
+      shortname: 'test_short',
+      secret: 'secret_value',
+      ip: '1.2.3.4',
     }
   end
 

spec/defines/config_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::config' do
 
   let(:params) do
     {
-      :content => 'test content',
+      content: 'test content',
     }
   end
 

spec/defines/dictionary_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::dictionary' do
 
   let(:params) do
     {
-      :source => 'puppet:///modules/test/path/to/dict',
+      source: 'puppet:///modules/test/path/to/dict',
     }
   end
 

spec/defines/home_server_pool_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::home_server_pool' do
 
   let(:params) do
     {
-      :home_server => [
+      home_server: [
         'test_home_server_1',
         'test_home_server_2',
       ],

spec/defines/home_server_spec.rb

@@ -7,8 +7,8 @@ describe 'freeradius::home_server' do
 
   let(:params) do
     {
-      :secret => 'test_secret',
-      :ipaddr => '1.2.3.4',
+      secret: 'test_secret',
+      ipaddr: '1.2.3.4',
     }
   end
 

spec/defines/krb5_spec.rb

@@ -7,8 +7,8 @@ describe 'freeradius::krb5' do
 
   let(:params) do
     {
-      :keytab => 'test_keytab',
-      :principal => 'test_principal',
+      keytab: 'test_keytab',
+      principal: 'test_principal',
     }
   end
 

spec/defines/module_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::module' do
 
   let(:params) do
     {
-      :source => 'puppet:///modules/test/path/to/module',
+      source:  'puppet:///modules/test/path/to/module',
     }
   end
 

spec/defines/policy_spec.rb

@@ -7,7 +7,7 @@ describe 'freeradius::policy' do
 
   let(:params) do
     {
-      :source => 'puppet:///modules/test/path/to/policy',
+      source:  'puppet:///modules/test/path/to/policy',
     }
   end
 

spec/defines/sql_spec.rb

@@ -44,9 +44,9 @@ describe 'freeradius::sql' do
 
       context 'with sqltrace' do
         let(:params) do
-          super().merge({
+          super().merge(
             sqltrace: 'yes',
-          })
+          )
         end
 
         it do
@@ -68,12 +68,11 @@ describe 'freeradius::sql' do
 
       context 'with custom query file' do
         let(:params) do
-          super().merge({
+          super().merge(
             custom_query_file: 'puppet:///modules/path/to/custom/query/file',
-          })
+          )
         end
 
-
         it do
           is_expected.to contain_freeradius__config('test-queries.conf')
             .with_source('puppet:///modules/path/to/custom/query/file')

spec/spec_helper_local.rb

@@ -44,7 +44,7 @@ shared_context 'freeradius_default' do
   let(:pre_condition) do
     [
       redhat_params_class,
-      'class { freeradius: }'
+      'class { freeradius: }',
     ]
   end
 end

spec/unit/facter/freeradius_version_spec.rb

 require 'spec_helper'
 require 'facter/freeradius_version'
 
-describe :freeradius_version, :type => :fact do
+describe 'freeradius_version', type: :fact do
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+
       before :each do
         Facter.clear
-    expect(Facter::Core::Execution).to receive(:exec).with('radiusd -v').and_return('FreeRADIUS Version 3.0.21')
+        orig_facter_value_method = Facter.method(:value)
+        allow(Facter).to receive(:value) do |fact|
+          if facts.key?(fact)
+            facts[fact]
+          else
+            orig_facter_value_method.call(fact)
+          end
+        end
+
+        orig_exec_method = Facter::Core::Execution.method(:exec)
+        allow(Facter::Core::Execution).to receive(:exec) do |cmd|
+          case cmd
+          when %r{^(radiusd|freeradius) -v$}
+            'FreeRADIUS Version 3.0.21'
+          else
+            orig_exec_method.call(cmd)
+          end
+        end
       end
 
-  it 'should set freeradius_version' do
+      it 'sets freeradius_version' do
         expect(Facter.fact(:freeradius_version).value).to eq('3.0.21')
       end
 
-  it 'should set freeradius_maj_version' do
+      it 'sets freeradius_maj_version' do
         expect(Facter.fact(:freeradius_maj_version).value).to eq('3')
       end
     end
+  end
+end