Merge branch '220040-solving-rails-savebang-rubocop-rule-offenses' into 'master'

Fix Rails/SaveBang RuboCop offenses for spec/models

See merge request gitlab-org/gitlab!73494

.rubocop_manual_todo.yml

@@ -39,7 +39,6 @@ Rails/SaveBang:
     - 'ee/spec/models/license_spec.rb'
     - 'ee/spec/models/merge_request_spec.rb'
     - 'ee/spec/models/merge_train_spec.rb'
-    - 'spec/models/packages/package_spec.rb'
     - 'ee/spec/models/project_ci_cd_setting_spec.rb'
     - 'ee/spec/models/project_spec.rb'
     - 'ee/spec/models/protected_environment_spec.rb'
@@ -135,32 +134,6 @@ Rails/SaveBang:
     - 'spec/lib/gitlab/middleware/go_spec.rb'
     - 'spec/lib/gitlab/shard_health_cache_spec.rb'
     - 'spec/mailers/notify_spec.rb'
-    - 'spec/models/clusters/applications/helm_spec.rb'
-    - 'spec/models/design_management/version_spec.rb'
-    - 'spec/models/environment_spec.rb'
-    - 'spec/models/event_spec.rb'
-    - 'spec/models/fork_network_spec.rb'
-    - 'spec/models/generic_commit_status_spec.rb'
-    - 'spec/models/grafana_integration_spec.rb'
-    - 'spec/models/group_spec.rb'
-    - 'spec/models/identity_spec.rb'
-    - 'spec/models/jira_import_state_spec.rb'
-    - 'spec/models/namespace_spec.rb'
-    - 'spec/models/note_spec.rb'
-    - 'spec/models/notification_setting_spec.rb'
-    - 'spec/models/operations/feature_flag_scope_spec.rb'
-    - 'spec/models/operations/feature_flags/strategy_spec.rb'
-    - 'spec/models/operations/feature_flags/user_list_spec.rb'
-    - 'spec/models/pages_domain_spec.rb'
-    - 'spec/models/protectable_dropdown_spec.rb'
-    - 'spec/models/redirect_route_spec.rb'
-    - 'spec/models/release_spec.rb'
-    - 'spec/models/remote_mirror_spec.rb'
-    - 'spec/models/resource_milestone_event_spec.rb'
-    - 'spec/models/route_spec.rb'
-    - 'spec/models/sentry_issue_spec.rb'
-    - 'spec/models/snippet_spec.rb'
-    - 'spec/models/upload_spec.rb'
 
 Rails/TimeZone:
   Enabled: true

spec/factories/operations/feature_flags/strategy.rb

@@ -5,5 +5,37 @@ FactoryBot.define do
     association :feature_flag, factory: :operations_feature_flag
     name { "default" }
     parameters { {} }
+
+    trait :default do
+      name { "default" }
+      parameters { {} }
+    end
+
+    trait :gitlab_userlist do
+      association :user_list, factory: :operations_feature_flag_user_list
+      name { "gitlabUserList" }
+      parameters { {} }
+    end
+
+    trait :flexible_rollout do
+      name { "flexibleRollout" }
+      parameters do
+        {
+          groupId: 'default',
+          rollout: '10',
+          stickiness: 'default'
+        }
+      end
+    end
+
+    trait :gradual_rollout do
+      name { "gradualRolloutUserId" }
+      parameters { { percentage: '10', groupId: 'default' } }
+    end
+
+    trait :userwithid do
+      name { "userWithId" }
+      parameters { { userIds: 'user1' } }
+    end
   end
 end

spec/models/design_management/version_spec.rb

@@ -283,7 +283,7 @@ RSpec.describe DesignManagement::Version do
     it 'retrieves author from the Commit if author_id is nil and version has been persisted' do
       author = create(:user)
       version = create(:design_version, :committed, author: author)
-      author.destroy
+      author.destroy!
       version.reload
       commit = version.issue.project.design_repository.commit(version.sha)
       commit_user = create(:user, email: commit.author_email, name: commit.author_name)

spec/models/environment_spec.rb

@@ -39,7 +39,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
     it 'ensures environment tier when a new object is created' do
       environment = build(:environment, name: 'gprd', tier: nil)
 
-      expect { environment.save }.to change { environment.tier }.from(nil).to('production')
+      expect { environment.save! }.to change { environment.tier }.from(nil).to('production')
     end
 
     it 'ensures environment tier when an existing object is updated' do
@@ -418,7 +418,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
 
       context 'not in the same branch' do
         before do
-          deployment.update(sha: project.commit('feature').id)
+          deployment.update!(sha: project.commit('feature').id)
         end
 
         it 'returns false' do
