Addressing Rubocop offenses

Author: LukasAud

.rubocop_todo.yml

@@ -1,30 +1,30 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2023-04-28 12:30:24 UTC using RuboCop version 1.48.1.
+# on 2024-01-17 13:31:29 UTC using RuboCop version 1.50.0.
 # 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: 12
+# Offense count: 13
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
-  Max: 94
+  Max: 92
 
-# Offense count: 1
+# Offense count: 2
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 200
+  Max: 149
 
-# Offense count: 9
+# Offense count: 8
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/CyclomaticComplexity:
-  Max: 25
+  Max: 23
 
-# Offense count: 19
+# Offense count: 20
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
-  Max: 69
+  Max: 70
 
 # Offense count: 3
 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters.
@@ -34,7 +34,7 @@ Metrics/ParameterLists:
 # Offense count: 7
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/PerceivedComplexity:
-  Max: 30
+  Max: 25
 
 # Offense count: 2
 # Configuration parameters: IgnoredMetadata.
@@ -48,30 +48,50 @@ RSpec/DescribeClass:
     - 'spec/tasks/abs_spec.rb'
     - 'spec/unit/task_helper_spec.rb'
 
-# Offense count: 4
+# Offense count: 12
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
-  Max: 30
+  Max: 27
 
-# Offense count: 6
+# Offense count: 13
 RSpec/MultipleExpectations:
   Max: 13
 
-# Offense count: 2
+# Offense count: 6
 # Configuration parameters: AllowSubject.
 RSpec/MultipleMemoizedHelpers:
   Max: 6
 
-# Offense count: 4
+# Offense count: 6
 RSpec/StubbedMock:
   Exclude:
     - 'spec/tasks/abs_spec.rb'
 
-# Offense count: 8
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+Style/CollectionCompact:
+  Exclude:
+    - 'tasks/vagrant.rb'
+
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: nonzero?
+Style/IfWithBooleanLiteralBranches:
+  Exclude:
+    - 'tasks/vagrant.rb'
+
+# Offense count: 7
 Style/MixinUsage:
   Exclude:
     - 'spec/spec_helper.rb'
     - 'tasks/docker.rb'
     - 'tasks/docker_exp.rb'
-    - 'tasks/provision_service.rb'
     - 'tasks/vagrant.rb'
+
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: Methods.
+Style/RedundantArgument:
+  Exclude:
+    - 'tasks/update_node_pp.rb'

tasks/abs.rb

@@ -26,11 +26,11 @@ def abs_host
   def provision(platform, inventory_location, vars)
     uri = URI.parse("https://#{abs_host}/api/v2/request")
     jenkins_build_url = if ENV['CI'] == 'true' && ENV['TRAVIS'] == 'true'
-                          ENV['TRAVIS_JOB_WEB_URL']
+                          ENV.fetch('TRAVIS_JOB_WEB_URL', nil)
                         elsif ENV['CI'] == 'True' && ENV['APPVEYOR'] == 'True'
-                          "https://ci.appveyor.com/project/#{ENV['APPVEYOR_REPO_NAME']}/build/job/#{ENV['APPVEYOR_JOB_ID']}"
+                          "https://ci.appveyor.com/project/#{ENV.fetch('APPVEYOR_REPO_NAME', nil)}/build/job/#{ENV.fetch('APPVEYOR_JOB_ID', nil)}"
                         elsif ENV['GITHUB_ACTIONS'] == 'true'
-                          "https://github.com/#{ENV['GITHUB_REPOSITORY']}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
+                          "https://github.com/#{ENV.fetch('GITHUB_REPOSITORY', nil)}/actions/runs/#{ENV.fetch('GITHUB_RUN_ID', nil)}"
                         else
                           'https://litmus_manual'
                         end
@@ -91,17 +91,18 @@ def provision(platform, inventory_location, vars)
     data.each do |host|
       if platform_uses_ssh(host['type'])
         node = { 'uri' => host['hostname'],
-                 'config' => { 'transport' => 'ssh', 'ssh' => { 'user' => ENV['ABS_USER'], 'host-key-check' => false, 'connect-timeout' => 120 } },
+                 'config' => { 'transport' => 'ssh', 'ssh' => { 'user' => ENV.fetch('ABS_USER', nil), 'host-key-check' => false, 'connect-timeout' => 120 } },
                  'facts' => { 'provisioner' => 'abs', 'platform' => host['type'], 'job_id' => job_id } }
         if !ENV['ABS_SSH_PRIVATE_KEY'].nil? && !ENV['ABS_SSH_PRIVATE_KEY'].empty?
-          node['config']['ssh']['private-key'] = ENV['ABS_SSH_PRIVATE_KEY']
+          node['config']['ssh']['private-key'] = ENV.fetch('ABS_SSH_PRIVATE_KEY', nil)
         else
-          node['config']['ssh']['password'] = ENV['ABS_PASSWORD']
+          node['config']['ssh']['password'] = ENV.fetch('ABS_PASSWORD', nil)
         end
         group_name = 'ssh_nodes'
       else
         node = { 'uri' => host['hostname'],
-                 'config' => { 'transport' => 'winrm', 'winrm' => { 'user' => ENV['ABS_WIN_USER'], 'password' => ENV['ABS_PASSWORD'], 'ssl' => false, 'connect-timeout' => 120 } },
+                 'config' => { 'transport' => 'winrm',
+                               'winrm' => { 'user' => ENV.fetch('ABS_WIN_USER', nil), 'password' => ENV.fetch('ABS_PASSWORD', nil), 'ssl' => false, 'connect-timeout' => 120 } },
                  'facts' => { 'provisioner' => 'abs', 'platform' => host['type'], 'job_id' => job_id } }
         group_name = 'winrm_nodes'
       end
@@ -112,7 +113,7 @@ def provision(platform, inventory_location, vars)
       add_node_to_group(inventory_hash, node, group_name)
     end
 
-    File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+    File.write(inventory_full_path, inventory_hash.to_yaml)
     { status: 'ok', nodes: data.length }
   end
 
@@ -146,7 +147,7 @@ def tear_down(node_name, inventory_location)
     targets_to_remove.each do |target|
       remove_node(inventory_hash, target)
     end
-    File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+    File.write(inventory_full_path, inventory_hash.to_yaml)
     { status: 'ok', removed: targets_to_remove }
   end
 

tasks/docker.rb

@@ -23,16 +23,16 @@ def install_ssh_components(distro, version, container)
       # sometimes the redhat 6 variant containers like to eat their rpmdb, leading to
       # issues with "rpmdb: unable to join the environment" errors
       # This "fix" is from https://www.srv24x7.com/criticalyum-main-error-rpmdb-open-failed/
-      run_local_command("docker exec #{container} bash -exc \"rm -f /var/lib/rpm/__db*; "\
-        'db_verify /var/lib/rpm/Packages; '\
-        'rpm --rebuilddb; '\
-        'yum clean all; '\
-        'yum install -y sudo openssh-server openssh-clients"')
+      run_local_command("docker exec #{container} bash -exc \"rm -f /var/lib/rpm/__db*; " \
+                        'db_verify /var/lib/rpm/Packages; ' \
+                        'rpm --rebuilddb; ' \
+                        'yum clean all; ' \
+                        'yum install -y sudo openssh-server openssh-clients"')
     else
       # If systemd is running for init, ensure systemd has finished starting up before proceeding:
-      check_init_cmd = 'if [[ "$(readlink /proc/1/exe)" == "/usr/lib/systemd/systemd" ]]; then '\
-            'count=0 ; while ! [[ "$(systemctl is-system-running)" =~ ^running|degraded$ && $count > 20 ]]; '\
-            'do sleep 0.1 ; count=$((count+1)) ; done ; fi'
+      check_init_cmd = 'if [[ "$(readlink /proc/1/exe)" == "/usr/lib/systemd/systemd" ]]; then ' \
+                       'count=0 ; while ! [[ "$(systemctl is-system-running)" =~ ^running|degraded$ && $count > 20 ]]; ' \
+                       'do sleep 0.1 ; count=$((count+1)) ; done ; fi'
       run_local_command("docker exec #{container} bash -c '#{check_init_cmd}'")
       run_local_command("docker exec #{container} yum install -y sudo openssh-server openssh-clients")
     end
@@ -74,10 +74,10 @@ def fix_ssh(distro, version, container)
     # https://bugzilla.redhat.com/show_bug.cgi?id=1728777
     run_local_command("docker exec #{container} sed 's@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g' -i /etc/pam.d/sshd") if distro =~ %r{redhat|centos} && version =~ %r{^7}
 
-    if !%r{^(7|8|9|2)}.match?(version)
-      run_local_command("docker exec #{container} service sshd restart")
-    else
+    if %r{^(7|8|9|2)}.match?(version)
       run_local_command("docker exec #{container} /usr/sbin/sshd")
+    else
+      run_local_command("docker exec #{container} service sshd restart")
     end
   when %r{sles}
     run_local_command("docker exec #{container} /usr/sbin/sshd")
@@ -161,7 +161,7 @@ def provision(image, inventory_location, vars)
   distro = os_release_facts['ID']
   version = os_release_facts['VERSION_ID']
 
