From 5d85d79fdc4443d031e039aec4067946ac7f567a Mon Sep 17 00:00:00 2001
From: Colby Swandale <996377+colby-swandale@users.noreply.github.com>
Date: Wed, 9 Oct 2024 18:04:01 +1100
Subject: [PATCH] Replace `import_and_refresh` helper with reindex factory
 trait

---
 .../api/v1/searches_controller_test.rb        | 12 +++---
 test/functional/searches_controller_test.rb   | 30 ++++++-------
 test/helpers/es_helper.rb                     |  7 ----
 test/integration/page_params_test.rb          |  9 ++--
 test/integration/search_test.rb               | 25 +++++------
 test/jobs/fastly_log_processor_job_test.rb    |  8 ++--
 .../concerns/rubygem_searchable_test.rb       | 42 +++++++++----------
 test/models/deletion_test.rb                  |  1 -
 test/models/gem_download_test.rb              | 13 +++---
 test/system/advanced_search_test.rb           |  8 +---
 test/system/autocompletes_test.rb             |  5 +--
 11 files changed, 66 insertions(+), 94 deletions(-)

diff --git a/test/functional/api/v1/searches_controller_test.rb b/test/functional/api/v1/searches_controller_test.rb
index cb70b5c1aff..4132e123d83 100644
--- a/test/functional/api/v1/searches_controller_test.rb
+++ b/test/functional/api/v1/searches_controller_test.rb
@@ -39,9 +39,8 @@ def self.should_respond_to(format)
     setup do
       @match = create(:rubygem, name: "match")
       @other = create(:rubygem, name: "other")
-      create(:version, rubygem: @match)
-      create(:version, rubygem: @other)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @match)
+      create(:version, :reindex, rubygem: @other)
     end
 
     should_respond_to(:json) do |body|
@@ -79,10 +78,9 @@ def self.should_respond_to(format)
       @match1 = create(:rubygem, name: "match1")
       @match2 = create(:rubygem, name: "match2")
       @other = create(:rubygem, name: "other")
-      create(:version, rubygem: @match1)
-      create(:version, rubygem: @match2)
-      create(:version, rubygem: @other)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @match1)
+      create(:version, :reindex, rubygem: @match2)
+      create(:version, :reindex, rubygem: @other)
     end
 
     context "with elasticsearch up" do
diff --git a/test/functional/searches_controller_test.rb b/test/functional/searches_controller_test.rb
index 050f11b5236..9420a9d486c 100644
--- a/test/functional/searches_controller_test.rb
+++ b/test/functional/searches_controller_test.rb
@@ -15,8 +15,7 @@ class SearchesControllerTest < ActionController::TestCase
 
   context "on GET to show with search parameters for a rubygem without versions" do
     setup do
-      @sinatra = create(:rubygem, name: "sinatra")
-      import_and_refresh
+      @sinatra = create(:rubygem, :reindex, name: "sinatra")
 
       assert_nil @sinatra.most_recent_version
       assert_predicate @sinatra.reload.versions.count, :zero?
@@ -35,10 +34,10 @@ class SearchesControllerTest < ActionController::TestCase
       @sinatra = create(:rubygem, name: "sinatra")
       @sinatra_redux = create(:rubygem, name: "sinatra-redux")
       @brando = create(:rubygem, name: "brando")
-      create(:version, rubygem: @sinatra)
-      create(:version, rubygem: @sinatra_redux)
-      create(:version, rubygem: @brando)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @sinatra)
+      create(:version, :reindex, rubygem: @sinatra_redux)
+      create(:version, :reindex, rubygem: @brando)
+
       get :show, params: { query: "sinatra" }
     end
 
@@ -61,10 +60,9 @@ class SearchesControllerTest < ActionController::TestCase
       @sinatra = create(:rubygem, name: "sinatra")
       @sinatra_redux = create(:rubygem, name: "sinatra-redux")
       @brando = create(:rubygem, name: "brando")
