From 15e5a05bcd3525dd6c046dca2682b04532ba9bd1 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 25 Apr 2023 00:08:36 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../commands/metrics_server/metrics_server_spec.rb | 2 +- spec/commands/sidekiq_cluster/cli_spec.rb | 46 ++---- spec/config/object_store_settings_spec.rb | 16 +-- spec/config/settings_spec.rb | 2 +- spec/config/smime_signature_settings_spec.rb | 6 +- spec/features/markdown/sandboxed_mermaid_spec.rb | 2 +- spec/frontend/jobs/mock_data.js | 2 + .../components/table/admin_job_table_app_spec.js | 38 ++++- spec/initializers/settings_spec.rb | 7 +- spec/lib/gitlab/auth/o_auth/provider_spec.rb | 2 +- .../lib/gitlab/ci/components/instance_path_spec.rb | 2 +- spec/lib/gitlab/consul/internal_spec.rb | 2 +- .../self_monitoring/project/create_service_spec.rb | 2 +- .../lib/gitlab/legacy_github_import/client_spec.rb | 4 +- spec/lib/gitlab/prometheus/internal_spec.rb | 4 +- spec/lib/gitlab/sidekiq_config_spec.rb | 21 +++ .../tracking/destinations/snowplow_micro_spec.rb | 2 +- spec/lib/gitlab/tracking_spec.rb | 2 +- spec/lib/gitlab/url_blocker_spec.rb | 9 +- spec/lib/gitlab_settings/options_spec.rb | 155 +++++++++++++++++++++ spec/lib/gitlab_settings/settings_spec.rb | 53 +++++++ spec/metrics_server/metrics_server_spec.rb | 4 +- ...backfill_is_finished_for_gitlab_dot_com_spec.rb | 35 +++++ ...ji_note_id_to_bigint_for_gitlab_dot_com_spec.rb | 67 +++++++++ spec/models/instance_configuration_spec.rb | 2 +- spec/support/helpers/stub_configuration.rb | 6 +- spec/support/helpers/stub_object_storage.rb | 4 +- spec/uploaders/object_storage/cdn_spec.rb | 2 +- spec/uploaders/object_storage_spec.rb | 4 +- 29 files changed, 420 insertions(+), 83 deletions(-) create mode 100644 spec/lib/gitlab_settings/options_spec.rb create mode 100644 spec/lib/gitlab_settings/settings_spec.rb create mode 100644 spec/migrations/ensure_award_emoji_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb create mode 100644 spec/migrations/swap_award_emoji_note_id_to_bigint_for_gitlab_dot_com_spec.rb (limited to 'spec') diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb index 310e31da045..88a28b02903 100644 --- a/spec/commands/metrics_server/metrics_server_spec.rb +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -71,7 +71,7 @@ RSpec.describe 'GitLab metrics server', :aggregate_failures do if use_golang_server stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') allow(Settings).to receive(:monitoring).and_return( - Settingslogic.new(config.dig('test', 'monitoring'))) + GitlabSettings::Options.build(config.dig('test', 'monitoring'))) else config_file.write(YAML.dump(config)) config_file.close diff --git a/spec/commands/sidekiq_cluster/cli_spec.rb b/spec/commands/sidekiq_cluster/cli_spec.rb index 3951ef49288..499432c2605 100644 --- a/spec/commands/sidekiq_cluster/cli_spec.rb +++ b/spec/commands/sidekiq_cluster/cli_spec.rb @@ -18,17 +18,12 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, feature_category: :gitlab_cli, stub_ let(:sidekiq_exporter_enabled) { false } let(:sidekiq_exporter_port) { '3807' } - let(:config_file) { Tempfile.new('gitlab.yml') } let(:config) do { - 'test' => { - 'monitoring' => { - 'sidekiq_exporter' => { - 'address' => 'localhost', - 'enabled' => sidekiq_exporter_enabled, - 'port' => sidekiq_exporter_port - } - } + 'sidekiq_exporter' => { + 'address' => 'localhost', + 'enabled' => sidekiq_exporter_enabled, + 'port' => sidekiq_exporter_port } } end @@ -37,14 +32,6 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, feature_category: :gitlab_cli, stub_ let(:metrics_cleanup_service) { instance_double(Prometheus::CleanupMultiprocDirService, execute: nil) } before do - stub_env('RAILS_ENV', 'test') - - config_file.write(YAML.dump(config)) - config_file.close - - allow(::Settings).to receive(:source).and_return(config_file.path) - ::Settings.reload! - allow(Gitlab::ProcessManagement).to receive(:write_pid) allow(Gitlab::SidekiqCluster::SidekiqProcessSupervisor).to receive(:instance).and_return(supervisor) allow(supervisor).to receive(:supervise) @@ -52,8 +39,13 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, feature_category: :gitlab_cli, stub_ allow(Prometheus::CleanupMultiprocDirService).to receive(:new).and_return(metrics_cleanup_service) end - after do - config_file.unlink + around do |example| + original = Settings['monitoring'] + Settings['monitoring'] = config + + example.run + + Settings['monitoring'] = original end describe '#run' do @@ -318,13 +310,7 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, feature_category: :gitlab_cli, stub_ context 'when sidekiq_exporter is not set up' do let(:config) do - { - 'test' => { - 'monitoring' => { - 'sidekiq_exporter' => {} - } - } - } + { 'sidekiq_exporter' => {} } end it 'does not start a sidekiq metrics server' do @@ -336,13 +322,7 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, feature_category: :gitlab_cli, stub_ context 'with missing sidekiq_exporter setting' do let(:config) do - { - 'test' => { - 'monitoring' => { - 'sidekiq_exporter' => nil - } - } - } + { 'sidekiq_exporter' => nil } end it 'does not start a sidekiq metrics server' do diff --git a/spec/config/object_store_settings_spec.rb b/spec/config/object_store_settings_spec.rb index 025966c8940..b8e46affc2a 100644 --- a/spec/config/object_store_settings_spec.rb +++ b/spec/config/object_store_settings_spec.rb @@ -5,7 +5,7 @@ require Rails.root.join('config', 'object_store_settings.rb') RSpec.describe ObjectStoreSettings, feature_category: :shared do describe '#parse!' do - let(:settings) { Settingslogic.new(config) } + let(:settings) { GitlabSettings::Options.build(config) } subject { described_class.new(settings).parse! } @@ -68,7 +68,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do expect(settings.artifacts['enabled']).to be true expect(settings.artifacts['object_store']['enabled']).to be true - expect(settings.artifacts['object_store']['connection']).to eq(connection) + expect(settings.artifacts['object_store']['connection'].to_hash).to eq(connection) expect(settings.artifacts['object_store']['direct_upload']).to be true expect(settings.artifacts['object_store']['proxy_download']).to be false expect(settings.artifacts['object_store']['remote_directory']).to eq('artifacts') @@ -78,7 +78,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do expect(settings.lfs['enabled']).to be true expect(settings.lfs['object_store']['enabled']).to be true - expect(settings.lfs['object_store']['connection']).to eq(connection) + expect(settings.lfs['object_store']['connection'].to_hash).to eq(connection) expect(settings.lfs['object_store']['direct_upload']).to be true expect(settings.lfs['object_store']['proxy_download']).to be true expect(settings.lfs['object_store']['remote_directory']).to eq('lfs-objects') @@ -88,7 +88,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do expect(settings.pages['enabled']).to be true expect(settings.pages['object_store']['enabled']).to be true - expect(settings.pages['object_store']['connection']).to eq(connection) + expect(settings.pages['object_store']['connection'].to_hash).to eq(connection) expect(settings.pages['object_store']['remote_directory']).to eq('pages') expect(settings.pages['object_store']['bucket_prefix']).to eq(nil) expect(settings.pages['object_store']['consolidated_settings']).to be true @@ -128,7 +128,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do it 'populates artifacts CDN config' do subject - expect(settings.artifacts['object_store']['cdn']).to eq(cdn_config) + expect(settings.artifacts['object_store']['cdn'].to_hash).to eq(cdn_config) end end @@ -163,7 +163,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do it 'allows pages to define its own connection' do expect { subject }.not_to raise_error - expect(settings.pages['object_store']['connection']).to eq(pages_connection) + expect(settings.pages['object_store']['connection'].to_hash).to eq(pages_connection) expect(settings.pages['object_store']['consolidated_settings']).to be_falsey end end @@ -230,7 +230,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do end it 'respects original values' do - original_settings = Settingslogic.new({ + original_settings = GitlabSettings::Options.build({ 'enabled' => true, 'remote_directory' => 'artifacts' }) @@ -244,7 +244,7 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do end it 'supports bucket prefixes' do - original_settings = Settingslogic.new({ + original_settings = GitlabSettings::Options.build({ 'enabled' => true, 'remote_directory' => 'gitlab/artifacts' }) diff --git a/spec/config/settings_spec.rb b/spec/config/settings_spec.rb index b464a4eee8b..d6cddc215f5 100644 --- a/spec/config/settings_spec.rb +++ b/spec/config/settings_spec.rb @@ -31,7 +31,7 @@ RSpec.describe Settings, feature_category: :system_access do with_them do before do allow(Gitlab.config).to receive(:gitlab).and_return( - Settingslogic.new({ + GitlabSettings::Options.build({ 'host' => host, 'https' => true, 'port' => port, diff --git a/spec/config/smime_signature_settings_spec.rb b/spec/config/smime_signature_settings_spec.rb index 73dca66c666..53e70f1f2cc 100644 --- a/spec/config/smime_signature_settings_spec.rb +++ b/spec/config/smime_signature_settings_spec.rb @@ -19,7 +19,7 @@ RSpec.describe SmimeSignatureSettings, feature_category: :shared do context 'when providing custom values' do it 'sets correct default values to disabled' do - custom_settings = Settingslogic.new({}) + custom_settings = GitlabSettings::Options.build({}) parsed_settings = described_class.parse(custom_settings) @@ -30,7 +30,7 @@ RSpec.describe SmimeSignatureSettings, feature_category: :shared do end it 'enables smime with default key and cert' do - custom_settings = Settingslogic.new({ + custom_settings = GitlabSettings::Options.build({ 'enabled' => true }) @@ -46,7 +46,7 @@ RSpec.describe SmimeSignatureSettings, feature_category: :shared do custom_key = '/custom/key' custom_cert = '/custom/cert' custom_ca_certs = '/custom/ca_certs' - custom_settings = Settingslogic.new({ + custom_settings = GitlabSettings::Options.build({ 'enabled' => true, 'key_file' => custom_key, 'cert_file' => custom_cert, diff --git a/spec/features/markdown/sandboxed_mermaid_spec.rb b/spec/features/markdown/sandboxed_mermaid_spec.rb index 0ecba8df88b..f8a535191da 100644 --- a/spec/features/markdown/sandboxed_mermaid_spec.rb +++ b/spec/features/markdown/sandboxed_mermaid_spec.rb @@ -57,7 +57,7 @@ RSpec.describe 'Sandboxed Mermaid rendering', :js, feature_category: :team_plann context 'in a project milestone' do let(:milestone) { create(:project_milestone, project: project, description: description) } - it 'includes mermaid frame correctly' do + it 'includes mermaid frame correctly', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/408560' do visit(project_milestone_path(project, milestone)) wait_for_requests diff --git a/spec/frontend/jobs/mock_data.js b/spec/frontend/jobs/mock_data.js index ed89221d062..de0c06e001a 100644 --- a/spec/frontend/jobs/mock_data.js +++ b/spec/frontend/jobs/mock_data.js @@ -1,5 +1,6 @@ import mockJobsCount from 'test_fixtures/graphql/jobs/get_jobs_count.query.graphql.json'; import mockJobsEmpty from 'test_fixtures/graphql/jobs/get_jobs.query.graphql.empty.json'; +import mockAllJobsEmpty from 'test_fixtures/graphql/jobs/get_all_jobs.query.graphql.empty.json'; import mockJobsPaginated from 'test_fixtures/graphql/jobs/get_jobs.query.graphql.paginated.json'; import mockAllJobsPaginated from 'test_fixtures/graphql/jobs/get_all_jobs.query.graphql.paginated.json'; import mockJobs from 'test_fixtures/graphql/jobs/get_jobs.query.graphql.json'; @@ -16,6 +17,7 @@ threeWeeksAgo.setDate(threeWeeksAgo.getDate() - 21); export const mockJobsResponsePaginated = mockJobsPaginated; export const mockAllJobsResponsePaginated = mockAllJobsPaginated; export const mockJobsResponseEmpty = mockJobsEmpty; +export const mockAllJobsResponseEmpty = mockAllJobsEmpty; export const mockJobsNodes = mockJobs.data.project.jobs.nodes; export const mockAllJobsNodes = mockAllJobs.data.jobs.nodes; export const mockJobsNodesAsGuest = mockJobsAsGuest.data.project.jobs.nodes; diff --git a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js index 1341949439b..53d69b01d97 100644 --- a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js +++ b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js @@ -1,6 +1,6 @@ -import { GlLoadingIcon, GlEmptyState, GlAlert } from '@gitlab/ui'; +import { GlLoadingIcon, GlEmptyState, GlAlert, GlIntersectionObserver } from '@gitlab/ui'; import { mount, shallowMount } from '@vue/test-utils'; -import Vue from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -14,8 +14,8 @@ import JobsTable from '~/jobs/components/table/jobs_table.vue'; import { mockAllJobsResponsePaginated, - mockJobsResponseEmpty, mockCancelableJobsCountResponse, + mockAllJobsResponseEmpty, statuses, } from '../../../../../jobs/mock_data'; @@ -25,9 +25,9 @@ describe('Job table app', () => { let wrapper; const successHandler = jest.fn().mockResolvedValue(mockAllJobsResponsePaginated); - const emptyHandler = jest.fn().mockResolvedValue(mockJobsResponseEmpty); const failedHandler = jest.fn().mockRejectedValue(new Error('GraphQL error')); const cancelHandler = jest.fn().mockResolvedValue(mockCancelableJobsCountResponse); + const emptyHandler = jest.fn().mockResolvedValue(mockAllJobsResponseEmpty); const findSkeletonLoader = () => wrapper.findComponent(JobsSkeletonLoader); const findLoadingSpinner = () => wrapper.findComponent(GlLoadingIcon); @@ -37,6 +37,9 @@ describe('Job table app', () => { const findTabs = () => wrapper.findComponent(JobsTableTabs); const findCancelJobsButton = () => wrapper.findComponent(CancelJobs); + const triggerInfiniteScroll = () => + wrapper.findComponent(GlIntersectionObserver).vm.$emit('appear'); + const createMockApolloProvider = (handler, cancelableHandler) => { const requestHandlers = [ [getAllJobsQuery, handler], @@ -106,6 +109,33 @@ describe('Job table app', () => { expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(1); }); + + describe('when infinite scrolling is triggered', () => { + it('does not display a skeleton loader', () => { + triggerInfiniteScroll(); + + expect(findSkeletonLoader().exists()).toBe(false); + }); + + it('handles infinite scrolling by calling fetch more', async () => { + triggerInfiniteScroll(); + + await nextTick(); + + const pageSize = 50; + + expect(findLoadingSpinner().exists()).toBe(true); + + await waitForPromises(); + + expect(findLoadingSpinner().exists()).toBe(false); + + expect(successHandler).toHaveBeenLastCalledWith({ + first: pageSize, + after: mockAllJobsResponsePaginated.data.jobs.pageInfo.endCursor, + }); + }); + }); }); describe('empty state', () => { diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb index c3200d2fab1..09064a21099 100644 --- a/spec/initializers/settings_spec.rb +++ b/spec/initializers/settings_spec.rb @@ -9,12 +9,7 @@ RSpec.describe Settings do expect(Gitlab.config.ldap.servers.main.label).to eq('ldap') end - # Specifically trying to cause this error discovered in EE when removing the - # reassignment of each server element with Settingslogic. - # - # `undefined method `label' for #` - # - it 'can be accessed in a very specific way that breaks without reassigning each element with Settingslogic' do + it 'can be accessed in a very specific way that breaks without reassigning each element' do server_settings = Gitlab.config.ldap.servers['main'] expect(server_settings.label).to eq('ldap') end diff --git a/spec/lib/gitlab/auth/o_auth/provider_spec.rb b/spec/lib/gitlab/auth/o_auth/provider_spec.rb index 96a31c50989..226669bab33 100644 --- a/spec/lib/gitlab/auth/o_auth/provider_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/provider_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Gitlab::Auth::OAuth::Provider do context 'for an LDAP provider' do context 'when the provider exists' do it 'returns the config' do - expect(described_class.config_for('ldapmain')).to be_a(Hash) + expect(described_class.config_for('ldapmain')).to be_a(GitlabSettings::Options) end end diff --git a/spec/lib/gitlab/ci/components/instance_path_spec.rb b/spec/lib/gitlab/ci/components/instance_path_spec.rb index e037c37c817..b80422d03e5 100644 --- a/spec/lib/gitlab/ci/components/instance_path_spec.rb +++ b/spec/lib/gitlab/ci/components/instance_path_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Gitlab::Ci::Components::InstancePath, feature_category: :pipeline let_it_be(:user) { create(:user) } let(:path) { described_class.new(address: address, content_filename: 'template.yml') } - let(:settings) { Settingslogic.new({ 'component_fqdn' => current_host }) } + let(:settings) { GitlabSettings::Options.build({ 'component_fqdn' => current_host }) } let(:current_host) { 'acme.com/' } before do diff --git a/spec/lib/gitlab/consul/internal_spec.rb b/spec/lib/gitlab/consul/internal_spec.rb index ff76269fe76..cd3436b3fa4 100644 --- a/spec/lib/gitlab/consul/internal_spec.rb +++ b/spec/lib/gitlab/consul/internal_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Gitlab::Consul::Internal do context 'when consul setting is not present in gitlab.yml' do before do - allow(Gitlab.config).to receive(:consul).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:consul).and_raise(GitlabSettings::MissingSetting) end it 'does not fail' do diff --git a/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb b/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb index ad91320c6eb..5d193245b02 100644 --- a/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb +++ b/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb @@ -228,7 +228,7 @@ RSpec.describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService context 'when prometheus setting is not present in gitlab.yml' do before do - allow(Gitlab.config).to receive(:prometheus).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:prometheus).and_raise(GitlabSettings::MissingSetting) end it 'does not fail' do diff --git a/spec/lib/gitlab/legacy_github_import/client_spec.rb b/spec/lib/gitlab/legacy_github_import/client_spec.rb index 08679b7e9f1..d0f63d11469 100644 --- a/spec/lib/gitlab/legacy_github_import/client_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/client_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::Client do let(:token) { '123456' } - let(:github_provider) { Settingslogic.new('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) } + let(:github_provider) { GitlabSettings::Options.build('app_id' => 'asd123', 'app_secret' => 'asd123', 'name' => 'github', 'args' => { 'client_options' => {} }) } let(:wait_for_rate_limit_reset) { true } subject(:client) { described_class.new(token, wait_for_rate_limit_reset: wait_for_rate_limit_reset) } @@ -17,7 +17,7 @@ RSpec.describe Gitlab::LegacyGithubImport::Client do expect(client.client.options.keys).to all(be_kind_of(Symbol)) end - it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do + it 'does not crash (e.g. GitlabSettings::MissingSetting) when verify_ssl config is not present' do expect { client.api }.not_to raise_error end diff --git a/spec/lib/gitlab/prometheus/internal_spec.rb b/spec/lib/gitlab/prometheus/internal_spec.rb index b08b8813470..ff5da301347 100644 --- a/spec/lib/gitlab/prometheus/internal_spec.rb +++ b/spec/lib/gitlab/prometheus/internal_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Gitlab::Prometheus::Internal do context 'when prometheus setting is not present in gitlab.yml' do before do - allow(Gitlab.config).to receive(:prometheus).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:prometheus).and_raise(GitlabSettings::MissingSetting) end it 'does not fail' do @@ -97,7 +97,7 @@ RSpec.describe Gitlab::Prometheus::Internal do context 'when prometheus setting is not present in gitlab.yml' do before do - allow(Gitlab.config).to receive(:prometheus).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:prometheus).and_raise(GitlabSettings::MissingSetting) end it 'does not fail' do diff --git a/spec/lib/gitlab/sidekiq_config_spec.rb b/spec/lib/gitlab/sidekiq_config_spec.rb index 5f72a3feba7..00b1666106f 100644 --- a/spec/lib/gitlab/sidekiq_config_spec.rb +++ b/spec/lib/gitlab/sidekiq_config_spec.rb @@ -17,6 +17,27 @@ RSpec.describe Gitlab::SidekiqConfig do end end + describe '.cron_jobs' do + it 'renames job_class to class and removes incomplete jobs' do + expect(Gitlab) + .to receive(:config) + .twice + .and_return(GitlabSettings::Options.build( + load_dynamic_cron_schedules!: true, + cron_jobs: { + job: { cron: '0 * * * *', job_class: 'SomeWorker' }, + incomplete_job: { cron: '0 * * * *' } + })) + + expect(Gitlab::AppLogger) + .to receive(:error) + .with("Invalid cron_jobs config key: 'incomplete_job'. Check your gitlab config file.") + + expect(described_class.cron_jobs) + .to eq('job' => { 'class' => 'SomeWorker', 'cron' => '0 * * * *' }) + end + end + describe '.worker_queues' do it 'includes all queues' do queues = described_class.worker_queues diff --git a/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb b/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb index 48092a33da3..ea3c030541f 100644 --- a/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb +++ b/spec/lib/gitlab/tracking/destinations/snowplow_micro_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Gitlab::Tracking::Destinations::SnowplowMicro do context 'when snowplow_micro config is not set' do before do - allow(Gitlab.config).to receive(:snowplow_micro).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:snowplow_micro).and_raise(GitlabSettings::MissingSetting) end it 'returns localhost hostname' do diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb index 56be80678e9..a3de64a002e 100644 --- a/spec/lib/gitlab/tracking_spec.rb +++ b/spec/lib/gitlab/tracking_spec.rb @@ -312,7 +312,7 @@ RSpec.describe Gitlab::Tracking, feature_category: :application_instrumentation end it 'returns false when snowplow_micro is not configured' do - allow(Gitlab.config).to receive(:snowplow_micro).and_raise(Settingslogic::MissingSetting) + allow(Gitlab.config).to receive(:snowplow_micro).and_raise(GitlabSettings::MissingSetting) expect(described_class).not_to be_snowplow_micro_enabled end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 7b6c89b5dd3..d790d13f66c 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -166,8 +166,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only, feature_category: :sh let(:lfs_config) do { 'enabled' => lfs_enabled, - # This nesting of Settingslogic is necessary to trigger the bug - 'object_store' => Settingslogic.new({ 'enabled' => true }) + # This nesting of settings is necessary to trigger the bug + 'object_store' => GitlabSettings::Options.build({ 'enabled' => true }) } end @@ -175,16 +175,15 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only, feature_category: :sh { 'gitlab' => Gitlab.config.gitlab, 'repositories' => { 'storages' => { 'default' => 'test' } }, - 'lfs' => Settingslogic.new(lfs_config) + 'lfs' => GitlabSettings::Options.build(lfs_config) } end let(:host) { 'http://127.0.0.1:9000' } - let(:settings) { Settingslogic.new(config) } + let(:settings) { GitlabSettings::Options.build(config) } before do allow(Gitlab).to receive(:config).and_return(settings) - # Triggers Settingslogic bug: https://gitlab.com/gitlab-org/gitlab/-/issues/286873 settings.repositories.storages.default end diff --git a/spec/lib/gitlab_settings/options_spec.rb b/spec/lib/gitlab_settings/options_spec.rb new file mode 100644 index 00000000000..4b57e91c2e1 --- /dev/null +++ b/spec/lib/gitlab_settings/options_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: :shared do + let(:config) { { foo: { bar: 'baz' } } } + + subject(:options) { described_class.build(config) } + + describe '.build' do + context 'when argument is a hash' do + it 'creates a new GitlabSettings::Options instance' do + options = described_class.build(config) + + expect(options).to be_a described_class + expect(options.foo).to be_a described_class + expect(options.foo.bar).to eq 'baz' + end + end + end + + describe '#[]' do + it 'accesses the configuration key as string' do + expect(options['foo']).to be_a described_class + expect(options['foo']['bar']).to eq 'baz' + + expect(options['inexistent']).to be_nil + end + + it 'accesses the configuration key as symbol' do + expect(options[:foo]).to be_a described_class + expect(options[:foo][:bar]).to eq 'baz' + + expect(options[:inexistent]).to be_nil + end + end + + describe '#[]=' do + it 'changes the configuration key as string' do + options['foo']['bar'] = 'anothervalue' + + expect(options['foo']['bar']).to eq 'anothervalue' + end + + it 'changes the configuration key as symbol' do + options[:foo][:bar] = 'anothervalue' + + expect(options[:foo][:bar]).to eq 'anothervalue' + end + + context 'when key does not exist' do + it 'creates a new configuration by string key' do + options['inexistent'] = 'value' + + expect(options['inexistent']).to eq 'value' + end + + it 'creates a new configuration by string key' do + options[:inexistent] = 'value' + + expect(options[:inexistent]).to eq 'value' + end + end + end + + describe '#key?' do + it 'checks if a string key exists' do + expect(options.key?('foo')).to be true + expect(options.key?('inexistent')).to be false + end + + it 'checks if a symbol key exists' do + expect(options.key?(:foo)).to be true + expect(options.key?(:inexistent)).to be false + end + end + + describe '#to_hash' do + it 'returns the hash representation of the config' do + expect(options.to_hash).to eq('foo' => { 'bar' => 'baz' }) + end + end + + describe '#merge' do + it 'merges a hash to the existing options' do + expect(options.merge(more: 'configs').to_hash).to eq( + 'foo' => { 'bar' => 'baz' }, + 'more' => 'configs' + ) + end + + context 'when the merge hash replaces existing configs' do + it 'merges a hash to the existing options' do + expect(options.merge(foo: 'configs').to_hash).to eq('foo' => 'configs') + end + end + end + + describe '#deep_merge' do + it 'merges a hash to the existing options' do + expect(options.deep_merge(foo: { more: 'configs' }).to_hash).to eq('foo' => { + 'bar' => 'baz', + 'more' => 'configs' + }) + end + + context 'when the merge hash replaces existing configs' do + it 'merges a hash to the existing options' do + expect(options.deep_merge(foo: { bar: 'configs' }).to_hash).to eq('foo' => { + 'bar' => 'configs' + }) + end + end + end + + describe '#is_a?' do + it 'returns false for anything different of Hash or GitlabSettings::Options' do + expect(options.is_a?(described_class)).to be true + expect(options.is_a?(Hash)).to be true + expect(options.is_a?(String)).to be false + end + end + + describe '#method_missing' do + context 'when method is an option' do + it 'delegates methods to options keys' do + expect(options.foo.bar).to eq('baz') + end + + it 'uses methods to change options values' do + expect { options.foo = 1 } + .to change { options.foo } + .to(1) + end + end + + context 'when method is not an option' do + it 'delegates the method to the internal options hash' do + expect { options.foo.delete('bar') } + .to change { options.to_hash } + .to({ 'foo' => {} }) + end + end + + context 'when method is not an option and does not exist in hash' do + it 'raises GitlabSettings::MissingSetting' do + expect { options.anything } + .to raise_error( + ::GitlabSettings::MissingSetting, + "option 'anything' not defined" + ) + end + end + end +end diff --git a/spec/lib/gitlab_settings/settings_spec.rb b/spec/lib/gitlab_settings/settings_spec.rb new file mode 100644 index 00000000000..55ceff4ce82 --- /dev/null +++ b/spec/lib/gitlab_settings/settings_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSettings::Settings, :aggregate_failures, feature_category: :shared do + let(:config) do + { + section1: { + config1: { + value1: 1 + } + } + } + end + + let(:source) { Tempfile.new('config.yaml') } + + before do + File.write(source, config.to_yaml) + end + + subject(:settings) { described_class.new(source.path, 'section1') } + + it 'requires a source' do + expect { described_class.new('', '') } + .to raise_error(ArgumentError, 'config source is required') + end + + it 'requires a section' do + expect { described_class.new(source, '') } + .to raise_error(ArgumentError, 'config section is required') + end + + it 'loads the given section config' do + expect(settings.config1.value1).to eq(1) + end + + describe '#reload!' do + it 'reloads the config' do + expect(settings.config1.value1).to eq(1) + + File.write(source, { section1: { config1: { value1: 2 } } }.to_yaml) + + # config doesn't change when source changes + expect(settings.config1.value1).to eq(1) + + settings.reload! + + # config changes after reload! if source changed + expect(settings.config1.value1).to eq(2) + end + end +end diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb index efa716754f1..ad80835549f 100644 --- a/spec/metrics_server/metrics_server_spec.rb +++ b/spec/metrics_server/metrics_server_spec.rb @@ -99,7 +99,7 @@ RSpec.describe MetricsServer, feature_category: :application_performance do # ru context 'for Golang server' do let(:log_enabled) { false } let(:settings) do - Settingslogic.new( + GitlabSettings::Options.build( { 'web_exporter' => { 'enabled' => true, @@ -304,7 +304,7 @@ RSpec.describe MetricsServer, feature_category: :application_performance do # ru end context 'for sidekiq' do - let(:settings) { Settingslogic.new({ "sidekiq_exporter" => { "enabled" => true } }) } + let(:settings) { GitlabSettings::Options.build({ "sidekiq_exporter" => { "enabled" => true } }) } before do allow(::Settings).to receive(:monitoring).and_return(settings) diff --git a/spec/migrations/ensure_award_emoji_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb b/spec/migrations/ensure_award_emoji_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..826320ec6c2 --- /dev/null +++ b/spec/migrations/ensure_award_emoji_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe EnsureAwardEmojiBigintBackfillIsFinishedForGitlabDotCom, feature_category: :database do + describe '#up' do + let(:migration_arguments) do + { + job_class_name: 'CopyColumnUsingBackgroundMigrationJob', + table_name: 'award_emoji', + column_name: 'id', + job_arguments: [['awardable_id'], ['awardable_id_convert_to_bigint']] + } + end + + it 'ensures the migration is completed for GitLab.com, dev, or test' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + expect(instance).to receive(:ensure_batched_background_migration_is_finished).with(migration_arguments) + end + + migrate! + end + + it 'skips the check for other instances' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + expect(instance).not_to receive(:ensure_batched_background_migration_is_finished) + end + + migrate! + end + end +end diff --git a/spec/migrations/swap_award_emoji_note_id_to_bigint_for_gitlab_dot_com_spec.rb b/spec/migrations/swap_award_emoji_note_id_to_bigint_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..c133deaf250 --- /dev/null +++ b/spec/migrations/swap_award_emoji_note_id_to_bigint_for_gitlab_dot_com_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SwapAwardEmojiNoteIdToBigintForGitlabDotCom, feature_category: :database do + describe '#up' do + before do + # A we call `schema_migrate_down!` before each example, and for this migration + # `#down` is same as `#up`, we need to ensure we start from the expected state. + connection = described_class.new.connection + connection.execute('ALTER TABLE award_emoji ALTER COLUMN awardable_id TYPE integer') + connection.execute('ALTER TABLE award_emoji ALTER COLUMN awardable_id_convert_to_bigint TYPE bigint') + end + + # rubocop: disable RSpec/AnyInstanceOf + it 'swaps the integer and bigint columns for GitLab.com, dev, or test' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + + award_emoji = table(:award_emoji) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + award_emoji.reset_column_information + + expect(award_emoji.columns.find { |c| c.name == 'awardable_id' }.sql_type).to eq('integer') + expect(award_emoji.columns.find { |c| c.name == 'awardable_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + award_emoji.reset_column_information + + expect(award_emoji.columns.find { |c| c.name == 'awardable_id' }.sql_type).to eq('bigint') + expect(award_emoji.columns.find { |c| c.name == 'awardable_id_convert_to_bigint' }.sql_type) + .to eq('integer') + } + end + end + end + + it 'is a no-op for other instances' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + + award_emoji = table(:award_emoji) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + award_emoji.reset_column_information + + expect(award_emoji.columns.find { |c| c.name == 'awardable_id' }.sql_type).to eq('integer') + expect(award_emoji.columns.find { |c| c.name == 'awardable_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + award_emoji.reset_column_information + + expect(award_emoji.columns.find { |c| c.name == 'awardable_id' }.sql_type).to eq('integer') + expect(award_emoji.columns.find { |c| c.name == 'awardable_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + end + end + end + # rubocop: enable RSpec/AnyInstanceOf + end +end diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 4c5c1bc3151..7710a05820c 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -73,7 +73,7 @@ RSpec.describe InstanceConfiguration do it 'returns Settings.pages' do gitlab_pages.delete(:ip_address) - expect(gitlab_pages).to eq(Settings.pages.symbolize_keys) + expect(gitlab_pages).to eq(Settings.pages.to_hash.deep_symbolize_keys) end it 'returns the GitLab\'s pages host ip address' do diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index 2a7b36a4c00..4c997aceeee 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -102,7 +102,7 @@ module StubConfiguration messages[storage_name] = Gitlab::GitalyClient::StorageSettings.new(storage_hash.to_h) end - allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages)) + allow(Gitlab.config.repositories).to receive(:storages).and_return(::GitlabSettings::Options.build(messages)) end def stub_sentry_settings(enabled: true) @@ -175,11 +175,11 @@ module StubConfiguration end end - # Support nested hashes by converting all values into Settingslogic objects + # Support nested hashes by converting all values into GitlabSettings::Objects objects def to_settings(hash) hash.transform_values do |value| if value.is_a? Hash - Settingslogic.new(value.to_h.deep_stringify_keys) + ::GitlabSettings::Options.build(value) else value end diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 4efe2a98a45..88fef1aa0b3 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -17,7 +17,7 @@ module StubObjectStorage direct_upload: false, cdn: {} ) - old_config = Settingslogic.new(config.to_h.deep_stringify_keys) + old_config = ::GitlabSettings::Options.build(config.to_h.deep_stringify_keys) new_config = config.to_h.deep_symbolize_keys.merge({ enabled: enabled, proxy_download: proxy_download, @@ -32,7 +32,7 @@ module StubObjectStorage allow(config).to receive(:proxy_download) { proxy_download } allow(config).to receive(:direct_upload) { direct_upload } - uploader_config = Settingslogic.new(new_config.to_h.deep_stringify_keys) + uploader_config = ::GitlabSettings::Options.build(new_config.to_h.deep_stringify_keys) allow(uploader).to receive(:object_store_options).and_return(uploader_config) allow(uploader.options).to receive(:object_store).and_return(uploader_config) diff --git a/spec/uploaders/object_storage/cdn_spec.rb b/spec/uploaders/object_storage/cdn_spec.rb index 0c1966b4df2..28b3313428b 100644 --- a/spec/uploaders/object_storage/cdn_spec.rb +++ b/spec/uploaders/object_storage/cdn_spec.rb @@ -41,7 +41,7 @@ RSpec.describe ObjectStorage::CDN, feature_category: :build_artifacts do before do stub_artifacts_object_storage(enabled: true) - options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options)) + options = Gitlab.config.uploads.deep_merge(cdn_options) allow(uploader_class).to receive(:options).and_return(options) end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 0e293ec973c..ef46803fc56 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -446,7 +446,7 @@ RSpec.describe ObjectStorage do end describe '#fog_credentials' do - let(:connection) { Settingslogic.new("provider" => "AWS") } + let(:connection) { GitlabSettings::Options.build("provider" => "AWS") } before do allow(uploader_class).to receive(:options) do @@ -479,7 +479,7 @@ RSpec.describe ObjectStorage do } end - let(:options) { Settingslogic.new(raw_options) } + let(:options) { GitlabSettings::Options.build(raw_options) } before do allow(uploader_class).to receive(:options) do -- cgit v1.2.3