diff --git a/.rubocop_todo/layout/empty_line_after_magic_comment.yml b/.rubocop_todo/layout/empty_line_after_magic_comment.yml index c2bb25afe05334cb4980cc46a5db1ca9fb580380..a465332f5134f23af7d34720cae79904d87bc1cf 100644 --- a/.rubocop_todo/layout/empty_line_after_magic_comment.yml +++ b/.rubocop_todo/layout/empty_line_after_magic_comment.yml @@ -251,7 +251,6 @@ Layout/EmptyLineAfterMagicComment: - 'ee/spec/lib/gitlab/middleware/ip_restrictor_spec.rb' - 'ee/spec/lib/gitlab/search/client_spec.rb' - 'ee/spec/lib/gitlab/spdx/catalogue_spec.rb' - - 'ee/spec/lib/system_check/app/advanced_search_migrations_check_spec.rb' - 'ee/spec/lib/system_check/geo/http_connection_check_spec.rb' - 'ee/spec/models/analytics/cycle_analytics/aggregation_context_spec.rb' - 'ee/spec/models/ci/minutes/quota_spec.rb' diff --git a/ee/app/services/elastic/data_migration_service.rb b/ee/app/services/elastic/data_migration_service.rb index 32b07f2189f933e9a05c1360584bb074f036928a..b6a04b6ba2f0e6297083d95f5323d2e7856c82a7 100644 --- a/ee/app/services/elastic/data_migration_service.rb +++ b/ee/app/services/elastic/data_migration_service.rb @@ -76,6 +76,8 @@ def pending_migrations? end def pending_migrations + return [] unless ::Gitlab::CurrentSettings.elasticsearch_indexing? + migrations(exclude_skipped: true).select do |migration| !migration_has_finished?(migration.name_for_key) end diff --git a/ee/spec/lib/system_check/app/advanced_search_migrations_check_spec.rb b/ee/spec/lib/system_check/app/advanced_search_migrations_check_spec.rb index 516cc9e3f3de84a94edc1b8f807276e7df5dc80e..409b7a3fd67f1563d2090adc8576db7a8bb07dd7 100644 --- a/ee/spec/lib/system_check/app/advanced_search_migrations_check_spec.rb +++ b/ee/spec/lib/system_check/app/advanced_search_migrations_check_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'spec_helper' RSpec.describe SystemCheck::App::AdvancedSearchMigrationsCheck, feature_category: :global_search do @@ -86,31 +87,27 @@ end describe '#pending_migrations_count' do - before do - allow(Elastic::DataMigrationService).to receive(:migration_files).and_return(migration_files) - end - subject { described_class.pending_migrations_count } context 'with pending migrations' do before do - allow(Elastic::DataMigrationService).to receive(:migration_has_finished?).with('test_migrate').and_return false + allow(Elastic::DataMigrationService).to receive(:pending_migrations).and_return([migration]) end it { is_expected.to eq 1 } + end - context 'when the migration is skipped' do - before do - allow(migration).to receive(:skip?).and_return true - end - - it { is_expected.to eq 0 } + context 'without pending migrations' do + before do + allow(Elastic::DataMigrationService).to receive(:pending_migrations).and_return([]) end + + it { is_expected.to eq 0 } end - context 'without pending migrations' do + context 'when pending_migrations returns a nil value' do before do - allow(Elastic::DataMigrationService).to receive(:migration_has_finished?).with('test_migrate').and_return true + allow(Elastic::DataMigrationService).to receive(:pending_migrations).and_return(nil) end it { is_expected.to eq 0 } diff --git a/ee/spec/services/elastic/data_migration_service_spec.rb b/ee/spec/services/elastic/data_migration_service_spec.rb index 26bbd862d5f4fedeca40b714d5f5753359b0d4e6..8963cc4555356aff8cd1195641ebc78fb367cb0b 100644 --- a/ee/spec/services/elastic/data_migration_service_spec.rb +++ b/ee/spec/services/elastic/data_migration_service_spec.rb @@ -285,21 +285,49 @@ end describe 'pending_migrations' do - let(:pending_migration1) { subject.migrations[1] } - let(:pending_migration2) { subject.migrations[2] } + let_it_be(:pending_migration1) { described_class.migrations[1] } + let_it_be(:pending_migration2) { described_class.migrations[2] } - before do + before_all do pending_migration1.save!(completed: false) pending_migration2.save!(completed: false) end - after do + after(:all) do # reset migration index to prevent flakiness described_class.mark_all_as_completed! end - it 'returns only pending migrations' do - expect(subject.pending_migrations.map(&:name)).to eq([pending_migration1, pending_migration2].map(&:name)) + subject(:pending_migrations) { described_class.pending_migrations } + + context 'when elasticsearch_indexing is enabled' do + before do + stub_ee_application_setting(elasticsearch_indexing: true) + end + + it 'returns only pending migrations' do + expected = [pending_migration1, pending_migration2].map(&:name) + + expect(described_class.pending_migrations.map(&:name)).to eq(expected) + end + + it 'does not include pending migrations which are skipped' do + allow_next_instance_of(Elastic::MigrationRecord) do |m| + allow(m).to receive(:skip?).and_return(true) + end + + expect(described_class.pending_migrations.map(&:name)).to eq([]) + end + end + + context 'when elasticsearch_indexing is disabled' do + before do + stub_ee_application_setting(elasticsearch_indexing: false) + end + + it 'returns no pending migrations' do + expect(described_class.pending_migrations).to eq([]) + end end end end