PERF: Make tests faster by prefabricating more things (#15370)

spec/components/guardian_spec.rb

@@ -8,6 +8,8 @@ describe Guardian do
 
   fab!(:user) { Fabricate(:user) }
   fab!(:another_user) { Fabricate(:user) }
+  fab!(:member) { Fabricate(:user) }
+  fab!(:owner) { Fabricate(:user) }
   fab!(:moderator) { Fabricate(:moderator) }
   fab!(:admin) { Fabricate(:admin) }
   fab!(:anonymous_user) { Fabricate(:anonymous) }
@@ -735,8 +737,6 @@ describe Guardian do
       end
 
       it 'allows members of an authorized group' do
-        user = Fabricate(:user)
-
         secure_category = plain_category
         secure_category.set_permissions(group => :readonly)
         secure_category.save
@@ -940,7 +940,7 @@ describe Guardian do
         end
 
         it 'is true when logged in' do
-          expect(Guardian.new(Fabricate(:user)).can_see?(post_revision)).to be_truthy
+          expect(Guardian.new(user).can_see?(post_revision)).to be_truthy
         end
       end
 
@@ -1418,7 +1418,6 @@ describe Guardian do
         post = Fabricate(:post, topic: topic)
         post.wiki = true
 
-        user = Fabricate(:user)
         expect(Guardian.new(user).can_edit?(post)).to eq(false)
       end
 
@@ -3403,11 +3402,9 @@ describe Guardian do
     it 'Correctly handles owner visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:owners])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3422,11 +3419,9 @@ describe Guardian do
     it 'Correctly handles staff visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:staff])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3441,11 +3436,9 @@ describe Guardian do
     it 'Correctly handles member visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:members])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3459,11 +3452,9 @@ describe Guardian do
 
     it 'Correctly handles logged-on-user visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:logged_on_users])
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3487,11 +3478,9 @@ describe Guardian do
     it 'Correctly handles group members visibility for owner' do
       group = Group.new(name: 'group', members_visibility_level: Group.visibility_levels[:owners])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3506,11 +3495,9 @@ describe Guardian do
     it 'Correctly handles group members visibility for staff' do
       group = Group.new(name: 'group', members_visibility_level: Group.visibility_levels[:staff])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3525,11 +3512,9 @@ describe Guardian do
     it 'Correctly handles group members visibility for member' do
       group = Group.new(name: 'group', members_visibility_level: Group.visibility_levels[:members])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3543,11 +3528,9 @@ describe Guardian do
 
     it 'Correctly handles group members visibility for logged-on-user' do
       group = Group.new(name: 'group', members_visibility_level: Group.visibility_levels[:logged_on_users])
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3571,11 +3554,9 @@ describe Guardian do
     it 'correctly handles owner visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:owners])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3592,11 +3573,9 @@ describe Guardian do
       group2 = Group.new(name: 'group2', visibility_level: Group.visibility_levels[:owners])
       group2.save!
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3611,11 +3590,9 @@ describe Guardian do
     it 'correctly handles staff visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:staff])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3630,11 +3607,9 @@ describe Guardian do
     it 'correctly handles member visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:members])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3649,11 +3624,9 @@ describe Guardian do
     it 'correctly handles logged-on-user visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:logged_on_users])
 
-      member = Fabricate(:user)
       group.add(member)
       group.save!
 
-      owner = Fabricate(:user)
       group.add_owner(owner)
       group.reload
 
@@ -3670,11 +3643,9 @@ describe Guardian do
       group2 = Group.new(name: 'group2', visibility_level: Group.visibility_levels[:members])
       group2.save!
 
-      member = Fabricate(:user)
       group1.add(member)
       group1.save!
 
-      owner = Fabricate(:user)
       group1.add_owner(owner)
       group1.reload
 

spec/components/topic_query_spec.rb

@@ -5,11 +5,15 @@ require 'topic_view'
 
 describe TopicQuery do
 
-  # TODO: this let! here has impact on all tests
+  # TODO:
-  #  it indeed happens first, but is not obvious later in the tests we depend on the user being
+  #   This fab! here has impact on all tests.
-  #  created so early otherwise finding new topics does not work
+  #
-  #  we should remove the let! here and use freeze time to communicate how the clock moves
+  #   It happens first, but is not obvious later in the tests that we depend on
-  let!(:user) { Fabricate(:user) }
+  #   the user being created so early otherwise finding new topics does not
+  #   work.
+  #
+  #   We should use be more explicit in communicating how the clock moves
+  fab!(:user) { Fabricate(:user) }
 
   fab!(:creator) { Fabricate(:user) }
   let(:topic_query) { TopicQuery.new(user) }

spec/jobs/export_user_archive_spec.rb

@@ -4,7 +4,8 @@ require 'rails_helper'
 require 'csv'
 
 describe Jobs::ExportUserArchive do
-  let(:user) { Fabricate(:user, username: "john_doe") }
+  fab!(:user) { Fabricate(:user, username: "john_doe") }
+  fab!(:user2) { Fabricate(:user) }
   let(:extra) { {} }
   let(:job) {
     j = Jobs::ExportUserArchive.new
@@ -14,10 +15,10 @@ describe Jobs::ExportUserArchive do
   }
   let(:component) { raise 'component not set' }
 
-  let(:admin) { Fabricate(:admin) }
+  fab!(:admin) { Fabricate(:admin) }
-  let(:category) { Fabricate(:category_with_definition) }
+  fab!(:category) { Fabricate(:category_with_definition) }
-  let(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
+  fab!(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
-  let(:topic) { Fabricate(:topic, category: category) }
+  fab!(:topic) { Fabricate(:topic, category: category) }
   let(:post) { Fabricate(:post, user: user, topic: topic) }
 
   def make_component_csv
@@ -111,7 +112,6 @@ describe Jobs::ExportUserArchive do
 
   context 'user_archive posts' do
     let(:component) { 'user_archive' }
-    let(:user2) { Fabricate(:user) }
     let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id) }
     let(:subsubtopic) { Fabricate(:topic, category: subsubcategory) }
     let(:subsubpost) { Fabricate(:post, user: user, topic: subsubtopic) }
@@ -453,7 +453,6 @@ describe Jobs::ExportUserArchive do
 
   context 'visits' do
     let(:component) { 'visits' }
-    let(:user2) { Fabricate(:user) }
 
     it 'correctly exports the UserVisit table' do
       freeze_time '2017-03-01 12:00'

spec/models/group_user_spec.rb

@@ -33,13 +33,13 @@ describe GroupUser do
   end
 
   describe "default category notifications" do
-    let(:group) { Fabricate(:group) }
+    fab!(:group) { Fabricate(:group) }
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
-    let(:category1) { Fabricate(:category) }
+    fab!(:category1) { Fabricate(:category) }
-    let(:category2) { Fabricate(:category) }
+    fab!(:category2) { Fabricate(:category) }
-    let(:category3) { Fabricate(:category) }
+    fab!(:category3) { Fabricate(:category) }
-    let(:category4) { Fabricate(:category) }
+    fab!(:category4) { Fabricate(:category) }
-    let(:category5) { Fabricate(:category) }
+    fab!(:category5) { Fabricate(:category) }
 
     def levels
       CategoryUser.notification_levels
@@ -98,14 +98,14 @@ describe GroupUser do
   end
 
   describe "default tag notifications" do
-    let(:group) { Fabricate(:group) }
+    fab!(:group) { Fabricate(:group) }
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
-    let(:tag1) { Fabricate(:tag) }
+    fab!(:tag1) { Fabricate(:tag) }
-    let(:tag2) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
-    let(:tag3) { Fabricate(:tag) }
+    fab!(:tag3) { Fabricate(:tag) }
-    let(:tag4) { Fabricate(:tag) }
+    fab!(:tag4) { Fabricate(:tag) }
-    let(:tag5) { Fabricate(:tag) }
+    fab!(:tag5) { Fabricate(:tag) }
-    let(:synonym1) { Fabricate(:tag, target_tag: tag1) }
+    fab!(:synonym1) { Fabricate(:tag, target_tag: tag1) }
 
     def levels
       TagUser.notification_levels

spec/models/topic_spec.rb

@@ -6,12 +6,13 @@ require 'rails_helper'
 describe Topic do
   let(:now) { Time.zone.local(2013, 11, 20, 8, 0) }
   fab!(:user) { Fabricate(:user) }
+  fab!(:user1) { Fabricate(:user) }
+  fab!(:user2) { Fabricate(:user) }
   fab!(:moderator) { Fabricate(:moderator) }
   fab!(:coding_horror) { Fabricate(:coding_horror) }
   fab!(:evil_trout) { Fabricate(:evil_trout) }
   fab!(:admin) { Fabricate(:admin) }
   fab!(:group) { Fabricate(:group) }
-  fab!(:another_user) { Fabricate(:user) }
   fab!(:trust_level_2) { Fabricate(:user, trust_level: SiteSetting.min_trust_level_to_allow_invite) }
 
   context 'validations' do
@@ -269,7 +270,7 @@ describe Topic do
     fab!(:category1) { Fabricate(:category) }
     fab!(:category2) { Fabricate(:category) }
 
-    let!(:topic) { Fabricate(:topic, category: category1) }
+    fab!(:topic) { Fabricate(:topic, category: category1) }
     let(:new_topic) { Fabricate.build(:topic, title: topic.title, category: category1) }
     let(:new_topic_different_cat) { Fabricate.build(:topic, title: topic.title, category: category2) }
 
@@ -649,7 +650,7 @@ describe Topic do
         topic.invite(topic.user, user.username)
 
         expect {
-          topic.invite(topic.user, another_user.username)
+          topic.invite(topic.user, user1.username)
         }.to raise_error(RateLimiter::LimitExceeded)
       end
 
@@ -662,7 +663,7 @@ describe Topic do
         topic.invite(topic.user, user.username)
 
         expect {
-          topic.invite(topic.user, another_user.username)
+          topic.invite(topic.user, user1.username)
         }.to raise_error(RateLimiter::LimitExceeded)
       end
     end
@@ -677,9 +678,9 @@ describe Topic do
 
     describe 'when user is already allowed' do
       it 'should raise the right error' do
-        topic.allowed_users << another_user
+        topic.allowed_users << user1
 
-        expect { topic.invite(user, another_user.username) }
+        expect { topic.invite(user, user1.username) }
           .to raise_error(Topic::UserExists)
       end
     end
@@ -690,8 +691,8 @@ describe Topic do
 
       describe 'by username' do
         it 'should be able to invite a user' do
-          expect(topic.invite(user, another_user.username)).to eq(true)
+          expect(topic.invite(user, user1.username)).to eq(true)
-          expect(topic.allowed_users).to include(another_user)
+          expect(topic.allowed_users).to include(user1)
           expect(Post.last.action_code).to eq("invited_user")
 
           notification = Notification.last
@@ -699,39 +700,39 @@ describe Topic do
           expect(notification.notification_type)
             .to eq(Notification.types[:invited_to_private_message])
 
-          expect(topic.remove_allowed_user(user, another_user.username)).to eq(true)
+          expect(topic.remove_allowed_user(user, user1.username)).to eq(true)
-          expect(topic.reload.allowed_users).to_not include(another_user)
+          expect(topic.reload.allowed_users).to_not include(user1)
           expect(Post.last.action_code).to eq("removed_user")
         end
 
         it 'should not create a small action if user is already invited through a group' do
-          group = Fabricate(:group, users: [user, another_user])
+          group = Fabricate(:group, users: [user, user1])
           expect(topic.invite_group(user, group)).to eq(true)
 
-          expect { topic.invite(user, another_user.username) }
+          expect { topic.invite(user, user1.username) }
             .to change { Notification.count }.by(1)
             .and change { Post.where(post_type: Post.types[:small_action]).count }.by(0)
         end
 
         context "from a muted user" do
-          before { Fabricate(:muted_user, user: another_user, muted_user: user) }
+          before { Fabricate(:muted_user, user: user1, muted_user: user) }
 
           it 'fails with an error' do
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::NotAllowed)
-            expect(topic.allowed_users).to_not include(another_user)
+            expect(topic.allowed_users).to_not include(user1)
             expect(Post.last).to be_blank
             expect(Notification.last).to be_blank
           end
         end
 
         context "from a ignored user" do
