Fix auto-correctable rubocop offenses in test suite

.rubocop_todo.rspec.yml

@@ -6,41 +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: 36
-# Cop supports --auto-correct.
-# Configuration parameters: EnabledMethods.
-Capybara/FeatureMethods:
-  Exclude:
-    - 'spec/system/basic/authentication_spec.rb'
-    - 'spec/system/basic/redirects_spec.rb'
-    - 'spec/system/basic/richtext_spec.rb'
-    - 'spec/system/js/q_unit_spec.rb'
-    - 'spec/system/setup/auto_wizard_spec.rb'
-    - 'spec/system/setup/mail_accounts_spec.rb'
-    - 'spec/system/setup/system_spec.rb'
-    - 'spec/system/ticket/create_spec.rb'
-    - 'spec/system/ticket/update_spec.rb'
-
-# Offense count: 39
-# Cop supports --auto-correct.
-# Configuration parameters: AllowForAlignment, AllowBeforeTrailingComments, ForceEqualSignAlignment.
-Layout/ExtraSpacing:
-  Exclude:
-    - 'spec/factories/job.rb'
-    - 'spec/factories/postmaster_filter.rb'
-    - 'spec/factories/ticket.rb'
-    - 'spec/factories/ticket/article.rb'
-    - 'spec/lib/sequencer/unit/import/zendesk/sub_sequence/base_examples.rb'
-    - 'spec/models/calendar_spec.rb'
-    - 'spec/models/channel/email_parser_spec.rb'
-    - 'spec/models/cti/caller_id_spec.rb'
-    - 'spec/models/job_spec.rb'
-    - 'spec/models/recent_view_spec.rb'
-    - 'spec/models/role_spec.rb'
-    - 'spec/models/ticket_spec.rb'
-    - 'spec/models/trigger_spec.rb'
-    - 'spec/requests/ticket_spec.rb'
-
 # Offense count: 43
 Lint/UselessAssignment:
   Enabled: false
@@ -53,7 +18,7 @@ Lint/UselessAssignment:
 # Configuration parameters: CountComments, ExcludedMethods.
 # ExcludedMethods: refine
 Metrics/BlockLength:
-  Max: 1969
+  Max: 1987
 
 # Offense count: 16
 RSpec/AnyInstance:
@@ -84,48 +49,6 @@ RSpec/BeforeAfterAll:
 RSpec/ContextWording:
   Enabled: false
 
-# Offense count: 1
-RSpec/DescribeClass:
-  Exclude:
-    - 'spec/scripts/websocket_server_spec.rb'
-
-# Offense count: 207
-# Cop supports --auto-correct.
-# Configuration parameters: SkipBlocks, EnforcedStyle.
-# SupportedStyles: described_class, explicit
-RSpec/DescribedClass:
-  Enabled: false
-
-# Offense count: 15
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterFinalLet:
-  Exclude:
-    - 'spec/db/migrate/issue_1905_exchange_login_from_remote_id_spec.rb'
-    - 'spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb'
-    - 'spec/lib/import/zendesk/object_attribute/base_examples.rb'
-    - 'spec/models/cti/log_spec.rb'
-    - 'spec/models/object_manager/attribute/validation/future_past_spec.rb'
-    - 'spec/models/object_manager/attribute/validation/required_spec.rb'
-    - 'spec/models/ticket_spec.rb'
-    - 'spec/models/user_spec.rb'
-    - 'spec/requests/integration/twitter_webhook_spec.rb'
-
-# Offense count: 25
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterHook:
-  Exclude:
-    - 'spec/lib/notification_factory/slack_spec.rb'
-    - 'spec/lib/notification_factory_spec.rb'
-    - 'spec/models/role_spec.rb'
-    - 'spec/models/ticket_spec.rb'
-    - 'spec/models/trigger_spec.rb'
-    - 'spec/models/user_spec.rb'
-
-# Offense count: 34
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterSubject:
-  Enabled: false
-
 # Offense count: 540
 # Configuration parameters: Max.
 RSpec/ExampleLength:
@@ -144,13 +67,6 @@ RSpec/ExpectActual:
     - 'spec/requests/user/organization_spec.rb'
     - 'spec/requests/user_spec.rb'
 