@@ -496,7 +496,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
     context 'when no other actions' do
       context 'environment is available' do
         before do
-          environment.update(state: :available)
+          environment.update!(state: :available)
         end
 
         it do
@@ -508,7 +508,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
 
       context 'environment is already stopped' do
         before do
-          environment.update(state: :stopped)
+          environment.update!(state: :stopped)
         end
 
         it do
@@ -1502,7 +1502,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
       deployment = create(:deployment, :success, environment: environment, project: project)
       deployment.create_ref
 
-      expect { environment.destroy }.to change { project.commit(deployment.ref_path) }.to(nil)
+      expect { environment.destroy! }.to change { project.commit(deployment.ref_path) }.to(nil)
     end
   end
 
@@ -1517,7 +1517,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
       end
 
       it 'returns the environments count grouped by state with zero value' do
-        environment2.update(state: 'stopped')
+        environment2.update!(state: 'stopped')
         expect(project.environments.count_by_state).to eq({ stopped: 3, available: 0 })
       end
     end

spec/models/event_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe Event do
     describe 'after_create :set_last_repository_updated_at' do
       context 'with a push event' do
         it 'updates the project last_repository_updated_at' do
-          project.update(last_repository_updated_at: 1.year.ago)
+          project.update!(last_repository_updated_at: 1.year.ago)
 
           create_push_event(project, project.owner)
 
@@ -44,7 +44,7 @@ RSpec.describe Event do
 
       context 'without a push event' do
         it 'does not update the project last_repository_updated_at' do
-          project.update(last_repository_updated_at: 1.year.ago)
+          project.update!(last_repository_updated_at: 1.year.ago)
 
           create(:closed_issue_event, project: project, author: project.owner)
 
@@ -58,7 +58,7 @@ RSpec.describe Event do
     describe '#set_last_repository_updated_at' do
       it 'only updates once every Event::REPOSITORY_UPDATED_AT_INTERVAL minutes' do
         last_known_timestamp = (Event::REPOSITORY_UPDATED_AT_INTERVAL - 1.minute).ago
-        project.update(last_repository_updated_at: last_known_timestamp)
+        project.update!(last_repository_updated_at: last_known_timestamp)
         project.reload # a reload removes fractions of seconds
 
         expect do
@@ -73,7 +73,7 @@ RSpec.describe Event do
 
       it 'passes event to UserInteractedProject.track' do
         expect(UserInteractedProject).to receive(:track).with(event)
-        event.save
+        event.save!
       end
     end
   end
@@ -824,7 +824,7 @@ RSpec.describe Event do
 
     context 'when a project was updated less than 1 hour ago' do
       it 'does not update the project' do
-        project.update(last_activity_at: Time.current)
+        project.update!(last_activity_at: Time.current)
 
         expect(project).not_to receive(:update_column)
           .with(:last_activity_at, a_kind_of(Time))
@@ -835,7 +835,7 @@ RSpec.describe Event do
 
     context 'when a project was updated more than 1 hour ago' do
       it 'updates the project' do
-        project.update(last_activity_at: 1.year.ago)
+        project.update!(last_activity_at: 1.year.ago)
 
         create_push_event(project, project.owner)
 

spec/models/fork_network_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe ForkNetwork do
   describe '#add_root_as_member' do
     it 'adds the root project as a member when creating a new root network' do
       project = create(:project)
-      fork_network = described_class.create(root_project: project)
+      fork_network = described_class.create!(root_project: project)
 
       expect(fork_network.projects).to include(project)
     end
@@ -54,8 +54,8 @@ RSpec.describe ForkNetwork do
       first_fork = fork_project(first_project)
       second_fork = fork_project(second_project)
 
-      first_project.destroy
-      second_project.destroy
+      first_project.destroy!
+      second_project.destroy!
 
       expect(first_fork.fork_network).not_to be_nil
       expect(first_fork.fork_network.root_project).to be_nil

spec/models/generic_commit_status_spec.rb

@@ -133,7 +133,7 @@ RSpec.describe GenericCommitStatus do
     before do
       generic_commit_status.context = nil
       generic_commit_status.stage = nil
-      generic_commit_status.save
+      generic_commit_status.save!
     end
 
     describe '#context' do

spec/models/grafana_integration_spec.rb

@@ -60,7 +60,7 @@ RSpec.describe GrafanaIntegration do
 
     context 'with grafana integration enabled' do
       it 'returns nil' do
-        grafana_integration.update(enabled: false)
+        grafana_integration.update!(enabled: false)
 
         expect(grafana_integration.client).to be(nil)
       end
