(RUBOCOP) Resolve Rubocop Issues

Resolved:
- Layout:ClosingHeredocIndentation
- Lint/RedundantCopEnableDirective
- Naming/MemoizedInstanceVariableName
- RSpec/ContextWording
- RSpec/EmptyLineAfterExample
- RSpec/EmptyLineAfterExampleGroup
- RSpec/EmptyLineAfterHook
- RSpec/ImplicitSubject
- RSpec/RepeatedExampleGroupBody
- Style/IfUnlessModifier
- Style/RedundantArgument
- Style/RescueStandardError
- Style/SlicingWithRange
- Style/TrailingCommaInHashLiteral

Left in Place:
- RSpec/NoExpectationExample: Unable to properly wrap applies in such a way as to serve as an expectation.
- Style/GlobalVars: Replacing global variables with Constants causes warnings to be thrown by puppet applies.

Author: david22swan

Diff for: .rubocop_todo.yml

@@ -1,23 +1,11 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2023-08-29 15:24:31 UTC using RuboCop version 1.48.1.
+# on 2023-08-29 15:26:08 UTC using RuboCop version 1.48.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 1
-# This cop supports safe autocorrection (--autocorrect).
-Layout/ClosingHeredocIndentation:
-  Exclude:
-    - 'spec/spec_helper_acceptance_local.rb'
-
-# Offense count: 1
-# This cop supports safe autocorrection (--autocorrect).
-Lint/RedundantCopEnableDirective:
-  Exclude:
-    - 'spec/unit/classes/firewall_spec.rb'
-
 # Offense count: 20
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
@@ -49,69 +37,29 @@ Metrics/MethodLength:
 Metrics/PerceivedComplexity:
   Max: 122
 
-# Offense count: 1
-# Configuration parameters: EnforcedStyleForLeadingUnderscores.
-# SupportedStylesForLeadingUnderscores: disallowed, required, optional
-Naming/MemoizedInstanceVariableName:
-  Exclude:
-    - 'spec/spec_helper_acceptance_local.rb'
-
-# Offense count: 7
-# Configuration parameters: Prefixes, AllowedPatterns.
-# Prefixes: when, with, without
-RSpec/ContextWording:
-  Exclude:
-    - 'spec/acceptance/firewall_attributes_exceptions_spec.rb'
-    - 'spec/unit/classes/firewall_linux_redhat_spec.rb'
-    - 'spec/unit/classes/firewall_linux_spec.rb'
-
 # Offense count: 18
 # Configuration parameters: IgnoredMetadata.
 RSpec/DescribeClass:
   Enabled: false
 
-# Offense count: 8
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: AllowConsecutiveOneLiners.
-RSpec/EmptyLineAfterExample:
-  Exclude:
-    - 'spec/unit/classes/firewall_linux_archlinux_spec.rb'
-    - 'spec/unit/classes/firewall_linux_debian_spec.rb'
-    - 'spec/unit/classes/firewall_linux_redhat_spec.rb'
-
-# Offense count: 1
-# This cop supports safe autocorrection (--autocorrect).
-RSpec/EmptyLineAfterExampleGroup:
-  Exclude:
-    - 'spec/unit/classes/firewall_spec.rb'
-
-# Offense count: 7
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: AllowConsecutiveOneLiners.
-RSpec/EmptyLineAfterHook:
-  Exclude:
-    - 'spec/unit/facter/iptables_persistent_version_spec.rb'
-    - 'spec/unit/facter/iptables_spec.rb'
-
 # Offense count: 9
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
   Max: 8
 
 # Offense count: 41
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: single_line_only, single_statement_only, disallow, require_implicit
-RSpec/ImplicitSubject:
+RSpec/MultipleExpectations:
+  Max: 2
+
+# Offense count: 38
+# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
+# SupportedStyles: always, named_only
+RSpec/NamedSubject:
   Exclude:
     - 'spec/unit/classes/firewall_linux_archlinux_spec.rb'
     - 'spec/unit/classes/firewall_linux_debian_spec.rb'
     - 'spec/unit/classes/firewall_linux_redhat_spec.rb'
 
-# Offense count: 41
-RSpec/MultipleExpectations:
-  Max: 2
-
 # Offense count: 38
 # Configuration parameters: AllowedGroups.
 RSpec/NestedGroups:
@@ -129,61 +77,14 @@ RSpec/NoExpectationExample:
     - 'spec/acceptance/rules_spec.rb'
     - 'spec/acceptance/standard_usage_spec.rb'
 
-# Offense count: 3
-RSpec/RepeatedExampleGroupBody:
-  Exclude:
-    - 'spec/unit/classes/firewall_linux_debian_spec.rb'
-
 # Offense count: 83
 # Configuration parameters: AllowedVariables.
 Style/GlobalVars:
   Exclude:
     - 'lib/puppet/provider/firewall/firewall.rb'
     - 'lib/puppet/provider/firewallchain/firewallchain.rb'
 
