Commit 6f58e18a authored by Dylan Griffith's avatar Dylan Griffith Committed by Mark Chao

Fix false positive test global_search_spec

The tests were missing `sidekiq_inline` so basically they data was never
getting into the index and leading to false positives in the N+1 tests
as no results were being found. Also other tests were failing because
the search crieria didn't match or another bug that was found at
https://gitlab.com/gitlab-org/gitlab/issues/103458

We try to avoid this in future by asserting we at least see search
results.
parent 56e03fdd
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe 'Global elastic search', :elastic do describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) } let(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) }
...@@ -15,21 +15,25 @@ describe 'Global elastic search', :elastic do ...@@ -15,21 +15,25 @@ describe 'Global elastic search', :elastic do
shared_examples 'an efficient database result' do shared_examples 'an efficient database result' do
it 'avoids N+1 database queries' do it 'avoids N+1 database queries' do
create(object, creation_args) create(object, *creation_traits, creation_args)
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
control_count = ActiveRecord::QueryRecorder.new { visit path }.count control_count = ActiveRecord::QueryRecorder.new { visit path }.count
expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives
create_list(object, 10, creation_args) create_list(object, 10, *creation_traits, creation_args)
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
control_count = control_count + (10 * query_count_multiplier) + 1 control_count = control_count + (10 * query_count_multiplier) + 1
expect { visit path }.not_to exceed_query_limit(control_count) expect { visit path }.not_to exceed_query_limit(control_count)
expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives
end end
end end
describe 'I do not overload the database' do describe 'I do not overload the database' do
let(:creation_traits) { [] }
context 'searching issues' do context 'searching issues' do
let(:object) { :issue } let(:object) { :issue }
let(:creation_args) { { project: project, title: 'initial' } } let(:creation_args) { { project: project, title: 'initial' } }
...@@ -53,8 +57,9 @@ describe 'Global elastic search', :elastic do ...@@ -53,8 +57,9 @@ describe 'Global elastic search', :elastic do
context 'searching merge requests' do context 'searching merge requests' do
let(:object) { :merge_request } let(:object) { :merge_request }
let(:creation_args) { { title: 'initial' } } let(:creation_traits) { [:sequence_source_branch] }
let(:path) { search_path(search: 'initial*', scope: 'merge_requests') } let(:creation_args) { { source_project: project, title: 'initial' } }
let(:path) { search_path(search: '*', scope: 'merge_requests') }
let(:query_count_multiplier) { 0 } let(:query_count_multiplier) { 0 }
it_behaves_like 'an efficient database result' it_behaves_like 'an efficient database result'
...@@ -63,7 +68,7 @@ describe 'Global elastic search', :elastic do ...@@ -63,7 +68,7 @@ describe 'Global elastic search', :elastic do
context 'searching milestones' do context 'searching milestones' do
let(:object) { :milestone } let(:object) { :milestone }
let(:creation_args) { { project: project } } let(:creation_args) { { project: project } }
let(:path) { search_path(search: 'milestone*', scope: 'milestones') } let(:path) { search_path(search: '*', scope: 'milestones') }
let(:query_count_multiplier) { 0 } let(:query_count_multiplier) { 0 }
it_behaves_like 'an efficient database result' it_behaves_like 'an efficient database result'
...@@ -77,7 +82,7 @@ describe 'Global elastic search', :elastic do ...@@ -77,7 +82,7 @@ describe 'Global elastic search', :elastic do
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
end end
it "has a pagination", :sidekiq_might_not_need_inline do it "has a pagination" do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('initial') submit_search('initial')
...@@ -95,7 +100,7 @@ describe 'Global elastic search', :elastic do ...@@ -95,7 +100,7 @@ describe 'Global elastic search', :elastic do
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
end end
it "has a pagination", :sidekiq_might_not_need_inline do it "has a pagination" do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('foo') submit_search('foo')
...@@ -114,7 +119,7 @@ describe 'Global elastic search', :elastic do ...@@ -114,7 +119,7 @@ describe 'Global elastic search', :elastic do
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
end end
it "finds files", :sidekiq_might_not_need_inline do it "finds files" do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('application.js') submit_search('application.js')
...@@ -155,7 +160,7 @@ describe 'Global elastic search', :elastic do ...@@ -155,7 +160,7 @@ describe 'Global elastic search', :elastic do
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
end end
it "finds files", :sidekiq_might_not_need_inline do it "finds files" do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('term') submit_search('term')
...@@ -173,7 +178,7 @@ describe 'Global elastic search', :elastic do ...@@ -173,7 +178,7 @@ describe 'Global elastic search', :elastic do
Gitlab::Elastic::Helper.refresh_index Gitlab::Elastic::Helper.refresh_index
end end
it "finds commits", :sidekiq_might_not_need_inline do it "finds commits" do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('add') submit_search('add')
...@@ -183,7 +188,7 @@ describe 'Global elastic search', :elastic do ...@@ -183,7 +188,7 @@ describe 'Global elastic search', :elastic do
expect(page).to have_selector('.project-namespace') expect(page).to have_selector('.project-namespace')
end end
it 'shows proper page 2 results', :sidekiq_might_not_need_inline do it 'shows proper page 2 results' do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('add') submit_search('add')
...@@ -209,7 +214,7 @@ describe 'Global elastic search', :elastic do ...@@ -209,7 +214,7 @@ describe 'Global elastic search', :elastic do
submit_search('project') submit_search('project')
end end
it 'displays result counts for all categories', :sidekiq_might_not_need_inline do it 'displays result counts for all categories' do
expect(page).to have_content('Projects 1') expect(page).to have_content('Projects 1')
expect(page).to have_content('Issues 1') expect(page).to have_content('Issues 1')
expect(page).to have_content('Merge requests 0') expect(page).to have_content('Merge requests 0')
...@@ -226,17 +231,21 @@ describe 'Global elastic search', :elastic do ...@@ -226,17 +231,21 @@ describe 'Global elastic search', :elastic do
it 'allows basic search without Elasticsearch' do it 'allows basic search without Elasticsearch' do
visit dashboard_projects_path visit dashboard_projects_path
submit_search('project') # Disable sidekiq to ensure it does not end up in the index
Sidekiq::Testing.disable! do
create(:project, namespace: user.namespace, name: 'Will not be found but searchable')
end
# Project won't be found since ES index is not up to date submit_search('searchable')
expect(page).not_to have_content('Projects 1')
expect(page).not_to have_content('Will not be found')
# Since there are no results you have the option to instead use basic # Since there are no results you have the option to instead use basic
# search # search
click_link 'basic search' click_link 'basic search'
# Project is found now that we are using basic search # Project is found now that we are using basic search
expect(page).to have_content('Projects 1') expect(page).to have_content('Will not be found')
end end
context 'when performing Commits search' do context 'when performing Commits search' do
......
...@@ -188,6 +188,10 @@ FactoryBot.define do ...@@ -188,6 +188,10 @@ FactoryBot.define do
end end
end end
trait :sequence_source_branch do
sequence(:source_branch) { |n| "feature#{n}" }
end
after(:build) do |merge_request| after(:build) do |merge_request|
target_project = merge_request.target_project target_project = merge_request.target_project
source_project = merge_request.source_project source_project = merge_request.source_project
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment