Fix some RuboCop rules (#848)

* Fix Rails/FindEach cop

find_each loads records in batches of 1000, which means it consumes less
memory than loading everything at once.

* Fix Rails/TimeZoneAssignment cop

Setting the time zone as a block is now built into the library, so the
custom implementation is no longer needed.

* Fix Minitest/TestFileName cop

Test files need to have the _test suffix, else they're not executed.

* Add required_ruby_version to gemspec

* Fix Style/SlicingWithRange cop

In Ruby 2.6+ this is possible.

.rubocop.yml

@@ -10,7 +10,7 @@ inherit_mode:
     - Exclude
 
 AllCops:
-  TargetRubyVersion: 2.5
+  TargetRubyVersion: 2.7
   TargetRailsVersion: 5.2
   Exclude:
     - 'node_modules/**/*'

.rubocop_todo.yml

@@ -6,14 +6,6 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 2
-# Configuration parameters: Include.
-# Include: **/*.gemspec
-Gemspec/RequiredRubyVersion:
-  Exclude:
-    - 'foreman_remote_execution.gemspec'
-    - 'foreman_remote_execution_core.gemspec'
-
 # Offense count: 1
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyleAlignWith, Severity.

db/migrate/20151215114631_add_host_id_to_template_invocation.rb

@@ -9,7 +9,7 @@ def up
     FakeTemplateInvocation.reset_column_information
 
     say 'Migrating existing execution locks to explicit relations, this may take a while'
-    FakeTemplateInvocation.all.each do |template_invocation|
+    FakeTemplateInvocation.all.find_each do |template_invocation|
       task = ForemanTasks::Task.for_action_types('Actions::RemoteExecution::RunHostJob').joins(:locks).where(
         :'foreman_tasks_locks.resource_type' => 'TemplateInvocation',
         :'foreman_tasks_locks.resource_id' => template_invocation.id

db/migrate/20151217092555_migrate_to_task_groups.rb

@@ -5,7 +5,7 @@ class FakeJobInvocation < ApplicationRecord
 
   def up
     say 'Migrating from locks to task groups'
-    FakeJobInvocation.where('task_group_id IS NULL AND task_id IS NOT NULL').each do |job_invocation|
+    FakeJobInvocation.where('task_group_id IS NULL AND task_id IS NOT NULL').find_each do |job_invocation|
       task_group = JobInvocationTaskGroup.new
       task_group.task_ids = [job_invocation.task_id]
       task_group.save!

db/migrate/20160113162007_expand_all_template_invocations.rb

@@ -14,7 +14,7 @@ def up
     FakeTemplateInvocation.update_all 'host_id = NULL'
 
     # expand all pattern template invocations and link RunHostJob
-    JobInvocation.joins(:targeting).where("#{Targeting.table_name}.resolved_at IS NOT NULL").includes([:pattern_template_invocations, :targeting]).each do |job_invocation|
+    JobInvocation.joins(:targeting).where("#{Targeting.table_name}.resolved_at IS NOT NULL").includes([:pattern_template_invocations, :targeting]).find_each do |job_invocation|
       job_invocation.pattern_template_invocations.each do |pattern_template_invocation|
         job_invocation.targeting.hosts.each do |host|
           task = job_invocation.sub_tasks.find do |sub_task|
@@ -38,7 +38,7 @@ def up
 
   def down
     # we can't determine the last host for pattern, but keeping template invocations as pattern is safe
-    JobInvocation.joins(:targeting).where("#{Targeting.table_name}.resolved_at IS NOT NULL").includes(:template_invocations).each do |job_invocation|
+    JobInvocation.joins(:targeting).where("#{Targeting.table_name}.resolved_at IS NOT NULL").includes(:template_invocations).find_each do |job_invocation|
       job_invocation.template_invocations.delete_all
     end
   end

db/migrate/20160114125628_rename_job_name_to_job_category.rb

@@ -3,13 +3,13 @@ def up
     rename_column :templates, :job_name, :job_category
     rename_column :job_invocations, :job_name, :job_category
     JobTemplate.where(:description_format => '%{job_name} %{command}').update_all(:description_format => 'Run %{command}')
-    JobTemplate.where("description_format LIKE '%\%{job_name}%'").each do |template|
+    JobTemplate.where("description_format LIKE '%\%{job_name}%'").find_each do |template|
       JobTemplate.where(:id => template.id).update_all(:description_format => template.description_format.gsub('%{job_name}', '%{job_category}'))
     end
   end
 
   def down
-    JobTemplate.where("description_format LIKE '%\%{job_category}%'").each do |template|
+    JobTemplate.where("description_format LIKE '%\%{job_category}%'").find_each do |template|
       JobTemplate.where(:id => template.id).update_all(:description_format => template.description_format.gsub('%{job_category}', '%{job_name}'))
     end
     JobTemplate.where(:description_format => 'Run %{command}').update_all(:description_format => '%{job_name} %{command}')

db/migrate/20200623073022_rename_sudo_password_to_effective_user_password.rb

@@ -2,7 +2,7 @@ class RenameSudoPasswordToEffectiveUserPassword < ActiveRecord::Migration[6.0]
   def up
     rename_column :job_invocations, :sudo_password, :effective_user_password
 
-    Parameter.where(name: 'remote_execution_sudo_password').each do |parameter|
+    Parameter.where(name: 'remote_execution_sudo_password').find_each do |parameter|
       record = Parameter.find_by(type: parameter.type, reference_id: parameter.reference_id, name: "remote_execution_effective_user_password")
       if record.nil?
         parameter.update(name: "remote_execution_effective_user_password")
@@ -19,7 +19,7 @@ def up
   def down
     rename_column :job_invocations, :effective_user_password, :sudo_password
 
-    Parameter.where(name: 'remote_execution_effective_user_password').each do |parameter|
+    Parameter.where(name: 'remote_execution_effective_user_password').find_each do |parameter|
       record = Parameter.find_by(type: parameter.type, reference_id: parameter.reference_id, name: "remote_execution_sudo_password")
       if record.nil?
         parameter.update(name: "remote_execution_sudo_password")

db/migrate/20220321101835_rename_ssh_provider_to_script.rb

@@ -6,7 +6,7 @@ def do_change(from, to, from_re, new_label)
     setting = Setting.find_by(:name => 'remote_execution_form_job_template')
     default_template = nil
 
-    Template.where(:provider_type => from).each do |t|
+    Template.where(:provider_type => from).find_each do |t|
       default_template = t if t.name == setting&.value
       t.provider_type = to
       t.name = t.name.gsub(from_re, new_label)

extra/cockpit/foreman-cockpit-session

@@ -124,7 +124,7 @@ class ProxyBuffer
 
   def write_available!
     count = @dst_io.write_nonblock(@buffer)
-    @buffer = @buffer[count..-1]
+    @buffer = @buffer[count..]
   rescue IO::WaitWritable
     0
   rescue IO::WaitReadable

foreman_remote_execution.gemspec

@@ -22,6 +22,8 @@ Gem::Specification.new do |s|
   s.test_files =       `git ls-files test`.split("\n")
   s.extra_rdoc_files = Dir['README*', 'LICENSE']
 
+  s.required_ruby_version = '>= 2.7', '< 4'
+
   s.add_dependency 'deface'
   s.add_dependency 'dynflow', '>= 1.0.2', '< 2.0.0'
   s.add_dependency 'foreman-tasks', '>= 8.3.0'

test/unit/api_params_test.rb

@@ -11,23 +11,15 @@ class ApiParamsTest < ActiveSupport::TestCase
     end
 
     it 'honors explicitly supplied time zone' do
-      in_time_zone(ActiveSupport::TimeZone['America/New_York']) do
+      Time.use_zone(ActiveSupport::TimeZone['America/New_York']) do
         assert_equal '2022-07-08 08:53', params.send(:format_datetime, '2022-07-08 12:53:20 UTC')
       end
     end
 
     it 'implicitly honors current user\'s time zone' do
-      in_time_zone(ActiveSupport::TimeZone['America/New_York']) do
+      Time.use_zone(ActiveSupport::TimeZone['America/New_York']) do
         assert_equal '2022-07-08 12:53', params.send(:format_datetime, '2022-07-08 12:53:20')
       end
     end
   end
-
-  def in_time_zone(zone)
-    old_tz = Time.zone
-    Time.zone = zone
-    yield
-  ensure
-    Time.zone = old_tz
-  end
 end