-# Offense count: 2
-# This cop supports safe autocorrection (--autocorrect).
-Style/IfUnlessModifier:
-  Exclude:
-    - 'spec/spec_helper_acceptance_local.rb'
-
 # Offense count: 1
 Style/MixinUsage:
   Exclude:
     - 'spec/spec_helper.rb'
-
-# Offense count: 1
-# This cop supports unsafe autocorrection (--autocorrect-all).
-# Configuration parameters: Methods.
-Style/RedundantArgument:
-  Exclude:
-    - 'spec/spec_helper_acceptance_local.rb'
-
-# Offense count: 2
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: implicit, explicit
-Style/RescueStandardError:
-  Exclude:
-    - 'spec/spec_helper.rb'
-    - 'spec/spec_helper_acceptance_local.rb'
-
-# Offense count: 1
-# This cop supports unsafe autocorrection (--autocorrect-all).
-Style/SlicingWithRange:
-  Exclude:
-    - 'spec/spec_helper_acceptance_local.rb'
-
-# Offense count: 33
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: EnforcedStyleForMultiline.
-# SupportedStylesForMultiline: comma, consistent_comma, no_comma
-Style/TrailingCommaInHashLiteral:
-  Exclude:
-    - 'spec/spec_helper.rb'
-    - 'spec/spec_helper_local.rb'
-    - 'spec/unit/classes/firewall_linux_archlinux_spec.rb'
-    - 'spec/unit/classes/firewall_linux_debian_spec.rb'
-    - 'spec/unit/classes/firewall_linux_redhat_spec.rb'
-    - 'spec/unit/classes/firewall_linux_spec.rb'
-    - 'spec/unit/facter/iptables_persistent_version_spec.rb'

Diff for: spec/acceptance/firewall_attributes_exceptions_spec.rb

@@ -24,7 +24,7 @@
                                         (fetch_os_name == 'oraclelinux' && os[:release][0] <= '7') ||
                                         (os[:family] == 'ubuntu') do
     describe 'bytecode' do