-  hostname = (ENV['DOCKER_HOST'].nil? || ENV['DOCKER_HOST'].empty?) ? 'localhost' : URI.parse(ENV['DOCKER_HOST']).host || ENV['DOCKER_HOST']
+  hostname = (ENV['DOCKER_HOST'].nil? || ENV['DOCKER_HOST'].empty?) ? 'localhost' : URI.parse(ENV.fetch('DOCKER_HOST', nil)).host || ENV.fetch('DOCKER_HOST', nil)
   begin
     # Use the current docker context to determine the docker hostname
     docker_context = JSON.parse(run_local_command('docker context inspect'))[0]
@@ -207,7 +207,7 @@ def provision(image, inventory_location, vars)
   install_ssh_components(distro, version, full_container_name)
   fix_ssh(distro, version, full_container_name)
   add_node_to_group(inventory_hash, node, group_name)
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok', node_name: "#{hostname}:#{front_facing_port}", node: node }
 end
 
@@ -222,7 +222,7 @@ def tear_down(node_name, inventory_location)
   run_local_command(remove_docker)
   remove_node(inventory_hash, node_name)
   puts "Removed #{node_name}"
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok' }
 end
 

tasks/docker_exp.rb

@@ -38,7 +38,7 @@ def provision(docker_platform, inventory_location, vars)
 
   group_name = 'docker_nodes'
   add_node_to_group(inventory_hash, node, group_name)
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok', node_name: container_id, node: node }
 end
 
@@ -53,7 +53,7 @@ def tear_down(node_name, inventory_location)
   run_local_command(remove_docker)
   remove_node(inventory_hash, node_name)
   puts "Removed #{node_name}"
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok' }
 end
 

tasks/provision_service.rb

@@ -46,7 +46,7 @@ def invoke_cloud_request(params, uri, job_url, verb, retry_attempts)
       request.body = if job_url
                        { url: job_url, VMs: machines }.to_json
                      else
-                       { github_token: ENV['GITHUB_TOKEN'], VMs: machines }.to_json
+                       { github_token: ENV.fetch('GITHUB_TOKEN', nil), VMs: machines }.to_json
                      end
     when 'delete'
       request = Net::HTTP::Delete.new(uri, headers)
@@ -55,11 +55,7 @@ def invoke_cloud_request(params, uri, job_url, verb, retry_attempts)
       raise StandardError "Unknown verb: '#{verb}'"
     end
 
