Commit ce0dbf65 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'issue#220040-fix-robocop-savebang-spec-models' into 'master'

Fixed Rails Save Bang offenses in few spec/models/* files

See merge request gitlab-org/gitlab!61862
parents 044964c4 3a252c32
...@@ -158,9 +158,6 @@ Rails/SaveBang: ...@@ -158,9 +158,6 @@ Rails/SaveBang:
- 'spec/lib/gitlab/middleware/go_spec.rb' - 'spec/lib/gitlab/middleware/go_spec.rb'
- 'spec/lib/gitlab/shard_health_cache_spec.rb' - 'spec/lib/gitlab/shard_health_cache_spec.rb'
- 'spec/mailers/notify_spec.rb' - 'spec/mailers/notify_spec.rb'
- 'spec/models/appearance_spec.rb'
- 'spec/models/application_record_spec.rb'
- 'spec/models/application_setting_spec.rb'
- 'spec/models/clusters/applications/helm_spec.rb' - 'spec/models/clusters/applications/helm_spec.rb'
- 'spec/models/container_repository_spec.rb' - 'spec/models/container_repository_spec.rb'
- 'spec/models/design_management/version_spec.rb' - 'spec/models/design_management/version_spec.rb'
......
---
title: Fixed Rails Save Bang offenses in few spec/models/* files
merge_request: 61862
author: Suraj Tripathi @surajtripathy07
type: fixed
...@@ -15,7 +15,7 @@ RSpec.describe Appearance do ...@@ -15,7 +15,7 @@ RSpec.describe Appearance do
create(:appearance) create(:appearance)
new_row = build(:appearance) new_row = build(:appearance)
new_row.save expect { new_row.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Only 1 appearances row can exist')
expect(new_row.valid?).to eq(false) expect(new_row.valid?).to eq(false)
end end
...@@ -39,7 +39,7 @@ RSpec.describe Appearance do ...@@ -39,7 +39,7 @@ RSpec.describe Appearance do
end end
it 'returns the path when the upload has been orphaned' do it 'returns the path when the upload has been orphaned' do
appearance.send(logo_type).upload.destroy appearance.send(logo_type).upload.destroy!
appearance.reload appearance.reload
expect(appearance.send("#{logo_type}_path")).to eq(expected_path) expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
......
...@@ -13,20 +13,24 @@ RSpec.describe ApplicationRecord do ...@@ -13,20 +13,24 @@ RSpec.describe ApplicationRecord do
describe '.safe_ensure_unique' do describe '.safe_ensure_unique' do
let(:model) { build(:suggestion) } let(:model) { build(:suggestion) }
let_it_be(:note) { create(:diff_note_on_merge_request) }
let(:klass) { model.class } let(:klass) { model.class }
before do before do
allow(model).to receive(:save).and_raise(ActiveRecord::RecordNotUnique) allow(model).to receive(:save!).and_raise(ActiveRecord::RecordNotUnique)
end end
it 'returns false when ActiveRecord::RecordNotUnique is raised' do it 'returns false when ActiveRecord::RecordNotUnique is raised' do
expect(model).to receive(:save).once expect(model).to receive(:save!).once
expect(klass.safe_ensure_unique { model.save }).to be_falsey model.note_id = note.id
expect(klass.safe_ensure_unique { model.save! }).to be_falsey
end end
it 'retries based on retry count specified' do it 'retries based on retry count specified' do
expect(model).to receive(:save).exactly(3).times expect(model).to receive(:save!).exactly(3).times
expect(klass.safe_ensure_unique(retries: 2) { model.save }).to be_falsey model.note_id = note.id
expect(klass.safe_ensure_unique(retries: 2) { model.save! }).to be_falsey
end end
end end
......
...@@ -252,7 +252,9 @@ RSpec.describe ApplicationSetting do ...@@ -252,7 +252,9 @@ RSpec.describe ApplicationSetting do
context "when user accepted let's encrypt terms of service" do context "when user accepted let's encrypt terms of service" do
before do before do
setting.update(lets_encrypt_terms_of_service_accepted: true) expect do
setting.update!(lets_encrypt_terms_of_service_accepted: true)
end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Lets encrypt notification email can't be blank")
end end
it { is_expected.not_to allow_value(nil).for(:lets_encrypt_notification_email) } it { is_expected.not_to allow_value(nil).for(:lets_encrypt_notification_email) }
...@@ -302,26 +304,30 @@ RSpec.describe ApplicationSetting do ...@@ -302,26 +304,30 @@ RSpec.describe ApplicationSetting do
describe 'default_artifacts_expire_in' do describe 'default_artifacts_expire_in' do
it 'sets an error if it cannot parse' do it 'sets an error if it cannot parse' do
setting.update(default_artifacts_expire_in: 'a') expect do
setting.update!(default_artifacts_expire_in: 'a')
end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Default artifacts expire in is not a correct duration")
expect_invalid expect_invalid
end end
it 'sets an error if it is blank' do it 'sets an error if it is blank' do
setting.update(default_artifacts_expire_in: ' ') expect do
setting.update!(default_artifacts_expire_in: ' ')
end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Default artifacts expire in can't be blank")
expect_invalid expect_invalid
end end
it 'sets the value if it is valid' do it 'sets the value if it is valid' do
setting.update(default_artifacts_expire_in: '30 days') setting.update!(default_artifacts_expire_in: '30 days')
expect(setting).to be_valid expect(setting).to be_valid
expect(setting.default_artifacts_expire_in).to eq('30 days') expect(setting.default_artifacts_expire_in).to eq('30 days')
end end
it 'sets the value if it is 0' do it 'sets the value if it is 0' do
setting.update(default_artifacts_expire_in: '0') setting.update!(default_artifacts_expire_in: '0')
expect(setting).to be_valid expect(setting).to be_valid
expect(setting.default_artifacts_expire_in).to eq('0') expect(setting.default_artifacts_expire_in).to eq('0')
...@@ -400,18 +406,18 @@ RSpec.describe ApplicationSetting do ...@@ -400,18 +406,18 @@ RSpec.describe ApplicationSetting do
context 'auto_devops_domain setting' do context 'auto_devops_domain setting' do
context 'when auto_devops_enabled? is true' do context 'when auto_devops_enabled? is true' do
before do before do
setting.update(auto_devops_enabled: true) setting.update!(auto_devops_enabled: true)
end end
it 'can be blank' do it 'can be blank' do
setting.update(auto_devops_domain: '') setting.update!(auto_devops_domain: '')
expect(setting).to be_valid expect(setting).to be_valid
end end
context 'with a valid value' do context 'with a valid value' do
before do before do
setting.update(auto_devops_domain: 'domain.com') setting.update!(auto_devops_domain: 'domain.com')
end end
it 'is valid' do it 'is valid' do
...@@ -421,7 +427,9 @@ RSpec.describe ApplicationSetting do ...@@ -421,7 +427,9 @@ RSpec.describe ApplicationSetting do
context 'with an invalid value' do context 'with an invalid value' do
before do before do
setting.update(auto_devops_domain: 'definitelynotahostname') expect do
setting.update!(auto_devops_domain: 'definitelynotahostname')
end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Auto devops domain is not a fully qualified domain name")
end end
it 'is invalid' do it 'is invalid' do
......
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