@@ -81,8 +81,8 @@ RSpec.describe GrafanaIntegration do
       end
 
       it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do
-        expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) }
-        expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) }
+        expect { grafana_integration.update!(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) }
+        expect { grafana_integration.update!(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) }
       end
     end
   end

spec/models/group_spec.rb

@@ -160,7 +160,7 @@ RSpec.describe Group do
       context 'when sub group is deleted' do
         it 'does not delete parent notification settings' do
           expect do
-            sub_group.destroy
+            sub_group.destroy!
           end.to change { NotificationSetting.count }.by(-1)
         end
       end
@@ -404,7 +404,7 @@ RSpec.describe Group do
 
       subject do
         recorded_queries.record do
-          group.update(parent: new_parent)
+          group.update!(parent: new_parent)
         end
       end
 
@@ -496,7 +496,7 @@ RSpec.describe Group do
       let!(:group) { create(:group, parent: parent_group) }
 
       before do
-        parent_group.update(parent: new_grandparent)
+        parent_group.update!(parent: new_grandparent)
       end
 
       it 'updates traversal_ids for all descendants' do
@@ -866,7 +866,7 @@ RSpec.describe Group do
       before do
         parent_group = create(:group)
         create(:group_member, :owner, group: parent_group)
-        group.update(parent: parent_group)
+        group.update!(parent: parent_group)
       end
 
       it { expect(group.last_owner?(@members[:owner])).to be_falsy }
@@ -923,7 +923,7 @@ RSpec.describe Group do
         before do
           parent_group = create(:group)
           create(:group_member, :owner, group: parent_group)
-          group.update(parent: parent_group)
+          group.update!(parent: parent_group)
         end
 
         it { expect(group.member_last_blocked_owner?(member)).to be(false) }
@@ -1971,7 +1971,7 @@ RSpec.describe Group do
           let(:environment) { 'foo%bar/test' }
 
           it 'matches literally for %' do
-            ci_variable.update(environment_scope: 'foo%bar/*')
+            ci_variable.update_attribute(:environment_scope, 'foo%bar/*')
 
             is_expected.to contain_exactly(ci_variable)
           end
@@ -2112,7 +2112,7 @@ RSpec.describe Group do
       let(:ancestor_group) { create(:group) }
 
       before do
-        group.update(parent: ancestor_group)
+        group.update!(parent: ancestor_group)
       end
 
       it 'returns all ancestor group ids' do
@@ -2629,7 +2629,7 @@ RSpec.describe Group do
       let_it_be(:project) { create(:project, group: group, service_desk_enabled: false) }
 
       before do
-        project.update(service_desk_enabled: false)
+        project.update!(service_desk_enabled: false)
       end
 
       it { is_expected.to eq(false) }

spec/models/identity_spec.rb

@@ -118,19 +118,19 @@ RSpec.describe Identity do
 
         it 'if extern_uid changes' do
           expect(ldap_identity).not_to receive(:ensure_normalized_extern_uid)
-          ldap_identity.save
+          ldap_identity.save!
         end
 
         it 'if current_uid is nil' do
           expect(ldap_identity).to receive(:ensure_normalized_extern_uid)
 
-          ldap_identity.update(extern_uid: nil)
+          ldap_identity.update!(extern_uid: nil)
 
           expect(ldap_identity.extern_uid).to be_nil
         end
 
         it 'if extern_uid changed and not nil' do
-          ldap_identity.update(extern_uid: 'uid=john1,ou=PEOPLE,dc=example,dc=com')
+          ldap_identity.update!(extern_uid: 'uid=john1,ou=PEOPLE,dc=example,dc=com')
 
           expect(ldap_identity.extern_uid).to eq 'uid=john1,ou=people,dc=example,dc=com'
         end
@@ -150,7 +150,7 @@ RSpec.describe Identity do
 
             expect(user.user_synced_attributes_metadata.provider).to eq 'ldapmain'
 
-            ldap_identity.destroy
+            ldap_identity.destroy!
 
             expect(user.reload.user_synced_attributes_metadata).to be_nil
           end
@@ -162,7 +162,7 @@ RSpec.describe Identity do
 
             expect(user.user_synced_attributes_metadata.provider).to eq 'other'
 
-            ldap_identity.destroy
+            ldap_identity.destroy!
 
             expect(user.reload.user_synced_attributes_metadata.provider).to eq 'other'
           end

spec/models/jira_import_state_spec.rb

@@ -175,7 +175,7 @@ RSpec.describe JiraImportState do
       let(:jira_import) { build(:jira_import_state, project: project)}
 
       it 'does not run the callback', :aggregate_failures do