-    if job_url
-      File.open('request.json', 'wb') do |f|
-        f.write(request.body)
-      end
-    end
+    File.binwrite('request.json', request.body) if job_url
 
     req_options = {
       use_ssl: uri.scheme == 'https',
@@ -89,14 +85,14 @@ def provision(platform, inventory_location, vars, retry_attempts)
     # Call the provision service with the information necessary and write the inventory file locally
 
     if ENV['GITHUB_RUN_ID']
-      job_url = ENV['GITHUB_URL'] || "https://api.github.com/repos/#{ENV['GITHUB_REPOSITORY']}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
+      job_url = ENV['GITHUB_URL'] || "https://api.github.com/repos/#{ENV.fetch('GITHUB_REPOSITORY', nil)}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
     else
       puts 'Using GITHUB_TOKEN as no GITHHUB_RUN_ID found'
     end
     uri = URI.parse(ENV['SERVICE_URL'] || default_uri)
-    cloud = ENV['CLOUD']
-    region = ENV['REGION']
-    zone = ENV['ZONE']
+    cloud = ENV.fetch('CLOUD', nil)
+    region = ENV.fetch('REGION', nil)
+    zone = ENV.fetch('ZONE', nil)
     if job_url.nil? && vars
       data = JSON.parse(vars.tr(';', ','))
       job_url = data['job_url']
@@ -134,12 +130,10 @@ def provision(platform, inventory_location, vars, retry_attempts)
           g['targets'] = g['targets'] + bg['targets'] if g['name'] == bg['name']
         end
       end
-      File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+      File.write(inventory_full_path, inventory_hash.to_yaml)
     else
       FileUtils.mkdir_p(File.join(Dir.pwd, '/spec/fixtures'))
-      File.open(inventory_full_path, 'wb') do |f|
-        f.write(YAML.dump(response_hash))
-      end
+      File.binwrite(inventory_full_path, YAML.dump(response_hash))
     end
 
     {

tasks/update_node_pp.rb

@@ -21,7 +21,7 @@ def update_file(manifest, target_node)
   else
     final_manifest = "node '#{target_node}' \n{\n  #{manifest} \n}"
   end
-  File.open(site_path, 'w+') { |f| f.write(final_manifest) }
+  File.write(site_path, final_manifest)
   "#{site_path} updated"
 end
 

tasks/update_site_pp.rb

@@ -11,7 +11,7 @@ def update_file(manifest)
   raise Puppet::Error, "stderr: ' %{stderr}')" % { stderr: stderr } if status != 0
 
   site_path = File.join(path, 'site.pp')
-  File.open(site_path, 'w+') { |f| f.write(manifest) }
+  File.write(site_path, manifest)
   'site.pp updated'
 end
 

tasks/vagrant.rb

@@ -54,7 +54,7 @@ def generate_vagrantfile(file_path, platform, enable_synced_folder, provider, cp
                      ''
                    end
   vf = <<~VF
-    Vagrant.configure(\"2\") do |config|
+    Vagrant.configure("2") do |config|
       config.vm.box = '#{platform}'
       config.vm.boot_timeout = 600
       config.ssh.insert_key = false
@@ -64,9 +64,7 @@ def generate_vagrantfile(file_path, platform, enable_synced_folder, provider, cp
       #{provider_config_block}
     end
   VF
-  File.open(file_path, 'w') do |f|
-    f.write(vf)
-  end
+  File.write(file_path, vf)
 end
 
 def get_vagrant_dir(platform, vagrant_dirs, int = 0)
@@ -187,7 +185,7 @@ def provision(platform, inventory_location, enable_synced_folder, provider, cpus
     group_name = 'winrm_nodes'
   end
   add_node_to_group(inventory_hash, node, group_name)
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok', node_name: node_name, node: node }
 end
 
@@ -203,7 +201,7 @@ def tear_down(node_name, inventory_location)
     FileUtils.rm_r(vagrant_env)
   end
   warn "Removed #{node_name}"
-  File.open(inventory_full_path, 'w') { |f| f.write inventory_hash.to_yaml }
+  File.write(inventory_full_path, inventory_hash.to_yaml)
   { status: 'ok' }
 end
 
@@ -213,18 +211,18 @@ def tear_down(node_name, inventory_location)
 action = params['action']
 node_name = params['node_name']
 inventory_location = sanitise_inventory_location(params['inventory'])
-enable_synced_folder = params['enable_synced_folder'].nil? ? ENV['VAGRANT_ENABLE_SYNCED_FOLDER'] : params['enable_synced_folder']
+enable_synced_folder = params['enable_synced_folder'].nil? ? ENV.fetch('VAGRANT_ENABLE_SYNCED_FOLDER', nil) : params['enable_synced_folder']
 if enable_synced_folder.is_a?(String)
   enable_synced_folder = enable_synced_folder.casecmp('true').zero? ? true : false
 end
-provider            = params['provider'].nil? ? ENV['VAGRANT_PROVIDER'] : params['provider']
-cpus                = params['cpus'].nil? ? ENV['VAGRANT_CPUS'] : params['cpus']
-memory              = params['memory'].nil? ? ENV['VAGRANT_MEMORY'] : params['memory']
-hyperv_vswitch      = params['hyperv_vswitch'].nil? ? ENV['VAGRANT_HYPERV_VSWITCH'] : params['hyperv_vswitch']
-hyperv_smb_username = params['hyperv_smb_username'].nil? ? ENV['VAGRANT_HYPERV_SMB_USERNAME'] : params['hyperv_smb_username']
-hyperv_smb_password = params['hyperv_smb_password'].nil? ? ENV['VAGRANT_HYPERV_SMB_PASSWORD'] : params['hyperv_smb_password']
-box_url             = params['box_url'].nil? ? ENV['VAGRANT_BOX_URL'] : params['box_url']
-password            = params['password'].nil? ? ENV['VAGRANT_PASSWORD'] : params['password']
+provider            = params['provider'].nil? ? ENV.fetch('VAGRANT_PROVIDER', nil) : params['provider']
+cpus                = params['cpus'].nil? ? ENV.fetch('VAGRANT_CPUS', nil) : params['cpus']
+memory              = params['memory'].nil? ? ENV.fetch('VAGRANT_MEMORY', nil) : params['memory']
+hyperv_vswitch      = params['hyperv_vswitch'].nil? ? ENV.fetch('VAGRANT_HYPERV_VSWITCH', nil) : params['hyperv_vswitch']
+hyperv_smb_username = params['hyperv_smb_username'].nil? ? ENV.fetch('VAGRANT_HYPERV_SMB_USERNAME', nil) : params['hyperv_smb_username']
+hyperv_smb_password = params['hyperv_smb_password'].nil? ? ENV.fetch('VAGRANT_HYPERV_SMB_PASSWORD', nil) : params['hyperv_smb_password']
+box_url             = params['box_url'].nil? ? ENV.fetch('VAGRANT_BOX_URL', nil) : params['box_url']
+password            = params['password'].nil? ? ENV.fetch('VAGRANT_PASSWORD', nil) : params['password']
 raise 'specify a node_name when tearing down' if action == 'tear_down' && node_name.nil?
 raise 'specify a platform when provisioning' if action == 'provision' && platform.nil?