diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9047ba10b547d2e7e50eee88cf6b7236dbf9d693..0c92efbcd8a10c364ca32a25af581a54cf28f8aa 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -763,9 +763,6 @@ Rails/SaveBang: - 'ee/spec/requests/lfs_http_spec.rb' - 'ee/spec/services/approval_rules/finalize_service_spec.rb' - 'ee/spec/services/approval_rules/update_service_spec.rb' - - 'ee/spec/services/ci/minutes/email_notification_service_spec.rb' - - 'ee/spec/services/ci/process_build_service_spec.rb' - - 'ee/spec/services/ci/register_job_service_spec.rb' - 'ee/spec/services/ee/boards/issues/create_service_spec.rb' - 'ee/spec/services/ee/boards/issues/list_service_spec.rb' - 'ee/spec/services/ee/boards/lists/list_service_spec.rb' @@ -1146,11 +1143,6 @@ Rails/SaveBang: - '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/ci/create_cross_project_pipeline_service_spec.rb' - - 'spec/services/ci/create_pipeline_service_spec.rb' - - 'spec/services/ci/register_job_service_spec.rb' - - 'spec/services/ci/retry_build_service_spec.rb' - - 'spec/services/ci/update_runner_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' diff --git a/changelogs/unreleased/rails-save-bang-20.yml b/changelogs/unreleased/rails-save-bang-20.yml new file mode 100644 index 0000000000000000000000000000000000000000..04d9b6223d7ebb9da244553a3f46de1cb6263816 --- /dev/null +++ b/changelogs/unreleased/rails-save-bang-20.yml @@ -0,0 +1,5 @@ +--- +title: Fix Rails/SaveBang offenses for */spec/services/ci/* +merge_request: 41317 +author: Rajendra Kadam +type: other diff --git a/ee/spec/services/ci/minutes/email_notification_service_spec.rb b/ee/spec/services/ci/minutes/email_notification_service_spec.rb index 04c2fae2d752ba994937197cbfccfc597f8706c5..51faba75b4c075b6e30548b5fa71a75b840c98c3 100644 --- a/ee/spec/services/ci/minutes/email_notification_service_spec.rb +++ b/ee/spec/services/ci/minutes/email_notification_service_spec.rb @@ -42,7 +42,7 @@ context 'with a personal namespace' do before do - namespace.update(owner_id: user.id) + namespace.update!(owner_id: user.id) end it_behaves_like 'namespace with available CI minutes' do diff --git a/ee/spec/services/ci/process_build_service_spec.rb b/ee/spec/services/ci/process_build_service_spec.rb index 9e5574995cc40033ed611e2de5ea858ea667bd36..d3f4a68b20904b33e0a52070fad4fa33671b2593 100644 --- a/ee/spec/services/ci/process_build_service_spec.rb +++ b/ee/spec/services/ci/process_build_service_spec.rb @@ -45,7 +45,7 @@ context 'when user has access to the environment' do before do - protected_environment.deploy_access_levels.create(user: user) + protected_environment.deploy_access_levels.create!(user: user) end it 'enqueues the build' do diff --git a/ee/spec/services/ci/register_job_service_spec.rb b/ee/spec/services/ci/register_job_service_spec.rb index 133dba2f3f3d9c06b7d9b8f7140ead3b0618e11c..133a1682efc00723b5f6985006c6b22ff196ed0e 100644 --- a/ee/spec/services/ci/register_job_service_spec.rb +++ b/ee/spec/services/ci/register_job_service_spec.rb @@ -13,7 +13,7 @@ subject { described_class.new(shared_runner).execute } before do - project.update(shared_runners_enabled: false) + project.update!(shared_runners_enabled: false) end it 'result is valid if replica did caught-up' do @@ -73,7 +73,7 @@ context 'and project is public' do context 'and public projects cost factor is 0 (default)' do before do - project.update(visibility_level: Project::PUBLIC) + project.update!(visibility_level: Project::PUBLIC) end it_behaves_like 'returns a build', 11 @@ -81,8 +81,8 @@ context 'and public projects cost factor is > 0' do before do - project.update(visibility_level: Project::PUBLIC) - shared_runner.update(public_projects_minutes_cost_factor: 1.1) + project.update!(visibility_level: Project::PUBLIC) + shared_runner.update!(public_projects_minutes_cost_factor: 1.1) end it_behaves_like 'does not return a build', 11 @@ -92,7 +92,7 @@ context 'and extra shared runners minutes purchased' do before do - project.namespace.update(extra_shared_runners_minutes_limit: 10) + project.namespace.update!(extra_shared_runners_minutes_limit: 10) end context 'and usage is below the combined limit' do @@ -107,12 +107,12 @@ context 'when limit set at namespace level' do before do - project.namespace.update(shared_runners_minutes_limit: 5) + project.namespace.update!(shared_runners_minutes_limit: 5) end context 'and limit set to unlimited' do before do - project.namespace.update(shared_runners_minutes_limit: 0) + project.namespace.update!(shared_runners_minutes_limit: 0) end it_behaves_like 'returns a build', 10 @@ -128,7 +128,7 @@ context 'and extra shared runners minutes purchased' do before do - project.namespace.update(extra_shared_runners_minutes_limit: 5) + project.namespace.update!(extra_shared_runners_minutes_limit: 5) end context 'and usage is below the combined limit' do @@ -145,7 +145,7 @@ context 'and namespace limit lower than global limit' do before do stub_application_setting(shared_runners_minutes: 10) - project.namespace.update(shared_runners_minutes_limit: 5) + project.namespace.update!(shared_runners_minutes_limit: 5) end it_behaves_like 'does not return a build', 6 @@ -154,7 +154,7 @@ context 'and namespace limit higher than global limit' do before do stub_application_setting(shared_runners_minutes: 5) - project.namespace.update(shared_runners_minutes_limit: 10) + project.namespace.update!(shared_runners_minutes_limit: 10) end it_behaves_like 'returns a build', 6 @@ -168,7 +168,7 @@ context 'and usage below the limit on root namespace' do before do - root_ancestor.update(shared_runners_minutes_limit: 10) + root_ancestor.update!(shared_runners_minutes_limit: 10) end it_behaves_like 'returns a build', 9 @@ -177,9 +177,9 @@ context 'and usage above the limit on root namespace' do before do # limit is ignored on subnamespace - group.update(shared_runners_minutes_limit: 20) + group.update_columns(shared_runners_minutes_limit: 20) - root_ancestor.update(shared_runners_minutes_limit: 10) + root_ancestor.update!(shared_runners_minutes_limit: 10) root_ancestor.create_namespace_statistics( shared_runners_seconds: 60 * 11) end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 7c8aecb41aedc1f82c6323dfe752aaf504beb1a0..e0893ed6de3b9513c3fab260171b3194c0698915 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -223,7 +223,7 @@ def execute_service( context 'auto-cancel enabled' do before do - project.update(auto_cancel_pending_pipelines: 'enabled') + project.update!(auto_cancel_pending_pipelines: 'enabled') end it 'does not cancel HEAD pipeline' do @@ -248,7 +248,7 @@ def execute_service( end it 'cancel created outdated pipelines', :sidekiq_might_not_need_inline do - pipeline_on_previous_commit.update(status: 'created') + pipeline_on_previous_commit.update!(status: 'created') pipeline expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) @@ -439,7 +439,7 @@ def execute_service( context 'auto-cancel disabled' do before do - project.update(auto_cancel_pending_pipelines: 'disabled') + project.update!(auto_cancel_pending_pipelines: 'disabled') end it 'does not auto cancel pending non-HEAD pipelines' do diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 921f5ba4c7e820a25c9512155774e81c20875cb1..0cdc8d2c87085dee7f4f547b31abafff7f3ea7ef 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -15,14 +15,14 @@ module Ci describe '#execute' do context 'runner follow tag list' do it "picks build with the same tag" do - pending_job.update(tag_list: ["linux"]) - specific_runner.update(tag_list: ["linux"]) + pending_job.update!(tag_list: ["linux"]) + specific_runner.update!(tag_list: ["linux"]) expect(execute(specific_runner)).to eq(pending_job) end it "does not pick build with different tag" do - pending_job.update(tag_list: ["linux"]) - specific_runner.update(tag_list: ["win32"]) + pending_job.update!(tag_list: ["linux"]) + specific_runner.update!(tag_list: ["win32"]) expect(execute(specific_runner)).to be_falsey end @@ -31,24 +31,24 @@ module Ci end it "does not pick build with tag" do - pending_job.update(tag_list: ["linux"]) + pending_job.update!(tag_list: ["linux"]) expect(execute(specific_runner)).to be_falsey end it "pick build without tag" do - specific_runner.update(tag_list: ["win32"]) + specific_runner.update!(tag_list: ["win32"]) expect(execute(specific_runner)).to eq(pending_job) end end context 'deleted projects' do before do - project.update(pending_delete: true) + project.update!(pending_delete: true) end context 'for shared runners' do before do - project.update(shared_runners_enabled: true) + project.update!(shared_runners_enabled: true) end it 'does not pick a build' do @@ -65,7 +65,7 @@ module Ci context 'allow shared runners' do before do - project.update(shared_runners_enabled: true) + project.update!(shared_runners_enabled: true) end context 'for multiple builds' do @@ -131,7 +131,7 @@ module Ci context 'disallow shared runners' do before do - project.update(shared_runners_enabled: false) + project.update!(shared_runners_enabled: false) end context 'shared runner' do @@ -152,7 +152,7 @@ module Ci context 'disallow when builds are disabled' do before do - project.update(shared_runners_enabled: true, group_runners_enabled: true) + project.update!(shared_runners_enabled: true, group_runners_enabled: true) project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) end @@ -591,8 +591,8 @@ module Ci .with(:job_queue_duration_seconds, anything, anything, anything) .and_return(job_queue_duration_seconds) - project.update(shared_runners_enabled: true) - pending_job.update(created_at: current_time - 3600, queued_at: current_time - 1800) + project.update!(shared_runners_enabled: true) + pending_job.update!(created_at: current_time - 3600, queued_at: current_time - 1800) end shared_examples 'attempt counter collector' do @@ -661,7 +661,7 @@ module Ci context 'when pending job with queued_at=nil is used' do before do - pending_job.update(queued_at: nil) + pending_job.update!(queued_at: nil) end it_behaves_like 'attempt counter collector' diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 9f419437e7ddd5e45094c05dd610438a2ba9fba5..9ff6367b5a5451e5481202c2a7862b178b301741 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -70,7 +70,7 @@ # Make sure that build has both `stage_id` and `stage` because FactoryBot # can reset one of the fields when assigning another. We plan to deprecate # and remove legacy `stage` column in the future. - build.update(stage: 'test', stage_id: stage.id) + build.update!(stage: 'test', stage_id: stage.id) # Make sure we have one instance for every possible job_artifact_X # associations to check they are correctly rejected on build duplication. diff --git a/spec/services/ci/update_runner_service_spec.rb b/spec/services/ci/update_runner_service_spec.rb index cad9e893335f0385a3c66e62209b3b552bfd26cf..1c875b2f54a2043b4ed1e843d98f01a47fe599b8 100644 --- a/spec/services/ci/update_runner_service_spec.rb +++ b/spec/services/ci/update_runner_service_spec.rb @@ -50,7 +50,7 @@ end def update - described_class.new(runner).update(params) + described_class.new(runner).update(params) # rubocop: disable Rails/SaveBang end end end