From 30eeb5dd5d6d222e81dedacefcd8a9ad8d427443 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 20 Mar 2024 11:28:56 -0400 Subject: [PATCH 1/4] Add shoulda-matchers gem --- Gemfile | 2 ++ Gemfile.lock | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Gemfile b/Gemfile index ef52d50cac1..e7820118c87 100644 --- a/Gemfile +++ b/Gemfile @@ -155,6 +155,8 @@ group :test do # Test harness fo rack components gem 'rack-test', '~> 2.1' + gem 'shoulda-matchers' + # Coverage formatter for RSpec test if DISABLE_SIMPLECOV is false gem 'simplecov', '~> 0.22', require: false gem 'simplecov-lcov', '~> 0.8', require: false diff --git a/Gemfile.lock b/Gemfile.lock index c9781a40500..9f5e59d6bb6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -788,6 +788,8 @@ GEM rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) semantic_range (3.0.0) + shoulda-matchers (6.2.0) + activesupport (>= 5.2.0) sidekiq (6.5.12) connection_pool (>= 2.2.5, < 3) rack (~> 2.0) @@ -1036,6 +1038,7 @@ DEPENDENCIES sanitize (~> 6.0) scenic (~> 1.7) selenium-webdriver + shoulda-matchers sidekiq (~> 6.5) sidekiq-bulk (~> 0.2.0) sidekiq-scheduler (~> 5.0) From 0194ecbc801126b606b080d6d5cc7e8aa0e0b1d6 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 20 Mar 2024 11:29:15 -0400 Subject: [PATCH 2/4] Configure shoulda matchers to integrate with rspec --- spec/support/shoulda_matchers.rb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 spec/support/shoulda_matchers.rb diff --git a/spec/support/shoulda_matchers.rb b/spec/support/shoulda_matchers.rb new file mode 100644 index 00000000000..edcf9dd8590 --- /dev/null +++ b/spec/support/shoulda_matchers.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Shoulda::Matchers.configure do |config| + config.integrate do |with| + with.test_framework :rspec + with.library :rails + end +end From 6b51f4c10bde947d33f9e239ebcf2fa38c65c981 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 20 Mar 2024 11:31:11 -0400 Subject: [PATCH 3/4] Use shoulda matchers validation presence matchers --- spec/models/account_spec.rb | 6 +--- spec/models/announcement_spec.rb | 9 +----- spec/models/block_spec.rb | 13 ++------- spec/models/custom_emoji_category_spec.rb | 7 +---- spec/models/custom_filter_spec.rb | 15 ++-------- spec/models/domain_allow_spec.rb | 6 +--- spec/models/domain_block_spec.rb | 6 +--- spec/models/follow_spec.rb | 13 ++------- spec/models/import_spec.rb | 17 ++--------- spec/models/ip_block_spec.rb | 15 ++-------- spec/models/marker_spec.rb | 11 ++----- spec/models/media_attachment_spec.rb | 7 +---- spec/models/mention_spec.rb | 13 ++------- spec/models/poll_spec.rb | 7 ++--- spec/models/user_spec.rb | 6 +--- spec/models/webauthn_credential_spec.rb | 35 +++-------------------- spec/models/webhook_spec.rb | 7 +---- 17 files changed, 29 insertions(+), 164 deletions(-) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index dfb1f5bc615..981d9966117 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -714,11 +714,7 @@ RSpec.describe Account do end describe 'validations' do - it 'is invalid without a username' do - account = Fabricate.build(:account, username: nil) - account.valid? - expect(account).to model_have_error_on_field(:username) - end + it { is_expected.to validate_presence_of(:username) } it 'squishes the username before validation' do account = Fabricate(:account, domain: nil, username: " \u3000bob \t \u00a0 \n ") diff --git a/spec/models/announcement_spec.rb b/spec/models/announcement_spec.rb index 1e7283ca770..82e81669d32 100644 --- a/spec/models/announcement_spec.rb +++ b/spec/models/announcement_spec.rb @@ -64,14 +64,7 @@ describe Announcement do end describe 'Validations' do - describe 'text' do - it 'validates presence of attribute' do - record = Fabricate.build(:announcement, text: nil) - - expect(record).to_not be_valid - expect(record.errors[:text]).to be_present - end - end + it { is_expected.to validate_presence_of(:text) } describe 'ends_at' do it 'validates presence when starts_at is present' do diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index 8249503c592..2976b0d796a 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -4,17 +4,8 @@ require 'rails_helper' RSpec.describe Block do describe 'validations' do - it 'is invalid without an account' do - block = Fabricate.build(:block, account: nil) - block.valid? - expect(block).to model_have_error_on_field(:account) - end - - it 'is invalid without a target_account' do - block = Fabricate.build(:block, target_account: nil) - block.valid? - expect(block).to model_have_error_on_field(:target_account) - end + it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:target_account) } end it 'removes blocking cache after creation' do diff --git a/spec/models/custom_emoji_category_spec.rb b/spec/models/custom_emoji_category_spec.rb index 30de07bd81c..73deae3a36f 100644 --- a/spec/models/custom_emoji_category_spec.rb +++ b/spec/models/custom_emoji_category_spec.rb @@ -4,11 +4,6 @@ require 'rails_helper' describe CustomEmojiCategory do describe 'validations' do - it 'validates name presence' do - record = described_class.new(name: nil) - - expect(record).to_not be_valid - expect(record).to model_have_error_on_field(:name) - end + it { is_expected.to validate_presence_of(:name) } end end diff --git a/spec/models/custom_filter_spec.rb b/spec/models/custom_filter_spec.rb index 8ac9dbb8964..45d25c9f451 100644 --- a/spec/models/custom_filter_spec.rb +++ b/spec/models/custom_filter_spec.rb @@ -4,19 +4,8 @@ require 'rails_helper' RSpec.describe CustomFilter do describe 'Validations' do - it 'requires presence of title' do - record = described_class.new(title: '') - record.valid? - - expect(record).to model_have_error_on_field(:title) - end - - it 'requires presence of context' do - record = described_class.new(context: nil) - record.valid? - - expect(record).to model_have_error_on_field(:context) - end + it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_presence_of(:context) } it 'requires non-empty of context' do record = described_class.new(context: []) diff --git a/spec/models/domain_allow_spec.rb b/spec/models/domain_allow_spec.rb index 12504211a1f..67b9ce1db0b 100644 --- a/spec/models/domain_allow_spec.rb +++ b/spec/models/domain_allow_spec.rb @@ -4,11 +4,7 @@ require 'rails_helper' describe DomainAllow do describe 'Validations' do - it 'is invalid without a domain' do - domain_allow = Fabricate.build(:domain_allow, domain: nil) - domain_allow.valid? - expect(domain_allow).to model_have_error_on_field(:domain) - end + it { is_expected.to validate_presence_of(:domain) } it 'is invalid if the same normalized domain already exists' do _domain_allow = Fabricate(:domain_allow, domain: 'にゃん') diff --git a/spec/models/domain_block_spec.rb b/spec/models/domain_block_spec.rb index d595441fd30..8278454cd78 100644 --- a/spec/models/domain_block_spec.rb +++ b/spec/models/domain_block_spec.rb @@ -4,11 +4,7 @@ require 'rails_helper' RSpec.describe DomainBlock do describe 'validations' do - it 'is invalid without a domain' do - domain_block = Fabricate.build(:domain_block, domain: nil) - domain_block.valid? - expect(domain_block).to model_have_error_on_field(:domain) - end + it { is_expected.to validate_presence_of(:domain) } it 'is invalid if the same normalized domain already exists' do _domain_block = Fabricate(:domain_block, domain: 'にゃん') diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index 9aa172b2f21..b17668fffb8 100644 --- a/spec/models/follow_spec.rb +++ b/spec/models/follow_spec.rb @@ -9,17 +9,8 @@ RSpec.describe Follow do describe 'validations' do subject { described_class.new(account: alice, target_account: bob, rate_limit: true) } - it 'is invalid without an account' do - follow = Fabricate.build(:follow, account: nil) - follow.valid? - expect(follow).to model_have_error_on_field(:account) - end - - it 'is invalid without a target_account' do - follow = Fabricate.build(:follow, target_account: nil) - follow.valid? - expect(follow).to model_have_error_on_field(:target_account) - end + it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:target_account) } it 'is invalid if account already follows too many people' do alice.update(following_count: FollowLimitValidator::LIMIT) diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb index 10df5f8c0b2..587e0a9d260 100644 --- a/spec/models/import_spec.rb +++ b/spec/models/import_spec.rb @@ -3,19 +3,8 @@ require 'rails_helper' RSpec.describe Import do - let(:account) { Fabricate(:account) } - let(:type) { 'following' } - let(:data) { attachment_fixture('imports.txt') } - - describe 'validations' do - it 'is invalid without an type' do - import = described_class.create(account: account, data: data) - expect(import).to model_have_error_on_field(:type) - end - - it 'is invalid without a data' do - import = described_class.create(account: account, type: type) - expect(import).to model_have_error_on_field(:data) - end + describe 'Validations' do + it { is_expected.to validate_presence_of(:type) } + it { is_expected.to validate_presence_of(:data) } end end diff --git a/spec/models/ip_block_spec.rb b/spec/models/ip_block_spec.rb index 290b99b2884..f3a74bbd874 100644 --- a/spec/models/ip_block_spec.rb +++ b/spec/models/ip_block_spec.rb @@ -4,19 +4,8 @@ require 'rails_helper' describe IpBlock do describe 'validations' do - it 'validates ip presence', :aggregate_failures do - ip_block = described_class.new(ip: nil, severity: :no_access) - - expect(ip_block).to_not be_valid - expect(ip_block).to model_have_error_on_field(:ip) - end - - it 'validates severity presence', :aggregate_failures do - ip_block = described_class.new(ip: '127.0.0.1', severity: nil) - - expect(ip_block).to_not be_valid - expect(ip_block).to model_have_error_on_field(:severity) - end + it { is_expected.to validate_presence_of(:ip) } + it { is_expected.to validate_presence_of(:severity) } it 'validates ip uniqueness', :aggregate_failures do described_class.create!(ip: '127.0.0.1', severity: :no_access) diff --git a/spec/models/marker_spec.rb b/spec/models/marker_spec.rb index 51dd584388d..946a6b15990 100644 --- a/spec/models/marker_spec.rb +++ b/spec/models/marker_spec.rb @@ -3,14 +3,7 @@ require 'rails_helper' describe Marker do - describe 'validations' do - describe 'timeline' do - it 'must be included in valid list' do - record = described_class.new(timeline: 'not real timeline') - - expect(record).to_not be_valid - expect(record).to model_have_error_on_field(:timeline) - end - end + describe 'Validations' do + it { is_expected.to validate_inclusion_of(:timeline).in_array(described_class::TIMELINES) } end end diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 3142b291fb2..f2b5a495d1a 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -257,12 +257,7 @@ RSpec.describe MediaAttachment, :attachment_processing do end end - it 'is invalid without file' do - media = described_class.new - - expect(media.valid?).to be false - expect(media).to model_have_error_on_field(:file) - end + it { is_expected.to validate_presence_of(:file) } describe 'size limit validation' do it 'rejects video files that are too large' do diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb index b241049a54b..31225a90ea9 100644 --- a/spec/models/mention_spec.rb +++ b/spec/models/mention_spec.rb @@ -4,16 +4,7 @@ require 'rails_helper' RSpec.describe Mention do describe 'validations' do - it 'is invalid without an account' do - mention = Fabricate.build(:mention, account: nil) - mention.valid? - expect(mention).to model_have_error_on_field(:account) - end - - it 'is invalid without a status' do - mention = Fabricate.build(:mention, status: nil) - mention.valid? - expect(mention).to model_have_error_on_field(:status) - end + it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:status) } end end diff --git a/spec/models/poll_spec.rb b/spec/models/poll_spec.rb index ebcc459078b..227f94c666b 100644 --- a/spec/models/poll_spec.rb +++ b/spec/models/poll_spec.rb @@ -32,12 +32,9 @@ describe Poll do describe 'validations' do context 'when not valid' do - let(:poll) { Fabricate.build(:poll, expires_at: nil) } + subject { Fabricate.build(:poll) } - it 'is invalid without an expire date' do - poll.valid? - expect(poll).to model_have_error_on_field(:expires_at) - end + it { is_expected.to validate_presence_of(:expires_at) } end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4755500fc4e..47e68129250 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -32,11 +32,7 @@ RSpec.describe User do end describe 'validations' do - it 'is invalid without an account' do - user = Fabricate.build(:user, account: nil) - user.valid? - expect(user).to model_have_error_on_field(:account) - end + it { is_expected.to belong_to(:account) } it 'is invalid without a valid email' do user = Fabricate.build(:user, email: 'john@') diff --git a/spec/models/webauthn_credential_spec.rb b/spec/models/webauthn_credential_spec.rb index 23f0229a67a..c4105d91500 100644 --- a/spec/models/webauthn_credential_spec.rb +++ b/spec/models/webauthn_credential_spec.rb @@ -4,37 +4,10 @@ require 'rails_helper' RSpec.describe WebauthnCredential do describe 'validations' do - it 'is invalid without an external id' do - webauthn_credential = Fabricate.build(:webauthn_credential, external_id: nil) - - webauthn_credential.valid? - - expect(webauthn_credential).to model_have_error_on_field(:external_id) - end - - it 'is invalid without a public key' do - webauthn_credential = Fabricate.build(:webauthn_credential, public_key: nil) - - webauthn_credential.valid? - - expect(webauthn_credential).to model_have_error_on_field(:public_key) - end - - it 'is invalid without a nickname' do - webauthn_credential = Fabricate.build(:webauthn_credential, nickname: nil) - - webauthn_credential.valid? - - expect(webauthn_credential).to model_have_error_on_field(:nickname) - end - - it 'is invalid without a sign_count' do - webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: nil) - - webauthn_credential.valid? - - expect(webauthn_credential).to model_have_error_on_field(:sign_count) - end + it { is_expected.to validate_presence_of(:external_id) } + it { is_expected.to validate_presence_of(:public_key) } + it { is_expected.to validate_presence_of(:nickname) } + it { is_expected.to validate_presence_of(:sign_count) } it 'is invalid if already exist a webauthn credential with the same external id' do Fabricate(:webauthn_credential, external_id: '_Typ0ygudDnk9YUVWLQayw') diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb index effaf92e9c8..be332dadeca 100644 --- a/spec/models/webhook_spec.rb +++ b/spec/models/webhook_spec.rb @@ -6,12 +6,7 @@ RSpec.describe Webhook do let(:webhook) { Fabricate(:webhook) } describe 'Validations' do - it 'requires presence of events' do - record = described_class.new(events: nil) - record.valid? - - expect(record).to model_have_error_on_field(:events) - end + it { is_expected.to validate_presence_of(:events) } it 'requires non-empty events value' do record = described_class.new(events: []) From b33aa097871ab4b66a1515aff9b7e9aa98d7188f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 18 Jun 2024 10:11:56 -0400 Subject: [PATCH 4/4] Add required to belongs to check --- spec/models/block_spec.rb | 4 ++-- spec/models/follow_spec.rb | 4 ++-- spec/models/mention_spec.rb | 4 ++-- spec/models/user_spec.rb | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index 2976b0d796a..84f0f318f42 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -4,8 +4,8 @@ require 'rails_helper' RSpec.describe Block do describe 'validations' do - it { is_expected.to belong_to(:account) } - it { is_expected.to belong_to(:target_account) } + it { is_expected.to belong_to(:account).required } + it { is_expected.to belong_to(:target_account).required } end it 'removes blocking cache after creation' do diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index b17668fffb8..f22bd6ea88d 100644 --- a/spec/models/follow_spec.rb +++ b/spec/models/follow_spec.rb @@ -9,8 +9,8 @@ RSpec.describe Follow do describe 'validations' do subject { described_class.new(account: alice, target_account: bob, rate_limit: true) } - it { is_expected.to belong_to(:account) } - it { is_expected.to belong_to(:target_account) } + it { is_expected.to belong_to(:account).required } + it { is_expected.to belong_to(:target_account).required } it 'is invalid if account already follows too many people' do alice.update(following_count: FollowLimitValidator::LIMIT) diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb index 31225a90ea9..3a9b9fddf2d 100644 --- a/spec/models/mention_spec.rb +++ b/spec/models/mention_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' RSpec.describe Mention do describe 'validations' do - it { is_expected.to belong_to(:account) } - it { is_expected.to belong_to(:status) } + it { is_expected.to belong_to(:account).required } + it { is_expected.to belong_to(:status).required } end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 47e68129250..c9f5c42f85b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -32,7 +32,7 @@ RSpec.describe User do end describe 'validations' do - it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:account).required } it 'is invalid without a valid email' do user = Fabricate.build(:user, email: 'john@')