-          before { Fabricate(:ignored_user, user: another_user, ignored_user: user) }
+          before { Fabricate(:ignored_user, user: user1, ignored_user: user) }
 
           it 'fails with an error' do
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::NotAllowed)
-            expect(topic.allowed_users).to_not include(another_user)
+            expect(topic.allowed_users).to_not include(user1)
             expect(Post.last).to be_blank
             expect(Notification.last).to be_blank
           end
@@ -743,58 +744,57 @@ describe Topic do
           end
 
           it 'should raise error' do
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::UserExists)
           end
         end
 
         context "when invited_user has enabled allow_list" do
-          fab!(:user2) { Fabricate(:user) }
           fab!(:pm) { Fabricate(:private_message_topic, user: user, topic_allowed_users: [
             Fabricate.build(:topic_allowed_user, user: user),
             Fabricate.build(:topic_allowed_user, user: user2)
           ]) }
 
           before do
-            another_user.user_option.update!(enable_allowed_pm_users: true)
+            user1.user_option.update!(enable_allowed_pm_users: true)
           end
 
           it 'succeeds when inviter is in allowed list' do
-            AllowedPmUser.create!(user: another_user, allowed_pm_user: user)
+            AllowedPmUser.create!(user: user1, allowed_pm_user: user)
-            expect(topic.invite(user, another_user.username)).to eq(true)
+            expect(topic.invite(user, user1.username)).to eq(true)
           end
 
           it 'should raise error when inviter not in allowed list' do