-        expect { jira_import.save }.to change { JiraImportState.count }.by(1)
+        expect { jira_import.save! }.to change { JiraImportState.count }.by(1)
         expect(jira_import.reload.error_message).to be_nil
       end
     end
@@ -184,7 +184,7 @@ RSpec.describe JiraImportState do
       let(:jira_import) { build(:jira_import_state, project: project, error_message: 'error')}
 
       it 'does not run the callback', :aggregate_failures do
-        expect { jira_import.save }.to change { JiraImportState.count }.by(1)
+        expect { jira_import.save! }.to change { JiraImportState.count }.by(1)
         expect(jira_import.reload.error_message).to eq('error')
       end
     end

spec/models/namespace_spec.rb

@@ -228,7 +228,7 @@ RSpec.describe Namespace do
 
           expect(namespace.path).to eq('j')
 
-          namespace.update(name: 'something new')
+          namespace.update!(name: 'something new')
 
           expect(namespace).to be_valid
           expect(namespace.name).to eq('something new')
@@ -240,7 +240,7 @@ RSpec.describe Namespace do
 
         it 'allows to update path to single char' do
           namespace = create(:project_namespace)
-          namespace.update(path: 'j')
+          namespace.update!(path: 'j')
 
           expect(namespace).to be_valid
         end
@@ -740,7 +740,7 @@ RSpec.describe Namespace do
       end
 
       it "moves dir if path changed" do
-        namespace.update(path: namespace.full_path + '_new')
+        namespace.update!(path: namespace.full_path + '_new')
 
         expect(gitlab_shell.repository_exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy
       end
@@ -751,7 +751,7 @@ RSpec.describe Namespace do
             expect(namespace).to receive(:write_projects_repository_config).and_raise('foo')
 
             expect do
-              namespace.update(path: namespace.full_path + '_new')
+              namespace.update!(path: namespace.full_path + '_new')
             end.to raise_error('foo')
           end
         end
@@ -768,7 +768,7 @@ RSpec.describe Namespace do
             end
             expect(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) # like prod
 
-            namespace.update(path: namespace.full_path + '_new')
+            namespace.update!(path: namespace.full_path + '_new')
           end
         end
       end
@@ -998,7 +998,7 @@ RSpec.describe Namespace do
 
       it "repository directory remains unchanged if path changed" do
         before_disk_path = project.disk_path
-        namespace.update(path: namespace.full_path + '_new')
+        namespace.update!(path: namespace.full_path + '_new')
 
         expect(before_disk_path).to eq(project.disk_path)
         expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
@@ -1013,7 +1013,7 @@ RSpec.describe Namespace do
       let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') }
 
       it 'updates project full path in .git/config' do
-        parent.update(path: 'mygroup_new')
+        parent.update!(path: 'mygroup_new')
 
         expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
         expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
@@ -1025,7 +1025,7 @@ RSpec.describe Namespace do
         repository_hashed_project_in_subgroup = hashed_project_in_subgroup.project_repository
         repository_legacy_project_in_subgroup = legacy_project_in_subgroup.project_repository
 
-        parent.update(path: 'mygroup_moved')
+        parent.update!(path: 'mygroup_moved')
 
         expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}"
         expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path
@@ -1059,7 +1059,7 @@ RSpec.describe Namespace do
       it 'renames its dirs when deleted' do
         allow(GitlabShellWorker).to receive(:perform_in)
 
-        namespace.destroy
+        namespace.destroy!
 
         expect(File.exist?(deleted_path_in_dir)).to be(true)
       end
@@ -1067,7 +1067,7 @@ RSpec.describe Namespace do
       it 'schedules the namespace for deletion' do
         expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path)
 
-        namespace.destroy
+        namespace.destroy!
       end
 
       context 'in sub-groups' do
@@ -1081,7 +1081,7 @@ RSpec.describe Namespace do
         it 'renames its dirs when deleted' do
           allow(GitlabShellWorker).to receive(:perform_in)
 
-          child.destroy
+          child.destroy!
 
           expect(File.exist?(deleted_path_in_dir)).to be(true)
         end
@@ -1089,7 +1089,7 @@ RSpec.describe Namespace do
         it 'schedules the namespace for deletion' do
           expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path)
 
-          child.destroy
+          child.destroy!
         end
       end
     end
@@ -1102,7 +1102,7 @@ RSpec.describe Namespace do
 
         expect(File.exist?(path_in_dir)).to be(false)
 
-        namespace.destroy
+        namespace.destroy!
 
         expect(File.exist?(deleted_path_in_dir)).to be(false)
       end
