From 102fa71ef37ded9f05bc80dea637a2dc293bc5c6 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Wed, 15 Dec 2021 11:41:14 -0600 Subject: [PATCH] PERF: Speed up the tests by pre-fabricating more things (#15318) --- spec/components/post_creator_spec.rb | 24 ++--- spec/components/post_destroyer_spec.rb | 11 ++- spec/components/post_revisor_spec.rb | 5 +- spec/models/notification_spec.rb | 8 +- spec/models/post_spec.rb | 11 ++- spec/models/topic_spec.rb | 28 ++---- spec/models/user_action_spec.rb | 7 +- spec/models/user_spec.rb | 57 ++++++------ spec/requests/admin/users_controller_spec.rb | 12 ++- spec/requests/post_actions_controller_spec.rb | 15 +-- spec/services/post_alerter_spec.rb | 93 ++++++------------- spec/services/user_merger_spec.rb | 5 +- 12 files changed, 118 insertions(+), 158 deletions(-) diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 4948eb05483..0e723088f25 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -7,6 +7,8 @@ require 'topic_subtype' describe PostCreator do fab!(:user) { Fabricate(:user) } + fab!(:coding_horror) { Fabricate(:coding_horror) } + fab!(:evil_trout) { Fabricate(:evil_trout) } let(:topic) { Fabricate(:topic, user: user) } context "new topic" do @@ -810,7 +812,7 @@ describe PostCreator do context "topic stats" do before do PostCreator.new( - Fabricate(:coding_horror), + coding_horror, raw: 'first post in topic', topic_id: topic.id, created_at: Time.zone.now - 24.hours @@ -916,7 +918,7 @@ describe PostCreator do # integration test ... minimise db work context 'private message' do - let(:target_user1) { Fabricate(:coding_horror) } + let(:target_user1) { coding_horror } fab!(:target_user2) { Fabricate(:moderator) } fab!(:unrelated_user) { Fabricate(:user) } let(:post) do @@ -1021,7 +1023,7 @@ describe PostCreator do end context "warnings" do - let(:target_user1) { Fabricate(:coding_horror) } + let(:target_user1) { coding_horror } fab!(:target_user2) { Fabricate(:moderator) } let(:base_args) do { title: 'you need a warning buddy!', @@ -1102,7 +1104,7 @@ describe PostCreator do end context 'private message to group' do - let(:target_user1) { Fabricate(:coding_horror) } + let(:target_user1) { coding_horror } fab!(:target_user2) { Fabricate(:moderator) } let(:group) do g = Fabricate.build(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) @@ -1482,7 +1484,7 @@ describe PostCreator do end context "private message to a muted user" do - fab!(:muted_me) { Fabricate(:evil_trout) } + fab!(:muted_me) { evil_trout } fab!(:another_user) { Fabricate(:user) } it 'should fail' do @@ -1523,7 +1525,7 @@ describe PostCreator do end context "private message to an ignored user" do - fab!(:ignorer) { Fabricate(:evil_trout) } + fab!(:ignorer) { evil_trout } fab!(:another_user) { Fabricate(:user) } context "when post author is ignored" do @@ -1565,7 +1567,7 @@ describe PostCreator do end context "private message to user in allow list" do - fab!(:sender) { Fabricate(:evil_trout) } + fab!(:sender) { evil_trout } fab!(:allowed_user) { Fabricate(:user) } context "when post author is allowed" do @@ -1611,7 +1613,7 @@ describe PostCreator do end context "private message to user not in allow list" do - fab!(:sender) { Fabricate(:evil_trout) } + fab!(:sender) { evil_trout } fab!(:allowed_user) { Fabricate(:user) } fab!(:not_allowed_user) { Fabricate(:user) } @@ -1672,7 +1674,7 @@ describe PostCreator do end context "private message to multiple users and one is not allowed" do - fab!(:sender) { Fabricate(:evil_trout) } + fab!(:sender) { evil_trout } fab!(:allowed_user) { Fabricate(:user) } fab!(:not_allowed_user) { Fabricate(:user) } @@ -1700,8 +1702,8 @@ describe PostCreator do end context "private message recipients limit (max_allowed_message_recipients) reached" do - fab!(:target_user1) { Fabricate(:coding_horror) } - fab!(:target_user2) { Fabricate(:evil_trout) } + fab!(:target_user1) { coding_horror } + fab!(:target_user2) { evil_trout } fab!(:target_user3) { Fabricate(:walter_white) } before do diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 530df2385ce..51a2bcc538a 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -10,6 +10,7 @@ describe PostDestroyer do fab!(:moderator) { Fabricate(:moderator) } fab!(:admin) { Fabricate(:admin) } + fab!(:coding_horror) { Fabricate(:coding_horror) } let(:post) { create_post } describe "destroy_old_hidden_posts" do @@ -92,7 +93,7 @@ describe PostDestroyer do # flag the post, it should not nuke the stub anymore topic.recover! - reviewable = PostActionCreator.spam(Fabricate(:coding_horror), reply1).reviewable + reviewable = PostActionCreator.spam(coding_horror, reply1).reviewable PostDestroyer.destroy_stubs @@ -321,7 +322,7 @@ describe PostDestroyer do end describe "recovery and post actions" do - fab!(:codinghorror) { Fabricate(:coding_horror) } + fab!(:codinghorror) { coding_horror } let!(:like) { PostActionCreator.like(codinghorror, post).post_action } let!(:another_like) { PostActionCreator.like(moderator, post).post_action } @@ -644,7 +645,7 @@ describe PostDestroyer do fab!(:user) { Fabricate(:user) } let!(:post) { create_post(user: user) } let(:topic) { post.topic } - fab!(:second_user) { Fabricate(:coding_horror) } + fab!(:second_user) { coding_horror } let!(:second_post) { create_post(topic: topic, user: second_user) } before do @@ -777,7 +778,7 @@ describe PostDestroyer do describe 'after delete' do - fab!(:coding_horror) { Fabricate(:coding_horror) } + fab!(:coding_horror) { coding_horror } fab!(:post) { Fabricate(:post, raw: "Hello @CodingHorror") } it "should feature the users again (in case they've changed)" do @@ -930,7 +931,7 @@ describe PostDestroyer do end describe "user actions" do - let(:codinghorror) { Fabricate(:coding_horror) } + let(:codinghorror) { coding_horror } let(:second_post) { Fabricate(:post, topic_id: post.topic_id) } def create_user_action(action_type) diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index d0c318a2c11..e45e95de338 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -8,6 +8,7 @@ describe PostRevisor do fab!(:topic) { Fabricate(:topic) } fab!(:newuser) { Fabricate(:newuser, last_seen_at: Date.today) } fab!(:user) { Fabricate(:user) } + fab!(:coding_horror) { Fabricate(:coding_horror) } fab!(:admin) { Fabricate(:admin) } fab!(:moderator) { Fabricate(:moderator) } let(:post_args) { { user: newuser, topic: topic } } @@ -523,7 +524,7 @@ describe PostRevisor do end describe 'rate limiter' do - fab!(:changed_by) { Fabricate(:coding_horror) } + fab!(:changed_by) { coding_horror } before do RateLimiter.enable @@ -618,7 +619,7 @@ describe PostRevisor do SiteSetting.editing_grace_period_max_diff = 1000 end - fab!(:changed_by) { Fabricate(:coding_horror) } + fab!(:changed_by) { coding_horror } let!(:result) { subject.revise!(changed_by, raw: "lets update the body. Здравствуйте") } it 'correctly updates raw' do diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index a99cde5c04b..449a5c10433 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe Notification do + fab!(:coding_horror) { Fabricate(:coding_horror) } + before do NotificationEmailer.enable end @@ -60,8 +62,6 @@ describe Notification do { user: topic.user, topic: topic } end - let(:coding_horror) { Fabricate(:coding_horror) } - describe 'replies' do def process_alerts(post) PostAlerter.post_created(post) @@ -89,7 +89,7 @@ describe Notification do describe 'watching' do it "does notify watching users of new posts" do post = PostAlerter.post_created(Fabricate(:post, post_args)) - user2 = Fabricate(:coding_horror) + user2 = coding_horror post_args[:topic].notify_watch!(user2) expect { PostAlerter.post_created(Fabricate(:post, user: post.user, topic: post.topic)) @@ -101,7 +101,7 @@ describe Notification do it "does not notify users of new posts" do post = Fabricate(:post, post_args) user = post_args[:user] - user2 = Fabricate(:coding_horror) + user2 = coding_horror post_args[:topic].notify_muted!(user) expect { diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index ccd9e20481c..84475ff147e 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe Post do + fab!(:coding_horror) { Fabricate(:coding_horror) } + before { Oneboxer.stubs :onebox } let(:upload_path) { Discourse.store.upload_path } @@ -195,7 +197,7 @@ describe Post do describe 'flagging helpers' do fab!(:post) { Fabricate(:post) } - fab!(:user) { Fabricate(:coding_horror) } + fab!(:user) { coding_horror } fab!(:admin) { Fabricate(:admin) } it 'is_flagged? is accurate' do @@ -736,7 +738,7 @@ describe Post do end describe 'rate limiter' do - let(:changed_by) { Fabricate(:coding_horror) } + let(:changed_by) { coding_horror } it "triggers a rate limiter" do EditRateLimiter.any_instance.expects(:performed!) @@ -745,7 +747,7 @@ describe Post do end describe 'with a new body' do - let(:changed_by) { Fabricate(:coding_horror) } + let(:changed_by) { coding_horror } let!(:result) { post.revise(changed_by, raw: 'updated body') } it 'acts correctly' do @@ -836,7 +838,7 @@ describe Post do describe 'a new reply' do fab!(:topic) { Fabricate(:topic) } - let(:other_user) { Fabricate(:coding_horror) } + let(:other_user) { coding_horror } let(:reply_text) { "[quote=\"Evil Trout, post:1\"]\nhello\n[/quote]\nHmmm!" } let!(:post) { PostCreator.new(topic.user, raw: Fabricate.build(:post).raw, topic_id: topic.id).create } let!(:reply) { PostCreator.new(other_user, raw: reply_text, topic_id: topic.id, reply_to_post_number: post.post_number).create } @@ -1232,7 +1234,6 @@ describe Post do describe "#set_owner" do fab!(:post) { Fabricate(:post) } - fab!(:coding_horror) { Fabricate(:coding_horror) } it "will change owner of a post correctly" do post.set_owner(coding_horror, Discourse.system_user) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 75131283913..17c06689060 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -6,7 +6,11 @@ require 'rails_helper' describe Topic do let(:now) { Time.zone.local(2013, 11, 20, 8, 0) } fab!(:user) { 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) } @@ -262,8 +266,8 @@ describe Topic do end context 'topic title uniqueness' do - let!(:category1) { Fabricate(:category) } - let!(:category2) { Fabricate(:category) } + fab!(:category1) { Fabricate(:category) } + fab!(:category2) { Fabricate(:category) } let!(:topic) { Fabricate(:topic, category: category1) } let(:new_topic) { Fabricate.build(:topic, title: topic.title, category: category1) } @@ -849,8 +853,6 @@ describe Topic do end describe 'when topic belongs to a private category' do - fab!(:group) { Fabricate(:group) } - fab!(:category) do Fabricate(:category_with_definition, groups: [group]).tap do |category| category.set_permissions(group => :full) @@ -932,8 +934,6 @@ describe Topic do end context 'private message' do - let(:coding_horror) { Fabricate(:coding_horror) } - fab!(:evil_trout) { Fabricate(:evil_trout) } let(:topic) do PostCreator.new( Fabricate(:user), @@ -957,8 +957,6 @@ describe Topic do context 'existing user' do context 'by group name' do - fab!(:group) { Fabricate(:group) } - it 'can add admin to allowed groups' do admins = Group[:admins] admins.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) @@ -1123,21 +1121,21 @@ describe Topic do it "bumps the topic when a new version is made of the last post" do expect { - @last_post.revise(Fabricate(:moderator), raw: 'updated contents') + @last_post.revise(moderator, raw: 'updated contents') topic.reload }.to change(topic, :bumped_at) end it "doesn't bump the topic when a post that isn't the last post receives a new version" do expect { - @earlier_post.revise(Fabricate(:moderator), raw: 'updated contents') + @earlier_post.revise(moderator, raw: 'updated contents') topic.reload }.not_to change(topic, :bumped_at) end it "doesn't bump the topic when a post have invalid topic title while edit" do expect { - @last_post.revise(Fabricate(:moderator), title: 'invalid title') + @last_post.revise(moderator, title: 'invalid title') topic.reload }.not_to change(topic, :bumped_at) end @@ -1145,7 +1143,6 @@ describe Topic do end context 'moderator posts' do - fab!(:moderator) { Fabricate(:moderator) } fab!(:topic) { Fabricate(:topic) } it 'creates a moderator post' do @@ -1340,7 +1337,6 @@ describe Topic do it_behaves_like 'a status that closes a topic' it 'should archive group message' do - group = Fabricate(:group) group.add(@user) topic = Fabricate(:private_message_topic, allowed_groups: [group]) @@ -1474,7 +1470,7 @@ describe Topic do context 'after a second post' do before do - @second_user = Fabricate(:coding_horror) + @second_user = coding_horror @new_post = create_post(topic: @topic, user: @second_user) @topic.reload end @@ -2173,7 +2169,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) - group = Fabricate(:group) user = Fabricate(:user) group.add(user) private_category = Fabricate(:private_category_with_definition, group: group) @@ -2189,7 +2184,6 @@ describe Topic do end describe 'all_allowed_users' do - fab!(:group) { Fabricate(:group) } fab!(:topic) { Fabricate(:topic, allowed_groups: [group]) } fab!(:allowed_user) { Fabricate(:user) } fab!(:allowed_group_user) { Fabricate(:user) } @@ -2274,7 +2268,6 @@ describe Topic do describe 'trash!' do context "its category's topic count" do - fab!(:moderator) { Fabricate(:moderator) } fab!(:category) { Fabricate(:category_with_definition) } it "subtracts 1 if topic is being deleted" do @@ -2482,7 +2475,6 @@ describe Topic do expect(topic.message_archived?(user)).to eq(false) - group = Fabricate(:group) group2 = Fabricate(:group) group.add(user) diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index d1d781fd880..efc053952b6 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' describe UserAction do + fab!(:coding_horror) { Fabricate(:coding_horror) } before do UserActionManager.enable @@ -91,7 +92,7 @@ describe UserAction do expect(stream).to eq([]) group = Fabricate(:group) - u = Fabricate(:coding_horror) + u = coding_horror group.add(u) category.set_permissions(group => :full) @@ -167,7 +168,7 @@ describe UserAction do fab!(:post) { Fabricate(:post) } let(:likee) { post.user } - fab!(:liker) { Fabricate(:coding_horror) } + fab!(:liker) { coding_horror } def likee_stream UserAction.stream(user_id: likee.id, guardian: Guardian.new) @@ -257,7 +258,7 @@ describe UserAction do describe 'when another user posts on the topic' do before do - @other_user = Fabricate(:coding_horror) + @other_user = coding_horror @mentioned = Fabricate(:admin) @response = PostCreator.new(@other_user, reply_to_post_number: 1, topic_id: @post.topic_id, raw: "perhaps @#{@mentioned.username} knows how this works?").create diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 564503d6b3f..26db9bf921d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe User do + fab!(:group) { Fabricate(:group) } + let(:user) { Fabricate(:user, last_seen_at: 1.day.ago) } def user_error_message(*keys) @@ -28,7 +30,6 @@ describe User do describe 'when group with a same name already exists' do it 'should not be valid' do - group = Fabricate(:group) new_user = Fabricate.build(:user, username: group.name.upcase) expect(new_user).to_not be_valid @@ -152,7 +153,7 @@ describe User do end context '.enqueue_welcome_message' do - let(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user) } it 'enqueues the system message' do SiteSetting.send_welcome_message = true @@ -172,7 +173,7 @@ describe User do end context 'enqueue_staff_welcome_message' do - let!(:first_admin) { Fabricate(:admin) } + fab!(:first_admin) { Fabricate(:admin) } let(:user) { Fabricate(:user) } it 'enqueues message for admin' do @@ -261,7 +262,7 @@ describe User do end describe 'bookmark' do - before do + before_all do @post = Fabricate(:post) end @@ -286,7 +287,7 @@ describe User do end describe 'delete posts in batches' do - before do + before_all do @post1 = Fabricate(:post) @user = @post1.user @post2 = Fabricate(:post, topic: @post1.topic, user: @user) @@ -486,7 +487,7 @@ describe User do end describe 'email_hash' do - before do + before_all do @user = Fabricate(:user) end @@ -652,7 +653,7 @@ describe User do end describe 'username uniqueness' do - before do + before_all do @user = Fabricate.build(:user) @user.save! @codinghorror = Fabricate.build(:coding_horror) @@ -934,7 +935,7 @@ describe User do end describe "update_last_seen!" do - let(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user) } let!(:first_visit_date) { Time.zone.now } let!(:second_visit_date) { 2.hours.from_now } @@ -1092,10 +1093,10 @@ describe User do describe "flag_linked_posts_as_spam" do fab!(:user) { Fabricate(:user) } - let!(:admin) { Fabricate(:admin) } - let!(:post) { PostCreator.new(user, title: "this topic contains spam", raw: "this post has a link: http://discourse.org").create } - let!(:another_post) { PostCreator.new(user, title: "this topic also contains spam", raw: "this post has a link: http://discourse.org/asdfa").create } - let!(:post_without_link) { PostCreator.new(user, title: "this topic shouldn't be spam", raw: "this post has no links in it.").create } + fab!(:admin) { Fabricate(:admin) } + fab!(:post) { PostCreator.new(user, title: "this topic contains spam", raw: "this post has a link: http://discourse.org").create } + fab!(:another_post) { PostCreator.new(user, title: "this topic also contains spam", raw: "this post has a link: http://discourse.org/asdfa").create } + fab!(:post_without_link) { PostCreator.new(user, title: "this topic shouldn't be spam", raw: "this post has no links in it.").create } it "has flagged all the user's posts as spam" do user.flag_linked_posts_as_spam @@ -1366,7 +1367,7 @@ describe User do describe "update_posts_read!" do context "with a UserVisit record" do - let!(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user) } let!(:now) { Time.zone.now } before { user.update_last_seen!(now) } after do @@ -1390,14 +1391,13 @@ describe User do end describe "primary_group_id" do - let!(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user) } it "has no primary_group_id by default" do expect(user.primary_group_id).to eq(nil) end context "when the user has a group" do - let!(:group) { Fabricate(:group) } before do group.usernames = user.username @@ -1470,12 +1470,12 @@ describe User do end describe "#purge_unactivated" do - let!(:user) { Fabricate(:user) } - let!(:unactivated) { Fabricate(:user, active: false) } - let!(:unactivated_old) { Fabricate(:user, active: false, created_at: 1.month.ago) } - let!(:unactivated_old_with_system_pm) { Fabricate(:user, active: false, created_at: 2.months.ago) } - let!(:unactivated_old_with_human_pm) { Fabricate(:user, active: false, created_at: 2.months.ago) } - let!(:unactivated_old_with_post) { Fabricate(:user, active: false, created_at: 1.month.ago) } + fab!(:user) { Fabricate(:user) } + fab!(:unactivated) { Fabricate(:user, active: false) } + fab!(:unactivated_old) { Fabricate(:user, active: false, created_at: 1.month.ago) } + fab!(:unactivated_old_with_system_pm) { Fabricate(:user, active: false, created_at: 2.months.ago) } + fab!(:unactivated_old_with_human_pm) { Fabricate(:user, active: false, created_at: 2.months.ago) } + fab!(:unactivated_old_with_post) { Fabricate(:user, active: false, created_at: 1.month.ago) } before do PostCreator.new(Discourse.system_user, @@ -1540,7 +1540,7 @@ describe User do describe "automatic group membership" do - let!(:group) { + fab!(:group) { Fabricate(:group, automatic_membership_email_domains: "bar.com|wat.com", grant_trust_level: 1, @@ -1599,10 +1599,9 @@ describe User do describe 'staff info' do fab!(:user) { Fabricate(:user) } + fab!(:moderator) { Fabricate(:moderator) } describe "#number_of_flags_given" do - fab!(:moderator) { Fabricate(:moderator) } - it "doesn't count disagreed flags" do post_agreed = Fabricate(:post) PostActionCreator.inappropriate(user, post_agreed).reviewable.perform(moderator, :agree_and_keep) @@ -1618,8 +1617,6 @@ describe User do end describe "number_of_deleted_posts" do - fab!(:moderator) { Fabricate(:moderator) } - it "counts all the posts" do # at least 1 "unchanged" post Fabricate(:post, user: user) @@ -1841,7 +1838,7 @@ describe User do describe ".clear_global_notice_if_needed" do fab!(:user) { Fabricate(:user) } - let(:admin) { Fabricate(:admin) } + fab!(:admin) { Fabricate(:admin) } before do SiteSetting.has_login_hint = true @@ -2449,9 +2446,9 @@ describe User do end describe 'Granting admin or moderator status' do - it 'approves the associated reviewable when granting admin status' do - reviewable_user = Fabricate(:reviewable_user) + fab!(:reviewable_user) { Fabricate(:reviewable_user) } + it 'approves the associated reviewable when granting admin status' do reviewable_user.target.grant_admin! expect(reviewable_user.reload.status).to eq Reviewable.statuses[:approved] @@ -2467,8 +2464,6 @@ describe User do end it 'approves the associated reviewable when granting moderator status' do - reviewable_user = Fabricate(:reviewable_user) - reviewable_user.target.grant_moderation! expect(reviewable_user.reload.status).to eq Reviewable.statuses[:approved] diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 19b41802705..9c2b249d339 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -7,6 +7,7 @@ require 'rotp' RSpec.describe Admin::UsersController do fab!(:admin) { Fabricate(:admin) } fab!(:user) { Fabricate(:user) } + fab!(:coding_horror) { Fabricate(:coding_horror) } it 'is a subclass of AdminController' do expect(Admin::UsersController < Admin::AdminController).to eq(true) @@ -339,7 +340,7 @@ RSpec.describe Admin::UsersController do end describe '#grant_admin' do - fab!(:another_user) { Fabricate(:coding_horror) } + fab!(:another_user) { coding_horror } after do Discourse.redis.flushdb @@ -462,7 +463,10 @@ RSpec.describe Admin::UsersController do end describe '#trust_level' do - fab!(:another_user) { Fabricate(:coding_horror, created_at: 1.month.ago) } + fab!(:another_user) { + coding_horror.update!(created_at: 1.month.ago) + coding_horror + } it "raises an error when the user doesn't have permission" do sign_in(user) @@ -509,7 +513,7 @@ RSpec.describe Admin::UsersController do end describe '#grant_moderation' do - fab!(:another_user) { Fabricate(:coding_horror) } + fab!(:another_user) { coding_horror } it "raises an error when the user doesn't have permission" do sign_in(user) @@ -570,7 +574,7 @@ RSpec.describe Admin::UsersController do describe '#primary_group' do fab!(:group) { Fabricate(:group) } - fab!(:another_user) { Fabricate(:coding_horror) } + fab!(:another_user) { coding_horror } fab!(:another_group) { Fabricate(:group, title: 'New') } it "raises an error when the user doesn't have permission" do diff --git a/spec/requests/post_actions_controller_spec.rb b/spec/requests/post_actions_controller_spec.rb index f2b8d60dd8d..cfc829e7d06 100644 --- a/spec/requests/post_actions_controller_spec.rb +++ b/spec/requests/post_actions_controller_spec.rb @@ -3,8 +3,11 @@ require 'rails_helper' RSpec.describe PostActionsController do + fab!(:user) { Fabricate(:user) } + fab!(:coding_horror) { Fabricate(:coding_horror) } + describe '#destroy' do - fab!(:post) { Fabricate(:post, user: Fabricate(:coding_horror)) } + fab!(:post) { Fabricate(:post, user: coding_horror) } it 'requires you to be logged in' do delete "/post_actions/#{post.id}.json" @@ -12,8 +15,6 @@ RSpec.describe PostActionsController do end context 'logged in' do - fab!(:user) { Fabricate(:user) } - before do sign_in(user) end @@ -81,8 +82,8 @@ RSpec.describe PostActionsController do end it 'fails when the user does not have permission to see the post' do - sign_in(Fabricate(:user)) - pm = Fabricate(:private_message_post, user: Fabricate(:coding_horror)) + sign_in(user) + pm = Fabricate(:private_message_post, user: coding_horror) post "/post_actions.json", params: { id: pm.id, @@ -93,7 +94,7 @@ RSpec.describe PostActionsController do end it 'fails when the user tries to notify user that has disabled PM' do - sign_in(Fabricate(:user)) + sign_in(user) user2 = Fabricate(:user) post = Fabricate(:post, user: user2) @@ -115,7 +116,7 @@ RSpec.describe PostActionsController do describe 'as a moderator' do fab!(:user) { Fabricate(:moderator) } - fab!(:post_1) { Fabricate(:post, user: Fabricate(:coding_horror)) } + fab!(:post_1) { Fabricate(:post, user: coding_horror) } before do sign_in(user) diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 1723c8fa0fc..9e03a0ba0cc 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -26,8 +26,21 @@ end RSpec::Matchers.define_negated_matcher :not_add_notification, :add_notification describe PostAlerter do + fab!(:category) { Fabricate(:category) } + fab!(:topic) { Fabricate(:topic) } + fab!(:post) { Fabricate(:post) } + + fab!(:private_message_topic) { Fabricate(:private_message_topic) } + fab!(:private_message_topic_post1) { Fabricate(:post, topic: private_message_topic) } + fab!(:private_message_topic_post2) { Fabricate(:post, topic: private_message_topic) } + + fab!(:group) { Fabricate(:group) } + + fab!(:admin) { Fabricate(:admin) } fab!(:evil_trout) { Fabricate(:evil_trout) } + fab!(:coding_horror) { Fabricate(:coding_horror) } + fab!(:walterwhite) { Fabricate(:walter_white) } fab!(:user) { Fabricate(:user) } fab!(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) } @@ -174,7 +187,6 @@ describe PostAlerter do end it "Doesn't notify non-admin users when their post is quoted inside a whisper" do - admin = Fabricate(:admin) group.add(admin) TopicUser.change(user2.id, pm.id, notification_level: TopicUser.notification_levels[:regular]) @@ -193,14 +205,13 @@ describe PostAlerter do context "unread" do it "does not return whispers as unread posts" do - op = Fabricate(:post) _whisper = Fabricate(:post, raw: 'this is a whisper post', - user: Fabricate(:admin), - topic: op.topic, - reply_to_post_number: op.post_number, + user: admin, + topic: post.topic, + reply_to_post_number: post.post_number, post_type: Post.types[:whisper]) - expect(PostAlerter.new.first_unread_post(op.user, op.topic)).to be_blank + expect(PostAlerter.new.first_unread_post(post.user, post.topic)).to be_blank end end @@ -211,8 +222,6 @@ describe PostAlerter do post = Fabricate(:post, raw: 'I love waffles') - admin = Fabricate(:admin) - expect do post.revise(admin, raw: 'I made a revision') end.to add_notification(post.user, :edited) @@ -254,8 +263,6 @@ describe PostAlerter do it 'notifies flaggers when flagged post gets unhidden by edit' do post = create_post - walterwhite = Fabricate(:walter_white) - coding_horror = Fabricate(:coding_horror) PostActionNotifier.enable Reviewable.set_priorities(high: 4.0) @@ -295,8 +302,8 @@ describe PostAlerter do end context 'quotes' do - let(:category) { Fabricate(:category) } - let(:topic) { Fabricate(:topic, category: category) } + fab!(:category) { Fabricate(:category) } + fab!(:topic) { Fabricate(:topic, category: category) } it 'does not notify for muted users' do post = Fabricate(:post, raw: '[quote="EvilTrout, post:1"]whatup[/quote]', topic: topic) @@ -383,8 +390,6 @@ describe PostAlerter do expect(user.notifications.count).to eq(1) - topic = Fabricate(:topic) - watcher = Fabricate(:user) TopicUser.create!(user_id: watcher.id, topic_id: topic.id, notification_level: TopicUser.notification_levels[:watching]) @@ -409,7 +414,6 @@ describe PostAlerter do it "doesn't notify the linked user if the user is staged and the category is restricted and allows strangers" do staged_user = Fabricate(:staged) - group = Fabricate(:group) group_member = Fabricate(:user) group.add(group_member) @@ -431,9 +435,8 @@ describe PostAlerter do end context '@here' do - let(:topic) { Fabricate(:topic) } let(:post) { create_post_with_alerts(raw: "Hello @here how are you?", user: tl2_user, topic: topic) } - let(:other_post) { Fabricate(:post, topic: topic) } + fab!(:other_post) { Fabricate(:post, topic: topic) } before do Jobs.run_immediately! @@ -567,13 +570,12 @@ describe PostAlerter do end it "notification comes from editor if mention is added later" do - admin = Fabricate(:admin) - post = create_post_with_alerts(user: user, raw: 'No mention here.') - expect { - post.revise(admin, raw: "Mention @eviltrout in this edit.") - }.to change(evil_trout.notifications, :count) - n = evil_trout.notifications.last - expect(n.data_hash["original_username"]).to eq(admin.username) + post = create_post_with_alerts(user: user, raw: 'No mention here.') + expect { + post.revise(admin, raw: "Mention @eviltrout in this edit.") + }.to change(evil_trout.notifications, :count) + n = evil_trout.notifications.last + expect(n.data_hash["original_username"]).to eq(admin.username) end it "doesn't notify the last post editor if they mention themselves" do @@ -984,7 +986,6 @@ describe PostAlerter do describe "create_notification_alert" do it "does nothing for suspended users" do evil_trout.update_columns(suspended_till: 1.year.from_now) - post = Fabricate(:post) events = nil messages = MessageBus.track_publish do @@ -1006,7 +1007,6 @@ describe PostAlerter do it "does not publish to MessageBus /notification-alert if the user has not been seen for > 30 days, but still sends a push notification" do evil_trout.update_columns(last_seen_at: 31.days.ago) - post = Fabricate(:post) SiteSetting.allowed_user_api_push_urls = "https://site2.com/push" UserApiKey.create!(user_id: evil_trout.id, @@ -1035,7 +1035,6 @@ describe PostAlerter do end describe "watching_first_post" do - fab!(:group) { Fabricate(:group) } fab!(:user) { Fabricate(:user) } fab!(:category) { Fabricate(:category) } fab!(:tag) { Fabricate(:tag) } @@ -1092,8 +1091,6 @@ describe PostAlerter do context "replies" do it "triggers :before_create_notifications_for_users" do - user = Fabricate(:user) - topic = Fabricate(:topic) _post = Fabricate(:post, user: user, topic: topic) reply = Fabricate(:post, topic: topic, reply_to_post_number: 1) events = DiscourseEvent.track_events do @@ -1103,8 +1100,6 @@ describe PostAlerter do end it "notifies about regular reply" do - user = Fabricate(:user) - topic = Fabricate(:topic) _post = Fabricate(:post, user: user, topic: topic) reply = Fabricate(:post, topic: topic, reply_to_post_number: 1) @@ -1114,10 +1109,6 @@ describe PostAlerter do end it "doesn't notify regular user about whispered reply" do - user = Fabricate(:user) - admin = Fabricate(:admin) - - topic = Fabricate(:topic) _post = Fabricate(:post, user: user, topic: topic) whispered_reply = Fabricate(:post, user: admin, topic: topic, post_type: Post.types[:whisper], reply_to_post_number: 1) @@ -1127,11 +1118,9 @@ describe PostAlerter do end it "notifies staff user about whispered reply" do - user = Fabricate(:user) admin1 = Fabricate(:admin) admin2 = Fabricate(:admin) - topic = Fabricate(:topic) _post = Fabricate(:post, user: user, topic: topic) whispered_reply1 = Fabricate(:post, user: admin1, topic: topic, post_type: Post.types[:whisper], reply_to_post_number: 1) @@ -1231,8 +1220,6 @@ describe PostAlerter do context "category" do context "watching" do it "triggers :before_create_notifications_for_users" do - user = Fabricate(:user) - category = Fabricate(:category) topic = Fabricate(:topic, category: category) post = Fabricate(:post, topic: topic) level = CategoryUser.notification_levels[:watching] @@ -1244,10 +1231,7 @@ describe PostAlerter do end it "notifies staff about whispered post" do - category = Fabricate(:category) topic = Fabricate(:topic, category: category) - admin = Fabricate(:admin) - user = Fabricate(:user) level = CategoryUser.notification_levels[:watching] CategoryUser.set_notification_level_for_category(admin, level, category.id) CategoryUser.set_notification_level_for_category(user, level, category.id) @@ -1263,7 +1247,6 @@ describe PostAlerter do it "notifies a staged user about a private post, but only if the user has access" do staged_member = Fabricate(:staged) staged_non_member = Fabricate(:staged) - group = Fabricate(:group) group_member = Fabricate(:user) group.add(group_member) @@ -1289,7 +1272,6 @@ describe PostAlerter do end it "does not update existing unread notification" do - category = Fabricate(:category) CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:watching], category.id) topic = Fabricate(:topic, category: category) @@ -1313,7 +1295,6 @@ describe PostAlerter do context "tags" do context "watching" do it "triggers :before_create_notifications_for_users" do - user = Fabricate(:user) tag = Fabricate(:tag) topic = Fabricate(:topic, tags: [tag]) post = Fabricate(:post, topic: topic) @@ -1350,7 +1331,6 @@ describe PostAlerter do topic = Fabricate(:topic, tags: [tag]) post = Fabricate(:post, topic: topic) tag_group = Fabricate(:tag_group, tags: [tag]) - group = Fabricate(:group) Fabricate(:tag_group_permission, tag_group: tag_group, group: group) TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching]) @@ -1363,7 +1343,6 @@ describe PostAlerter do topic = Fabricate(:topic, tags: [tag]) post = Fabricate(:post, topic: topic) tag_group = Fabricate(:tag_group, tags: [tag]) - group = Fabricate(:group) Fabricate(:group_user, group: group, user: user) Fabricate(:tag_group_permission, tag_group: tag_group, group: group) @@ -1377,7 +1356,6 @@ describe PostAlerter do fab!(:user) { Fabricate(:user) } fab!(:other_tag) { Fabricate(:tag) } fab!(:watched_tag) { Fabricate(:tag) } - fab!(:post) { Fabricate(:post) } before do SiteSetting.tagging_enabled = true @@ -1413,7 +1391,6 @@ describe PostAlerter do fab!(:other_tag3) { Fabricate(:tag) } fab!(:user) { Fabricate(:user) } fab!(:staged) { Fabricate(:staged) } - fab!(:admin) { Fabricate(:admin) } before do SiteSetting.tagging_enabled = true @@ -1440,7 +1417,6 @@ describe PostAlerter do context "with tag groups" do fab!(:tag) { Fabricate(:tag) } - fab!(:group) { Fabricate(:group) } fab!(:user) { Fabricate(:user) } fab!(:topic) { Fabricate(:topic, tags: [tag]) } fab!(:post) { Fabricate(:post, topic: topic) } @@ -1498,7 +1474,6 @@ describe PostAlerter do end describe '#extract_linked_users' do - fab!(:topic) { Fabricate(:topic) } fab!(:post) { Fabricate(:post, topic: topic) } fab!(:post2) { Fabricate(:post) } @@ -1523,7 +1498,6 @@ describe PostAlerter do end describe '#notify_post_users' do - fab!(:topic) { Fabricate(:topic) } fab!(:post) { Fabricate(:post, topic: topic) } fab!(:last_editor) { Fabricate(:user) } fab!(:tag) { Fabricate(:tag) } @@ -1611,26 +1585,17 @@ describe PostAlerter do end it "does not error if SMTP is enabled and the topic has no incoming email or allowed groups" do - topic = Fabricate(:private_message_topic) - Fabricate(:post, topic: topic) - post = Fabricate(:post, topic: topic) expect { PostAlerter.new.after_save_post(post, true) }.not_to raise_error end it "does not error if SMTP is enabled and the topic has no incoming email but does have an allowed group" do - topic = Fabricate(:private_message_topic) - Fabricate(:post, topic: topic) - post = Fabricate(:post, topic: topic) - TopicAllowedGroup.create(topic: topic, group: Fabricate(:group)) + TopicAllowedGroup.create(topic: private_message_topic, group: group) expect { PostAlerter.new.after_save_post(post, true) }.not_to raise_error end it "does not error if SMTP is enabled and the topic has no incoming email but has multiple allowed groups" do - topic = Fabricate(:private_message_topic) - Fabricate(:post, topic: topic) - post = Fabricate(:post, topic: topic) - TopicAllowedGroup.create(topic: topic, group: Fabricate(:group)) - TopicAllowedGroup.create(topic: topic, group: Fabricate(:group)) + TopicAllowedGroup.create(topic: private_message_topic, group: group) + TopicAllowedGroup.create(topic: private_message_topic, group: Fabricate(:group)) expect { PostAlerter.new.after_save_post(post, true) }.not_to raise_error end diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 3ffb7ef8479..a063d4e892e 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -6,6 +6,7 @@ describe UserMerger do fab!(:target_user) { Fabricate(:user, username: 'alice', email: 'alice@example.com') } fab!(:source_user) { Fabricate(:user, username: 'alice1', email: 'alice@work.com') } fab!(:walter) { Fabricate(:walter_white) } + fab!(:coding_horror) { Fabricate(:coding_horror) } fab!(:p1) { Fabricate(:post) } fab!(:p2) { Fabricate(:post) } @@ -271,7 +272,6 @@ describe UserMerger do muted1 = Fabricate(:user) muted2 = Fabricate(:user) muted3 = Fabricate(:user) - coding_horror = Fabricate(:coding_horror) MutedUser.create!(user_id: source_user.id, muted_user_id: muted1.id) MutedUser.create!(user_id: source_user.id, muted_user_id: muted2.id) @@ -297,7 +297,6 @@ describe UserMerger do ignored1 = Fabricate(:user) ignored2 = Fabricate(:user) ignored3 = Fabricate(:user) - coding_horror = Fabricate(:coding_horror) Fabricate(:ignored_user, user: source_user, ignored_user: ignored1) Fabricate(:ignored_user, user: source_user, ignored_user: ignored2) @@ -705,8 +704,6 @@ describe UserMerger do end it "merges when acting_user is neither source_user nor target_user" do - coding_horror = Fabricate(:coding_horror) - pm_topic1 = Fabricate(:private_message_topic, topic_allowed_users: [ Fabricate.build(:topic_allowed_user, user: walter), Fabricate.build(:topic_allowed_user, user: source_user),