Merge branch 'rails-save-bang-16' into 'master'

Fix Rails/SaveBang offenses for /spec/support/shared_examples/models/* and ee/spec/support/shared_examples/models*

See merge request gitlab-org/gitlab!40695

.rubocop_todo.yml

@@ -802,11 +802,6 @@ Rails/SaveBang:
     - 'ee/spec/support/shared_examples/finders/geo/framework_registry_finder_shared_examples.rb'
     - 'ee/spec/support/shared_examples/graphql/geo/geo_registries_resolver_shared_examples.rb'
     - 'ee/spec/support/shared_examples/lib/analytics/common_merge_request_metrics_refresh_shared_examples.rb'
-    - 'ee/spec/support/shared_examples/models/concerns/replicator_shared_examples.rb'
-    - 'ee/spec/support/shared_examples/models/elasticsearch_indexed_container_shared_examples.rb'
-    - 'ee/spec/support/shared_examples/models/geo_framework_registry_shared_examples.rb'
-    - 'ee/spec/support/shared_examples/models/member_shared_examples.rb'
-    - 'ee/spec/support/shared_examples/models/mentionable_shared_examples.rb'
     - 'ee/spec/support/shared_examples/policies/protected_environments_shared_examples.rb'
     - 'ee/spec/workers/adjourned_project_deletion_worker_spec.rb'
     - 'ee/spec/workers/clear_shared_runners_minutes_worker_spec.rb'
@@ -1191,18 +1186,6 @@ Rails/SaveBang:
     - 'spec/support/shared_examples/controllers/sessionless_auth_controller_shared_examples.rb'
     - 'spec/support/shared_examples/features/editable_merge_request_shared_examples.rb'
     - 'spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb'
-    - 'spec/support/shared_examples/models/chat_slash_commands_shared_examples.rb'
-    - 'spec/support/shared_examples/models/cluster_application_helm_cert_shared_examples.rb'
-    - 'spec/support/shared_examples/models/concerns/limitable_shared_examples.rb'
-    - 'spec/support/shared_examples/models/concerns/timebox_shared_examples.rb'
-    - 'spec/support/shared_examples/models/diff_note_after_commit_shared_examples.rb'
-    - 'spec/support/shared_examples/models/member_shared_examples.rb'
-    - 'spec/support/shared_examples/models/members_notifications_shared_example.rb'
-    - 'spec/support/shared_examples/models/mentionable_shared_examples.rb'
-    - 'spec/support/shared_examples/models/project_latest_successful_build_for_shared_examples.rb'
-    - 'spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb'
-    - 'spec/support/shared_examples/models/update_project_statistics_shared_examples.rb'
-    - 'spec/support/shared_examples/models/with_uploads_shared_examples.rb'
     - 'spec/support/shared_examples/policies/project_policy_shared_examples.rb'
     - 'spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb'
     - 'spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb'

changelogs/unreleased/rails-save-bang-16.yml

+---
+title: Refactor ee/spec/support/shared_examples/models/* and spec/support/shared_examples/models/* to fix Rails/SaveBang Cop
+merge_request: 40695
+author: Rajendra Kadam
+type: other

ee/spec/support/shared_examples/models/concerns/replicator_shared_examples.rb

@@ -19,7 +19,7 @@ RSpec.shared_examples 'a replicator' do
 
       before do
         create(registry_factory, :synced)
-        create(registry_factory)
+        create(registry_factory) # rubocop: disable Rails/SaveBang
         create(registry_factory, :failed)
       end
 

ee/spec/support/shared_examples/models/elasticsearch_indexed_container_shared_examples.rb

@@ -2,7 +2,7 @@
 
 RSpec.shared_examples 'an elasticsearch indexed container' do
   describe 'validations' do
-    subject { create(container, container_attributes) }
+    subject { create(container, container_attributes) } # rubocop:disable Rails/SaveBang
 
     it 'validates uniqueness of main attribute' do
       is_expected.to validate_uniqueness_of(required_attribute)
@@ -27,7 +27,7 @@ RSpec.shared_examples 'an elasticsearch indexed container' do
     end
 
     describe 'on destroy' do
-      subject { create(container, container_attributes) }
+      subject { create(container, container_attributes) } # rubocop:disable Rails/SaveBang
 
       it 'triggers delete_from_index' do
         is_expected.to receive(:delete_from_index)

ee/spec/support/shared_examples/models/geo_framework_registry_shared_examples.rb

@@ -6,8 +6,8 @@ RSpec.shared_examples 'a Geo framework registry' do
   context 'finders' do
     let!(:failed_item1) { create(registry_class_factory, :failed) }
     let!(:failed_item2) { create(registry_class_factory, :failed) }
-    let!(:unsynced_item1) { create(registry_class_factory) }
-    let!(:unsynced_item2) { create(registry_class_factory) }
+    let!(:unsynced_item1) { create(registry_class_factory) } # rubocop:disable Rails/SaveBang
+    let!(:unsynced_item2) { create(registry_class_factory) } # rubocop:disable Rails/SaveBang
 
     describe '.find_registries_never_attempted_sync' do
       it 'returns unsynced items' do

ee/spec/support/shared_examples/models/member_shared_examples.rb

@@ -30,7 +30,7 @@ RSpec.shared_examples 'member validations' do
           let!(:subgroup) { create(:group, parent: group) }
 
           before do
-            entity.update(group: subgroup) if entity.is_a?(Project)
+            entity.update!(group: subgroup) if entity.is_a?(Project)
           end
 
           it 'does not allow adding a group member with SSO enforced on subgroup' do

ee/spec/support/shared_examples/models/mentionable_shared_examples.rb

@@ -54,7 +54,7 @@ RSpec.shared_examples 'an editable mentionable with EE-specific mentions' do
   let(:new_epic) { create(:epic, group: group) }
 
   it 'creates new cross-reference notes when the mentionable text is edited' do
-    subject.save
+    subject.save!
     subject.create_cross_references!
 
     new_text = <<-MSG.strip_heredoc

spec/support/shared_examples/models/chat_slash_commands_shared_examples.rb

@@ -85,7 +85,7 @@ RSpec.shared_examples 'chat slash commands service' do
         let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } }
 
         subject do
-          described_class.create(project: project, properties: { token: 'token' })
+          described_class.create!(project: project, properties: { token: 'token' })
         end
 
         it 'triggers the command' do

spec/support/shared_examples/models/cluster_application_helm_cert_shared_examples.rb

 # frozen_string_literal: true
 
 RSpec.shared_examples 'cluster application helm specs' do |application_name|
-  let(:application) { create(application_name) }
+  let(:application) { create(application_name) } # rubocop:disable Rails/SaveBang
 
   describe '#uninstall_command' do
     subject { application.uninstall_command }

spec/support/shared_examples/models/concerns/limitable_shared_examples.rb

@@ -8,26 +8,29 @@ RSpec.shared_examples 'includes Limitable concern' do
 
     context 'without plan limits configured' do
       it 'can create new models' do
-        expect { subject.save }.to change { described_class.count }
+        expect { subject.save! }.to change { described_class.count }
       end
     end
 
     context 'with plan limits configured' do
       before do
-        plan_limits.update(subject.class.limit_name => 1)
+        plan_limits.update!(subject.class.limit_name => 1)
       end
 
       it 'can create new models' do
-        expect { subject.save }.to change { described_class.count }
+        expect { subject.save! }.to change { described_class.count }
       end
 
       context 'with an existing model' do
         before do
-          subject.dup.save
+          subject.dup.save!
         end
 
         it 'cannot create new models exceeding the plan limits' do
-          expect { subject.save }.not_to change { described_class.count }
+          expect do
+            expect { subject.save! }.to raise_error(ActiveRecord::RecordInvalid)
+          end
+            .not_to change { described_class.count }
           expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded")
         end
       end

spec/support/shared_examples/models/concerns/timebox_shared_examples.rb

@@ -102,7 +102,7 @@ RSpec.shared_examples 'a timebox' do |timebox_type|
         let(:timebox) { create(timebox_type, *timebox_args, group: group) }
 
         before do
-          project.update(group: group)
+          project.update!(group: group)
         end
 
         it "does not accept the same title in a group twice" do

spec/support/shared_examples/models/diff_note_after_commit_shared_examples.rb

@@ -10,7 +10,7 @@ RSpec.shared_examples 'a valid diff note with after commit callback' do
       it 'raises an error' do
         allow(diff_file_from_repository).to receive(:line_for_position).with(position).and_return(nil)
 
-        expect { subject.save }.to raise_error(::DiffNote::NoteDiffFileCreationError,
+        expect { subject.save! }.to raise_error(::DiffNote::NoteDiffFileCreationError,
                                                "Failed to find diff line for: #{diff_file_from_repository.file_path}, "\
                                                "old_line: #{position.old_line}"\
                                                ", new_line: #{position.new_line}")
@@ -25,11 +25,11 @@ RSpec.shared_examples 'a valid diff note with after commit callback' do
       it 'fallback to fetch file from repository' do
         expect_any_instance_of(::Gitlab::Diff::Position).to receive(:diff_file).with(project.repository)
 
-        subject.save
+        subject.save!
       end
 
       it 'creates a diff note file' do
-        subject.save
+        subject.save!
 
         expect(subject.reload.note_diff_file).to be_present
       end
@@ -40,7 +40,7 @@ RSpec.shared_examples 'a valid diff note with after commit callback' do
     it 'raises an error' do
       allow_any_instance_of(::Gitlab::Diff::Position).to receive(:diff_file).with(project.repository).and_return(nil)
 
-      expect { subject.save }.to raise_error(::DiffNote::NoteDiffFileCreationError, 'Failed to find diff file')
+      expect { subject.save! }.to raise_error(::DiffNote::NoteDiffFileCreationError, 'Failed to find diff file')
     end
   end
 end

spec/support/shared_examples/models/member_shared_examples.rb

@@ -25,21 +25,21 @@ RSpec.shared_examples 'inherited access level as a member of entity' do
     it 'is allowed to change to be a developer of the entity' do
       entity.add_maintainer(user)
 
-      expect { member.update(access_level: Gitlab::Access::DEVELOPER) }
+      expect { member.update!(access_level: Gitlab::Access::DEVELOPER) }
         .to change { member.access_level }.to(Gitlab::Access::DEVELOPER)
     end
 
     it 'is not allowed to change to be a guest of the entity' do
       entity.add_maintainer(user)
 
-      expect { member.update(access_level: Gitlab::Access::GUEST) }
+      expect { member.update(access_level: Gitlab::Access::GUEST) } # rubocop:disable Rails/SaveBang
         .not_to change { member.reload.access_level }
     end
 
     it "shows an error if the member can't be updated" do
       entity.add_maintainer(user)
 
-      member.update(access_level: Gitlab::Access::REPORTER)
+      expect { member.update!(access_level: Gitlab::Access::REPORTER) }.to raise_error(ActiveRecord::RecordInvalid)
 
       expect(member.errors.full_messages).to eq(["Access level should be greater than or equal to Developer inherited membership from group #{parent_entity.name}"])
     end
@@ -51,7 +51,7 @@ RSpec.shared_examples 'inherited access level as a member of entity' do
 
       non_member = entity.is_a?(Group) ? entity.group_member(non_member_user) : entity.project_member(non_member_user)
 
-      expect { non_member.update(access_level: Gitlab::Access::GUEST) }
+      expect { non_member.update!(access_level: Gitlab::Access::GUEST) }
         .to change { non_member.reload.access_level }
     end
   end
@@ -60,7 +60,7 @@ end
 RSpec.shared_examples '#valid_level_roles' do |entity_name|
   let(:member_user) { create(:user) }
   let(:group) { create(:group) }
-  let(:entity) { create(entity_name) }
+  let(:entity) { create(entity_name) } # rubocop:disable Rails/SaveBang
   let(:entity_member) { create("#{entity_name}_member", :developer, source: entity, user: member_user) }
   let(:presenter) { described_class.new(entity_member, current_user: member_user) }
   let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Reporter' => 20 } }

spec/support/shared_examples/models/members_notifications_shared_example.rb

@@ -13,7 +13,7 @@ RSpec.shared_examples 'members notifications' do |entity_type|
     it "sends email to user" do
       expect(notification_service).to receive(:"new_#{entity_type}_member").with(member)
 
-      member.save
+      member.save!
     end
   end
 

spec/support/shared_examples/models/mentionable_shared_examples.rb

@@ -162,7 +162,7 @@ RSpec.shared_examples 'an editable mentionable' do
   end
 
   it 'creates new cross-reference notes when the mentionable text is edited' do
-    subject.save
+    subject.save!
     subject.create_cross_references!
 
     new_text = <<-MSG.strip_heredoc
@@ -270,7 +270,7 @@ RSpec.shared_examples 'mentions in notes' do |mentionable_type|
     let!(:mentionable) { note.noteable }
 
     before do
-      note.update(note: note_desc)
+      note.update!(note: note_desc)
       note.store_mentions!
       add_member(user)
     end
@@ -292,7 +292,7 @@ RSpec.shared_examples 'load mentions from DB' do |mentionable_type|
     let_it_be(:note_desc) { "#{mentioned_user.to_reference} and #{group.to_reference(full: true)} and @all" }
 
     before do
-      note.update(note: note_desc)
+      note.update!(note: note_desc)
       note.store_mentions!
       add_member(user)
     end
@@ -305,7 +305,7 @@ RSpec.shared_examples 'load mentions from DB' do |mentionable_type|
           mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << non_existing_record_id,
           mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << non_existing_record_id
         }
-        user_mention.update(mention_ids)
+        user_mention.update!(mention_ids)
       end
 
       it 'filters out inexistent mentions' do
@@ -328,7 +328,7 @@ RSpec.shared_examples 'load mentions from DB' do |mentionable_type|
           mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << private_project.id,
           mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << private_group.id
         }
-        user_mention.update(mention_ids)
+        user_mention.update!(mention_ids)
 
         add_member(mega_user)
         private_project.add_developer(mega_user)

spec/support/shared_examples/models/project_latest_successful_build_for_shared_examples.rb

@@ -53,7 +53,7 @@ RSpec.shared_examples 'latest successful build for sha or ref' do
     let(:build_name) { pending_build.name }
 
     before do
-      pipeline.update(status: 'pending')
+      pipeline.update!(status: 'pending')
     end
 
     it 'returns empty relation' do

spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb

@@ -164,7 +164,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
     context "event channels" do
       it "uses the right channel for push event" do
-        chat_service.update(push_channel: "random")
+        chat_service.update!(push_channel: "random")
 
         expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
@@ -172,7 +172,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       end
 
       it "uses the right channel for merge request event" do
-        chat_service.update(merge_request_channel: "random")
+        chat_service.update!(merge_request_channel: "random")
 
         expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
@@ -180,7 +180,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       end
 
       it "uses the right channel for issue event" do
-        chat_service.update(issue_channel: "random")
+        chat_service.update!(issue_channel: "random")
 
         expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
@@ -191,7 +191,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         let(:issue_service_options) { { title: 'Secret', confidential: true } }
 
         it "uses confidential issue channel" do
-          chat_service.update(confidential_issue_channel: 'confidential')
+          chat_service.update!(confidential_issue_channel: 'confidential')
 
           expect(Slack::Messenger).to execute_with_options(channel: ['confidential'])
 
@@ -199,7 +199,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         end
 
         it 'falls back to issue channel' do
-          chat_service.update(issue_channel: 'fallback_channel')
+          chat_service.update!(issue_channel: 'fallback_channel')
 
           expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel'])
 
@@ -208,7 +208,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       end
 
       it "uses the right channel for wiki event" do
-        chat_service.update(wiki_page_channel: "random")
+        chat_service.update!(wiki_page_channel: "random")
 
         expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
@@ -221,7 +221,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         end
 
         it "uses the right channel" do
-          chat_service.update(note_channel: "random")
+          chat_service.update!(note_channel: "random")
 
           note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
@@ -236,7 +236,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
           end
 
           it "uses confidential channel" do
-            chat_service.update(confidential_note_channel: "confidential")
+            chat_service.update!(confidential_note_channel: "confidential")
 
             note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
@@ -246,7 +246,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
           end
 
           it 'falls back to note channel' do
-            chat_service.update(note_channel: "fallback_channel")
+            chat_service.update!(note_channel: "fallback_channel")
 
             note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 

spec/support/shared_examples/models/update_project_statistics_shared_examples.rb

@@ -105,7 +105,7 @@ RSpec.shared_examples 'UpdateProjectStatistics' do
         expect(ProjectStatistics)
           .not_to receive(:increment_statistic)
 
-        project.update(pending_delete: true)
+        project.update!(pending_delete: true)
         project.destroy!
       end
 
@@ -113,7 +113,7 @@ RSpec.shared_examples 'UpdateProjectStatistics' do
         expect(Namespaces::ScheduleAggregationWorker)
           .not_to receive(:perform_async)
 
-        project.update(pending_delete: true)
+        project.update!(pending_delete: true)
         project.destroy!
       end
     end

spec/support/shared_examples/models/with_uploads_shared_examples.rb

@@ -12,7 +12,7 @@ RSpec.shared_examples 'model with uploads' do |supports_fileuploads|
       it 'deletes remote uploads' do
         expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original
 
-        expect { model_object.destroy }.to change { Upload.count }.by(-1)
+        expect { model_object.destroy! }.to change { Upload.count }.by(-1)
       end
     end
 
@@ -21,13 +21,13 @@ RSpec.shared_examples 'model with uploads' do |supports_fileuploads|
         let!(:uploads) { create_list(:upload, 2, uploader: FileUploader, model: model_object) }
 
         it 'deletes any FileUploader uploads which are not mounted' do
-          expect { model_object.destroy }.to change { Upload.count }.by(-3)
+          expect { model_object.destroy! }.to change { Upload.count }.by(-3)
         end
 
         it 'deletes local files' do
           expect_any_instance_of(Uploads::Local).to receive(:delete_keys).with(uploads.map(&:absolute_path))
 
-          model_object.destroy
+          model_object.destroy!
         end
       end
 
@@ -35,14 +35,14 @@ RSpec.shared_examples 'model with uploads' do |supports_fileuploads|
         let!(:uploads) { create_list(:upload, 2, :object_storage, uploader: FileUploader, model: model_object) }
 
         it 'deletes any FileUploader uploads which are not mounted' do
-          expect { model_object.destroy }.to change { Upload.count }.by(-3)
+          expect { model_object.destroy! }.to change { Upload.count }.by(-3)
         end
 
         it 'deletes remote files' do
           expected_array = array_including(*uploads.map(&:path))
           expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(expected_array)
 
-          model_object.destroy
+          model_object.destroy!
         end
       end
     end