@@ -1628,7 +1628,7 @@ RSpec.describe Namespace do
       it 'returns the path before last save' do
         group = create(:group)
 
-        group.update(parent: nil)
+        group.update!(parent: nil)
 
         expect(group.full_path_before_last_save).to eq(group.path_before_last_save)
       end
@@ -1639,7 +1639,7 @@ RSpec.describe Namespace do
         group = create(:group, parent: nil)
         parent = create(:group)
 
-        group.update(parent: parent)
+        group.update!(parent: parent)
 
         expect(group.full_path_before_last_save).to eq("#{group.path_before_last_save}")
       end
@@ -1650,7 +1650,7 @@ RSpec.describe Namespace do
         parent = create(:group)
         group = create(:group, parent: parent)
 
-        group.update(parent: nil)
+        group.update!(parent: nil)
 
         expect(group.full_path_before_last_save).to eq("#{parent.full_path}/#{group.path}")
       end
@@ -1662,7 +1662,7 @@ RSpec.describe Namespace do
         group = create(:group, parent: parent)
         new_parent = create(:group)
 
-        group.update(parent: new_parent)
+        group.update!(parent: new_parent)
 
         expect(group.full_path_before_last_save).to eq("#{parent.full_path}/#{group.path}")
       end

spec/models/note_spec.rb

@@ -155,7 +155,7 @@ RSpec.describe Note do
         expect(note).to receive(:notify_after_destroy).and_call_original
         expect(note.noteable).to receive(:after_note_destroyed).with(note)
 
-        note.destroy
+        note.destroy!
       end
 
       it 'does not error if noteable is nil' do
@@ -163,7 +163,7 @@ RSpec.describe Note do
 
         expect(note).to receive(:notify_after_destroy).and_call_original
         expect(note).to receive(:noteable).at_least(:once).and_return(nil)
-        expect { note.destroy }.not_to raise_error
+        expect { note.destroy! }.not_to raise_error
       end
     end
   end
@@ -226,8 +226,8 @@ RSpec.describe Note do
 
     describe 'read' do
       before do
-        @p1.project_members.create(user: @u2, access_level: ProjectMember::GUEST)
-        @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST)
+        @p1.project_members.create!(user: @u2, access_level: ProjectMember::GUEST)
+        @p2.project_members.create!(user: @u3, access_level: ProjectMember::GUEST)
       end
 
       it { expect(Ability.allowed?(@u1, :read_note, @p1)).to be_falsey }
@@ -237,8 +237,8 @@ RSpec.describe Note do
 
     describe 'write' do
       before do
-        @p1.project_members.create(user: @u2, access_level: ProjectMember::DEVELOPER)
-        @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER)
+        @p1.project_members.create!(user: @u2, access_level: ProjectMember::DEVELOPER)
+        @p2.project_members.create!(user: @u3, access_level: ProjectMember::DEVELOPER)
       end
 
       it { expect(Ability.allowed?(@u1, :create_note, @p1)).to be_falsey }
@@ -248,9 +248,9 @@ RSpec.describe Note do
 
     describe 'admin' do
       before do
-        @p1.project_members.create(user: @u1, access_level: ProjectMember::REPORTER)
-        @p1.project_members.create(user: @u2, access_level: ProjectMember::MAINTAINER)
-        @p2.project_members.create(user: @u3, access_level: ProjectMember::MAINTAINER)
+        @p1.project_members.create!(user: @u1, access_level: ProjectMember::REPORTER)
+        @p1.project_members.create!(user: @u2, access_level: ProjectMember::MAINTAINER)
+        @p2.project_members.create!(user: @u3, access_level: ProjectMember::MAINTAINER)
       end
 
       it { expect(Ability.allowed?(@u1, :admin_note, @p1)).to be_falsey }
@@ -1468,7 +1468,7 @@ RSpec.describe Note do
     shared_examples 'assignee check' do
       context 'when the provided user is one of the assignees' do
         before do
-          note.noteable.update(assignees: [user, create(:user)])
+          note.noteable.update!(assignees: [user, create(:user)])
         end
 
         it 'returns true' do
@@ -1480,7 +1480,7 @@ RSpec.describe Note do
     shared_examples 'author check' do
       context 'when the provided user is the author' do
         before do
-          note.noteable.update(author: user)
+          note.noteable.update!(author: user)
         end
 
         it 'returns true' do

spec/models/notification_setting_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe NotificationSetting do
         notification_setting.merge_merge_request = "t"
         notification_setting.close_merge_request = "nil"
         notification_setting.reopen_merge_request = "false"