-            AllowedPmUser.create!(user: another_user, allowed_pm_user: user2)
+            AllowedPmUser.create!(user: user1, allowed_pm_user: user2)
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::NotAllowed)
               .with_message(I18n.t("topic_invite.receiver_does_not_allow_pm"))
           end
 
           it 'should succeed for staff even when not allowed' do
-            AllowedPmUser.create!(user: another_user, allowed_pm_user: user2)
+            AllowedPmUser.create!(user: user1, allowed_pm_user: user2)
-            expect(topic.invite(another_user, admin.username)).to eq(true)
+            expect(topic.invite(user1, admin.username)).to eq(true)
           end
 
           it 'should raise error when target_user is not in inviters allowed list' do
             user.user_option.update!(enable_allowed_pm_users: true)
-            AllowedPmUser.create!(user: another_user, allowed_pm_user: user)
+            AllowedPmUser.create!(user: user1, allowed_pm_user: user)
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::NotAllowed)
               .with_message(I18n.t("topic_invite.sender_does_not_allow_pm"))
           end
 
           it 'succeeds when inviter is in allowed list even though other participants are not in allowed list' do
-            AllowedPmUser.create!(user: another_user, allowed_pm_user: user)
+            AllowedPmUser.create!(user: user1, allowed_pm_user: user)
-            expect(pm.invite(user, another_user.username)).to eq(true)
+            expect(pm.invite(user, user1.username)).to eq(true)
           end
         end
       end
 
       describe 'by email' do
         it 'should be able to invite a user' do