-      context '4,48 0 0 9,21 0 1 6,6 0 0 1,6 0 0 0' do
+      context 'when 4,48 0 0 9,21 0 1 6,6 0 0 1,6 0 0 0' do
         pp = <<-PUPPETCODE
             class { '::firewall': }
             firewall { '102 - test':
@@ -1292,7 +1292,7 @@ class { '::firewall': }
   end
 
   describe 'condition', condition_parameter_test: false do
-    context 'is set' do
+    context 'when is set' do
       pp = <<-PUPPETCODE
         if $facts['os']['name'] == 'Ubuntu' {
           firewall { '010 isblue ipv4':

Diff for: spec/spec_helper.rb

@@ -13,7 +13,7 @@
 
 default_facts = {
   puppetversion: Puppet.version,
-  facterversion: Facter.version,
+  facterversion: Facter.version
 }
 
 default_fact_files = [
@@ -26,7 +26,7 @@
 
   begin
     default_facts.merge!(YAML.safe_load(File.read(f), [], [], true))
-  rescue => e
+  rescue StandardError => e
     RSpec.configuration.reporter.message "WARNING: Unable to load #{f}: #{e}"
   end
 end

Diff for: spec/spec_helper_acceptance_local.rb

@@ -21,7 +21,7 @@ def ip6tables_flush_all_tables
 
 def install_iptables
   LitmusHelper.instance.run_shell('iptables -V')
-rescue
+rescue StandardError
   if os[:family] == 'redhat'
     if fetch_os_name == 'oraclelinux' && os[:release].to_i == 7
       LitmusHelper.instance.run_shell('yum install iptables -y')
@@ -36,7 +36,7 @@ def install_iptables
 def iptables_version
   install_iptables
   x = LitmusHelper.instance.run_shell('iptables -V')
-  x.stdout.split(' ')[1][1..-1]
+  x.stdout.split[1][1..]
 end
 
 def pre_setup
@@ -51,7 +51,7 @@ def update_profile_file
 end
 
 def fetch_os_name
-  @facter_os_name ||= LitmusHelper.instance.run_shell('facter os.name').stdout.delete("\n").downcase
+  @fetch_os_name ||= LitmusHelper.instance.run_shell('facter os.name').stdout.delete("\n").downcase
 end
 
 RSpec.configure do |c|
@@ -61,9 +61,7 @@ def fetch_os_name
   c.filter_run_excluding condition_parameter_test: false
   c.before :suite do
     # Depmod is not availible by default on our AlmaLinux/CentOS 8 docker image
-    if ['almalinux-8', 'centos-8'].include?("#{fetch_os_name}-#{os[:release].to_i}")
-      LitmusHelper.instance.run_shell('yum install kmod -y')
-    end
+    LitmusHelper.instance.run_shell('yum install kmod -y') if ['almalinux-8', 'centos-8'].include?("#{fetch_os_name}-#{os[:release].to_i}")
     if ['centos-6', 'centos-7', 'oraclelinux-6', 'scientific-6', 'scientific-7'].include?("#{fetch_os_name}-#{os[:release].to_i}")
       LitmusHelper.instance.run_shell('yum update -y')
       LitmusHelper.instance.run_shell('depmod -a')
@@ -94,7 +92,7 @@ def fetch_os_name
         package { 'net-tools':
           ensure   => 'latest',
         }
-        PUPPETCODE
+      PUPPETCODE
       LitmusHelper.instance.apply_manifest(pp)
       LitmusHelper.instance.run_shell('update-alternatives --set iptables /usr/sbin/iptables-legacy', expect_failures: true)
       LitmusHelper.instance.run_shell('update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy', expect_failures: true)
@@ -115,8 +113,6 @@ def fetch_os_name
     # Ensure that policycoreutils is present. In the future we could probably refactor
     # this so that policycoreutils is installed on platform where the os.family fact
     # is set to 'redhat'
-    if ['almalinux-8', 'rocky-8'].include?("#{fetch_os_name}-#{os[:release].to_i}")
-      LitmusHelper.instance.run_shell('yum install policycoreutils -y')
-    end
+    LitmusHelper.instance.run_shell('yum install policycoreutils -y') if ['almalinux-8', 'rocky-8'].include?("#{fetch_os_name}-#{os[:release].to_i}")
   end
 end

Diff for: spec/spec_helper_local.rb

@@ -29,15 +29,53 @@
   end
 end
 
-def with_debian_facts
+shared_context 'when ArchLinux' do
   let :facts do
     {
       kernel: 'Linux',
       os: {
-        name: 'Debian',
-        release: { full: '8.0' },
+        name: 'ArchLinux',
+        family: 'ArchLinux'
+      },
+      identity: {
+        uid: 'root'
+      }
+    }
+  end
+end
+
+shared_context 'when Debian 10' do
+  let(:facts) { on_supported_os['debian-10-x86_64'] }
+end
+
+shared_context 'when Debian 11' do
+  let(:facts) { on_supported_os['debian-11-x86_64'] }
+end
+
+shared_context 'when Debian Unstable' do
+  let(:facts) do
+    {
+      kernel: 'Linux',
+      os: {
         family: 'Debian',
+        name: 'Debian',
+        release: { full: 'unstable' }
       },
+      identity: {
+        uid: 'root'
+      }
     }
   end
 end
+
+shared_context 'when Ubuntu 18.04' do
+  let(:facts) { on_supported_os['ubuntu-18.04-x86_64'] }
+end
+
+shared_context 'when RedHat 7' do
+  let(:facts) { on_supported_os['redhat-7-x86_64'] }
+end
+
+shared_context 'when RedHat 8' do
+  let(:facts) { on_supported_os['redhat-8-x86_64'] }
+end

Diff for: spec/unit/classes/firewall_linux_archlinux_spec.rb

@@ -3,23 +3,17 @@
 require 'spec_helper'
 
 describe 'firewall::linux::archlinux', type: :class do
-  let(:facts) do
-    {
-      os: {
-        family: 'ArchLinux',
-        name: 'ArchLinux',
-      },
-    }
-  end
+  include_examples 'when ArchLinux'
 
   it {
-    is_expected.to contain_service('iptables').with(
+    expect(subject).to contain_service('iptables').with(
       ensure: 'running',
       enable: 'true',
     )
   }
+
   it {
-    is_expected.to contain_service('ip6tables').with(
+    expect(subject).to contain_service('ip6tables').with(
       ensure: 'running',
       enable: 'true',
     )
@@ -29,12 +23,13 @@
     let(:params) { { ensure: 'stopped' } }
 
     it {
-      is_expected.to contain_service('iptables').with(
+      expect(subject).to contain_service('iptables').with(
         ensure: 'stopped',
       )
     }
+
     it {
-      is_expected.to contain_service('ip6tables').with(
+      expect(subject).to contain_service('ip6tables').with(
         ensure: 'stopped',
       )
     }
@@ -44,12 +39,13 @@
     let(:params) { { enable: 'false' } }
 
     it {
-      is_expected.to contain_service('iptables').with(
+      expect(subject).to contain_service('iptables').with(
         enable: 'false',
       )
     }
+
     it {
-      is_expected.to contain_service('ip6tables').with(
+      expect(subject).to contain_service('ip6tables').with(
         enable: 'false',
       )
     }