-        notification_setting.save
+        notification_setting.save!
       end
 
       it "parses boolean before saving" do
@@ -52,12 +52,12 @@ RSpec.describe NotificationSetting do
     context 'notification_email' do
       let_it_be(:user) { create(:user) }
 
-      subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) }
+      subject { build(:notification_setting, user_id: user.id) }
 
       it 'allows to change email to verified one' do
         email = create(:email, :confirmed, user: user)
 
-        subject.update(notification_email: email.email)
+        subject.notification_email = email.email
 
         expect(subject).to be_valid
       end
@@ -65,13 +65,13 @@ RSpec.describe NotificationSetting do
       it 'does not allow to change email to not verified one' do
         email = create(:email, user: user)
 
-        subject.update(notification_email: email.email)
+        subject.notification_email = email.email
 
         expect(subject).to be_invalid
       end
 
       it 'allows to change email to empty one' do
-        subject.update(notification_email: '')
+        subject.notification_email = ''
 
         expect(subject).to be_valid
       end
@@ -85,7 +85,7 @@ RSpec.describe NotificationSetting do
       1.upto(4) do |i|
         setting = create(:notification_setting, user: user)
 
-        setting.project.update(pending_delete: true) if i.even?
+        setting.project.update!(pending_delete: true) if i.even?
       end
     end
 

spec/models/operations/feature_flags/user_list_spec.rb

@@ -20,9 +20,9 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'is valid with a string of comma separated values' do
-          user_list = described_class.create(user_xids: valid_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: valid_value)
 
-          expect(user_list.errors[:user_xids]).to be_empty
+          expect(user_list).to be_valid
         end
       end
 
@@ -31,9 +31,10 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'automatically casts values of other types' do
-          user_list = described_class.create(user_xids: typecast_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: typecast_value)
+
+          expect(user_list).to be_valid
 
-          expect(user_list.errors[:user_xids]).to be_empty
           expect(user_list.user_xids).to eq(typecast_value.to_s)
         end
       end
@@ -45,7 +46,9 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'is invalid' do
-          user_list = described_class.create(user_xids: invalid_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: invalid_value)
+
+          expect(user_list).to be_invalid
 
           expect(user_list.errors[:user_xids]).to include(
             'user_xids must be a string of unique comma separated values each 256 characters or less'
@@ -70,20 +73,20 @@ RSpec.describe Operations::FeatureFlags::UserList do
   describe '#destroy' do
     it 'deletes the model if it is not associated with any feature flag strategies' do
       project = create(:project)
-      user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
+      user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2')
 
-      user_list.destroy
+      user_list.destroy!
 
       expect(described_class.count).to eq(0)
     end
 
     it 'does not delete the model if it is associated with a feature flag strategy' do
       project = create(:project)
-      user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
+      user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2')
       feature_flag = create(:operations_feature_flag, :new_version_flag, project: project)
       strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: user_list)
 
-      user_list.destroy
+      user_list.destroy # rubocop:disable Rails/SaveBang
 
       expect(described_class.count).to eq(1)
       expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(1)

spec/models/pages_domain_spec.rb

@@ -104,7 +104,7 @@ RSpec.describe PagesDomain do
     let(:domain) { build(:pages_domain) }
 
     it 'saves validity time' do
-      domain.save
+      domain.save!
 
       expect(domain.certificate_valid_not_before).to be_like_time(Time.zone.parse("2020-03-16 14:20:34 UTC"))
       expect(domain.certificate_valid_not_after).to be_like_time(Time.zone.parse("2220-01-28 14:20:34 UTC"))
@@ -161,7 +161,7 @@ RSpec.describe PagesDomain do
 
       context 'when certificate is already saved' do
         it "doesn't add error to certificate" do
-          domain.save(validate: false)
+          domain.save!(validate: false)
 
           domain.valid?
 

spec/models/protectable_dropdown_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe ProtectableDropdown do
   describe '#protectable_ref_names' do
     context 'when project repository is not empty' do
       before do
-        project.protected_branches.create(name: 'master')
+        create(:protected_branch, project: project, name: 'master')
       end
 
       it { expect(subject.protectable_ref_names).to include('feature') }

spec/models/redirect_route_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe RedirectRoute do
   let(:group) { create(:group) }
-  let!(:redirect_route) { group.redirect_routes.create(path: 'gitlabb') }
+  let!(:redirect_route) { group.redirect_routes.create!(path: 'gitlabb') }
 
   describe 'relationships' do
     it { is_expected.to belong_to(:source) }
@@ -17,10 +17,10 @@ RSpec.describe RedirectRoute do
   end
 
   describe '.matching_path_and_descendants' do
