Configure RSpec to fail on potential false positives

Previously RSpec only warned in the following cases:

   expect(foo).to raise_error

   expect(foo).not_to raise_error(SomeSpecifcError)

and suggested to correct potential false positives with:

   expect(foo).to raise_error(SomeSpecifcError)

   expect(foo).not_to raise_error

This commit corrects all warnings and make RSpec fail instead of warn in
such situations preventing future false positives.

ee/spec/lib/ee/api/helpers_spec.rb

@@ -73,11 +73,15 @@ def app
     end
     context 'unauthorized param' do
+      let(:exception) { Exception.new('Forbidden') }
+
       before do
-        allow(subject).to receive(:authorize!).and_raise(Exception.new("Forbidden"))
+        allow(subject).to receive(:authorize!).and_raise(exception)
       end
+
       it 'throws exception if unauthorized param is present' do
-        expect { subject.authorize_change_param(project, :change_commit_committer_check) }.to raise_error
+        expect { subject.authorize_change_param(project, :change_commit_committer_check) }
+          .to raise_error(exception)
       end
       it 'does not throw exception is unauthorized param is not present' do

ee/spec/lib/gitlab/analytics/cycle_analytics/data_collector_spec.rb

@@ -347,7 +347,7 @@ def create_data_for_end_event(issue, example_class)
             subject { described_class.new(stage: stage, params: params) }
             it 'does not raise query syntax error' do
-              expect { subject.records_fetcher.serialized_records }.not_to raise_error(ActiveRecord::StatementInvalid)
+              expect { subject.records_fetcher.serialized_records }.not_to raise_error
             end
           end
         end

ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb

@@ -30,7 +30,7 @@
       expect(::Gitlab::IpAddressState).to receive(:nullify_address)
       expect(app).to receive(:call).and_raise('boom')
-      expect { middleware.call(env) }.to raise_error
+      expect { middleware.call(env) }.to raise_error('boom')
     end
     context 'when it is internal endpoint' do

ee/spec/models/ee/iteration_spec.rb

@@ -417,8 +417,8 @@
         it "does not raise an error on creation with a unique sequence number within a cadence", :aggregate_failures do
           expect do
-            create(:iteration, :with_due_date, sequence: 2, iterations_cadence: iterations_cadence1, start_date: 2.weeks.from_now)
-          end.not_to raise_error(ActiveRecord::RecordNotUnique)
+            create(:iteration, :with_due_date, sequence: 2, iterations_cadence: iterations_cadence, start_date: 2.weeks.from_now)
+          end.not_to raise_error
         end
       end
@@ -481,7 +481,7 @@
               iteration1.update_columns(dates2)
               iteration2.update_columns(dates1)
             end
-          end.not_to raise_exception(ActiveRecord::StatementInvalid, /PG::ExclusionViolation/)
+          end.not_to raise_exception
         end
       end
     end

ee/spec/services/audit_event_service_spec.rb

@@ -672,10 +672,8 @@ def disable_license_audit_features
       context 'when the event is created within a transaction' do
         it 'does not raise an error about a job being enqueued from within a transaction' do
-          RSpec::Expectations.configuration.on_potential_false_positives = :nothing
-
           ApplicationRecord.transaction do
-            expect { event }.not_to raise_error(Sidekiq::Worker::EnqueueFromTransactionError)
+            expect { event }.not_to raise_error
           end
         end
       end

ee/spec/services/ee/groups/import_export/import_service_spec.rb

@@ -56,7 +56,7 @@
     it 'does not call the group wiki restorer' do
       expect(::Gitlab::ImportExport::RepoRestorer).not_to receive(:new)
-      expect { import_service.execute }.not_to raise_error(Gitlab::ImportExport::Error)
+      expect { import_service.execute }.not_to raise_error
     end
   end
 end

ee/spec/support/shared_examples/services/ci/play_job_service_shared_examples.rb

@@ -14,7 +14,7 @@
     end
     it 'does not raise any exception' do