-      create(:version, rubygem: @sinatra)
-      create(:version, rubygem: @sinatra_redux)
-      create(:version, rubygem: @brando)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @sinatra)
+      create(:version, :reindex, rubygem: @sinatra_redux)
+      create(:version, :reindex, rubygem: @brando)
       get :show, params: { query: "sinatra" }
     end
 
@@ -103,10 +101,9 @@ class SearchesControllerTest < ActionController::TestCase
       @sinatra = create(:rubygem, name: "sinatra")
       @sinatra_redux = create(:rubygem, name: "sinatra-redux")
       @brando = create(:rubygem, name: "brando")
-      create(:version, rubygem: @sinatra)
-      create(:version, rubygem: @sinatra_redux)
-      create(:version, rubygem: @brando)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @sinatra)
+      create(:version, :reindex, rubygem: @sinatra_redux)
+      create(:version, :reindex, rubygem: @brando)
       get :show, params: { query: "sinatre" }
     end
 
@@ -132,9 +129,8 @@ class SearchesControllerTest < ActionController::TestCase
     setup do
       @sinatra = create(:rubygem, name: "sinatra")
       @sinatra_redux = create(:rubygem, name: "sinatra-redux")
-      create(:version, rubygem: @sinatra)
-      create(:version, :yanked, rubygem: @sinatra_redux)
-      import_and_refresh
+      create(:version, :reindex, rubygem: @sinatra)
+      create(:version, :yanked, :reindex, rubygem: @sinatra_redux)
       get :show, params: { query: @sinatra_redux.name.to_s, yanked: true }
     end
 
diff --git a/test/helpers/es_helper.rb b/test/helpers/es_helper.rb
index 47552fd0548..3c2032ee898 100644
--- a/test/helpers/es_helper.rb
+++ b/test/helpers/es_helper.rb
@@ -1,11 +1,4 @@
 module SearchKickHelper
-  def import_and_refresh
-    Rubygem.searchkick_reindex
-
-    # wait for indexing to finish
-    Searchkick.client.cluster.health wait_for_status: "yellow"
-  end
-
   def es_downloads(id)
     response = get_response(id)
     response["_source"]["downloads"]
diff --git a/test/integration/page_params_test.rb b/test/integration/page_params_test.rb
index 95260aad9cd..47c048b9e6f 100644
--- a/test/integration/page_params_test.rb
+++ b/test/integration/page_params_test.rb
@@ -36,8 +36,7 @@ class PageParamsTest < SystemTest
   end
 
   test "api search with page smaller than 1" do
-    create(:rubygem, name: "some", number: "1.0.0")
-    import_and_refresh
+    create(:rubygem, :reindex, name: "some", number: "1.0.0")
     visit api_v1_search_path(page: "0", query: "some", format: :json)
 
     assert redirect_to(api_v1_search_path(page: "1", query: "some", format: :json))
@@ -45,8 +44,7 @@ class PageParamsTest < SystemTest
   end
 
   test "api search with page is not a numer" do
-    create(:rubygem, name: "some", number: "1.0.0")
-    import_and_refresh
+    create(:rubygem, :reindex, name: "some", number: "1.0.0")
     visit api_v1_search_path(page: "foo", query: "some", format: :json)
 
     assert redirect_to(api_v1_search_path(page: "1", query: "some", format: :json))
@@ -54,8 +52,7 @@ class PageParamsTest < SystemTest
   end
 
   test "api search with page that can't be converted to a number" do
-    create(:rubygem, name: "some", number: "1.0.0")
-    import_and_refresh
+    create(:rubygem, :reindex, name: "some", number: "1.0.0")
     visit api_v1_search_path(page: { "$acunetix" => "1" }, query: "some", format: :json)
 
     assert redirect_to(api_v1_search_path(page: "1", query: "some", format: :json))
diff --git a/test/integration/search_test.rb b/test/integration/search_test.rb
index ad5c2c91b6f..bc0c39fed95 100644
--- a/test/integration/search_test.rb
+++ b/test/integration/search_test.rb
@@ -4,9 +4,8 @@ class SearchTest < SystemTest
   include SearchKickHelper
 
   test "searching for a gem" do