-    let!(:redirect2) { group.redirect_routes.create(path: 'gitlabb/test') }
-    let!(:redirect3) { group.redirect_routes.create(path: 'gitlabb/test/foo') }
-    let!(:redirect4) { group.redirect_routes.create(path: 'gitlabb/test/foo/bar') }
-    let!(:redirect5) { group.redirect_routes.create(path: 'gitlabb/test/baz') }
+    let!(:redirect2) { group.redirect_routes.create!(path: 'gitlabb/test') }
+    let!(:redirect3) { group.redirect_routes.create!(path: 'gitlabb/test/foo') }
+    let!(:redirect4) { group.redirect_routes.create!(path: 'gitlabb/test/foo/bar') }
+    let!(:redirect5) { group.redirect_routes.create!(path: 'gitlabb/test/baz') }
 
     context 'when the redirect route matches with same casing' do
       it 'returns correct routes' do

spec/models/release_spec.rb

@@ -26,10 +26,10 @@ RSpec.describe Release do
     context 'when a release exists in the database without a name' do
       it 'does not require name' do
         existing_release_without_name = build(:release, project: project, author: user, name: nil)
-        existing_release_without_name.save(validate: false)
+        existing_release_without_name.save!(validate: false)
 
         existing_release_without_name.description = "change"
-        existing_release_without_name.save
+        existing_release_without_name.save!
         existing_release_without_name.reload
 
         expect(existing_release_without_name).to be_valid
@@ -88,7 +88,7 @@ RSpec.describe Release do
 
   describe '.create' do
     it "fills released_at using created_at if it's not set" do
-      release = described_class.create(project: project, author: user)
+      release = create(:release, project: project, author: user, released_at: nil)
 
       expect(release.released_at).to eq(release.created_at)
     end
@@ -96,14 +96,14 @@ RSpec.describe Release do
     it "does not change released_at if it's set explicitly" do
       released_at = Time.zone.parse('2018-10-20T18:00:00Z')
 
-      release = described_class.create(project: project, author: user, released_at: released_at)
+      release = create(:release, project: project, author: user, released_at: released_at)
 
       expect(release.released_at).to eq(released_at)
     end
   end
 
   describe '#update' do
-    subject { release.update(params) }
+    subject { release.update!(params) }
 
     context 'when links do not exist' do
       context 'when params are specified for creation' do
@@ -182,7 +182,7 @@ RSpec.describe Release do
       it 'also deletes the associated evidence' do
         release_with_evidence
 
-        expect { release_with_evidence.destroy }.to change(Releases::Evidence, :count).by(-1)
+        expect { release_with_evidence.destroy! }.to change(Releases::Evidence, :count).by(-1)
       end
     end
   end
@@ -190,7 +190,7 @@ RSpec.describe Release do
   describe '#name' do
     context 'name is nil' do
       before do
-        release.update(name: nil)
+        release.update!(name: nil)
       end
 
       it 'returns tag' do

spec/models/remote_mirror_spec.rb

@@ -289,7 +289,7 @@ RSpec.describe RemoteMirror, :mailer do
 
     context 'with remote mirroring disabled' do
       it 'returns nil' do
-        remote_mirror.update(enabled: false)
+        remote_mirror.update!(enabled: false)
 
         expect(remote_mirror.sync).to be_nil
       end
@@ -354,7 +354,7 @@ RSpec.describe RemoteMirror, :mailer do
     let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first }
 
     it 'resets all the columns when URL changes' do