-      expect { subject }.not_to raise_error(::Gitlab::Access::AccessDeniedError)
+      expect { subject }.not_to raise_error
     end
   end

spec/controllers/projects/jobs_controller_spec.rb

@@ -929,13 +929,13 @@ def post_play
       context 'when continue url is present' do
         let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) }
+        before do
+          post_cancel(continue: { to: url })
+        end
+
         context 'when continue to is a safe url' do
           let(:url) { '/test' }
-          before do
-            post_cancel(continue: { to: url })
-          end
-
           it 'redirects to the continue url' do
             expect(response).to have_gitlab_http_status(:found)
             expect(response).to redirect_to(url)
@@ -949,8 +949,9 @@ def post_play
         context 'when continue to is not a safe url' do
           let(:url) { 'http://example.com' }
-          it 'raises an error' do
-            expect { cancel_with_redirect(url) }.to raise_error
+          it 'redirects to the builds page' do
+            expect(response).to have_gitlab_http_status(:found)
+            expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id))
           end
         end
       end

spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb

@@ -29,7 +29,7 @@
     # Tagging records
     expect { tagging_1.reload }.to raise_error(ActiveRecord::RecordNotFound)
     expect { tagging_2.reload }.to raise_error(ActiveRecord::RecordNotFound)
-    expect { other_tagging.reload }.not_to raise_error(ActiveRecord::RecordNotFound)
+    expect { other_tagging.reload }.not_to raise_error
     expect { tagging_3.reload }.to raise_error(ActiveRecord::RecordNotFound)
     expect { tagging_4.reload }.to raise_error(ActiveRecord::RecordNotFound)
     expect { tagging_5.reload }.to raise_error(ActiveRecord::RecordNotFound)

spec/lib/gitlab/checks/branch_check_spec.rb

@@ -103,7 +103,7 @@
           it 'prevents force push' do
             expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true)
-            expect { subject.validate! }.to raise_error
+            expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError)
           end
         end
       end
@@ -126,7 +126,7 @@
           it 'prevents force push' do
             expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true)
-            expect { subject.validate! }.to raise_error
+            expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError)
           end
         end
@@ -141,7 +141,7 @@
           it 'prevents force push' do
             expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true)
-            expect { subject.validate! }.to raise_error
+            expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError)
           end
         end
       end

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -3180,15 +3180,15 @@ def setup
     context 'without proper permissions' do
       before do
-        allow(model).to receive(:execute).with(/CREATE EXTENSION IF NOT EXISTS #{extension}/).and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied')
+        allow(model).to receive(:execute)
+          .with(/CREATE EXTENSION IF NOT EXISTS #{extension}/)
+          .and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied')
       end
-      it 'raises the exception' do
-        expect { subject }.to raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/)
-      end
-
-      it 'prints an error message' do
-        expect { subject }.to output(/user is not allowed/).to_stderr.and raise_error
+      it 'raises an exception and prints an error message' do
+        expect { subject }
+          .to output(/user is not allowed/).to_stderr
+          .and raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/)
       end
     end
   end
@@ -3206,15 +3206,15 @@ def setup
     context 'without proper permissions' do
       before do
