Commit d802e948 authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'mb_rails_save_bang_fix5' into 'master'

Fix Rails/SaveBang offenses for spec/services/* and spec/sidekiq/* [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!45391
parents 77038586 24611f5c
......@@ -1125,33 +1125,6 @@ Rails/SaveBang:
- 'spec/requests/api/labels_spec.rb'
- 'spec/requests/api/project_import_spec.rb'
- 'spec/requests/projects/cycle_analytics_events_spec.rb'
- 'spec/services/auth/container_registry_authentication_service_spec.rb'
- 'spec/services/auto_merge/base_service_spec.rb'
- 'spec/services/auto_merge_service_spec.rb'
- 'spec/services/clusters/update_service_spec.rb'
- 'spec/services/deployments/after_create_service_spec.rb'
- 'spec/services/design_management/generate_image_versions_service_spec.rb'
- 'spec/services/discussions/resolve_service_spec.rb'
- 'spec/services/draft_notes/destroy_service_spec.rb'
- 'spec/services/emails/confirm_service_spec.rb'
- 'spec/services/groups/destroy_service_spec.rb'
- 'spec/services/groups/import_export/import_service_spec.rb'
- 'spec/services/labels/promote_service_spec.rb'
- 'spec/services/notes/create_service_spec.rb'
- 'spec/services/notification_recipients/build_service_spec.rb'
- 'spec/services/notification_service_spec.rb'
- 'spec/services/packages/conan/create_package_file_service_spec.rb'
- 'spec/services/reset_project_cache_service_spec.rb'
- 'spec/services/resource_events/change_milestone_service_spec.rb'
- 'spec/services/system_hooks_service_spec.rb'
- 'spec/services/system_note_service_spec.rb'
- 'spec/services/system_notes/issuables_service_spec.rb'
- 'spec/services/todo_service_spec.rb'
- 'spec/services/todos/destroy/confidential_issue_service_spec.rb'
- 'spec/services/users/destroy_service_spec.rb'
- 'spec/services/users/repair_ldap_blocked_service_spec.rb'
- 'spec/services/verify_pages_domain_service_spec.rb'
- 'spec/sidekiq/cron/job_gem_dependency_spec.rb'
# Offense count: 187
# Cop supports --auto-correct.
......
---
title: Fix Rails/SaveBang offenses for spec/services/* and spec/sidekiq/*
merge_request: 45391
author: matthewbried
type: other
......@@ -642,7 +642,7 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do
let_it_be(:project) { create(:project, :public, container_registry_enabled: false) }
before do
project.update(container_registry_enabled: false)
project.update!(container_registry_enabled: false)
end
context 'disallow when pulling' do
......
......@@ -131,7 +131,7 @@ RSpec.describe AutoMerge::BaseService do
end
describe '#update' do
subject { service.update(merge_request) }
subject { service.update(merge_request) } # rubocop:disable Rails/SaveBang
let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
......
......@@ -148,7 +148,7 @@ RSpec.describe AutoMergeService do
end
describe '#update' do
subject { service.update(merge_request) }
subject { service.update(merge_request) } # rubocop:disable Rails/SaveBang
context 'when auto merge is enabled' do
let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
......
......@@ -197,7 +197,7 @@ RSpec.describe Clusters::UpdateService do
context 'manangement_project is outside of the namespace scope' do
before do
management_project.update(group: create(:group))
management_project.update!(group: create(:group))
end
let(:params) do
......@@ -224,7 +224,7 @@ RSpec.describe Clusters::UpdateService do
context 'manangement_project is outside of the namespace scope' do
before do
management_project.update(group: create(:group))
management_project.update!(group: create(:group))
end
let(:params) do
......
......@@ -44,7 +44,7 @@ RSpec.describe DesignManagement::GenerateImageVersionsService do
end
it 'logs if the raw image cannot be found' do
version.designs.first.update(filename: 'foo.png')
version.designs.first.update!(filename: 'foo.png')
expect(Gitlab::AppLogger).to receive(:error).with("No design file found for Action: #{action.id}")
......
......@@ -40,11 +40,11 @@ RSpec.describe Discussions::ResolveService do
context 'with a project that requires all discussion to be resolved' do
before do
project.update(only_allow_merge_if_all_discussions_are_resolved: true)
project.update!(only_allow_merge_if_all_discussions_are_resolved: true)
end
after do
project.update(only_allow_merge_if_all_discussions_are_resolved: false)
project.update!(only_allow_merge_if_all_discussions_are_resolved: false)
end
let_it_be(:other_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
......
......@@ -22,7 +22,7 @@ RSpec.describe DraftNotes::DestroyService do
it 'destroys all draft notes for a user in a merge request' do
create_list(:draft_note, 2, merge_request: merge_request, author: user)
expect { destroy }.to change { DraftNote.count }.by(-2)
expect { destroy }.to change { DraftNote.count }.by(-2) # rubocop:disable Rails/SaveBang
expect(DraftNote.count).to eq(0)
end
......@@ -45,7 +45,7 @@ RSpec.describe DraftNotes::DestroyService do
allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
destroy
destroy # rubocop:disable Rails/SaveBang
end
end
end
......
......@@ -9,7 +9,7 @@ RSpec.describe Emails::ConfirmService do
describe '#execute' do
it 'enqueues a background job to send confirmation email again' do
email = user.emails.create(email: 'new@email.com')
email = user.emails.create!(email: 'new@email.com')
expect { service.execute(email) }.to have_enqueued_job.on_queue('mailers')
end
......
......@@ -95,7 +95,7 @@ RSpec.describe Groups::DestroyService do
context 'projects in pending_delete' do
before do
project.pending_delete = true
project.save
project.save!
end
it_behaves_like 'group destruction', false
......
......@@ -63,7 +63,7 @@ RSpec.describe Groups::ImportExport::ImportService do
before do
stub_feature_flags(group_import_ndjson: false)
ImportExportUpload.create(group: group, import_file: import_file)
ImportExportUpload.create!(group: group, import_file: import_file)
allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
allow(import_logger).to receive(:error)
......@@ -105,7 +105,7 @@ RSpec.describe Groups::ImportExport::ImportService do
subject { service.execute }
before do
ImportExportUpload.create(group: group, import_file: import_file)
ImportExportUpload.create!(group: group, import_file: import_file)
allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
allow(import_logger).to receive(:error)
......@@ -216,7 +216,7 @@ RSpec.describe Groups::ImportExport::ImportService do
subject { service.execute }
before do
ImportExportUpload.create(group: group, import_file: import_file)
ImportExportUpload.create!(group: group, import_file: import_file)
allow(Gitlab::Import::Logger).to receive(:build).and_return(import_logger)
allow(import_logger).to receive(:error)
......
......@@ -89,10 +89,10 @@ RSpec.describe Labels::PromoteService do
it 'keeps users\' subscriptions' do
user2 = create(:user)
project_label_1_1.subscriptions.create(user: user, subscribed: true)
project_label_2_1.subscriptions.create(user: user, subscribed: true)
project_label_3_2.subscriptions.create(user: user, subscribed: true)
project_label_2_1.subscriptions.create(user: user2, subscribed: true)
project_label_1_1.subscriptions.create!(user: user, subscribed: true)
project_label_2_1.subscriptions.create!(user: user, subscribed: true)
project_label_3_2.subscriptions.create!(user: user, subscribed: true)
project_label_2_1.subscriptions.create!(user: user2, subscribed: true)
expect { service.execute(project_label_1_1) }.to change { Subscription.count }.from(4).to(3)
......
......@@ -286,7 +286,7 @@ RSpec.describe Notes::CreateService do
QuickAction.new(
action_text: "/wip",
before_action: -> {
issuable.reload.update(title: "title")
issuable.reload.update!(title: "title")
},
expectation: ->(issuable, can_use_quick_action) {
expect(issuable.work_in_progress?).to eq(can_use_quick_action)
......@@ -296,7 +296,7 @@ RSpec.describe Notes::CreateService do
QuickAction.new(
action_text: "/wip",
before_action: -> {
issuable.reload.update(title: "WIP: title")
issuable.reload.update!(title: "WIP: title")
},
expectation: ->(noteable, can_use_quick_action) {
expect(noteable.work_in_progress?).not_to eq(can_use_quick_action)
......
......@@ -44,7 +44,7 @@ RSpec.describe NotificationRecipients::BuildService do
context 'when there are multiple subscribers' do
def create_user
subscriber = create(:user)
issue.subscriptions.create(user: subscriber, project: project, subscribed: true)
issue.subscriptions.create!(user: subscriber, project: project, subscribed: true)
end
include_examples 'no N+1 queries'
......@@ -96,7 +96,7 @@ RSpec.describe NotificationRecipients::BuildService do
context 'when there are multiple subscribers' do
def create_user
subscriber = create(:user)
merge_request.subscriptions.create(user: subscriber, project: project, subscribed: true)
merge_request.subscriptions.create!(user: subscriber, project: project, subscribed: true)
end
include_examples 'no N+1 queries'
......
......@@ -361,7 +361,7 @@ RSpec.describe NotificationService, :mailer do
context 'where the project has disabled the feature' do
before do
project.update(service_desk_enabled: false)
project.update!(service_desk_enabled: false)
end
it_should_not_email!
......@@ -453,7 +453,7 @@ RSpec.describe NotificationService, :mailer do
context 'by note' do
before do
note.author = @u_lazy_participant
note.save
note.save!
end
it { expect { subject }.not_to have_enqueued_email(@u_lazy_participant.id, note.id, mail: "note_issue_email") }
......@@ -467,7 +467,7 @@ RSpec.describe NotificationService, :mailer do
note.project.namespace_id = group.id
group.add_user(@u_watcher, GroupMember::MAINTAINER)
group.add_user(@u_custom_global, GroupMember::MAINTAINER)
note.project.save
note.project.save!
@u_watcher.notification_settings_for(note.project).participating!
@u_watcher.notification_settings_for(group).global!
......@@ -1719,7 +1719,7 @@ RSpec.describe NotificationService, :mailer do
before do
merge_request.author = participant
merge_request.save
merge_request.save!
notification.new_merge_request(merge_request, @u_disabled)
end
......@@ -1962,7 +1962,7 @@ RSpec.describe NotificationService, :mailer do
describe 'when merge_when_pipeline_succeeds is true' do
before do
merge_request.update(
merge_request.update!(
merge_when_pipeline_succeeds: true,
merge_user: create(:user)
)
......@@ -2725,7 +2725,7 @@ RSpec.describe NotificationService, :mailer do
before do
group = create(:group)
project.update(group: group)
project.update!(group: group)
create(:email, :confirmed, user: u_custom_notification_enabled, email: group_notification_email)
create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email)
......@@ -2761,7 +2761,7 @@ RSpec.describe NotificationService, :mailer do
before do
group = create(:group)
project.update(group: group)
project.update!(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end
......@@ -2821,7 +2821,7 @@ RSpec.describe NotificationService, :mailer do
context 'when the creator has no read_build access' do
before do
pipeline = create_pipeline(u_member, :failed)
project.update(public_builds: false)
project.update!(public_builds: false)
project.team.truncate
notification.pipeline_finished(pipeline)
end
......@@ -2855,7 +2855,7 @@ RSpec.describe NotificationService, :mailer do
before do
group = create(:group)
project.update(group: group)
project.update!(group: group)
create(:email, :confirmed, user: u_member, email: group_notification_email)
create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
end
......@@ -3212,7 +3212,7 @@ RSpec.describe NotificationService, :mailer do
# with different notification settings
def build_group(project, visibility: :public)
group = create_nested_group(visibility)
project.update(namespace_id: group.id)
project.update!(namespace_id: group.id)
# Group member: global=disabled, group=watch
@g_watcher ||= create_user_with_notification(:watch, 'group_watcher', project.group)
......@@ -3277,11 +3277,11 @@ RSpec.describe NotificationService, :mailer do
end
def add_user_subscriptions(issuable)
issuable.subscriptions.create(user: @unsubscribed_mentioned, project: project, subscribed: false)
issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false)
issuable.subscriptions.create!(user: @unsubscribed_mentioned, project: project, subscribed: false)
issuable.subscriptions.create!(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create!(user: @subscribed_participant, project: project, subscribed: true)
issuable.subscriptions.create!(user: @unsubscriber, project: project, subscribed: false)
# Make the watcher a subscriber to detect dupes
issuable.subscriptions.create(user: @watcher_and_subscriber, project: project, subscribed: true)
issuable.subscriptions.create!(user: @watcher_and_subscriber, project: project, subscribed: true)
end
end
......@@ -100,7 +100,7 @@ RSpec.describe Packages::Conan::CreatePackageFileService do
end
let(:tmp_object) do
fog_connection.directories.new(key: 'packages').files.create(
fog_connection.directories.new(key: 'packages').files.create( # rubocop:disable Rails/SaveBang
key: "tmp/uploads/#{file_name}",
body: 'content'
)
......
......@@ -20,7 +20,7 @@ RSpec.describe ResetProjectCacheService do
context 'when project cache_index is a numeric value' do
before do
project.update(jobs_cache_index: 1)
project.update!(jobs_cache_index: 1)
end
it 'increments project cache index' do
......
......@@ -11,7 +11,7 @@ RSpec.describe ResourceEvents::ChangeMilestoneService do
[:issue, :merge_request].each do |issuable|
it_behaves_like 'timebox(milestone or iteration) resource events creator', ResourceMilestoneEvent do
let_it_be(:resource) { create(issuable) }
let_it_be(:resource) { create(issuable) } # rubocop:disable Rails/SaveBang
end
end
end
......@@ -85,7 +85,7 @@ RSpec.describe SystemHooksService do
end
it 'handles nil datetime columns' do
user.update(created_at: nil, updated_at: nil)
user.update!(created_at: nil, updated_at: nil)
data = event_data(user, :destroy)
expect(data[:created_at]).to be(nil)
......
......@@ -378,13 +378,13 @@ RSpec.describe SystemNoteService do
noteable_types.each do |type|
context "when noteable is a #{type}" do
it "blocks cross reference when #{type.underscore}_events is false" do
jira_tracker.update("#{type}_events" => false)
jira_tracker.update!("#{type}_events" => false)
expect(cross_reference(type)).to eq(s_('JiraService|Events for %{noteable_model_name} are disabled.') % { noteable_model_name: type.pluralize.humanize.downcase })
end
it "creates cross reference when #{type.underscore}_events is true" do
jira_tracker.update("#{type}_events" => true)
jira_tracker.update!("#{type}_events" => true)
expect(cross_reference(type)).to eq(success_message)
end
......
......@@ -373,7 +373,7 @@ RSpec.describe ::SystemNotes::IssuablesService do
before do
# Mention issue (noteable) from commit0
system_note = service.cross_reference(commit0)
system_note.update(note: system_note.note.capitalize)
system_note.update!(note: system_note.note.capitalize)
end
it 'is truthy when already mentioned' do
......@@ -407,7 +407,7 @@ RSpec.describe ::SystemNotes::IssuablesService do
before do
# Mention commit1 from commit0
system_note = service.cross_reference(commit1)
system_note.update(note: system_note.note.capitalize)
system_note.update!(note: system_note.note.capitalize)
end
it 'is truthy when already mentioned' do
......@@ -436,7 +436,7 @@ RSpec.describe ::SystemNotes::IssuablesService do
context 'legacy capitalized cross reference' do
before do
system_note = service.cross_reference(commit0)
system_note.update(note: system_note.note.capitalize)
system_note.update!(note: system_note.note.capitalize)
end
it 'is true when a fork mentions an external issue' do
......@@ -582,7 +582,7 @@ RSpec.describe ::SystemNotes::IssuablesService do
it 'creates the note text correctly' do
[:issue, :merge_request].each do |type|
issuable = create(type)
issuable = create(type) # rubocop:disable Rails/SaveBang
service = described_class.new(noteable: issuable, author: author)
expect(service.discussion_lock.note)
......
......@@ -145,7 +145,7 @@ RSpec.describe TodoService do
end
it 'creates correct todos for each valid user based on the type of mention' do
issue.update(description: directly_addressed_and_mentioned)
issue.update!(description: directly_addressed_and_mentioned)
service.new_issue(issue, author)
......@@ -222,7 +222,7 @@ RSpec.describe TodoService do
end
it 'creates a todo for each valid user not included in skip_users based on the type of mention' do
issue.update(description: directly_addressed_and_mentioned)
issue.update!(description: directly_addressed_and_mentioned)
service.update_issue(issue, author, skip_users)
......@@ -291,7 +291,7 @@ RSpec.describe TodoService do
context 'issues with a task list' do
it 'does not create todo when tasks are marked as completed' do
issue.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
issue.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_issue(issue, author)
......@@ -304,7 +304,7 @@ RSpec.describe TodoService do
end
it 'does not create directly addressed todo when tasks are marked as completed' do
addressed_issue.update(description: "#{directly_addressed}\n- [x] Task 1\n- [x] Task 2\n")
addressed_issue.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [x] Task 2\n")
service.update_issue(addressed_issue, author)
......@@ -317,7 +317,7 @@ RSpec.describe TodoService do
end
it 'does not raise an error when description not change' do
issue.update(title: 'Sample')
issue.update!(title: 'Sample')
expect { service.update_issue(issue, author) }.not_to raise_error
end
......@@ -427,7 +427,7 @@ RSpec.describe TodoService do
end
it 'creates a todo for each valid user based on the type of mention' do
note.update(note: directly_addressed_and_mentioned)
note.update!(note: directly_addressed_and_mentioned)
service.new_note(note, john_doe)
......@@ -694,7 +694,7 @@ RSpec.describe TodoService do
end
it 'creates a todo for each valid user based on the type of mention' do
mr_assigned.update(description: directly_addressed_and_mentioned)
mr_assigned.update!(description: directly_addressed_and_mentioned)
service.new_merge_request(mr_assigned, author)
......@@ -726,7 +726,7 @@ RSpec.describe TodoService do
end
it 'creates a todo for each valid user not included in skip_users based on the type of mention' do
mr_assigned.update(description: directly_addressed_and_mentioned)
mr_assigned.update!(description: directly_addressed_and_mentioned)
service.update_merge_request(mr_assigned, author, skip_users)
......@@ -772,7 +772,7 @@ RSpec.describe TodoService do
context 'with a task list' do
it 'does not create todo when tasks are marked as completed' do
mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
mr_assigned.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_merge_request(mr_assigned, author)
......@@ -786,7 +786,7 @@ RSpec.describe TodoService do
end
it 'does not create directly addressed todo when tasks are marked as completed' do
addressed_mr_assigned.update(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
addressed_mr_assigned.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
service.update_merge_request(addressed_mr_assigned, author)
......@@ -800,7 +800,7 @@ RSpec.describe TodoService do
end
it 'does not raise an error when description not change' do
mr_assigned.update(title: 'Sample')
mr_assigned.update!(title: 'Sample')
expect { service.update_merge_request(mr_assigned, author) }.not_to raise_error
end
......@@ -883,7 +883,7 @@ RSpec.describe TodoService do
end
it 'creates a pending todo for each merge_participant' do
mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin)
mr_unassigned.update!(merge_when_pipeline_succeeds: true, merge_user: admin)
service.merge_request_became_unmergeable(mr_unassigned)
merge_participants.each do |participant|
......@@ -991,7 +991,7 @@ RSpec.describe TodoService do
end
it 'creates a todo for each valid user not included in skip_users based on the type of mention' do
note.update(note: directly_addressed_and_mentioned)
note.update!(note: directly_addressed_and_mentioned)
service.update_note(note, author, skip_users)
......
......@@ -37,7 +37,7 @@ RSpec.describe Todos::Destroy::ConfidentialIssueService do
context 'when provided issue is not confidential' do
it 'does not remove any todos' do
issue_1.update(confidential: false)
issue_1.update!(confidential: false)
expect { subject }.not_to change { Todo.count }
end
......
......@@ -108,7 +108,7 @@ RSpec.describe Users::DestroyService do
context 'projects in pending_delete' do
before do
project.pending_delete = true
project.save
project.save!
end
it 'destroys a project in pending_delete' do
......@@ -310,7 +310,7 @@ RSpec.describe Users::DestroyService do
it 'of group_members' do
group_member = create(:group_member)
group_member.group.group_members.create(user: user, access_level: 40)
group_member.group.group_members.create!(user: user, access_level: 40)
expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:find).once
expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:initialize).once
......
......@@ -10,7 +10,7 @@ RSpec.describe Users::RepairLdapBlockedService do
describe '#execute' do
it 'changes to normal block after destroying last ldap identity' do
identity.destroy
identity.destroy!
service.execute
expect(user.reload).not_to be_ldap_blocked
......
......@@ -189,7 +189,7 @@ RSpec.describe VerifyPagesDomainService do
let(:domain) { build(:pages_domain, :expired, :with_missing_chain) }
before do
domain.save(validate: false)
domain.save!(validate: false)
end
it 'can be disabled' do
......
......@@ -7,7 +7,7 @@ RSpec.describe Sidekiq::Cron::Job do
context 'when Fugit depends on ZoTime or EoTime' do
before do
described_class
.create(name: 'TestCronWorker',
.create(name: 'TestCronWorker', # rubocop:disable Rails/SaveBang
cron: Settings.cron_jobs[:pipeline_schedule_worker]['cron'],
class: Settings.cron_jobs[:pipeline_schedule_worker]['job_class'])
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment