diff options
Diffstat (limited to 'spec/lib')
123 files changed, 3499 insertions, 1861 deletions
diff --git a/spec/lib/api/github/entities_spec.rb b/spec/lib/api/github/entities_spec.rb new file mode 100644 index 00000000000..00ea60c5d65 --- /dev/null +++ b/spec/lib/api/github/entities_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Github::Entities do + describe API::Github::Entities::User do + let(:user) { create(:user, username: username) } + let(:username) { 'name_of_user' } + let(:gitlab_protocol_and_host) { "#{Gitlab.config.gitlab.protocol}://#{Gitlab.config.gitlab.host}" } + let(:expected_user_url) { "#{gitlab_protocol_and_host}/#{username}" } + let(:entity) { described_class.new(user) } + + subject { entity.as_json } + + specify :aggregate_failure do + expect(subject[:id]).to eq user.id + expect(subject[:login]).to eq 'name_of_user' + expect(subject[:url]).to eq expected_user_url + expect(subject[:html_url]).to eq expected_user_url + expect(subject[:avatar_url]).to include('https://www.gravatar.com/avatar') + end + + context 'with avatar' do + let(:user) { create(:user, :with_avatar, username: username) } + + specify do + expect(subject[:avatar_url]).to include("#{gitlab_protocol_and_host}/uploads/-/system/user/avatar/") + end + end + end +end diff --git a/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb b/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb index ccf96bcbad6..6d06fc3618d 100644 --- a/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb +++ b/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb @@ -24,6 +24,7 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do shared_examples 'executing redirect' do it 'redirects to package registry' do + expect(helper).to receive(:track_event).with('npm_request_forward').once expect(helper).to receive(:registry_url).once expect(helper).to receive(:redirect).once expect(helper).to receive(:fallback).never @@ -63,6 +64,7 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do let(:package_type) { pkg_type } it 'raises an error' do + allow(helper).to receive(:track_event) expect { subject }.to raise_error(ArgumentError, "Can't build registry_url for package_type #{package_type}") end end diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 51a45dff6a4..8e738af0fa3 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -191,41 +191,32 @@ RSpec.describe API::Helpers do describe '#increment_unique_values' do let(:value) { '9f302fea-f828-4ca9-aef4-e10bd723c0b3' } - let(:event_name) { 'my_event' } + let(:event_name) { 'g_compliance_dashboard' } let(:unknown_event) { 'unknown' } let(:feature) { "usage_data_#{event_name}" } + before do + skip_feature_flags_yaml_validation + end + context 'with feature enabled' do before do stub_feature_flags(feature => true) end it 'tracks redis hll event' do - stub_application_setting(usage_ping_enabled: true) - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(value, event_name) subject.increment_unique_values(event_name, value) end - it 'does not track event usage ping is not enabled' do - stub_application_setting(usage_ping_enabled: false) - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - - subject.increment_unique_values(event_name, value) - end - it 'logs an exception for unknown event' do - stub_application_setting(usage_ping_enabled: true) - expect(Gitlab::AppLogger).to receive(:warn).with("Redis tracking event failed for event: #{unknown_event}, message: Unknown event #{unknown_event}") subject.increment_unique_values(unknown_event, value) end it 'does not track event for nil values' do - stub_application_setting(usage_ping_enabled: true) - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) subject.increment_unique_values(unknown_event, nil) diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb index c2dbaac7f15..45cc73974d6 100644 --- a/spec/lib/backup/files_spec.rb +++ b/spec/lib/backup/files_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Backup::Files do let(:timestamp) { Time.utc(2017, 3, 22) } around do |example| - Timecop.freeze(timestamp) { example.run } + travel_to(timestamp) { example.run } end describe 'folders with permission' do diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repositories_spec.rb index 718f38f9452..540c64e74ca 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repositories_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' -RSpec.describe Backup::Repository do - let_it_be(:project) { create(:project, :wiki_repo) } - +RSpec.describe Backup::Repositories do let(:progress) { StringIO.new } subject { described_class.new(progress) } @@ -12,7 +10,6 @@ RSpec.describe Backup::Repository do before do allow(progress).to receive(:puts) allow(progress).to receive(:print) - allow(FileUtils).to receive(:mv).and_return(true) allow_next_instance_of(described_class) do |instance| allow(instance).to receive(:progress).and_return(progress) @@ -20,13 +17,33 @@ RSpec.describe Backup::Repository do end describe '#dump' do - before do - allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(storage_keys) + let_it_be(:projects) { create_list(:project, 5, :repository) } + + RSpec.shared_examples 'creates repository bundles' do + specify :aggregate_failures do + # Add data to the wiki and design repositories, so they will be included in the dump. + create(:wiki_page, container: project) + create(:design, :with_file, issue: create(:issue, project: project)) + + subject.dump(max_concurrency: 1, max_storage_concurrency: 1) + + expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.bundle')) + expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.wiki' + '.bundle')) + expect(File).to exist(File.join(Gitlab.config.backup.path, 'repositories', project.disk_path + '.design' + '.bundle')) + end end - let_it_be(:projects) { create_list(:project, 5, :wiki_repo) + [project] } + context 'hashed storage' do + let_it_be(:project) { create(:project, :repository) } + + it_behaves_like 'creates repository bundles' + end - let(:storage_keys) { %w[default test_second_storage] } + context 'legacy storage' do + let_it_be(:project) { create(:project, :repository, :legacy_storage) } + + it_behaves_like 'creates repository bundles' + end context 'no concurrency' do it 'creates the expected number of threads' do @@ -58,7 +75,7 @@ RSpec.describe Backup::Repository do subject.dump(max_concurrency: 1, max_storage_concurrency: 1) end.count - create_list(:project, 2, :wiki_repo) + create_list(:project, 2, :repository) expect do subject.dump(max_concurrency: 1, max_storage_concurrency: 1) @@ -68,6 +85,12 @@ RSpec.describe Backup::Repository do [4, 10].each do |max_storage_concurrency| context "max_storage_concurrency #{max_storage_concurrency}", quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/241701' do + let(:storage_keys) { %w[default test_second_storage] } + + before do + allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(storage_keys) + end + it 'creates the expected number of threads' do expect(Thread).to receive(:new) .exactly(storage_keys.length * (max_storage_concurrency + 1)).times @@ -120,7 +143,7 @@ RSpec.describe Backup::Repository do subject.dump(max_concurrency: 1, max_storage_concurrency: max_storage_concurrency) end.count - create_list(:project, 2, :wiki_repo) + create_list(:project, 2, :repository) expect do subject.dump(max_concurrency: 1, max_storage_concurrency: max_storage_concurrency) @@ -131,45 +154,55 @@ RSpec.describe Backup::Repository do end describe '#restore' do - let(:timestamp) { Time.utc(2017, 3, 22) } - let(:temp_dirs) do - Gitlab.config.repositories.storages.map do |name, storage| - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s) - end + let_it_be(:project) { create(:project) } + + it 'restores repositories from bundles', :aggregate_failures do + next_path_to_bundle = [ + Rails.root.join('spec/fixtures/lib/backup/project_repo.bundle'), + Rails.root.join('spec/fixtures/lib/backup/wiki_repo.bundle'), + Rails.root.join('spec/fixtures/lib/backup/design_repo.bundle') + ].to_enum + + allow_next_instance_of(described_class::BackupRestore) do |backup_restore| + allow(backup_restore).to receive(:path_to_bundle).and_return(next_path_to_bundle.next) end - end - around do |example| - Timecop.freeze(timestamp) { example.run } - end + subject.restore + + collect_commit_shas = -> (repo) { repo.commits('master', limit: 10).map(&:sha) } - after do - temp_dirs.each { |path| FileUtils.rm_rf(path) } + expect(collect_commit_shas.call(project.repository)).to eq(['393a7d860a5a4c3cc736d7eb00604e3472bb95ec']) + expect(collect_commit_shas.call(project.wiki.repository)).to eq(['c74b9948d0088d703ee1fafeddd9ed9add2901ea']) + expect(collect_commit_shas.call(project.design_repository)).to eq(['c3cd4d7bd73a51a0f22045c3a4c871c435dc959d']) end describe 'command failure' do before do - # Allow us to set expectations on the project directly expect(Project).to receive(:find_each).and_yield(project) - expect(project.repository).to receive(:create_repository) { raise 'Fail in tests' } + + allow_next_instance_of(DesignManagement::Repository) do |repository| + allow(repository).to receive(:create_repository) { raise 'Fail in tests' } + end + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:create_repository) { raise 'Fail in tests' } + end end context 'hashed storage' do it 'shows the appropriate error' do subject.restore - expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} repository") + expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} (#{project.disk_path})") end end context 'legacy storage' do - let!(:project) { create(:project, :legacy_storage) } + let_it_be(:project) { create(:project, :legacy_storage) } it 'shows the appropriate error' do subject.restore - expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} repository") + expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} (#{project.disk_path})") end end end @@ -188,45 +221,18 @@ RSpec.describe Backup::Repository do end it 'cleans existing repositories' do - wiki_repository_spy = spy(:wiki) - - allow_next_instance_of(ProjectWiki) do |project_wiki| - allow(project_wiki).to receive(:repository).and_return(wiki_repository_spy) - end - - expect_next_instance_of(Repository) do |repo| - expect(repo).to receive(:remove) + expect_next_instance_of(DesignManagement::Repository) do |repository| + expect(repository).to receive(:remove) end + expect(Repository).to receive(:new).twice.and_wrap_original do |method, *original_args| + repository = method.call(*original_args) - subject.restore - - expect(wiki_repository_spy).to have_received(:remove) - end - end + expect(repository).to receive(:remove) - describe '#empty_repo?' do - context 'for a wiki' do - let(:wiki) { create(:project_wiki) } - - it 'invalidates the emptiness cache' do - expect(wiki.repository).to receive(:expire_emptiness_caches).once - - subject.send(:empty_repo?, wiki) + repository end - context 'wiki repo has content' do - let!(:wiki_page) { create(:wiki_page, wiki: wiki) } - - it 'returns true, regardless of bad cache value' do - expect(subject.send(:empty_repo?, wiki)).to be(false) - end - end - - context 'wiki repo does not have content' do - it 'returns true, regardless of bad cache value' do - expect(subject.send(:empty_repo?, wiki)).to be_truthy - end - end + subject.restore end end end diff --git a/spec/lib/banzai/filter/design_reference_filter_spec.rb b/spec/lib/banzai/filter/design_reference_filter_spec.rb index 1b558754932..847c398964a 100644 --- a/spec/lib/banzai/filter/design_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/design_reference_filter_spec.rb @@ -74,26 +74,6 @@ RSpec.describe Banzai::Filter::DesignReferenceFilter do it_behaves_like 'a no-op filter' end - - context 'design reference filter is not enabled' do - before do - stub_feature_flags(described_class::FEATURE_FLAG => false) - end - - it_behaves_like 'a no-op filter' - - it 'issues no queries' do - expect { process(input_text) }.not_to exceed_query_limit(0) - end - end - - context 'the filter is enabled for the context project' do - before do - stub_feature_flags(described_class::FEATURE_FLAG => project) - end - - it_behaves_like 'a good link reference' - end end end diff --git a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb index e7b6c910b8a..35ef2abfa63 100644 --- a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Banzai::Filter::ExternalIssueReferenceFilter do include FilterSpecHelper + let_it_be_with_refind(:project) { create(:project) } + shared_examples_for "external issue tracker" do it_behaves_like 'a reference containing an element node' @@ -116,7 +118,7 @@ RSpec.describe Banzai::Filter::ExternalIssueReferenceFilter do end context "redmine project" do - let(:project) { create(:redmine_project) } + let_it_be(:service) { create(:redmine_service, project: project) } before do project.update!(issues_enabled: false) @@ -138,7 +140,7 @@ RSpec.describe Banzai::Filter::ExternalIssueReferenceFilter do end context "youtrack project" do - let(:project) { create(:youtrack_project) } + let_it_be(:service) { create(:youtrack_service, project: project) } before do project.update!(issues_enabled: false) @@ -181,7 +183,7 @@ RSpec.describe Banzai::Filter::ExternalIssueReferenceFilter do end context "jira project" do - let(:project) { create(:jira_project) } + let_it_be(:service) { create(:jira_service, project: project) } let(:reference) { issue.to_reference } context "with right markdown" do @@ -210,7 +212,7 @@ RSpec.describe Banzai::Filter::ExternalIssueReferenceFilter do end context "ewm project" do - let_it_be(:project) { create(:ewm_project) } + let_it_be(:service) { create(:ewm_service, project: project) } before do project.update!(issues_enabled: false) diff --git a/spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb b/spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb index 8bdb24ab08c..d29af311ee5 100644 --- a/spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb +++ b/spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Banzai::Filter::InlineGrafanaMetricsFilter do it_behaves_like 'a metrics embed filter' around do |example| - Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run } + travel_to(Time.utc(2019, 3, 17, 13, 10)) { example.run } end context 'when grafana is not configured' do diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 447802d18a7..4b8b575c1f0 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -296,6 +296,12 @@ RSpec.describe Banzai::Filter::IssueReferenceFilter do .to eq reference end + it 'link with trailing slash' do + doc = reference_filter("Fixed (#{issue_url + "/"}.)") + + expect(doc.to_html).to match(%r{\(<a.+>#{Regexp.escape(issue.to_reference(project))}</a>\.\)}) + end + it 'links with adjacent text' do doc = reference_filter("Fixed (#{reference}.)") diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 62b1711ee57..276fa7952be 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -5,9 +5,11 @@ require 'spec_helper' RSpec.describe Banzai::Filter::MilestoneReferenceFilter do include FilterSpecHelper - let(:parent_group) { create(:group, :public) } - let(:group) { create(:group, :public, parent: parent_group) } - let(:project) { create(:project, :public, group: group) } + let_it_be(:parent_group) { create(:group, :public) } + let_it_be(:group) { create(:group, :public, parent: parent_group) } + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:namespace) { create(:namespace) } + let_it_be(:another_project) { create(:project, :public, namespace: namespace) } it 'requires project context' do expect { described_class.call('') }.to raise_error(ArgumentError, /:project/) @@ -188,11 +190,9 @@ RSpec.describe Banzai::Filter::MilestoneReferenceFilter do end shared_examples 'cross-project / cross-namespace complete reference' do - let(:namespace) { create(:namespace) } - let(:another_project) { create(:project, :public, namespace: namespace) } - let(:milestone) { create(:milestone, project: another_project) } - let(:reference) { "#{another_project.full_path}%#{milestone.iid}" } - let!(:result) { reference_filter("See #{reference}") } + let_it_be(:milestone) { create(:milestone, project: another_project) } + let(:reference) { "#{another_project.full_path}%#{milestone.iid}" } + let!(:result) { reference_filter("See #{reference}") } it 'points to referenced project milestone page' do expect(result.css('a').first.attr('href')).to eq urls @@ -226,12 +226,10 @@ RSpec.describe Banzai::Filter::MilestoneReferenceFilter do end shared_examples 'cross-project / same-namespace complete reference' do - let(:namespace) { create(:namespace) } - let(:project) { create(:project, :public, namespace: namespace) } - let(:another_project) { create(:project, :public, namespace: namespace) } - let(:milestone) { create(:milestone, project: another_project) } - let(:reference) { "#{another_project.full_path}%#{milestone.iid}" } - let!(:result) { reference_filter("See #{reference}") } + let_it_be(:project) { create(:project, :public, namespace: namespace) } + let_it_be(:milestone) { create(:milestone, project: another_project) } + let(:reference) { "#{another_project.full_path}%#{milestone.iid}" } + let!(:result) { reference_filter("See #{reference}") } it 'points to referenced project milestone page' do expect(result.css('a').first.attr('href')).to eq urls @@ -265,12 +263,10 @@ RSpec.describe Banzai::Filter::MilestoneReferenceFilter do end shared_examples 'cross project shorthand reference' do - let(:namespace) { create(:namespace) } - let(:project) { create(:project, :public, namespace: namespace) } - let(:another_project) { create(:project, :public, namespace: namespace) } - let(:milestone) { create(:milestone, project: another_project) } - let(:reference) { "#{another_project.path}%#{milestone.iid}" } - let!(:result) { reference_filter("See #{reference}") } + let_it_be(:project) { create(:project, :public, namespace: namespace) } + let_it_be(:milestone) { create(:milestone, project: another_project) } + let(:reference) { "#{another_project.path}%#{milestone.iid}" } + let!(:result) { reference_filter("See #{reference}") } it 'points to referenced project milestone page' do expect(result.css('a').first.attr('href')).to eq urls @@ -439,13 +435,13 @@ RSpec.describe Banzai::Filter::MilestoneReferenceFilter do context 'when milestone is open' do context 'project milestones' do - let(:milestone) { create(:milestone, project: project) } + let_it_be_with_reload(:milestone) { create(:milestone, project: project) } include_context 'project milestones' end context 'group milestones' do - let(:milestone) { create(:milestone, group: group) } + let_it_be_with_reload(:milestone) { create(:milestone, group: group) } include_context 'group milestones' end @@ -453,13 +449,13 @@ RSpec.describe Banzai::Filter::MilestoneReferenceFilter do context 'when milestone is closed' do context 'project milestones' do - let(:milestone) { create(:milestone, :closed, project: project) } + let_it_be_with_reload(:milestone) { create(:milestone, :closed, project: project) } include_context 'project milestones' end context 'group milestones' do - let(:milestone) { create(:milestone, :closed, group: group) } + let_it_be_with_reload(:milestone) { create(:milestone, :closed, group: group) } include_context 'group milestones' end diff --git a/spec/lib/feature/definition_spec.rb b/spec/lib/feature/definition_spec.rb index 49224cf4279..fa0207d829a 100644 --- a/spec/lib/feature/definition_spec.rb +++ b/spec/lib/feature/definition_spec.rb @@ -105,6 +105,7 @@ RSpec.describe Feature::Definition do describe '.load_all!' do let(:store1) { Dir.mktmpdir('path1') } let(:store2) { Dir.mktmpdir('path2') } + let(:definitions) { {} } before do allow(described_class).to receive(:paths).and_return( @@ -115,28 +116,30 @@ RSpec.describe Feature::Definition do ) end + subject { described_class.send(:load_all!) } + it "when there's no feature flags a list of definitions is empty" do - expect(described_class.load_all!).to be_empty + is_expected.to be_empty end it "when there's a single feature flag it properly loads them" do write_feature_flag(store1, path, yaml_content) - expect(described_class.load_all!).to be_one + is_expected.to be_one end it "when the same feature flag is stored multiple times raises exception" do write_feature_flag(store1, path, yaml_content) write_feature_flag(store2, path, yaml_content) - expect { described_class.load_all! } + expect { subject } .to raise_error(/Feature flag 'feature_flag' is already defined/) end it "when one of the YAMLs is invalid it does raise exception" do write_feature_flag(store1, path, '{}') - expect { described_class.load_all! } + expect { subject } .to raise_error(/Feature flag is missing name/) end diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index acd7d97ac85..5dff9dbd995 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Feature, stub_feature_flags: false do before do # reset Flipper AR-engine Feature.reset + skip_feature_flags_yaml_validation end describe '.get' do @@ -253,6 +254,9 @@ RSpec.describe Feature, stub_feature_flags: false do end before do + stub_env('LAZILY_CREATE_FEATURE_FLAG', '0') + + allow(Feature::Definition).to receive(:valid_usage!).and_call_original allow(Feature::Definition).to receive(:definitions) do { definition.key => definition } end diff --git a/spec/lib/gitlab/alert_management/alert_params_spec.rb b/spec/lib/gitlab/alert_management/alert_params_spec.rb deleted file mode 100644 index c3171be5e29..00000000000 --- a/spec/lib/gitlab/alert_management/alert_params_spec.rb +++ /dev/null @@ -1,101 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::AlertManagement::AlertParams do - let_it_be(:project) { create(:project, :repository, :private) } - - describe '.from_generic_alert' do - let(:started_at) { Time.current.change(usec: 0).rfc3339 } - let(:default_payload) do - { - 'title' => 'Alert title', - 'description' => 'Description', - 'monitoring_tool' => 'Monitoring tool name', - 'service' => 'Service', - 'hosts' => ['gitlab.com'], - 'start_time' => started_at, - 'some' => { 'extra' => { 'payload' => 'here' } } - } - end - - let(:payload) { default_payload } - - subject { described_class.from_generic_alert(project: project, payload: payload) } - - it 'returns Alert compatible parameters' do - is_expected.to eq( - project_id: project.id, - title: 'Alert title', - description: 'Description', - monitoring_tool: 'Monitoring tool name', - service: 'Service', - severity: 'critical', - hosts: ['gitlab.com'], - payload: payload, - started_at: started_at, - ended_at: nil, - fingerprint: nil, - environment: nil - ) - end - - context 'when severity given' do - let(:payload) { default_payload.merge(severity: 'low') } - - it 'returns Alert compatible parameters' do - expect(subject[:severity]).to eq('low') - end - end - - context 'when there are no hosts in the payload' do - let(:payload) { {} } - - it 'hosts param is an empty array' do - expect(subject[:hosts]).to be_empty - end - end - end - - describe '.from_prometheus_alert' do - let(:payload) do - { - 'status' => 'firing', - 'labels' => { - 'alertname' => 'GitalyFileServerDown', - 'channel' => 'gitaly', - 'pager' => 'pagerduty', - 'severity' => 's1' - }, - 'annotations' => { - 'description' => 'Alert description', - 'runbook' => 'troubleshooting/gitaly-down.md', - 'title' => 'Alert title' - }, - 'startsAt' => '2020-04-27T10:10:22.265949279Z', - 'endsAt' => '0001-01-01T00:00:00Z', - 'generatorURL' => 'http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1', - 'fingerprint' => 'b6ac4d42057c43c1' - } - end - - let(:parsed_alert) { Gitlab::Alerting::Alert.new(project: project, payload: payload) } - - subject { described_class.from_prometheus_alert(project: project, parsed_alert: parsed_alert) } - - it 'returns Alert-compatible params' do - is_expected.to eq( - project_id: project.id, - title: 'Alert title', - description: 'Alert description', - monitoring_tool: 'Prometheus', - payload: payload, - started_at: parsed_alert.starts_at, - ended_at: parsed_alert.ends_at, - fingerprint: parsed_alert.gitlab_fingerprint, - environment: parsed_alert.environment, - prometheus_alert: parsed_alert.gitlab_alert - ) - end - end -end diff --git a/spec/lib/gitlab/alert_management/payload/generic_spec.rb b/spec/lib/gitlab/alert_management/payload/generic_spec.rb index 538a822503e..b7660462b0d 100644 --- a/spec/lib/gitlab/alert_management/payload/generic_spec.rb +++ b/spec/lib/gitlab/alert_management/payload/generic_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Gitlab::AlertManagement::Payload::Generic do subject { parsed_payload.starts_at } around do |example| - Timecop.freeze(current_time) { example.run } + travel_to(current_time) { example.run } end context 'without start_time' do @@ -86,4 +86,34 @@ RSpec.describe Gitlab::AlertManagement::Payload::Generic do it_behaves_like 'parsable alert payload field', 'gitlab_environment_name' end + + describe '#description' do + subject { parsed_payload.description } + + it_behaves_like 'parsable alert payload field', 'description' + end + + describe '#ends_at' do + let(:current_time) { Time.current.change(usec: 0).utc } + + subject { parsed_payload.ends_at } + + around do |example| + travel_to(current_time) { example.run } + end + + context 'without end_time' do + it { is_expected.to be_nil } + end + + context "with end_time" do + let(:value) { 10.minutes.ago.change(usec: 0).utc } + + before do + raw_payload['end_time'] = value.to_s + end + + it { is_expected.to eq(value) } + end + end end diff --git a/spec/lib/gitlab/alerting/alert_spec.rb b/spec/lib/gitlab/alerting/alert_spec.rb deleted file mode 100644 index b53b71e3f3e..00000000000 --- a/spec/lib/gitlab/alerting/alert_spec.rb +++ /dev/null @@ -1,299 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Alerting::Alert do - let_it_be(:project) { create(:project) } - - let(:alert) { build(:alerting_alert, project: project, payload: payload) } - let(:payload) { {} } - - shared_context 'gitlab alert' do - let!(:gitlab_alert) { create(:prometheus_alert, project: project) } - let(:gitlab_alert_id) { gitlab_alert.id } - - before do - payload['labels'] = { - 'gitlab_alert_id' => gitlab_alert.prometheus_metric_id.to_s, - 'gitlab_prometheus_alert_id' => gitlab_alert_id - } - end - end - - shared_context 'full query' do - before do - payload['generatorURL'] = 'http://localhost:9090/graph?g0.expr=vector%281%29' - end - end - - shared_examples 'invalid alert' do - it 'is invalid' do - expect(alert).not_to be_valid - end - end - - shared_examples 'parse payload' do |*pairs| - context 'without payload' do - it { is_expected.to be_nil } - end - - pairs.each do |pair| - context "with #{pair}" do - let(:value) { 'some value' } - - before do - section, name = pair.split('/') - payload[section] = { name => value } - end - - it { is_expected.to eq(value) } - end - end - end - - describe '#gitlab_alert' do - subject { alert.gitlab_alert } - - context 'without payload' do - it { is_expected.to be_nil } - end - - context 'with gitlab alert' do - include_context 'gitlab alert' - - it { is_expected.to eq(gitlab_alert) } - end - - context 'with unknown gitlab alert' do - include_context 'gitlab alert' do - let(:gitlab_alert_id) { 'unknown' } - end - - it { is_expected.to be_nil } - end - - context 'when two alerts with the same metric exist' do - include_context 'gitlab alert' - - let!(:second_gitlab_alert) do - create(:prometheus_alert, - project: project, - prometheus_metric_id: gitlab_alert.prometheus_metric_id - ) - end - - context 'alert id given in params' do - before do - payload['labels'] = { - 'gitlab_alert_id' => gitlab_alert.prometheus_metric_id.to_s, - 'gitlab_prometheus_alert_id' => second_gitlab_alert.id - } - end - - it { is_expected.to eq(second_gitlab_alert) } - end - - context 'metric id given in params' do - # This tests the case when two alerts are found, as metric id - # is not unique. - - # Note the metric id was incorrectly named as 'gitlab_alert_id' - # in PrometheusAlert#to_param. - before do - payload['labels'] = { 'gitlab_alert_id' => gitlab_alert.prometheus_metric_id } - end - - it { is_expected.to be_nil } - end - end - end - - describe '#title' do - subject { alert.title } - - it_behaves_like 'parse payload', - 'annotations/title', - 'annotations/summary', - 'labels/alertname' - - context 'with gitlab alert' do - include_context 'gitlab alert' - - context 'with annotations/title' do - let(:value) { 'annotation title' } - - before do - payload['annotations'] = { 'title' => value } - end - - it { is_expected.to eq(gitlab_alert.title) } - end - end - end - - describe '#description' do - subject { alert.description } - - it_behaves_like 'parse payload', 'annotations/description' - end - - describe '#annotations' do - subject { alert.annotations } - - context 'without payload' do - it { is_expected.to eq([]) } - end - - context 'with payload' do - before do - payload['annotations'] = { 'foo' => 'value1', 'bar' => 'value2' } - end - - it 'parses annotations' do - expect(subject.size).to eq(2) - expect(subject.map(&:label)).to eq(%w[foo bar]) - expect(subject.map(&:value)).to eq(%w[value1 value2]) - end - end - end - - describe '#environment' do - subject { alert.environment } - - context 'without gitlab_alert' do - it { is_expected.to be_nil } - end - - context 'with gitlab alert' do - include_context 'gitlab alert' - - it { is_expected.to eq(gitlab_alert.environment) } - end - end - - describe '#starts_at' do - subject { alert.starts_at } - - context 'with empty startsAt' do - before do - payload['startsAt'] = nil - end - - it { is_expected.to be_nil } - end - - context 'with invalid startsAt' do - before do - payload['startsAt'] = 'invalid' - end - - it { is_expected.to be_nil } - end - - context 'with payload' do - let(:time) { Time.current.change(usec: 0) } - - before do - payload['startsAt'] = time.rfc3339 - end - - it { is_expected.to eq(time) } - end - end - - describe '#full_query' do - using RSpec::Parameterized::TableSyntax - - subject { alert.full_query } - - where(:generator_url, :expected_query) do - nil | nil - 'http://localhost' | nil - 'invalid url' | nil - 'http://localhost:9090/graph?g1.expr=vector%281%29' | nil - 'http://localhost:9090/graph?g0.expr=vector%281%29' | 'vector(1)' - end - - with_them do - before do - payload['generatorURL'] = generator_url - end - - it { is_expected.to eq(expected_query) } - end - - context 'with gitlab alert' do - include_context 'gitlab alert' - include_context 'full query' - - it { is_expected.to eq(gitlab_alert.full_query) } - end - end - - describe '#y_label' do - subject { alert.y_label } - - it_behaves_like 'parse payload', 'annotations/gitlab_y_label' - - context 'when y_label is not included in the payload' do - it_behaves_like 'parse payload', 'annotations/title' - end - end - - describe '#alert_markdown' do - subject { alert.alert_markdown } - - it_behaves_like 'parse payload', 'annotations/gitlab_incident_markdown' - end - - describe '#gitlab_fingerprint' do - subject { alert.gitlab_fingerprint } - - context 'when the alert is a GitLab managed alert' do - include_context 'gitlab alert' - - it 'returns a fingerprint' do - plain_fingerprint = [alert.metric_id, alert.starts_at_raw].join('/') - - is_expected.to eq(Digest::SHA1.hexdigest(plain_fingerprint)) - end - end - - context 'when the alert is from self managed Prometheus' do - include_context 'full query' - - it 'returns a fingerprint' do - plain_fingerprint = [alert.starts_at_raw, alert.title, alert.full_query].join('/') - - is_expected.to eq(Digest::SHA1.hexdigest(plain_fingerprint)) - end - end - end - - describe '#valid?' do - before do - payload.update( - 'annotations' => { 'title' => 'some title' }, - 'startsAt' => Time.current.rfc3339 - ) - end - - subject { alert } - - it { is_expected.to be_valid } - - context 'without project' do - let(:project) { nil } - - it { is_expected.not_to be_valid } - end - - context 'without starts_at' do - before do - payload['startsAt'] = nil - end - - it { is_expected.not_to be_valid } - end - end -end diff --git a/spec/lib/gitlab/alerting/notification_payload_parser_spec.rb b/spec/lib/gitlab/alerting/notification_payload_parser_spec.rb deleted file mode 100644 index ff5ab1116fa..00000000000 --- a/spec/lib/gitlab/alerting/notification_payload_parser_spec.rb +++ /dev/null @@ -1,204 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Alerting::NotificationPayloadParser do - let_it_be(:project) { build(:project) } - - describe '.call' do - let(:starts_at) { Time.current.change(usec: 0) } - let(:ends_at) { Time.current.change(usec: 0) } - let(:payload) do - { - 'title' => 'alert title', - 'start_time' => starts_at.rfc3339, - 'end_time' => ends_at.rfc3339, - 'description' => 'Description', - 'monitoring_tool' => 'Monitoring tool name', - 'service' => 'Service', - 'hosts' => ['gitlab.com'], - 'severity' => 'low' - } - end - - subject { described_class.call(payload, project) } - - it 'returns Prometheus-like payload' do - is_expected.to eq( - { - 'annotations' => { - 'title' => 'alert title', - 'description' => 'Description', - 'monitoring_tool' => 'Monitoring tool name', - 'service' => 'Service', - 'hosts' => ['gitlab.com'], - 'severity' => 'low' - }, - 'startsAt' => starts_at.rfc3339, - 'endsAt' => ends_at.rfc3339 - } - ) - end - - context 'when title is blank' do - before do - payload[:title] = '' - end - - it 'sets a predefined title' do - expect(subject.dig('annotations', 'title')).to eq('New: Incident') - end - end - - context 'when hosts attribute is a string' do - before do - payload[:hosts] = 'gitlab.com' - end - - it 'returns hosts as an array of one element' do - expect(subject.dig('annotations', 'hosts')).to eq(['gitlab.com']) - end - end - - context 'when the time is in unsupported format' do - before do - payload[:start_time] = 'invalid/date/format' - end - - it 'sets startsAt to a current time in RFC3339 format' do - expect(subject['startsAt']).to eq(starts_at.rfc3339) - end - end - - context 'when payload is blank' do - let(:payload) { {} } - - it 'returns default parameters' do - is_expected.to match( - 'annotations' => { - 'title' => described_class::DEFAULT_TITLE, - 'severity' => described_class::DEFAULT_SEVERITY - }, - 'startsAt' => starts_at.rfc3339 - ) - end - - context 'when severity is blank' do - before do - payload[:severity] = '' - end - - it 'sets severity to the default ' do - expect(subject.dig('annotations', 'severity')).to eq(described_class::DEFAULT_SEVERITY) - end - end - end - - context 'with fingerprint' do - before do - payload[:fingerprint] = data - end - - shared_examples 'fingerprint generation' do - it 'generates the fingerprint correctly' do - expect(result).to eq(Gitlab::AlertManagement::Fingerprint.generate(data)) - end - end - - context 'with blank fingerprint' do - it_behaves_like 'fingerprint generation' do - let(:data) { ' ' } - let(:result) { subject.dig('annotations', 'fingerprint') } - end - end - - context 'with fingerprint given' do - it_behaves_like 'fingerprint generation' do - let(:data) { 'fingerprint' } - let(:result) { subject.dig('annotations', 'fingerprint') } - end - end - - context 'with array fingerprint given' do - it_behaves_like 'fingerprint generation' do - let(:data) { [1, 'fingerprint', 'given'] } - let(:result) { subject.dig('annotations', 'fingerprint') } - end - end - end - - context 'with environment' do - let(:environment) { create(:environment, project: project) } - - before do - payload[:gitlab_environment_name] = environment.name - end - - it 'sets the environment ' do - expect(subject.dig('annotations', 'environment')).to eq(environment) - end - end - - context 'when payload attributes have blank lines' do - let(:payload) do - { - 'title' => '', - 'start_time' => '', - 'end_time' => '', - 'description' => '', - 'monitoring_tool' => '', - 'service' => '', - 'hosts' => [''] - } - end - - it 'returns default parameters' do - is_expected.to eq( - 'annotations' => { - 'title' => 'New: Incident', - 'severity' => described_class::DEFAULT_SEVERITY - }, - 'startsAt' => starts_at.rfc3339 - ) - end - end - - context 'when payload has secondary params' do - let(:payload) do - { - 'description' => 'Description', - 'additional' => { - 'params' => { - '1' => 'Some value 1', - '2' => 'Some value 2', - 'blank' => '' - } - } - } - end - - it 'adds secondary params to annotations' do - is_expected.to eq( - 'annotations' => { - 'title' => 'New: Incident', - 'severity' => described_class::DEFAULT_SEVERITY, - 'description' => 'Description', - 'additional.params.1' => 'Some value 1', - 'additional.params.2' => 'Some value 2' - }, - 'startsAt' => starts_at.rfc3339 - ) - end - end - - context 'when secondary params hash is too big' do - before do - allow(Gitlab::Utils::SafeInlineHash).to receive(:merge_keys!).and_raise(ArgumentError) - end - - it 'catches and re-raises an error' do - expect { subject }.to raise_error Gitlab::Alerting::NotificationPayloadParser::BadPayloadError, 'The payload is too big' - end - end - end -end diff --git a/spec/lib/gitlab/analytics/unique_visits_spec.rb b/spec/lib/gitlab/analytics/unique_visits_spec.rb index 1432c9ac58f..6ac58e13f4c 100644 --- a/spec/lib/gitlab/analytics/unique_visits_spec.rb +++ b/spec/lib/gitlab/analytics/unique_visits_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state # Without freezing the time, the test may behave inconsistently # depending on which day of the week test is run. reference_time = Time.utc(2020, 6, 1) - Timecop.freeze(reference_time) { example.run } + travel_to(reference_time) { example.run } end describe '#track_visit' do diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index 1ac8ebe1369..2ebde145bfd 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -419,10 +419,30 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect(find_user_from_web_access_token(:ics)).to eq(user) end - it 'returns the user for API requests' do - set_header('SCRIPT_NAME', '/api/endpoint') + context 'for API requests' do + it 'returns the user' do + set_header('SCRIPT_NAME', '/api/endpoint') + + expect(find_user_from_web_access_token(:api)).to eq(user) + end + + it 'returns nil if URL does not start with /api/' do + set_header('SCRIPT_NAME', '/relative_root/api/endpoint') + + expect(find_user_from_web_access_token(:api)).to be_nil + end - expect(find_user_from_web_access_token(:api)).to eq(user) + context 'when relative_url_root is set' do + before do + stub_config_setting(relative_url_root: '/relative_root') + end + + it 'returns the user' do + set_header('SCRIPT_NAME', '/relative_root/api/endpoint') + + expect(find_user_from_web_access_token(:api)).to eq(user) + end + end end end diff --git a/spec/lib/gitlab/auth/current_user_mode_spec.rb b/spec/lib/gitlab/auth/current_user_mode_spec.rb index 60b403780c0..ffd7813190a 100644 --- a/spec/lib/gitlab/auth/current_user_mode_spec.rb +++ b/spec/lib/gitlab/auth/current_user_mode_spec.rb @@ -121,7 +121,7 @@ RSpec.describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_ subject.enable_admin_mode!(password: user.password) expect(subject.admin_mode?).to be(true), 'admin mode is not active in the present' - Timecop.freeze(Gitlab::Auth::CurrentUserMode::MAX_ADMIN_MODE_TIME.from_now) do + travel_to(Gitlab::Auth::CurrentUserMode::MAX_ADMIN_MODE_TIME.from_now) do # in the future this will be a new request, simulate by clearing the RequestStore Gitlab::SafeRequestStore.clear! diff --git a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb index a3840e3a22e..85a9c88ebff 100644 --- a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb +++ b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb @@ -73,7 +73,7 @@ RSpec.describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgre described_class.new.perform - expect(Feature.enabled?(:multiple_merge_request_assignees)).to eq(true) + expect(Feature.enabled?(:multiple_merge_request_assignees, type: :licensed)).to eq(true) end end diff --git a/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb index db3cbe7ccdc..3cec5cb4c35 100644 --- a/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb @@ -82,21 +82,4 @@ RSpec.describe Gitlab::BackgroundMigration::MigrateUsersBioToUserDetails, :migra expect(user_detail).to be_nil end - - context 'when `migrate_bio_to_user_details` feature flag is off' do - before do - stub_feature_flags(migrate_bio_to_user_details: false) - end - - it 'does nothing' do - already_existing_user_details = user_details.where(user_id: [ - user_has_different_details.id, - user_already_has_details.id - ]) - - subject - - expect(user_details.all).to match_array(already_existing_user_details) - end - end end diff --git a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb index 392b44d1a1f..2dae4a65eeb 100644 --- a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb +++ b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb @@ -74,14 +74,14 @@ RSpec.describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMent let(:user_mentions) { merge_request_user_mentions } let(:resource) { merge_request } - it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, MergeRequest + it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, 'MergeRequest' context 'when FF disabled' do before do stub_feature_flags(migrate_user_mentions: false) end - it_behaves_like 'resource migration not run', MigrateMergeRequestMentionsToDb, MergeRequest + it_behaves_like 'resource migration not run', MigrateMergeRequestMentionsToDb, 'MergeRequest' end end @@ -103,14 +103,14 @@ RSpec.describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMent let(:user_mentions) { commit_user_mentions } let(:resource) { commit } - it_behaves_like 'resource notes mentions migration', MigrateCommitNotesMentionsToDb, Commit + it_behaves_like 'resource notes mentions migration', MigrateCommitNotesMentionsToDb, 'Commit' context 'when FF disabled' do before do stub_feature_flags(migrate_user_mentions: false) end - it_behaves_like 'resource notes migration not run', MigrateCommitNotesMentionsToDb, Commit + it_behaves_like 'resource notes migration not run', MigrateCommitNotesMentionsToDb, 'Commit' end end end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index d4483bf1754..b723c31c4aa 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -312,7 +312,7 @@ RSpec.describe Gitlab::BitbucketImport::Importer do # attributes later. existing_label.reload - Timecop.freeze(Time.now + 1.minute) do + travel_to(Time.now + 1.minute) do importer.execute label_after_import = project.labels.find(existing_label.id) diff --git a/spec/lib/gitlab/checks/matching_merge_request_spec.rb b/spec/lib/gitlab/checks/matching_merge_request_spec.rb new file mode 100644 index 00000000000..ca7ee784ee3 --- /dev/null +++ b/spec/lib/gitlab/checks/matching_merge_request_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Checks::MatchingMergeRequest do + describe '#match?' do + let_it_be(:newrev) { '012345678' } + let_it_be(:target_branch) { 'feature' } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:locked_merge_request) do + create(:merge_request, + :locked, + source_project: project, + target_project: project, + target_branch: target_branch, + in_progress_merge_commit_sha: newrev) + end + + subject { described_class.new(newrev, target_branch, project) } + + it 'matches a merge request' do + expect(subject.match?).to be true + end + + it 'does not match any merge request' do + matcher = described_class.new(newrev, 'test', project) + + expect(matcher.match?).to be false + end + end +end diff --git a/spec/lib/gitlab/ci/ansi2json/line_spec.rb b/spec/lib/gitlab/ci/ansi2json/line_spec.rb index 8b1cd812a70..d681447a0e8 100644 --- a/spec/lib/gitlab/ci/ansi2json/line_spec.rb +++ b/spec/lib/gitlab/ci/ansi2json/line_spec.rb @@ -58,6 +58,15 @@ RSpec.describe Gitlab::Ci::Ansi2json::Line do end end + describe '#set_section_options' do + it 'sets the current section\'s options' do + options = { collapsed: true } + subject.set_section_options(options) + + expect(subject.to_h[:section_options]).to eq(options) + end + end + describe '#set_as_section_header' do it 'change the section_header to true' do expect { subject.set_as_section_header } diff --git a/spec/lib/gitlab/ci/ansi2json_spec.rb b/spec/lib/gitlab/ci/ansi2json_spec.rb index cb6949fddc2..c9c0d1a744e 100644 --- a/spec/lib/gitlab/ci/ansi2json_spec.rb +++ b/spec/lib/gitlab/ci/ansi2json_spec.rb @@ -229,7 +229,7 @@ RSpec.describe Gitlab::Ci::Ansi2json do expect(convert_json(trace)).to eq([ { offset: 0, - content: [{ text: "section_end:1:2<div>hello</div>" }], + content: [{ text: 'section_end:1:2<div>hello</div>' }], section: 'prepare-script', section_header: true }, @@ -329,6 +329,32 @@ RSpec.describe Gitlab::Ci::Ansi2json do ]) end end + + context 'with section options' do + let(:option_section_start) { "section_start:#{section_start_time.to_i}:#{section_name}[collapsed=true,unused_option=123]\r\033[0K"} + + it 'provides section options when set' do + trace = "#{option_section_start}hello#{section_end}" + expect(convert_json(trace)).to eq([ + { + offset: 0, + content: [{ text: 'hello' }], + section: 'prepare-script', + section_header: true, + section_options: { + 'collapsed' => 'true', + 'unused_option' => '123' + } + }, + { + offset: 83, + content: [], + section: 'prepare-script', + section_duration: '01:03' + } + ]) + end + end end describe 'incremental updates' do @@ -339,7 +365,7 @@ RSpec.describe Gitlab::Ci::Ansi2json do context 'with split word' do let(:pre_text) { "\e[1mHello " } - let(:text) { "World" } + let(:text) { 'World' } let(:lines) do [ @@ -355,7 +381,7 @@ RSpec.describe Gitlab::Ci::Ansi2json do context 'with split word on second line' do let(:pre_text) { "Good\nmorning " } - let(:text) { "World" } + let(:text) { 'World' } let(:lines) do [ @@ -514,7 +540,7 @@ RSpec.describe Gitlab::Ci::Ansi2json do end describe 'trucates' do - let(:text) { "Hello World" } + let(:text) { 'Hello World' } let(:stream) { StringIO.new(text) } let(:subject) { described_class.convert(stream) } @@ -522,11 +548,11 @@ RSpec.describe Gitlab::Ci::Ansi2json do stream.seek(3, IO::SEEK_SET) end - it "returns truncated output" do + it 'returns truncated output' do expect(subject.truncated).to be_truthy end - it "does not append output" do + it 'does not append output' do expect(subject.append).to be_falsey end end diff --git a/spec/lib/gitlab/ci/config/entry/product/matrix_spec.rb b/spec/lib/gitlab/ci/config/entry/product/matrix_spec.rb index 39697884e3b..3388ae0af2f 100644 --- a/spec/lib/gitlab/ci/config/entry/product/matrix_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/product/matrix_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' require_dependency 'active_model' RSpec.describe ::Gitlab::Ci::Config::Entry::Product::Matrix do @@ -46,33 +46,140 @@ RSpec.describe ::Gitlab::Ci::Config::Entry::Product::Matrix do end end - context 'when entry config has only one variable' do - let(:config) do - [ - { - 'VAR_1' => %w[test] - } - ] + context 'with one_dimensional_matrix feature flag enabled' do + before do + stub_feature_flags(one_dimensional_matrix: true) + matrix.compose! end - describe '#valid?' do - it { is_expected.not_to be_valid } - end + context 'when entry config has only one variable with multiple values' do + let(:config) do + [ + { + 'VAR_1' => %w[build test] + } + ] + end - describe '#errors' do - it 'returns error about too many jobs' do - expect(matrix.errors) - .to include('variables config requires at least 2 items') + describe '#valid?' do + it { is_expected.to be_valid } + end + + describe '#errors' do + it 'returns no errors' do + expect(matrix.errors) + .to be_empty + end + end + + describe '#value' do + before do + matrix.compose! + end + + it 'returns the value without raising an error' do + expect(matrix.value).to eq([{ 'VAR_1' => %w[build test] }]) + end end + + context 'when entry config has only one variable with one value' do + let(:config) do + [ + { + 'VAR_1' => %w[test] + } + ] + end + + describe '#valid?' do + it { is_expected.to be_valid } + end + + describe '#errors' do + it 'returns no errors' do + expect(matrix.errors) + .to be_empty + end + end + + describe '#value' do + before do + matrix.compose! + end + + it 'returns the value without raising an error' do + expect(matrix.value).to eq([{ 'VAR_1' => %w[test] }]) + end + end + end + end + end + + context 'with one_dimensional_matrix feature flag disabled' do + before do + stub_feature_flags(one_dimensional_matrix: false) + matrix.compose! end - describe '#value' do - before do - matrix.compose! + context 'when entry config has only one variable with multiple values' do + let(:config) do + [ + { + 'VAR_1' => %w[build test] + } + ] end - it 'returns the value without raising an error' do - expect(matrix.value).to eq([{ 'VAR_1' => ['test'] }]) + describe '#valid?' do + it { is_expected.not_to be_valid } + end + + describe '#errors' do + it 'returns error about too many jobs' do + expect(matrix.errors) + .to include('variables config requires at least 2 items') + end + end + + describe '#value' do + before do + matrix.compose! + end + + it 'returns the value without raising an error' do + expect(matrix.value).to eq([{ 'VAR_1' => %w[build test] }]) + end + end + + context 'when entry config has only one variable with one value' do + let(:config) do + [ + { + 'VAR_1' => %w[test] + } + ] + end + + describe '#valid?' do + it { is_expected.not_to be_valid } + end + + describe '#errors' do + it 'returns no errors' do + expect(matrix.errors) + .to include('variables config requires at least 2 items') + end + end + + describe '#value' do + before do + matrix.compose! + end + + it 'returns the value without raising an error' do + expect(matrix.value).to eq([{ 'VAR_1' => %w[test] }]) + end + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/product/variables_spec.rb b/spec/lib/gitlab/ci/config/entry/product/variables_spec.rb index 230b001d620..407efb438b5 100644 --- a/spec/lib/gitlab/ci/config/entry/product/variables_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/product/variables_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require 'fast_spec_helper' +# After Feature one_dimensional_matrix is removed, this can be changed back to fast_spec_helper +require 'spec_helper' require_dependency 'active_model' RSpec.describe Gitlab::Ci::Config::Entry::Product::Variables do @@ -45,43 +46,71 @@ RSpec.describe Gitlab::Ci::Config::Entry::Product::Variables do end end - context 'when entry value is not correct' do - shared_examples 'invalid variables' do |message| - describe '#errors' do - it 'saves errors' do - expect(entry.errors).to include(message) - end + context 'with one_dimensional_matrix feature flag enabled' do + context 'with only one variable' do + before do + stub_feature_flags(one_dimensional_matrix: true) end + let(:config) { { VAR: 'test' } } describe '#valid?' do - it 'is not valid' do - expect(entry).not_to be_valid + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#errors' do + it 'does not append errors' do + expect(entry.errors).to be_empty end end end + end - context 'with array' do - let(:config) { [:VAR, 'test'] } + context 'with one_dimensional_matrix feature flag disabled' do + context 'when entry value is not correct' do + before do + stub_feature_flags(one_dimensional_matrix: false) + end + shared_examples 'invalid variables' do |message| + describe '#errors' do + it 'saves errors' do + expect(entry.errors).to include(message) + end + end - it_behaves_like 'invalid variables', /should be a hash of key value pairs/ - end + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + end - context 'with empty array' do - let(:config) { { VAR: 'test', VAR2: [] } } + context 'with array' do + let(:config) { [:VAR, 'test'] } - it_behaves_like 'invalid variables', /should be a hash of key value pairs/ - end + it_behaves_like 'invalid variables', /should be a hash of key value pairs/ + end - context 'with nested array' do - let(:config) { { VAR: 'test', VAR2: [1, [2]] } } + context 'with empty array' do + let(:config) { { VAR: 'test', VAR2: [] } } - it_behaves_like 'invalid variables', /should be a hash of key value pairs/ - end + it_behaves_like 'invalid variables', /should be a hash of key value pairs/ + end - context 'with only one variable' do - let(:config) { { VAR: 'test' } } + context 'with nested array' do + let(:config) { { VAR: 'test', VAR2: [1, [2]] } } + + it_behaves_like 'invalid variables', /should be a hash of key value pairs/ + end - it_behaves_like 'invalid variables', /variables config requires at least 2 items/ + context 'with one_dimensional_matrix feature flag disabled' do + context 'with only one variable' do + let(:config) { { VAR: 'test' } } + + it_behaves_like 'invalid variables', /variables config requires at least 2 items/ + end + end end end end diff --git a/spec/lib/gitlab/ci/cron_parser_spec.rb b/spec/lib/gitlab/ci/cron_parser_spec.rb index f724825a9cc..dd27b4045c9 100644 --- a/spec/lib/gitlab/ci/cron_parser_spec.rb +++ b/spec/lib/gitlab/ci/cron_parser_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when PST (Pacific Standard Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 1, 1)) do + travel_to(Time.utc(2017, 1, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -90,7 +90,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when PDT (Pacific Daylight Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 6, 1)) do + travel_to(Time.utc(2017, 6, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -117,7 +117,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when CET (Central European Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 1, 1)) do + travel_to(Time.utc(2017, 1, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -125,7 +125,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when CEST (Central European Summer Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 6, 1)) do + travel_to(Time.utc(2017, 6, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -152,7 +152,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when EST (Eastern Standard Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 1, 1)) do + travel_to(Time.utc(2017, 1, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -160,7 +160,7 @@ RSpec.describe Gitlab::Ci::CronParser do context 'when EDT (Eastern Daylight Time)' do it 'converts time in server time zone' do - Timecop.freeze(Time.utc(2017, 6, 1)) do + travel_to(Time.utc(2017, 6, 1)) do expect(subject.hour).to eq(hour_in_utc) end end @@ -174,7 +174,7 @@ RSpec.describe Gitlab::Ci::CronParser do # (e.g. America/Chicago) at the start of the test. Stubbing # TZ doesn't appear to be enough. it 'generates day without TZInfo::AmbiguousTime error' do - Timecop.freeze(Time.utc(2020, 1, 1)) do + travel_to(Time.utc(2020, 1, 1)) do expect(subject.year).to eq(year) expect(subject.month).to eq(12) expect(subject.day).to eq(1) diff --git a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb index 1f497dea2bf..5a3a9b53da6 100644 --- a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb +++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb @@ -4,7 +4,7 @@ require 'fast_spec_helper' RSpec.describe Gitlab::Ci::Parsers::Test::Junit do describe '#parse!' do - subject { described_class.new.parse!(junit, test_suite, args) } + subject { described_class.new.parse!(junit, test_suite, **args) } let(:test_suite) { Gitlab::Ci::Reports::TestSuite.new('rspec') } let(:test_cases) { flattened_test_cases(test_suite) } diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 34df0e86a18..0b961336f3f 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do - let(:project) { create(:project, :repository) } - let(:head_sha) { project.repository.head_commit.id } - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: head_sha) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:head_sha) { project.repository.head_commit.id } + let(:pipeline) { build(:ci_empty_pipeline, project: project, sha: head_sha) } let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } } let(:previous_stages) { [] } @@ -503,7 +503,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do using RSpec::Parameterized let(:pipeline) do - build(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source) + build(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source, project: project) end context 'matches' do @@ -766,7 +766,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do context 'with a matching changes: rule' do let(:pipeline) do - create(:ci_pipeline, project: project).tap do |pipeline| + build(:ci_pipeline, project: project).tap do |pipeline| stub_pipeline_modified_paths(pipeline, %w[app/models/ci/pipeline.rb spec/models/ci/pipeline_spec.rb .gitlab-ci.yml]) end end diff --git a/spec/lib/gitlab/ci/runner/backoff_spec.rb b/spec/lib/gitlab/ci/runner/backoff_spec.rb new file mode 100644 index 00000000000..f147d69f7cd --- /dev/null +++ b/spec/lib/gitlab/ci/runner/backoff_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' +require 'active_support/testing/time_helpers' + +RSpec.describe Gitlab::Ci::Runner::Backoff do + include ActiveSupport::Testing::TimeHelpers + + describe '#duration' do + it 'returns backoff duration from start' do + freeze_time do + described_class.new(5.minutes.ago).then do |backoff| + expect(backoff.duration).to eq 5.minutes + end + end + end + + it 'returns an integer value' do + freeze_time do + described_class.new(5.seconds.ago).then do |backoff| + expect(backoff.duration).to be 5 + end + end + end + + it 'returns the smallest number greater than or equal to duration' do + freeze_time do + described_class.new(0.5.seconds.ago).then do |backoff| + expect(backoff.duration).to be 1 + end + end + end + end + + describe '#slot' do + using RSpec::Parameterized::TableSyntax + + where(:started, :slot) do + 0 | 0 + 0.1 | 0 + 0.9 | 0 + 1 | 0 + 1.1 | 0 + 1.9 | 0 + 2 | 0 + 2.9 | 0 + 3 | 0 + 4 | 1 + 5 | 1 + 6 | 1 + 7 | 1 + 8 | 2 + 9 | 2 + 9.9 | 2 + 10 | 2 + 15 | 2 + 16 | 3 + 31 | 3 + 32 | 4 + 63 | 4 + 64 | 5 + 127 | 5 + 128 | 6 + 250 | 6 + 310 | 7 + 520 | 8 + 999 | 8 + end + + with_them do + it 'falls into an appropaite backoff slot' do + freeze_time do + backoff = described_class.new(started.seconds.ago) + expect(backoff.slot).to eq slot + end + end + end + end + + describe '#to_seconds' do + using RSpec::Parameterized::TableSyntax + + where(:started, :backoff) do + 0 | 1 + 0.1 | 1 + 0.9 | 1 + 1 | 1 + 1.1 | 1 + 1.9 | 1 + 2 | 1 + 3 | 1 + 4 | 2 + 5 | 2 + 6 | 2 + 6.5 | 2 + 7 | 2 + 8 | 4 + 9 | 4 + 9.9 | 4 + 10 | 4 + 15 | 4 + 16 | 8 + 31 | 8 + 32 | 16 + 63 | 16 + 64 | 32 + 127 | 32 + 128 | 64 + 250 | 64 + 310 | 64 + 520 | 64 + 999 | 64 + end + + with_them do + it 'calculates backoff based on an appropriate slot' do + freeze_time do + described_class.new(started.seconds.ago).then do |delay| + expect(delay.to_seconds).to eq backoff + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/trace/checksum_spec.rb b/spec/lib/gitlab/ci/trace/checksum_spec.rb new file mode 100644 index 00000000000..794794c3f69 --- /dev/null +++ b/spec/lib/gitlab/ci/trace/checksum_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Trace::Checksum do + let(:build) { create(:ci_build, :running) } + + subject { described_class.new(build) } + + context 'when build pending state exists' do + before do + create(:ci_build_pending_state, build: build, trace_checksum: 'crc32:d4777540') + end + + context 'when matching persisted trace chunks exist' do + before do + create_chunk(index: 0, data: 'a' * 128.kilobytes) + create_chunk(index: 1, data: 'b' * 128.kilobytes) + create_chunk(index: 2, data: 'ccccccccccccccccc') + end + + it 'calculates combined trace chunks CRC32 correctly' do + expect(subject.chunks_crc32).to eq 3564598592 + expect(subject).to be_valid + end + end + + context 'when trace chunks were persisted in a wrong order' do + before do + create_chunk(index: 0, data: 'b' * 128.kilobytes) + create_chunk(index: 1, data: 'a' * 128.kilobytes) + create_chunk(index: 2, data: 'ccccccccccccccccc') + end + + it 'makes trace checksum invalid' do + expect(subject).not_to be_valid + end + end + + context 'when one of the trace chunks is missing' do + before do + create_chunk(index: 0, data: 'a' * 128.kilobytes) + create_chunk(index: 2, data: 'ccccccccccccccccc') + end + + it 'makes trace checksum invalid' do + expect(subject).not_to be_valid + end + end + + context 'when checksums of persisted trace chunks do not match' do + before do + create_chunk(index: 0, data: 'a' * 128.kilobytes) + create_chunk(index: 1, data: 'X' * 128.kilobytes) + create_chunk(index: 2, data: 'ccccccccccccccccc') + end + + it 'makes trace checksum invalid' do + expect(subject).not_to be_valid + end + end + + context 'when persisted trace chunks are missing' do + it 'makes trace checksum invalid' do + expect(subject.state_crc32).to eq 3564598592 + expect(subject).not_to be_valid + end + end + end + + context 'when build pending state is missing' do + describe '#state_crc32' do + it 'returns nil' do + expect(subject.state_crc32).to be_nil + end + end + + describe '#valid?' do + it { is_expected.not_to be_valid } + end + end + + describe '#trace_chunks' do + before do + create_chunk(index: 0, data: 'abcdefg') + end + + it 'does not load raw_data from a database store' do + subject.trace_chunks.first.then do |chunk| + expect(chunk).to be_database + expect { chunk.raw_data } + .to raise_error ActiveModel::MissingAttributeError + end + end + end + + describe '#last_chunk' do + context 'when there are no chunks' do + it 'returns nil' do + expect(subject.last_chunk).to be_nil + end + end + + context 'when there are multiple chunks' do + before do + create_chunk(index: 1, data: '1234') + create_chunk(index: 0, data: 'abcd') + end + + it 'returns chunk with the highest index' do + expect(subject.last_chunk.chunk_index).to eq 1 + end + end + end + + def create_chunk(index:, data:) + create(:ci_build_trace_chunk, :persisted, build: build, + chunk_index: index, + initial_data: data) + end +end diff --git a/spec/lib/gitlab/ci/trace/metrics_spec.rb b/spec/lib/gitlab/ci/trace/metrics_spec.rb new file mode 100644 index 00000000000..6518d0ab075 --- /dev/null +++ b/spec/lib/gitlab/ci/trace/metrics_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Trace::Metrics, :prometheus do + describe '#increment_trace_bytes' do + context 'when incrementing by more than one' do + it 'increments a single counter' do + subject.increment_trace_bytes(10) + subject.increment_trace_bytes(20) + subject.increment_trace_bytes(30) + + expect(described_class.trace_bytes.get).to eq 60 + expect(described_class.trace_bytes.values.count).to eq 1 + end + end + end +end diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb index 171877dbaee..f037e803fb4 100644 --- a/spec/lib/gitlab/ci/trace_spec.rb +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -111,4 +111,13 @@ RSpec.describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do end end end + + describe '#lock' do + it 'acquires an exclusive lease on the trace' do + trace.lock do + expect { trace.lock } + .to raise_error described_class::LockedError + end + end + end end diff --git a/spec/lib/gitlab/ci/yaml_processor/result_spec.rb b/spec/lib/gitlab/ci/yaml_processor/result_spec.rb new file mode 100644 index 00000000000..7e3cd7ec254 --- /dev/null +++ b/spec/lib/gitlab/ci/yaml_processor/result_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +module Gitlab + module Ci + class YamlProcessor + RSpec.describe Result do + include StubRequests + + let(:user) { create(:user) } + let(:ci_config) { Gitlab::Ci::Config.new(config_content, user: user) } + let(:result) { described_class.new(ci_config: ci_config, warnings: ci_config&.warnings) } + + describe '#merged_yaml' do + subject(:merged_yaml) { result.merged_yaml } + + let(:config_content) do + YAML.dump( + include: { remote: 'https://example.com/sample.yml' }, + test: { stage: 'test', script: 'echo' } + ) + end + + let(:included_yml) do + YAML.dump( + another_test: { stage: 'test', script: 'echo 2' } + ) + end + + before do + stub_full_request('https://example.com/sample.yml').to_return(body: included_yml) + end + + it 'returns expanded yaml config' do + expanded_config = YAML.safe_load(merged_yaml, [Symbol]) + included_config = YAML.safe_load(included_yml, [Symbol]) + + expect(expanded_config).to include(*included_config.keys) + end + end + end + end + end +end diff --git a/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb b/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb index efdfc0a980b..6b568320953 100644 --- a/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb +++ b/spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb @@ -42,12 +42,24 @@ RSpec.describe Gitlab::Cleanup::OrphanLfsFileReferences do expect(null_logger).to receive(:info).with("Looking for orphan LFS files for project #{project.name_with_namespace}") expect(null_logger).to receive(:info).with("Removed invalid references: 1") expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:lfs_objects_size]) + expect(service).to receive(:remove_orphan_references).and_call_original expect { service.run! }.to change { project.lfs_objects.count }.from(2).to(1) expect(LfsObjectsProject.exists?(invalid_reference.id)).to be_falsey end + it 'does nothing if the project has no LFS objects' do + expect(null_logger).to receive(:info).with(/Looking for orphan LFS files/) + expect(null_logger).to receive(:info).with(/Nothing to do/) + + project.lfs_objects_projects.delete_all + + expect(service).not_to receive(:remove_orphan_references) + + service.run! + end + context 'LFS object is in design repository' do before do expect(project.design_repository).to receive(:exists?).and_return(true) diff --git a/spec/lib/gitlab/code_navigation_path_spec.rb b/spec/lib/gitlab/code_navigation_path_spec.rb index 4dc864b158d..206541f7c0d 100644 --- a/spec/lib/gitlab/code_navigation_path_spec.rb +++ b/spec/lib/gitlab/code_navigation_path_spec.rb @@ -16,10 +16,6 @@ RSpec.describe Gitlab::CodeNavigationPath do subject { described_class.new(project, commit_sha).full_json_path_for(path) } - before do - stub_feature_flags(code_navigation: project) - end - context 'when a pipeline exist for a sha' do it 'returns path to a file in the artifact' do expect(subject).to eq(lsif_path) @@ -41,15 +37,5 @@ RSpec.describe Gitlab::CodeNavigationPath do expect(subject).to eq(lsif_path) end end - - context 'when code_navigation feature is disabled' do - before do - stub_feature_flags(code_navigation: false) - end - - it 'returns nil' do - expect(subject).to be_nil - end - end end end diff --git a/spec/lib/gitlab/cycle_analytics/events_spec.rb b/spec/lib/gitlab/cycle_analytics/events_spec.rb index e0a8e2c17a3..a31f34d82d7 100644 --- a/spec/lib/gitlab/cycle_analytics/events_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/events_spec.rb @@ -2,16 +2,20 @@ require 'spec_helper' -RSpec.describe 'cycle analytics events' do - let(:project) { create(:project, :repository) } +RSpec.describe 'cycle analytics events', :aggregate_failures do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user, :admin) } let(:from_date) { 10.days.ago } - let(:user) { create(:user, :admin) } let!(:context) { create(:issue, project: project, created_at: 2.days.ago) } let(:events) do - CycleAnalytics::ProjectLevel.new(project, options: { from: from_date, current_user: user })[stage].events + CycleAnalytics::ProjectLevel + .new(project, options: { from: from_date, current_user: user })[stage] + .events end + let(:event) { events.first } + before do setup(context) end @@ -19,36 +23,15 @@ RSpec.describe 'cycle analytics events' do describe '#issue_events' do let(:stage) { :issue } - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty - end - - it 'has a title' do - expect(events.first[:title]).to eq(context.title) - end - - it 'has the URL' do - expect(events.first[:url]).not_to be_nil - end - - it 'has an iid' do - expect(events.first[:iid]).to eq(context.iid.to_s) - end - - it 'has a created_at timestamp' do - expect(events.first[:created_at]).to end_with('ago') - end - - it "has the author's URL" do - expect(events.first[:author][:web_url]).not_to be_nil - end - - it "has the author's avatar URL" do - expect(events.first[:author][:avatar_url]).not_to be_nil - end - - it "has the author's name" do - expect(events.first[:author][:name]).to eq(context.author.name) + it 'has correct attributes' do + expect(event[:total_time]).not_to be_empty + expect(event[:title]).to eq(context.title) + expect(event[:url]).not_to be_nil + expect(event[:iid]).to eq(context.iid.to_s) + expect(event[:created_at]).to end_with('ago') + expect(event[:author][:web_url]).not_to be_nil + expect(event[:author][:avatar_url]).not_to be_nil + expect(event[:author][:name]).to eq(context.author.name) end end @@ -59,36 +42,15 @@ RSpec.describe 'cycle analytics events' do create_commit_referencing_issue(context) end - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty - end - - it 'has a title' do - expect(events.first[:title]).to eq(context.title) - end - - it 'has the URL' do - expect(events.first[:url]).not_to be_nil - end - - it 'has an iid' do - expect(events.first[:iid]).to eq(context.iid.to_s) - end - - it 'has a created_at timestamp' do - expect(events.first[:created_at]).to end_with('ago') - end - - it "has the author's URL" do - expect(events.first[:author][:web_url]).not_to be_nil - end - - it "has the author's avatar URL" do - expect(events.first[:author][:avatar_url]).not_to be_nil - end - - it "has the author's name" do - expect(events.first[:author][:name]).to eq(context.author.name) + it 'has correct attributes' do + expect(event[:total_time]).not_to be_empty + expect(event[:title]).to eq(context.title) + expect(event[:url]).not_to be_nil + expect(event[:iid]).to eq(context.iid.to_s) + expect(event[:created_at]).to end_with('ago') + expect(event[:author][:web_url]).not_to be_nil + expect(event[:author][:avatar_url]).not_to be_nil + expect(event[:author][:name]).to eq(context.author.name) end end @@ -100,32 +62,14 @@ RSpec.describe 'cycle analytics events' do create_commit_referencing_issue(context) end - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty - end - - it 'has a title' do - expect(events.first[:title]).to eq('Awesome merge_request') - end - - it 'has an iid' do - expect(events.first[:iid]).to eq(context.iid.to_s) - end - - it 'has a created_at timestamp' do - expect(events.first[:created_at]).to end_with('ago') - end - - it "has the author's URL" do - expect(events.first[:author][:web_url]).not_to be_nil - end - - it "has the author's avatar URL" do - expect(events.first[:author][:avatar_url]).not_to be_nil - end - - it "has the author's name" do - expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) + it 'has correct attributes' do + expect(event[:total_time]).not_to be_empty + expect(event[:title]).to eq('Awesome merge_request') + expect(event[:iid]).to eq(context.iid.to_s) + expect(event[:created_at]).to end_with('ago') + expect(event[:author][:web_url]).not_to be_nil + expect(event[:author][:avatar_url]).not_to be_nil + expect(event[:author][:name]).to eq(MergeRequest.first.author.name) end end @@ -152,40 +96,16 @@ RSpec.describe 'cycle analytics events' do merge_merge_requests_closing_issue(user, project, context) end - it 'has the name' do - expect(events.first[:name]).not_to be_nil - end - - it 'has the ID' do - expect(events.first[:id]).not_to be_nil - end - - it 'has the URL' do - expect(events.first[:url]).not_to be_nil - end - - it 'has the branch name' do - expect(events.first[:branch]).not_to be_nil - end - - it 'has the branch URL' do - expect(events.first[:branch][:url]).not_to be_nil - end - - it 'has the short SHA' do - expect(events.first[:short_sha]).not_to be_nil - end - - it 'has the commit URL' do - expect(events.first[:commit_url]).not_to be_nil - end - - it 'has the date' do - expect(events.first[:date]).not_to be_nil - end - - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty + it 'has correct attributes' do + expect(event[:name]).not_to be_nil + expect(event[:id]).not_to be_nil + expect(event[:url]).not_to be_nil + expect(event[:branch]).not_to be_nil + expect(event[:branch][:url]).not_to be_nil + expect(event[:short_sha]).not_to be_nil + expect(event[:commit_url]).not_to be_nil + expect(event[:date]).not_to be_nil + expect(event[:total_time]).not_to be_empty end end @@ -197,40 +117,16 @@ RSpec.describe 'cycle analytics events' do merge_merge_requests_closing_issue(user, project, context) end - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty - end - - it 'has a title' do - expect(events.first[:title]).to eq('Awesome merge_request') - end - - it 'has an iid' do - expect(events.first[:iid]).to eq(context.iid.to_s) - end - - it 'has the URL' do - expect(events.first[:url]).not_to be_nil - end - - it 'has a state' do - expect(events.first[:state]).not_to be_nil - end - - it 'has a created_at timestamp' do - expect(events.first[:created_at]).not_to be_nil - end - - it "has the author's URL" do - expect(events.first[:author][:web_url]).not_to be_nil - end - - it "has the author's avatar URL" do - expect(events.first[:author][:avatar_url]).not_to be_nil - end - - it "has the author's name" do - expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) + it 'has correct attributes' do + expect(event[:total_time]).not_to be_empty + expect(event[:title]).to eq('Awesome merge_request') + expect(event[:iid]).to eq(context.iid.to_s) + expect(event[:url]).not_to be_nil + expect(event[:state]).not_to be_nil + expect(event[:created_at]).not_to be_nil + expect(event[:author][:web_url]).not_to be_nil + expect(event[:author][:avatar_url]).not_to be_nil + expect(event[:author][:name]).to eq(MergeRequest.first.author.name) end end @@ -257,58 +153,25 @@ RSpec.describe 'cycle analytics events' do deploy_master(user, project) end - it 'has the name' do - expect(events.first[:name]).not_to be_nil - end - - it 'has the ID' do - expect(events.first[:id]).not_to be_nil - end - - it 'has the URL' do - expect(events.first[:url]).not_to be_nil - end - - it 'has the branch name' do - expect(events.first[:branch]).not_to be_nil - end - - it 'has the branch URL' do - expect(events.first[:branch][:url]).not_to be_nil - end - - it 'has the short SHA' do - expect(events.first[:short_sha]).not_to be_nil - end - - it 'has the commit URL' do - expect(events.first[:commit_url]).not_to be_nil - end - - it 'has the date' do - expect(events.first[:date]).not_to be_nil - end - - it 'has the total time' do - expect(events.first[:total_time]).not_to be_empty - end - - it "has the author's URL" do - expect(events.first[:author][:web_url]).not_to be_nil - end - - it "has the author's avatar URL" do - expect(events.first[:author][:avatar_url]).not_to be_nil - end - - it "has the author's name" do - expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) + it 'has correct attributes' do + expect(event[:name]).not_to be_nil + expect(event[:id]).not_to be_nil + expect(event[:url]).not_to be_nil + expect(event[:branch]).not_to be_nil + expect(event[:branch][:url]).not_to be_nil + expect(event[:short_sha]).not_to be_nil + expect(event[:commit_url]).not_to be_nil + expect(event[:date]).not_to be_nil + expect(event[:total_time]).not_to be_empty + expect(event[:author][:web_url]).not_to be_nil + expect(event[:author][:avatar_url]).not_to be_nil + expect(event[:author][:name]).to eq(MergeRequest.first.author.name) end end def setup(context) milestone = create(:milestone, project: project) - context.update(milestone: milestone) + context.update!(milestone: milestone) mr = create_merge_request_closing_issue(user, project, context, commit_message: "References #{context.to_reference}") ProcessCommitWorker.new.perform(project.id, user.id, mr.commits.last.to_hash) diff --git a/spec/lib/gitlab/danger/commit_linter_spec.rb b/spec/lib/gitlab/danger/commit_linter_spec.rb index c31522c538d..882cede759b 100644 --- a/spec/lib/gitlab/danger/commit_linter_spec.rb +++ b/spec/lib/gitlab/danger/commit_linter_spec.rb @@ -323,6 +323,16 @@ RSpec.describe Gitlab::Danger::CommitLinter do end end + context 'when message includes a value that is surrounded by backticks' do + let(:commit_message) { "A commit message `%20`" } + + it 'does not add a problem' do + expect(commit_linter).not_to receive(:add_problem) + + commit_linter.lint + end + end + context 'when message includes a short reference' do [ 'A commit message to fix #1234', @@ -336,7 +346,9 @@ RSpec.describe Gitlab::Danger::CommitLinter do 'A commit message to fix gitlab-org/gitlab#1234', 'A commit message to fix gitlab-org/gitlab!1234', 'A commit message to fix gitlab-org/gitlab&1234', - 'A commit message to fix gitlab-org/gitlab%1234' + 'A commit message to fix gitlab-org/gitlab%1234', + 'A commit message to fix "gitlab-org/gitlab%1234"', + 'A commit message to fix `gitlab-org/gitlab%1234' ].each do |message| let(:commit_message) { message } diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index c7d55c396ef..708e9a13aed 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -435,6 +435,28 @@ RSpec.describe Gitlab::Danger::Helper do end end + describe '#draft_mr?' do + it 'returns false when `gitlab_helper` is unavailable' do + expect(helper).to receive(:gitlab_helper).and_return(nil) + + expect(helper).not_to be_draft_mr + end + + it 'returns true for a draft MR' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'Draft: My MR title') + + expect(helper).to be_draft_mr + end + + it 'returns false for non draft MR' do + expect(fake_gitlab).to receive(:mr_json) + .and_return('title' => 'My MR title') + + expect(helper).not_to be_draft_mr + end + end + describe '#cherry_pick_mr?' do it 'returns false when `gitlab_helper` is unavailable' do expect(helper).to receive(:gitlab_helper).and_return(nil) diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb index b471e17e2e7..9acaa57ee10 100644 --- a/spec/lib/gitlab/danger/roulette_spec.rb +++ b/spec/lib/gitlab/danger/roulette_spec.rb @@ -7,7 +7,7 @@ require 'gitlab/danger/roulette' RSpec.describe Gitlab::Danger::Roulette do around do |example| - Timecop.freeze(Time.utc(2020, 06, 22, 10)) { example.run } + travel_to(Time.utc(2020, 06, 22, 10)) { example.run } end let(:backend_available) { true } @@ -67,14 +67,18 @@ RSpec.describe Gitlab::Danger::Roulette do ) end - let(:teammate_json) do + let(:teammates) do [ backend_maintainer.to_h, frontend_maintainer.to_h, frontend_reviewer.to_h, software_engineer_in_test.to_h, engineering_productivity_reviewer.to_h - ].to_json + ] + end + + let(:teammate_json) do + teammates.to_json end subject(:roulette) { Object.new.extend(described_class) } @@ -210,6 +214,69 @@ RSpec.describe Gitlab::Danger::Roulette do end end end + + describe 'reviewer suggestion probability' do + let(:reviewer) { teammate_with_capability('reviewer', 'reviewer backend') } + let(:hungry_reviewer) { teammate_with_capability('hungry_reviewer', 'reviewer backend', hungry: true) } + let(:traintainer) { teammate_with_capability('traintainer', 'trainee_maintainer backend') } + let(:hungry_traintainer) { teammate_with_capability('hungry_traintainer', 'trainee_maintainer backend', hungry: true) } + let(:teammates) do + [ + reviewer.to_h, + hungry_reviewer.to_h, + traintainer.to_h, + hungry_traintainer.to_h + ] + end + + let(:categories) { [:backend] } + + # This test is testing probability with inherent randomness. + # The variance is inversely related to sample size + # Given large enough sample size, the variance would be smaller, + # but the test would take longer. + # Given smaller sample size, the variance would be larger, + # but the test would take less time. + let!(:sample_size) { 500 } + let!(:variance) { 0.1 } + + before do + # This test needs actual randomness to simulate probabilities + allow(subject).to receive(:new_random).and_return(Random.new) + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: teammate_json) + end + + it 'has 1:2:3:4 probability of picking reviewer, hungry_reviewer, traintainer, hungry_traintainer' do + picks = Array.new(sample_size).map do + spins = subject.spin(project, categories, timezone_experiment: timezone_experiment) + spins.first.reviewer.name + end + + expect(probability(picks, 'reviewer')).to be_within(variance).of(0.1) + expect(probability(picks, 'hungry_reviewer')).to be_within(variance).of(0.2) + expect(probability(picks, 'traintainer')).to be_within(variance).of(0.3) + expect(probability(picks, 'hungry_traintainer')).to be_within(variance).of(0.4) + end + + def probability(picks, role) + picks.count(role).to_f / picks.length + end + + def teammate_with_capability(name, capability, hungry: false) + Gitlab::Danger::Teammate.new( + { + 'name' => name, + 'projects' => { + 'gitlab' => capability + }, + 'available' => true, + 'hungry' => hungry + } + ) + end + end end RSpec::Matchers.define :match_teammates do |expected| diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index 6fd32493d6b..5a47d74a7f3 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -149,7 +149,7 @@ RSpec.describe Gitlab::Danger::Teammate do describe '#local_hour' do around do |example| - Timecop.freeze(Time.utc(2020, 6, 23, 10)) { example.run } + travel_to(Time.utc(2020, 6, 23, 10)) { example.run } end context 'when author is given' do diff --git a/spec/lib/gitlab/database/background_migration_job_spec.rb b/spec/lib/gitlab/database/background_migration_job_spec.rb index dd5bf8b512f..42695925a1c 100644 --- a/spec/lib/gitlab/database/background_migration_job_spec.rb +++ b/spec/lib/gitlab/database/background_migration_job_spec.rb @@ -85,7 +85,7 @@ RSpec.describe Gitlab::Database::BackgroundMigrationJob do let!(:job1) { create(:background_migration_job, :succeeded, created_at: initial_time, updated_at: initial_time) } it 'does not update non-pending jobs' do - Timecop.freeze(initial_time + 1.day) do + travel_to(initial_time + 1.day) do expect { described_class.mark_all_as_succeeded('TestJob', [1, 100]) } .to change { described_class.succeeded.count }.from(1).to(2) end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index 71d3666602f..93c499eb56d 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -108,6 +108,24 @@ RSpec.describe Gitlab::Database::BatchCount do expect { described_class.batch_count(model.distinct(column)) }.to raise_error 'Use distinct count for optimized distinct counting' end end + + context 'when a relation is grouped' do + let!(:one_more_issue) { create(:issue, author: user, project: model.first.project) } + + before do + stub_const('Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE', 1) + end + + context 'count by default column' do + let(:count) do + described_class.batch_count(model.group(column), batch_size: 2) + end + + it 'counts grouped records' do + expect(count).to eq({ user.id => 4, another_user.id => 2 }) + end + end + end end describe '#batch_distinct_count' do @@ -175,6 +193,24 @@ RSpec.describe Gitlab::Database::BatchCount do end.to raise_error 'Use distinct count only with non id fields' end end + + context 'when a relation is grouped' do + let!(:one_more_issue) { create(:issue, author: user, project: model.first.project) } + + before do + stub_const('Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE', 1) + end + + context 'distinct count by non-unique column' do + let(:count) do + described_class.batch_distinct_count(model.group(column), :project_id, batch_size: 2) + end + + it 'counts grouped records' do + expect(count).to eq({ user.id => 3, another_user.id => 2 }) + end + end + end end describe '#batch_sum' do diff --git a/spec/lib/gitlab/database/concurrent_reindex_spec.rb b/spec/lib/gitlab/database/concurrent_reindex_spec.rb deleted file mode 100644 index 4e2c3f547d4..00000000000 --- a/spec/lib/gitlab/database/concurrent_reindex_spec.rb +++ /dev/null @@ -1,207 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do - subject { described_class.new(index_name, logger: logger) } - - let(:table_name) { '_test_reindex_table' } - let(:column_name) { '_test_column' } - let(:index_name) { '_test_reindex_index' } - let(:logger) { double('logger', debug: nil, info: nil, error: nil ) } - let(:connection) { ActiveRecord::Base.connection } - - before do - connection.execute(<<~SQL) - CREATE TABLE #{table_name} ( - id serial NOT NULL PRIMARY KEY, - #{column_name} integer NOT NULL); - - CREATE INDEX #{index_name} ON #{table_name} (#{column_name}); - SQL - end - - context 'when the index does not exist' do - before do - connection.execute(<<~SQL) - DROP INDEX #{index_name} - SQL - end - - it 'raises an error' do - expect { subject.execute }.to raise_error(described_class::ReindexError, /does not exist/) - end - end - - context 'when the index is unique' do - before do - connection.execute(<<~SQL) - DROP INDEX #{index_name}; - CREATE UNIQUE INDEX #{index_name} ON #{table_name} (#{column_name}) - SQL - end - - it 'raises an error' do - expect do - subject.execute - end.to raise_error(described_class::ReindexError, /UNIQUE indexes are currently not supported/) - end - end - - context 'replacing the original index with a rebuilt copy' do - let(:replacement_name) { 'tmp_reindex__test_reindex_index' } - let(:replaced_name) { 'old_reindex__test_reindex_index' } - - let(:create_index) { "CREATE INDEX CONCURRENTLY #{replacement_name} ON public.#{table_name} USING btree (#{column_name})" } - let(:drop_index) { "DROP INDEX CONCURRENTLY IF EXISTS #{replacement_name}" } - - let!(:original_index) { find_index_create_statement } - - before do - allow(subject).to receive(:connection).and_return(connection) - allow(subject).to receive(:disable_statement_timeout).and_yield - end - - it 'replaces the existing index with an identical index' do - expect(subject).to receive(:disable_statement_timeout).exactly(3).times.and_yield - - expect_to_execute_concurrently_in_order(drop_index) - expect_to_execute_concurrently_in_order(create_index) - - expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| - expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield - end - - expect_to_execute_in_order("ALTER INDEX #{index_name} RENAME TO #{replaced_name}") - expect_to_execute_in_order("ALTER INDEX #{replacement_name} RENAME TO #{index_name}") - expect_to_execute_in_order("ALTER INDEX #{replaced_name} RENAME TO #{replacement_name}") - - expect_to_execute_concurrently_in_order(drop_index) - - subject.execute - - check_index_exists - end - - context 'when a dangling index is left from a previous run' do - before do - connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})") - end - - it 'replaces the existing index with an identical index' do - expect(subject).to receive(:disable_statement_timeout).exactly(3).times.and_yield - - expect_to_execute_concurrently_in_order(drop_index) - expect_to_execute_concurrently_in_order(create_index) - - expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| - expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield - end - - expect_to_execute_in_order("ALTER INDEX #{index_name} RENAME TO #{replaced_name}") - expect_to_execute_in_order("ALTER INDEX #{replacement_name} RENAME TO #{index_name}") - expect_to_execute_in_order("ALTER INDEX #{replaced_name} RENAME TO #{replacement_name}") - - expect_to_execute_concurrently_in_order(drop_index) - - subject.execute - - check_index_exists - end - end - - context 'when it fails to create the replacement index' do - it 'safely cleans up and signals the error' do - expect_to_execute_concurrently_in_order(drop_index) - - expect(connection).to receive(:execute).with(create_index).ordered - .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') - - expect_to_execute_concurrently_in_order(drop_index) - - expect { subject.execute }.to raise_error(described_class::ReindexError, /connect timeout/) - - check_index_exists - end - end - - context 'when the replacement index is not valid' do - it 'safely cleans up and signals the error' do - expect_to_execute_concurrently_in_order(drop_index) - expect_to_execute_concurrently_in_order(create_index) - - expect(subject).to receive(:replacement_index_valid?).and_return(false) - - expect_to_execute_concurrently_in_order(drop_index) - - expect { subject.execute }.to raise_error(described_class::ReindexError, /replacement index was created as INVALID/) - - check_index_exists - end - end - - context 'when a database error occurs while swapping the indexes' do - it 'safely cleans up and signals the error' do - expect_to_execute_concurrently_in_order(drop_index) - expect_to_execute_concurrently_in_order(create_index) - - expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| - expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield - end - - expect(connection).to receive(:execute).ordered - .with("ALTER INDEX #{index_name} RENAME TO #{replaced_name}") - .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') - - expect_to_execute_concurrently_in_order(drop_index) - - expect { subject.execute }.to raise_error(described_class::ReindexError, /connect timeout/) - - check_index_exists - end - end - - context 'when with_lock_retries fails to acquire the lock' do - it 'safely cleans up and signals the error' do - expect_to_execute_concurrently_in_order(drop_index) - expect_to_execute_concurrently_in_order(create_index) - - expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| - expect(instance).to receive(:run).with(raise_on_exhaustion: true) - .and_raise(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, 'exhausted') - end - - expect_to_execute_concurrently_in_order(drop_index) - - expect { subject.execute }.to raise_error(described_class::ReindexError, /exhausted/) - - check_index_exists - end - end - end - - def expect_to_execute_concurrently_in_order(sql) - # Indexes cannot be created CONCURRENTLY in a transaction. Since the tests are wrapped in transactions, - # verify the original call but pass through the non-concurrent form. - expect(connection).to receive(:execute).with(sql).ordered.and_wrap_original do |method, sql| - method.call(sql.sub(/CONCURRENTLY/, '')) - end - end - - def expect_to_execute_in_order(sql) - expect(connection).to receive(:execute).with(sql).ordered.and_call_original - end - - def find_index_create_statement - ActiveRecord::Base.connection.select_value(<<~SQL) - SELECT indexdef - FROM pg_indexes - WHERE schemaname = 'public' - AND indexname = #{ActiveRecord::Base.connection.quote(index_name)} - SQL - end - - def check_index_exists - expect(find_index_create_statement).to eq(original_index) - end -end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 0bdcca630aa..727ad243349 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1128,7 +1128,65 @@ RSpec.describe Gitlab::Database::MigrationHelpers do name: 'index_on_issues_gl_project_id', length: [], order: [], - opclasses: { 'gl_project_id' => 'bar' }) + opclass: { 'gl_project_id' => 'bar' }) + + model.copy_indexes(:issues, :project_id, :gl_project_id) + end + end + + context 'using an index with multiple columns and custom operator classes' do + it 'copies the index' do + index = double(:index, + columns: %w(project_id foobar), + name: 'index_on_issues_project_id_foobar', + using: :gin, + where: nil, + opclasses: { 'project_id' => 'bar', 'foobar' => :gin_trgm_ops }, + unique: false, + lengths: [], + orders: []) + + allow(model).to receive(:indexes_for).with(:issues, 'project_id') + .and_return([index]) + + expect(model).to receive(:add_concurrent_index) + .with(:issues, + %w(gl_project_id foobar), + unique: false, + name: 'index_on_issues_gl_project_id_foobar', + length: [], + order: [], + opclass: { 'gl_project_id' => 'bar', 'foobar' => :gin_trgm_ops }, + using: :gin) + + model.copy_indexes(:issues, :project_id, :gl_project_id) + end + end + + context 'using an index with multiple columns and a custom operator class on the non affected column' do + it 'copies the index' do + index = double(:index, + columns: %w(project_id foobar), + name: 'index_on_issues_project_id_foobar', + using: :gin, + where: nil, + opclasses: { 'foobar' => :gin_trgm_ops }, + unique: false, + lengths: [], + orders: []) + + allow(model).to receive(:indexes_for).with(:issues, 'project_id') + .and_return([index]) + + expect(model).to receive(:add_concurrent_index) + .with(:issues, + %w(gl_project_id foobar), + unique: false, + name: 'index_on_issues_gl_project_id_foobar', + length: [], + order: [], + opclass: { 'foobar' => :gin_trgm_ops }, + using: :gin) model.copy_indexes(:issues, :project_id, :gl_project_id) end @@ -1400,15 +1458,32 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ) end - after do - 'DROP INDEX IF EXISTS test_index;' - end - it 'returns true if an index exists' do expect(model.index_exists_by_name?(:projects, 'test_index')) .to be_truthy end end + + context 'when an index exists for a table with the same name in another schema' do + before do + ActiveRecord::Base.connection.execute( + 'CREATE SCHEMA new_test_schema' + ) + + ActiveRecord::Base.connection.execute( + 'CREATE TABLE new_test_schema.projects (id integer, name character varying)' + ) + + ActiveRecord::Base.connection.execute( + 'CREATE INDEX test_index_on_name ON new_test_schema.projects (LOWER(name));' + ) + end + + it 'returns false if the index does not exist in the current schema' do + expect(model.index_exists_by_name?(:projects, 'test_index_on_name')) + .to be_falsy + end + end end describe '#create_or_update_plan_limit' do @@ -1863,11 +1938,17 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ActiveRecord::Base.connection.execute( 'ALTER TABLE projects ADD CONSTRAINT check_1 CHECK (char_length(path) <= 5) NOT VALID' ) - end - after do ActiveRecord::Base.connection.execute( - 'ALTER TABLE projects DROP CONSTRAINT IF EXISTS check_1' + 'CREATE SCHEMA new_test_schema' + ) + + ActiveRecord::Base.connection.execute( + 'CREATE TABLE new_test_schema.projects (id integer, name character varying)' + ) + + ActiveRecord::Base.connection.execute( + 'ALTER TABLE new_test_schema.projects ADD CONSTRAINT check_2 CHECK (char_length(name) <= 5)' ) end @@ -1885,6 +1966,11 @@ RSpec.describe Gitlab::Database::MigrationHelpers do expect(model.check_constraint_exists?(:users, 'check_1')) .to be_falsy end + + it 'returns false if a constraint with the same name exists for the same table in another schema' do + expect(model.check_constraint_exists?(:projects, 'check_2')) + .to be_falsy + end end describe '#add_check_constraint' do diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index 034bf966db7..8a35d8149ad 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -52,7 +52,7 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do describe '#execute' do it 'returns a list of class names and columns pairs' do - Timecop.freeze(REMOVE_DATE) do + travel_to(REMOVE_DATE) do expect(subject.execute).to eq([ ['Testing::A', { 'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'), diff --git a/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb index 334cac653cf..885eef5723e 100644 --- a/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb +++ b/spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Gitlab::Database::Partitioning::MonthlyStrategy do let(:partitioning_key) { :created_at } around do |example| - Timecop.freeze(Date.parse('2020-08-22')) { example.run } + travel_to(Date.parse('2020-08-22')) { example.run } end context 'with existing partitions' do diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb index ec3d0a6dbcb..c43b51e10a0 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb @@ -116,23 +116,6 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::BackfillPartition expect(jobs_updated).to eq(1) end - context 'when the feature flag is disabled' do - let(:mock_connection) { double('connection') } - - before do - allow(subject).to receive(:connection).and_return(mock_connection) - stub_feature_flags(backfill_partitioned_audit_events: false) - end - - it 'exits without attempting to copy data' do - expect(mock_connection).not_to receive(:execute) - - subject.perform(1, 100, source_table, destination_table, unique_key) - - expect(destination_model.count).to eq(0) - end - end - context 'when the job is run within an explicit transaction block' do let(:mock_connection) { double('connection') } diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index 44ef0b307fe..147637cf471 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -213,7 +213,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe it 'creates partitions including the next month from today' do today = Date.new(2020, 5, 8) - Timecop.freeze(today) do + travel_to(today) do migration.partition_table_by_date source_table, partition_column, min_date: min_date expect_range_partitions_for(partitioned_table, { @@ -233,7 +233,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe context 'without min_date, max_date' do it 'creates partitions for the current and next month' do current_date = Date.new(2020, 05, 22) - Timecop.freeze(current_date.to_time) do + travel_to(current_date.to_time) do migration.partition_table_by_date source_table, partition_column expect_range_partitions_for(partitioned_table, { @@ -514,6 +514,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe allow(migration).to receive(:table_exists?).with(partitioned_table).and_return(true) allow(migration).to receive(:copy_missed_records) allow(migration).to receive(:execute).with(/VACUUM/) + allow(migration).to receive(:execute).with(/^(RE)?SET/) end it 'finishes remaining jobs for the correct table' do @@ -567,6 +568,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe allow(Gitlab::BackgroundMigration).to receive(:steal) allow(migration).to receive(:execute).with(/VACUUM/) + allow(migration).to receive(:execute).with(/^(RE)?SET/) end it 'idempotently cleans up after failed background migrations' do diff --git a/spec/lib/gitlab/database/postgres_index_spec.rb b/spec/lib/gitlab/database/postgres_index_spec.rb new file mode 100644 index 00000000000..1da67a5a6c0 --- /dev/null +++ b/spec/lib/gitlab/database/postgres_index_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::PostgresIndex do + before do + ActiveRecord::Base.connection.execute(<<~SQL) + CREATE INDEX foo_idx ON public.users (name); + CREATE UNIQUE INDEX bar_key ON public.users (id); + + CREATE TABLE example_table (id serial primary key); + SQL + end + + def find(name) + described_class.by_identifier(name) + end + + describe '.by_identifier' do + it 'finds the index' do + expect(find('public.foo_idx')).to be_a(Gitlab::Database::PostgresIndex) + end + + it 'raises an error if not found' do + expect { find('public.idontexist') }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'raises ArgumentError if given a non-fully qualified index name' do + expect { find('foo') }.to raise_error(ArgumentError, /not fully qualified/) + end + end + + describe '.regular' do + it 'only non-unique indexes' do + expect(described_class.regular).to all(have_attributes(unique: false)) + end + + it 'only non partitioned indexes ' do + expect(described_class.regular).to all(have_attributes(partitioned: false)) + end + + it 'only indexes that dont serve an exclusion constraint' do + expect(described_class.regular).to all(have_attributes(exclusion: false)) + end + end + + describe '.not_match' do + it 'excludes indexes matching the given regex' do + expect(described_class.not_match('^bar_k').map(&:name)).to all(match(/^(?!bar_k).*/)) + end + + it 'matches indexes without this prefix regex' do + expect(described_class.not_match('^bar_k')).not_to be_empty + end + end + + describe '.random_few' do + it 'limits to two records by default' do + expect(described_class.random_few(2).size).to eq(2) + end + end + + describe '#unique?' do + it 'returns true for a unique index' do + expect(find('public.bar_key')).to be_unique + end + + it 'returns false for a regular, non-unique index' do + expect(find('public.foo_idx')).not_to be_unique + end + + it 'returns true for a primary key index' do + expect(find('public.example_table_pkey')).to be_unique + end + end + + describe '#valid_index?' do + it 'returns true if the index is invalid' do + expect(find('public.foo_idx')).to be_valid_index + end + + it 'returns false if the index is marked as invalid' do + ActiveRecord::Base.connection.execute(<<~SQL) + UPDATE pg_index SET indisvalid=false + FROM pg_class + WHERE pg_class.relname = 'foo_idx' AND pg_index.indexrelid = pg_class.oid + SQL + + expect(find('public.foo_idx')).not_to be_valid_index + end + end + + describe '#to_s' do + it 'returns the index name' do + expect(find('public.foo_idx').to_s).to eq('foo_idx') + end + end + + describe '#name' do + it 'returns the name' do + expect(find('public.foo_idx').name).to eq('foo_idx') + end + end + + describe '#schema' do + it 'returns the index schema' do + expect(find('public.foo_idx').schema).to eq('public') + end + end + + describe '#definition' do + it 'returns the index definition' do + expect(find('public.foo_idx').definition).to eq('CREATE INDEX foo_idx ON public.users USING btree (name)') + end + end +end diff --git a/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb new file mode 100644 index 00000000000..a80bf8176d2 --- /dev/null +++ b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb @@ -0,0 +1,255 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do + subject { described_class.new(index, logger: logger) } + + let(:table_name) { '_test_reindex_table' } + let(:column_name) { '_test_column' } + let(:index_name) { '_test_reindex_index' } + let(:index) { instance_double(Gitlab::Database::PostgresIndex, indexrelid: 42, name: index_name, schema: 'public', partitioned?: false, unique?: false, exclusion?: false, definition: 'CREATE INDEX _test_reindex_index ON public._test_reindex_table USING btree (_test_column)') } + let(:logger) { double('logger', debug: nil, info: nil, error: nil ) } + let(:connection) { ActiveRecord::Base.connection } + + before do + connection.execute(<<~SQL) + CREATE TABLE #{table_name} ( + id serial NOT NULL PRIMARY KEY, + #{column_name} integer NOT NULL); + + CREATE INDEX #{index.name} ON #{table_name} (#{column_name}); + SQL + end + + context 'when the index is unique' do + before do + allow(index).to receive(:unique?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /UNIQUE indexes are currently not supported/) + end + end + + context 'when the index is partitioned' do + before do + allow(index).to receive(:partitioned?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /partitioned indexes are currently not supported/) + end + end + + context 'when the index serves an exclusion constraint' do + before do + allow(index).to receive(:exclusion?).and_return(true) + end + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /indexes serving an exclusion constraint are currently not supported/) + end + end + + context 'when the index is a lingering temporary index from a previous reindexing run' do + context 'with the temporary index prefix' do + let(:index_name) { 'tmp_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + + context 'with the replaced index prefix' do + let(:index_name) { 'old_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + end + + context 'replacing the original index with a rebuilt copy' do + let(:replacement_name) { 'tmp_reindex_42' } + let(:replaced_name) { 'old_reindex_42' } + + let(:create_index) { "CREATE INDEX CONCURRENTLY #{replacement_name} ON public.#{table_name} USING btree (#{column_name})" } + let(:drop_index) do + <<~SQL + DROP INDEX CONCURRENTLY + IF EXISTS "public"."#{replacement_name}" + SQL + end + + let!(:original_index) { find_index_create_statement } + + it 'integration test: executing full index replacement without mocks' do + allow(connection).to receive(:execute).and_wrap_original do |method, sql| + method.call(sql.sub(/CONCURRENTLY/, '')) + end + + subject.perform + + check_index_exists + end + + context 'mocked specs' do + before do + allow(subject).to receive(:connection).and_return(connection) + allow(subject).to receive(:disable_statement_timeout).and_yield + end + + it 'replaces the existing index with an identical index' do + expect(subject).to receive(:disable_statement_timeout).twice.and_yield + + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name) + expect_index_rename(replacement_name, index.name) + expect_index_rename(replaced_name, replacement_name) + + expect_to_execute_concurrently_in_order(drop_index) + + subject.perform + + check_index_exists + end + + context 'when a dangling index is left from a previous run' do + before do + connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})") + end + + it 'replaces the existing index with an identical index' do + expect(subject).to receive(:disable_statement_timeout).exactly(3).times.and_yield + + expect_to_execute_concurrently_in_order(drop_index) + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name) + expect_index_rename(replacement_name, index.name) + expect_index_rename(replaced_name, replacement_name) + + expect_to_execute_concurrently_in_order(drop_index) + + subject.perform + + check_index_exists + end + end + + context 'when it fails to create the replacement index' do + it 'safely cleans up and signals the error' do + expect(connection).to receive(:execute).with(create_index).ordered + .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(ActiveRecord::ConnectionTimeoutError, /connect timeout/) + + check_index_exists + end + end + + context 'when the replacement index is not valid' do + it 'safely cleans up and signals the error' do + replacement_index = double('replacement index', valid_index?: false) + allow(Gitlab::Database::PostgresIndex).to receive(:find_by).with(schema: 'public', name: replacement_name).and_return(nil, replacement_index) + + expect_to_execute_concurrently_in_order(create_index) + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(described_class::ReindexError, /replacement index was created as INVALID/) + + check_index_exists + end + end + + context 'when a database error occurs while swapping the indexes' do + it 'safely cleans up and signals the error' do + replacement_index = double('replacement index', valid_index?: true) + allow(Gitlab::Database::PostgresIndex).to receive(:find_by).with(schema: 'public', name: replacement_name).and_return(nil, replacement_index) + + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield + end + + expect_index_rename(index.name, replaced_name).and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout') + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(ActiveRecord::ConnectionTimeoutError, /connect timeout/) + + check_index_exists + end + end + + context 'when with_lock_retries fails to acquire the lock' do + it 'safely cleans up and signals the error' do + expect_to_execute_concurrently_in_order(create_index) + + expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance| + expect(instance).to receive(:run).with(raise_on_exhaustion: true) + .and_raise(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, 'exhausted') + end + + expect_to_execute_concurrently_in_order(drop_index) + + expect { subject.perform }.to raise_error(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, /exhausted/) + + check_index_exists + end + end + end + end + + def expect_to_execute_concurrently_in_order(sql) + # Indexes cannot be created CONCURRENTLY in a transaction. Since the tests are wrapped in transactions, + # verify the original call but pass through the non-concurrent form. + expect(connection).to receive(:execute).with(sql).ordered.and_wrap_original do |method, sql| + method.call(sql.sub(/CONCURRENTLY/, '')) + end + end + + def expect_index_rename(from, to) + expect(connection).to receive(:execute).with(<<~SQL).ordered + ALTER INDEX "public"."#{from}" + RENAME TO "#{to}" + SQL + end + + def find_index_create_statement + ActiveRecord::Base.connection.select_value(<<~SQL) + SELECT indexdef + FROM pg_indexes + WHERE schemaname = 'public' + AND indexname = #{ActiveRecord::Base.connection.quote(index.name)} + SQL + end + + def check_index_exists + expect(find_index_create_statement).to eq(original_index) + end +end diff --git a/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb b/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb new file mode 100644 index 00000000000..efb5b8463a1 --- /dev/null +++ b/spec/lib/gitlab/database/reindexing/reindex_action_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing::ReindexAction, '.keep_track_of' do + let(:index) { double('index', identifier: 'public.something', ondisk_size_bytes: 10240, reload: nil) } + let(:size_after) { 512 } + + it 'yields to the caller' do + expect { |b| described_class.keep_track_of(index, &b) }.to yield_control + end + + def find_record + described_class.find_by(index_identifier: index.identifier) + end + + it 'creates the record with a start time and updates its end time' do + freeze_time do + described_class.keep_track_of(index) do + expect(find_record.action_start).to be_within(1.second).of(Time.zone.now) + + travel(10.seconds) + end + + duration = find_record.action_end - find_record.action_start + + expect(duration).to be_within(1.second).of(10.seconds) + end + end + + it 'creates the record with its status set to :started and updates its state to :finished' do + described_class.keep_track_of(index) do + expect(find_record).to be_started + end + + expect(find_record).to be_finished + end + + it 'creates the record with the indexes start size and updates its end size' do + described_class.keep_track_of(index) do + expect(find_record.ondisk_size_bytes_start).to eq(index.ondisk_size_bytes) + + expect(index).to receive(:reload).once + allow(index).to receive(:ondisk_size_bytes).and_return(size_after) + end + + expect(find_record.ondisk_size_bytes_end).to eq(size_after) + end + + context 'in case of errors' do + it 'sets the state to failed' do + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record).to be_failed + end + + it 'records the end time' do + freeze_time do + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record.action_end).to be_within(1.second).of(Time.zone.now) + end + end + + it 'records the resulting index size' do + expect(index).to receive(:reload).once + allow(index).to receive(:ondisk_size_bytes).and_return(size_after) + + expect do + described_class.keep_track_of(index) do + raise 'something went wrong' + end + end.to raise_error(/something went wrong/) + + expect(find_record.ondisk_size_bytes_end).to eq(size_after) + end + end +end diff --git a/spec/lib/gitlab/database/reindexing_spec.rb b/spec/lib/gitlab/database/reindexing_spec.rb new file mode 100644 index 00000000000..26954a9a32f --- /dev/null +++ b/spec/lib/gitlab/database/reindexing_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reindexing do + describe '.perform' do + before do + allow(Gitlab::Database::Reindexing::ReindexAction).to receive(:keep_track_of).and_yield + end + + shared_examples_for 'reindexing' do + before do + indexes.zip(reindexers).each do |index, reindexer| + allow(Gitlab::Database::Reindexing::ConcurrentReindex).to receive(:new).with(index).and_return(reindexer) + allow(reindexer).to receive(:perform) + end + end + + it 'performs concurrent reindexing for each index' do + indexes.zip(reindexers).each do |index, reindexer| + expect(Gitlab::Database::Reindexing::ConcurrentReindex).to receive(:new).with(index).ordered.and_return(reindexer) + expect(reindexer).to receive(:perform) + end + + subject + end + + it 'keeps track of actions and creates ReindexAction records' do + indexes.each do |index| + expect(Gitlab::Database::Reindexing::ReindexAction).to receive(:keep_track_of).with(index).and_yield + end + + subject + end + end + + context 'with multiple indexes' do + subject { described_class.perform(indexes) } + + let(:indexes) { [instance_double('Gitlab::Database::PostgresIndex'), instance_double('Gitlab::Database::PostgresIndex')] } + let(:reindexers) { [instance_double('Gitlab::Database::Reindexing::ConcurrentReindex'), instance_double('Gitlab::Database::Reindexing::ConcurrentReindex')] } + + it_behaves_like 'reindexing' + end + + context 'single index' do + subject { described_class.perform(indexes.first) } + + let(:indexes) { [instance_double('Gitlab::Database::PostgresIndex')] } + let(:reindexers) { [instance_double('Gitlab::Database::Reindexing::ConcurrentReindex')] } + + it_behaves_like 'reindexing' + end + end + + describe '.candidate_indexes' do + subject { described_class.candidate_indexes } + + it 'retrieves regular indexes that are no left-overs from previous runs' do + result = double + expect(Gitlab::Database::PostgresIndex).to receive_message_chain('regular.not_match.not_match').with(no_args).with('^tmp_reindex_').with('^old_reindex_').and_return(result) + + expect(subject).to eq(result) + end + end +end diff --git a/spec/lib/gitlab/database/similarity_score_spec.rb b/spec/lib/gitlab/database/similarity_score_spec.rb index e36a4f610e1..cf75e5a72d9 100644 --- a/spec/lib/gitlab/database/similarity_score_spec.rb +++ b/spec/lib/gitlab/database/similarity_score_spec.rb @@ -90,4 +90,15 @@ RSpec.describe Gitlab::Database::SimilarityScore do expect(subject).to eq(%w[different same gitlab-danger]) end end + + describe 'annotation' do + it 'annotates the generated SQL expression' do + expression = Gitlab::Database::SimilarityScore.build_expression(search: 'test', rules: [ + { column: Arel.sql('path'), multiplier: 1 }, + { column: Arel.sql('name'), multiplier: 0.8 } + ]) + + expect(Gitlab::Database::SimilarityScore).to be_order_by_similarity(expression) + end + end end diff --git a/spec/lib/gitlab/database/with_lock_retries_spec.rb b/spec/lib/gitlab/database/with_lock_retries_spec.rb index 2cc6e175500..220ae705e71 100644 --- a/spec/lib/gitlab/database/with_lock_retries_spec.rb +++ b/spec/lib/gitlab/database/with_lock_retries_spec.rb @@ -104,9 +104,69 @@ RSpec.describe Gitlab::Database::WithLockRetries do end context 'after 3 iterations' do - let(:retry_count) { 4 } + it_behaves_like 'retriable exclusive lock on `projects`' do + let(:retry_count) { 4 } + end + + context 'setting the idle transaction timeout' do + context 'when there is no outer transaction: disable_ddl_transaction! is set in the migration' do + it 'does not disable the idle transaction timeout' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow(subject).to receive(:run_block_with_transaction).once.and_raise(ActiveRecord::LockWaitTimeout) + allow(subject).to receive(:run_block_with_transaction).once + + expect(subject).not_to receive(:disable_idle_in_transaction_timeout) + + subject.run {} + end + end - it_behaves_like 'retriable exclusive lock on `projects`' + context 'when there is outer transaction: disable_ddl_transaction! is not set in the migration' do + it 'disables the idle transaction timeout so the code can sleep and retry' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(true) + + n = 0 + allow(subject).to receive(:run_block_with_transaction).twice do + n += 1 + raise(ActiveRecord::LockWaitTimeout) if n == 1 + end + + expect(subject).to receive(:disable_idle_in_transaction_timeout).once + + subject.run {} + end + end + end + end + + context 'after the retries are exhausted' do + let(:timing_configuration) do + [ + [1.second, 1.second] + ] + end + + context 'when there is no outer transaction: disable_ddl_transaction! is set in the migration' do + it 'does not disable the lock_timeout' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow(subject).to receive(:run_block_with_transaction).once.and_raise(ActiveRecord::LockWaitTimeout) + + expect(subject).not_to receive(:disable_lock_timeout) + + subject.run {} + end + end + + context 'when there is outer transaction: disable_ddl_transaction! is not set in the migration' do + it 'disables the lock_timeout' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(true) + allow(subject).to receive(:run_block_with_transaction).once.and_raise(ActiveRecord::LockWaitTimeout) + + expect(subject).to receive(:disable_lock_timeout) + + subject.run {} + end + end end context 'after the retries, without setting lock_timeout' do diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 420aa0a8df6..dd3daf1b2c5 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -70,25 +70,6 @@ RSpec.describe Gitlab::Database do end end - describe '.postgresql_9_or_less?' do - it 'returns true when using postgresql 8.4' do - allow(described_class).to receive(:version).and_return('8.4') - expect(described_class.postgresql_9_or_less?).to eq(true) - end - - it 'returns true when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') - - expect(described_class.postgresql_9_or_less?).to eq(true) - end - - it 'returns false when using PostgreSQL 10 or newer' do - allow(described_class).to receive(:version).and_return('10') - - expect(described_class.postgresql_9_or_less?).to eq(false) - end - end - describe '.postgresql_minimum_supported_version?' do it 'returns false when using PostgreSQL 10' do allow(described_class).to receive(:version).and_return('10') @@ -150,68 +131,6 @@ RSpec.describe Gitlab::Database do end end - describe '.pg_wal_lsn_diff' do - it 'returns old name when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') - - expect(described_class.pg_wal_lsn_diff).to eq('pg_xlog_location_diff') - end - - it 'returns new name when using PostgreSQL 10 or newer' do - allow(described_class).to receive(:version).and_return('10') - - expect(described_class.pg_wal_lsn_diff).to eq('pg_wal_lsn_diff') - end - end - - describe '.pg_current_wal_insert_lsn' do - it 'returns old name when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') - - expect(described_class.pg_current_wal_insert_lsn).to eq('pg_current_xlog_insert_location') - end - - it 'returns new name when using PostgreSQL 10 or newer' do - allow(described_class).to receive(:version).and_return('10') - - expect(described_class.pg_current_wal_insert_lsn).to eq('pg_current_wal_insert_lsn') - end - end - - describe '.pg_last_wal_receive_lsn' do - it 'returns old name when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') - - expect(described_class.pg_last_wal_receive_lsn).to eq('pg_last_xlog_receive_location') - end - - it 'returns new name when using PostgreSQL 10 or newer' do - allow(described_class).to receive(:version).and_return('10') - - expect(described_class.pg_last_wal_receive_lsn).to eq('pg_last_wal_receive_lsn') - end - end - - describe '.pg_last_wal_replay_lsn' do - it 'returns old name when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') - - expect(described_class.pg_last_wal_replay_lsn).to eq('pg_last_xlog_replay_location') - end - - it 'returns new name when using PostgreSQL 10 or newer' do - allow(described_class).to receive(:version).and_return('10') - - expect(described_class.pg_last_wal_replay_lsn).to eq('pg_last_wal_replay_lsn') - end - end - - describe '.pg_last_xact_replay_timestamp' do - it 'returns pg_last_xact_replay_timestamp' do - expect(described_class.pg_last_xact_replay_timestamp).to eq('pg_last_xact_replay_timestamp') - end - end - describe '.nulls_last_order' do it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column ASC NULLS LAST'} it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC NULLS LAST'} diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_batch_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_batch_spec.rb index bd60c24859c..72a66b0451e 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_batch_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/merge_request_diff_batch_spec.rb @@ -120,7 +120,7 @@ RSpec.describe Gitlab::Diff::FileCollection::MergeRequestDiffBatch do described_class.new(merge_request.merge_request_diff, batch_page, batch_size, - collection_default_args) + **collection_default_args) end end diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb index 7e926f86096..f6810d7a966 100644 --- a/spec/lib/gitlab/diff/highlight_cache_spec.rb +++ b/spec/lib/gitlab/diff/highlight_cache_spec.rb @@ -43,7 +43,8 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do describe '#decorate' do # Manually creates a Diff::File object to avoid triggering the cache on - # the FileCollection::MergeRequestDiff + # the FileCollection::MergeRequestDiff + # let(:diff_file) do diffs = merge_request.diffs raw_diff = diffs.diffable.raw_diffs(diffs.diff_options.merge(paths: ['CHANGELOG'])).first @@ -73,6 +74,37 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do expect(rich_texts).to all(be_html_safe) end + + context "when diff_file is uncached due to default_max_patch_bytes change" do + before do + expect(cache).to receive(:read_file).at_least(:once).and_return([]) + + # Stub out the application's default and current patch size limits. We + # want them to be different, and the diff file to be sized between + # the 2 values. + # + diff_file_size_kb = (diff_file.diff.diff.bytesize * 10) + + stub_const("#{diff_file.diff.class}::DEFAULT_MAX_PATCH_BYTES", diff_file_size_kb - 1 ) + expect(diff_file.diff.class).to receive(:patch_safe_limit_bytes).and_return(diff_file_size_kb + 1) + expect(diff_file.diff.class) + .to receive(:patch_safe_limit_bytes) + .with(diff_file.diff.class::DEFAULT_MAX_PATCH_BYTES) + .and_call_original + end + + it "manually writes highlighted lines to the cache" do + expect(cache).to receive(:write_to_redis_hash).and_call_original + + cache.decorate(diff_file) + end + + it "assigns highlighted diff lines to the DiffFile" do + expect(diff_file.highlighted_diff_lines.size).to be > 5 + + cache.decorate(diff_file) + end + end end shared_examples 'caches missing entries' do diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index 07b8070be30..ef448ee96a4 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -65,24 +65,15 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do end end - [true, false].each do |state_tracking_enabled| - context "and current user can update noteable #{state_tracking_enabled ? 'enabled' : 'disabled'}" do - before do - stub_feature_flags(track_resource_state_change_events: state_tracking_enabled) - - project.add_developer(user) - end + context "and current user can update noteable" do + before do + project.add_developer(user) + end - it 'does not raise an error' do - if state_tracking_enabled - expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1) - else - # One system note is created for the 'close' event - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - end + it 'does not raise an error' do + expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1) - expect(noteable.reload).to be_closed - end + expect(noteable.reload).to be_closed end end end diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb index 01e2fe8ce17..40669f06371 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb @@ -25,13 +25,17 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d let!(:lease) { stub_exclusive_lease(unique_key, 'uuid') } it 'calls the given block' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) } + .to yield_with_args(false, an_instance_of(described_class::SleepingLock)) end it 'calls the given block continuously' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) } + .to yield_with_args(false, an_instance_of(described_class::SleepingLock)) + expect { |b| class_instance.in_lock(unique_key, &b) } + .to yield_with_args(false, an_instance_of(described_class::SleepingLock)) + expect { |b| class_instance.in_lock(unique_key, &b) } + .to yield_with_args(false, an_instance_of(described_class::SleepingLock)) end it 'cancels the exclusive lease after the block' do @@ -74,7 +78,8 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d expect(lease).to receive(:try_obtain).exactly(3).times { nil } expect(lease).to receive(:try_obtain).once { unique_key } - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(true) + expect { |b| class_instance.in_lock(unique_key, &b) } + .to yield_with_args(true, an_instance_of(described_class::SleepingLock)) end end end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 9bc865f4d29..aecbc8734db 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -69,12 +69,26 @@ RSpec.describe Gitlab::Experimentation do end end + describe '#push_frontend_experiment' do + it 'pushes an experiment to the frontend' do + gon = instance_double('gon') + experiments = { experiments: { 'myExperiment' => true } } + + stub_experiment_for_user(my_experiment: true) + allow(controller).to receive(:gon).and_return(gon) + + expect(gon).to receive(:push).with(experiments, true) + + controller.push_frontend_experiment(:my_experiment) + end + end + describe '#experiment_enabled?' do subject { controller.experiment_enabled?(:test_experiment) } context 'cookie is not present' do - it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do - expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil) + it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of nil' do + expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, nil) controller.experiment_enabled?(:test_experiment) end end @@ -85,22 +99,22 @@ RSpec.describe Gitlab::Experimentation do get :index end - it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do + it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do # 'abcd1234'.hex % 100 = 76 - expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, 76) + expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, 76) controller.experiment_enabled?(:test_experiment) end end it 'returns true when DNT: 0 is set in the request' do - allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + allow(Gitlab::Experimentation).to receive(:enabled_for_value?) { true } controller.request.headers['DNT'] = '0' is_expected.to be_truthy end it 'returns false when DNT: 1 is set in the request' do - allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + allow(Gitlab::Experimentation).to receive(:enabled_for_value?) { true } controller.request.headers['DNT'] = '1' is_expected.to be_falsy @@ -336,8 +350,8 @@ RSpec.describe Gitlab::Experimentation do end end - describe '.enabled_for_user?' do - subject { described_class.enabled_for_user?(:test_experiment, experimentation_subject_index) } + describe '.enabled_for_value?' do + subject { described_class.enabled_for_value?(:test_experiment, experimentation_subject_index) } let(:experimentation_subject_index) { 9 } @@ -377,4 +391,32 @@ RSpec.describe Gitlab::Experimentation do end end end + + describe '.enabled_for_attribute?' do + subject { described_class.enabled_for_attribute?(:test_experiment, attribute) } + + let(:attribute) { 'abcd' } # Digest::SHA1.hexdigest('abcd').hex % 100 = 7 + + context 'experiment is disabled' do + before do + allow(described_class).to receive(:enabled?).and_return(false) + end + + it { is_expected.to be false } + end + + context 'experiment is enabled' do + before do + allow(described_class).to receive(:enabled?).and_return(true) + end + + it { is_expected.to be true } + + context 'outside enabled ratio' do + let(:attribute) { 'abc' } # Digest::SHA1.hexdigest('abc').hex % 100 = 17 + + it { is_expected.to be false } + end + end + end end diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index e1bcf4aeeb1..9271f635b14 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -85,9 +85,9 @@ RSpec.describe Gitlab::Git::Branch, :seed_helper do } end - let(:stale_sha) { Timecop.freeze(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago - 5.days) { create_commit } } - let(:active_sha) { Timecop.freeze(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago + 5.days) { create_commit } } - let(:future_sha) { Timecop.freeze(100.days.since) { create_commit } } + let(:stale_sha) { travel_to(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago - 5.days) { create_commit } } + let(:active_sha) { travel_to(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago + 5.days) { create_commit } } + let(:future_sha) { travel_to(100.days.since) { create_commit } } before do repository.create_branch('stale-1', stale_sha) diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index b202015464f..8198c2651a7 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -531,7 +531,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do let(:iterator) { [fake_diff(1, 1)] * 4 } before do - stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: 2, max_lines: max_lines }) + allow(Gitlab::Git::DiffCollection) + .to receive(:default_limits) + .and_return({ max_files: 2, max_lines: max_lines }) end it 'prunes diffs by default even little ones' do @@ -556,7 +558,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do end before do - stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 }) + allow(Gitlab::Git::DiffCollection) + .to receive(:default_limits) + .and_return({ max_files: max_files, max_lines: 80 }) end it 'prunes diffs by default even little ones' do @@ -581,7 +585,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do end before do - stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 }) + allow(Gitlab::Git::DiffCollection) + .to receive(:default_limits) + .and_return({ max_files: max_files, max_lines: 80 }) end it 'prunes diffs by default even little ones' do @@ -665,8 +671,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do end before do - stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', - { max_files: max_files, max_lines: 80 }) + allow(Gitlab::Git::DiffCollection) + .to receive(:default_limits) + .and_return({ max_files: max_files, max_lines: 80 }) end it 'considers size of diffs before the offset for prunning' do diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 117c519e98d..980a52bb61e 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -284,13 +284,21 @@ EOT end describe '#line_count' do - it 'returns the correct number of lines' do - diff = described_class.new(gitaly_diff) + let(:diff) { described_class.new(gitaly_diff) } + it 'returns the correct number of lines' do expect(diff.line_count).to eq(7) end end + describe "#diff_bytesize" do + let(:diff) { described_class.new(gitaly_diff) } + + it "returns the size of the diff in bytes" do + expect(diff.diff_bytesize).to eq(diff.diff.bytesize) + end + end + describe '#too_large?' do it 'returns true for a diff that is too large' do diff = described_class.new(diff: 'a' * 204800) diff --git a/spec/lib/gitlab/git/object_pool_spec.rb b/spec/lib/gitlab/git/object_pool_spec.rb index c8fbc674c73..e1873c6ddb5 100644 --- a/spec/lib/gitlab/git/object_pool_spec.rb +++ b/spec/lib/gitlab/git/object_pool_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Gitlab::Git::ObjectPool do describe '#create' do before do - subject.create + subject.create # rubocop:disable Rails/SaveBang end context "when the pool doesn't exist yet" do @@ -31,7 +31,7 @@ RSpec.describe Gitlab::Git::ObjectPool do context 'when the pool already exists' do it 'raises an FailedPrecondition' do expect do - subject.create + subject.create # rubocop:disable Rails/SaveBang end.to raise_error(GRPC::FailedPrecondition) end end diff --git a/spec/lib/gitlab/git/remote_mirror_spec.rb b/spec/lib/gitlab/git/remote_mirror_spec.rb index 423c4aa9620..92504b7aafe 100644 --- a/spec/lib/gitlab/git/remote_mirror_spec.rb +++ b/spec/lib/gitlab/git/remote_mirror_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Gitlab::Git::RemoteMirror do .to receive(:update_remote_mirror) .with(ref_name, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true) - remote_mirror.update + remote_mirror.update # rubocop:disable Rails/SaveBang end it 'wraps gitaly errors' do @@ -24,7 +24,7 @@ RSpec.describe Gitlab::Git::RemoteMirror do .to receive(:update_remote_mirror) .and_raise(StandardError) - expect { remote_mirror.update }.to raise_error(StandardError) + expect { remote_mirror.update }.to raise_error(StandardError) # rubocop:disable Rails/SaveBang end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 73eecd3401a..3dc0db1bc3c 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2079,7 +2079,7 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do let(:object_pool_rugged) { Rugged::Repository.new(object_pool_path) } before do - object_pool.create + object_pool.create # rubocop:disable Rails/SaveBang end it 'does not raise an error when disconnecting a non-linked repository' do diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb index 4f6a3fb823e..16cea1dc1a3 100644 --- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -7,7 +7,7 @@ require 'tempfile' RSpec.describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do let(:project) { create(:project, :repository) } let(:repository) { project.repository } - let(:feature_flag_name) { 'feature-flag-name' } + let(:feature_flag_name) { wrapper.rugged_feature_keys.first } let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } before(:all) do @@ -47,7 +47,7 @@ RSpec.describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do end end - context 'when feature flag is not persisted' do + context 'when feature flag is not persisted', stub_feature_flags: false do context 'when running puma with multiple threads' do before do allow(subject).to receive(:running_puma_with_multiple_threads?).and_return(true) diff --git a/spec/lib/gitlab/git_access_snippet_spec.rb b/spec/lib/gitlab/git_access_snippet_spec.rb index 3b8b5fd82c6..362ea3c006e 100644 --- a/spec/lib/gitlab/git_access_snippet_spec.rb +++ b/spec/lib/gitlab/git_access_snippet_spec.rb @@ -232,29 +232,6 @@ RSpec.describe Gitlab::GitAccessSnippet do end end - context 'when geo is enabled', if: Gitlab.ee? do - let(:user) { snippet.author } - let!(:primary_node) { FactoryBot.create(:geo_node, :primary) } - - before do - allow(::Gitlab::Database).to receive(:read_only?).and_return(true) - allow(::Gitlab::Geo).to receive(:secondary_with_primary?).and_return(true) - end - - # Without override, push access would return Gitlab::GitAccessResult::CustomAction - it 'skips geo for snippet' do - expect { push_access_check }.to raise_forbidden(/You can't push code to a read-only GitLab instance/) - end - - context 'when user is migration bot' do - let(:user) { migration_bot } - - it 'skips geo for snippet' do - expect { push_access_check }.to raise_forbidden(/You can't push code to a read-only GitLab instance/) - end - end - end - context 'when changes are specific' do let(:changes) { "2d1db523e11e777e49377cfb22d368deec3f0793 ddd0f15ae83993f5cb66a927a28673882e99100b master" } let(:user) { snippet.author } diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 9581b017839..f977fe1638f 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -13,6 +13,10 @@ RSpec.describe Gitlab::GitalyClient::CommitService do let(:client) { described_class.new(repository) } describe '#diff_from_parent' do + before do + stub_feature_flags(increased_diff_limits: false) + end + context 'when a commit has a parent' do it 'sends an RPC request with the parent ID as left commit' do request = Gitaly::CommitDiffRequest.new( diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index 95db6b2b4e0..3d3f381b6d2 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -10,6 +10,10 @@ RSpec.describe Gitlab::GonHelper do end describe '#push_frontend_feature_flag' do + before do + skip_feature_flags_yaml_validation + end + it 'pushes a feature flag to the frontend' do gon = instance_double('gon') thing = stub_feature_flag_gate('thing') diff --git a/spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb b/spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb index e68c1446502..9538c4bae2b 100644 --- a/spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb +++ b/spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Gitlab::GrapeLogging::Loggers::QueueDurationLogger do end it 'returns the correct duration in seconds' do - Timecop.freeze(start_time) do + travel_to(start_time) do subject.before expect(subject.parameters(mock_request, nil)).to eq( { 'queue_duration_s': 1.hour.to_f }) diff --git a/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb b/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb deleted file mode 100644 index af604e1c7d5..00000000000 --- a/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::MarkdownField::Resolver do - include Gitlab::Routing - let(:resolver) { described_class.new(:note) } - - describe '#proc' do - let(:project) { create(:project, :public) } - let(:issue) { create(:issue, project: project) } - let(:note) do - create(:note, - note: "Referencing #{issue.to_reference(full: true)}") - end - - it 'renders markdown correctly' do - expect(resolver.proc.call(note, {}, {})).to include(issue_path(issue)) - end - - context 'when the issue is not publicly accessible' do - let(:project) { create(:project, :private) } - - it 'hides the references from users that are not allowed to see the reference' do - expect(resolver.proc.call(note, {}, {})).not_to include(issue_path(issue)) - end - - it 'shows the reference to users that are allowed to see it' do - expect(resolver.proc.call(note, {}, { current_user: project.owner })) - .to include(issue_path(issue)) - end - end - end -end diff --git a/spec/lib/gitlab/graphql/markdown_field_spec.rb b/spec/lib/gitlab/graphql/markdown_field_spec.rb index e3da925376e..82090f992eb 100644 --- a/spec/lib/gitlab/graphql/markdown_field_spec.rb +++ b/spec/lib/gitlab/graphql/markdown_field_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' RSpec.describe Gitlab::Graphql::MarkdownField do + include Gitlab::Routing + describe '.markdown_field' do it 'creates the field with some default attributes' do field = class_with_markdown_field(:test_html, null: true, method: :hello).fields['testHtml'] @@ -13,7 +15,7 @@ RSpec.describe Gitlab::Graphql::MarkdownField do end context 'developer warnings' do - let(:expected_error) { /Only `method` is allowed to specify the markdown field/ } + let_it_be(:expected_error) { /Only `method` is allowed to specify the markdown field/ } it 'raises when passing a resolver' do expect { class_with_markdown_field(:test_html, null: true, resolver: 'not really') } @@ -27,30 +29,61 @@ RSpec.describe Gitlab::Graphql::MarkdownField do end context 'resolving markdown' do - let(:note) { build(:note, note: '# Markdown!') } - let(:thing_with_markdown) { double('markdown thing', object: note) } - let(:expected_markdown) { '<h1 data-sourcepos="1:1-1:11" dir="auto">Markdown!</h1>' } - let(:query_type) { GraphQL::ObjectType.new } - let(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)} - let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) } + let_it_be(:note) { build(:note, note: '# Markdown!') } + let_it_be(:expected_markdown) { '<h1 data-sourcepos="1:1-1:11" dir="auto">Markdown!</h1>' } + let_it_be(:query_type) { GraphQL::ObjectType.new } + let_it_be(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)} + let_it_be(:query) { GraphQL::Query.new(schema, document: nil, context: {}, variables: {}) } + let_it_be(:context) { GraphQL::Query::Context.new(query: query, values: {}, object: nil) } + + let(:type_class) { class_with_markdown_field(:note_html, null: false) } + let(:type_instance) { type_class.authorized_new(note, context) } + let(:field) { type_class.fields['noteHtml'] } it 'renders markdown from the same property as the field name without the `_html` suffix' do - field = class_with_markdown_field(:note_html, null: false).fields['noteHtml'] + expect(field.to_graphql.resolve(type_instance, {}, context)).to eq(expected_markdown) + end + + context 'when a `method` argument is passed' do + let(:type_class) { class_with_markdown_field(:test_html, null: false, method: :note) } + let(:field) { type_class.fields['testHtml'] } - expect(field.to_graphql.resolve(thing_with_markdown, {}, context)).to eq(expected_markdown) + it 'renders markdown from a specific property' do + expect(field.to_graphql.resolve(type_instance, {}, context)).to eq(expected_markdown) + end end - it 'renders markdown from a specific property when a `method` argument is passed' do - field = class_with_markdown_field(:test_html, null: false, method: :note).fields['testHtml'] + describe 'basic verification that references work' do + let_it_be(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let(:note) { build(:note, note: "Referencing #{issue.to_reference(full: true)}") } + + it 'renders markdown correctly' do + expect(field.to_graphql.resolve(type_instance, {}, context)).to include(issue_path(issue)) + end + + context 'when the issue is not publicly accessible' do + let_it_be(:project) { create(:project, :private) } + + it 'hides the references from users that are not allowed to see the reference' do + expect(field.to_graphql.resolve(type_instance, {}, context)).not_to include(issue_path(issue)) + end + + it 'shows the reference to users that are allowed to see it' do + context = GraphQL::Query::Context.new(query: query, values: { current_user: project.owner }, object: nil) + type_instance = type_class.authorized_new(note, context) - expect(field.to_graphql.resolve(thing_with_markdown, {}, context)).to eq(expected_markdown) + expect(field.to_graphql.resolve(type_instance, {}, context)).to include(issue_path(issue)) + end + end end end end def class_with_markdown_field(name, **args) - Class.new(GraphQL::Schema::Object) do + Class.new(Types::BaseObject) do prepend Gitlab::Graphql::MarkdownField + graphql_name 'MarkdownFieldTest' markdown_field name, **args end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb index 444c10074a0..77a8588e2cb 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb @@ -63,6 +63,17 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do expect(order_list.first.sort_direction).to eq :desc end end + + context 'when ordering by CASE', :aggregate_failuers do + let(:relation) { Project.order(Arel::Nodes::Case.new(Project.arel_table[:pending_delete]).when(true).then(100).else(1000).asc) } + + it 'assigns the right attribute name, named function, and direction' do + expect(order_list.count).to eq 1 + expect(order_list.first.attribute_name).to eq 'pending_delete' + expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::Case) + expect(order_list.first.sort_direction).to eq :asc + end + end end describe '#validate_ordering' do diff --git a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb index c7e7db4d535..fa631aa5666 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb @@ -136,11 +136,12 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) } let(:arel_table) { Project.arel_table } let(:decoded_cursor) { { 'similarity' => 0.5, 'id' => 100 } } + let(:similarity_function_call) { Gitlab::Database::SimilarityScore::SIMILARITY_FUNCTION_CALL_WITH_ANNOTATION } let(:similarity_sql) do [ - '(SIMILARITY(COALESCE("projects"."path", \'\'), \'test\') * CAST(\'1\' AS numeric))', - '(SIMILARITY(COALESCE("projects"."name", \'\'), \'test\') * CAST(\'0.7\' AS numeric))', - '(SIMILARITY(COALESCE("projects"."description", \'\'), \'test\') * CAST(\'0.2\' AS numeric))' + "(#{similarity_function_call}(COALESCE(\"projects\".\"path\", ''), 'test') * CAST('1' AS numeric))", + "(#{similarity_function_call}(COALESCE(\"projects\".\"name\", ''), 'test') * CAST('0.7' AS numeric))", + "(#{similarity_function_call}(COALESCE(\"projects\".\"description\", ''), 'test') * CAST('0.2' AS numeric))" ].join(' + ') end diff --git a/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb index 89d2ab8bb87..c8432513185 100644 --- a/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb +++ b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do end it 'returns a duration in seconds' do - allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2]) + allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2, [[], []]]) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) allow(Gitlab::GraphqlLogger).to receive(:info) diff --git a/spec/lib/gitlab/group_search_results_spec.rb b/spec/lib/gitlab/group_search_results_spec.rb index 045c922783a..009f66d2108 100644 --- a/spec/lib/gitlab/group_search_results_spec.rb +++ b/spec/lib/gitlab/group_search_results_spec.rb @@ -17,10 +17,17 @@ RSpec.describe Gitlab::GroupSearchResults do describe 'issues search' do let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') } let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') } + let_it_be(:confidential_result) { create(:issue, :confidential, project: project, title: 'foo confidential') } + let(:query) { 'foo' } let(:scope) { 'issues' } + before do + project.add_developer(user) + end + include_examples 'search results filtered by state' + include_examples 'search results filtered by confidential' end describe 'merge_requests search' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 3126d87a0d6..7c33f29ebf3 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -51,6 +51,7 @@ issues: - status_page_published_incident - namespace - note_authors +- issue_email_participants events: - author - project @@ -242,6 +243,7 @@ ci_pipelines: - latest_builds_report_results - messages - pipeline_artifacts +- latest_statuses ci_refs: - project - ci_pipelines @@ -536,6 +538,8 @@ project: - vulnerability_historical_statistics - product_analytics_events - pipeline_artifacts +- terraform_states +- alert_management_http_integrations award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb index 93b6f93f0ec..d084b9d7f7e 100644 --- a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb @@ -10,14 +10,17 @@ RSpec.describe Gitlab::ImportExport::FastHashSerializer do # all items are properly serialized while traversing the simple hash. subject { Gitlab::Json.parse(Gitlab::Json.generate(described_class.new(project, tree).execute)) } - let!(:project) { setup_project } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { setup_project } let(:shared) { project.import_export_shared } let(:reader) { Gitlab::ImportExport::Reader.new(shared: shared) } let(:tree) { reader.project_tree } - before do + before_all do project.add_maintainer(user) + end + + before do allow_any_instance_of(MergeRequest).to receive(:source_branch_sha).and_return('ABCD') allow_any_instance_of(MergeRequest).to receive(:target_branch_sha).and_return('DCBA') end @@ -224,7 +227,6 @@ RSpec.describe Gitlab::ImportExport::FastHashSerializer do group: group, approvals_before_merge: 1 ) - allow(project).to receive(:commit).and_return(Commit.new(RepoHelpers.sample_commit, project)) issue = create(:issue, assignees: [user], project: project) snippet = create(:project_snippet, project: project) diff --git a/spec/lib/gitlab/import_export/group/relation_factory_spec.rb b/spec/lib/gitlab/import_export/group/relation_factory_spec.rb index eb9a3fa9bd8..6b2f80cc80a 100644 --- a/spec/lib/gitlab/import_export/group/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/group/relation_factory_spec.rb @@ -5,16 +5,19 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::Group::RelationFactory do let(:group) { create(:group) } let(:members_mapper) { double('members_mapper').as_null_object } - let(:user) { create(:admin) } + let(:admin) { create(:admin) } + let(:importer_user) { admin } let(:excluded_keys) { [] } let(:created_object) do - described_class.create(relation_sym: relation_sym, - relation_hash: relation_hash, - members_mapper: members_mapper, - object_builder: Gitlab::ImportExport::Group::ObjectBuilder, - user: user, - importable: group, - excluded_keys: excluded_keys) + described_class.create( + relation_sym: relation_sym, + relation_hash: relation_hash, + members_mapper: members_mapper, + object_builder: Gitlab::ImportExport::Group::ObjectBuilder, + user: importer_user, + importable: group, + excluded_keys: excluded_keys + ) end context 'label object' do @@ -24,18 +27,18 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do let(:relation_hash) do { - 'id' => 123456, - 'title' => 'Bruffefunc', - 'color' => '#1d2da4', - 'project_id' => nil, - 'created_at' => '2019-11-20T17:02:20.546Z', - 'updated_at' => '2019-11-20T17:02:20.546Z', - 'template' => false, + 'id' => 123456, + 'title' => 'Bruffefunc', + 'color' => '#1d2da4', + 'project_id' => nil, + 'created_at' => '2019-11-20T17:02:20.546Z', + 'updated_at' => '2019-11-20T17:02:20.546Z', + 'template' => false, 'description' => 'Description', - 'group_id' => original_group_id, - 'type' => 'GroupLabel', - 'priorities' => [], - 'textColor' => '#FFFFFF' + 'group_id' => original_group_id, + 'type' => 'GroupLabel', + 'priorities' => [], + 'textColor' => '#FFFFFF' } end @@ -60,58 +63,28 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do end end - context 'Notes user references' do - let(:relation_sym) { :notes } - let(:new_user) { create(:user) } - let(:exported_member) do - { - 'id' => 111, - 'access_level' => 30, - 'source_id' => 1, - 'source_type' => 'Namespace', - 'user_id' => 3, - 'notification_level' => 3, - 'created_at' => '2016-11-18T09:29:42.634Z', - 'updated_at' => '2016-11-18T09:29:42.634Z', - 'user' => { - 'id' => 999, - 'email' => new_user.email, - 'username' => new_user.username - } - } - end - + it_behaves_like 'Notes user references' do + let(:importable) { group } let(:relation_hash) do { - 'id' => 4947, - 'note' => 'note', + 'id' => 4947, + 'note' => 'note', 'noteable_type' => 'Epic', - 'author_id' => 999, - 'created_at' => '2016-11-18T09:29:42.634Z', - 'updated_at' => '2016-11-18T09:29:42.634Z', - 'project_id' => 1, - 'attachment' => { + 'author_id' => 999, + 'created_at' => '2016-11-18T09:29:42.634Z', + 'updated_at' => '2016-11-18T09:29:42.634Z', + 'project_id' => 1, + 'attachment' => { 'url' => nil }, - 'noteable_id' => 377, - 'system' => true, - 'author' => { + 'noteable_id' => 377, + 'system' => true, + 'author' => { 'name' => 'Administrator' }, 'events' => [] } end - - let(:members_mapper) do - Gitlab::ImportExport::MembersMapper.new( - exported_members: [exported_member], - user: user, - importable: group) - end - - it 'maps the right author to the imported note' do - expect(created_object.author).to eq(new_user) - end end def random_id diff --git a/spec/lib/gitlab/import_export/lfs_saver_spec.rb b/spec/lib/gitlab/import_export/lfs_saver_spec.rb index db76eb9538b..55b4f7479b8 100644 --- a/spec/lib/gitlab/import_export/lfs_saver_spec.rb +++ b/spec/lib/gitlab/import_export/lfs_saver_spec.rb @@ -74,14 +74,6 @@ RSpec.describe Gitlab::ImportExport::LfsSaver do } ) end - - it 'does not save a json file if feature is disabled' do - stub_feature_flags(export_lfs_objects_projects: false) - - saver.save - - expect(File.exist?(lfs_json_file)).to eq(false) - end end end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 31cf2362628..50bc6a30044 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -3,19 +3,22 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::Project::RelationFactory do - let(:group) { create(:group) } + let(:group) { create(:group) } let(:project) { create(:project, :repository, group: group) } let(:members_mapper) { double('members_mapper').as_null_object } - let(:user) { create(:admin) } + let(:admin) { create(:admin) } + let(:importer_user) { admin } let(:excluded_keys) { [] } let(:created_object) do - described_class.create(relation_sym: relation_sym, - relation_hash: relation_hash, - object_builder: Gitlab::ImportExport::Project::ObjectBuilder, - members_mapper: members_mapper, - user: user, - importable: project, - excluded_keys: excluded_keys) + described_class.create( + relation_sym: relation_sym, + relation_hash: relation_hash, + object_builder: Gitlab::ImportExport::Project::ObjectBuilder, + members_mapper: members_mapper, + user: importer_user, + importable: project, + excluded_keys: excluded_keys + ) end before do @@ -113,9 +116,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do "created_at" => "2016-11-18T09:29:42.634Z", "updated_at" => "2016-11-18T09:29:42.634Z", "user" => { - "id" => user.id, - "email" => user.email, - "username" => user.username + "id" => admin.id, + "email" => admin.email, + "username" => admin.username } } end @@ -123,7 +126,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do let(:members_mapper) do Gitlab::ImportExport::MembersMapper.new( exported_members: [exported_member], - user: user, + user: importer_user, importable: project) end @@ -134,9 +137,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do 'source_branch' => "feature_conflict", 'source_project_id' => project.id, 'target_project_id' => project.id, - 'author_id' => user.id, - 'assignee_id' => user.id, - 'updated_by_id' => user.id, + 'author_id' => admin.id, + 'assignee_id' => admin.id, + 'updated_by_id' => admin.id, 'title' => "MR1", 'created_at' => "2016-06-14T15:02:36.568Z", 'updated_at' => "2016-06-14T15:02:56.815Z", @@ -151,11 +154,11 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do end it 'has preloaded author' do - expect(created_object.author).to equal(user) + expect(created_object.author).to equal(admin) end it 'has preloaded updated_by' do - expect(created_object.updated_by).to equal(user) + expect(created_object.updated_by).to equal(admin) end it 'has preloaded source project' do @@ -264,27 +267,8 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do end end - context 'Notes user references' do - let(:relation_sym) { :notes } - let(:new_user) { create(:user) } - let(:exported_member) do - { - "id" => 111, - "access_level" => 30, - "source_id" => 1, - "source_type" => "Project", - "user_id" => 3, - "notification_level" => 3, - "created_at" => "2016-11-18T09:29:42.634Z", - "updated_at" => "2016-11-18T09:29:42.634Z", - "user" => { - "id" => 999, - "email" => new_user.email, - "username" => new_user.username - } - } - end - + it_behaves_like 'Notes user references' do + let(:importable) { project } let(:relation_hash) do { "id" => 4947, @@ -305,17 +289,6 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do "events" => [] } end - - let(:members_mapper) do - Gitlab::ImportExport::MembersMapper.new( - exported_members: [exported_member], - user: user, - importable: project) - end - - it 'maps the right author to the imported note' do - expect(created_object.author).to eq(new_user) - end end context 'encrypted attributes' do diff --git a/spec/lib/gitlab/job_waiter_spec.rb b/spec/lib/gitlab/job_waiter_spec.rb index 7aa0a3485fb..a9edb2b530b 100644 --- a/spec/lib/gitlab/job_waiter_spec.rb +++ b/spec/lib/gitlab/job_waiter_spec.rb @@ -2,23 +2,26 @@ require 'spec_helper' -RSpec.describe Gitlab::JobWaiter do +RSpec.describe Gitlab::JobWaiter, :redis do describe '.notify' do it 'pushes the jid to the named queue' do - key = 'gitlab:job_waiter:foo' - jid = 1 + key = described_class.new.key - redis = double('redis') - expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) - expect(redis).to receive(:lpush).with(key, jid) + described_class.notify(key, 123) - described_class.notify(key, jid) + Gitlab::Redis::SharedState.with do |redis| + expect(redis.ttl(key)).to be > 0 + end end end describe '#wait' do let(:waiter) { described_class.new(2) } + before do + allow_any_instance_of(described_class).to receive(:wait).and_call_original + end + it 'returns when all jobs have been completed' do described_class.notify(waiter.key, 'a') described_class.notify(waiter.key, 'b') diff --git a/spec/lib/gitlab/lfs/client_spec.rb b/spec/lib/gitlab/lfs/client_spec.rb index 03563a632d6..14af9d02cb5 100644 --- a/spec/lib/gitlab/lfs/client_spec.rb +++ b/spec/lib/gitlab/lfs/client_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Gitlab::Lfs::Client do let(:username) { 'user' } let(:password) { 'password' } let(:credentials) { { user: username, password: password, auth_method: 'password' } } + let(:git_lfs_content_type) { 'application/vnd.git-lfs+json' } + let(:git_lfs_user_agent) { "GitLab #{Gitlab::VERSION} LFS client" } let(:basic_auth_headers) do { 'Authorization' => "Basic #{Base64.strict_encode64("#{username}:#{password}")}" } @@ -21,6 +23,15 @@ RSpec.describe Gitlab::Lfs::Client do } end + let(:verify_action) do + { + "href" => "#{base_url}/some/file/verify", + "header" => { + "Key" => "value" + } + } + end + subject(:lfs_client) { described_class.new(base_url, credentials: credentials) } describe '#batch' do @@ -34,10 +45,10 @@ RSpec.describe Gitlab::Lfs::Client do ).to_return( status: 200, body: { 'objects' => 'anything', 'transfer' => 'basic' }.to_json, - headers: { 'Content-Type' => 'application/vnd.git-lfs+json' } + headers: { 'Content-Type' => git_lfs_content_type } ) - result = lfs_client.batch('upload', objects) + result = lfs_client.batch!('upload', objects) expect(stub).to have_been_requested expect(result).to eq('objects' => 'anything', 'transfer' => 'basic') @@ -48,7 +59,7 @@ RSpec.describe Gitlab::Lfs::Client do it 'raises an error' do stub_batch(objects: objects, headers: basic_auth_headers).to_return(status: 400) - expect { lfs_client.batch('upload', objects) }.to raise_error(/Failed/) + expect { lfs_client.batch!('upload', objects) }.to raise_error(/Failed/) end end @@ -56,7 +67,7 @@ RSpec.describe Gitlab::Lfs::Client do it 'raises an error' do stub_batch(objects: objects, headers: basic_auth_headers).to_return(status: 400) - expect { lfs_client.batch('upload', objects) }.to raise_error(/Failed/) + expect { lfs_client.batch!('upload', objects) }.to raise_error(/Failed/) end end @@ -68,17 +79,23 @@ RSpec.describe Gitlab::Lfs::Client do ).to_return( status: 200, body: { 'transfer' => 'carrier-pigeon' }.to_json, - headers: { 'Content-Type' => 'application/vnd.git-lfs+json' } + headers: { 'Content-Type' => git_lfs_content_type } ) - expect { lfs_client.batch('upload', objects) }.to raise_error(/Unsupported transfer/) + expect { lfs_client.batch!('upload', objects) }.to raise_error(/Unsupported transfer/) end end def stub_batch(objects:, headers:, operation: 'upload', transfer: 'basic') - objects = objects.map { |o| { oid: o.oid, size: o.size } } + objects = objects.as_json(only: [:oid, :size]) body = { operation: operation, 'transfers': [transfer], objects: objects }.to_json + headers = { + 'Accept' => git_lfs_content_type, + 'Content-Type' => git_lfs_content_type, + 'User-Agent' => git_lfs_user_agent + }.merge(headers) + stub_request(:post, base_url + '/info/lfs/objects/batch').with(body: body, headers: headers) end end @@ -90,7 +107,7 @@ RSpec.describe Gitlab::Lfs::Client do it "makes an HTTP PUT with expected parameters" do stub_upload(object: object, headers: upload_action['header']).to_return(status: 200) - lfs_client.upload(object, upload_action, authenticated: true) + lfs_client.upload!(object, upload_action, authenticated: true) end end @@ -101,7 +118,7 @@ RSpec.describe Gitlab::Lfs::Client do headers: basic_auth_headers.merge(upload_action['header']) ).to_return(status: 200) - lfs_client.upload(object, upload_action, authenticated: false) + lfs_client.upload!(object, upload_action, authenticated: false) expect(stub).to have_been_requested end @@ -110,13 +127,13 @@ RSpec.describe Gitlab::Lfs::Client do context 'LFS object has no file' do let(:object) { LfsObject.new } - it 'makes an HJTT PUT with expected parameters' do + it 'makes an HTTP PUT with expected parameters' do stub = stub_upload( object: object, headers: upload_action['header'] ).to_return(status: 200) - lfs_client.upload(object, upload_action, authenticated: true) + lfs_client.upload!(object, upload_action, authenticated: true) expect(stub).to have_been_requested end @@ -126,7 +143,7 @@ RSpec.describe Gitlab::Lfs::Client do it 'raises an error' do stub_upload(object: object, headers: upload_action['header']).to_return(status: 400) - expect { lfs_client.upload(object, upload_action, authenticated: true) }.to raise_error(/Failed/) + expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed/) end end @@ -134,15 +151,75 @@ RSpec.describe Gitlab::Lfs::Client do it 'raises an error' do stub_upload(object: object, headers: upload_action['header']).to_return(status: 500) - expect { lfs_client.upload(object, upload_action, authenticated: true) }.to raise_error(/Failed/) + expect { lfs_client.upload!(object, upload_action, authenticated: true) }.to raise_error(/Failed/) end end def stub_upload(object:, headers:) + headers = { + 'Content-Type' => 'application/octet-stream', + 'Content-Length' => object.size.to_s, + 'User-Agent' => git_lfs_user_agent + }.merge(headers) + stub_request(:put, upload_action['href']).with( body: object.file.read, headers: headers.merge('Content-Length' => object.size.to_s) ) end end + + describe "#verify" do + let_it_be(:object) { create(:lfs_object) } + + context 'server returns 200 OK to an authenticated request' do + it "makes an HTTP POST with expected parameters" do + stub_verify(object: object, headers: verify_action['header']).to_return(status: 200) + + lfs_client.verify!(object, verify_action, authenticated: true) + end + end + + context 'server returns 200 OK to an unauthenticated request' do + it "makes an HTTP POST with expected parameters" do + stub = stub_verify( + object: object, + headers: basic_auth_headers.merge(upload_action['header']) + ).to_return(status: 200) + + lfs_client.verify!(object, verify_action, authenticated: false) + + expect(stub).to have_been_requested + end + end + + context 'server returns 400 error' do + it 'raises an error' do + stub_verify(object: object, headers: verify_action['header']).to_return(status: 400) + + expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed/) + end + end + + context 'server returns 500 error' do + it 'raises an error' do + stub_verify(object: object, headers: verify_action['header']).to_return(status: 500) + + expect { lfs_client.verify!(object, verify_action, authenticated: true) }.to raise_error(/Failed/) + end + end + + def stub_verify(object:, headers:) + headers = { + 'Accept' => git_lfs_content_type, + 'Content-Type' => git_lfs_content_type, + 'User-Agent' => git_lfs_user_agent + }.merge(headers) + + stub_request(:post, verify_action['href']).with( + body: object.to_json(only: [:oid, :size]), + headers: headers + ) + end + end end diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb index 9b8b2c1417a..4b40e8960b2 100644 --- a/spec/lib/gitlab/lfs_token_spec.rb +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -104,7 +104,7 @@ RSpec.describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do # Needs to be at least LfsToken::DEFAULT_EXPIRE_TIME + 60 seconds # in order to check whether it is valid 1 minute after it has expired - Timecop.freeze(Time.now + described_class::DEFAULT_EXPIRE_TIME + 60) do + travel_to(Time.now + described_class::DEFAULT_EXPIRE_TIME + 60) do expect(lfs_token.token_valid?(expired_token)).to be false end end diff --git a/spec/lib/gitlab/manifest_import/manifest_spec.rb b/spec/lib/gitlab/manifest_import/manifest_spec.rb index 2e8753b0880..352120c079d 100644 --- a/spec/lib/gitlab/manifest_import/manifest_spec.rb +++ b/spec/lib/gitlab/manifest_import/manifest_spec.rb @@ -12,19 +12,7 @@ RSpec.describe Gitlab::ManifestImport::Manifest do end context 'missing or invalid attributes' do - let(:file) { Tempfile.new('foo') } - - before do - content = <<~EOS - <manifest> - <remote review="invalid-url" /> - <project name="platform/build"/> - </manifest> - EOS - - file.write(content) - file.rewind - end + let(:file) { File.open(Rails.root.join('spec/fixtures/invalid_manifest.xml')) } it { expect(manifest.valid?).to be false } diff --git a/spec/lib/gitlab/manifest_import/metadata_spec.rb b/spec/lib/gitlab/manifest_import/metadata_spec.rb new file mode 100644 index 00000000000..c8158d3e148 --- /dev/null +++ b/spec/lib/gitlab/manifest_import/metadata_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::ManifestImport::Metadata, :clean_gitlab_redis_shared_state do + let(:user) { double(id: 1) } + let(:repositories) do + [ + { id: 'test1', url: 'http://demo.host/test1' }, + { id: 'test2', url: 'http://demo.host/test2' } + ] + end + + describe '#save' do + it 'stores data in Redis with an expiry of EXPIRY_TIME' do + status = described_class.new(user) + repositories_key = 'manifest_import:metadata:user:1:repositories' + group_id_key = 'manifest_import:metadata:user:1:group_id' + + status.save(repositories, 2) + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.ttl(repositories_key)).to be_within(5).of(described_class::EXPIRY_TIME) + expect(redis.ttl(group_id_key)).to be_within(5).of(described_class::EXPIRY_TIME) + end + end + end + + describe '#repositories' do + it 'allows repositories to round-trip with symbol keys' do + status = described_class.new(user) + + status.save(repositories, 2) + + expect(status.repositories).to eq(repositories) + end + + it 'uses the fallback when there is nothing in Redis' do + fallback = { manifest_import_repositories: repositories } + status = described_class.new(user, fallback: fallback) + + expect(status.repositories).to eq(repositories) + end + end + + describe '#group_id' do + it 'returns the group ID as an integer' do + status = described_class.new(user) + + status.save(repositories, 2) + + expect(status.group_id).to eq(2) + end + + it 'uses the fallback when there is nothing in Redis' do + fallback = { manifest_import_group_id: 3 } + status = described_class.new(user, fallback: fallback) + + expect(status.group_id).to eq(3) + end + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb index 09d5e048f6a..ff8f5797f9d 100644 --- a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb @@ -8,9 +8,16 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do describe '#execute' do let(:project) { create(:project) } let(:dashboard_path) { 'path/to/dashboard.yml' } + let(:prometheus_adapter) { double('adapter', clear_prometheus_reactive_cache!: nil) } subject { described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path) } + before do + allow_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |update_service| + allow(update_service).to receive(:execute) + end + end + context 'valid dashboard' do let(:dashboard_hash) { load_sample_dashboard } @@ -21,20 +28,32 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do end context 'with existing metrics' do + let(:existing_metric_attributes) do + { + project: project, + identifier: 'metric_b', + title: 'overwrite', + y_label: 'overwrite', + query: 'overwrite', + unit: 'overwrite', + legend: 'overwrite', + dashboard_path: dashboard_path + } + end + let!(:existing_metric) do - create(:prometheus_metric, { - project: project, - identifier: 'metric_b', - title: 'overwrite', - y_label: 'overwrite', - query: 'overwrite', - unit: 'overwrite', - legend: 'overwrite' - }) + create(:prometheus_metric, existing_metric_attributes) + end + + let!(:existing_alert) do + alert = create(:prometheus_alert, project: project, prometheus_metric: existing_metric) + existing_metric.prometheus_alerts << alert + + alert end it 'updates existing PrometheusMetrics' do - described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path).execute + subject.execute expect(existing_metric.reload.attributes.with_indifferent_access).to include({ title: 'Super Chart B', @@ -49,6 +68,15 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do expect { subject.execute }.to change { PrometheusMetric.count }.by(2) end + it 'updates affected environments' do + expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with( + existing_alert.environment.cluster_prometheus_adapter, + project + ).and_return(double('ScheduleUpdateService', execute: true)) + + subject.execute + end + context 'with stale metrics' do let!(:stale_metric) do create(:prometheus_metric, @@ -59,11 +87,45 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do ) end + let!(:stale_alert) do + alert = create(:prometheus_alert, project: project, prometheus_metric: stale_metric) + stale_metric.prometheus_alerts << alert + + alert + end + + it 'updates existing PrometheusMetrics' do + subject.execute + + expect(existing_metric.reload.attributes.with_indifferent_access).to include({ + title: 'Super Chart B', + y_label: 'y_label', + query: 'query', + unit: 'unit', + legend: 'Legend Label' + }) + end + it 'deletes stale metrics' do subject.execute expect { stale_metric.reload }.to raise_error(ActiveRecord::RecordNotFound) end + + it 'deletes stale alert' do + subject.execute + + expect { stale_alert.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'updates affected environments' do + expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with( + existing_alert.environment.cluster_prometheus_adapter, + project + ).and_return(double('ScheduleUpdateService', execute: true)) + + subject.execute + end end end end diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 69b779d36eb..0c77dc540f3 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -38,69 +38,49 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do end context 'request is a health check endpoint' do - it 'increments health endpoint counter' do - env['PATH_INFO'] = '/-/liveness' + ['/-/liveness', '/-/liveness/', '/-/%6D%65%74%72%69%63%73'].each do |path| + context "when path is #{path}" do + before do + env['PATH_INFO'] = path + end - expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get') + it 'increments health endpoint counter rather than overall counter' do + expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get') + expect(described_class).not_to receive(:http_request_total) - subject.call(env) - end - - context 'with trailing slash' do - before do - env['PATH_INFO'] = '/-/liveness/' - end + subject.call(env) + end - it 'increments health endpoint counter' do - expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get') + it 'does not record the request duration' do + expect(described_class).not_to receive(:http_request_duration_seconds) - subject.call(env) - end - end - - context 'with percent encoded values' do - before do - env['PATH_INFO'] = '/-/%6D%65%74%72%69%63%73' # /-/metrics - end - - it 'increments health endpoint counter' do - expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get') - - subject.call(env) + subject.call(env) + end end end end context 'request is not a health check endpoint' do - it 'does not increment health endpoint counter' do - env['PATH_INFO'] = '/-/ordinary-requests' - - expect(described_class).not_to receive(:http_health_requests_total) - - subject.call(env) - end - - context 'path info is a root path' do - before do - env['PATH_INFO'] = '/-/' - end - - it 'does not increment health endpoint counter' do - expect(described_class).not_to receive(:http_health_requests_total) - - subject.call(env) - end - end - - context 'path info is a subpath' do - before do - env['PATH_INFO'] = '/-/health/subpath' - end - - it 'does not increment health endpoint counter' do - expect(described_class).not_to receive(:http_health_requests_total) - - subject.call(env) + ['/-/ordinary-requests', '/-/', '/-/health/subpath'].each do |path| + context "when path is #{path}" do + before do + env['PATH_INFO'] = path + end + + it 'increments overall counter rather than health endpoint counter' do + expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get') + expect(described_class).not_to receive(:http_health_requests_total) + + subject.call(env) + end + + it 'records the request duration' do + expect(described_class) + .to receive_message_chain(:http_request_duration_seconds, :observe) + .with({ method: 'get', status: '200' }, a_positive_execution_time) + + subject.call(env) + end end end end diff --git a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb index cdb48024531..a9dae72f4db 100644 --- a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb +++ b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Gitlab::Middleware::RailsQueueDuration do expect(transaction).to receive(:observe).with(:gitlab_rails_queue_duration_seconds, 1) - Timecop.freeze(Time.at(3)) do + travel_to(Time.at(3)) do expect(middleware.call(env)).to eq('yay') end end diff --git a/spec/lib/gitlab/pagination/offset_pagination_spec.rb b/spec/lib/gitlab/pagination/offset_pagination_spec.rb index be20f0194f7..c9a23170137 100644 --- a/spec/lib/gitlab/pagination/offset_pagination_spec.rb +++ b/spec/lib/gitlab/pagination/offset_pagination_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do let(:request_context) { double("request_context") } - subject do + subject(:paginator) do described_class.new(request_context) end @@ -119,6 +119,34 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do subject.paginate(resource) end end + + it 'does not return the total headers when excluding them' do + expect_no_header('X-Total') + expect_no_header('X-Total-Pages') + expect_header('X-Per-Page', '2') + expect_header('X-Page', '1') + + paginator.paginate(resource, exclude_total_headers: true) + end + end + + context 'when resource is a paginatable array' do + let(:resource) { Kaminari.paginate_array(Project.all.to_a) } + + it_behaves_like 'response with pagination headers' + + it 'only returns the requested resources' do + expect(paginator.paginate(resource).count).to eq(2) + end + + it 'does not return total headers when excluding them' do + expect_no_header('X-Total') + expect_no_header('X-Total-Pages') + expect_header('X-Per-Page', '2') + expect_header('X-Page', '1') + + paginator.paginate(resource, exclude_total_headers: true) + end end end diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index fe0735b8043..a76ad1f6f4c 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -265,9 +265,15 @@ RSpec.describe Gitlab::ProjectSearchResults do let_it_be(:project) { create(:project, :public) } let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') } let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') } + let_it_be(:confidential_result) { create(:issue, :confidential, project: project, title: 'foo confidential') } let(:query) { 'foo' } + before do + project.add_developer(user) + end + include_examples 'search results filtered by state' + include_examples 'search results filtered by confidential' end end diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index fa45c605b1b..98bd2efdbc6 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -8,9 +8,9 @@ RSpec.describe Gitlab::ProjectTemplate do expected = %w[ rails spring express iosswift dotnetcore android gomicro gatsby hugo jekyll plainhtml gitbook - hexo sse_middleman nfhugo nfjekyll nfplainhtml - nfgitbook nfhexo salesforcedx serverless_framework - jsonnet cluster_management + hexo sse_middleman gitpod_spring_petclinic nfhugo + nfjekyll nfplainhtml nfgitbook nfhexo salesforcedx + serverless_framework jsonnet cluster_management ] expect(described_class.all).to be_an(Array) diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb index 8abc944eeb1..b2350eff9f9 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery do around do |example| - Timecop.freeze(Time.local(2008, 9, 1, 12, 0, 0)) { example.run } + travel_to(Time.local(2008, 9, 1, 12, 0, 0)) { example.run } end include_examples 'additional metrics query' do diff --git a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb index 4683c4eae28..66b93d0dd72 100644 --- a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Gitlab::Prometheus::Queries::DeploymentQuery do around do |example| time_without_subsecond_values = Time.local(2008, 9, 1, 12, 0, 0) - Timecop.freeze(time_without_subsecond_values) { example.run } + travel_to(time_without_subsecond_values) { example.run } end it 'sends appropriate queries to prometheus' do diff --git a/spec/lib/gitlab/prometheus/query_variables_spec.rb b/spec/lib/gitlab/prometheus/query_variables_spec.rb index 1422d48152a..1dbdb892a5d 100644 --- a/spec/lib/gitlab/prometheus/query_variables_spec.rb +++ b/spec/lib/gitlab/prometheus/query_variables_spec.rb @@ -4,12 +4,12 @@ require 'spec_helper' RSpec.describe Gitlab::Prometheus::QueryVariables do describe '.call' do + let_it_be_with_refind(:environment) { create(:environment) } let(:project) { environment.project } - let(:environment) { create(:environment) } let(:slug) { environment.slug } let(:params) { {} } - subject { described_class.call(environment, params) } + subject { described_class.call(environment, **params) } it { is_expected.to include(ci_environment_slug: slug) } it { is_expected.to include(ci_project_name: project.name) } diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 88c3315150b..704a4e7b224 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -99,6 +99,36 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('foo-') } end + describe '.build_trace_section_regex' do + subject { described_class.build_trace_section_regex } + + context 'without options' do + example = "section_start:1600445393032:NAME\r\033\[0K" + + it { is_expected.to match(example) } + it { is_expected.to match("section_end:12345678:aBcDeFg1234\r\033\[0K") } + it { is_expected.to match("section_start:0:sect_for_alpha-v1.0\r\033\[0K") } + it { is_expected.not_to match("section_start:section:0\r\033\[0K") } + it { is_expected.not_to match("section_:1600445393032:NAME\r\033\[0K") } + it { is_expected.not_to match(example.upcase) } + end + + context 'with options' do + it { is_expected.to match("section_start:1600445393032:NAME[collapsed=true]\r\033\[0K") } + it { is_expected.to match("section_start:1600445393032:NAME[collapsed=true, example_option=false]\r\033\[0K") } + it { is_expected.to match("section_start:1600445393032:NAME[collapsed=true,example_option=false]\r\033\[0K") } + it { is_expected.to match("section_start:1600445393032:NAME[numeric_option=1234567]\r\033\[0K") } + # Without splitting the regex in one for start and one for end, + # this is possible, however, it is ignored for section_end. + it { is_expected.to match("section_end:1600445393032:NAME[collapsed=true]\r\033\[0K") } + it { is_expected.not_to match("section_start:1600445393032:NAME[collapsed=[]]]\r\033\[0K") } + it { is_expected.not_to match("section_start:1600445393032:NAME[collapsed = true]\r\033\[0K") } + it { is_expected.not_to match("section_start:1600445393032:NAME[collapsed = true, example_option=false]\r\033\[0K") } + it { is_expected.not_to match("section_start:1600445393032:NAME[collapsed=true, example_option=false]\r\033\[0K") } + it { is_expected.not_to match("section_start:1600445393032:NAME[]\r\033\[0K") } + end + end + describe '.container_repository_name_regex' do subject { described_class.container_repository_name_regex } @@ -384,6 +414,140 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('%2e%2e%2f1.2.3') } end + describe '.debian_package_name_regex' do + subject { described_class.debian_package_name_regex } + + it { is_expected.to match('0ad') } + it { is_expected.to match('g++') } + it { is_expected.to match('lua5.1') } + it { is_expected.to match('samba') } + + # may not be empty string + it { is_expected.not_to match('') } + # must start with an alphanumeric character + it { is_expected.not_to match('-a') } + it { is_expected.not_to match('+a') } + it { is_expected.not_to match('.a') } + it { is_expected.not_to match('_a') } + # only letters, digits and characters '-+._' + it { is_expected.not_to match('a~') } + it { is_expected.not_to match('aé') } + + # More strict Lintian regex + # at least 2 chars + it { is_expected.not_to match('a') } + # lowercase only + it { is_expected.not_to match('Aa') } + it { is_expected.not_to match('aA') } + # No underscore + it { is_expected.not_to match('a_b') } + end + + describe '.debian_version_regex' do + subject { described_class.debian_version_regex } + + context 'valid versions' do + it { is_expected.to match('1.0') } + it { is_expected.to match('1.0~alpha1') } + it { is_expected.to match('2:4.9.5+dfsg-5+deb10u1') } + end + + context 'dpkg errors' do + # version string is empty + it { is_expected.not_to match('') } + # version string has embedded spaces + it { is_expected.not_to match('1 0') } + # epoch in version is empty + it { is_expected.not_to match(':1.0') } + # epoch in version is not number + it { is_expected.not_to match('a:1.0') } + # epoch in version is negative + it { is_expected.not_to match('-1:1.0') } + # epoch in version is too big + it { is_expected.not_to match('9999999999:1.0') } + # nothing after colon in version number + it { is_expected.not_to match('2:') } + # revision number is empty + # Note: we are less strict here + # it { is_expected.not_to match('1.0-') } + # version number is empty + it { is_expected.not_to match('-1') } + it { is_expected.not_to match('2:-1') } + end + + context 'dpkg warnings' do + # version number does not start with digit + it { is_expected.not_to match('a') } + it { is_expected.not_to match('a1.0') } + # invalid character in version number + it { is_expected.not_to match('1_0') } + # invalid character in revision number + it { is_expected.not_to match('1.0-1_0') } + end + + context 'dpkg accepts' do + # dpkg accepts leading or trailing space + it { is_expected.not_to match(' 1.0') } + it { is_expected.not_to match('1.0 ') } + # dpkg accepts multiple colons + it { is_expected.not_to match('1:2:3') } + end + end + + describe '.debian_architecture_regex' do + subject { described_class.debian_architecture_regex } + + it { is_expected.to match('amd64') } + it { is_expected.to match('kfreebsd-i386') } + + # may not be empty string + it { is_expected.not_to match('') } + # must start with an alphanumeric + it { is_expected.not_to match('-a') } + it { is_expected.not_to match('+a') } + it { is_expected.not_to match('.a') } + it { is_expected.not_to match('_a') } + # only letters, digits and characters '-' + it { is_expected.not_to match('a+b') } + it { is_expected.not_to match('a.b') } + it { is_expected.not_to match('a_b') } + it { is_expected.not_to match('a~') } + it { is_expected.not_to match('aé') } + + # More strict + # Enforce lowercase + it { is_expected.not_to match('AMD64') } + it { is_expected.not_to match('Amd64') } + it { is_expected.not_to match('aMD64') } + end + + describe '.debian_distribution_regex' do + subject { described_class.debian_distribution_regex } + + it { is_expected.to match('buster') } + it { is_expected.to match('buster-updates') } + it { is_expected.to match('Debian10.5') } + + # Do not allow slash, even if this exists in the wild + it { is_expected.not_to match('jessie/updates') } + + # Do not allow Unicode + it { is_expected.not_to match('hé') } + end + + describe '.debian_component_regex' do + subject { described_class.debian_component_regex } + + it { is_expected.to match('main') } + it { is_expected.to match('non-free') } + + # Do not allow slash + it { is_expected.not_to match('non/free') } + + # Do not allow Unicode + it { is_expected.not_to match('hé') } + end + describe '.semver_regex' do subject { described_class.semver_regex } @@ -434,4 +598,45 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('%2e%2e%2f1.2.3') } it { is_expected.not_to match('') } end + + describe '.generic_package_name_regex' do + subject { described_class.generic_package_name_regex } + + it { is_expected.to match('123') } + it { is_expected.to match('foo') } + it { is_expected.to match('foo.bar.baz-2.0-20190901.47283-1') } + it { is_expected.not_to match('../../foo') } + it { is_expected.not_to match('..\..\foo') } + it { is_expected.not_to match('%2f%2e%2e%2f%2essh%2fauthorized_keys') } + it { is_expected.not_to match('$foo/bar') } + it { is_expected.not_to match('my file name') } + it { is_expected.not_to match('!!()()') } + end + + describe '.generic_package_file_name_regex' do + subject { described_class.generic_package_file_name_regex } + + it { is_expected.to match('123') } + it { is_expected.to match('foo') } + it { is_expected.to match('foo.bar.baz-2.0-20190901.47283-1.jar') } + it { is_expected.not_to match('../../foo') } + it { is_expected.not_to match('..\..\foo') } + it { is_expected.not_to match('%2f%2e%2e%2f%2essh%2fauthorized_keys') } + it { is_expected.not_to match('$foo/bar') } + it { is_expected.not_to match('my file name') } + it { is_expected.not_to match('!!()()') } + end + + describe '.prefixed_semver_regex' do + subject { described_class.prefixed_semver_regex } + + it { is_expected.to match('v1.2.3') } + it { is_expected.to match('v1.2.3-beta') } + it { is_expected.to match('v1.2.3-alpha.3') } + it { is_expected.not_to match('v1') } + it { is_expected.not_to match('v1.2') } + it { is_expected.not_to match('v1./2.3') } + it { is_expected.not_to match('v../../../../../1.2.3') } + it { is_expected.not_to match('v%2e%2e%2f1.2.3') } + end end diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index b4cf6a568b4..cdb626778a8 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -11,9 +11,11 @@ RSpec.describe Gitlab::SearchResults do let_it_be(:issue) { create(:issue, project: project, title: 'foo') } let_it_be(:milestone) { create(:milestone, project: project, title: 'foo') } let(:merge_request) { create(:merge_request, source_project: project, title: 'foo') } + let(:query) { 'foo' } let(:filters) { {} } + let(:sort) { nil } - subject(:results) { described_class.new(user, 'foo', Project.order(:id), filters: filters) } + subject(:results) { described_class.new(user, query, Project.order(:id), sort: sort, filters: filters) } context 'as a user with access' do before do @@ -137,10 +139,12 @@ RSpec.describe Gitlab::SearchResults do end describe '#merge_requests' do + let(:scope) { 'merge_requests' } + it 'includes project filter by default' do expect(results).to receive(:project_ids_relation).and_call_original - results.objects('merge_requests') + results.objects(scope) end it 'skips project filter if default project context is used' do @@ -148,24 +152,34 @@ RSpec.describe Gitlab::SearchResults do expect(results).not_to receive(:project_ids_relation) - results.objects('merge_requests') + results.objects(scope) end context 'filtering' do let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') } let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') } - let(:scope) { 'merge_requests' } let(:query) { 'foo' } include_examples 'search results filtered by state' end + + context 'ordering' do + let(:query) { 'sorted' } + let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) } + let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) } + let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) } + + include_examples 'search results sorted' + end end describe '#issues' do + let(:scope) { 'issues' } + it 'includes project filter by default' do expect(results).to receive(:project_ids_relation).and_call_original - results.objects('issues') + results.objects(scope) end it 'skips project filter if default project context is used' do @@ -173,16 +187,25 @@ RSpec.describe Gitlab::SearchResults do expect(results).not_to receive(:project_ids_relation) - results.objects('issues') + results.objects(scope) end context 'filtering' do - let(:scope) { 'issues' } - let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') } let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo open') } + let_it_be(:confidential_result) { create(:issue, :confidential, project: project, title: 'foo confidential') } include_examples 'search results filtered by state' + include_examples 'search results filtered by confidential' + end + + context 'ordering' do + let(:query) { 'sorted' } + let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) } + let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) } + let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) } + + include_examples 'search results sorted' end end diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb index 220ac2ff6da..9bf6f0b82bc 100644 --- a/spec/lib/gitlab/sql/pattern_spec.rb +++ b/spec/lib/gitlab/sql/pattern_spec.rb @@ -3,6 +3,43 @@ require 'spec_helper' RSpec.describe Gitlab::SQL::Pattern do + using RSpec::Parameterized::TableSyntax + + describe '.fuzzy_search' do + let_it_be(:issue1) { create(:issue, title: 'noise foo noise', description: 'noise bar noise') } + let_it_be(:issue2) { create(:issue, title: 'noise baz noise', description: 'noise foo noise') } + let_it_be(:issue3) { create(:issue, title: 'Oh', description: 'Ah') } + + subject(:fuzzy_search) { Issue.fuzzy_search(query, columns) } + + where(:query, :columns, :expected) do + 'foo' | [Issue.arel_table[:title]] | %i[issue1] + + 'foo' | %i[title] | %i[issue1] + 'foo' | %w[title] | %i[issue1] + 'foo' | %i[description] | %i[issue2] + 'foo' | %i[title description] | %i[issue1 issue2] + 'bar' | %i[title description] | %i[issue1] + 'baz' | %i[title description] | %i[issue2] + 'qux' | %i[title description] | [] + + 'oh' | %i[title description] | %i[issue3] + 'OH' | %i[title description] | %i[issue3] + 'ah' | %i[title description] | %i[issue3] + 'AH' | %i[title description] | %i[issue3] + 'oh' | %i[title] | %i[issue3] + 'ah' | %i[description] | %i[issue3] + end + + with_them do + let(:expected_issues) { expected.map { |sym| send(sym) } } + + it 'finds the expected issues' do + expect(fuzzy_search).to match_array(expected_issues) + end + end + end + describe '.to_pattern' do subject(:to_pattern) { User.to_pattern(query) } diff --git a/spec/lib/gitlab/static_site_editor/config/file_config/entry/global_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config/entry/global_spec.rb new file mode 100644 index 00000000000..9ce6007165b --- /dev/null +++ b/spec/lib/gitlab/static_site_editor/config/file_config/entry/global_spec.rb @@ -0,0 +1,245 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::Global do + let(:global) { described_class.new(hash) } + let(:default_image_upload_path_value) { 'source/images' } + + let(:default_mounts_value) do + [ + { + source: 'source', + target: '' + } + ] + end + + let(:default_static_site_generator_value) { 'middleman' } + + shared_examples_for 'valid default configuration' do + describe '#compose!' do + before do + global.compose! + end + + it 'creates nodes hash' do + expect(global.descendants).to be_an Array + end + + it 'creates node object for each entry' do + expect(global.descendants.count).to eq 3 + end + + it 'creates node object using valid class' do + expect(global.descendants.map(&:class)).to match_array(expected_node_object_classes) + end + + it 'sets a description containing "Static Site Editor" for all nodes' do + expect(global.descendants.map(&:description)).to all(match(/Static Site Editor/)) + end + + describe '#leaf?' do + it 'is not leaf' do + expect(global).not_to be_leaf + end + end + end + + context 'when not composed' do + describe '#static_site_generator_value' do + it 'returns nil' do + expect(global.static_site_generator_value).to be nil + end + end + + describe '#leaf?' do + it 'is leaf' do + expect(global).to be_leaf + end + end + end + + context 'when composed' do + before do + global.compose! + end + + describe '#errors' do + it 'has no errors' do + expect(global.errors).to be_empty + end + end + + describe '#image_upload_path_value' do + it 'returns correct values' do + expect(global.image_upload_path_value).to eq(default_image_upload_path_value) + end + end + + describe '#mounts_value' do + it 'returns correct values' do + expect(global.mounts_value).to eq(default_mounts_value) + end + end + + describe '#static_site_generator_value' do + it 'returns correct values' do + expect(global.static_site_generator_value).to eq(default_static_site_generator_value) + end + end + end + end + + describe '.nodes' do + it 'returns a hash' do + expect(described_class.nodes).to be_a(Hash) + end + + context 'when filtering all the entry/node names' do + it 'contains the expected node names' do + expected_node_names = %i[ + image_upload_path + mounts + static_site_generator + ] + expect(described_class.nodes.keys).to match_array(expected_node_names) + end + end + end + + context 'when configuration is valid' do + context 'when some entries defined' do + let(:expected_node_object_classes) do + [ + Gitlab::StaticSiteEditor::Config::FileConfig::Entry::ImageUploadPath, + Gitlab::StaticSiteEditor::Config::FileConfig::Entry::Mounts, + Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator + ] + end + + let(:hash) do + { + image_upload_path: default_image_upload_path_value, + mounts: default_mounts_value, + static_site_generator: default_static_site_generator_value + } + end + + it_behaves_like 'valid default configuration' + end + end + + context 'when value is an empty hash' do + let(:expected_node_object_classes) do + [ + Gitlab::Config::Entry::Unspecified, + Gitlab::Config::Entry::Unspecified, + Gitlab::Config::Entry::Unspecified + ] + end + + let(:hash) { {} } + + it_behaves_like 'valid default configuration' + end + + context 'when configuration is not valid' do + before do + global.compose! + end + + context 'when a single entry is invalid' do + let(:hash) do + { image_upload_path: { not_a_string: true } } + end + + describe '#errors' do + it 'reports errors' do + expect(global.errors) + .to include 'image_upload_path config should be a string' + end + end + end + + context 'when a multiple entries are invalid' do + let(:hash) do + { + image_upload_path: { not_a_string: true }, + static_site_generator: { not_a_string: true } + } + end + + describe '#errors' do + it 'reports errors' do + expect(global.errors) + .to match_array([ + 'image_upload_path config should be a string', + 'static_site_generator config should be a string', + "static_site_generator config should be 'middleman'" + ]) + end + end + end + + context 'when there is an invalid key' do + let(:hash) do + { invalid_key: true } + end + + describe '#errors' do + it 'reports errors' do + expect(global.errors) + .to include 'global config contains unknown keys: invalid_key' + end + end + end + end + + context 'when value is not a hash' do + let(:hash) { [] } + + describe '#valid?' do + it 'is not valid' do + expect(global).not_to be_valid + end + end + + describe '#errors' do + it 'returns error about invalid type' do + expect(global.errors.first).to match /should be a hash/ + end + end + end + + describe '#specified?' do + it 'is concrete entry that is defined' do + expect(global.specified?).to be true + end + end + + describe '#[]' do + before do + global.compose! + end + + let(:hash) do + { static_site_generator: default_static_site_generator_value } + end + + context 'when entry exists' do + it 'returns correct entry' do + expect(global[:static_site_generator]) + .to be_an_instance_of Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator + expect(global[:static_site_generator].value).to eq default_static_site_generator_value + end + end + + context 'when entry does not exist' do + it 'always return unspecified node' do + expect(global[:some][:unknown][:node]) + .not_to be_specified + end + end + end +end diff --git a/spec/lib/gitlab/static_site_editor/config/file_config/entry/image_upload_path_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config/entry/image_upload_path_spec.rb new file mode 100644 index 00000000000..c2b7fbf6f98 --- /dev/null +++ b/spec/lib/gitlab/static_site_editor/config/file_config/entry/image_upload_path_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::ImageUploadPath do + subject(:image_upload_path_entry) { described_class.new(config) } + + describe 'validations' do + context 'with a valid config' do + let(:config) { 'an-image-upload-path' } + + it { is_expected.to be_valid } + + describe '#value' do + it 'returns a image_upload_path key' do + expect(image_upload_path_entry.value).to eq config + end + end + end + + context 'with an invalid config' do + let(:config) { { not_a_string: true } } + + it { is_expected.not_to be_valid } + + it 'reports errors about wrong type' do + expect(image_upload_path_entry.errors) + .to include 'image upload path config should be a string' + end + end + end + + describe '.default' do + it 'returns default image_upload_path' do + expect(described_class.default).to eq 'source/images' + end + end +end diff --git a/spec/lib/gitlab/static_site_editor/config/file_config/entry/mount_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config/entry/mount_spec.rb new file mode 100644 index 00000000000..04248fc60a5 --- /dev/null +++ b/spec/lib/gitlab/static_site_editor/config/file_config/entry/mount_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::Mount do + subject(:entry) { described_class.new(config) } + + describe 'validations' do + context 'with a valid config' do + context 'and target is a non-empty string' do + let(:config) do + { + source: 'source', + target: 'sub-site' + } + end + + it { is_expected.to be_valid } + + describe '#value' do + it 'returns mount configuration' do + expect(entry.value).to eq config + end + end + end + + context 'and target is an empty string' do + let(:config) do + { + source: 'source', + target: '' + } + end + + it { is_expected.to be_valid } + + describe '#value' do + it 'returns mount configuration' do + expect(entry.value).to eq config + end + end + end + end + + context 'with an invalid config' do + context 'when source is not a string' do + let(:config) { { source: 123, target: 'target' } } + + it { is_expected.not_to be_valid } + + it 'reports error' do + expect(entry.errors) + .to include 'mount source should be a string' + end + end + + context 'when source is not present' do + let(:config) { { target: 'target' } } + + it { is_expected.not_to be_valid } + + it 'reports error' do + expect(entry.errors) + .to include "mount source can't be blank" + end + end + + context 'when target is not a string' do + let(:config) { { source: 'source', target: 123 } } + + it { is_expected.not_to be_valid } + + it 'reports error' do + expect(entry.errors) + .to include 'mount target should be a string' + end + end + + context 'when there is an unknown key present' do + let(:config) { { test: 100 } } + + it { is_expected.not_to be_valid } + + it 'reports error' do + expect(entry.errors) + .to include 'mount config contains unknown keys: test' + end + end + end + end + + describe '.default' do + it 'returns default mount' do + expect(described_class.default) + .to eq({ + source: 'source', + target: '' + }) + end + end +end diff --git a/spec/lib/gitlab/static_site_editor/config/file_config/entry/mounts_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config/entry/mounts_spec.rb new file mode 100644 index 00000000000..0ae2ece9474 --- /dev/null +++ b/spec/lib/gitlab/static_site_editor/config/file_config/entry/mounts_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::Mounts do + subject(:entry) { described_class.new(config) } + + describe 'validations' do + context 'with a valid config' do + let(:config) do + [ + { + source: 'source', + target: '' + }, + { + source: 'sub-site/source', + target: 'sub-site' + } + ] + end + + it { is_expected.to be_valid } + + describe '#value' do + it 'returns mounts configuration' do + expect(entry.value).to eq config + end + end + end + + context 'with an invalid config' do + let(:config) { { not_an_array: true } } + + it { is_expected.not_to be_valid } + + it 'reports errors about wrong type' do + expect(entry.errors) + .to include 'mounts config should be a array' + end + end + end + + describe '.default' do + it 'returns default mounts' do + expect(described_class.default) + .to eq([{ + source: 'source', + target: '' + }]) + end + end +end diff --git a/spec/lib/gitlab/static_site_editor/config/file_config/entry/static_site_generator_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config/entry/static_site_generator_spec.rb new file mode 100644 index 00000000000..a9c730218cf --- /dev/null +++ b/spec/lib/gitlab/static_site_editor/config/file_config/entry/static_site_generator_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator do + let(:static_site_generator) { described_class.new(config) } + + describe 'validations' do + context 'when value is valid' do + let(:config) { 'middleman' } + + describe '#value' do + it 'returns a static_site_generator key' do + expect(static_site_generator.value).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(static_site_generator).to be_valid + end + end + end + + context 'when value is invalid' do + let(:config) { 'not-a-valid-generator' } + + describe '#valid?' do + it 'is not valid' do + expect(static_site_generator).not_to be_valid + end + end + end + + context 'when value has a wrong type' do + let(:config) { { not_a_string: true } } + + it 'reports errors about wrong type' do + expect(static_site_generator.errors) + .to include 'static site generator config should be a string' + end + end + end + + describe '.default' do + it 'returns default static_site_generator' do + expect(described_class.default).to eq 'middleman' + end + end +end diff --git a/spec/lib/gitlab/static_site_editor/config/file_config_spec.rb b/spec/lib/gitlab/static_site_editor/config/file_config_spec.rb index 594425c2dab..d444d4f1df7 100644 --- a/spec/lib/gitlab/static_site_editor/config/file_config_spec.rb +++ b/spec/lib/gitlab/static_site_editor/config/file_config_spec.rb @@ -3,13 +3,85 @@ require 'spec_helper' RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig do - subject(:config) { described_class.new } + let(:config) do + described_class.new(yml) + end + + context 'when config is valid' do + context 'when config has valid values' do + let(:yml) do + <<-EOS + static_site_generator: middleman + EOS + end + + describe '#to_hash_with_defaults' do + it 'returns hash created from string' do + expect(config.to_hash_with_defaults.fetch(:static_site_generator)).to eq 'middleman' + end + end + + describe '#valid?' do + it 'is valid' do + expect(config).to be_valid + end + + it 'has no errors' do + expect(config.errors).to be_empty + end + end + end + end + + context 'when a config entry has an empty value' do + let(:yml) { 'static_site_generator: ' } + + describe '#to_hash' do + it 'returns default value' do + expect(config.to_hash_with_defaults.fetch(:static_site_generator)).to eq 'middleman' + end + end + + describe '#valid?' do + it 'is valid' do + expect(config).to be_valid + end + + it 'has no errors' do + expect(config.errors).to be_empty + end + end + end + + context 'when config is invalid' do + context 'when yml is incorrect' do + let(:yml) { '// invalid' } + + describe '.new' do + it 'raises error' do + expect { config }.to raise_error(described_class::ConfigError, /Invalid configuration format/) + end + end + end + + context 'when config value exists but is not a valid value' do + let(:yml) { 'static_site_generator: "unsupported-generator"' } + + describe '#valid?' do + it 'is not valid' do + expect(config).not_to be_valid + end - describe '#data' do - subject { config.data } + it 'has errors' do + expect(config.errors).not_to be_empty + end + end - it 'returns hardcoded data for now' do - is_expected.to match(static_site_generator: 'middleman') + describe '#errors' do + it 'returns an array of strings' do + expect(config.errors).to all(be_an_instance_of(String)) + end + end end end end diff --git a/spec/lib/gitlab/static_site_editor/config/generated_config_spec.rb b/spec/lib/gitlab/static_site_editor/config/generated_config_spec.rb index 3433a54be9c..2f761b69e60 100644 --- a/spec/lib/gitlab/static_site_editor/config/generated_config_spec.rb +++ b/spec/lib/gitlab/static_site_editor/config/generated_config_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do project: 'project', project_id: project.id, return_url: 'http://example.com', - is_supported_content: 'true', + is_supported_content: true, base_url: '/namespace/project/-/sse/master%2FREADME.md', merge_requests_illustration_path: %r{illustrations/merge_requests} }) @@ -65,7 +65,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do stub_feature_flags(sse_erb_support: project) end - it { is_expected.to include(is_supported_content: 'true') } + it { is_expected.to include(is_supported_content: true) } end context 'when feature flag is disabled' do @@ -75,7 +75,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do stub_feature_flags(sse_erb_support: false) end - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end end @@ -88,31 +88,31 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do context 'when branch is not master' do let(:ref) { 'my-branch' } - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end context 'when file does not have a markdown extension' do let(:path) { 'README.txt' } - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end context 'when file does not have an extension' do let(:path) { 'README' } - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end context 'when file does not exist' do let(:path) { 'UNKNOWN.md' } - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end context 'when repository is empty' do let(:repository) { create(:project_empty_repo).repository } - it { is_expected.to include(is_supported_content: 'false') } + it { is_expected.to include(is_supported_content: false) } end context 'when return_url is not a valid URL' do @@ -132,5 +132,11 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do it { is_expected.to include(return_url: nil) } end + + context 'when a commit for the ref cannot be found' do + let(:ref) { 'nonexistent-ref' } + + it { is_expected.to include(commit_id: nil) } + end end end diff --git a/spec/lib/gitlab/themes_spec.rb b/spec/lib/gitlab/themes_spec.rb index 68ff28becfa..6d03cf496b8 100644 --- a/spec/lib/gitlab/themes_spec.rb +++ b/spec/lib/gitlab/themes_spec.rb @@ -47,4 +47,18 @@ RSpec.describe Gitlab::Themes, lib: true do expect(ids).not_to be_empty end end + + describe 'theme.css_filename' do + described_class.each do |theme| + next unless theme.css_filename + + context "for #{theme.name}" do + it 'returns an existing CSS filename' do + css_file_path = Rails.root.join('app/assets/stylesheets/themes', theme.css_filename + '.scss') + + expect(File.exist?(css_file_path)).to eq(true) + end + end + end + end end diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb index f0bf7b9964f..6ddeaf98370 100644 --- a/spec/lib/gitlab/tracking_spec.rb +++ b/spec/lib/gitlab/tracking_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Gitlab::Tracking do end around do |example| - Timecop.freeze(timestamp) { example.run } + travel_to(timestamp) { example.run } end before do diff --git a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb index 2a674557b76..f2c1d8718d7 100644 --- a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb @@ -41,11 +41,11 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red context 'for web IDE edit actions' do it_behaves_like 'tracks and counts action' do def track_action(params) - described_class.track_web_ide_edit_action(params) + described_class.track_web_ide_edit_action(**params) end def count_unique(params) - described_class.count_web_ide_edit_actions(params) + described_class.count_web_ide_edit_actions(**params) end end end @@ -53,11 +53,11 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red context 'for SFE edit actions' do it_behaves_like 'tracks and counts action' do def track_action(params) - described_class.track_sfe_edit_action(params) + described_class.track_sfe_edit_action(**params) end def count_unique(params) - described_class.count_sfe_edit_actions(params) + described_class.count_sfe_edit_actions(**params) end end end @@ -65,11 +65,11 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red context 'for snippet editor edit actions' do it_behaves_like 'tracks and counts action' do def track_action(params) - described_class.track_snippet_editor_edit_action(params) + described_class.track_snippet_editor_edit_action(**params) end def count_unique(params) - described_class.count_snippet_editor_edit_actions(params) + described_class.count_snippet_editor_edit_actions(**params) end end end diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index f881da71251..3255e3616b2 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s # depending on which day of the week test is run. # Monday 6th of June reference_time = Time.utc(2020, 6, 1) - Timecop.freeze(reference_time) { example.run } + travel_to(reference_time) { example.run } end describe '.categories' do @@ -238,16 +238,20 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s it 'returns the number of unique events for all known events' do results = { - 'category1' => { - 'event1_slot' => 1, - 'event2_slot' => 1, - 'category1_total_unique_counts_weekly' => 2, - 'category1_total_unique_counts_monthly' => 3 - }, - 'category2' => { - 'event3' => 1, - 'event4' => 1 - } + "category1" => { + "event1_slot_weekly" => 1, + "event1_slot_monthly" => 1, + "event2_slot_weekly" => 1, + "event2_slot_monthly" => 2, + "category1_total_unique_counts_weekly" => 2, + "category1_total_unique_counts_monthly" => 3 + }, + "category2" => { + "event3_weekly" => 1, + "event3_monthly" => 1, + "event4_weekly" => 1, + "event4_monthly" => 1 + } } expect(subject.unique_events_data).to eq(results) diff --git a/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb index 479fe36bcdd..4cc85c86de1 100644 --- a/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git let(:action) { described_class::ISSUE_TITLE_CHANGED } def track_action(params) - described_class.track_issue_title_changed_action(params) + described_class.track_issue_title_changed_action(**params) end end end @@ -57,7 +57,7 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git let(:action) { described_class::ISSUE_DESCRIPTION_CHANGED } def track_action(params) - described_class.track_issue_description_changed_action(params) + described_class.track_issue_description_changed_action(**params) end end end @@ -67,7 +67,7 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git let(:action) { described_class::ISSUE_ASSIGNEE_CHANGED } def track_action(params) - described_class.track_issue_assignee_changed_action(params) + described_class.track_issue_assignee_changed_action(**params) end end end @@ -77,7 +77,7 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git let(:action) { described_class::ISSUE_MADE_CONFIDENTIAL } def track_action(params) - described_class.track_issue_made_confidential_action(params) + described_class.track_issue_made_confidential_action(**params) end end end @@ -87,7 +87,47 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git let(:action) { described_class::ISSUE_MADE_VISIBLE } def track_action(params) - described_class.track_issue_made_visible_action(params) + described_class.track_issue_made_visible_action(**params) + end + end + end + + context 'for Issue created actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_CREATED } + + def track_action(params) + described_class.track_issue_created_action(**params) + end + end + end + + context 'for Issue closed actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_CLOSED } + + def track_action(params) + described_class.track_issue_closed_action(**params) + end + end + end + + context 'for Issue reopened actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_REOPENED } + + def track_action(params) + described_class.track_issue_reopened_action(**params) + end + end + end + + context 'for Issue label changed actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_LABEL_CHANGED } + + def track_action(params) + described_class.track_issue_label_changed_action(**params) end end end diff --git a/spec/lib/gitlab/usage_data_counters/track_unique_events_spec.rb b/spec/lib/gitlab/usage_data_counters/track_unique_events_spec.rb index 8f5f1347ce8..d1144dd0bc5 100644 --- a/spec/lib/gitlab/usage_data_counters/track_unique_events_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/track_unique_events_spec.rb @@ -8,11 +8,11 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueEvents, :clean_gitlab_redis let(:time) { Time.zone.now } def track_event(params) - track_unique_events.track_event(params) + track_unique_events.track_event(**params) end def count_unique(params) - track_unique_events.count_unique_events(params) + track_unique_events.count_unique_events(**params) end context 'tracking an event' do diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 6631a0d3cc6..984fd4c08e6 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -29,7 +29,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do user_minimum_id user_maximum_id unique_visit_service deployment_minimum_id deployment_maximum_id approval_merge_request_rule_minimum_id - approval_merge_request_rule_maximum_id) + approval_merge_request_rule_maximum_id + auth_providers) values.each do |key| expect(described_class).to receive(:clear_memoization).with(key) end @@ -167,6 +168,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do describe 'usage_activity_by_stage_manage' do it 'includes accurate usage_activity_by_stage data' do + described_class.clear_memoization(:auth_providers) + stub_config( omniauth: { providers: omniauth_providers } @@ -174,21 +177,29 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do for_defined_days_back do user = create(:user) + user2 = create(:user) create(:event, author: user) create(:group_member, user: user) + create(:authentication_event, user: user, provider: :ldapmain, result: :success) + create(:authentication_event, user: user2, provider: :ldapsecondary, result: :success) + create(:authentication_event, user: user2, provider: :group_saml, result: :success) + create(:authentication_event, user: user2, provider: :group_saml, result: :success) + create(:authentication_event, user: user, provider: :group_saml, result: :failed) end expect(described_class.usage_activity_by_stage_manage({})).to include( events: 2, groups: 2, - users_created: 4, - omniauth_providers: ['google_oauth2'] + users_created: 6, + omniauth_providers: ['google_oauth2'], + user_auth_by_provider: { 'group_saml' => 2, 'ldap' => 4 } ) expect(described_class.usage_activity_by_stage_manage(described_class.last_28_days_time_period)).to include( events: 1, groups: 1, - users_created: 2, - omniauth_providers: ['google_oauth2'] + users_created: 3, + omniauth_providers: ['google_oauth2'], + user_auth_by_provider: { 'group_saml' => 1, 'ldap' => 2 } ) end @@ -244,6 +255,20 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ) end + it 'includes group imports usage data' do + for_defined_days_back do + user = create(:user) + group = create(:group) + group.add_owner(user) + create(:group_import_state, group: group, user: user) + end + + expect(described_class.usage_activity_by_stage_manage({})) + .to include(groups_imported: 2) + expect(described_class.usage_activity_by_stage_manage(described_class.last_28_days_time_period)) + .to include(groups_imported: 1) + end + def omniauth_providers [ OpenStruct.new(name: 'google_oauth2'), @@ -260,17 +285,20 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do cluster = create(:cluster, user: user) create(:project, creator: user) create(:clusters_applications_prometheus, :installed, cluster: cluster) + create(:project_tracing_setting) end expect(described_class.usage_activity_by_stage_monitor({})).to include( clusters: 2, clusters_applications_prometheus: 2, - operations_dashboard_default_dashboard: 2 + operations_dashboard_default_dashboard: 2, + projects_with_tracing_enabled: 2 ) expect(described_class.usage_activity_by_stage_monitor(described_class.last_28_days_time_period)).to include( clusters: 1, clusters_applications_prometheus: 1, - operations_dashboard_default_dashboard: 1 + operations_dashboard_default_dashboard: 1, + projects_with_tracing_enabled: 1 ) end end @@ -420,6 +448,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(count_data[:projects_inheriting_instance_mattermost_active]).to eq(1) expect(count_data[:projects_with_repositories_enabled]).to eq(3) expect(count_data[:projects_with_error_tracking_enabled]).to eq(1) + expect(count_data[:projects_with_tracing_enabled]).to eq(1) expect(count_data[:projects_with_alerts_service_enabled]).to eq(1) expect(count_data[:projects_with_prometheus_alerts]).to eq(2) expect(count_data[:projects_with_terraform_reports]).to eq(2) @@ -472,8 +501,10 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(count_data[:personal_snippets]).to eq(2) expect(count_data[:project_snippets]).to eq(4) + expect(count_data[:projects_creating_incidents]).to eq(2) expect(count_data[:projects_with_packages]).to eq(2) expect(count_data[:packages]).to eq(4) + expect(count_data[:user_preferences_user_gitpod_enabled]).to eq(1) end it 'gathers object store usage correctly' do @@ -628,6 +659,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(subject[:gitlab_shared_runners_enabled]).to eq(Gitlab.config.gitlab_ci.shared_runners_enabled) expect(subject[:web_ide_clientside_preview_enabled]).to eq(Gitlab::CurrentSettings.web_ide_clientside_preview_enabled?) expect(subject[:grafana_link_enabled]).to eq(Gitlab::CurrentSettings.grafana_enabled?) + expect(subject[:gitpod_enabled]).to eq(Gitlab::CurrentSettings.gitpod_enabled?) end context 'with embedded Prometheus' do @@ -657,6 +689,20 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(subject[:grafana_link_enabled]).to eq(false) end end + + context 'with Gitpod' do + it 'returns true when is enabled' do + stub_application_setting(gitpod_enabled: true) + + expect(subject[:gitpod_enabled]).to eq(true) + end + + it 'returns false when is disabled' do + stub_application_setting(gitpod_enabled: false) + + expect(subject[:gitpod_enabled]).to eq(false) + end + end end describe '.components_usage_data' do @@ -979,7 +1025,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - def for_defined_days_back(days: [29, 2]) + def for_defined_days_back(days: [31, 3]) days.each do |n| Timecop.travel(n.days.ago) do yield @@ -1146,11 +1192,13 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do categories.each do |category| keys = ::Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(category) + metrics = keys.map { |key| "#{key}_weekly" } + keys.map { |key| "#{key}_monthly" } + if ineligible_total_categories.exclude?(category) - keys.append("#{category}_total_unique_counts_weekly", "#{category}_total_unique_counts_monthly") + metrics.append("#{category}_total_unique_counts_weekly", "#{category}_total_unique_counts_monthly") end - expect(subject[:redis_hll_counters][category].keys).to match_array(keys) + expect(subject[:redis_hll_counters][category].keys).to match_array(metrics) end end end @@ -1169,6 +1217,8 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end describe '.snowplow_event_counts' do + let_it_be(:time_period) { { collector_tstamp: 8.days.ago..1.day.ago } } + context 'when self-monitoring project exists' do let_it_be(:project) { create(:project) } @@ -1181,14 +1231,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do stub_feature_flags(product_analytics: project) create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote') - create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote', collector_tstamp: 28.days.ago) + create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote', collector_tstamp: 2.days.ago) + create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote', collector_tstamp: 9.days.ago) + + create(:product_analytics_event, project: project, se_category: 'foo', se_action: 'bar', collector_tstamp: 2.days.ago) end it 'returns promoted_issues for the time period' do - expect(described_class.snowplow_event_counts[:promoted_issues]).to eq(2) - expect(described_class.snowplow_event_counts( - time_period: described_class.last_28_days_time_period(column: :collector_tstamp) - )[:promoted_issues]).to eq(1) + expect(described_class.snowplow_event_counts(time_period)[:promoted_issues]).to eq(1) end end @@ -1198,14 +1248,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end it 'returns an empty hash' do - expect(described_class.snowplow_event_counts).to eq({}) + expect(described_class.snowplow_event_counts(time_period)).to eq({}) end end end context 'when self-monitoring project does not exist' do it 'returns an empty hash' do - expect(described_class.snowplow_event_counts).to eq({}) + expect(described_class.snowplow_event_counts(time_period)).to eq({}) end end end diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb index 362cbaa78e9..9c0dc69ccd1 100644 --- a/spec/lib/gitlab/utils/usage_data_spec.rb +++ b/spec/lib/gitlab/utils/usage_data_spec.rb @@ -212,33 +212,26 @@ RSpec.describe Gitlab::Utils::UsageData do describe '#track_usage_event' do let(:value) { '9f302fea-f828-4ca9-aef4-e10bd723c0b3' } - let(:event_name) { 'my_event' } + let(:event_name) { 'incident_management_alert_status_changed' } let(:unknown_event) { 'unknown' } let(:feature) { "usage_data_#{event_name}" } + before do + skip_feature_flags_yaml_validation + end + context 'with feature enabled' do before do stub_feature_flags(feature => true) end it 'tracks redis hll event' do - stub_application_setting(usage_ping_enabled: true) - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(value, event_name) described_class.track_usage_event(event_name, value) end - it 'does not track event when usage ping is not enabled' do - stub_application_setting(usage_ping_enabled: false) - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - - described_class.track_usage_event(event_name, value) - end - it 'raise an error for unknown event' do - stub_application_setting(usage_ping_enabled: true) - expect { described_class.track_usage_event(unknown_event, value) }.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) end end diff --git a/spec/lib/gitlab/webpack/manifest_spec.rb b/spec/lib/gitlab/webpack/manifest_spec.rb new file mode 100644 index 00000000000..e3aeceda148 --- /dev/null +++ b/spec/lib/gitlab/webpack/manifest_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'json' + +RSpec.describe Gitlab::Webpack::Manifest do + let(:manifest) do + <<-EOF + { + "errors": [], + "assetsByChunkName": { + "entry1": [ "entry1.js", "entry1-a.js" ], + "entry2": "entry2.js" + } + } + EOF + end + + around do |example| + Gitlab::Webpack::Manifest.clear_manifest! + + example.run + + Gitlab::Webpack::Manifest.clear_manifest! + end + + shared_examples_for "a valid manifest" do + it "returns single entry asset paths from the manifest" do + expect(Gitlab::Webpack::Manifest.asset_paths("entry2")).to eq(["/public_path/entry2.js"]) + end + + it "returns multiple entry asset paths from the manifest" do + expect(Gitlab::Webpack::Manifest.asset_paths("entry1")).to eq(["/public_path/entry1.js", "/public_path/entry1-a.js"]) + end + + it "errors on a missing entry point" do + expect { Gitlab::Webpack::Manifest.asset_paths("herp") }.to raise_error(Gitlab::Webpack::Manifest::AssetMissingError) + end + end + + before do + # Test that config variables work while we're here + ::Rails.configuration.webpack.dev_server.host = 'hostname' + ::Rails.configuration.webpack.dev_server.port = 1999 + ::Rails.configuration.webpack.dev_server.manifest_host = 'hostname' + ::Rails.configuration.webpack.dev_server.manifest_port = 2000 + ::Rails.configuration.webpack.manifest_filename = "my_manifest.json" + ::Rails.configuration.webpack.public_path = "public_path" + ::Rails.configuration.webpack.output_dir = "manifest_output" + end + + context "with dev server enabled" do + before do + ::Rails.configuration.webpack.dev_server.enabled = true + + stub_request(:get, "http://hostname:2000/public_path/my_manifest.json").to_return(body: manifest, status: 200) + end + + describe ".asset_paths" do + it_behaves_like "a valid manifest" + + it "errors if we can't find the manifest" do + ::Rails.configuration.webpack.manifest_filename = "broken.json" + stub_request(:get, "http://hostname:2000/public_path/broken.json").to_raise(SocketError) + + expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.to raise_error(Gitlab::Webpack::Manifest::ManifestLoadError) + end + + describe "webpack errors" do + context "when webpack has 'Module build failed' errors in its manifest" do + it "errors" do + error_manifest = Gitlab::Json.parse(manifest).merge("errors" => [ + "somethingModule build failed something", + "I am an error" + ]).to_json + stub_request(:get, "http://hostname:2000/public_path/my_manifest.json").to_return(body: error_manifest, status: 200) + + expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.to raise_error(Gitlab::Webpack::Manifest::WebpackError) + end + end + + context "when webpack does not have 'Module build failed' errors in its manifest" do + it "does not error" do + error_manifest = Gitlab::Json.parse(manifest).merge("errors" => ["something went wrong"]).to_json + stub_request(:get, "http://hostname:2000/public_path/my_manifest.json").to_return(body: error_manifest, status: 200) + + expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.not_to raise_error + end + end + + it "does not error if errors is present but empty" do + error_manifest = Gitlab::Json.parse(manifest).merge("errors" => []).to_json + stub_request(:get, "http://hostname:2000/public_path/my_manifest.json").to_return(body: error_manifest, status: 200) + expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.not_to raise_error + end + end + end + end + + context "with dev server disabled" do + before do + ::Rails.configuration.webpack.dev_server.enabled = false + allow(File).to receive(:read).with(::Rails.root.join("manifest_output/my_manifest.json")).and_return(manifest) + end + + describe ".asset_paths" do + it_behaves_like "a valid manifest" + + it "errors if we can't find the manifest" do + ::Rails.configuration.webpack.manifest_filename = "broken.json" + allow(File).to receive(:read).with(::Rails.root.join("manifest_output/broken.json")).and_raise(Errno::ENOENT) + expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.to raise_error(Gitlab::Webpack::Manifest::ManifestLoadError) + end + end + end +end diff --git a/spec/lib/gitlab_danger_spec.rb b/spec/lib/gitlab_danger_spec.rb index b534823a888..e332647cf8a 100644 --- a/spec/lib/gitlab_danger_spec.rb +++ b/spec/lib/gitlab_danger_spec.rb @@ -9,7 +9,7 @@ RSpec.describe GitlabDanger do describe '.local_warning_message' do it 'returns an informational message with rules that can run' do - expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, documentation, frozen_string, duplicate_yarn_dependencies, prettier, eslint, karma, database, commit_messages, telemetry, utility_css, pajamas') + expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, documentation, frozen_string, duplicate_yarn_dependencies, prettier, eslint, karma, database, commit_messages, product_analytics, utility_css, pajamas') end end diff --git a/spec/lib/google_api/auth_spec.rb b/spec/lib/google_api/auth_spec.rb index eeb99bfbb6c..92cb9e494ac 100644 --- a/spec/lib/google_api/auth_spec.rb +++ b/spec/lib/google_api/auth_spec.rb @@ -12,12 +12,12 @@ RSpec.describe GoogleApi::Auth do end describe '#authorize_url' do - subject { client.authorize_url } + subject { Addressable::URI.parse(client.authorize_url) } it 'returns authorize_url' do - is_expected.to start_with('https://accounts.google.com/o/oauth2') - is_expected.to include(URI.encode(redirect_uri, URI::PATTERN::RESERVED)) - is_expected.to include(URI.encode(redirect_to, URI::PATTERN::RESERVED)) + expect(subject.to_s).to start_with('https://accounts.google.com/o/oauth2') + expect(subject.query_values['state']).to eq(redirect_to) + expect(subject.query_values['redirect_uri']).to eq(redirect_uri) end end diff --git a/spec/lib/grafana/time_window_spec.rb b/spec/lib/grafana/time_window_spec.rb index 9ee65c6cf20..0657bed7b28 100644 --- a/spec/lib/grafana/time_window_spec.rb +++ b/spec/lib/grafana/time_window_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Grafana::TimeWindow do let(:to) { '1552828200000' } around do |example| - Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run } + travel_to(Time.utc(2019, 3, 17, 13, 10)) { example.run } end describe '#formatted' do @@ -37,7 +37,7 @@ RSpec.describe Grafana::RangeWithDefaults do let(:to) { Grafana::Timestamp.from_ms_since_epoch('1552828200000') } around do |example| - Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run } + travel_to(Time.utc(2019, 3, 17, 13, 10)) { example.run } end describe '#to_hash' do @@ -82,7 +82,7 @@ RSpec.describe Grafana::Timestamp do let(:timestamp) { Time.at(1552799400) } around do |example| - Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run } + travel_to(Time.utc(2019, 3, 17, 13, 10)) { example.run } end describe '#formatted' do diff --git a/spec/lib/pager_duty/webhook_payload_parser_spec.rb b/spec/lib/pager_duty/webhook_payload_parser_spec.rb index 0010165318d..54c61b9121c 100644 --- a/spec/lib/pager_duty/webhook_payload_parser_spec.rb +++ b/spec/lib/pager_duty/webhook_payload_parser_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'json_schemer' RSpec.describe PagerDuty::WebhookPayloadParser do describe '.call' do @@ -8,36 +9,36 @@ RSpec.describe PagerDuty::WebhookPayloadParser do File.read(File.join(File.dirname(__FILE__), '../../fixtures/pager_duty/webhook_incident_trigger.json')) end + let(:triggered_event) do + { + 'event' => 'incident.trigger', + 'incident' => { + 'url' => 'https://webdemo.pagerduty.com/incidents/PRORDTY', + 'incident_number' => 33, + 'title' => 'My new incident', + 'status' => 'triggered', + 'created_at' => '2017-09-26T15:14:36Z', + 'urgency' => 'high', + 'incident_key' => nil, + 'assignees' => [{ + 'summary' => 'Laura Haley', + 'url' => 'https://webdemo.pagerduty.com/users/P553OPV' + }], + 'impacted_services' => [{ + 'summary' => 'Production XDB Cluster', + 'url' => 'https://webdemo.pagerduty.com/services/PN49J75' + }] + } + } + end + subject(:parse) { described_class.call(payload) } context 'when payload is a correct PagerDuty payload' do let(:payload) { Gitlab::Json.parse(fixture_file) } it 'returns parsed payload' do - is_expected.to eq( - [ - { - 'event' => 'incident.trigger', - 'incident' => { - 'url' => 'https://webdemo.pagerduty.com/incidents/PRORDTY', - 'incident_number' => 33, - 'title' => 'My new incident', - 'status' => 'triggered', - 'created_at' => '2017-09-26T15:14:36Z', - 'urgency' => 'high', - 'incident_key' => nil, - 'assignees' => [{ - 'summary' => 'Laura Haley', - 'url' => 'https://webdemo.pagerduty.com/users/P553OPV' - }], - 'impacted_services' => [{ - 'summary' => 'Production XDB Cluster', - 'url' => 'https://webdemo.pagerduty.com/services/PN49J75' - }] - } - } - ] - ) + is_expected.to eq([triggered_event]) end context 'when assignments summary and html_url are blank' do @@ -69,11 +70,42 @@ RSpec.describe PagerDuty::WebhookPayloadParser do end end - context 'when payload has no incidents' do + context 'when payload schema is invalid' do let(:payload) { { 'messages' => [{ 'event' => 'incident.trigger' }] } } it 'returns payload with blank incidents' do - is_expected.to eq([{ 'event' => 'incident.trigger', 'incident' => {} }]) + is_expected.to eq([]) + end + end + + context 'when payload consists of two messages' do + context 'when one of the messages has no incident data' do + let(:payload) do + valid_payload = Gitlab::Json.parse(fixture_file) + event = { 'event' => 'incident.trigger' } + valid_payload['messages'] = valid_payload['messages'].append(event) + valid_payload + end + + it 'returns parsed payload with valid events only' do + is_expected.to eq([triggered_event]) + end + end + + context 'when one of the messages has unknown event' do + let(:payload) do + valid_payload = Gitlab::Json.parse(fixture_file) + event = { 'event' => 'incident.unknown', 'incident' => valid_payload['messages'].first['incident'] } + valid_payload['messages'] = valid_payload['messages'].append(event) + valid_payload + end + + it 'returns parsed payload' do + unknown_event = triggered_event.dup + unknown_event['event'] = 'incident.unknown' + + is_expected.to contain_exactly(triggered_event, unknown_event) + end end end end diff --git a/spec/lib/safe_zip/extract_spec.rb b/spec/lib/safe_zip/extract_spec.rb index 30b7e1cdd2c..443430b267d 100644 --- a/spec/lib/safe_zip/extract_spec.rb +++ b/spec/lib/safe_zip/extract_spec.rb @@ -15,11 +15,7 @@ RSpec.describe SafeZip::Extract do describe '#extract' do subject { object.extract(directories: directories, to: target_path) } - shared_examples 'extracts archive' do |param| - before do - stub_feature_flags(safezip_use_rubyzip: param) - end - + shared_examples 'extracts archive' do it 'does extract archive' do subject @@ -28,11 +24,7 @@ RSpec.describe SafeZip::Extract do end end - shared_examples 'fails to extract archive' do |param| - before do - stub_feature_flags(safezip_use_rubyzip: param) - end - + shared_examples 'fails to extract archive' do it 'does not extract archive' do expect { subject }.to raise_error(SafeZip::Extract::Error) end @@ -42,13 +34,7 @@ RSpec.describe SafeZip::Extract do context "when using #{name} archive" do let(:archive_name) { name } - context 'for RubyZip' do - it_behaves_like 'extracts archive', true - end - - context 'for UnZip' do - it_behaves_like 'extracts archive', false - end + it_behaves_like 'extracts archive' end end @@ -56,13 +42,7 @@ RSpec.describe SafeZip::Extract do context "when using #{name} archive" do let(:archive_name) { name } - context 'for RubyZip' do - it_behaves_like 'fails to extract archive', true - end - - context 'for UnZip (UNSAFE)' do - it_behaves_like 'extracts archive', false - end + it_behaves_like 'fails to extract archive' end end @@ -70,13 +50,7 @@ RSpec.describe SafeZip::Extract do let(:archive_name) { 'valid-simple.zip' } let(:directories) { %w(non/existing) } - context 'for RubyZip' do - it_behaves_like 'fails to extract archive', true - end - - context 'for UnZip' do - it_behaves_like 'fails to extract archive', false - end + it_behaves_like 'fails to extract archive' end end end |