-    create(:rubygem, name: "LDAP", number: "1.0.0")
-    create(:rubygem, name: "LDAP-PLUS", number: "1.0.0")
-    import_and_refresh
+    create(:rubygem, :reindex, name: "LDAP", number: "1.0.0")
+    create(:rubygem, :reindex, name: "LDAP-PLUS", number: "1.0.0")
 
     visit search_path
 
@@ -20,8 +19,7 @@ class SearchTest < SystemTest
 
   test "searching for a yanked gem" do
     rubygem = create(:rubygem, name: "LDAP")
-    create(:version, rubygem: rubygem, indexed: false)
-    import_and_refresh
+    create(:version, :reindex, rubygem: rubygem, indexed: false)
 
     visit search_path
 
@@ -39,9 +37,8 @@ class SearchTest < SystemTest
 
   test "searching for a gem with yanked versions" do
     rubygem = create(:rubygem, name: "LDAP")
-    create(:version, rubygem: rubygem, number: "1.1.1", indexed: true)
-    create(:version, rubygem: rubygem, number: "2.2.2", indexed: false)
-    import_and_refresh
+    create(:version, :reindex, rubygem: rubygem, number: "1.1.1", indexed: true)
+    create(:version, :reindex, rubygem: rubygem, number: "2.2.2", indexed: false)
 
     visit search_path
 
@@ -60,9 +57,8 @@ class SearchTest < SystemTest
 
   test "params has non white listed keys" do
     Kaminari.configure { |c| c.default_per_page = 1 }
-    create(:rubygem, name: "ruby-ruby", number: "1.0.0")
-    create(:rubygem, name: "ruby-gems", number: "1.0.0")
-    import_and_refresh
+    create(:rubygem, :reindex, name: "ruby-ruby", number: "1.0.0")
+    create(:rubygem, :reindex, name: "ruby-gems", number: "1.0.0")
 
     visit "/search?query=ruby&original_script_name=javascript:alert(1)//&script_name=javascript:alert(1)//"
 
@@ -77,10 +73,9 @@ class SearchTest < SystemTest
       orignal_val = Gemcutter::SEARCH_MAX_PAGES
       Gemcutter::SEARCH_MAX_PAGES = 2
 
-      create(:rubygem, name: "ruby-ruby", number: "1.0.0")
-      create(:rubygem, name: "ruby-gems", number: "1.0.0")
-      create(:rubygem, name: "ruby-thing", number: "1.0.0")
-      import_and_refresh
+      create(:rubygem, :reindex, name: "ruby-ruby", number: "1.0.0")
+      create(:rubygem, :reindex, name: "ruby-gems", number: "1.0.0")
+      create(:rubygem, :reindex, name: "ruby-thing", number: "1.0.0")
 
       visit "/search?query=ruby"
 
diff --git a/test/jobs/fastly_log_processor_job_test.rb b/test/jobs/fastly_log_processor_job_test.rb
index ed5d46b048f..e240840925c 100644
--- a/test/jobs/fastly_log_processor_job_test.rb
+++ b/test/jobs/fastly_log_processor_job_test.rb
@@ -21,7 +21,6 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
     @processor = FastlyLogProcessor.new("test-bucket", "fastly-fake.log")
     @job = FastlyLogProcessorJob.new(bucket: "test-bucket", key: "fastly-fake.log")
     create(:gem_download)
-    import_and_refresh
   end
 
   teardown do
@@ -58,8 +57,6 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
       create(:version, rubygem: json, number: "1.8.3", platform: "java")
       create(:version, rubygem: json, number: "1.8.3")
       create(:version, rubygem: json, number: "1.8.2")
-
-      import_and_refresh
     end
 
     context "#perform" do
@@ -69,6 +66,8 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
         assert_equal 0, GemDownload.count_for_rubygem(json.id)
         3.times { @job.perform_now }
 
+        json.reindex(refresh: true)
+
         assert_equal 7, es_downloads(json.id)
         assert_equal 7, GemDownload.count_for_rubygem(json.id)
       end