-# Offense count: 99
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: method_call, block
-RSpec/ExpectChange:
-  Enabled: false
-
 # Offense count: 3
 RSpec/ExpectInHook:
   Exclude:
@@ -175,20 +91,6 @@ RSpec/FilePath:
     - 'spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb'
     - 'spec/lib/import/base_factory_spec.rb'
 
-# Offense count: 30
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: implicit, each, example
-RSpec/HookArgument:
-  Enabled: false
-
-# Offense count: 4
-# Cop supports --auto-correct.
-RSpec/HooksBeforeExamples:
-  Exclude:
-    - 'spec/models/concerns/has_groups_examples.rb'
-    - 'spec/models/trigger_spec.rb'
-
 # Offense count: 60
 # Configuration parameters: AssignmentOnly.
 RSpec/InstanceVariable:
@@ -205,36 +107,6 @@ RSpec/InstanceVariable:
     - 'spec/requests/ticket/article_attachments_spec.rb'
     - 'spec/requests/user_spec.rb'
 
-# Offense count: 2
-RSpec/IteratedExpectation:
-  Exclude:
-    - 'spec/jobs/update_cti_logs_by_caller_job_spec.rb'
-
-# Offense count: 10
-# Cop supports --auto-correct.
-RSpec/LeadingSubject:
-  Exclude:
-    - 'spec/lib/notification_factory/slack_spec.rb'
-    - 'spec/models/object_manager/attribute/validation/backend_spec.rb'
-    - 'spec/models/object_manager/attribute/validation/future_past_spec.rb'
-    - 'spec/models/object_manager/attribute/validation/required_spec.rb'
-    - 'spec/models/role_spec.rb'
-    - 'spec/models/ticket/article_spec.rb'
-    - 'spec/models/trigger_spec.rb'
-    - 'spec/models/user_spec.rb'
-
-# Offense count: 15
-# Cop supports --auto-correct.
-RSpec/LetBeforeExamples:
-  Exclude:
-    - 'spec/lib/import/otrs/article/attachment_factory_spec.rb'
-    - 'spec/lib/import/otrs/dynamic_field_factory_spec.rb'
-    - 'spec/lib/import/otrs/dynamic_field_spec.rb'
-    - 'spec/lib/ldap/group_spec.rb'
-    - 'spec/lib/ldap/user_spec.rb'
-    - 'spec/lib/ldap_spec.rb'
-    - 'spec/models/user_spec.rb'
-
 # Offense count: 34
 RSpec/LetSetup:
   Enabled: false
@@ -274,40 +146,6 @@ RSpec/NamedSubject:
 RSpec/NestedGroups:
   Max: 8
 
-# Offense count: 28
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: not_to, to_not
-RSpec/NotToNot:
-  Exclude:
-    - 'spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb'
-    - 'spec/lib/import/otrs/user_factory_spec.rb'
-    - 'spec/lib/migration_job/ldap_samaccountname_to_uid_spec.rb'
-    - 'spec/lib/report/ticket_generic_time_spec.rb'
-    - 'spec/lib/stats_spec.rb'
-    - 'spec/models/object_manager/attribute/validation_spec.rb'
-    - 'spec/models/object_manager/attribute_spec.rb'
-    - 'spec/requests/integration/idoit_spec.rb'
-    - 'spec/requests/integration/twilio_sms_spec.rb'
-    - 'spec/requests/integration/user_device_spec.rb'
-    - 'spec/requests/search_spec.rb'
-    - 'spec/requests/text_module_spec.rb'
-    - 'spec/requests/ticket/article_spec.rb'
-
-# Offense count: 81
-# Cop supports --auto-correct.
-# Configuration parameters: Strict, EnforcedStyle.
-# SupportedStyles: inflected, explicit
-RSpec/PredicateMatcher:
-  Exclude:
-    - 'spec/lib/password_hash_spec.rb'
-    - 'spec/models/trigger/sms_spec.rb'
-    - 'spec/requests/api_auth_on_behalf_of_spec.rb'
-    - 'spec/requests/api_auth_spec.rb'
-    - 'spec/requests/integration/monitoring_spec.rb'
-    - 'spec/requests/organization_spec.rb'
-    - 'spec/requests/user_spec.rb'
-
 # Offense count: 12
 RSpec/RepeatedDescription:
   Exclude:
