Resolve Rails/SaveBang rubocop exceptions in spec/models

- continue to resolve exceptions

.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 @@
     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 @@ def call_with_mismatched_designs
     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 @@
     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 @@
       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 @@
     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 @@
       context 'environment is already stopped' do
         before do
-          environment.update(state: :stopped)
+          environment.update!(state: :stopped)
         end
         it do
@@ -1502,7 +1502,7 @@
       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 @@
       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 @@
     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 @@
       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 @@
     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 @@
       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 @@ def visible_to_all_except(*roles)
     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 @@ def visible_to_all_except(*roles)
     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 @@
   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 @@
       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 @@
     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 @@
     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 @@
       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 @@
       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 @@
       subject do
         recorded_queries.record do
-          group.update(parent: new_parent)
+          group.update!(parent: new_parent)
         end
       end
@@ -496,7 +496,7 @@
       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 @@
       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 @@
         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 @@ def setup_group_members(group)
           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 @@ def setup_group_members(group)
       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 @@ def setup_group_members(group)
       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 @@
         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 @@
             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 @@
             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 @@
       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 @@
       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 @@
           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 @@
         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
@@ -721,7 +721,7 @@
       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
@@ -732,7 +732,7 @@
             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
@@ -749,7 +749,7 @@
             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
@@ -979,7 +979,7 @@ def expect_project_directories_at(namespace_path, with_pages: true)
       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
@@ -994,7 +994,7 @@ def expect_project_directories_at(namespace_path, with_pages: true)
       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}"
@@ -1006,7 +1006,7 @@ def expect_project_directories_at(namespace_path, with_pages: true)
         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
@@ -1040,7 +1040,7 @@ def project_rugged(project)
       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
@@ -1048,7 +1048,7 @@ def project_rugged(project)
       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
@@ -1062,7 +1062,7 @@ def project_rugged(project)
         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
@@ -1070,7 +1070,7 @@ def project_rugged(project)
         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
@@ -1083,7 +1083,7 @@ def project_rugged(project)
         expect(File.exist?(path_in_dir)).to be(false)
-        namespace.destroy
+        namespace.destroy!
         expect(File.exist?(deleted_path_in_dir)).to be(false)
       end
@@ -1609,7 +1609,7 @@ def project_rugged(project)
       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
@@ -1620,7 +1620,7 @@ def project_rugged(project)
         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
@@ -1631,7 +1631,7 @@ def project_rugged(project)
         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
@@ -1643,7 +1643,7 @@ def project_rugged(project)
         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 @@
         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 @@
         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 @@ def retrieve_participants
     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 @@ def retrieve_participants
     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 @@ def retrieve_participants
     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 @@ def expect_expiration(noteable)
     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 @@ def expect_expiration(noteable)
     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 @@
         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 @@
     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 @@
       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 @@
       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 @@
       end
       with_them do
         it 'is valid with a string of comma separated values' do
-          user_list = described_class.create(user_xids: valid_value)
-          expect(user_list.errors[:user_xids]).to be_empty
+          user_list = build(:operations_feature_flag_user_list, user_xids: valid_value)
+          expect(user_list).to be_valid
         end
       end
@@ -31,9 +31,10 @@
       end
       with_them do
         it 'automatically casts values of other types' do
-          user_list = described_class.create(user_xids: typecast_value)
-          expect(user_list.errors[:user_xids]).to be_empty
+          user_list = build(:operations_feature_flag_user_list, user_xids: typecast_value)
+
+          expect(user_list).to be_valid
           expect(user_list.user_xids).to eq(typecast_value.to_s)
         end
       end
@@ -45,7 +46,9 @@
       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 @@
   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.destroy
+      user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2')
+      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 @@
     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 @@
       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 @@
   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 @@
 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 @@
   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 @@
     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 @@
   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 @@
     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 @@
       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 @@
   describe '#name' do
     context 'name is nil' do
       before do
-        release.update(name: nil)
+        release.update!(name: nil)
       end
       it 'returns tag' do