@@ -85,6 +84,7 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
         end
 
         json = Rubygem.find_by_name("json")
+        json.reindex(refresh: true)
 
         assert_equal 7, GemDownload.count_for_rubygem(json.id)
         assert_equal 7, es_downloads(json.id)
@@ -93,6 +93,7 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
 
       should "not run if already processed" do
         json = Rubygem.find_by_name("json")
+        json.reindex(refresh: true)
 
         assert_equal 0, json.downloads
         assert_equal 0, es_downloads(json.id)
@@ -119,6 +120,7 @@ class FastlyLogProcessorJobTest < ActiveJob::TestCase
 
         @job.perform_now
         json = Rubygem.find_by_name("json")
+        json.reindex(refresh: true)
 
         assert_equal 0, json.downloads
         assert_equal 0, es_downloads(json.id)
diff --git a/test/models/concerns/rubygem_searchable_test.rb b/test/models/concerns/rubygem_searchable_test.rb
index 9271ccee4d1..f2b64d40609 100644
--- a/test/models/concerns/rubygem_searchable_test.rb
+++ b/test/models/concerns/rubygem_searchable_test.rb
@@ -25,6 +25,8 @@ class RubygemSearchableTest < ActiveSupport::TestCase
       run_gem = create(:rubygem, name: "example_gem_run_dep")
       @dev_dep = create(:dependency, :development, rubygem: dev_gem, version: version)
       @run_dep = create(:dependency, :runtime, rubygem: run_gem, version: version)
+
+      @rubygem.reindex(refresh: true)
     end
 
     should "return a hash" do
@@ -107,8 +109,7 @@ class RubygemSearchableTest < ActiveSupport::TestCase
     context "when the number of downloads exceeds a 32 bit integer" do
       setup do
         @rubygem = create(:rubygem, name: "large_downloads_example_gem", downloads: 10_000_000_000) # 10 Billion downloads
-        @version = create(:version, number: "1.0.0", rubygem: @rubygem)
-        import_and_refresh
+        @version = create(:version, :reindex, number: "1.0.0", rubygem: @rubygem)
       end
 
       should "allow the number of downloads to be stored as a 64 bit integer" do
@@ -121,10 +122,9 @@ class RubygemSearchableTest < ActiveSupport::TestCase
 
   context "rubygems analyzer" do
     setup do
-      create(:rubygem, name: "example-gem", number: "0.0.1")
-      create(:rubygem, name: "example_1", number: "0.0.1")
-      create(:rubygem, name: "example.rb", number: "0.0.1")
-
+      create(:rubygem, :reindex, name: "example-gem", number: "0.0.1")
+      create(:rubygem, :reindex, name: "example_1", number: "0.0.1")
+      create(:rubygem, :reindex, name: "example.rb", number: "0.0.1")
     end
 
     should "find all gems with matching tokens" do
@@ -141,9 +141,8 @@ class RubygemSearchableTest < ActiveSupport::TestCase
     setup do
       example_1 = create(:rubygem, name: "example_1")
       example_2 = create(:rubygem, name: "example_2")
-      create(:version, rubygem: example_1, indexed: false)
-      create(:version, rubygem: example_2)
-
+      create(:version, :reindex, rubygem: example_1, indexed: false)
+      create(:version, :reindex, rubygem: example_2)
     end
 
     should "filter yanked gems from the result" do
@@ -160,10 +159,9 @@ class RubygemSearchableTest < ActiveSupport::TestCase
       example_gem1 = create(:rubygem, name: "keyword", downloads: 1)
       example_gem2 = create(:rubygem, name: "example_gem2", downloads: 1)
       example_gem3 = create(:rubygem, name: "example_gem3", downloads: 1)
-      create(:version, rubygem: example_gem1, description: "some", summary: "some")
-      create(:version, rubygem: example_gem2, description: "keyword", summary: "some")
-      create(:version, rubygem: example_gem3, summary: "keyword", description: "some")
-
+      create(:version, :reindex, rubygem: example_gem1, description: "some", summary: "some")
+      create(:version, :reindex, rubygem: example_gem2, description: "keyword", summary: "some")
+      create(:version, :reindex, rubygem: example_gem3, summary: "keyword", description: "some")
     end
 
     should "look for keyword in name, summary and description and order them in same priority order" do
