Refactor: Remove memoization from spec test (#1168)

Removed Memoization (let, before, etc.) from specs: - spec/helpers/url_helpers.rb - spec/integrations/feed_importing_spec.rb - spec/models/story_spec.rb - spec/requests/feeds_controller_spec.rb - spec/requests/imports_controller_spec.rb --------- Co-authored-by: Rob Young <[email protected]>
stringer-rss · Mar 24, 2024 · 1af7783 · 1af7783
1 parent 5ba729c
commit 1af7783
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 40 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -133,6 +133,7 @@ RSpec/ExampleLength:
     - 'spec/commands/fever_api/read_feeds_groups_spec.rb'
     - 'spec/commands/fever_api/read_items_spec.rb'
     - 'spec/helpers/url_helpers_spec.rb'
+    - 'spec/integration/feed_importing_spec.rb'
     - 'spec/models/feed_spec.rb'
     - 'spec/models/migration_status_spec.rb'
     - 'spec/models/story_spec.rb'

diff --git a/spec/helpers/url_helpers_spec.rb b/spec/helpers/url_helpers_spec.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe UrlHelpers do
-  let(:helper) do
+  def helper
     helper_class = Class.new { include UrlHelpers }
     helper_class.new
   end
@@ -41,7 +41,7 @@
       content = <<~HTML
         <div>
         <img foo="bar">
-        <a name="something"/></a>
+        <a name="something"></a>
         <video foo="bar"></video>
         </div>
       HTML

diff --git a/spec/integration/feed_importing_spec.rb b/spec/integration/feed_importing_spec.rb
@@ -3,49 +3,53 @@
 require "support/feed_server"
 
 RSpec.describe "Feed importing" do
-  let(:server) { FeedServer.new }
-  let(:feed) do
+  def create_server(url: "feed01_valid_feed/feed.xml")
+    server = FeedServer.new
+    server.response = sample_data(url)
+    server
+  end
+
+  def create_feed(**options)
     create(
       :feed,
       name: "Example feed",
       last_fetched: Time.zone.local(2014, 1, 1),
-      url: server.url
+      **options
     )
   end
 
   describe "Valid feed" do
-    before do
-      # articles older than 3 days are ignored, so freeze time within
-      # applicable range of the stories in the sample feed
-      travel_to(Time.parse("2014-08-15T17:30:00Z"))
-    end
+    # articles older than 3 days are ignored, so freeze time within
+    # applicable range of the stories in the sample feed
+    # travel_to(Time.parse("2014-08-15T17:30:00Z"))
 
     describe "Importing for the first time" do
       it "imports all entries" do
-        server.response = sample_data("feeds/feed01_valid_feed/feed.xml")
+        travel_to(Time.parse("2014-08-15T17:30:00Z"))
+        feed = create_feed(url: create_server.url)
         expect { Feed::FetchOne.call(feed) }
           .to change(feed.stories, :count).to(5)
       end
     end
 
     describe "Importing for the second time" do
-      before do
-        server.response = sample_data("feeds/feed01_valid_feed/feed.xml")
-        Feed::FetchOne.call(feed)
-      end
-
       context "no new entries" do
         it "does not create new stories" do
-          server.response = sample_data("feeds/feed01_valid_feed/feed.xml")
+          travel_to(Time.parse("2014-08-15T17:30:00Z"))
+          feed = create_feed(url: create_server.url)
+          Feed::FetchOne.call(feed)
           expect { Feed::FetchOne.call(feed) }
             .not_to change(feed.stories, :count)
         end
       end
 
       context "new entries" do
         it "creates new stories" do
-          server.response =
-            sample_data("feeds/feed01_valid_feed/feed_updated.xml")
+          travel_to(Time.parse("2014-08-15T17:30:00Z"))
+          server = create_server
+          feed = create_feed(url: server.url)
+          Feed::FetchOne.call(feed)
+          server.response = sample_data("feed01_valid_feed/feed_updated.xml")
           expect { Feed::FetchOne.call(feed) }
             .to change(feed.stories, :count).by(1)
         end
@@ -54,9 +58,10 @@
   end
 
   describe "Feed with incorrect pubdates" do
-    before { travel_to(Time.parse("2014-08-12T17:30:00Z")) }
-
     context "has been fetched before" do
+      url = "feed02_invalid_published_dates/feed.xml"
+      last_fetched = Time.parse("2014-08-12T00:01:00Z")
+
       it "imports all new stories" do
         # This spec describes a scenario where the feed is reporting incorrect
         # published dates for stories. The feed in question is
@@ -67,9 +72,9 @@
         # last time this feed was fetched is after 00:00 the day the article
         # was published.
 
-        feed.last_fetched = Time.parse("2014-08-12T00:01:00Z")
-        server.response =
-          sample_data("feeds/feed02_invalid_published_dates/feed.xml")
+        travel_to(Time.parse("2014-08-12T17:30:00Z"))
+        server = create_server(url:)
+        feed = create_feed(url: server.url, last_fetched:)
 
         expect { Feed::FetchOne.call(feed) }
           .to change(feed.stories, :count).by(1)
@@ -79,5 +84,5 @@
 end
 
 def sample_data(path)
-  File.new(File.join("spec", "sample_data", path)).read
+  File.new(File.join("spec", "sample_data", "feeds", path)).read
 end
diff --git a/spec/models/story_spec.rb b/spec/models/story_spec.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 RSpec.describe "Story" do
-  let(:story) { build_stubbed(:story) }
-
   describe ".unread" do
     it "returns stories where is_read is false" do
       story = create(:story, :unread)
@@ -25,11 +23,13 @@
     end
 
     it "uses a fallback string if story has no title" do
+      story = build_stubbed(:story)
       story.title = nil
       expect(story.headline).to eq(Story::UNTITLED)
     end
 
     it "strips html out" do
+      story = build_stubbed(:story)
       story.title = "<b>Super cool</b> stuff"
       expect(story.headline).to eq("Super cool stuff")
     end
@@ -43,17 +43,18 @@
     end
 
     it "strips html out" do
+      story = build_stubbed(:story)
       story.body = "<a href='http://github.com'>Yo</a> dawg"
       expect(story.lead).to eq("Yo dawg")
     end
   end
 
   describe "#source" do
-    let(:feed) { Feed.new(name: "Superfeed") }
-
-    before { story.feed = feed }
-
     it "returns the feeds name" do
+      story = build_stubbed(:story)
+      feed = Feed.new(name: "Superfeed")
+      story.feed = feed
+
       expect(story.source).to eq(feed.name)
     end
   end

diff --git a/spec/requests/feeds_controller_spec.rb b/spec/requests/feeds_controller_spec.rb
@@ -103,7 +103,7 @@ def params(feed, **overrides)
 
   describe "#create" do
     context "when the feed url is valid" do
-      let(:feed_url) { "http://example.com/" }
+      feed_url = "http://example.com/"
 
       it "adds the feed and queues it to be fetched" do
         login_as(default_user)
@@ -123,7 +123,7 @@ def params(feed, **overrides)
     end
 
     context "when the feed url is invalid" do
-      let(:feed_url) { "http://not-a-valid-feed.com/" }
+      feed_url = "http://not-a-valid-feed.com/"
 
       it "does not add the feed" do
         login_as(default_user)
@@ -136,7 +136,7 @@ def params(feed, **overrides)
     end
 
     context "when the feed url is one we already subscribe to" do
-      let(:feed_url) { "http://example.com/" }
+      feed_url = "http://example.com/"
 
       it "does not add the feed" do
         login_as(default_user)

diff --git a/spec/requests/imports_controller_spec.rb b/spec/requests/imports_controller_spec.rb
@@ -12,12 +12,10 @@
   end
 
   describe "POST /feeds/import" do
-    let(:opml_file) do
-      Rack::Test::UploadedFile.new(
-        "spec/sample_data/subscriptions.xml",
-        "application/xml"
-      )
-    end
+    opml_file = Rack::Test::UploadedFile.new(
+      "spec/sample_data/subscriptions.xml",
+      "application/xml"
+    )
 
     it "parses OPML and starts fetching" do
       expect(Feed::ImportFromOpml).to receive(:call).once