From 9e2aff874fc84f6eed48f7eaac1f5a23b5eeec67 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Tue, 25 Oct 2022 22:21:23 -0500
Subject: [PATCH] tests: fix strategy_should_work to not perform API calls
 outside of tests.

Fix strategy_should_work to not perform API calls outside of `should`
blocks. This could cause the whole test suite to crash if a source test
raised an unexpected exception.
---
 test/test_helpers/source_test_helper.rb | 41 +++++++++++++++----------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/test/test_helpers/source_test_helper.rb b/test/test_helpers/source_test_helper.rb
index 33c9127d0..c5997567f 100644
--- a/test/test_helpers/source_test_helper.rb
+++ b/test/test_helpers/source_test_helper.rb
@@ -9,46 +9,48 @@ module SourceTestHelper
     # XXX: can't use **kwargs because of a bug with shoulda-context
     referer, download_size, deleted = [:referer, :download_size, :deleted].map { |arg| arguments.delete(arg) }
 
-    strategy = Source::Extractor.find(url, referer)
-
     should "not raise anything" do
+      strategy = Source::Extractor.find(url, referer)
       assert_nothing_raised { strategy.to_h }
     end
 
     should "make sure that image_urls is an array of valid elements" do
+      strategy = Source::Extractor.find(url, referer)
       assert((strategy.image_urls.instance_of? Array))
       assert_not(strategy.image_urls.include?(nil))
     end
 
-    should_download_successfully(strategy, download_size) unless deleted || strategy.image_urls.empty?
+    should_download_successfully(url, referer, download_size) unless deleted
 
     # {profile_url: nil}[:profile_url].present? -> false
     # Doing it this way instead we can check profile_url even if it's passed as a nil.
     if arguments.include? :profile_url
       profile_url = arguments.delete(:profile_url)
-      should_handle_artists_correctly(strategy, profile_url)
+      should_handle_artists_correctly(url, referer, profile_url)
     end
 
     tags = arguments.delete(:tags)
-    should_validate_tags(strategy, tags)
+    should_validate_tags(url, referer, tags)
 
-    should_match_source_data(strategy, arguments)
+    should_match_source_data(url, referer, arguments)
   end
 
-  def should_download_successfully(strategy, download_size = nil) # XXX perhaps refactor into should_upload_successfully in order to streamline the source tests further?
+  def should_download_successfully(url, referer, download_size = nil) # XXX perhaps refactor into should_upload_successfully in order to streamline the source tests further?
     should "download successfully" do
-      file = strategy.download_file!(strategy.image_urls.first)
-      if download_size.present?
+      strategy = Source::Extractor.find(url, referer)
+
+      if download_size.present? && strategy.image_urls.present?
+        file = strategy.download_file!(strategy.image_urls.first)
         assert_equal(download_size, file.size)
-      else
-        assert_not_nil(file.size)
       end
     end
   end
 
-  def should_handle_artists_correctly(strategy, profile_url)
+  def should_handle_artists_correctly(url, referer, profile_url)
     if profile_url.present?
       should "correctly match a strategy to an artist with the same profile url" do
+        strategy = Source::Extractor.find(url, referer)
+
         assert_not_nil(Danbooru::URL.parse(strategy.profile_url))
         assert_equal(profile_url, strategy.profile_url)
         artist = FactoryBot.create(:artist, name: strategy.tag_name, url_string: profile_url)
@@ -56,6 +58,8 @@ module SourceTestHelper
       end
     else
       should "not incorrectly extract a profile url or artist data when there's none to be found" do
+        strategy = Source::Extractor.find(url, referer)
+
         assert_nil(strategy.profile_url.presence)
         assert_nil(strategy.artist_name.presence)
         assert_equal([], strategy.other_names)
@@ -63,8 +67,10 @@ module SourceTestHelper
     end
   end
 
-  def should_validate_tags(strategy, tags = nil)
+  def should_validate_tags(url, referer, tags = nil)
     should "make sure that tags return an array of arrays" do
+      strategy = Source::Extractor.find(url, referer)
+
       assert((strategy.tags.instance_of? Array))
       if strategy.tags.present?
         assert((strategy.tags.first.instance_of? Array))
@@ -74,6 +80,8 @@ module SourceTestHelper
     return unless tags.present?
 
     should "make sure that tags match" do
+      strategy = Source::Extractor.find(url, referer)
+
       if tags&.first.instance_of? Array
         assert_equal(tags.sort, strategy.tags.sort)
       elsif tags&.first.instance_of? String
@@ -82,13 +90,14 @@ module SourceTestHelper
     end
   end
 
-  def should_match_source_data(strategy, methods_to_test)
+  def should_match_source_data(url, referer, methods_to_test)
     # check any method that is passed as kwargs, in order to hardcode as few things as possible
     # XXX can't use **kwargs because of a bug with shoulda-context, so we're using a hash temporarily
     methods_to_test.each do |method_name, expected_value|
-      actual_value = strategy.public_send(method_name)
-
       should "make sure that '#{method_name}' matches" do
+        strategy = Source::Extractor.find(url, referer)
+        actual_value = strategy.public_send(method_name)
+
         if expected_value.instance_of? Regexp
           assert_match(expected_value, actual_value)
         elsif expected_value.instance_of? Array