@@ -178,9 +176,8 @@ class RubygemSearchableTest < ActiveSupport::TestCase
     setup do
       (10..30).step(10) do |downloads|
         rubygem = create(:rubygem, name: "gem_#{downloads}", downloads: downloads)
-        create(:version, rubygem: rubygem)
+        create(:version, :reindex, rubygem: rubygem)
       end
-
     end
 
     should "boost score of result by downloads count" do
@@ -219,8 +216,7 @@ class RubygemSearchableTest < ActiveSupport::TestCase
       example1 = create(:rubygem, name: "keyword")
       example2 = create(:rubygem, name: "keywordo")
       example3 = create(:rubygem, name: "keywo")
-      [example1, example2, example3].each { |gem| create(:version, rubygem: gem) }
-
+      [example1, example2, example3].each { |gem| create(:version, :reindex, rubygem: gem) }
     end
 
     should "suggest names of possible gems" do
@@ -242,9 +238,8 @@ class RubygemSearchableTest < ActiveSupport::TestCase
     setup do
       rubygem1 = create(:rubygem, name: "example", downloads: 101)
       rubygem2 = create(:rubygem, name: "web-rubygem", downloads: 99)
-      create(:version, rubygem: rubygem1, summary: "special word with web-rubygem")
-      create(:version, rubygem: rubygem2, description: "example special word")
-
+      create(:version, :reindex, rubygem: rubygem1, summary: "special word with web-rubygem")
+      create(:version, :reindex, rubygem: rubygem2, description: "example special word")
     end
 
     should "filter gems on downloads" do
@@ -299,6 +294,9 @@ class RubygemSearchableTest < ActiveSupport::TestCase
       rubygem1.update_column("updated_at", 2.days.ago)
       rubygem2.update_column("updated_at", 10.days.ago)
 
+      rubygem1.reindex(refresh: true)
+      rubygem2.reindex(refresh: true)
+
       _, @response = ElasticSearcher.new("example").search
     end
 
@@ -357,7 +355,6 @@ class RubygemSearchableTest < ActiveSupport::TestCase
           rubygem = create(:rubygem, name: gem_name, downloads: 10)
           create(:version, rubygem: rubygem)
         end
-
       end
 
       should "not affect results" do
@@ -372,9 +369,8 @@ class RubygemSearchableTest < ActiveSupport::TestCase
   context "query matches gem name prefix" do
     setup do
       %w[term-ansicolor term-an].each do |gem_name|
-        create(:rubygem, name: gem_name, number: "0.0.1", downloads: 10)
+        create(:rubygem, :reindex, name: gem_name, number: "0.0.1", downloads: 10)
       end
-
     end
 
     should "return results" do
diff --git a/test/models/deletion_test.rb b/test/models/deletion_test.rb
index 574275a2076..12e585b54dc 100644
--- a/test/models/deletion_test.rb
+++ b/test/models/deletion_test.rb
@@ -12,7 +12,6 @@ class DeletionTest < ActiveSupport::TestCase
     @gem_file.rewind
     @version = Version.last
     @spec_rz = RubygemFs.instance.get("quick/Marshal.4.8/#{@version.full_name}.gemspec.rz")
-    import_and_refresh
   end
 
   teardown do
diff --git a/test/models/gem_download_test.rb b/test/models/gem_download_test.rb
index fa52869b656..5c49865ea18 100644
--- a/test/models/gem_download_test.rb
+++ b/test/models/gem_download_test.rb
@@ -5,7 +5,6 @@ class GemDownloadTest < ActiveSupport::TestCase
 
   setup do
     create(:gem_download, count: 0)
-    import_and_refresh
   end
 
   context ".increment" do