@@ -315,11 +153,6 @@ RSpec/RepeatedDescription:
     - 'spec/requests/form_spec.rb'
     - 'spec/requests/ticket_spec.rb'
 
-# Offense count: 2
-RSpec/RepeatedExample:
-  Exclude:
-    - 'spec/models/translation_spec.rb'
-
 # Offense count: 3
 RSpec/ScatteredLet:
   Exclude:
@@ -347,16 +180,3 @@ RSpec/SubjectStub:
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Enabled: false
-
-# Offense count: 2
-RSpec/VoidExpect:
-  Exclude:
-    - 'spec/lib/ldap/group_spec.rb'
-    - 'spec/lib/ldap/user_spec.rb'
-
-# Offense count: 741
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: numeric, symbolic
-Rails/HttpStatus:
-  Enabled: false

spec/db/migrate/issue_1219_zhtw_locale_typo_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe Issue1219ZhtwLocaleTypo, type: :db_migration do
   let(:translation) { create(:translation, locale: premigrate_locale) }
   let(:user)        { create(:user, preferences: { locale: premigrate_locale }) }
 
-  before(:each) do
+  before do
     Locale.find_by(name: 'Chinese (Tradi.) (正體中文)')&.destroy
     stub_const("#{described_class}::CURRENT_VERSION", version)
   end

spec/db/migrate/issue_1905_exchange_login_from_remote_id_spec.rb

@@ -44,6 +44,7 @@ RSpec.describe Issue1905ExchangeLoginFromRemoteId, type: :db_migration do
 
     context 'blank config' do
       let(:config) { nil }
+
       it_behaves_like 'irrelevant config'
     end
 
@@ -53,6 +54,7 @@ RSpec.describe Issue1905ExchangeLoginFromRemoteId, type: :db_migration do
           some: 'config'
         }
       end
+
       it_behaves_like 'irrelevant config'
     end
 
@@ -64,6 +66,7 @@ RSpec.describe Issue1905ExchangeLoginFromRemoteId, type: :db_migration do
           }
         }
       end
+
       it_behaves_like 'irrelevant config'
     end
 
@@ -76,6 +79,7 @@ RSpec.describe Issue1905ExchangeLoginFromRemoteId, type: :db_migration do
           }
         }
       end
+
       it_behaves_like 'irrelevant config'
     end
   end

spec/db/migrate/issue_1977_remove_invalid_user_foreign_keys_spec.rb

@@ -16,9 +16,7 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do
 
         expect do
           migrate
-        end.to change {
-          OnlineNotification.count
-        }.by(-1)
+        end.to change(OnlineNotification, :count).by(-1)
       end
 
       it 'cleans up RecentView#created_by_id', db_strategy: :reset do
@@ -30,9 +28,7 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do
 
         expect do
           migrate
-        end.to change {
-          RecentView.count
-        }.by(-1)
+        end.to change(RecentView, :count).by(-1)
       end
 
       it 'cleans up Avatar#o_id', db_strategy: :reset do
@@ -44,9 +40,7 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do
 
         expect do
           migrate
-        end.to change {
-          Avatar.count
-        }.by(-1)
+        end.to change(Avatar, :count).by(-1)
       end
 
     end

spec/db/migrate/issue_2333_object_country_already_exists_spec.rb

@@ -1,23 +1,20 @@
 require 'rails_helper'
 
 RSpec.describe AddCountryAttributeToUsers, type: :db_migration do
-
   context 'AddCountryAttributeToUsers migration' do
-
-    def country_attribute
-      ObjectManager::Attribute.find_by(object_lookup_id: ObjectLookup.by_name('User'), name: 'country')
-    end
-
     it 'preserves the existing country attribute' do
       expect { migrate }
-        .not_to(change { country_attribute.present? })
+        .not_to change { ObjectManager::Attribute.find_by(object_lookup_id: ObjectLookup.by_name('User'), name: 'country') }
     end
 