-          expect(topic.invite(user, another_user.email)).to eq(true)
+          expect(topic.invite(user, user1.email)).to eq(true)
-          expect(topic.allowed_users).to include(another_user)
+          expect(topic.allowed_users).to include(user1)
 
           expect(Notification.last.notification_type)
             .to eq(Notification.types[:invited_to_private_message])
@@ -841,15 +841,15 @@ describe Topic do
 
       describe 'by username' do
         it 'should invite user into a topic' do
-          topic.invite(user, another_user.username)
+          topic.invite(user, user1.username)
-          expect_the_right_notification_to_be_created(user, another_user)
+          expect_the_right_notification_to_be_created(user, user1)
         end
       end
 
       describe 'by email' do
         it 'should be able to invite a user' do
-          expect(topic.invite(user, another_user.email)).to eq(true)
+          expect(topic.invite(user, user1.email)).to eq(true)
-          expect_the_right_notification_to_be_created(user, another_user)
+          expect_the_right_notification_to_be_created(user, user1)
         end
 
         describe 'when topic belongs to a private category' do
@@ -861,7 +861,7 @@ describe Topic do
           end
 
           fab!(:topic) { Fabricate(:topic, category: category) }
-          let(:inviter) { Fabricate(:user).tap { |user| group.add_owner(user) } }
+          fab!(:inviter) { Fabricate(:user).tap { |user| group.add_owner(user) } }
           fab!(:invitee) { Fabricate(:user) }
 
           describe 'as a group owner' do