@@ -64,12 +63,13 @@ def test_update_the_count_atomically
         @counts   = Array.new(3) { rand(100) }
         @data     = @versions.map.with_index { |v, i| [v.full_name, @counts[i]] }
         @gem_downloads = [(@counts[0] + @counts[2]), @counts[1]]
-        import_and_refresh
       end
 
       should "write the proper values" do
         GemDownload.bulk_update(@data)
 
+        @gems.each { |g| g.reindex(refresh: true) }
+
         3.times do |i|
           assert_equal @counts[i], GemDownload.count_for_version(@versions[i].id)
         end
@@ -104,8 +104,9 @@ def test_update_the_count_atomically
             @gem_downloads[i] += counts[i]
           end
         end
-        import_and_refresh
         GemDownload.bulk_update(data)
+
+        @gems.each { |g| g.reindex(refresh: true) }
       end
 
       should "update rubygems downloads irrespective of rubygem_ids order" do
@@ -127,10 +128,11 @@ def test_update_the_count_atomically
     context "with prerelease versions" do
       setup do
         @rubygem = create(:rubygem, number: "0.0.1.rc")
-        import_and_refresh
         most_recent_version = @rubygem.most_recent_version
         version_downloads = [most_recent_version.full_name, 40]
         GemDownload.bulk_update([version_downloads])
+
+        @rubygem.reindex(refresh: true)
       end
 
       should "set version_downloads of ES record with prerelease downloads" do
@@ -141,9 +143,10 @@ def test_update_the_count_atomically
     context "with no ruby platform versions" do
       setup do
         @version = create(:version, platform: "java")
-        import_and_refresh
         version_downloads = [@version.full_name, 40]
         GemDownload.bulk_update([version_downloads])
+
+        @version.rubygem.reindex(refresh: true)
       end
 
       should "set version_downloads of ES record with platform downloads" do
diff --git a/test/system/advanced_search_test.rb b/test/system/advanced_search_test.rb
index 95775a32a6f..73705d4cb19 100644
--- a/test/system/advanced_search_test.rb
+++ b/test/system/advanced_search_test.rb
@@ -9,9 +9,7 @@ class AdvancedSearchTest < ApplicationSystemTestCase
 
   test "searches for a gem while scoping advanced attributes" do
     rubygem = create(:rubygem, name: "LDAP", number: "1.0.0", downloads: 3)
-    create(:version, summary: "some summary", description: "Hello World", rubygem: rubygem)
-
-    import_and_refresh
+    create(:version, :reindex, summary: "some summary", description: "Hello World", rubygem: rubygem)
 
     fill_in "query", with: "downloads: <5"
     click_button "advanced_search_submit"
@@ -21,8 +19,6 @@ class AdvancedSearchTest < ApplicationSystemTestCase
   end
 
   test "enter inside any field will submit form" do
-    import_and_refresh
-
     ["#name", "#summary", "#description", "#downloads", "#updated"].each do |el|
       visit advanced_search_path
       find(el).send_keys(:return)
@@ -32,8 +28,6 @@ class AdvancedSearchTest < ApplicationSystemTestCase
   end
 
   test "forms search query out of advanced attributes" do
-    import_and_refresh
-
     fill_in "name", with: "hello"
     fill_in "summary", with: "world"
     fill_in "description", with: "foo"
diff --git a/test/system/autocompletes_test.rb b/test/system/autocompletes_test.rb
index 5fa88a50fe0..d152d01f24f 100644
--- a/test/system/autocompletes_test.rb
+++ b/test/system/autocompletes_test.rb
@@ -5,10 +5,9 @@ class AutocompletesTest < ApplicationSystemTestCase
 
   setup do
     rubygem = create(:rubygem, name: "rubocop")
-    create(:version, rubygem: rubygem, indexed: true)
+    create(:version, :reindex, rubygem: rubygem, indexed: true)
     rubygem = create(:rubygem, name: "rubocop-performance")
-    create(:version, rubygem: rubygem, indexed: true)
-    import_and_refresh
+    create(:version, :reindex, rubygem: rubygem, indexed: true)
 
     visit root_path
     @fill_field = find_by_id "query"