DEV: Prefabricate more things in topic_spec.rb (#7490)

DEV: Prefabricate more things in topic_spec.rb (#7490)

  • Moved let to more appropriate scopes

  • Refactored tests

It’s confusing when let blocks in a parent context depend on other let blocks from a child context.

  • Moved fabrication to top level

  • Removed unnecessary user fabrications

  • Added a trust level 2 user at the top level

  • Factored out category

  • Made test use generic user

  • Prefabricate topic

  • Cut down redundant users

  • Prefabricated more things

diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index dd3f8cb..8b6d441 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -7,7 +7,8 @@ require_dependency 'post_destroyer'
 describe Topic do
   let(:now) { Time.zone.local(2013, 11, 20, 8, 0) }
   fab!(:user) { Fabricate(:user) }
-  let(:topic) { Fabricate(:topic) }
+  fab!(:another_user) { Fabricate(:user) }
+  fab!(:trust_level_2) { Fabricate(:user, trust_level: TrustLevel[2]) }
 
   context 'validations' do
     let(:topic) { Fabricate.build(:topic) }
@@ -122,33 +123,35 @@ describe Topic do
   end
 
   context 'slug' do
-    let(:title) { "hello world topic" }
-    let(:slug) { "hello-world-topic" }
-    let!(:expected_title) { title.dup }
-    let!(:expected_slug) { slug.dup }
-    let(:topic) { Fabricate.build(:topic, title: title) }
-
     context 'encoded generator' do
       before { SiteSetting.slug_generation_method = 'encoded' }
 
-      it "returns a Slug for a title" do
-        expect(topic.title).to eq(expected_title)
-        expect(topic.slug).to eq(expected_slug)
+      context 'with ascii letters' do
+        let!(:title) { "hello world topic" }
+        let!(:slug) { "hello-world-topic" }
+        let!(:topic) { Fabricate.build(:topic, title: title) }
+
+        it "returns a Slug for a title" do
+          expect(topic.title).to eq(title)
+          expect(topic.slug).to eq(slug)
+        end
       end
 
       context 'for cjk characters' do
-        let(:title) { "熱帶風暴畫眉" }
-        let!(:expected_title) { title.dup }
+        let!(:title) { "熱帶風暴畫眉" }
+        let!(:topic) { Fabricate.build(:topic, title: title) }
 
         it "returns encoded Slug for a title" do
-          expect(topic.title).to eq(expected_title)
-          expect(topic.slug).to eq(expected_title)
+          expect(topic.title).to eq(title)
+          expect(topic.slug).to eq(title)
         end
       end
 
       context 'for numbers' do
-        let(:title) { "123456789" }
-        let(:slug) { "topic" }
+        let!(:title) { "123456789" }
+        let!(:slug) { "topic" }
+        let!(:topic) { Fabricate.build(:topic, title: title) }
+
         it 'generates default slug' do
           Slug.expects(:for).with(title).returns("topic")
           expect(Fabricate.build(:topic, title: title).slug).to eq("topic")
@@ -157,10 +160,11 @@ describe Topic do
     end
 
     context 'none generator' do
-      before { SiteSetting.slug_generation_method = 'none' }
+      let!(:title) { "熱帶風暴畫眉" }
+      let!(:slug) { "topic" }
+      let!(:topic) { Fabricate.build(:topic, title: title) }
 
-      let(:title) { "熱帶風暴畫眉" }
-      let(:slug) { "topic" }
+      before { SiteSetting.slug_generation_method = 'none' }
 
       it "returns a Slug for a title" do
         Slug.expects(:for).with(title).returns('topic')
@@ -171,14 +175,21 @@ describe Topic do
     context '#ascii_generator' do
       before { SiteSetting.slug_generation_method = 'ascii' }
 
-      it "returns a Slug for a title" do
-        Slug.expects(:for).with(title).returns(slug)
-        expect(Fabricate.build(:topic, title: title).slug).to eq(slug)
+      context 'with ascii letters' do
+        let!(:title) { "hello world topic" }
+        let!(:slug) { "hello-world-topic" }
+        let!(:topic) { Fabricate.build(:topic, title: title) }
+
+        it "returns a Slug for a title" do
+          Slug.expects(:for).with(title).returns(slug)
+          expect(Fabricate.build(:topic, title: title).slug).to eq(slug)
+        end
       end
 
       context 'for cjk characters' do
-        let(:title) { "熱帶風暴畫眉" }
-        let(:slug) { 'topic' }
+        let!(:title) { "熱帶風暴畫眉" }
+        let!(:slug) { 'topic' }
+        let!(:topic) { Fabricate.build(:topic, title: title) }
 
         it "returns 'topic' when the slug is empty (say, non-latin characters)" do
           Slug.expects(:for).with(title).returns("topic")
@@ -379,6 +390,8 @@ describe Topic do
   end
 
   context 'category validation' do
+    fab!(:category) { Fabricate(:category) }
+
     context 'allow_uncategorized_topics is false' do
       before do
         SiteSetting.allow_uncategorized_topics = false
@@ -396,7 +409,7 @@ describe Topic do
       end
 
       it 'passes for topics with a category' do
-        expect(Fabricate.build(:topic, category: Fabricate(:category))).to be_valid
+        expect(Fabricate.build(:topic, category: category)).to be_valid
       end
     end
 
@@ -410,7 +423,7 @@ describe Topic do
       end
 
       it 'passes for topics with a category' do
-        expect(Fabricate.build(:topic, category: Fabricate(:category))).to be_valid
+        expect(Fabricate.build(:topic, category: category)).to be_valid
       end
     end
   end
@@ -441,7 +454,7 @@ describe Topic do
       end
 
       context "secure categories" do
-        let(:category) { Fabricate(:category, read_restricted: true) }
+        fab!(:category) { Fabricate(:category, read_restricted: true) }
 
         before do
           topic.category = category
@@ -478,8 +491,7 @@ describe Topic do
   end
 
   describe '#invite' do
-    let(:topic) { Fabricate(:topic, user: user) }
-    let(:another_user) { Fabricate(:user) }
+    fab!(:topic) { Fabricate(:topic, user: user) }
 
     context 'rate limits' do
       before do
@@ -496,7 +508,6 @@ describe Topic do
         start = Time.now.tomorrow.beginning_of_day
         freeze_time(start)
 
-        trust_level_2 = Fabricate(:user, trust_level: 2)
         topic = Fabricate(:topic, user: trust_level_2)
 
         topic.invite(topic.user, user.username)
@@ -510,7 +521,6 @@ describe Topic do
         start = Time.now.tomorrow.beginning_of_day
         freeze_time(start)
 
-        trust_level_2 = Fabricate(:user, trust_level: 2)
         topic = Fabricate(:private_message_topic, user: trust_level_2)
 
         topic.invite(topic.user, user.username)
@@ -539,8 +549,8 @@ describe Topic do
     end
 
     describe 'private message' do
-      let(:user) { Fabricate(:user, trust_level: TrustLevel[2]) }
-      let(:topic) { Fabricate(:private_message_topic, user: user) }
+      fab!(:user) { trust_level_2 }
+      fab!(:topic) { Fabricate(:private_message_topic, user: trust_level_2) }
 
       describe 'by username' do
         it 'should be able to invite a user' do
@@ -643,18 +653,18 @@ describe Topic do
         end
 
         describe 'when topic belongs to a private category' do
-          let(:group) { Fabricate(:group) }
+          fab!(:group) { Fabricate(:group) }
 
-          let(:category) do
+          fab!(:category) do
             Fabricate(:category, groups: [group]).tap do |category|
               category.set_permissions(group => :full)
               category.save!
             end
           end
 
-          let(:topic) { Fabricate(:topic, category: category) }
+          fab!(:topic) { Fabricate(:topic, category: category) }
           let(:inviter) { Fabricate(:user).tap { |user| group.add_owner(user) } }
-          let(:invitee) { Fabricate(:user) }
+          fab!(:invitee) { Fabricate(:user) }
 
           describe 'as a group owner' do
             it 'should be able to invite a user' do
@@ -724,7 +734,7 @@ describe Topic do
 
   context 'private message' do
     let(:coding_horror) { User.find_by(username: "CodingHorror") }
-    let(:evil_trout) { Fabricate(:evil_trout) }
+    fab!(:evil_trout) { Fabricate(:evil_trout) }
     let(:topic) { Fabricate(:private_message_topic) }
 
     it "should integrate correctly" do
@@ -740,7 +750,7 @@ describe Topic do
       context 'existing user' do
 
         context 'by group name' do
-          let(:group) { Fabricate(:group) }
+          fab!(:group) { Fabricate(:group) }
 
           it 'can add admin to allowed groups' do
             admins = Group[:admins]
@@ -784,12 +794,12 @@ describe Topic do
       it "should set up actions correctly" do
         UserActionManager.enable
 
-        post = create_post(archetype: 'private_message', target_usernames: [Fabricate(:coding_horror).username])

[... diff too long, it was truncated ...]

GitHub sha: c7a0baeb