PERF: speed up spec suite, avoid featuring topics

Before: 6:05
After: 5:42

Featuring topics for `list/categories` is a very expensive operation that
happened each time we created a topic. This introduces a test only bypass
This commit is contained in:
Sam Saffron 2019-05-10 11:37:37 +10:00
parent 41f4f9302d
commit 0ab6c6e24e
3 changed files with 28 additions and 13 deletions

View File

@ -671,6 +671,8 @@ class Topic < ActiveRecord::Base
SQL
end
cattr_accessor :update_featured_topics
def changed_to_category(new_category)
return true if new_category.blank? || Category.exists?(topic_id: id)
return false if new_category.id == SiteSetting.uncategorized_category_id && !SiteSetting.allow_uncategorized_topics
@ -701,8 +703,11 @@ class Topic < ActiveRecord::Base
end
Category.where(id: new_category.id).update_all("topic_count = topic_count + 1")
CategoryFeaturedTopic.feature_topics_for(old_category) unless @import_mode
CategoryFeaturedTopic.feature_topics_for(new_category) unless @import_mode || old_category.try(:id) == new_category.id
if Topic.update_featured_topics != false
CategoryFeaturedTopic.feature_topics_for(old_category) unless @import_mode
CategoryFeaturedTopic.feature_topics_for(new_category) unless @import_mode || old_category.try(:id) == new_category.id
end
end
true

View File

@ -4,6 +4,10 @@ require 'rails_helper'
require 'category_list'
describe CategoryList do
before do
# we need automatic updating here cause we are testing this
Topic.update_featured_topics = true
end
fab!(:user) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
@ -66,7 +70,7 @@ describe CategoryList do
fab!(:topic_category) { Fabricate(:category, num_featured_topics: 2) }
context "with a topic in a category" do
fab!(:topic) { Fabricate(:topic, category: topic_category) }
let(:topic) { Fabricate(:topic, category: topic_category) }
let(:category) { category_list.categories.find { |c| c.id == topic_category.id } }
it "should return the category" do
@ -77,19 +81,22 @@ describe CategoryList do
end
context "with pinned topics in a category" do
fab!(:topic1) { Fabricate(:topic, category: topic_category, bumped_at: 8.minutes.ago) }
fab!(:topic2) { Fabricate(:topic, category: topic_category, bumped_at: 5.minutes.ago) }
fab!(:topic3) { Fabricate(:topic, category: topic_category, bumped_at: 2.minutes.ago) }
fab!(:pinned) { Fabricate(:topic, category: topic_category, pinned_at: 10.minutes.ago, bumped_at: 10.minutes.ago) }
let(:category) { category_list.categories.find { |c| c.id == topic_category.id } }
let!(:topic1) { Fabricate(:topic, category: topic_category, bumped_at: 8.minutes.ago) }
let!(:topic2) { Fabricate(:topic, category: topic_category, bumped_at: 5.minutes.ago) }
let!(:topic3) { Fabricate(:topic, category: topic_category, bumped_at: 2.minutes.ago) }
let!(:pinned) { Fabricate(:topic, category: topic_category, pinned_at: 10.minutes.ago, bumped_at: 10.minutes.ago) }
it "returns pinned topic first" do
expect(category.displayable_topics.map(&:id)).to eq([pinned.id, topic3.id])
def displayable_topics
category_list = CategoryList.new(Guardian.new(user), include_topics: true)
category_list.categories.find { |c| c.id == topic_category.id }.displayable_topics.map(&:id)
end
it "returns topics in bumped_at order if pinned was unpinned" do
PinnedCheck.stubs(:unpinned?).returns(true)
expect(category.displayable_topics.map(&:id)).to eq([topic3.id, topic2.id])
it "returns pinned topic first" do
expect(displayable_topics).to eq([pinned.id, topic3.id])
TopicUser.change(user.id, pinned.id, cleared_pinned_at: pinned.pinned_at + 10)
expect(displayable_topics).to eq([topic3.id, topic2.id])
end
end

View File

@ -117,6 +117,9 @@ module TestSetup
$test_cleanup_callbacks = nil
end
# in test this is very expensive, we explicitly enable when needed
Topic.update_featured_topics = false
# Running jobs are expensive and most of our tests are not concern with
# code that runs inside jobs. run_later! means they are put on the redis
# queue and never processed.