-        allow(model).to receive(:execute).with(/DROP EXTENSION IF EXISTS #{extension}/).and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied')
-      end
-
-      it 'raises the exception' do
-        expect { subject }.to raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/)
+        allow(model).to receive(:execute)
+          .with(/DROP EXTENSION IF EXISTS #{extension}/)
+          .and_raise(ActiveRecord::StatementInvalid, 'InsufficientPrivilege: permission denied')
       end
-      it 'prints an error message' do
-        expect { subject }.to output(/user is not allowed/).to_stderr.and raise_error
+      it 'raises an exception and prints an error message' do
+        expect { subject }
+          .to output(/user is not allowed/).to_stderr
+          .and raise_error(ActiveRecord::StatementInvalid, /InsufficientPrivilege/)
       end
     end
   end

spec/lib/gitlab/email/handler/create_note_handler_spec.rb

@@ -62,7 +62,7 @@
         end
         it 'does not raise a UserNotFoundError' do
-          expect { receiver.execute }.not_to raise_error(Gitlab::Email::UserNotFoundError)
+          expect { receiver.execute }.not_to raise_error
         end
       end
     end
@@ -71,7 +71,7 @@
       let(:original_recipient) { User.support_bot }
       it 'does not raise a UserNotFoundError' do
-        expect { receiver.execute }.not_to raise_error(Gitlab::Email::UserNotFoundError)
+        expect { receiver.execute }.not_to raise_error
       end
     end
   end

spec/lib/gitlab/email/message/in_product_marketing/base_spec.rb

@@ -24,7 +24,7 @@
       let(:series) { 0 }
       it 'does not raise error' do
-        expect { subject }.not_to raise_error(ArgumentError)
+        expect { subject }.not_to raise_error
       end
     end
   end

spec/lib/gitlab/git/diff_spec.rb

@@ -177,9 +177,9 @@
           expect(diff_two.diff).to include(Gitlab::EncodingHelper::UNICODE_REPLACEMENT_CHARACTER)
           expect(diff_three.diff).to include(Gitlab::EncodingHelper::UNICODE_REPLACEMENT_CHARACTER)
-          expect { Oj.dump(diff) }.not_to raise_error(EncodingError)
-          expect { Oj.dump(diff_two) }.not_to raise_error(EncodingError)
-          expect { Oj.dump(diff_three) }.not_to raise_error(EncodingError)
+          expect { Oj.dump(diff) }.not_to raise_error
+          expect { Oj.dump(diff_two) }.not_to raise_error
+          expect { Oj.dump(diff_three) }.not_to raise_error
         end
         context 'when the diff is binary' do

spec/lib/gitlab/popen_spec.rb

@@ -103,7 +103,7 @@
     it 'raises error' do
       expect do
         @klass.new.popen(%w[foobar])
-      end.to raise_error
+      end.to raise_error(Errno::ENOENT)
     end
   end
 end

spec/lib/gitlab/tracking/event_definition_spec.rb

@@ -33,7 +33,7 @@ def write_metric(metric, path, content)
   end
   it 'has all definitions valid' do
-    expect { described_class.definitions }.not_to raise_error(Gitlab::Tracking::InvalidEventError)
+    expect { described_class.definitions }.not_to raise_error
   end
   describe '#validate' do

spec/migrations/finalize_project_namespaces_backfill_spec.rb

@@ -42,7 +42,7 @@
       context 'when project namespace backfilling migration finished successfully' do
         it 'does not raise exception' do
-          expect { migrate! }.not_to raise_error(/Expected batched background migration for the given configuration to be marked as 'finished'/)
+          expect { migrate! }.not_to raise_error
         end
       end

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -494,7 +494,7 @@
       end
       it 'does not raise kubeclient http error' do
-        expect { subject }.not_to raise_error(Kubeclient::HttpError)
+        expect { subject }.not_to raise_error
       end
     end

spec/models/concerns/reactive_caching_spec.rb

@@ -237,7 +237,7 @@ def self.from_cache(*args); end
       end
       it 'does not raise the exception' do
-        expect { go! }.not_to raise_exception(ReactiveCaching::ExceededReactiveCacheLimit)
+        expect { go! }.not_to raise_exception
       end
     end

spec/models/environment_spec.rb

@@ -621,7 +621,7 @@
             expect(close_action.processed).to be_falsey
             # it encounters the StaleObjectError at first, but reloads the object and runs `build.play`
-            expect { subject }.not_to raise_error(ActiveRecord::StaleObjectError)
+            expect { subject }.not_to raise_error
             # Now the build should be processed.
             expect(close_action.reload.processed).to be_truthy