@@ -904,12 +904,12 @@ describe Topic do
         end
 
         context "for a muted topic" do
-          before { TopicUser.change(another_user.id, topic.id, notification_level: TopicUser.notification_levels[:muted]) }
+          before { TopicUser.change(user1.id, topic.id, notification_level: TopicUser.notification_levels[:muted]) }
 
           it 'fails with an error message' do
-            expect { topic.invite(user, another_user.username) }
+            expect { topic.invite(user, user1.username) }
               .to raise_error(Topic::NotAllowed)
-            expect(topic.allowed_users).to_not include(another_user)
+            expect(topic.allowed_users).to_not include(user1)
             expect(Post.last).to be_blank
             expect(Notification.last).to be_blank
           end
@@ -934,7 +934,7 @@ describe Topic do
   end
 
   context 'private message' do
-    let(:topic) do
+    fab!(:topic) do
       PostCreator.new(
         Fabricate(:user),
         title: "This is a private message",
@@ -1036,8 +1036,6 @@ describe Topic do
             # clear up the state so we can be more explicit with the test
             TopicAllowedUser.where(topic: topic).delete_all
             user0 = topic.user
-            user1 = Fabricate(:user)
-            user2 = Fabricate(:user)
             user3 = Fabricate(:user)
             Fabricate(:topic_allowed_user, topic: topic, user: user0)
             Fabricate(:topic_allowed_user, topic: topic, user: user1)
@@ -1063,7 +1061,6 @@ describe Topic do
             # clear up the state so we can be more explicit with the test
             TopicAllowedUser.where(topic: topic).delete_all
             user0 = topic.user
-            user1 = Fabricate(:user)
             Fabricate(:topic_allowed_user, topic: topic, user: user0)
             Fabricate(:topic_allowed_user, topic: topic, user: user1)
 
@@ -1532,7 +1529,7 @@ describe Topic do
       end
 
       context 'new key' do
-        before do
+        before_all do
           topic.update_meta_data('other' => 'key')
           topic.save!
         end
@@ -1604,7 +1601,7 @@ describe Topic do
     end
 
     describe 'with a previous category' do
-      before do
+      before_all do
         topic.change_category_to_id(category.id)
         topic.reload
         category.reload
@@ -1645,7 +1642,7 @@ describe Topic do
             )
 
             CategoryUser.set_notification_level_for_category(
-              another_user,
+              user1,
               CategoryUser::notification_levels[:watching_first_post],
               new_category.id
             )