-    it 'adds the country attribute when it is not present' do
-      country_attribute.delete
-      expect { migrate }
-        .to change { country_attribute.present? }
-        .from( false ).to( true )
+    context 'when country attribute is not present' do
+      before { ObjectManager::Attribute.find_by(object_lookup_id: ObjectLookup.by_name('User'), name: 'country').delete }
+
+      it 'adds the country attribute when it is not present' do
+        expect { migrate }
+          .to change { ObjectManager::Attribute.exists?(object_lookup_id: ObjectLookup.by_name('User'), name: 'country') }
+          .from(false).to(true)
+      end
     end
   end
 end

spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb

@@ -38,6 +38,7 @@ RSpec.describe Issue2541FixNotificationEmailWithoutBody, type: :db_migration do
 
     context 'when migrating Jobs' do
       subject(:job) { create(:job) }
+
       let(:type) { 'notification.email' }
 
       it "updates empty perform['notification.email']['body'] attribute" do
@@ -48,6 +49,7 @@ RSpec.describe Issue2541FixNotificationEmailWithoutBody, type: :db_migration do
 
   describe 'scheduler management' do
     let(:scheduler) { Scheduler.find_by(method: 'Job.run') }
+
     before { scheduler.update!(active: false) }
 
     it "re-enables 'Job.run' Scheduler" do

spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe ObjectManagerAttributeDateRemoveFuturePast, type: :db_migration d
 
       migrate
 
-      expect(subject.data_option).to_not include(:past, :future)
+      expect(subject.data_option).not_to include(:past, :future)
     end
 
     context 'when incomplete data_option is given' do

spec/jobs/update_cti_logs_by_caller_job_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe UpdateCtiLogsByCallerJob, type: :job do
     it 'updates Cti::Logs from that number with "preferences" => {}' do
       described_class.perform_now(phone)
 
-      log_prefs.each { |p| expect(p).to be_empty }
+      expect(log_prefs).to all(be_empty)
     end
   end
 
@@ -24,7 +24,7 @@ RSpec.describe UpdateCtiLogsByCallerJob, type: :job do
     it 'updates Cti::Logs from that number with valid "preferences" hash' do
       described_class.perform_now(phone)
 
-      log_prefs.each { |p| expect(p).to include('from' => a_kind_of(Array)) }
+      expect(log_prefs).to all(include('from' => a_kind_of(Array)))
     end
   end
 end

spec/lib/application_handle_info_spec.rb

@@ -3,30 +3,30 @@ require 'rails_helper'
 RSpec.describe ApplicationHandleInfo do
   describe '.use' do
     it 'requires a block' do
-      expect { ApplicationHandleInfo.use('foo') }
+      expect { described_class.use('foo') }
         .to raise_error(ArgumentError)
     end
 
     context 'for a given starting ApplicationHandleInfo' do
-      before { ApplicationHandleInfo.current = 'foo' }
+      before { described_class.current = 'foo' }
 
       it 'runs the block using the given ApplicationHandleInfo' do
-        ApplicationHandleInfo.use('bar') do
-          expect(ApplicationHandleInfo.current).to eq('bar')
+        described_class.use('bar') do
+          expect(described_class.current).to eq('bar')
         end
       end
 
       it 'resets ApplicationHandleInfo to its original value' do
-        ApplicationHandleInfo.use('bar') {}
+        described_class.use('bar') {}
 
-        expect(ApplicationHandleInfo.current).to eq('foo')
+        expect(described_class.current).to eq('foo')
       end
 
       context 'when an error is raised in the given block' do
         it 'does not rescue the error, and still resets ApplicationHandleInfo' do
-          expect { ApplicationHandleInfo.use('bar') { raise } }
+          expect { described_class.use('bar') { raise } }
             .to raise_error(StandardError)
-            .and not_change { ApplicationHandleInfo.current }
+            .and not_change { described_class.current }
         end
       end
     end

spec/lib/cache_spec.rb

@@ -5,14 +5,14 @@ RSpec.describe Cache do
     before { allow(Rails.cache).to receive(:read) }
 
     it 'wraps Rails.cache.read' do
-      Cache.get('foo')
+      described_class.get('foo')
 
       expect(Rails.cache).to have_received(:read).with('foo')
     end
 
     context 'with a non-string argument' do
       it 'passes a string' do
-        Cache.get(:foo)
+        described_class.get(:foo)
 
         expect(Rails.cache).to have_received(:read).with('foo')
       end