-      remote_mirror.update(last_error: Time.current,
+      remote_mirror.update!(last_error: Time.current,
                            last_update_at: Time.current,
                            last_successful_update_at: Time.current,
                            update_status: 'started',
@@ -378,7 +378,7 @@ RSpec.describe RemoteMirror, :mailer do
     end
 
     before do
-      remote_mirror.update(last_update_started_at: Time.current)
+      remote_mirror.update!(last_update_started_at: Time.current)
     end
 
     context 'when remote mirror does not have status failed' do
@@ -393,7 +393,7 @@ RSpec.describe RemoteMirror, :mailer do
 
     context 'when remote mirror has status failed' do
       it 'returns false when last update started after the timestamp' do
-        remote_mirror.update(update_status: 'failed')
+        remote_mirror.update!(update_status: 'failed')
 
         expect(remote_mirror.updated_since?(timestamp)).to be false
       end
@@ -409,7 +409,7 @@ RSpec.describe RemoteMirror, :mailer do
                              updated_at: 25.hours.ago)
       project = mirror.project
       project.pending_delete = true
-      project.save
+      project.save!
       mirror.reload
 
       expect(mirror.sync).to be_nil

spec/models/route_spec.rb

@@ -31,18 +31,18 @@ RSpec.describe Route do
     context 'after update' do
       it 'calls #create_redirect_for_old_path' do
         expect(route).to receive(:create_redirect_for_old_path)
-        route.update(path: 'foo')
+        route.update!(path: 'foo')
       end
 
       it 'calls #delete_conflicting_redirects' do
         expect(route).to receive(:delete_conflicting_redirects)
-        route.update(path: 'foo')
+        route.update!(path: 'foo')
       end
     end
 
     context 'after create' do
       it 'calls #delete_conflicting_redirects' do
-        route.destroy
+        route.destroy!
         new_route = described_class.new(source: group, path: group.path)
         expect(new_route).to receive(:delete_conflicting_redirects)
         new_route.save!
@@ -81,7 +81,7 @@ RSpec.describe Route do
     context 'path update' do
       context 'when route name is set' do
         before do
-          route.update(path: 'bar')
+          route.update!(path: 'bar')
         end
 
         it 'updates children routes with new path' do
@@ -111,7 +111,7 @@ RSpec.describe Route do
         let!(:conflicting_redirect3) { route.create_redirect('gitlab-org') }
 
         it 'deletes the conflicting redirects' do
-          route.update(path: 'bar')
+          route.update!(path: 'bar')
 
           expect(RedirectRoute.exists?(path: 'bar/test')).to be_falsey
           expect(RedirectRoute.exists?(path: 'bar/test/foo')).to be_falsey
@@ -122,7 +122,7 @@ RSpec.describe Route do
 
     context 'name update' do
       it 'updates children routes with new path' do
-        route.update(name: 'bar')
+        route.update!(name: 'bar')
 
         expect(described_class.exists?(name: 'bar')).to be_truthy
         expect(described_class.exists?(name: 'bar / test')).to be_truthy
@@ -134,7 +134,7 @@ RSpec.describe Route do
         # Note: using `update_columns` to skip all validation and callbacks
         route.update_columns(name: nil)
 
-        expect { route.update(name: 'bar') }
+        expect { route.update!(name: 'bar') }
           .to change { route.name }.from(nil).to('bar')
       end
     end

spec/models/sentry_issue_spec.rb

@@ -53,7 +53,7 @@ RSpec.describe SentryIssue do
       create(:sentry_issue)
       project = sentry_issue.issue.project
       sentry_issue_3 = build(:sentry_issue, issue: create(:issue, project: project), sentry_issue_identifier: sentry_issue.sentry_issue_identifier)
-      sentry_issue_3.save(validate: false)
+      sentry_issue_3.save!(validate: false)
 
       result = described_class.for_project_and_identifier(project, sentry_issue.sentry_issue_identifier)
 

spec/models/snippet_spec.rb

@@ -98,7 +98,7 @@ RSpec.describe Snippet do
       snippet = build(:snippet)
       expect(snippet.statistics).to be_nil
 
-      snippet.save
+      snippet.save!
 
       expect(snippet.statistics).to be_persisted
     end
@@ -289,7 +289,7 @@ RSpec.describe Snippet do
     let(:access_level) { ProjectFeature::ENABLED }
 
     before do
-      project.project_feature.update(snippets_access_level: access_level)
+      project.project_feature.update!(snippets_access_level: access_level)
     end
 
     it 'includes snippets for projects with snippets enabled' do
@@ -623,7 +623,7 @@ RSpec.describe Snippet do
 
       context 'when snippet_repository does not exist' do
         it 'creates a snippet_repository' do
-          snippet.snippet_repository.destroy
+          snippet.snippet_repository.destroy!
           snippet.reload
 
           expect do

spec/models/upload_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe Upload do
       it 'schedules checksum calculation' do
         stub_const('UploadChecksumWorker', spy)
 
-        upload = described_class.create(
+        upload = described_class.create!(
           path: __FILE__,
           size: described_class::CHECKSUM_THRESHOLD + 1.kilobyte,
           model: build_stubbed(:user),
@@ -42,7 +42,7 @@ RSpec.describe Upload do
           store: ObjectStorage::Store::LOCAL
         )
 
-        expect { upload.save }
+        expect { upload.save! }
           .to change { upload.checksum }.from(nil)
           .to(a_string_matching(/\A\h{64}\z/))
       end
@@ -55,7 +55,7 @@ RSpec.describe Upload do
         it 'calls delete_file!' do
           is_expected.to receive(:delete_file!)
 
-          subject.destroy
+          subject.destroy!
         end
       end
     end