Merge branch '213740-flaky-test-ee-spec-workers-elastic_indexer_worker_spec-rb' into 'master'

Resolve "Flaky test `elastic_indexer_worker_spec.rb`"

Closes #213740

See merge request gitlab-org/gitlab!29211

ee/spec/elastic_integration/elasticsearch_model_client_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-# This module is monkey-patched in config/initializers/elastic_client_setup.rb
-describe "Monkey-patches to ::Elasticsearch::Model::Client" do
-  before do
-    stub_ee_application_setting(elasticsearch_url: ['http://localhost:9200'])
-  end
-
-  it 'uses the same client instance for all subclasses' do
-    a = Class.new { include ::Elasticsearch::Model }
-    b = Class.new { include ::Elasticsearch::Model }
-    c = Class.new(b)
-
-    expect(::Gitlab::Elastic::Client).to receive(:build).with(anything) { :fake_client }.once
-
-    # Ensure that the same client instance is used between classes and between
-    # requests
-    [a, b, c, b, c, b, a].each do |klass|
-      expect(klass.__elasticsearch__.client).to eq(:fake_client)
-    end
-  end
-end

ee/spec/services/elastic/index_record_service_spec.rb

@@ -5,6 +5,11 @@ require 'spec_helper'
 describe Elastic::IndexRecordService, :elastic do
   subject { described_class.new }
 
+  # Create admin user and search globally to avoid dealing with permissions in
+  # these tests
+  let(:user) { create(:admin) }
+  let(:search_options) { { options: { current_user: user, project_ids: :any } } }
+
   before do
     stub_ee_application_setting(elasticsearch_indexing: true)
 
@@ -27,13 +32,21 @@ describe Elastic::IndexRecordService, :elastic do
       it 'indexes new records' do
         object = create(type)
 
+        Sidekiq::Testing.disable! do
+          if type != :project
+            # You cannot find anything in the index if it's parent project is
+            # not first indexed.
+            subject.execute(object.project, true)
+          end
+        end
+
         # Prevent records from being added via bulk indexing updates
         ::Elastic::ProcessBookkeepingService.clear_tracking!
 
         expect do
           expect(subject.execute(object, true)).to eq(true)
           ensure_elasticsearch_index!
-        end.to change { Elasticsearch::Model.search('*').records.size }.by(1)
+        end.to change { object.class.elastic_search('*', search_options).total_count }.by(1)
       end
 
       it 'updates the index when object is changed' do
@@ -41,6 +54,13 @@ describe Elastic::IndexRecordService, :elastic do
 
         Sidekiq::Testing.disable! do
           object = create(type)
+
+          if type != :project
+            # You cannot find anything in the index if it's parent project is
+            # not first indexed.
+            subject.execute(object.project, true)
+          end
+
           expect(subject.execute(object, true)).to eq(true)
           object.update(attribute => "new")
         end
@@ -48,7 +68,7 @@ describe Elastic::IndexRecordService, :elastic do
         expect do
           expect(subject.execute(object, false)).to eq(true)
           ensure_elasticsearch_index!
-        end.to change { Elasticsearch::Model.search('new').records.size }.by(1)
+        end.to change { object.class.elastic_search('new', search_options).total_count }.by(1)
       end
 
       it 'ignores Elasticsearch::Transport::Transport::Errors::NotFound errors' do
@@ -74,7 +94,11 @@ describe Elastic::IndexRecordService, :elastic do
       end
 
       # Nothing should be in the index at this point
-      expect(Elasticsearch::Model.search('*').total_count).to be(0)
+      expect(Project.elastic_search('*', search_options).total_count).to be(0)
+      expect(Issue.elastic_search('*', search_options).total_count).to be(0)
+      expect(Milestone.elastic_search('*', search_options).total_count).to be(0)
+      expect(MergeRequest.elastic_search('*', search_options).total_count).to be(0)
+      expect(ProjectSnippet.elastic_search('*', search_options).total_count).to be(0)
     end
 
     it 'indexes records associated with the project' do

ee/spec/workers/elastic_indexer_worker_spec.rb

@@ -5,6 +5,11 @@ require 'spec_helper'
 describe ElasticIndexerWorker, :elastic do
   subject { described_class.new }
 
+  # Create admin user and search globally to avoid dealing with permissions in
+  # these tests
+  let(:user) { create(:admin) }
+  let(:search_options) { { options: { current_user: user, project_ids: :any } } }
+
   before do
     stub_ee_application_setting(elasticsearch_indexing: true)
 
@@ -23,12 +28,12 @@ describe ElasticIndexerWorker, :elastic do
   describe 'Indexing, updating, and deleting records' do
     using RSpec::Parameterized::TableSyntax
 
-    where(:type, :name, :attribute) do
-      :project       | "Project"      | :name
-      :issue         | "Issue"        | :title
-      :note          | "Note"         | :note
-      :milestone     | "Milestone"    | :title
-      :merge_request | "MergeRequest" | :title
+    where(:type, :name) do
+      :project       | "Project"
+      :issue         | "Issue"
+      :note          | "Note"
+      :milestone     | "Milestone"
+      :merge_request | "MergeRequest"
     end
 
     with_them do
@@ -47,6 +52,13 @@ describe ElasticIndexerWorker, :elastic do
 
         Sidekiq::Testing.disable! do
           object = create(type)
+
+          if type != :project
+            # You cannot find anything in the index if it's parent project is
+            # not first indexed.
+            subject.perform("index", "Project", object.project.id, object.project.es_id)
+          end
+
           subject.perform("index", name, object.id, object.es_id)
           ensure_elasticsearch_index!
           object.destroy
@@ -55,7 +67,7 @@ describe ElasticIndexerWorker, :elastic do
         expect do
           subject.perform("delete", name, object.id, object.es_id, { 'es_parent' => object.es_parent })
           ensure_elasticsearch_index!
-        end.to change { Elasticsearch::Model.search('*').total_count }.by(-1)
+        end.to change { object.class.elastic_search('*', search_options).total_count }.by(-1)
       end
     end
   end
@@ -84,12 +96,20 @@ describe ElasticIndexerWorker, :elastic do
     ensure_elasticsearch_index!
 
     ## All database objects + data from repository. The absolute value does not matter
-    expect(Elasticsearch::Model.search('*').total_count).to be > 40
+    expect(Project.elastic_search('*', search_options).records).to include(project)
+    expect(Issue.elastic_search('*', search_options).records).to include(issue)
+    expect(Milestone.elastic_search('*', search_options).records).to include(milestone)
+    expect(Note.elastic_search('*', search_options).records).to include(note)
+    expect(MergeRequest.elastic_search('*', search_options).records).to include(merge_request)
 
     subject.perform("delete", "Project", project.id, project.es_id)
     ensure_elasticsearch_index!
 
-    expect(Elasticsearch::Model.search('*').total_count).to be(0)
+    expect(Project.elastic_search('*', search_options).total_count).to be(0)
+    expect(Issue.elastic_search('*', search_options).total_count).to be(0)
+    expect(Milestone.elastic_search('*', search_options).total_count).to be(0)
+    expect(Note.elastic_search('*', search_options).total_count).to be(0)
+    expect(MergeRequest.elastic_search('*', search_options).total_count).to be(0)
   end
 
   it 'retries if index raises error' do