@@ -21,25 +21,25 @@ RSpec.describe Cache do
 
   describe '.write' do
     it 'stores string values' do
-      expect { Cache.write('123', 'some value') }
-        .to change { Cache.get('123') }.to('some value')
+      expect { described_class.write('123', 'some value') }
+        .to change { described_class.get('123') }.to('some value')
     end
 
     it 'stores hash values' do
-      expect { Cache.write('123', { key: 'some value' }) }
-        .to change { Cache.get('123') }.to({ key: 'some value' })
+      expect { described_class.write('123', { key: 'some value' }) }
+        .to change { described_class.get('123') }.to({ key: 'some value' })
     end
 
     it 'overwrites previous values' do
-      Cache.write('123', 'some value')
+      described_class.write('123', 'some value')
 
-      expect { Cache.write('123', { key: 'some value' }) }
-        .to change { Cache.get('123') }.to({ key: 'some value' })
+      expect { described_class.write('123', { key: 'some value' }) }
+        .to change { described_class.get('123') }.to({ key: 'some value' })
     end
 
     it 'stores hash values with non-ASCII content' do
-      expect { Cache.write('123', { key: 'some valueöäüß' }) }
-        .to change { Cache.get('123') }.to({ key: 'some valueöäüß' })
+      expect { described_class.write('123', { key: 'some valueöäüß' }) }
+        .to change { described_class.get('123') }.to({ key: 'some valueöäüß' })
     end
 
     context 'when expiring' do
@@ -51,58 +51,58 @@ RSpec.describe Cache do
       end
 
       it 'defaults to expires_in: 7.days' do
-        Cache.write('123', 'some value')
+        described_class.write('123', 'some value')
 
-        expect { travel 7.days - 1.second }.not_to change { Cache.get('123') }
-        expect { travel 2.seconds }.to change { Cache.get('123') }.to(nil)
+        expect { travel 7.days - 1.second }.not_to change { described_class.get('123') }
+        expect { travel 2.seconds }.to change { described_class.get('123') }.to(nil)
       end
 
       it 'accepts a custom :expires_in option' do
-        Cache.write('123', 'some value', expires_in: 3.seconds)
+        described_class.write('123', 'some value', expires_in: 3.seconds)
 
-        expect { travel 4.seconds }.to change { Cache.get('123') }.to(nil)
+        expect { travel 4.seconds }.to change { described_class.get('123') }.to(nil)
       end
     end
   end
 
   describe '.delete' do
     it 'deletes stored values' do
-      Cache.write('123', 'some value')
+      described_class.write('123', 'some value')
 
-      expect { Cache.delete('123') }
-        .to change { Cache.get('123') }.to(nil)
+      expect { described_class.delete('123') }
+        .to change { described_class.get('123') }.to(nil)
     end
 
     it 'is idempotent' do
-      Cache.write('123', 'some value')
-      Cache.delete('123')
+      described_class.write('123', 'some value')
+      described_class.delete('123')
 
-      expect { Cache.delete('123') }.not_to raise_error
+      expect { described_class.delete('123') }.not_to raise_error
     end
   end
 
   describe '.clear' do
     it 'deletes all stored values' do
-      Cache.write('123', 'some value')
-      Cache.write('456', 'some value')
+      described_class.write('123', 'some value')
+      described_class.write('456', 'some value')
 
-      expect { Cache.clear }
-        .to change { Cache.get('123') }.to(nil)
-        .and change { Cache.get('456') }.to(nil)
+      expect { described_class.clear }
+        .to change { described_class.get('123') }.to(nil)
+        .and change { described_class.get('456') }.to(nil)
     end
 
     it 'is idempotent' do
-      Cache.write('123', 'some value')
-      Cache.clear
+      described_class.write('123', 'some value')
+      described_class.clear
 
-      expect { Cache.clear }.not_to raise_error
+      expect { described_class.clear }.not_to raise_error
     end
 
     context 'when cache directory is not present on disk' do
       before { FileUtils.rm_rf(Rails.cache.cache_path) }
 
       it 'does not raise an error' do
-        expect { Cache.clear }.not_to raise_error
+        expect { described_class.clear }.not_to raise_error
       end
     end
   end