Commit 05ef2590 authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'mb_rails_save_bang_fix4' into 'master'

Fix Rails/SaveBang offenses in spec/services/projects/*

See merge request gitlab-org/gitlab!44980
parents a109d6ad 08f87e4d
...@@ -1141,19 +1141,6 @@ Rails/SaveBang: ...@@ -1141,19 +1141,6 @@ Rails/SaveBang:
- 'spec/services/notification_recipients/build_service_spec.rb' - 'spec/services/notification_recipients/build_service_spec.rb'
- 'spec/services/notification_service_spec.rb' - 'spec/services/notification_service_spec.rb'
- 'spec/services/packages/conan/create_package_file_service_spec.rb' - 'spec/services/packages/conan/create_package_file_service_spec.rb'
- 'spec/services/projects/after_rename_service_spec.rb'
- 'spec/services/projects/autocomplete_service_spec.rb'
- 'spec/services/projects/create_service_spec.rb'
- 'spec/services/projects/destroy_service_spec.rb'
- 'spec/services/projects/fork_service_spec.rb'
- 'spec/services/projects/hashed_storage/base_attachment_service_spec.rb'
- 'spec/services/projects/move_access_service_spec.rb'
- 'spec/services/projects/move_project_group_links_service_spec.rb'
- 'spec/services/projects/overwrite_project_service_spec.rb'
- 'spec/services/projects/propagate_service_template_spec.rb'
- 'spec/services/projects/unlink_fork_service_spec.rb'
- 'spec/services/projects/update_pages_service_spec.rb'
- 'spec/services/projects/update_service_spec.rb'
- 'spec/services/reset_project_cache_service_spec.rb' - 'spec/services/reset_project_cache_service_spec.rb'
- 'spec/services/resource_events/change_milestone_service_spec.rb' - 'spec/services/resource_events/change_milestone_service_spec.rb'
- 'spec/services/system_hooks_service_spec.rb' - 'spec/services/system_hooks_service_spec.rb'
......
---
title: Fix Rails/SaveBang offenses in spec/services/projects/*
merge_request: 44980
author: matthewbried
type: other
...@@ -243,7 +243,7 @@ RSpec.describe Projects::AfterRenameService do ...@@ -243,7 +243,7 @@ RSpec.describe Projects::AfterRenameService do
def service_execute def service_execute
# AfterRenameService is called by UpdateService after a successful model.update # AfterRenameService is called by UpdateService after a successful model.update
# the initialization will include before and after paths values # the initialization will include before and after paths values
project.update(path: path_after_rename) project.update!(path: path_after_rename)
described_class.new(project, path_before: path_before_rename, full_path_before: full_path_before_rename).execute described_class.new(project, path_before: path_before_rename, full_path_before: full_path_before_rename).execute
end end
......
...@@ -123,7 +123,7 @@ RSpec.describe Projects::AutocompleteService do ...@@ -123,7 +123,7 @@ RSpec.describe Projects::AutocompleteService do
let!(:subgroup_milestone) { create(:milestone, group: subgroup) } let!(:subgroup_milestone) { create(:milestone, group: subgroup) }
before do before do
project.update(namespace: subgroup) project.update!(namespace: subgroup)
end end
it 'includes project milestones and all acestors milestones' do it 'includes project milestones and all acestors milestones' do
......
...@@ -15,7 +15,7 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -15,7 +15,7 @@ RSpec.describe Projects::CreateService, '#execute' do
end end
it 'creates labels on Project creation if there are templates' do it 'creates labels on Project creation if there are templates' do
Label.create(title: "bug", template: true) Label.create!(title: "bug", template: true)
project = create_project(user, opts) project = create_project(user, opts)
created_label = project.reload.labels.last created_label = project.reload.labels.last
......
...@@ -72,7 +72,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -72,7 +72,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
context 'when project has remote mirrors' do context 'when project has remote mirrors' do
let!(:project) do let!(:project) do
create(:project, :repository, namespace: user.namespace).tap do |project| create(:project, :repository, namespace: user.namespace).tap do |project|
project.remote_mirrors.create(url: 'http://test.com') project.remote_mirrors.create!(url: 'http://test.com')
end end
end end
......
...@@ -179,7 +179,7 @@ RSpec.describe Projects::ForkService do ...@@ -179,7 +179,7 @@ RSpec.describe Projects::ForkService do
context "when origin has git depth specified" do context "when origin has git depth specified" do
before do before do
@from_project.update(ci_default_git_depth: 42) @from_project.update!(ci_default_git_depth: 42)
end end
it "inherits default_git_depth from the origin project" do it "inherits default_git_depth from the origin project" do
...@@ -201,7 +201,7 @@ RSpec.describe Projects::ForkService do ...@@ -201,7 +201,7 @@ RSpec.describe Projects::ForkService do
context "when project has restricted visibility level" do context "when project has restricted visibility level" do
context "and only one visibility level is restricted" do context "and only one visibility level is restricted" do
before do before do
@from_project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL) @from_project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
end end
......
...@@ -45,7 +45,7 @@ RSpec.describe Projects::HashedStorage::BaseAttachmentService do ...@@ -45,7 +45,7 @@ RSpec.describe Projects::HashedStorage::BaseAttachmentService do
describe '#move_folder!' do describe '#move_folder!' do
context 'when old_path is not a directory' do context 'when old_path is not a directory' do
it 'adds information to the logger and returns true' do it 'adds information to the logger and returns true' do
Tempfile.create do |old_path| Tempfile.create do |old_path| # rubocop:disable Rails/SaveBang
new_path = "#{old_path}-new" new_path = "#{old_path}-new"
expect(subject.send(:move_folder!, old_path, new_path)).to be_truthy expect(subject.send(:move_folder!, old_path, new_path)).to be_truthy
......
...@@ -17,9 +17,9 @@ RSpec.describe Projects::MoveAccessService do ...@@ -17,9 +17,9 @@ RSpec.describe Projects::MoveAccessService do
project_with_access.add_maintainer(maintainer_user) project_with_access.add_maintainer(maintainer_user)
project_with_access.add_developer(developer_user) project_with_access.add_developer(developer_user)
project_with_access.add_reporter(reporter_user) project_with_access.add_reporter(reporter_user)
project_with_access.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) project_with_access.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
project_with_access.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) project_with_access.project_group_links.create!(group: developer_group, group_access: Gitlab::Access::DEVELOPER)
project_with_access.project_group_links.create(group: reporter_group, group_access: Gitlab::Access::REPORTER) project_with_access.project_group_links.create!(group: reporter_group, group_access: Gitlab::Access::REPORTER)
end end
subject { described_class.new(target_project, user) } subject { described_class.new(target_project, user) }
...@@ -97,7 +97,7 @@ RSpec.describe Projects::MoveAccessService do ...@@ -97,7 +97,7 @@ RSpec.describe Projects::MoveAccessService do
end end
it 'does not remove remaining group links' do it 'does not remove remaining group links' do
target_project.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) target_project.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
subject.execute(project_with_access, options) subject.execute(project_with_access, options)
......
...@@ -14,9 +14,9 @@ RSpec.describe Projects::MoveProjectGroupLinksService do ...@@ -14,9 +14,9 @@ RSpec.describe Projects::MoveProjectGroupLinksService do
describe '#execute' do describe '#execute' do
before do before do
project_with_groups.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) project_with_groups.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
project_with_groups.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) project_with_groups.project_group_links.create!(group: developer_group, group_access: Gitlab::Access::DEVELOPER)
project_with_groups.project_group_links.create(group: reporter_group, group_access: Gitlab::Access::REPORTER) project_with_groups.project_group_links.create!(group: reporter_group, group_access: Gitlab::Access::REPORTER)
end end
it 'moves the group links from one project to another' do it 'moves the group links from one project to another' do
...@@ -30,8 +30,8 @@ RSpec.describe Projects::MoveProjectGroupLinksService do ...@@ -30,8 +30,8 @@ RSpec.describe Projects::MoveProjectGroupLinksService do
end end
it 'does not move existent group links in the current project' do it 'does not move existent group links in the current project' do
target_project.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) target_project.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
target_project.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) target_project.project_group_links.create!(group: developer_group, group_access: Gitlab::Access::DEVELOPER)
expect(project_with_groups.project_group_links.count).to eq 3 expect(project_with_groups.project_group_links.count).to eq 3
expect(target_project.project_group_links.count).to eq 2 expect(target_project.project_group_links.count).to eq 2
...@@ -55,8 +55,8 @@ RSpec.describe Projects::MoveProjectGroupLinksService do ...@@ -55,8 +55,8 @@ RSpec.describe Projects::MoveProjectGroupLinksService do
let(:options) { { remove_remaining_elements: false } } let(:options) { { remove_remaining_elements: false } }
it 'does not remove remaining project group links' do it 'does not remove remaining project group links' do
target_project.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) target_project.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
target_project.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) target_project.project_group_links.create!(group: developer_group, group_access: Gitlab::Access::DEVELOPER)
subject.execute(project_with_groups, options) subject.execute(project_with_groups, options)
......
...@@ -111,9 +111,9 @@ RSpec.describe Projects::OverwriteProjectService do ...@@ -111,9 +111,9 @@ RSpec.describe Projects::OverwriteProjectService do
create_list(:deploy_keys_project, 2, project: project_from) create_list(:deploy_keys_project, 2, project: project_from)
create_list(:notification_setting, 2, source: project_from) create_list(:notification_setting, 2, source: project_from)
create_list(:users_star_project, 2, project: project_from) create_list(:users_star_project, 2, project: project_from)
project_from.project_group_links.create(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER) project_from.project_group_links.create!(group: maintainer_group, group_access: Gitlab::Access::MAINTAINER)
project_from.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) project_from.project_group_links.create!(group: developer_group, group_access: Gitlab::Access::DEVELOPER)
project_from.project_group_links.create(group: reporter_group, group_access: Gitlab::Access::REPORTER) project_from.project_group_links.create!(group: reporter_group, group_access: Gitlab::Access::REPORTER)
project_from.add_maintainer(maintainer_user) project_from.add_maintainer(maintainer_user)
project_from.add_developer(developer_user) project_from.add_developer(developer_user)
project_from.add_reporter(reporter_user) project_from.add_reporter(reporter_user)
......
...@@ -61,7 +61,7 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin ...@@ -61,7 +61,7 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin
context 'when the original project was deleted' do context 'when the original project was deleted' do
it 'does not fail when the original project is deleted' do it 'does not fail when the original project is deleted' do
source = forked_project.forked_from_project source = forked_project.forked_from_project
source.destroy source.destroy!
forked_project.reload forked_project.reload
expect { subject.execute }.not_to raise_error expect { subject.execute }.not_to raise_error
......
...@@ -95,14 +95,14 @@ RSpec.describe Projects::UpdatePagesService do ...@@ -95,14 +95,14 @@ RSpec.describe Projects::UpdatePagesService do
expect(project.pages_deployed?).to be_truthy expect(project.pages_deployed?).to be_truthy
expect(Dir.exist?(File.join(project.pages_path))).to be_truthy expect(Dir.exist?(File.join(project.pages_path))).to be_truthy
project.destroy project.destroy!
expect(Dir.exist?(File.join(project.pages_path))).to be_falsey expect(Dir.exist?(File.join(project.pages_path))).to be_falsey
expect(ProjectPagesMetadatum.find_by_project_id(project)).to be_nil expect(ProjectPagesMetadatum.find_by_project_id(project)).to be_nil
end end
it 'fails if sha on branch is not latest' do it 'fails if sha on branch is not latest' do
build.update(ref: 'feature') build.update!(ref: 'feature')
expect(execute).not_to eq(:success) expect(execute).not_to eq(:success)
expect(project.pages_metadatum).not_to be_deployed expect(project.pages_metadatum).not_to be_deployed
...@@ -191,7 +191,7 @@ RSpec.describe Projects::UpdatePagesService do ...@@ -191,7 +191,7 @@ RSpec.describe Projects::UpdatePagesService do
it 'fails to remove project pages when no pages is deployed' do it 'fails to remove project pages when no pages is deployed' do
expect(PagesWorker).not_to receive(:perform_in) expect(PagesWorker).not_to receive(:perform_in)
expect(project.pages_deployed?).to be_falsey expect(project.pages_deployed?).to be_falsey
project.destroy project.destroy!
end end
it 'fails if no artifacts' do it 'fails if no artifacts' do
......
...@@ -141,7 +141,7 @@ RSpec.describe Projects::UpdateService do ...@@ -141,7 +141,7 @@ RSpec.describe Projects::UpdateService do
let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
before do before do
project.update(namespace: group, visibility_level: group.visibility_level) project.update!(namespace: group, visibility_level: group.visibility_level)
end end
it 'does not update project visibility level' do it 'does not update project visibility level' do
...@@ -256,7 +256,7 @@ RSpec.describe Projects::UpdateService do ...@@ -256,7 +256,7 @@ RSpec.describe Projects::UpdateService do
end end
it 'handles empty project feature attributes' do it 'handles empty project feature attributes' do
project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED)
result = update_project(project, user, { name: 'test1' }) result = update_project(project, user, { name: 'test1' })
...@@ -267,7 +267,7 @@ RSpec.describe Projects::UpdateService do ...@@ -267,7 +267,7 @@ RSpec.describe Projects::UpdateService do
context 'when enabling a wiki' do context 'when enabling a wiki' do
it 'creates a wiki' do it 'creates a wiki' do
project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED)
TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path) TestEnv.rm_storage_dir(project.repository_storage, project.wiki.path)
result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED }) result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED })
...@@ -278,7 +278,7 @@ RSpec.describe Projects::UpdateService do ...@@ -278,7 +278,7 @@ RSpec.describe Projects::UpdateService do
end end
it 'logs an error and creates a metric when wiki can not be created' do it 'logs an error and creates a metric when wiki can not be created' do
project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED)
expect_any_instance_of(ProjectWiki).to receive(:wiki).and_raise(Wiki::CouldNotCreateWikiError) expect_any_instance_of(ProjectWiki).to receive(:wiki).and_raise(Wiki::CouldNotCreateWikiError)
expect_any_instance_of(described_class).to receive(:log_error).with("Could not create wiki for #{project.full_name}") expect_any_instance_of(described_class).to receive(:log_error).with("Could not create wiki for #{project.full_name}")
......
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