From e38fc319dc6897ca867a509b0c7a5878d34d0f00 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 28 Jan 2022 00:46:42 +0100 Subject: Refactor and improve tests (#17386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Change account and user fabricators to simplify and improve tests - `Fabricate(:account)` implicitly fabricates an associated `user` if no `domain` attribute is given (an account with `domain: nil` is considered a local account, but no user record was created), unless `user: nil` is passed - `Fabricate(:account, user: Fabricate(:user))` should still be possible but is discouraged. * Fix and refactor tests - avoid passing unneeded attributes to `Fabricate(:user)` or `Fabricate(:account)` - avoid embedding `Fabricate(:user)` into a `Fabricate(:account)` or the other way around - prefer `Fabricate(:user, account_attributes: …)` to `Fabricate(:user, account: Fabricate(:account, …)` - also, some tests were using remote accounts with local user records, which is not representative of production code. --- spec/models/account_spec.rb | 2 +- spec/models/admin/account_action_spec.rb | 4 ++-- spec/models/public_feed_spec.rb | 10 ++++------ spec/models/user_spec.rb | 6 +++--- 4 files changed, 10 insertions(+), 12 deletions(-) (limited to 'spec/models') diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 65e6714c0..681134d49 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Account, type: :model do end context 'when the account is of a local user' do - let!(:subject) { Fabricate(:account, user: Fabricate(:user, email: 'foo+bar@domain.org')) } + let!(:subject) { Fabricate(:user, email: 'foo+bar@domain.org').account } it 'creates a canonical domain block' do subject.suspend! diff --git a/spec/models/admin/account_action_spec.rb b/spec/models/admin/account_action_spec.rb index 2366b9ca4..809c7fc46 100644 --- a/spec/models/admin/account_action_spec.rb +++ b/spec/models/admin/account_action_spec.rb @@ -5,8 +5,8 @@ RSpec.describe Admin::AccountAction, type: :model do describe '#save!' do subject { account_action.save! } - let(:account) { Fabricate(:account, user: Fabricate(:user, admin: true)) } - let(:target_account) { Fabricate(:account, user: Fabricate(:user)) } + let(:account) { Fabricate(:user, admin: true).account } + let(:target_account) { Fabricate(:account) } let(:type) { 'disable' } before do diff --git a/spec/models/public_feed_spec.rb b/spec/models/public_feed_spec.rb index 0392a582c..0ffc343f1 100644 --- a/spec/models/public_feed_spec.rb +++ b/spec/models/public_feed_spec.rb @@ -31,7 +31,6 @@ RSpec.describe PublicFeed, type: :model do end it 'filters out silenced accounts' do - account = Fabricate(:account) silenced_account = Fabricate(:account, silenced: true) status = Fabricate(:status, account: account) silenced_status = Fabricate(:status, account: silenced_account) @@ -176,8 +175,7 @@ RSpec.describe PublicFeed, type: :model do context 'with language preferences' do it 'excludes statuses in languages not allowed by the account user' do - user = Fabricate(:user, chosen_languages: [:en, :es]) - @account.update(user: user) + @account.user.update(chosen_languages: [:en, :es]) en_status = Fabricate(:status, language: 'en') es_status = Fabricate(:status, language: 'es') fr_status = Fabricate(:status, language: 'fr') @@ -188,8 +186,7 @@ RSpec.describe PublicFeed, type: :model do end it 'includes all languages when user does not have a setting' do - user = Fabricate(:user, chosen_languages: nil) - @account.update(user: user) + @account.user.update(chosen_languages: nil) en_status = Fabricate(:status, language: 'en') es_status = Fabricate(:status, language: 'es') @@ -199,7 +196,8 @@ RSpec.describe PublicFeed, type: :model do end it 'includes all languages when account does not have a user' do - expect(@account.user).to be_nil + @account.update(user: nil) + en_status = Fabricate(:status, language: 'en') es_status = Fabricate(:status, language: 'es') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 54bb6db7f..406438c22 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -194,12 +194,12 @@ RSpec.describe User, type: :model do end it "returns 'private' if user has not configured default privacy setting and account is locked" do - user = Fabricate(:user, account: Fabricate(:account, locked: true)) + user = Fabricate(:account, locked: true).user expect(user.setting_default_privacy).to eq 'private' end it "returns 'public' if user has not configured default privacy setting and account is not locked" do - user = Fabricate(:user, account: Fabricate(:account, locked: false)) + user = Fabricate(:account, locked: false).user expect(user.setting_default_privacy).to eq 'public' end end @@ -248,7 +248,7 @@ RSpec.describe User, type: :model do it_behaves_like 'Settings-extended' do def create! - User.create!(account: Fabricate(:account), email: 'foo@mastodon.space', password: 'abcd1234', agreement: true) + User.create!(account: Fabricate(:account, user: nil), email: 'foo@mastodon.space', password: 'abcd1234', agreement: true) end def fabricate -- cgit