@@ -1664,7 +1661,7 @@ describe Topic do
             ).exists?).to eq(true)
 
             expect(Notification.where(
-              user_id: another_user.id,
+              user_id: user1.id,
               topic_id: topic.id,
               post_number: 1,
               notification_type: Notification.types[:watching_first_post]
@@ -1700,7 +1697,7 @@ describe Topic do
             ).exists?).to eq(true)
 
             expect(Notification.where(
-              user_id: another_user.id,
+              user_id: user1.id,
               topic_id: topic.id,
               post_number: 1,
               notification_type: Notification.types[:watching_first_post]
@@ -2006,8 +2003,6 @@ describe Topic do
   end
 
   describe '.for_digest' do
-    let(:user) { Fabricate.build(:user) }
-
     context "no edit grace period" do
       before do
         SiteSetting.editing_grace_period = 0
@@ -2028,7 +2023,6 @@ describe Topic do
       end
 
       it "doesn't return topics from muted categories" do
-        user = Fabricate(:user)
         category = Fabricate(:category_with_definition, created_at: 2.minutes.ago)
         Fabricate(:topic, category: category, created_at: 1.minute.ago)
 
@@ -2039,7 +2033,6 @@ describe Topic do
 
       it "doesn't return topics that a user has muted" do
         topic = Fabricate(:topic, created_at: 1.minute.ago)
-        user = Fabricate(:user)
 
         Fabricate(:topic_user,
           user: user,
@@ -2051,7 +2044,6 @@ describe Topic do
       end
 
       it "does return watched topics from muted categories" do
-        user = Fabricate(:user)
         category = Fabricate(:category_with_definition, created_at: 2.minutes.ago)
         topic = Fabricate(:topic, category: category, created_at: 1.minute.ago)
 
@@ -2062,7 +2054,6 @@ describe Topic do
       end
 
       it "doesn't return topics from suppressed categories" do
-        user = Fabricate(:user)
         category = Fabricate(:category_with_definition, created_at: 2.minutes.ago)
         topic = Fabricate(:topic, category: category, created_at: 1.minute.ago)
 
@@ -2099,7 +2090,6 @@ describe Topic do
       end
 
       it "doesn't return topics with only muted tags" do
-        user = Fabricate(:user)
         tag = Fabricate(:tag)
         TagUser.change(user.id, tag.id, TagUser.notification_levels[:muted])
         Fabricate(:topic, tags: [tag], created_at: 1.minute.ago)
@@ -2108,7 +2098,6 @@ describe Topic do
       end
 
       it "returns topics with both muted and not muted tags" do
-        user = Fabricate(:user)
         muted_tag, other_tag = Fabricate(:tag), Fabricate(:tag)
         TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
         topic = Fabricate(:topic, tags: [muted_tag, other_tag], created_at: 1.minute.ago)
@@ -2117,7 +2106,6 @@ describe Topic do
       end
 
       it "returns topics with no tags too" do
-        user = Fabricate(:user)
         muted_tag = Fabricate(:tag)
         TagUser.change(user.id, muted_tag.id, TagUser.notification_levels[:muted])
         _topic1 = Fabricate(:topic, tags: [muted_tag], created_at: 1.minute.ago)
@@ -2143,7 +2131,6 @@ describe Topic do
       it "sorts by topic notification levels" do
         topics = []
         3.times { |i| topics << Fabricate(:topic, created_at: 1.minute.ago) }
-        user = Fabricate(:user)
         TopicUser.create(user_id: user.id, topic_id: topics[0].id, notification_level: TopicUser.notification_levels[:tracking])
         TopicUser.create(user_id: user.id, topic_id: topics[2].id, notification_level: TopicUser.notification_levels[:watching])
         for_digest = Topic.for_digest(user, 1.year.ago, top_order: true).pluck(:id)
@@ -2169,7 +2156,6 @@ describe Topic do
     it 'should return the right topics' do
       category = Fabricate(:category_with_definition, read_restricted: true)
       topic = Fabricate(:topic, category: category, created_at: 1.day.ago)
-      user = Fabricate(:user)
       group.add(user)
       private_category = Fabricate(:private_category_with_definition, group: group)
 
@@ -2388,8 +2374,6 @@ describe Topic do
     end
 
     it "limits according to max_personal_messages_per_day" do
-      user1 = Fabricate(:user)
-      user2 = Fabricate(:user)
       create_post(user: user, archetype: 'private_message', target_usernames: [user1.username, user2.username])
       expect {
         create_post(user: user, archetype: 'private_message', target_usernames: [user1.username, user2.username])
@@ -2546,7 +2530,6 @@ describe Topic do
 
     context 'when category restricts present' do
       let!(:link_category) { Fabricate(:link_category) }
-      fab!(:topic) { Fabricate(:topic) }
       let(:link_topic) { Fabricate(:topic, category: link_category) }
 
       it 'can save the featured link if it belongs to that category' do
@@ -2678,9 +2661,9 @@ describe Topic do
   end
 
   describe '#pm_with_non_human_user?' do
-    let(:robot) { Fabricate(:user, id: -3) }
+    fab!(:robot) { Fabricate(:user, id: -3) }
 
-    let(:topic) do
+    fab!(:topic) do
       topic = Fabricate(:private_message_topic,
         topic_allowed_users: [
           Fabricate.build(:topic_allowed_user, user: robot),
@@ -2736,10 +2719,10 @@ describe Topic do
 
     describe 'removing oneself' do
       it 'should remove onself' do
-        topic.allowed_users << another_user
+        topic.allowed_users << user1
 
-        expect(topic.remove_allowed_user(another_user, another_user)).to eq(true)
+        expect(topic.remove_allowed_user(user1, user1)).to eq(true)
-        expect(topic.allowed_users.include?(another_user)).to eq(false)
+        expect(topic.allowed_users.include?(user1)).to eq(false)
 
         post = Post.last
 

spec/models/user_spec.rb

@@ -174,7 +174,7 @@ describe User do
 
   context 'enqueue_staff_welcome_message' do
     fab!(:first_admin) { Fabricate(:admin) }
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
 
     it 'enqueues message for admin' do
       expect {
@@ -558,14 +558,14 @@ describe User do
   end
 
   describe 'username format' do
+    fab!(:user) { Fabricate(:user) }
+
     def assert_bad(username)
-      user = Fabricate(:user)
       user.username = username
       expect(user.valid?).to eq(false)
     end
 
     def assert_good(username)
-      user = Fabricate(:user)
       user.username = username
       expect(user.valid?).to eq(true)
     end
@@ -895,7 +895,7 @@ describe User do
   end
 
   describe "previous_visit_at" do
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
     let!(:first_visit_date) { Time.zone.now }
     let!(:second_visit_date) { 2.hours.from_now }
     let!(:third_visit_date) { 5.hours.from_now }
@@ -2189,7 +2189,7 @@ describe User do
   end
 
   describe '#title=' do
-    let(:badge) { Fabricate(:badge, name: 'Badge', allow_title: false) }
+    fab!(:badge) { Fabricate(:badge, name: 'Badge', allow_title: false) }
 
     it 'sets badge_granted_title correctly' do
       BadgeGranter.grant(badge, user)
@@ -2234,10 +2234,10 @@ describe User do
   end
 
   describe '#next_best_title' do
-    let(:group_a) { Fabricate(:group, title: 'Group A') }
+    fab!(:group_a) { Fabricate(:group, title: 'Group A') }
-    let(:group_b) { Fabricate(:group, title: 'Group B') }
+    fab!(:group_b) { Fabricate(:group, title: 'Group B') }
-    let(:group_c) { Fabricate(:group, title: 'Group C') }
+    fab!(:group_c) { Fabricate(:group, title: 'Group C') }
-    let(:badge) { Fabricate(:badge, name: 'Badge', allow_title: true) }
+    fab!(:badge) { Fabricate(:badge, name: 'Badge', allow_title: true) }
 
     it 'only includes groups with title' do
       group_a.add(user)

spec/requests/list_controller_spec.rb

@@ -142,9 +142,9 @@ RSpec.describe ListController do
   end
 
   describe "filter private messages by tag" do
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
-    let(:moderator) { Fabricate(:moderator) }
+    fab!(:moderator) { Fabricate(:moderator) }
-    let(:admin) { Fabricate(:admin) }
+    fab!(:admin) { Fabricate(:admin) }
     let(:tag) { Fabricate(:tag) }
     let(:private_message) { Fabricate(:private_message_topic, user: admin) }
 
@@ -191,7 +191,7 @@ RSpec.describe ListController do
   end
 
   describe '#private_messages_group' do
-    let(:user) { Fabricate(:user) }
+    fab!(:user) { Fabricate(:user) }
 
     describe 'with personal_messages disabled' do
       let!(:topic) { Fabricate(:private_message_topic, allowed_groups: [group]) }
@@ -732,10 +732,10 @@ RSpec.describe ListController do
   end
 
   describe "#private_messages_warnings" do
-    let(:target_user) { Fabricate(:user) }
+    fab!(:target_user) { Fabricate(:user) }
-    let(:admin) { Fabricate(:admin) }
+    fab!(:admin) { Fabricate(:admin) }
-    let(:moderator1) { Fabricate(:moderator) }
+    fab!(:moderator1) { Fabricate(:moderator) }
-    let(:moderator2) { Fabricate(:moderator) }
+    fab!(:moderator2) { Fabricate(:moderator) }
 
     let(:create_args) do
       { title: 'you need a warning buddy!',