diff options
Diffstat (limited to 'spec/models')
132 files changed, 3287 insertions, 1526 deletions
diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 24de46cb536..85a6717d259 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -132,5 +132,47 @@ RSpec.describe ApplicationRecord do end.to raise_error(ActiveRecord::QueryCanceled) end end + + context 'with database load balancing' do + let(:session) { double(:session) } + + before do + allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session) + allow(session).to receive(:fallback_to_replicas_for_ambiguous_queries).and_yield + end + + it 'yields control' do + expect do |blk| + described_class.with_fast_read_statement_timeout(&blk) + end.to yield_control.once + end + + context 'when the query runs faster than configured timeout' do + it 'executes the query without error' do + result = nil + + expect do + described_class.with_fast_read_statement_timeout(100) do + result = described_class.connection.exec_query('SELECT 1') + end + end.not_to raise_error + + expect(result).not_to be_nil + end + end + + # This query hangs for 10ms and then gets cancelled. As there is no + # other way to test the timeout for sure, 10ms of waiting seems to be + # reasonable! + context 'when the query runs longer than configured timeout' do + it 'cancels the query and raiss an exception' do + expect do + described_class.with_fast_read_statement_timeout(10) do + described_class.connection.exec_query('SELECT pg_sleep(0.1)') + end + end.to raise_error(ActiveRecord::QueryCanceled) + end + end + end end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index c13d83d1685..4e72d558b52 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -990,6 +990,34 @@ RSpec.describe ApplicationSetting do end end end + + describe '#diff_max_files' do + context 'validations' do + it { is_expected.to validate_presence_of(:diff_max_files) } + + specify do + is_expected + .to validate_numericality_of(:diff_max_files) + .only_integer + .is_greater_than_or_equal_to(Commit::DEFAULT_MAX_DIFF_FILES_SETTING) + .is_less_than_or_equal_to(Commit::MAX_DIFF_FILES_SETTING_UPPER_BOUND) + end + end + end + + describe '#diff_max_lines' do + context 'validations' do + it { is_expected.to validate_presence_of(:diff_max_lines) } + + specify do + is_expected + .to validate_numericality_of(:diff_max_lines) + .only_integer + .is_greater_than_or_equal_to(Commit::DEFAULT_MAX_DIFF_LINES_SETTING) + .is_less_than_or_equal_to(Commit::MAX_DIFF_LINES_SETTING_UPPER_BOUND) + end + end + end end describe '#sourcegraph_url_is_com?' do diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb new file mode 100644 index 00000000000..48f32a2092a --- /dev/null +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::ExportStatus do + let_it_be(:relation) { 'labels' } + let_it_be(:import) { create(:bulk_import) } + let_it_be(:config) { create(:bulk_import_configuration, bulk_import: import) } + let_it_be(:entity) { create(:bulk_import_entity, bulk_import: import, source_full_path: 'foo') } + let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) } + + let(:response_double) do + double(parsed_response: [{ 'relation' => 'labels', 'status' => status, 'error' => 'error!' }]) + end + + subject { described_class.new(tracker, relation) } + + before do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:get).and_return(response_double) + end + end + + describe '#started?' do + context 'when export status is started' do + let(:status) { BulkImports::Export::STARTED } + + it 'returns true' do + expect(subject.started?).to eq(true) + end + end + + context 'when export status is not started' do + let(:status) { BulkImports::Export::FAILED } + + it 'returns false' do + expect(subject.started?).to eq(false) + end + end + end + + describe '#failed' do + context 'when export status is failed' do + let(:status) { BulkImports::Export::FAILED } + + it 'returns true' do + expect(subject.failed?).to eq(true) + end + end + + context 'when export status is not failed' do + let(:status) { BulkImports::Export::STARTED } + + it 'returns false' do + expect(subject.failed?).to eq(false) + end + end + end + + describe '#error' do + let(:status) { BulkImports::Export::FAILED } + + it 'returns error message' do + expect(subject.error).to eq('error!') + end + + context 'when something goes wrong during export status fetch' do + it 'returns exception class as error' do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:get).and_raise(StandardError, 'Error!') + end + + expect(subject.error).to eq('Error!') + end + end + end +end diff --git a/spec/models/bulk_imports/export_upload_spec.rb b/spec/models/bulk_imports/export_upload_spec.rb index 641fa4a1b6c..d9ae41af0db 100644 --- a/spec/models/bulk_imports/export_upload_spec.rb +++ b/spec/models/bulk_imports/export_upload_spec.rb @@ -13,7 +13,7 @@ RSpec.describe BulkImports::ExportUpload do method = 'export_file' filename = 'labels.ndjson.gz' - subject.public_send("#{method}=", fixture_file_upload("spec/fixtures/bulk_imports/#{filename}")) + subject.public_send("#{method}=", fixture_file_upload("spec/fixtures/bulk_imports/gz/#{filename}")) subject.save! url = "/uploads/-/system/bulk_imports/export_upload/export_file/#{subject.id}/#{filename}" diff --git a/spec/models/bulk_imports/file_transfer/group_config_spec.rb b/spec/models/bulk_imports/file_transfer/group_config_spec.rb index 21da71de3c7..4611a00b0cc 100644 --- a/spec/models/bulk_imports/file_transfer/group_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/group_config_spec.rb @@ -12,8 +12,8 @@ RSpec.describe BulkImports::FileTransfer::GroupConfig do subject { described_class.new(exportable) } - describe '#exportable_tree' do - it 'returns exportable tree' do + describe '#portable_tree' do + it 'returns portable tree' do expect_next_instance_of(::Gitlab::ImportExport::AttributesFinder) do |finder| expect(finder).to receive(:find_root).with(:group).and_call_original end @@ -30,9 +30,21 @@ RSpec.describe BulkImports::FileTransfer::GroupConfig do end end - describe '#exportable_relations' do + describe '#portable_relations' do it 'returns a list of top level exportable relations' do expect(subject.portable_relations).to include('milestones', 'badges', 'boards', 'labels') end end + + describe '#top_relation_tree' do + it 'returns relation tree of a top level relation' do + expect(subject.top_relation_tree('labels')).to eq('priorities' => {}) + end + end + + describe '#relation_excluded_keys' do + it 'returns excluded keys for relation' do + expect(subject.relation_excluded_keys('group')).to include('owner_id') + end + end end diff --git a/spec/models/bulk_imports/file_transfer/project_config_spec.rb b/spec/models/bulk_imports/file_transfer/project_config_spec.rb index 021f96ac2a3..2995556a58d 100644 --- a/spec/models/bulk_imports/file_transfer/project_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/project_config_spec.rb @@ -12,8 +12,8 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do subject { described_class.new(exportable) } - describe '#exportable_tree' do - it 'returns exportable tree' do + describe 'portable_tree' do + it 'returns portable tree' do expect_next_instance_of(::Gitlab::ImportExport::AttributesFinder) do |finder| expect(finder).to receive(:find_root).with(:project).and_call_original end @@ -30,9 +30,21 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do end end - describe '#exportable_relations' do + describe '#portable_relations' do it 'returns a list of top level exportable relations' do expect(subject.portable_relations).to include('issues', 'labels', 'milestones', 'merge_requests') end end + + describe '#top_relation_tree' do + it 'returns relation tree of a top level relation' do + expect(subject.top_relation_tree('labels')).to eq('priorities' => {}) + end + end + + describe '#relation_excluded_keys' do + it 'returns excluded keys for relation' do + expect(subject.relation_excluded_keys('project')).to include('creator_id') + end + end end diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index d00d88ae397..331ba9953ca 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -187,15 +187,6 @@ RSpec.describe Ci::BuildDependencies do it { expect(cross_pipeline_deps).to contain_exactly(upstream_job) } it { is_expected.to be_valid } end - - context 'when feature flag `ci_cross_pipeline_artifacts_download` is disabled' do - before do - stub_feature_flags(ci_cross_pipeline_artifacts_download: false) - end - - it { expect(cross_pipeline_deps).to be_empty } - it { is_expected.to be_valid } - end end context 'when same job names exist in other pipelines in the hierarchy' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 66d2f5f4ee9..62dec522161 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -111,10 +111,6 @@ RSpec.describe Ci::Build do describe '.with_downloadable_artifacts' do subject { described_class.with_downloadable_artifacts } - before do - stub_feature_flags(drop_license_management_artifact: false) - end - context 'when job does not have a downloadable artifact' do let!(:job) { create(:ci_build) } @@ -320,11 +316,23 @@ RSpec.describe Ci::Build do end end + describe '#stick_build_if_status_changed' do + it 'sticks the build if the status changed' do + job = create(:ci_build, :pending) + + allow(Gitlab::Database::LoadBalancing).to receive(:enable?) + .and_return(true) + + expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + .with(:build, job.id) + + job.update!(status: :running) + end + end + describe '#enqueue' do let(:build) { create(:ci_build, :created) } - subject { build.enqueue } - before do allow(build).to receive(:any_unmet_prerequisites?).and_return(has_prerequisites) allow(Ci::PrepareBuildService).to receive(:perform_async) @@ -334,28 +342,74 @@ RSpec.describe Ci::Build do let(:has_prerequisites) { true } it 'transitions to preparing' do - subject + build.enqueue expect(build).to be_preparing end + + it 'does not push build to the queue' do + build.enqueue + + expect(build.queuing_entry).not_to be_present + end end context 'build has no prerequisites' do let(:has_prerequisites) { false } it 'transitions to pending' do - subject + build.enqueue expect(build).to be_pending end + + it 'pushes build to a queue' do + build.enqueue + + expect(build.queuing_entry).to be_present + end + + context 'when build status transition fails' do + before do + ::Ci::Build.find(build.id).update_column(:lock_version, 100) + end + + it 'does not push build to a queue' do + expect { build.enqueue! } + .to raise_error(ActiveRecord::StaleObjectError) + + expect(build.queuing_entry).not_to be_present + end + end + + context 'when there is a queuing entry already present' do + before do + ::Ci::PendingBuild.create!(build: build, project: build.project) + end + + it 'does not raise an error' do + expect { build.enqueue! }.not_to raise_error + expect(build.reload.queuing_entry).to be_present + end + end + + context 'when both failure scenario happen at the same time' do + before do + ::Ci::Build.find(build.id).update_column(:lock_version, 100) + ::Ci::PendingBuild.create!(build: build, project: build.project) + end + + it 'raises stale object error exception' do + expect { build.enqueue! } + .to raise_error(ActiveRecord::StaleObjectError) + end + end end end describe '#enqueue_preparing' do let(:build) { create(:ci_build, :preparing) } - subject { build.enqueue_preparing } - before do allow(build).to receive(:any_unmet_prerequisites?).and_return(has_unmet_prerequisites) end @@ -364,9 +418,10 @@ RSpec.describe Ci::Build do let(:has_unmet_prerequisites) { false } it 'transitions to pending' do - subject + build.enqueue_preparing expect(build).to be_pending + expect(build.queuing_entry).to be_present end end @@ -374,9 +429,10 @@ RSpec.describe Ci::Build do let(:has_unmet_prerequisites) { true } it 'remains in preparing' do - subject + build.enqueue_preparing expect(build).to be_preparing + expect(build.queuing_entry).not_to be_present end end end @@ -405,6 +461,64 @@ RSpec.describe Ci::Build do end end + describe '#run' do + context 'when build has been just created' do + let(:build) { create(:ci_build, :created) } + + it 'creates queuing entry and then removes it' do + build.enqueue! + expect(build.queuing_entry).to be_present + + build.run! + expect(build.reload.queuing_entry).not_to be_present + end + end + + context 'when build status transition fails' do + let(:build) { create(:ci_build, :pending) } + + before do + ::Ci::PendingBuild.create!(build: build, project: build.project) + ::Ci::Build.find(build.id).update_column(:lock_version, 100) + end + + it 'does not remove build from a queue' do + expect { build.run! } + .to raise_error(ActiveRecord::StaleObjectError) + + expect(build.queuing_entry).to be_present + end + end + + context 'when build has been picked by a shared runner' do + let(:build) { create(:ci_build, :pending) } + + it 'creates runtime metadata entry' do + build.runner = create(:ci_runner, :instance_type) + + build.run! + + expect(build.reload.runtime_metadata).to be_present + end + end + end + + describe '#drop' do + context 'when has a runtime tracking entry' do + let(:build) { create(:ci_build, :pending) } + + it 'removes runtime tracking entry' do + build.runner = create(:ci_runner, :instance_type) + + build.run! + expect(build.reload.runtime_metadata).to be_present + + build.drop! + expect(build.reload.runtime_metadata).not_to be_present + end + end + end + describe '#schedulable?' do subject { build.schedulable? } @@ -586,28 +700,10 @@ RSpec.describe Ci::Build do end end - context 'with runners_cached_states feature flag enabled' do - before do - stub_feature_flags(runners_cached_states: true) - end - - it 'caches the result in Redis' do - expect(Rails.cache).to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute) - - build.any_runners_online? - end - end - - context 'with runners_cached_states feature flag disabled' do - before do - stub_feature_flags(runners_cached_states: false) - end - - it 'does not cache' do - expect(Rails.cache).not_to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute) + it 'caches the result in Redis' do + expect(Rails.cache).to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute) - build.any_runners_online? - end + build.any_runners_online? end end @@ -624,28 +720,10 @@ RSpec.describe Ci::Build do it { is_expected.to be_truthy } end - context 'with runners_cached_states feature flag enabled' do - before do - stub_feature_flags(runners_cached_states: true) - end - - it 'caches the result in Redis' do - expect(Rails.cache).to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute) - - build.any_runners_available? - end - end - - context 'with runners_cached_states feature flag disabled' do - before do - stub_feature_flags(runners_cached_states: false) - end - - it 'does not cache' do - expect(Rails.cache).not_to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute) + it 'caches the result in Redis' do + expect(Rails.cache).to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute) - build.any_runners_available? - end + build.any_runners_available? end end @@ -1650,8 +1728,6 @@ RSpec.describe Ci::Build do subject { build.erase_erasable_artifacts! } before do - stub_feature_flags(drop_license_management_artifact: false) - Ci::JobArtifact.file_types.keys.each do |file_type| create(:ci_job_artifact, job: build, file_type: file_type, file_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS[file_type.to_sym]) end @@ -1840,6 +1916,26 @@ RSpec.describe Ci::Build do it { is_expected.not_to be_retryable } end + + context 'when a canceled build has been retried already' do + before do + project.add_developer(user) + build.cancel! + described_class.retry(build, user) + end + + context 'when prevent_retry_of_retried_jobs feature flag is enabled' do + it { is_expected.not_to be_retryable } + end + + context 'when prevent_retry_of_retried_jobs feature flag is disabled' do + before do + stub_feature_flags(prevent_retry_of_retried_jobs: false) + end + + it { is_expected.to be_retryable } + end + end end end @@ -2525,7 +2621,6 @@ RSpec.describe Ci::Build do { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false }, { key: 'CI_PROJECT_REPOSITORY_LANGUAGES', value: project.repository_languages.map(&:name).join(',').downcase, public: true, masked: false }, { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, - { key: 'CI_PROJECT_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false }, { key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false }, @@ -2566,6 +2661,17 @@ RSpec.describe Ci::Build do it { is_expected.to be_instance_of(Gitlab::Ci::Variables::Collection) } it { expect(subject.to_runner_variables).to eq(predefined_variables) } + it 'excludes variables that require an environment or user' do + environment_based_variables_collection = subject.filter do |variable| + %w[ + YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_ACTION CI_ENVIRONMENT_URL + ].include?(variable[:key]) + end + + expect(environment_based_variables_collection).to be_empty + end + context 'when ci_job_jwt feature flag is disabled' do before do stub_feature_flags(ci_job_jwt: false) @@ -2635,7 +2741,7 @@ RSpec.describe Ci::Build do let(:expected_variables) do predefined_variables.map { |variable| variable.fetch(:key) } + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG - CI_ENVIRONMENT_URL] + CI_ENVIRONMENT_TIER CI_ENVIRONMENT_ACTION CI_ENVIRONMENT_URL] end before do @@ -2653,6 +2759,50 @@ RSpec.describe Ci::Build do expect(received_variables).to eq expected_variables end + + describe 'CI_ENVIRONMENT_ACTION' do + let(:enviroment_action_variable) { subject.find { |variable| variable[:key] == 'CI_ENVIRONMENT_ACTION' } } + + shared_examples 'defaults value' do + it 'value matches start' do + expect(enviroment_action_variable[:value]).to eq('start') + end + end + + it_behaves_like 'defaults value' + + context 'when options is set' do + before do + build.update!(options: options) + end + + context 'when options is empty' do + let(:options) { {} } + + it_behaves_like 'defaults value' + end + + context 'when options is nil' do + let(:options) { nil } + + it_behaves_like 'defaults value' + end + + context 'when options environment is specified' do + let(:options) { { environment: {} } } + + it_behaves_like 'defaults value' + end + + context 'when options environment action specified' do + let(:options) { { environment: { action: 'stop' } } } + + it 'matches the specified action' do + expect(enviroment_action_variable[:value]).to eq('stop') + end + end + end + end end end end @@ -2691,7 +2841,8 @@ RSpec.describe Ci::Build do let(:environment_variables) do [ { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true, masked: false }, - { key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true, masked: false } + { key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_TIER', value: 'production', public: true, masked: false } ] end @@ -2700,6 +2851,7 @@ RSpec.describe Ci::Build do project: build.project, name: 'production', slug: 'prod-slug', + tier: 'production', external_url: '') end @@ -4693,7 +4845,7 @@ RSpec.describe Ci::Build do context 'with project services' do before do - create(:service, active: true, job_events: true, project: project) + create(:integration, active: true, job_events: true, project: project) end it 'executes services' do @@ -4707,7 +4859,7 @@ RSpec.describe Ci::Build do context 'without relevant project services' do before do - create(:service, active: true, job_events: false, project: project) + create(:integration, active: true, job_events: false, project: project) end it 'does not execute services' do @@ -4987,4 +5139,113 @@ RSpec.describe Ci::Build do it { is_expected.to be_truthy } end end + + describe '.build_matchers' do + let_it_be(:pipeline) { create(:ci_pipeline, :protected) } + + subject(:matchers) { pipeline.builds.build_matchers(pipeline.project) } + + context 'when the pipeline is empty' do + it 'does not throw errors' do + is_expected.to eq([]) + end + end + + context 'when the pipeline has builds' do + let_it_be(:build_without_tags) do + create(:ci_build, pipeline: pipeline) + end + + let_it_be(:build_with_tags) do + create(:ci_build, pipeline: pipeline, tag_list: %w[tag1 tag2]) + end + + let_it_be(:other_build_with_tags) do + create(:ci_build, pipeline: pipeline, tag_list: %w[tag2 tag1]) + end + + it { expect(matchers.size).to eq(2) } + + it 'groups build ids' do + expect(matchers.map(&:build_ids)).to match_array([ + [build_without_tags.id], + match_array([build_with_tags.id, other_build_with_tags.id]) + ]) + end + + it { expect(matchers.map(&:tag_list)).to match_array([[], %w[tag1 tag2]]) } + + it { expect(matchers.map(&:protected?)).to all be_falsey } + + context 'when the builds are protected' do + before do + pipeline.builds.update_all(protected: true) + end + + it { expect(matchers).to all be_protected } + end + end + end + + describe '#build_matcher' do + let_it_be(:build) do + build_stubbed(:ci_build, tag_list: %w[tag1 tag2]) + end + + subject(:matcher) { build.build_matcher } + + it { expect(matcher.build_ids).to eq([build.id]) } + + it { expect(matcher.tag_list).to match_array(%w[tag1 tag2]) } + + it { expect(matcher.protected?).to eq(build.protected?) } + + it { expect(matcher.project).to eq(build.project) } + end + + describe '#shared_runner_build?' do + context 'when build does not have a runner assigned' do + it 'is not a shared runner build' do + expect(build.runner).to be_nil + + expect(build).not_to be_shared_runner_build + end + end + + context 'when build has a project runner assigned' do + before do + build.runner = create(:ci_runner, :project) + end + + it 'is not a shared runner build' do + expect(build).not_to be_shared_runner_build + end + end + + context 'when build has an instance runner assigned' do + before do + build.runner = create(:ci_runner, :instance_type) + end + + it 'is a shared runner build' do + expect(build).to be_shared_runner_build + end + end + end + + describe '.without_coverage' do + let!(:build_with_coverage) { create(:ci_build, pipeline: pipeline, coverage: 100.0) } + + it 'returns builds without coverage values' do + expect(described_class.without_coverage).to eq([build]) + end + end + + describe '.with_coverage_regex' do + let!(:build_with_coverage_regex) { create(:ci_build, pipeline: pipeline, coverage_regex: '\d') } + + it 'returns builds with coverage regex values' do + expect(described_class.with_coverage_regex).to eq([build_with_coverage_regex]) + end + end end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 12bc5d9aa3c..a16453f3d01 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -2,13 +2,13 @@ require 'spec_helper' -RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do +RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_gitlab_redis_trace_chunks do include ExclusiveLeaseHelpers let_it_be(:build) { create(:ci_build, :running) } let(:chunk_index) { 0 } - let(:data_store) { :redis } + let(:data_store) { :redis_trace_chunks } let(:raw_data) { nil } let(:build_trace_chunk) do @@ -18,10 +18,17 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do it_behaves_like 'having unique enum values' before do - stub_feature_flags(ci_enable_live_trace: true, gitlab_ci_trace_read_consistency: true) + stub_feature_flags(ci_enable_live_trace: true) stub_artifacts_object_storage end + def redis_instance + { + redis: Gitlab::Redis::SharedState, + redis_trace_chunks: Gitlab::Redis::TraceChunks + }[data_store] + end + describe 'chunk creation' do let(:metrics) { spy('metrics') } @@ -85,7 +92,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end def external_data_counter - Gitlab::Redis::SharedState.with do |redis| + redis_instance.with do |redis| redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size end end @@ -101,24 +108,16 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do subject { described_class.all_stores } it 'returns a correctly ordered array' do - is_expected.to eq(%i[redis database fog]) - end - - it 'returns redis store as the lowest precedence' do - expect(subject.first).to eq(:redis) - end - - it 'returns fog store as the highest precedence' do - expect(subject.last).to eq(:fog) + is_expected.to eq(%i[redis database fog redis_trace_chunks]) end end describe '#data' do subject { build_trace_chunk.data } - context 'when data_store is redis' do - let(:data_store) { :redis } + where(:data_store) { %i[redis redis_trace_chunks] } + with_them do before do build_trace_chunk.send(:unsafe_set_data!, +'Sample data in redis') end @@ -148,6 +147,22 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end + describe '#data_store' do + subject { described_class.new.data_store } + + context 'default value' do + it { expect(subject).to eq('redis_trace_chunks') } + + context 'when dedicated_redis_trace_chunks is disabled' do + before do + stub_feature_flags(dedicated_redis_trace_chunks: false) + end + + it { expect(subject).to eq('redis') } + end + end + end + describe '#get_store_class' do using RSpec::Parameterized::TableSyntax @@ -155,6 +170,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do :redis | Ci::BuildTraceChunks::Redis :database | Ci::BuildTraceChunks::Database :fog | Ci::BuildTraceChunks::Fog + :redis_trace_chunks | Ci::BuildTraceChunks::RedisTraceChunks end with_them do @@ -302,9 +318,9 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - context 'when data_store is redis' do - let(:data_store) { :redis } + where(:data_store) { %i[redis redis_trace_chunks] } + with_them do context 'when there are no data' do let(:data) { +'' } @@ -441,8 +457,9 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - context 'when data_store is redis' do - let(:data_store) { :redis } + where(:data_store) { %i[redis redis_trace_chunks] } + + with_them do let(:data) { +'Sample data in redis' } before do @@ -475,9 +492,9 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do describe '#size' do subject { build_trace_chunk.size } - context 'when data_store is redis' do - let(:data_store) { :redis } + where(:data_store) { %i[redis redis_trace_chunks] } + with_them do context 'when data exists' do let(:data) { +'Sample data in redis' } @@ -537,9 +554,14 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do subject { build_trace_chunk.persist_data! } - context 'when data_store is redis' do - let(:data_store) { :redis } + where(:data_store, :redis_class) do + [ + [:redis, Ci::BuildTraceChunks::Redis], + [:redis_trace_chunks, Ci::BuildTraceChunks::RedisTraceChunks] + ] + end + with_them do context 'when data exists' do before do build_trace_chunk.send(:unsafe_set_data!, data) @@ -549,15 +571,15 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:data) { +'a' * described_class::CHUNK_SIZE } it 'persists the data' do - expect(build_trace_chunk.redis?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) + expect(build_trace_chunk.data_store).to eq(data_store.to_s) + expect(redis_class.new.data(build_trace_chunk)).to eq(data) expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to be_nil subject expect(build_trace_chunk.fog?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(redis_class.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) end @@ -575,8 +597,8 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do it 'does not persist the data and the orignal data is intact' do expect { subject }.to raise_error(described_class::FailedToPersistDataError) - expect(build_trace_chunk.redis?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) + expect(build_trace_chunk.data_store).to eq(data_store.to_s) + expect(redis_class.new.data(build_trace_chunk)).to eq(data) expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to be_nil end @@ -810,7 +832,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do shared_examples_for 'deletes all build_trace_chunk and data in redis' do it 'deletes all build_trace_chunk and data in redis', :sidekiq_might_not_need_inline do - Gitlab::Redis::SharedState.with do |redis| + redis_instance.with do |redis| expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(3) end @@ -820,7 +842,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(described_class.count).to eq(0) - Gitlab::Redis::SharedState.with do |redis| + redis_instance.with do |redis| expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) end end @@ -902,4 +924,38 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end end + + describe '#live?' do + subject { build_trace_chunk.live? } + + where(:data_store, :value) do + [ + [:redis, true], + [:redis_trace_chunks, true], + [:database, false], + [:fog, false] + ] + end + + with_them do + it { is_expected.to eq(value) } + end + end + + describe '#flushed?' do + subject { build_trace_chunk.flushed? } + + where(:data_store, :value) do + [ + [:redis, false], + [:redis_trace_chunks, false], + [:database, true], + [:fog, true] + ] + end + + with_them do + it { is_expected.to eq(value) } + end + end end diff --git a/spec/models/ci/build_trace_chunks/database_spec.rb b/spec/models/ci/build_trace_chunks/database_spec.rb index 313328ac037..d99aede853c 100644 --- a/spec/models/ci/build_trace_chunks/database_spec.rb +++ b/spec/models/ci/build_trace_chunks/database_spec.rb @@ -5,12 +5,6 @@ require 'spec_helper' RSpec.describe Ci::BuildTraceChunks::Database do let(:data_store) { described_class.new } - describe '#available?' do - subject { data_store.available? } - - it { is_expected.to be_truthy } - end - describe '#data' do subject { data_store.data(model) } diff --git a/spec/models/ci/build_trace_chunks/redis_spec.rb b/spec/models/ci/build_trace_chunks/redis_spec.rb index cb0b6baadeb..c004887d609 100644 --- a/spec/models/ci/build_trace_chunks/redis_spec.rb +++ b/spec/models/ci/build_trace_chunks/redis_spec.rb @@ -5,12 +5,6 @@ require 'spec_helper' RSpec.describe Ci::BuildTraceChunks::Redis, :clean_gitlab_redis_shared_state do let(:data_store) { described_class.new } - describe '#available?' do - subject { data_store.available? } - - it { is_expected.to be_truthy } - end - describe '#data' do subject { data_store.data(model) } diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 3c4769764d5..582639b105e 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -328,35 +328,9 @@ RSpec.describe Ci::JobArtifact do end end - describe 'validates if file format is supported' do - subject { artifact } - - let(:artifact) { build(:ci_job_artifact, file_type: :license_management, file_format: :raw) } - - context 'when license_management is supported' do - before do - stub_feature_flags(drop_license_management_artifact: false) - end - - it { is_expected.to be_valid } - end - - context 'when license_management is not supported' do - before do - stub_feature_flags(drop_license_management_artifact: true) - end - - it { is_expected.not_to be_valid } - end - end - describe 'validates file format' do subject { artifact } - before do - stub_feature_flags(drop_license_management_artifact: false) - end - described_class::TYPE_AND_FORMAT_PAIRS.except(:trace).each do |file_type, file_format| context "when #{file_type} type with #{file_format} format" do let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: file_format) } diff --git a/spec/models/ci/job_token/project_scope_link_spec.rb b/spec/models/ci/job_token/project_scope_link_spec.rb new file mode 100644 index 00000000000..d18495b9312 --- /dev/null +++ b/spec/models/ci/job_token/project_scope_link_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobToken::ProjectScopeLink do + it { is_expected.to belong_to(:source_project) } + it { is_expected.to belong_to(:target_project) } + it { is_expected.to belong_to(:added_by) } + + let_it_be(:project) { create(:project) } + + describe 'unique index' do + let!(:link) { create(:ci_job_token_project_scope_link) } + + it 'raises an error' do + expect do + create(:ci_job_token_project_scope_link, + source_project: link.source_project, + target_project: link.target_project) + end.to raise_error(ActiveRecord::RecordNotUnique) + end + end + + describe 'validations' do + it 'must have a source project', :aggregate_failures do + link = build(:ci_job_token_project_scope_link, source_project: nil) + + expect(link).not_to be_valid + expect(link.errors[:source_project]).to contain_exactly("can't be blank") + end + + it 'must have a target project', :aggregate_failures do + link = build(:ci_job_token_project_scope_link, target_project: nil) + + expect(link).not_to be_valid + expect(link.errors[:target_project]).to contain_exactly("can't be blank") + end + + it 'must have a target project different than source project', :aggregate_failures do + link = build(:ci_job_token_project_scope_link, target_project: project, source_project: project) + + expect(link).not_to be_valid + expect(link.errors[:target_project]).to contain_exactly("can't be the same as the source project") + end + end + + describe '.from_project' do + subject { described_class.from_project(project) } + + let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } + let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } + + it 'returns only the links having the given source project' do + expect(subject).to contain_exactly(source_link) + end + end + + describe '.to_project' do + subject { described_class.to_project(project) } + + let!(:source_link) { create(:ci_job_token_project_scope_link, source_project: project) } + let!(:target_link) { create(:ci_job_token_project_scope_link, target_project: project) } + + it 'returns only the links having the given target project' do + expect(subject).to contain_exactly(target_link) + end + end +end diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb new file mode 100644 index 00000000000..c731a2634f5 --- /dev/null +++ b/spec/models/ci/job_token/scope_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::JobToken::Scope do + let_it_be(:project) { create(:project) } + + let(:scope) { described_class.new(project) } + + describe '#all_projects' do + subject(:all_projects) { scope.all_projects } + + context 'when no projects are added to the scope' do + it 'returns the project defining the scope' do + expect(all_projects).to contain_exactly(project) + end + end + + context 'when other projects are added to the scope' do + let_it_be(:scoped_project) { create(:project) } + let_it_be(:unscoped_project) { create(:project) } + + let!(:link_in_scope) { create(:ci_job_token_project_scope_link, source_project: project, target_project: scoped_project) } + let!(:link_out_of_scope) { create(:ci_job_token_project_scope_link, target_project: unscoped_project) } + + it 'returns all projects that can be accessed from a given scope' do + expect(subject).to contain_exactly(project, scoped_project) + end + end + end + + describe 'includes?' do + subject { scope.includes?(target_project) } + + context 'when param is the project defining the scope' do + let(:target_project) { project } + + it { is_expected.to be_truthy } + end + + context 'when param is a project in scope' do + let(:target_link) { create(:ci_job_token_project_scope_link, source_project: project) } + let(:target_project) { target_link.target_project } + + it { is_expected.to be_truthy } + end + + context 'when param is a project in another scope' do + let(:scope_link) { create(:ci_job_token_project_scope_link) } + let(:target_project) { scope_link.target_project } + + it { is_expected.to be_falsey } + + context 'when project scope setting is disabled' do + before do + project.ci_job_token_scope_enabled = false + end + + it 'considers any project to be part of the scope' do + expect(subject).to be_truthy + end + end + end + end +end diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb new file mode 100644 index 00000000000..c1d4f4b0a5e --- /dev/null +++ b/spec/models/ci/pending_build_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PendingBuild do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:build) { create(:ci_build, :created, pipeline: pipeline) } + + describe '.upsert_from_build!' do + context 'another pending entry does not exist' do + it 'creates a new pending entry' do + result = described_class.upsert_from_build!(build) + + expect(result.rows.dig(0, 0)).to eq build.id + expect(build.reload.queuing_entry).to be_present + end + end + + context 'when another queuing entry exists for given build' do + before do + described_class.create!(build: build, project: project, protected: false) + end + + it 'returns a build id as a result' do + result = described_class.upsert_from_build!(build) + + expect(result.rows.dig(0, 0)).to eq build.id + end + end + end +end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index d5560edbbfd..cf73460bf1e 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Ci::PipelineSchedule do + let_it_be(:project) { create_default(:project) } + subject { build(:ci_pipeline_schedule) } it { is_expected.to belong_to(:project) } @@ -18,7 +20,7 @@ RSpec.describe Ci::PipelineSchedule do it { is_expected.to respond_to(:next_run_at) } it_behaves_like 'includes Limitable concern' do - subject { build(:ci_pipeline_schedule) } + subject { build(:ci_pipeline_schedule, project: project) } end describe 'validations' do @@ -103,26 +105,49 @@ RSpec.describe Ci::PipelineSchedule do end describe '#set_next_run_at' do - let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } - let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_from, Time.zone.now) } - let(:cron_worker_next_run_at) { pipeline_schedule.send(:cron_worker_next_run_from, Time.zone.now) } + using RSpec::Parameterized::TableSyntax + + where(:worker_cron, :schedule_cron, :plan_limit, :ff_enabled, :now, :result) do + '0 1 2 3 *' | '0 1 * * *' | nil | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) + '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) + '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) + '*/5 * * * *' | '*/1 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) + '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) + '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) + '*/5 * * * *' | '*/1 * * * *' | 200 | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) + '*/5 * * * *' | '*/1 * * * *' | 200 | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) + '*/5 * * * *' | '0 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 2.hours.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) + '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) + '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) + '*/5 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 1, 1, 0) | Time.zone.local(2021, 6, 1, 1, 0) + end + + with_them do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, cron: schedule_cron) } - context 'when PipelineScheduleWorker runs at a specific interval' do before do allow(Settings).to receive(:cron_jobs) do - { - 'pipeline_schedule_worker' => { - 'cron' => '0 1 2 3 *' - } - } + { 'pipeline_schedule_worker' => { 'cron' => worker_cron } } end + + create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: plan_limit) if plan_limit + stub_feature_flags(ci_daily_limit_for_pipeline_schedules: false) unless ff_enabled + + # Setting this here to override initial save with the current time + pipeline_schedule.next_run_at = now end - it "updates next_run_at to the sidekiq worker's execution time" do - expect(pipeline_schedule.next_run_at.min).to eq(0) - expect(pipeline_schedule.next_run_at.hour).to eq(1) - expect(pipeline_schedule.next_run_at.day).to eq(2) - expect(pipeline_schedule.next_run_at.month).to eq(3) + it 'updates next_run_at' do + travel_to(now) do + pipeline_schedule.set_next_run_at + + expect(pipeline_schedule.next_run_at).to eq(result) + end end end @@ -176,4 +201,26 @@ RSpec.describe Ci::PipelineSchedule do it { is_expected.to contain_exactly(*pipeline_schedule_variables.map(&:to_runner_variable)) } end + + describe '#daily_limit' do + let(:pipeline_schedule) { build(:ci_pipeline_schedule) } + + subject(:daily_limit) { pipeline_schedule.daily_limit } + + context 'when there is no limit' do + before do + create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: 0) + end + + it { is_expected.to be_nil } + end + + context 'when there is a limit' do + before do + create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: 144) + end + + it { is_expected.to eq(144) } + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b9457055a18..72af40e31e0 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -744,6 +744,42 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#update_builds_coverage' do + let_it_be(:pipeline) { create(:ci_empty_pipeline) } + + context 'builds with coverage_regex defined' do + let!(:build_1) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 60.0, pipeline: pipeline) } + let!(:build_2) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 80.0, pipeline: pipeline) } + + it 'updates the coverage value of each build from the trace' do + pipeline.update_builds_coverage + + expect(build_1.reload.coverage).to eq(60.0) + expect(build_2.reload.coverage).to eq(80.0) + end + end + + context 'builds without coverage_regex defined' do + let!(:build) { create(:ci_build, :success, :trace_with_coverage, coverage_regex: nil, trace_coverage: 60.0, pipeline: pipeline) } + + it 'does not update the coverage value of each build from the trace' do + pipeline.update_builds_coverage + + expect(build.reload.coverage).to eq(nil) + end + end + + context 'builds with coverage values already present' do + let!(:build) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 60.0, coverage: 10.0, pipeline: pipeline) } + + it 'does not update the coverage value of each build from the trace' do + pipeline.update_builds_coverage + + expect(build.reload.coverage).to eq(10.0) + end + end + end + describe '#retryable?' do subject { pipeline.retryable? } @@ -2726,7 +2762,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do pipeline2.cancel_running end - extra_update_queries = 3 # transition ... => :canceled + extra_update_queries = 4 # transition ... => :canceled, queue pop extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries) @@ -3162,6 +3198,81 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#environments_in_self_and_descendants' do + subject { pipeline.environments_in_self_and_descendants } + + context 'when pipeline is not child nor parent' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + let_it_be(:build) { create(:ci_build, :with_deployment, :deploy_to_production, pipeline: pipeline) } + + it 'returns just the pipeline environment' do + expect(subject).to contain_exactly(build.deployment.environment) + end + end + + context 'when pipeline is in extended family' do + let_it_be(:parent) { create(:ci_pipeline) } + let_it_be(:parent_build) { create(:ci_build, :with_deployment, environment: 'staging', pipeline: parent) } + + let_it_be(:pipeline) { create(:ci_pipeline, child_of: parent) } + let_it_be(:build) { create(:ci_build, :with_deployment, :deploy_to_production, pipeline: pipeline) } + + let_it_be(:child) { create(:ci_pipeline, child_of: pipeline) } + let_it_be(:child_build) { create(:ci_build, :with_deployment, environment: 'canary', pipeline: child) } + + let_it_be(:grandchild) { create(:ci_pipeline, child_of: child) } + let_it_be(:grandchild_build) { create(:ci_build, :with_deployment, environment: 'test', pipeline: grandchild) } + + let_it_be(:sibling) { create(:ci_pipeline, child_of: parent) } + let_it_be(:sibling_build) { create(:ci_build, :with_deployment, environment: 'review', pipeline: sibling) } + + it 'returns its own environment and from all descendants' do + expected_environments = [ + build.deployment.environment, + child_build.deployment.environment, + grandchild_build.deployment.environment + ] + expect(subject).to match_array(expected_environments) + end + + it 'does not return parent environment' do + expect(subject).not_to include(parent_build.deployment.environment) + end + + it 'does not return sibling environment' do + expect(subject).not_to include(sibling_build.deployment.environment) + end + end + + context 'when each pipeline has multiple environments' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + let_it_be(:build1) { create(:ci_build, :with_deployment, :deploy_to_production, pipeline: pipeline) } + let_it_be(:build2) { create(:ci_build, :with_deployment, environment: 'staging', pipeline: pipeline) } + + let_it_be(:child) { create(:ci_pipeline, child_of: pipeline) } + let_it_be(:child_build1) { create(:ci_build, :with_deployment, environment: 'canary', pipeline: child) } + let_it_be(:child_build2) { create(:ci_build, :with_deployment, environment: 'test', pipeline: child) } + + it 'returns all related environments' do + expected_environments = [ + build1.deployment.environment, + build2.deployment.environment, + child_build1.deployment.environment, + child_build2.deployment.environment + ] + expect(subject).to match_array(expected_environments) + end + end + + context 'when pipeline has no environment' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + + it 'returns empty' do + expect(subject).to be_empty + end + end + end + describe '#root_ancestor' do subject { pipeline.root_ancestor } @@ -4512,4 +4623,17 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do .not_to exceed_query_limit(control_count) end end + + describe '#build_matchers' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:builds) { create_list(:ci_build, 2, pipeline: pipeline, project: pipeline.project) } + + subject(:matchers) { pipeline.build_matchers } + + it 'returns build matchers' do + expect(matchers.size).to eq(1) + expect(matchers).to all be_a(Gitlab::Ci::Matching::BuildMatcher) + expect(matchers.first.build_ids).to match_array(builds.map(&:id)) + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ffe0b0d0b19..61f80bd43b1 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -75,6 +75,22 @@ RSpec.describe Ci::Runner do expect { create(:group, runners: [project_runner]) } .to raise_error(ActiveRecord::RecordInvalid) end + + context 'when runner has config' do + it 'is valid' do + runner = build(:ci_runner, config: { gpus: "all" }) + + expect(runner).to be_valid + end + end + + context 'when runner has an invalid config' do + it 'is invalid' do + runner = build(:ci_runner, config: { test: 1 }) + + expect(runner).not_to be_valid + end + end end context 'cost factors validations' do @@ -257,6 +273,20 @@ RSpec.describe Ci::Runner do end end + describe '.recent' do + subject { described_class.recent } + + before do + @runner1 = create(:ci_runner, :instance, contacted_at: nil, created_at: 2.months.ago) + @runner2 = create(:ci_runner, :instance, contacted_at: nil, created_at: 3.months.ago) + @runner3 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 2.months.ago) + @runner4 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 3.months.ago) + @runner5 = create(:ci_runner, :instance, contacted_at: 3.months.ago, created_at: 5.months.ago) + end + + it { is_expected.to eq([@runner1, @runner3, @runner4])} + end + describe '.online' do subject { described_class.online } @@ -349,6 +379,22 @@ RSpec.describe Ci::Runner do it { is_expected.to eq([@runner1])} end + describe '#tick_runner_queue' do + it 'sticks the runner to the primary and calls the original method' do + runner = create(:ci_runner) + + allow(Gitlab::Database::LoadBalancing).to receive(:enable?) + .and_return(true) + + expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + .with(:runner, runner.id) + + expect(Gitlab::Workhorse).to receive(:set_key_and_notify) + + runner.tick_runner_queue + end + end + describe '#can_pick?' do using RSpec::Parameterized::TableSyntax @@ -653,7 +699,7 @@ RSpec.describe Ci::Runner do describe '#heartbeat' do let(:runner) { create(:ci_runner, :project) } - subject { runner.heartbeat(architecture: '18-bit') } + subject { runner.heartbeat(architecture: '18-bit', config: { gpus: "all" }) } context 'when database was updated recently' do before do @@ -701,6 +747,7 @@ RSpec.describe Ci::Runner do def does_db_update expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } .and change { runner.reload.read_attribute(:architecture) } + .and change { runner.reload.read_attribute(:config) } end end @@ -826,12 +873,12 @@ RSpec.describe Ci::Runner do expect(described_class.search(runner.token)).to eq([runner]) end - it 'returns runners with a partially matching token' do - expect(described_class.search(runner.token[0..2])).to eq([runner]) + it 'does not return runners with a partially matching token' do + expect(described_class.search(runner.token[0..2])).to be_empty end - it 'returns runners with a matching token regardless of the casing' do - expect(described_class.search(runner.token.upcase)).to eq([runner]) + it 'does not return runners with a matching token with different casing' do + expect(described_class.search(runner.token.upcase)).to be_empty end it 'returns runners with a matching description' do @@ -919,29 +966,13 @@ RSpec.describe Ci::Runner do end end - context 'build picking improvement enabled' do - before do - stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: true) - end - + context 'build picking improvement' do it 'does not check if the build is assignable to a runner' do expect(runner).not_to receive(:can_pick?) runner.pick_build!(build) end end - - context 'build picking improvement disabled' do - before do - stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: false) - end - - it 'checks if the build is assignable to a runner' do - expect(runner).to receive(:can_pick?).and_call_original - - runner.pick_build!(build) - end - end end describe 'project runner without projects is destroyable' do @@ -975,6 +1006,108 @@ RSpec.describe Ci::Runner do end end + describe '.runner_matchers' do + subject(:matchers) { described_class.all.runner_matchers } + + context 'deduplicates on runner_type' do + before do + create_list(:ci_runner, 2, :instance) + create_list(:ci_runner, 2, :project) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:runner_type)).to match_array(%w[instance_type project_type]) + end + end + + context 'deduplicates on public_projects_minutes_cost_factor' do + before do + create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 5) + create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 10) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:public_projects_minutes_cost_factor)).to match_array([5, 10]) + end + end + + context 'deduplicates on private_projects_minutes_cost_factor' do + before do + create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 5) + create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 10) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:private_projects_minutes_cost_factor)).to match_array([5, 10]) + end + end + + context 'deduplicates on run_untagged' do + before do + create_list(:ci_runner, 2, run_untagged: true, tag_list: ['a']) + create_list(:ci_runner, 2, run_untagged: false, tag_list: ['a']) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:run_untagged)).to match_array([true, false]) + end + end + + context 'deduplicates on access_level' do + before do + create_list(:ci_runner, 2, access_level: :ref_protected) + create_list(:ci_runner, 2, access_level: :not_protected) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:access_level)).to match_array(%w[ref_protected not_protected]) + end + end + + context 'deduplicates on tag_list' do + before do + create_list(:ci_runner, 2, tag_list: %w[tag1 tag2]) + create_list(:ci_runner, 2, tag_list: %w[tag3 tag4]) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:tag_list)).to match_array([%w[tag1 tag2], %w[tag3 tag4]]) + end + end + end + + describe '#runner_matcher' do + let(:runner) do + build_stubbed(:ci_runner, :instance_type, tag_list: %w[tag1 tag2]) + end + + subject(:matcher) { runner.runner_matcher } + + it { expect(matcher.runner_type).to eq(runner.runner_type) } + + it { expect(matcher.public_projects_minutes_cost_factor).to eq(runner.public_projects_minutes_cost_factor) } + + it { expect(matcher.private_projects_minutes_cost_factor).to eq(runner.private_projects_minutes_cost_factor) } + + it { expect(matcher.run_untagged).to eq(runner.run_untagged) } + + it { expect(matcher.access_level).to eq(runner.access_level) } + + it { expect(matcher.tag_list).to match_array(runner.tag_list) } + end + describe '#uncached_contacted_at' do let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) } let(:runner) { create(:ci_runner, contacted_at: contacted_at_stored) } diff --git a/spec/models/ci/running_build_spec.rb b/spec/models/ci/running_build_spec.rb new file mode 100644 index 00000000000..589e5a86f4d --- /dev/null +++ b/spec/models/ci/running_build_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::RunningBuild do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:runner) { create(:ci_runner, :instance_type) } + let(:build) { create(:ci_build, :running, runner: runner, pipeline: pipeline) } + + describe '.upsert_shared_runner_build!' do + context 'another pending entry does not exist' do + it 'creates a new pending entry' do + result = described_class.upsert_shared_runner_build!(build) + + expect(result.rows.dig(0, 0)).to eq build.id + expect(build.reload.runtime_metadata).to be_present + end + end + + context 'when another queuing entry exists for given build' do + before do + described_class.create!(build: build, + project: project, + runner: runner, + runner_type: runner.runner_type) + end + + it 'returns a build id as a result' do + result = described_class.upsert_shared_runner_build!(build) + + expect(result.rows.dig(0, 0)).to eq build.id + end + end + + context 'when build has been picked by a specific runner' do + let(:runner) { create(:ci_runner, :project) } + + it 'raises an error' do + expect { described_class.upsert_shared_runner_build!(build) } + .to raise_error(ArgumentError, 'build has not been picked by a shared runner') + end + end + + context 'when build has not been picked by a runner yet' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'raises an error' do + expect { described_class.upsert_shared_runner_build!(build) } + .to raise_error(ArgumentError, 'build has not been picked by a shared runner') + end + end + end +end diff --git a/spec/models/clusters/applications/fluentd_spec.rb b/spec/models/clusters/applications/fluentd_spec.rb deleted file mode 100644 index ccdf6b0e40d..00000000000 --- a/spec/models/clusters/applications/fluentd_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Clusters::Applications::Fluentd do - let(:waf_log_enabled) { true } - let(:cilium_log_enabled) { true } - let(:fluentd) { create(:clusters_applications_fluentd, waf_log_enabled: waf_log_enabled, cilium_log_enabled: cilium_log_enabled) } - - include_examples 'cluster application core specs', :clusters_applications_fluentd - include_examples 'cluster application status specs', :clusters_applications_fluentd - include_examples 'cluster application version specs', :clusters_applications_fluentd - include_examples 'cluster application initial status specs' - - describe '#can_uninstall?' do - subject { fluentd.can_uninstall? } - - it { is_expected.to be true } - end - - describe '#install_command' do - subject { fluentd.install_command } - - it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::V3::InstallCommand) } - - it 'is initialized with fluentd arguments' do - expect(subject.name).to eq('fluentd') - expect(subject.chart).to eq('fluentd/fluentd') - expect(subject.version).to eq('2.4.0') - expect(subject).to be_rbac - end - - context 'application failed to install previously' do - let(:fluentd) { create(:clusters_applications_fluentd, :errored, version: '0.0.1') } - - it 'is initialized with the locked version' do - expect(subject.version).to eq('2.4.0') - end - end - end - - describe '#files' do - let(:application) { fluentd } - let(:values) { subject[:'values.yaml'] } - - subject { application.files } - - it 'includes fluentd specific keys in the values.yaml file' do - expect(values).to include('output.conf', 'general.conf') - end - end - - describe '#values' do - let(:modsecurity_log_path) { "/var/log/containers/*#{Clusters::Applications::Ingress::MODSECURITY_LOG_CONTAINER_NAME}*.log" } - let(:cilium_log_path) { "/var/log/containers/*#{described_class::CILIUM_CONTAINER_NAME}*.log" } - - subject { fluentd.values } - - context 'with both logs variables set to false' do - let(:waf_log_enabled) { false } - let(:cilium_log_enabled) { false } - - it "raises ActiveRecord::RecordInvalid" do - expect {subject}.to raise_error(ActiveRecord::RecordInvalid) - end - end - - context 'with both logs variables set to true' do - it { is_expected.to include("#{modsecurity_log_path},#{cilium_log_path}") } - end - - context 'with waf_log_enabled set to true' do - let(:cilium_log_enabled) { false } - - it { is_expected.to include(modsecurity_log_path) } - end - - context 'with cilium_log_enabled set to true' do - let(:waf_log_enabled) { false } - - it { is_expected.to include(cilium_log_path) } - end - end -end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 1bc1a4343aa..e16d97c42d9 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -172,94 +172,4 @@ RSpec.describe Clusters::Applications::Ingress do expect(values).to include('clusterIP') end end - - describe '#values' do - subject { ingress } - - context 'when modsecurity_enabled is enabled' do - before do - allow(subject).to receive(:modsecurity_enabled).and_return(true) - end - - it 'includes modsecurity module enablement' do - expect(subject.values).to include("enable-modsecurity: 'true'") - end - - it 'includes modsecurity core ruleset enablement set to false' do - expect(subject.values).to include("enable-owasp-modsecurity-crs: 'false'") - end - - it 'includes modsecurity snippet with information related to security rules' do - expect(subject.values).to include("SecRuleEngine DetectionOnly") - expect(subject.values).to include("Include #{described_class::MODSECURITY_OWASP_RULES_FILE}") - end - - context 'when modsecurity_mode is set to :blocking' do - before do - subject.blocking! - end - - it 'includes modsecurity snippet with information related to security rules' do - expect(subject.values).to include("SecRuleEngine On") - expect(subject.values).to include("Include #{described_class::MODSECURITY_OWASP_RULES_FILE}") - end - end - - it 'includes modsecurity.conf content' do - expect(subject.values).to include('modsecurity.conf') - # Includes file content from Ingress#modsecurity_config_content - expect(subject.values).to include('SecAuditLog') - - expect(subject.values).to include('extraVolumes') - expect(subject.values).to include('extraVolumeMounts') - end - - it 'includes modsecurity sidecar container' do - expect(subject.values).to include('modsecurity-log-volume') - - expect(subject.values).to include('extraContainers') - end - - it 'executes command to tail modsecurity logs with -F option' do - args = YAML.safe_load(subject.values).dig('controller', 'extraContainers', 0, 'args') - - expect(args).to eq(['/bin/sh', '-c', 'tail -F /var/log/modsec/audit.log']) - end - - it 'includes livenessProbe for modsecurity sidecar container' do - probe_config = YAML.safe_load(subject.values).dig('controller', 'extraContainers', 0, 'livenessProbe') - - expect(probe_config).to eq('exec' => { 'command' => ['ls', '/var/log/modsec/audit.log'] }) - end - end - - context 'when modsecurity_enabled is disabled' do - before do - allow(subject).to receive(:modsecurity_enabled).and_return(false) - end - - it 'excludes modsecurity module enablement' do - expect(subject.values).not_to include('enable-modsecurity') - end - - it 'excludes modsecurity core ruleset enablement' do - expect(subject.values).not_to include('enable-owasp-modsecurity-crs') - end - - it 'excludes modsecurity.conf content' do - expect(subject.values).not_to include('modsecurity.conf') - # Excludes file content from Ingress#modsecurity_config_content - expect(subject.values).not_to include('SecAuditLog') - - expect(subject.values).not_to include('extraVolumes') - expect(subject.values).not_to include('extraVolumeMounts') - end - - it 'excludes modsecurity sidecar container' do - expect(subject.values).not_to include('modsecurity-log-volume') - - expect(subject.values).not_to include('extraContainers') - end - end - end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index b2ed64fd9b0..278e200b05c 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -42,7 +42,8 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_knative).with_prefix } - it { is_expected.to delegate_method(:available?).to(:application_elastic_stack).with_prefix } + it { is_expected.to delegate_method(:available?).to(:integration_elastic_stack).with_prefix } + it { is_expected.to delegate_method(:available?).to(:integration_prometheus).with_prefix } it { is_expected.to delegate_method(:external_ip).to(:application_ingress).with_prefix } it { is_expected.to delegate_method(:external_hostname).to(:application_ingress).with_prefix } @@ -195,28 +196,6 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '.with_enabled_modsecurity' do - subject { described_class.with_enabled_modsecurity } - - let_it_be(:cluster) { create(:cluster) } - - context 'cluster has ingress application with enabled modsecurity' do - let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_logging, cluster: cluster) } - - it { is_expected.to include(cluster) } - end - - context 'cluster has ingress application with disabled modsecurity' do - let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_disabled, cluster: cluster) } - - it { is_expected.not_to include(cluster) } - end - - context 'cluster does not have ingress application' do - it { is_expected.not_to include(cluster) } - end - end - describe '.with_available_elasticstack' do subject { described_class.with_available_elasticstack } @@ -1042,7 +1021,6 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do where(:status_name, :cleanup_status) do provider_status | :cleanup_not_started - :cleanup_ongoing | :cleanup_uninstalling_applications :cleanup_ongoing | :cleanup_removing_project_namespaces :cleanup_ongoing | :cleanup_removing_service_account :cleanup_errored | :cleanup_errored @@ -1098,8 +1076,8 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end describe '#start_cleanup!' do - let(:expected_worker_class) { Clusters::Cleanup::AppWorker } - let(:to_state) { :cleanup_uninstalling_applications } + let(:expected_worker_class) { Clusters::Cleanup::ProjectNamespaceWorker } + let(:to_state) { :cleanup_removing_project_namespaces } subject { cluster.start_cleanup! } @@ -1137,25 +1115,13 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end describe '#continue_cleanup!' do - context 'when cleanup_status is cleanup_uninstalling_applications' do - let(:expected_worker_class) { Clusters::Cleanup::ProjectNamespaceWorker } - let(:from_state) { :cleanup_uninstalling_applications } - let(:to_state) { :cleanup_removing_project_namespaces } - - subject { cluster.continue_cleanup! } + let(:expected_worker_class) { Clusters::Cleanup::ServiceAccountWorker } + let(:from_state) { :cleanup_removing_project_namespaces } + let(:to_state) { :cleanup_removing_service_account } - it_behaves_like 'cleanup_status transition' - end - - context 'when cleanup_status is cleanup_removing_project_namespaces' do - let(:expected_worker_class) { Clusters::Cleanup::ServiceAccountWorker } - let(:from_state) { :cleanup_removing_project_namespaces } - let(:to_state) { :cleanup_removing_service_account } + subject { cluster.continue_cleanup! } - subject { cluster.continue_cleanup! } - - it_behaves_like 'cleanup_status transition' - end + it_behaves_like 'cleanup_status transition' end end @@ -1349,45 +1315,23 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '#application_prometheus_available?' do + describe '#integration_prometheus_available?' do let_it_be_with_reload(:cluster) { create(:cluster, :project) } - subject { cluster.application_prometheus_available? } + subject { cluster.integration_prometheus_available? } it { is_expected.to be_falsey } - context 'has a integration_prometheus' do - let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } + context 'when integration is enabled' do + let!(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } it { is_expected.to be_truthy } - - context 'disabled' do - before do - cluster.integration_prometheus.enabled = false - end - - it { is_expected.to be_falsey } - end end - context 'has a application_prometheus' do - let_it_be(:application) { create(:clusters_applications_prometheus, :installed, :no_helm_installed, cluster: cluster) } + context 'when integration is disabled' do + let!(:integration) { create(:clusters_integrations_prometheus, enabled: false, cluster: cluster) } - it { is_expected.to be_truthy } - - context 'errored' do - before do - cluster.application_prometheus.status = Clusters::Applications::Prometheus.state_machines[:status].states[:errored] - end - - it { is_expected.to be_falsey } - end - - context 'also has a integration_prometheus' do - let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } - - it { is_expected.to be_truthy } - end + it { is_expected.to be_falsey } end end @@ -1398,7 +1342,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do expect(cluster.prometheus_adapter).to be_nil end - context 'has a integration_prometheus' do + context 'has integration_prometheus' do let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } it 'returns the integration' do @@ -1406,11 +1350,11 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - context 'has a application_prometheus' do + context 'has application_prometheus' do let_it_be(:application) { create(:clusters_applications_prometheus, :no_helm_installed, cluster: cluster) } - it 'returns the application' do - expect(cluster.prometheus_adapter).to eq(application) + it 'returns nil' do + expect(cluster.prometheus_adapter).to be_nil end context 'also has a integration_prometheus' do diff --git a/spec/models/clusters/clusters_hierarchy_spec.rb b/spec/models/clusters/clusters_hierarchy_spec.rb index 5ac561eb2d0..5dd2fe98352 100644 --- a/spec/models/clusters/clusters_hierarchy_spec.rb +++ b/spec/models/clusters/clusters_hierarchy_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' RSpec.describe Clusters::ClustersHierarchy do describe '#base_and_ancestors' do - def base_and_ancestors(clusterable, include_management_project: true) - described_class.new(clusterable, include_management_project: include_management_project).base_and_ancestors + def base_and_ancestors(clusterable) + described_class.new(clusterable).base_and_ancestors end context 'project in nested group with clusters at every level' do @@ -101,10 +101,6 @@ RSpec.describe Clusters::ClustersHierarchy do expect(base_and_ancestors(management_project)).to eq([ancestor, child]) end - it 'returns clusters for management_project' do - expect(base_and_ancestors(management_project, include_management_project: false)).to eq([child, ancestor]) - end - it 'returns clusters for project' do expect(base_and_ancestors(project)).to eq([child, ancestor]) end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7c00f367844..8ffc198fc4d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -471,16 +471,25 @@ eos end it_behaves_like 'a mentionable' do - subject { create(:project, :repository).commit } + subject(:commit) { create(:project, :repository).commit } let(:author) { create(:user, email: subject.author_email) } let(:backref_text) { "commit #{subject.id}" } let(:set_mentionable_text) do - ->(txt) { allow(subject).to receive(:safe_message).and_return(txt) } + ->(txt) { allow(commit).to receive(:safe_message).and_return(txt) } end # Include the subject in the repository stub. - let(:extra_commits) { [subject] } + let(:extra_commits) { [commit] } + + it 'uses the CachedMarkdownField cache instead of the Mentionable cache', :use_clean_rails_redis_caching do + expect(commit.title_html).not_to be_present + + commit.all_references(project.owner).all + + expect(commit.title_html).to be_present + expect(Rails.cache.read("banzai/commit:#{commit.id}/safe_message/single_line")).to be_nil + end end describe '#hook_attrs' do @@ -663,6 +672,92 @@ eos it_behaves_like '#uri_type' end + describe '.diff_max_files' do + subject(:diff_max_files) { described_class.diff_max_files } + + let(:increased_diff_limits) { false } + let(:configurable_diff_limits) { false } + + before do + stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits) + end + + context 'when increased_diff_limits is enabled' do + let(:increased_diff_limits) { true } + + it 'returns 3000' do + expect(diff_max_files).to eq(3000) + end + end + + context 'when configurable_diff_limits is enabled' do + let(:configurable_diff_limits) { true } + + it 'returns the current settings' do + Gitlab::CurrentSettings.update!(diff_max_files: 1234) + expect(diff_max_files).to eq(1234) + end + end + + context 'when neither feature flag is enabled' do + it 'returns 1000' do + expect(diff_max_files).to eq(1000) + end + end + end + + describe '.diff_max_lines' do + subject(:diff_max_lines) { described_class.diff_max_lines } + + let(:increased_diff_limits) { false } + let(:configurable_diff_limits) { false } + + before do + stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits) + end + + context 'when increased_diff_limits is enabled' do + let(:increased_diff_limits) { true } + + it 'returns 100000' do + expect(diff_max_lines).to eq(100000) + end + end + + context 'when configurable_diff_limits is enabled' do + let(:configurable_diff_limits) { true } + + it 'returns the current settings' do + Gitlab::CurrentSettings.update!(diff_max_lines: 65321) + expect(diff_max_lines).to eq(65321) + end + end + + context 'when neither feature flag is enabled' do + it 'returns 50000' do + expect(diff_max_lines).to eq(50000) + end + end + end + + describe '.diff_safe_max_files' do + subject(:diff_safe_max_files) { described_class.diff_safe_max_files } + + it 'returns the commit diff max divided by the limit factor of 10' do + expect(::Commit).to receive(:diff_max_files).and_return(10) + expect(diff_safe_max_files).to eq(1) + end + end + + describe '.diff_safe_max_lines' do + subject(:diff_safe_max_lines) { described_class.diff_safe_max_lines } + + it 'returns the commit diff max divided by the limit factor of 10' do + expect(::Commit).to receive(:diff_max_lines).and_return(100) + expect(diff_safe_max_lines).to eq(10) + end + end + describe '.from_hash' do subject { described_class.from_hash(commit.to_hash, container) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index feb2f3630c1..69b4d752f4c 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -79,6 +79,32 @@ RSpec.describe CommitStatus do end end + describe '.updated_before' do + let!(:lookback) { 5.days.ago } + let!(:timeout) { 1.day.ago } + let!(:before_lookback) { lookback - 1.hour } + let!(:after_lookback) { lookback + 1.hour } + let!(:before_timeout) { timeout - 1.hour } + let!(:after_timeout) { timeout + 1.hour } + + subject { described_class.updated_before(lookback: lookback, timeout: timeout) } + + def create_build_with_set_timestamps(created_at:, updated_at:) + travel_to(created_at) { create(:ci_build, created_at: Time.current) }.tap do |build| + travel_to(updated_at) { build.update!(status: :failed) } + end + end + + it 'finds builds updated and created in the window between lookback and timeout' do + build_in_lookback_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: before_timeout) + build_outside_lookback_window = create_build_with_set_timestamps(created_at: before_lookback, updated_at: before_timeout) + build_outside_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: after_timeout) + + expect(subject).to contain_exactly(build_in_lookback_timeout_window) + expect(subject).not_to include(build_outside_lookback_window, build_outside_timeout_window) + end + end + describe '#processed' do subject { commit_status.processed } diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index b5b3772ecb6..b80b6ec95e2 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -108,7 +108,7 @@ RSpec.describe Awardable do it "doesn't include unused thumbs buttons when disabled in project" do issue_without_downvote.project.show_default_award_emojis = false - expect(issue_without_downvote.grouped_awards.keys.sort).to eq [] + expect(issue_without_downvote.grouped_awards.keys.sort).to be_empty end it "includes unused thumbs buttons when enabled in project" do @@ -118,7 +118,7 @@ RSpec.describe Awardable do end it "doesn't include unused thumbs buttons in summary" do - expect(issue_without_downvote.grouped_awards(with_thumbs: false).keys).to eq [] + expect(issue_without_downvote.grouped_awards(with_thumbs: false).keys).to be_empty end it "includes used thumbs buttons when disabled in project" do diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index ca6df506ee8..209ee1264d5 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -20,6 +20,13 @@ RSpec.describe BulkInsertSafe do t.index :name, unique: true end + + create_table :bulk_insert_items_with_composite_pk, id: false, force: true do |t| + t.integer :id, null: true + t.string :name, null: true + end + + execute("ALTER TABLE bulk_insert_items_with_composite_pk ADD PRIMARY KEY (id,name);") end end @@ -27,6 +34,7 @@ RSpec.describe BulkInsertSafe do ActiveRecord::Schema.define do drop_table :bulk_insert_items, force: true drop_table :bulk_insert_parent_items, force: true + drop_table :bulk_insert_items_with_composite_pk, force: true end end @@ -227,5 +235,28 @@ RSpec.describe BulkInsertSafe do end end end + + context 'when a model with composite primary key is inserted' do + let_it_be(:bulk_insert_items_with_composite_pk_class) do + Class.new(ActiveRecord::Base) do + self.table_name = 'bulk_insert_items_with_composite_pk' + + include BulkInsertSafe + end + end + + let(:new_object) { bulk_insert_items_with_composite_pk_class.new(id: 1, name: 'composite') } + + it 'successfully inserts an item' do + expect(ActiveRecord::InsertAll).to receive(:new) + .with( + bulk_insert_items_with_composite_pk_class, [new_object.as_json], on_duplicate: :raise, returning: false, unique_by: %w[id name] + ).and_call_original + + expect { bulk_insert_items_with_composite_pk_class.bulk_insert!([new_object]) }.to( + change(bulk_insert_items_with_composite_pk_class, :count).from(0).to(1) + ) + end + end end end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 2bb6aa27e21..7fa55184cf1 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -254,20 +254,8 @@ RSpec.describe DeploymentPlatform do create(:cluster, :provided_by_user, projects: [another_project], management_project: project) end - context 'cluster_management_project feature is enabled' do - it 'returns the cluster with management project' do - is_expected.to eq(cluster_with_management_project.platform_kubernetes) - end - end - - context 'cluster_management_project feature is disabled' do - before do - stub_feature_flags(cluster_management_project: false) - end - - it 'returns nothing' do - is_expected.to be_nil - end + it 'returns the cluster with management project' do + is_expected.to eq(cluster_with_management_project.platform_kubernetes) end end @@ -311,20 +299,8 @@ RSpec.describe DeploymentPlatform do create(:cluster, :provided_by_user, projects: [another_project], management_project: project) end - context 'cluster_management_project feature is enabled' do - it 'returns the cluster with management project' do - is_expected.to eq(cluster_with_management_project.platform_kubernetes) - end - end - - context 'cluster_management_project feature is disabled' do - before do - stub_feature_flags(cluster_management_project: false) - end - - it 'returns the group cluster' do - is_expected.to eq(group_cluster.platform_kubernetes) - end + it 'returns the cluster with management project' do + is_expected.to eq(cluster_with_management_project.platform_kubernetes) end end diff --git a/spec/models/concerns/has_timelogs_report_spec.rb b/spec/models/concerns/has_timelogs_report_spec.rb deleted file mode 100644 index f0dca47fae1..00000000000 --- a/spec/models/concerns/has_timelogs_report_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe HasTimelogsReport do - let_it_be(:user) { create(:user) } - - let(:group) { create(:group) } - let(:project) { create(:project, :public, group: group) } - let(:issue1) { create(:issue, project: project) } - let(:merge_request1) { create(:merge_request, source_project: project) } - - describe '#timelogs' do - let_it_be(:start_time) { 20.days.ago } - let_it_be(:end_time) { 8.days.ago } - - let!(:timelog1) { create_timelog(15.days.ago, issue: issue1) } - let!(:timelog2) { create_timelog(10.days.ago, merge_request: merge_request1) } - let!(:timelog3) { create_timelog(5.days.ago, issue: issue1) } - - before do - group.add_developer(user) - end - - it 'returns collection of timelogs between given times' do - expect(group.timelogs(start_time, end_time).to_a).to match_array([timelog1, timelog2]) - end - - it 'returns empty collection if times are not present' do - expect(group.timelogs(nil, nil)).to be_empty - end - - it 'returns empty collection if time range is invalid' do - expect(group.timelogs(end_time, start_time)).to be_empty - end - end - - describe '#user_can_access_group_timelogs?' do - it 'returns true if user can access group timelogs' do - group.add_developer(user) - - expect(group).to be_user_can_access_group_timelogs(user) - end - - it 'returns false if user has insufficient permissions' do - group.add_guest(user) - - expect(group).not_to be_user_can_access_group_timelogs(user) - end - end - - def create_timelog(time, issue: nil, merge_request: nil) - create(:timelog, issue: issue, merge_request: merge_request, user: user, spent_at: time) - end -end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index c87bbf24c30..a6a0e074589 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe User do specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) - .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot]) + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) diff --git a/spec/models/project_services/data_fields_spec.rb b/spec/models/concerns/integrations/has_data_fields_spec.rb index d3e6afe4978..54e0ac9c5a5 100644 --- a/spec/models/project_services/data_fields_spec.rb +++ b/spec/models/concerns/integrations/has_data_fields_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe DataFields do +RSpec.describe Integrations::HasDataFields do let(:url) { 'http://url.com' } let(:username) { 'username_one' } let(:properties) do @@ -100,7 +100,7 @@ RSpec.describe DataFields do context 'when service and data_fields are not persisted' do let(:service) do - JiraService.new + Integrations::Jira.new end describe 'data_fields_present?' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 14db9b530db..7b100b7a6f3 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -715,6 +715,12 @@ RSpec.describe Issuable do expect(issue.total_time_spent).to eq(1800) end + it 'stores the time change' do + spend_time(1800) + + expect(issue.time_change).to eq(1800) + end + it 'updates issues updated_at' do issue @@ -735,6 +741,12 @@ RSpec.describe Issuable do expect(issue.total_time_spent).to eq(900) end + it 'stores negative time change' do + spend_time(-900) + + expect(issue.time_change).to eq(-900) + end + context 'when time to subtract exceeds the total time spent' do it 'raise a validation error' do Timecop.travel(1.minute.from_now) do diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb index 753e2a8ee5e..6b25ed39efb 100644 --- a/spec/models/concerns/limitable_spec.rb +++ b/spec/models/concerns/limitable_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' +require 'active_model' RSpec.describe Limitable do let(:minimal_test_class) do @@ -35,6 +36,28 @@ RSpec.describe Limitable do instance.valid?(:create) end + + context 'with custom relation' do + before do + MinimalTestClass.limit_relation = :custom_relation + end + + it 'triggers custom limit_relation' do + instance = MinimalTestClass.new + + def instance.project + @project ||= Object.new + end + + limits = Object.new + custom_relation = Object.new + expect(instance).to receive(:custom_relation).and_return(custom_relation) + expect(instance.project).to receive(:actual_limits).and_return(limits) + expect(limits).to receive(:exceeded?).with(instance.class.name.demodulize.tableize, custom_relation).and_return(false) + + instance.valid?(:create) + end + end end context 'with global limit' do diff --git a/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb index 6f322a32a3b..671e51e3913 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb @@ -3,25 +3,99 @@ require 'spec_helper' RSpec.describe TokenAuthenticatableStrategies::EncryptionHelper do - let(:encrypted_token) { described_class.encrypt_token('my-value') } + let(:encrypted_token) { described_class.encrypt_token('my-value-my-value-my-value') } describe '.encrypt_token' do - it 'encrypts token' do - expect(encrypted_token).not_to eq('my-value') + context 'when dynamic_nonce feature flag is switched on' do + it 'adds nonce identifier on the beginning' do + expect(encrypted_token.first).to eq(described_class::DYNAMIC_NONCE_IDENTIFIER) + end + + it 'adds nonce at the end' do + nonce = encrypted_token.last(described_class::NONCE_SIZE) + + expect(nonce).to eq(::Digest::SHA256.hexdigest('my-value-my-value-my-value').bytes.take(described_class::NONCE_SIZE).pack('c*')) + end + + it 'encrypts token' do + expect(encrypted_token[1...-described_class::NONCE_SIZE]).not_to eq('my-value-my-value-my-value') + end + end + + context 'when dynamic_nonce feature flag is switched off' do + before do + stub_feature_flags(dynamic_nonce: false) + end + + it 'does not add nonce identifier on the beginning' do + expect(encrypted_token.first).not_to eq(described_class::DYNAMIC_NONCE_IDENTIFIER) + end + + it 'does not add nonce in the end' do + nonce = encrypted_token.last(described_class::NONCE_SIZE) + + expect(nonce).not_to eq(::Digest::SHA256.hexdigest('my-value-my-value-my-value').bytes.take(described_class::NONCE_SIZE).pack('c*')) + end + + it 'encrypts token with static iv' do + token = Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value-my-value-my-value') + + expect(encrypted_token).to eq(token) + end end end describe '.decrypt_token' do - it 'decrypts token with static iv' do - expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + context 'with feature flag switched off' do + before do + stub_feature_flags(dynamic_nonce: false) + end + + it 'decrypts token with static iv' do + encrypted_token = described_class.encrypt_token('my-value') + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + + it 'decrypts token if feature flag changed after encryption' do + encrypted_token = described_class.encrypt_token('my-value') + + expect(encrypted_token).not_to eq('my-value') + + stub_feature_flags(dynamic_nonce: true) + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + + it 'decrypts token with dynamic iv' do + iv = ::Digest::SHA256.hexdigest('my-value').bytes.take(described_class::NONCE_SIZE).pack('c*') + token = Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value', nonce: iv) + encrypted_token = "#{described_class::DYNAMIC_NONCE_IDENTIFIER}#{token}#{iv}" + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end end - it 'decrypts token with dynamic iv' do - iv = ::Digest::SHA256.hexdigest('my-value').bytes.take(described_class::NONCE_SIZE).pack('c*') - token = Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value', nonce: iv) - encrypted_token = "#{described_class::DYNAMIC_NONCE_IDENTIFIER}#{token}#{iv}" + context 'with feature flag switched on' do + before do + stub_feature_flags(dynamic_nonce: true) + end + + it 'decrypts token with dynamic iv' do + encrypted_token = described_class.encrypt_token('my-value') + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + + it 'decrypts token if feature flag changed after encryption' do + encrypted_token = described_class.encrypt_token('my-value') + + expect(encrypted_token).not_to eq('my-value') + + stub_feature_flags(dynamic_nonce: false) - expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end end end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index abaae5b059a..3232a559d0b 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ContainerRepository do before do group.parent = test_group - group.save + group.save! end it { is_expected.to contain_exactly(repository, another_repository) } @@ -331,6 +331,40 @@ RSpec.describe ContainerRepository do it { is_expected.to eq([]) } end + + context 'with read_container_registry_access_level disabled' do + before do + stub_feature_flags(read_container_registry_access_level: false) + end + + context 'in a group' do + let(:test_group) { group } + + it { is_expected.to contain_exactly(repository) } + end + + context 'with a subgroup' do + let(:test_group) { create(:group) } + let(:another_project) { create(:project, path: 'test', group: test_group) } + + let(:another_repository) do + create(:container_repository, name: 'my_image', project: another_project) + end + + before do + group.parent = test_group + group.save! + end + + it { is_expected.to contain_exactly(repository, another_repository) } + end + + context 'group without container_repositories' do + let(:test_group) { create(:group) } + + it { is_expected.to eq([]) } + end + end end describe '.search_by_name' do @@ -360,6 +394,17 @@ RSpec.describe ContainerRepository do it { is_expected.to contain_exactly(repository1, repository2, repository4) } end + describe '.with_stale_ongoing_cleanup' do + let_it_be(:repository1) { create(:container_repository, :cleanup_ongoing, expiration_policy_started_at: 1.day.ago) } + let_it_be(:repository2) { create(:container_repository, :cleanup_ongoing, expiration_policy_started_at: 25.minutes.ago) } + let_it_be(:repository3) { create(:container_repository, :cleanup_ongoing, expiration_policy_started_at: 1.week.ago) } + let_it_be(:repository4) { create(:container_repository, :cleanup_unscheduled, expiration_policy_started_at: 25.minutes.ago) } + + subject { described_class.with_stale_ongoing_cleanup(27.minutes.ago) } + + it { is_expected.to contain_exactly(repository1, repository3) } + end + describe '.waiting_for_cleanup' do let_it_be(:repository_cleanup_scheduled) { create(:container_repository, :cleanup_scheduled) } let_it_be(:repository_cleanup_unfinished) { create(:container_repository, :cleanup_unfinished) } @@ -423,6 +468,14 @@ RSpec.describe ContainerRepository do it { is_expected.to eq([repository]) } end + + context 'with repository cleanup started at after policy next run at' do + before do + repository.update!(expiration_policy_started_at: policy.next_run_at + 5.minutes) + end + + it { is_expected.to eq([]) } + end end describe '.with_unfinished_cleanup' do diff --git a/spec/models/cycle_analytics/project_level_stage_adapter_spec.rb b/spec/models/cycle_analytics/project_level_stage_adapter_spec.rb index 9bdee292938..ee13aae50dc 100644 --- a/spec/models/cycle_analytics/project_level_stage_adapter_spec.rb +++ b/spec/models/cycle_analytics/project_level_stage_adapter_spec.rb @@ -33,6 +33,6 @@ RSpec.describe CycleAnalytics::ProjectLevelStageAdapter, type: :model do end it 'presents the data as json' do - expect(subject.as_json).to include({ title: 'Review', value: 'about 1 hour' }) + expect(subject.as_json).to include({ title: 'Review', value: 1.hour }) end end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index c8917a7dd65..dfc37f9e661 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -248,68 +248,54 @@ RSpec.describe DeployToken do deploy_token.groups << group end - context 'and the allow_group_deploy_token feature flag is turned off' do - it 'is false' do - stub_feature_flags(allow_group_deploy_token: false) - - is_expected.to be_falsy - end + context 'and the passed-in project does not belong to any group' do + it { is_expected.to be_falsy } end - context 'and the allow_group_deploy_token feature flag is turned on' do - before do - stub_feature_flags(allow_group_deploy_token: true) - end + context 'and the passed-in project belongs to the token group' do + it 'is true' do + group.projects << project - context 'and the passed-in project does not belong to any group' do - it { is_expected.to be_falsy } + is_expected.to be_truthy end + end - context 'and the passed-in project belongs to the token group' do - it 'is true' do - group.projects << project + context 'and the passed-in project belongs to a subgroup' do + let(:child_group) { create(:group, parent_id: group.id) } + let(:grandchild_group) { create(:group, parent_id: child_group.id) } - is_expected.to be_truthy - end + before do + grandchild_group.projects << project end - context 'and the passed-in project belongs to a subgroup' do - let(:child_group) { create(:group, parent_id: group.id) } - let(:grandchild_group) { create(:group, parent_id: child_group.id) } - - before do - grandchild_group.projects << project - end - - context 'and the token group is an ancestor (grand-parent) of this group' do - it { is_expected.to be_truthy } - end + context 'and the token group is an ancestor (grand-parent) of this group' do + it { is_expected.to be_truthy } + end - context 'and the token group is not ancestor of this group' do - let(:child2_group) { create(:group, parent_id: group.id) } + context 'and the token group is not ancestor of this group' do + let(:child2_group) { create(:group, parent_id: group.id) } - it 'is false' do - deploy_token.groups = [child2_group] + it 'is false' do + deploy_token.groups = [child2_group] - is_expected.to be_falsey - end + is_expected.to be_falsey end end + end - context 'and the passed-in project does not belong to the token group' do - it { is_expected.to be_falsy } - end + context 'and the passed-in project does not belong to the token group' do + it { is_expected.to be_falsy } + end - context 'and the project belongs to a group that is parent of the token group' do - let(:super_group) { create(:group) } - let(:deploy_token) { create(:deploy_token, :group) } - let(:group) { create(:group, parent_id: super_group.id) } + context 'and the project belongs to a group that is parent of the token group' do + let(:super_group) { create(:group) } + let(:deploy_token) { create(:deploy_token, :group) } + let(:group) { create(:group, parent_id: super_group.id) } - it 'is false' do - super_group.projects << project + it 'is false' do + super_group.projects << project - is_expected.to be_falsey - end + is_expected.to be_falsey end end end diff --git a/spec/models/deployment_metrics_spec.rb b/spec/models/deployment_metrics_spec.rb index d0474777eb7..fadfc1b63ac 100644 --- a/spec/models/deployment_metrics_spec.rb +++ b/spec/models/deployment_metrics_spec.rb @@ -51,10 +51,10 @@ RSpec.describe DeploymentMetrics do context 'with a cluster Prometheus' do let(:deployment) { create(:deployment, :success, :on_cluster) } - let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } + let!(:prometheus) { create(:clusters_integrations_prometheus, cluster: deployment.cluster) } before do - expect(deployment.cluster.application_prometheus).to receive(:configured?).and_return(true) + expect(deployment.cluster.integration_prometheus).to receive(:configured?).and_return(true) end it { is_expected.to be_truthy } @@ -118,7 +118,7 @@ RSpec.describe DeploymentMetrics do expect(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) end - it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.finished_at.to_i })) } end end end diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 26b311fe629..2a2663149d0 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -21,9 +21,9 @@ RSpec.describe DiffDiscussion do describe '#merge_request_version_params' do let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } - let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } - let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create!(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'when the discussion is active' do it 'returns an empty hash, which will end up showing the latest version' do diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 215c733f26b..2731eadecc0 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -167,7 +167,7 @@ RSpec.describe DiffNote do end it 'creates a diff note file' do - subject.save + subject.save! expect(subject.note_diff_file).to be_present end end @@ -188,7 +188,7 @@ RSpec.describe DiffNote do end it 'raises an error' do - expect { subject.save }.to raise_error(::DiffNote::NoteDiffFileCreationError, + expect { subject.save! }.to raise_error(::DiffNote::NoteDiffFileCreationError, "Failed to find diff line for: #{diff_file.file_path}, "\ "old_line: #{position.old_line}"\ ", new_line: #{position.new_line}") @@ -201,7 +201,7 @@ RSpec.describe DiffNote do end it 'creates a diff note file' do - subject.save + subject.save! expect(subject.reload.note_diff_file).to be_present end end @@ -544,7 +544,7 @@ RSpec.describe DiffNote do it "does not update the position" do expect(subject).not_to receive(:update_position) - subject.save + subject.save! end end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index cd0938682db..2b09ee5c190 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Email do let(:user) { create(:user) } it 'synchronizes the gpg keys when the email is updated' do - email = user.emails.create(email: 'new@email.com') + email = user.emails.create!(email: 'new@email.com') expect(user).to receive(:update_invalid_gpg_signatures) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 759bb080172..ff4c8ae950d 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1157,51 +1157,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#prometheus_status' do - context 'when a cluster is present' do - context 'when a deployment platform is present' do - let(:cluster) { create(:cluster, :provided_by_user, :project) } - let(:environment) { create(:environment, project: cluster.project) } - - subject { environment.prometheus_status } - - context 'when the prometheus application status is :updating' do - let!(:prometheus) { create(:clusters_applications_prometheus, :updating, cluster: cluster) } - - it { is_expected.to eq(:updating) } - end - - context 'when the prometheus application state is :updated' do - let!(:prometheus) { create(:clusters_applications_prometheus, :updated, cluster: cluster) } - - it { is_expected.to eq(:updated) } - end - - context 'when the prometheus application is not installed' do - it { is_expected.to be_nil } - end - end - - context 'when a deployment platform is not present' do - let(:cluster) { create(:cluster, :project) } - let(:environment) { create(:environment, project: cluster.project) } - - subject { environment.prometheus_status } - - it { is_expected.to be_nil } - end - end - - context 'when a cluster is not present' do - let(:project) { create(:project, :stubbed_repository) } - let(:environment) { create(:environment, project: project) } - - subject { environment.prometheus_status } - - it { is_expected.to be_nil } - end - end - describe '#additional_metrics' do let(:project) { create(:prometheus_project) } let(:metric_params) { [] } @@ -1434,30 +1389,14 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do let!(:cluster) { create(:cluster, :project, :provided_by_user, projects: [project]) } let!(:deployment) { create(:deployment, :success, environment: environment, project: project, cluster: cluster) } - context 'when app does not exist' do + context 'when integration does not exist' do it 'returns false' do expect(environment.elastic_stack_available?).to be(false) end end - context 'when app exists' do - let!(:application) { create(:clusters_applications_elastic_stack, cluster: cluster) } - - it 'returns false' do - expect(environment.elastic_stack_available?).to be(false) - end - end - - context 'when app is installed' do - let!(:application) { create(:clusters_applications_elastic_stack, :installed, cluster: cluster) } - - it 'returns true' do - expect(environment.elastic_stack_available?).to be(true) - end - end - - context 'when app is updated' do - let!(:application) { create(:clusters_applications_elastic_stack, :updated, cluster: cluster) } + context 'when integration is enabled' do + let!(:integration) { create(:clusters_integrations_elastic_stack, cluster: cluster) } it 'returns true' do expect(environment.elastic_stack_available?).to be(true) diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 09a73a4cdcb..1b9b38a0932 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -245,6 +245,17 @@ RSpec.describe EnvironmentStatus do end end + context 'when there is a deployment in a child pipeline' do + let!(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_build) { create(:ci_build, :with_deployment, :start_review_app, pipeline: child_pipeline) } + let(:child_environment) { child_build.deployment.environment } + + it 'returns both parent and child entries' do + expect(subject.count).to eq(2) + expect(subject.map(&:id)).to contain_exactly(environment.id, child_environment.id) + end + end + context 'when environment is stopped' do before do environment.stop! diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 1517f426fa3..7f0d1e69924 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -79,7 +79,7 @@ RSpec.describe Experiment do context 'when an experiment with the provided name does not exist' do it 'creates a new experiment record' do allow_next(described_class, name: :experiment_key) - .to receive(:record_group_and_variant!).with(group, variant) + .to receive(:record_subject_and_variant!).with(group, variant) expect { add_group }.to change(described_class, :count).by(1) end @@ -235,23 +235,23 @@ RSpec.describe Experiment do end end - describe '#record_group_and_variant!' do - let_it_be(:group) { create(:group) } + describe '#record_subject_and_variant!' do + let_it_be(:subject_to_record) { create(:group) } let_it_be(:variant) { :control } let_it_be(:experiment) { create(:experiment) } - subject(:record_group_and_variant!) { experiment.record_group_and_variant!(group, variant) } + subject(:record_subject_and_variant!) { experiment.record_subject_and_variant!(subject_to_record, variant) } - context 'when no existing experiment_subject record exists for the given group' do + context 'when no existing experiment_subject record exists for the given subject' do it 'creates an experiment_subject record' do - expect { record_group_and_variant! }.to change(ExperimentSubject, :count).by(1) + expect { record_subject_and_variant! }.to change(ExperimentSubject, :count).by(1) expect(ExperimentSubject.last.variant).to eq(variant.to_s) end end - context 'when an existing experiment_subject exists for the given group' do + context 'when an existing experiment_subject exists for the given subject' do let_it_be(:experiment_subject) do - create(:experiment_subject, experiment: experiment, group: group, user: nil, variant: :experimental) + create(:experiment_subject, experiment: experiment, namespace: subject_to_record, user: nil, variant: :experimental) end context 'when it belongs to the same variant' do @@ -266,7 +266,55 @@ RSpec.describe Experiment do context 'but it belonged to a different variant' do it 'updates the variant value' do - expect { record_group_and_variant! }.to change { experiment_subject.reload.variant }.to('control') + expect { record_subject_and_variant! }.to change { experiment_subject.reload.variant }.to('control') + end + end + end + + describe 'providing a subject to record' do + context 'when given a group as subject' do + it 'saves the namespace as the experiment subject' do + expect(record_subject_and_variant!.namespace).to eq(subject_to_record) + end + end + + context 'when given a users namespace as subject' do + let_it_be(:subject_to_record) { build(:namespace) } + + it 'saves the namespace as the experiment_subject' do + expect(record_subject_and_variant!.namespace).to eq(subject_to_record) + end + end + + context 'when given a user as subject' do + let_it_be(:subject_to_record) { build(:user) } + + it 'saves the user as experiment_subject user' do + expect(record_subject_and_variant!.user).to eq(subject_to_record) + end + end + + context 'when given a project as subject' do + let_it_be(:subject_to_record) { build(:project) } + + it 'saves the project as experiment_subject user' do + expect(record_subject_and_variant!.project).to eq(subject_to_record) + end + end + + context 'when given no subject' do + let_it_be(:subject_to_record) { nil } + + it 'raises an error' do + expect { record_subject_and_variant! }.to raise_error('Incompatible subject provided!') + end + end + + context 'when given an incompatible subject' do + let_it_be(:subject_to_record) { build(:ci_build) } + + it 'raises an error' do + expect { record_subject_and_variant! }.to raise_error('Incompatible subject provided!') end end end diff --git a/spec/models/experiment_subject_spec.rb b/spec/models/experiment_subject_spec.rb index 4850814c5f5..d86dc3cbf65 100644 --- a/spec/models/experiment_subject_spec.rb +++ b/spec/models/experiment_subject_spec.rb @@ -6,7 +6,7 @@ RSpec.describe ExperimentSubject, type: :model do describe 'associations' do it { is_expected.to belong_to(:experiment) } it { is_expected.to belong_to(:user) } - it { is_expected.to belong_to(:group) } + it { is_expected.to belong_to(:namespace) } it { is_expected.to belong_to(:project) } end @@ -14,8 +14,8 @@ RSpec.describe ExperimentSubject, type: :model do it { is_expected.to validate_presence_of(:experiment) } describe 'must_have_one_subject_present' do - let(:experiment_subject) { build(:experiment_subject, user: nil, group: nil, project: nil) } - let(:error_message) { 'Must have exactly one of User, Group, or Project.' } + let(:experiment_subject) { build(:experiment_subject, user: nil, namespace: nil, project: nil) } + let(:error_message) { 'Must have exactly one of User, Namespace, or Project.' } it 'fails when no subject is present' do expect(experiment_subject).not_to be_valid @@ -27,8 +27,8 @@ RSpec.describe ExperimentSubject, type: :model do expect(experiment_subject).to be_valid end - it 'passes when group subject is present' do - experiment_subject.group = build(:group) + it 'passes when namespace subject is present' do + experiment_subject.namespace = build(:group) expect(experiment_subject).to be_valid end @@ -40,7 +40,7 @@ RSpec.describe ExperimentSubject, type: :model do it 'fails when more than one subject is present', :aggregate_failures do # two subjects experiment_subject.user = build(:user) - experiment_subject.group = build(:group) + experiment_subject.namespace = build(:group) expect(experiment_subject).not_to be_valid expect(experiment_subject.errors[:base]).to include(error_message) @@ -51,4 +51,22 @@ RSpec.describe ExperimentSubject, type: :model do end end end + + describe '.valid_subject?' do + subject(:valid_subject?) { described_class.valid_subject?(subject_class.new) } + + context 'when passing a Group, Namespace, User or Project' do + [Group, Namespace, User, Project].each do |subject_class| + let(:subject_class) { subject_class } + + it { is_expected.to be(true) } + end + end + + context 'when passing another object' do + let(:subject_class) { Issue } + + it { is_expected.to be(false) } + end + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5cc5c4d86d6..8f4bc43c38a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -517,6 +517,10 @@ RSpec.describe Group do it { expect(group.self_and_descendants.to_sql).not_to include 'traversal_ids @>' } end + describe '#self_and_descendant_ids' do + it { expect(group.self_and_descendant_ids.to_sql).not_to include 'traversal_ids @>' } + end + describe '#descendants' do it { expect(group.descendants.to_sql).not_to include 'traversal_ids @>' } end @@ -533,6 +537,10 @@ RSpec.describe Group do it { expect(group.self_and_descendants.to_sql).to include 'traversal_ids @>' } end + describe '#self_and_descendant_ids' do + it { expect(group.self_and_descendant_ids.to_sql).to include 'traversal_ids @>' } + end + describe '#descendants' do it { expect(group.descendants.to_sql).to include 'traversal_ids @>' } end @@ -1093,167 +1101,151 @@ RSpec.describe Group do it { expect(subject.parent).to be_kind_of(described_class) } end - context "with member access" do + describe '#max_member_access_for_user' do let_it_be(:group_user) { create(:user) } - describe '#max_member_access_for_user' do - context 'with user in the group' do - before do - group.add_owner(group_user) - end - - it 'returns correct access level' do - expect(group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::OWNER) - end + context 'with user in the group' do + before do + group.add_owner(group_user) end - context 'when user is nil' do - it 'returns NO_ACCESS' do - expect(group.max_member_access_for_user(nil)).to eq(Gitlab::Access::NO_ACCESS) - end + it 'returns correct access level' do + expect(group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::OWNER) end + end - context 'evaluating admin access level' do - let_it_be(:admin) { create(:admin) } - - context 'when admin mode is enabled', :enable_admin_mode do - it 'returns OWNER by default' do - expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) - end - end + context 'when user is nil' do + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(nil)).to eq(Gitlab::Access::NO_ACCESS) + end + end - context 'when admin mode is disabled' do - it 'returns NO_ACCESS' do - expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::NO_ACCESS) - end - end + context 'evaluating admin access level' do + let_it_be(:admin) { create(:admin) } - it 'returns NO_ACCESS when only concrete membership should be considered' do - expect(group.max_member_access_for_user(admin, only_concrete_membership: true)) - .to eq(Gitlab::Access::NO_ACCESS) + context 'when admin mode is enabled', :enable_admin_mode do + it 'returns OWNER by default' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) end end - context 'when max_access_for_group is set' do - let(:max_member_access) { 111 } - - before do - group_user.max_access_for_group[group.id] = max_member_access + context 'when admin mode is disabled' do + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::NO_ACCESS) end + end - it 'uses the cached value' do - expect(group.max_member_access_for_user(group_user)).to eq(max_member_access) - end + it 'returns NO_ACCESS when only concrete membership should be considered' do + expect(group.max_member_access_for_user(admin, only_concrete_membership: true)) + .to eq(Gitlab::Access::NO_ACCESS) end end - describe '#max_member_access' do - context 'group shared with another group' do - let_it_be(:parent_group_user) { create(:user) } - let_it_be(:child_group_user) { create(:user) } + context 'group shared with another group' do + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:child_group_user) { create(:user) } - let_it_be(:group_parent) { create(:group, :private) } - let_it_be(:group) { create(:group, :private, parent: group_parent) } - let_it_be(:group_child) { create(:group, :private, parent: group) } + let_it_be(:group_parent) { create(:group, :private) } + let_it_be(:group) { create(:group, :private, parent: group_parent) } + let_it_be(:group_child) { create(:group, :private, parent: group) } - let_it_be(:shared_group_parent) { create(:group, :private) } - let_it_be(:shared_group) { create(:group, :private, parent: shared_group_parent) } - let_it_be(:shared_group_child) { create(:group, :private, parent: shared_group) } + let_it_be(:shared_group_parent) { create(:group, :private) } + let_it_be(:shared_group) { create(:group, :private, parent: shared_group_parent) } + let_it_be(:shared_group_child) { create(:group, :private, parent: shared_group) } - before do - group_parent.add_owner(parent_group_user) - group.add_owner(group_user) - group_child.add_owner(child_group_user) + before do + group_parent.add_owner(parent_group_user) + group.add_owner(group_user) + group_child.add_owner(child_group_user) - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group, - group_access: GroupMember::DEVELOPER }) - end + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) + end - context 'with user in the group' do - it 'returns correct access level' do - expect(shared_group_parent.max_member_access(group_user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(group_user)).to eq(Gitlab::Access::DEVELOPER) - expect(shared_group_child.max_member_access(group_user)).to eq(Gitlab::Access::DEVELOPER) - end + context 'with user in the group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::DEVELOPER) + expect(shared_group_child.max_member_access_for_user(group_user)).to eq(Gitlab::Access::DEVELOPER) + end - context 'with lower group access level than max access level for share' do - let(:user) { create(:user) } + context 'with lower group access level than max access level for share' do + let(:user) { create(:user) } - it 'returns correct access level' do - group.add_reporter(user) + it 'returns correct access level' do + group.add_reporter(user) - expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::REPORTER) - expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::REPORTER) - end + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER) end end + end - context 'with user in the parent group' do - it 'returns correct access level' do - expect(shared_group_parent.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) - end + context 'with user in the parent group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) end + end - context 'with user in the child group' do - it 'returns correct access level' do - expect(shared_group_parent.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) - end + context 'with user in the child group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) end + end - context 'unrelated project owner' do - let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } - let!(:group) { create(:group, id: common_id) } - let!(:unrelated_project) { create(:project, id: common_id) } - let(:user) { unrelated_project.owner } + context 'unrelated project owner' do + let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } + let!(:group) { create(:group, id: common_id) } + let!(:unrelated_project) { create(:project, id: common_id) } + let(:user) { unrelated_project.owner } - it 'returns correct access level' do - expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - end + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) end + end - context 'user without accepted access request' do - let!(:user) { create(:user) } + context 'user without accepted access request' do + let!(:user) { create(:user) } - before do - create(:group_member, :developer, :access_request, user: user, group: group) - end + before do + create(:group_member, :developer, :access_request, user: user, group: group) + end - it 'returns correct access level' do - expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) - end + it 'returns correct access level' do + expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) end end + end - context 'multiple groups shared with group' do - let(:user) { create(:user) } - let(:group) { create(:group, :private) } - let(:shared_group_parent) { create(:group, :private) } - let(:shared_group) { create(:group, :private, parent: shared_group_parent) } + context 'multiple groups shared with group' do + let(:user) { create(:user) } + let(:group) { create(:group, :private) } + let(:shared_group_parent) { create(:group, :private) } + let(:shared_group) { create(:group, :private, parent: shared_group_parent) } - before do - group.add_owner(user) + before do + group.add_owner(user) - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group, - group_access: GroupMember::DEVELOPER }) - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group_parent, - group_access: GroupMember::MAINTAINER }) - end + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group_parent, + group_access: GroupMember::MAINTAINER }) + end - it 'returns correct access level' do - expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::MAINTAINER) - end + it 'returns correct access level' do + expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::MAINTAINER) end end end @@ -2248,14 +2240,16 @@ RSpec.describe Group do let_it_be(:group) { create(:group, :public) } it 'returns a maximum of ten owners of the group in recent_sign_in descending order' do - users = create_list(:user, 12, :with_sign_ins) + limit = 2 + stub_const("Member::ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT", limit) + users = create_list(:user, limit + 1, :with_sign_ins) active_owners = users.map do |user| create(:group_member, :owner, group: group, user: user) end active_owners_in_recent_sign_in_desc_order = group.members_and_requesters .id_in(active_owners) - .order_recent_sign_in.limit(10) + .order_recent_sign_in.limit(limit) expect(group.access_request_approvers_to_be_notified).to eq(active_owners_in_recent_sign_in_desc_order) end @@ -2619,4 +2613,66 @@ RSpec.describe Group do expect(group.activity_path).to eq(expected_path) end end + + context 'with export' do + let(:group) { create(:group, :with_export) } + + it '#export_file_exists? returns true' do + expect(group.export_file_exists?).to be true + end + + it '#export_archive_exists? returns true' do + expect(group.export_archive_exists?).to be true + end + end + + describe '#open_issues_count', :aggregate_failures do + let(:group) { build(:group) } + + it 'provides the issue count' do + expect(group.open_issues_count).to eq 0 + end + + it 'invokes the count service with current_user' do + user = build(:user) + count_service = instance_double(Groups::OpenIssuesCountService) + expect(Groups::OpenIssuesCountService).to receive(:new).with(group, user).and_return(count_service) + expect(count_service).to receive(:count) + + group.open_issues_count(user) + end + + it 'invokes the count service with no current_user' do + count_service = instance_double(Groups::OpenIssuesCountService) + expect(Groups::OpenIssuesCountService).to receive(:new).with(group, nil).and_return(count_service) + expect(count_service).to receive(:count) + + group.open_issues_count + end + end + + describe '#open_merge_requests_count', :aggregate_failures do + let(:group) { build(:group) } + + it 'provides the merge request count' do + expect(group.open_merge_requests_count).to eq 0 + end + + it 'invokes the count service with current_user' do + user = build(:user) + count_service = instance_double(Groups::MergeRequestsCountService) + expect(Groups::MergeRequestsCountService).to receive(:new).with(group, user).and_return(count_service) + expect(count_service).to receive(:count) + + group.open_merge_requests_count(user) + end + + it 'invokes the count service with no current_user' do + count_service = instance_double(Groups::MergeRequestsCountService) + expect(Groups::MergeRequestsCountService).to receive(:new).with(group, nil).and_return(count_service) + expect(count_service).to receive(:count) + + group.open_merge_requests_count + end + end end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 88149465232..d811f67d16b 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -39,4 +39,15 @@ RSpec.describe ProjectHook do expect(hook.rate_limit).to be(100) end end + + describe '#application_context' do + let_it_be(:hook) { build(:project_hook) } + + it 'includes the type and project' do + expect(hook.application_context).to include( + related_class: 'ProjectHook', + project: hook.project + ) + end + end end diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 651716c3280..4ce2e729d89 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -30,4 +30,14 @@ RSpec.describe ServiceHook do expect(hook.rate_limit).to be_nil end end + + describe '#application_context' do + let(:hook) { build(:service_hook) } + + it 'includes the type' do + expect(hook.application_context).to eq( + related_class: 'ServiceHook' + ) + end + end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index a72034f1ac5..a99263078b3 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -177,4 +177,14 @@ RSpec.describe SystemHook do expect(hook.rate_limit).to be_nil end end + + describe '#application_context' do + let(:hook) { build(:system_hook) } + + it 'includes the type' do + expect(hook.application_context).to eq( + related_class: 'SystemHook' + ) + end + end end diff --git a/spec/models/hooks/web_hook_log_archived_spec.rb b/spec/models/hooks/web_hook_log_archived_spec.rb deleted file mode 100644 index ac726dbaf4f..00000000000 --- a/spec/models/hooks/web_hook_log_archived_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe WebHookLogArchived do - let(:source_table) { WebHookLog } - let(:destination_table) { described_class } - - it 'has the same columns as the source table' do - column_names_from_source_table = column_names(source_table) - column_names_from_destination_table = column_names(destination_table) - - expect(column_names_from_destination_table).to match_array(column_names_from_source_table) - end - - it 'has the same null constraints as the source table' do - constraints_from_source_table = null_constraints(source_table) - constraints_from_destination_table = null_constraints(destination_table) - - expect(constraints_from_destination_table.to_a).to match_array(constraints_from_source_table.to_a) - end - - it 'inserts the same record as the one in the source table', :aggregate_failures do - expect { create(:web_hook_log) }.to change { destination_table.count }.by(1) - - event_from_source_table = source_table.connection.select_one( - "SELECT * FROM #{source_table.table_name} ORDER BY created_at desc LIMIT 1" - ) - event_from_destination_table = destination_table.connection.select_one( - "SELECT * FROM #{destination_table.table_name} ORDER BY created_at desc LIMIT 1" - ) - - expect(event_from_destination_table).to eq(event_from_source_table) - end - - def column_names(table) - table.connection.select_all(<<~SQL) - SELECT c.column_name - FROM information_schema.columns c - WHERE c.table_name = '#{table.table_name}' - SQL - end - - def null_constraints(table) - table.connection.select_all(<<~SQL) - SELECT c.column_name, c.is_nullable - FROM information_schema.columns c - WHERE c.table_name = '#{table.table_name}' - AND c.column_name != 'created_at' - SQL - end -end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index b528dbedd2c..1761b537dc0 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -268,11 +268,58 @@ RSpec.describe WebHook do end describe '#enable!' do - it 'makes a hook executable' do + it 'makes a hook executable if it was marked as failed' do hook.recent_failures = 1000 expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) end + + it 'makes a hook executable if it is currently backed off' do + hook.disabled_until = 1.hour.from_now + + expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) + end + + it 'does not update hooks unless necessary' do + expect(hook).not_to receive(:update!) + + hook.enable! + end + + it 'is idempotent on executable hooks' do + expect(hook).not_to receive(:update!) + + expect { hook.enable! }.not_to change(hook, :executable?) + end + end + + describe 'backoff!' do + it 'sets disabled_until to the next backoff' do + expect { hook.backoff! }.to change(hook, :disabled_until).to(hook.next_backoff.from_now) + end + + it 'increments the backoff count' do + expect { hook.backoff! }.to change(hook, :backoff_count).by(1) + end + + it 'does not let the backoff count exceed the maximum failure count' do + hook.backoff_count = described_class::MAX_FAILURES + + expect { hook.backoff! }.not_to change(hook, :backoff_count) + end + end + + describe 'failed!' do + it 'increments the failure count' do + expect { hook.failed! }.to change(hook, :recent_failures).by(1) + end + + it 'does not allow the failure count to exceed the maximum value' do + hook.recent_failures = described_class::MAX_FAILURES + expect(hook).not_to receive(:update!) + + expect { hook.failed! }.not_to change(hook, :recent_failures) + end end describe '#disable!' do diff --git a/spec/models/import_export_upload_spec.rb b/spec/models/import_export_upload_spec.rb index 46a611852ab..e13f504b82a 100644 --- a/spec/models/import_export_upload_spec.rb +++ b/spec/models/import_export_upload_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe ImportExportUpload do - subject { described_class.new(project: create(:project)) } + let(:project) { create(:project) } + + subject { described_class.new(project: project) } shared_examples 'stores the Import/Export file' do |method| it 'stores the import file' do @@ -24,4 +26,99 @@ RSpec.describe ImportExportUpload do context 'export' do it_behaves_like 'stores the Import/Export file', :export_file end + + describe 'scopes' do + let_it_be(:upload1) { create(:import_export_upload, export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) } + let_it_be(:upload2) { create(:import_export_upload) } + let_it_be(:upload3) { create(:import_export_upload, export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz'), updated_at: 25.hours.ago) } + let_it_be(:upload4) { create(:import_export_upload, updated_at: 2.days.ago) } + + describe '.with_export_file' do + it 'returns uploads with export file' do + expect(described_class.with_export_file).to contain_exactly(upload1, upload3) + end + end + + describe '.updated_before' do + it 'returns uploads for a specified date' do + expect(described_class.updated_before(24.hours.ago)).to contain_exactly(upload3, upload4) + end + end + end + + context 'ActiveRecord callbacks' do + let(:after_save_callbacks) { described_class._save_callbacks.select { |cb| cb.kind == :after } } + let(:after_commit_callbacks) { described_class._commit_callbacks.select { |cb| cb.kind == :after } } + + def find_callback(callbacks, key) + callbacks.find { |cb| cb.instance_variable_get(:@key) == key } + end + + it 'export file is stored in after_commit callback' do + expect(find_callback(after_commit_callbacks, :store_export_file!)).to be_present + expect(find_callback(after_save_callbacks, :store_export_file!)).to be_nil + end + + it 'import file is stored in after_save callback' do + expect(find_callback(after_save_callbacks, :store_import_file!)).to be_present + expect(find_callback(after_commit_callbacks, :store_import_file!)).to be_nil + end + end + + describe 'export file' do + it '#export_file_exists? returns false' do + expect(subject.export_file_exists?).to be false + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be false + end + + context 'with export' do + let(:project_with_export) { create(:project, :with_export) } + + subject { described_class.with_export_file.find_by(project: project_with_export) } + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be true + end + + context 'when object file does not exist' do + before do + subject.export_file.file.delete + end + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(subject.export_archive_exists?).to be false + end + end + + context 'when checking object existence raises a error' do + let(:exception) { Excon::Error::Forbidden.new('not allowed') } + + before do + file = double + allow(file).to receive(:exists?).and_raise(exception) + allow(subject).to receive(:carrierwave_export_file).and_return(file) + end + + it '#export_file_exists? returns true' do + expect(subject.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception) + expect(subject.export_archive_exists?).to be false + end + end + end + end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 77b3778122a..d4ea3e5d08a 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -18,6 +18,7 @@ RSpec.describe Integration do describe 'validations' do it { is_expected.to validate_presence_of(:type) } + it { is_expected.to validate_exclusion_of(:type).in_array(described_class::BASE_CLASSES) } where(:project_id, :group_id, :template, :instance, :valid) do 1 | nil | false | false | true @@ -159,7 +160,7 @@ RSpec.describe Integration do context 'when instance-level service' do Integration.available_services_types.each do |service_type| let(:service) do - service_type.constantize.new(instance: true) + described_class.send(:integration_type_to_model, service_type).new(instance: true) end it { is_expected.to be_falsey } @@ -169,7 +170,7 @@ RSpec.describe Integration do context 'when group-level service' do Integration.available_services_types.each do |service_type| let(:service) do - service_type.constantize.new(group_id: group.id) + described_class.send(:integration_type_to_model, service_type).new(group_id: group.id) end it { is_expected.to be_falsey } @@ -446,7 +447,7 @@ RSpec.describe Integration do describe "for pushover service" do let!(:service_template) do - PushoverService.create!( + Integrations::Pushover.create!( template: true, properties: { device: 'MyDevice', @@ -667,16 +668,16 @@ RSpec.describe Integration do end end - describe '.service_name_to_model' do + describe '.integration_name_to_model' do it 'returns the model for the given service name', :aggregate_failures do - expect(described_class.service_name_to_model('asana')).to eq(Integrations::Asana) + expect(described_class.integration_name_to_model('asana')).to eq(Integrations::Asana) # TODO We can remove this test when all models have been namespaced: # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60968#note_570994955 - expect(described_class.service_name_to_model('youtrack')).to eq(YoutrackService) + expect(described_class.integration_name_to_model('prometheus')).to eq(PrometheusService) end it 'raises an error if service name is invalid' do - expect { described_class.service_name_to_model('foo') }.to raise_exception(NameError, /uninitialized constant FooService/) + expect { described_class.integration_name_to_model('foo') }.to raise_exception(NameError, /uninitialized constant FooService/) end end @@ -802,7 +803,7 @@ RSpec.describe Integration do describe 'initialize service with no properties' do let(:service) do - BugzillaService.create!( + Integrations::Bugzilla.create!( project: project, project_url: 'http://gitlab.example.com' ) @@ -896,20 +897,6 @@ RSpec.describe Integration do end end - describe '#external_wiki?' do - where(:type, :active, :result) do - 'ExternalWikiService' | true | true - 'ExternalWikiService' | false | false - 'SlackService' | true | false - end - - with_them do - it 'returns the right result' do - expect(build(:service, type: type, active: active).external_wiki?).to eq(result) - end - end - end - describe '.available_services_names' do it 'calls the right methods' do expect(described_class).to receive(:services_names).and_call_original diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb index bf9033416e9..e5972bce95d 100644 --- a/spec/models/integrations/assembla_spec.rb +++ b/spec/models/integrations/assembla_spec.rb @@ -15,8 +15,8 @@ RSpec.describe Integrations::Assembla do let(:project) { create(:project, :repository) } before do - @assembla_service = described_class.new - allow(@assembla_service).to receive_messages( + @assembla_integration = described_class.new + allow(@assembla_integration).to receive_messages( project_id: project.id, project: project, service_hook: true, @@ -29,7 +29,7 @@ RSpec.describe Integrations::Assembla do end it "calls Assembla API" do - @assembla_service.execute(@sample_data) + @assembla_integration.execute(@sample_data) expect(WebMock).to have_requested(:post, stubbed_hostname(@api_url)).with( body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/ ).once diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 0ba1595bbd8..39966f7978d 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -82,45 +82,45 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do describe 'before_update :reset_password' do context 'when a password was previously set' do it 'resets password if url changed' do - bamboo_service = service + bamboo_integration = service - bamboo_service.bamboo_url = 'http://gitlab1.com' - bamboo_service.save! + bamboo_integration.bamboo_url = 'http://gitlab1.com' + bamboo_integration.save! - expect(bamboo_service.password).to be_nil + expect(bamboo_integration.password).to be_nil end it 'does not reset password if username changed' do - bamboo_service = service + bamboo_integration = service - bamboo_service.username = 'some_name' - bamboo_service.save! + bamboo_integration.username = 'some_name' + bamboo_integration.save! - expect(bamboo_service.password).to eq('password') + expect(bamboo_integration.password).to eq('password') end it "does not reset password if new url is set together with password, even if it's the same password" do - bamboo_service = service + bamboo_integration = service - bamboo_service.bamboo_url = 'http://gitlab_edited.com' - bamboo_service.password = 'password' - bamboo_service.save! + bamboo_integration.bamboo_url = 'http://gitlab_edited.com' + bamboo_integration.password = 'password' + bamboo_integration.save! - expect(bamboo_service.password).to eq('password') - expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com') + expect(bamboo_integration.password).to eq('password') + expect(bamboo_integration.bamboo_url).to eq('http://gitlab_edited.com') end end it 'saves password if new url is set together with password when no password was previously set' do - bamboo_service = service - bamboo_service.password = nil + bamboo_integration = service + bamboo_integration.password = nil - bamboo_service.bamboo_url = 'http://gitlab_edited.com' - bamboo_service.password = 'password' - bamboo_service.save! + bamboo_integration.bamboo_url = 'http://gitlab_edited.com' + bamboo_integration.password = 'password' + bamboo_integration.save! - expect(bamboo_service.password).to eq('password') - expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com') + expect(bamboo_integration.password).to eq('password') + expect(bamboo_integration.bamboo_url).to eq('http://gitlab_edited.com') end end end diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 62f97873a06..656eaa3bbdd 100644 --- a/spec/models/project_services/chat_notification_service_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ChatNotificationService do +RSpec.describe Integrations::BaseChatNotification do describe 'Associations' do before do allow(subject).to receive(:activated?).and_return(true) @@ -89,12 +89,6 @@ RSpec.describe ChatNotificationService do let(:data) { Gitlab::DataBuilder::Note.build(note, user) } - it 'notifies the chat service' do - expect(chat_service).to receive(:notify).with(any_args) - - chat_service.execute(data) - end - shared_examples 'notifies the chat service' do specify do expect(chat_service).to receive(:notify).with(any_args) @@ -111,6 +105,26 @@ RSpec.describe ChatNotificationService do end end + it_behaves_like 'notifies the chat service' + + context 'with label filter' do + subject(:chat_service) { described_class.new(labels_to_be_notified: '~Bug') } + + it_behaves_like 'notifies the chat service' + + context 'MergeRequest events' do + let(:data) { create(:merge_request, labels: [label]).to_hook_data(user) } + + it_behaves_like 'notifies the chat service' + end + + context 'Issue events' do + let(:data) { issue.to_hook_data(user) } + + it_behaves_like 'notifies the chat service' + end + end + context 'when labels_to_be_notified_behavior is not defined' do subject(:chat_service) { described_class.new(labels_to_be_notified: label_filter) } diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/integrations/base_issue_tracker_spec.rb index 5b12c7330b8..0f1bc39929a 100644 --- a/spec/models/project_services/issue_tracker_service_spec.rb +++ b/spec/models/integrations/base_issue_tracker_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' -RSpec.describe IssueTrackerService do +RSpec.describe Integrations::BaseIssueTracker do describe 'Validations' do let(:project) { create :project } describe 'only one issue tracker per project' do - let(:service) { RedmineService.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } + let(:service) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } before do - create(:custom_issue_tracker_service, project: project) + create(:custom_issue_tracker_integration, project: project) end context 'when service is changed manually by user' do diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/integrations/bugzilla_spec.rb index 560c7c3ee83..e75fa8dd4d4 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/integrations/bugzilla_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BugzillaService do +RSpec.describe Integrations::Bugzilla do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/integrations/buildkite_spec.rb index f6bf1551bf0..7dc81da7003 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BuildkiteService, :use_clean_rails_memory_store_caching do +RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers include StubRequests diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index b23edf03e8a..d68f8e0bd4e 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -33,8 +33,8 @@ RSpec.describe Integrations::Campfire do let(:project) { create(:project, :repository) } before do - @campfire_service = described_class.new - allow(@campfire_service).to receive_messages( + @campfire_integration = described_class.new + allow(@campfire_integration).to receive_messages( project_id: project.id, project: project, service_hook: true, @@ -62,7 +62,7 @@ RSpec.describe Integrations::Campfire do speak_url = 'https://project-name.campfirenow.com/room/123/speak.json' stub_full_request(speak_url, method: :post).with(basic_auth: @auth) - @campfire_service.execute(@sample_data) + @campfire_integration.execute(@sample_data) expect(WebMock).to have_requested(:get, stubbed_hostname(@rooms_url)).once expect(WebMock).to have_requested(:post, stubbed_hostname(speak_url)) @@ -78,7 +78,7 @@ RSpec.describe Integrations::Campfire do headers: @headers ) - @campfire_service.execute(@sample_data) + @campfire_integration.execute(@sample_data) expect(WebMock).to have_requested(:get, 'https://8.8.8.9/rooms.json').once expect(WebMock).not_to have_requested(:post, '*/room/.*/speak.json') diff --git a/spec/models/integrations/chat_message/wiki_page_message_spec.rb b/spec/models/integrations/chat_message/wiki_page_message_spec.rb index e8672a0f9c8..ded467dc910 100644 --- a/spec/models/integrations/chat_message/wiki_page_message_spec.rb +++ b/spec/models/integrations/chat_message/wiki_page_message_spec.rb @@ -5,20 +5,30 @@ require 'spec_helper' RSpec.describe Integrations::ChatMessage::WikiPageMessage do subject { described_class.new(args) } + let(:name) { 'Test User' } + let(:username) { 'test.user' } + let(:avatar_url) { 'http://someavatar.com' } + let(:project_name) { 'project_name' } + let(:project_url) {'http://somewhere.com' } + let(:url) { 'http://url.com' } + let(:diff_url) { 'http://url.com/diff?version_id=1234' } + let(:wiki_page_title) { 'Wiki page title' } + let(:commit_message) { 'Wiki page commit message' } let(:args) do { user: { - name: 'Test User', - username: 'test.user', - avatar_url: 'http://someavatar.com' + name: name, + username: username, + avatar_url: avatar_url }, - project_name: 'project_name', - project_url: 'http://somewhere.com', + project_name: project_name, + project_url: project_url, object_attributes: { - title: 'Wiki page title', - url: 'http://url.com', + title: wiki_page_title, + url: url, content: 'Wiki page content', - message: 'Wiki page commit message' + message: commit_message, + diff_url: diff_url } } end @@ -32,8 +42,8 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'Test User (test.user) created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + "#{name} (#{username}) created <#{url}|wiki page> (<#{diff_url}|Compare changes>) in <#{project_url}|#{project_name}>: "\ + "*#{wiki_page_title}*") end end @@ -44,8 +54,8 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'Test User (test.user) edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + "#{name} (#{username}) edited <#{url}|wiki page> (<#{diff_url}|Compare changes>) in <#{project_url}|#{project_name}>: "\ + "*#{wiki_page_title}*") end end end @@ -61,7 +71,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns the commit message for a new wiki page' do expect(subject.attachments).to eq([ { - text: "Wiki page commit message", + text: commit_message, color: color } ]) @@ -76,7 +86,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns the commit message for an updated wiki page' do expect(subject.attachments).to eq([ { - text: "Wiki page commit message", + text: commit_message, color: color } ]) @@ -98,7 +108,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'Test User (test.user) created [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + "#{name} (#{username}) created [wiki page](#{url}) ([Compare changes](#{diff_url})) in [#{project_name}](#{project_url}): *#{wiki_page_title}*") end end @@ -109,7 +119,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'Test User (test.user) edited [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + "#{name} (#{username}) edited [wiki page](#{url}) ([Compare changes](#{diff_url})) in [#{project_name}](#{project_url}): *#{wiki_page_title}*") end end end @@ -121,7 +131,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do end it 'returns the commit message for a new wiki page' do - expect(subject.attachments).to eq('Wiki page commit message') + expect(subject.attachments).to eq(commit_message) end end @@ -131,7 +141,7 @@ RSpec.describe Integrations::ChatMessage::WikiPageMessage do end it 'returns the commit message for an updated wiki page' do - expect(subject.attachments).to eq('Wiki page commit message') + expect(subject.attachments).to eq(commit_message) end end end diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index c217573f48d..08e18c99376 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -72,19 +72,19 @@ RSpec.describe Integrations::Confluence do subject { project.project_setting.has_confluence? } it 'sets the property to true when service is active' do - create(:confluence_service, project: project, active: true) + create(:confluence_integration, project: project, active: true) is_expected.to be(true) end it 'sets the property to false when service is not active' do - create(:confluence_service, project: project, active: false) + create(:confluence_integration, project: project, active: false) is_expected.to be(false) end it 'creates a project_setting record if one was not already created' do - expect { create(:confluence_service) }.to change { ProjectSetting.count }.by(1) + expect { create(:confluence_integration) }.to change(ProjectSetting, :count).by(1) end end end diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/integrations/custom_issue_tracker_spec.rb index 881ae60a680..25f2648e738 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/integrations/custom_issue_tracker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe CustomIssueTrackerService do +RSpec.describe Integrations::CustomIssueTracker do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/discord_service_spec.rb b/spec/models/integrations/discord_spec.rb index ffe0a36dcdc..bff6a8ee5b2 100644 --- a/spec/models/project_services/discord_service_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -2,8 +2,8 @@ require "spec_helper" -RSpec.describe DiscordService do - it_behaves_like "chat service", "Discord notifications" do +RSpec.describe Integrations::Discord do + it_behaves_like "chat integration", "Discord notifications" do let(:client) { Discordrb::Webhooks::Client } let(:client_arguments) { { url: webhook_url } } let(:payload) do diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 9aaf4f7a644..137f078edca 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do +RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers describe 'associations' do @@ -31,8 +31,8 @@ RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do end end - shared_context :drone_ci_service do - let(:drone) { DroneCiService.new } + shared_context :drone_ci_integration do + let(:drone) { described_class.new } let(:project) { create(:project, :repository, name: 'project') } let(:path) { project.full_path } let(:drone_url) { 'http://drone.example.com' } @@ -41,7 +41,7 @@ RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do let(:token) { 'secret' } let(:iid) { rand(1..9999) } - # URL's + # URLs let(:build_page) { "#{drone_url}/gitlab/#{path}/redirect/commits/#{sha}?branch=#{branch}" } let(:commit_status_path) { "#{drone_url}/gitlab/#{path}/commits/#{sha}?branch=#{branch}&access_token=#{token}" } @@ -67,14 +67,14 @@ RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do end describe "service page/path methods" do - include_context :drone_ci_service + include_context :drone_ci_integration it { expect(drone.build_page(sha, branch)).to eq(build_page) } it { expect(drone.commit_status_path(sha, branch)).to eq(commit_status_path) } end describe '#commit_status' do - include_context :drone_ci_service + include_context :drone_ci_integration it 'returns the contents of the reactive cache' do stub_reactive_cache(drone, { commit_status: 'foo' }, 'sha', 'ref') @@ -84,7 +84,7 @@ RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do end describe '#calculate_reactive_cache' do - include_context :drone_ci_service + include_context :drone_ci_integration describe '#commit_status' do subject { drone.calculate_reactive_cache(sha, branch)[:commit_status] } @@ -130,7 +130,7 @@ RSpec.describe DroneCiService, :use_clean_rails_memory_store_caching do end describe "execute" do - include_context :drone_ci_service + include_context :drone_ci_integration let(:user) { create(:user, username: 'username') } let(:push_sample_data) do diff --git a/spec/models/project_services/ewm_service_spec.rb b/spec/models/integrations/ewm_spec.rb index 311c456569e..38897adb447 100644 --- a/spec/models/project_services/ewm_service_spec.rb +++ b/spec/models/integrations/ewm_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe EwmService do +RSpec.describe Integrations::Ewm do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/external_wiki_service_spec.rb b/spec/models/integrations/external_wiki_spec.rb index c6891401a0f..8c20b810301 100644 --- a/spec/models/project_services/external_wiki_service_spec.rb +++ b/spec/models/integrations/external_wiki_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ExternalWikiService do +RSpec.describe Integrations::ExternalWiki do describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/integrations/flowdock_spec.rb index 94a49fb3080..2de6f7dd2f1 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/integrations/flowdock_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe FlowdockService do +RSpec.describe Integrations::Flowdock do describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/hangouts_chat_service_spec.rb b/spec/models/integrations/hangouts_chat_spec.rb index 9d3bd457fc8..17b40c484f5 100644 --- a/spec/models/project_services/hangouts_chat_service_spec.rb +++ b/spec/models/integrations/hangouts_chat_spec.rb @@ -2,8 +2,8 @@ require "spec_helper" -RSpec.describe HangoutsChatService do - it_behaves_like "chat service", "Hangouts Chat" do +RSpec.describe Integrations::HangoutsChat do + it_behaves_like "chat integration", "Hangouts Chat" do let(:client) { HangoutsChat::Sender } let(:client_arguments) { webhook_url } let(:payload) do diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/integrations/irker_spec.rb index 07963947de8..a69be1292e0 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/integrations/irker_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' require 'socket' require 'json' -RSpec.describe IrkerService do +RSpec.describe Integrations::Irker do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/issue_tracker_data_spec.rb b/spec/models/integrations/issue_tracker_data_spec.rb index a229285f09b..597df237c67 100644 --- a/spec/models/project_services/issue_tracker_data_spec.rb +++ b/spec/models/integrations/issue_tracker_data_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe IssueTrackerData do +RSpec.describe Integrations::IssueTrackerData do describe 'associations' do it { is_expected.to belong_to :integration } end diff --git a/spec/models/project_services/jenkins_service_spec.rb b/spec/models/integrations/jenkins_spec.rb index 4663e41736a..2374dfe4480 100644 --- a/spec/models/project_services/jenkins_service_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JenkinsService do +RSpec.describe Integrations::Jenkins do let(:project) { create(:project) } let(:jenkins_url) { 'http://jenkins.example.com/' } let(:jenkins_hook_url) { jenkins_url + 'project/my_project' } diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/integrations/jira_spec.rb index 73e91bf9ea8..f6310866773 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraService do +RSpec.describe Integrations::Jira do include AssetsHelpers let_it_be(:project) { create(:project, :repository) } @@ -35,20 +35,42 @@ RSpec.describe JiraService do username: 'username', password: 'test', jira_issue_transition_id: 24, - url: 'http://jira.test.com/path/' + url: 'http://jira.test.com:1234/path/' } end - let(:service) { described_class.create!(options) } + let(:integration) { described_class.create!(options) } it 'sets the URL properly' do # jira-ruby gem parses the URI and handles trailing slashes fine: # https://github.com/sumoheavy/jira-ruby/blob/v1.7.0/lib/jira/http_client.rb#L62 - expect(service.options[:site]).to eq('http://jira.test.com/') + expect(integration.options[:site]).to eq('http://jira.test.com:1234') end it 'leaves out trailing slashes in context' do - expect(service.options[:context_path]).to eq('/path') + expect(integration.options[:context_path]).to eq('/path') + end + + context 'URL without a path' do + before do + integration.url = 'http://jira.test.com/' + end + + it 'leaves out trailing slashes in context' do + expect(integration.options[:site]).to eq('http://jira.test.com') + expect(integration.options[:context_path]).to eq('') + end + end + + context 'URL with query string parameters' do + before do + integration.url << '?nosso&foo=bar' + end + + it 'removes query string parameters' do + expect(integration.options[:site]).to eq('http://jira.test.com:1234') + expect(integration.options[:context_path]).to eq('/path') + end end context 'username with trailing whitespaces' do @@ -57,13 +79,13 @@ RSpec.describe JiraService do end it 'leaves out trailing whitespaces in username' do - expect(service.options[:username]).to eq('username') + expect(integration.options[:username]).to eq('username') end end it 'provides additional cookies to allow basic auth with oracle webgate' do - expect(service.options[:use_cookies]).to eq(true) - expect(service.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog']) + expect(integration.options[:use_cookies]).to eq(true) + expect(integration.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog']) end context 'using api URL' do @@ -72,7 +94,7 @@ RSpec.describe JiraService do end it 'leaves out trailing slashes in context' do - expect(service.options[:context_path]).to eq('/api_path') + expect(integration.options[:context_path]).to eq('/api_path') end end end @@ -117,7 +139,8 @@ RSpec.describe JiraService do let(:params) do { project: project, - url: url, api_url: api_url, + url: url, + api_url: api_url, username: username, password: password, jira_issue_transition_id: transition_id } @@ -141,9 +164,9 @@ RSpec.describe JiraService do end context 'when loading serverInfo' do - let!(:jira_service) { subject } + let(:jira_service) { subject } - context 'Cloud instance' do + context 'from a Cloud instance' do let(:server_info_results) { { 'deploymentType' => 'Cloud' } } it 'is detected' do @@ -151,7 +174,7 @@ RSpec.describe JiraService do end end - context 'Server instance' do + context 'from a Server instance' do let(:server_info_results) { { 'deploymentType' => 'Server' } } it 'is detected' do @@ -159,11 +182,45 @@ RSpec.describe JiraService do end end - context 'Unknown instance' do + context 'from an Unknown instance' do let(:server_info_results) { { 'deploymentType' => 'FutureCloud' } } - it 'is detected' do - expect(jira_service.jira_tracker_data.deployment_unknown?).to be_truthy + context 'and URL ends in .atlassian.net' do + let(:api_url) { 'http://example-api.atlassian.net' } + + it 'deployment_type is set to cloud' do + expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy + end + end + + context 'and URL is something else' do + let(:api_url) { 'http://my-jira-api.someserver.com' } + + it 'deployment_type is set to server' do + expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy + end + end + end + + context 'and no ServerInfo response is received' do + let(:server_info_results) { {} } + + context 'and URL ends in .atlassian.net' do + let(:api_url) { 'http://example-api.atlassian.net' } + + it 'deployment_type is set to cloud' do + expect(Gitlab::AppLogger).to receive(:warn).with(message: "Jira API returned no ServerInfo, setting deployment_type from URL", server_info: server_info_results, url: api_url) + expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy + end + end + + context 'and URL is something else' do + let(:api_url) { 'http://my-jira-api.someserver.com' } + + it 'deployment_type is set to server' do + expect(Gitlab::AppLogger).to receive(:warn).with(message: "Jira API returned no ServerInfo, setting deployment_type from URL", server_info: server_info_results, url: api_url) + expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy + end end end end @@ -229,17 +286,31 @@ RSpec.describe JiraService do end context 'when updating the url, api_url, username, or password' do - it 'updates deployment type' do - service.update!(url: 'http://first.url') - service.jira_tracker_data.update!(deployment_type: 'server') + context 'when updating the integration' do + it 'updates deployment type' do + service.update!(url: 'http://first.url') + service.jira_tracker_data.update!(deployment_type: 'server') - expect(service.jira_tracker_data.deployment_server?).to be_truthy + expect(service.jira_tracker_data.deployment_server?).to be_truthy - service.update!(api_url: 'http://another.url') - service.jira_tracker_data.reload + service.update!(api_url: 'http://another.url') + service.jira_tracker_data.reload - expect(service.jira_tracker_data.deployment_cloud?).to be_truthy - expect(WebMock).to have_requested(:get, /serverInfo/).twice + expect(service.jira_tracker_data.deployment_cloud?).to be_truthy + expect(WebMock).to have_requested(:get, /serverInfo/).twice + end + end + + context 'when removing the integration' do + let(:server_info_results) { {} } + + it 'updates deployment type' do + service.update!(url: nil, api_url: nil, active: false) + + service.jira_tracker_data.reload + + expect(service.jira_tracker_data.deployment_unknown?).to be_truthy + end end it 'calls serverInfo for url' do @@ -493,7 +564,7 @@ RSpec.describe JiraService do before do jira_service.jira_issue_transition_id = '999' - # These stubs are needed to test JiraService#close_issue. + # These stubs are needed to test Integrations::Jira#close_issue. # We close the issue then do another request to API to check if it got closed. # Here is stubbed the API return with a closed and an opened issues. open_issue = JIRA::Resource::Issue.new(jira_service.client, attrs: issue_fields.deep_stringify_keys) @@ -829,7 +900,7 @@ RSpec.describe JiraService do context 'when disabled' do before do - allow_next_instance_of(JiraService) do |instance| + allow_next_instance_of(described_class) do |instance| allow(instance).to receive(:commit_events) { false } end end @@ -847,7 +918,7 @@ RSpec.describe JiraService do context 'when disabled' do before do - allow_next_instance_of(JiraService) do |instance| + allow_next_instance_of(described_class) do |instance| allow(instance).to receive(:merge_requests_events) { false } end end @@ -941,17 +1012,51 @@ RSpec.describe JiraService do end context 'generating external URLs' do - let(:service) { described_class.new(url: 'http://jira.test.com/path/') } + let(:integration) { described_class.new(url: 'http://jira.test.com/path/') } + + describe '#web_url' do + it 'handles paths, slashes, and query string' do + expect(integration.web_url).to eq(integration.url) + expect(integration.web_url('subpath/')).to eq('http://jira.test.com/path/subpath') + expect(integration.web_url('/subpath/')).to eq('http://jira.test.com/path/subpath') + expect(integration.web_url('subpath', foo: :bar)).to eq('http://jira.test.com/path/subpath?foo=bar') + end + + it 'preserves existing query string' do + integration.url = 'http://jira.test.com/path/?nosso&foo=bar%20bar' + + expect(integration.web_url).to eq("http://jira.test.com/path?foo=bar%20bar&nosso") + expect(integration.web_url('subpath/')).to eq('http://jira.test.com/path/subpath?foo=bar%20bar&nosso') + expect(integration.web_url('/subpath/')).to eq('http://jira.test.com/path/subpath?foo=bar%20bar&nosso') + expect(integration.web_url('subpath', bar: 'baz baz')).to eq('http://jira.test.com/path/subpath?bar=baz%20baz&foo=bar%20bar&nosso') + end + + it 'includes Atlassian referrer for gitlab.com' do + allow(Gitlab).to receive(:com?).and_return(true) + + expect(integration.web_url).to eq("http://jira.test.com/path?#{described_class::ATLASSIAN_REFERRER_GITLAB_COM.to_query}") + + allow(Gitlab).to receive(:staging?).and_return(true) + + expect(integration.web_url).to eq(integration.url) + end + + it 'includes Atlassian referrer for self-managed' do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) + + expect(integration.web_url).to eq("http://jira.test.com/path?#{described_class::ATLASSIAN_REFERRER_SELF_MANAGED.to_query}") + end + end describe '#issues_url' do - it 'handles trailing slashes' do - expect(service.issues_url).to eq('http://jira.test.com/path/browse/:id') + it 'returns the correct URL' do + expect(integration.issues_url).to eq('http://jira.test.com/path/browse/:id') end end describe '#new_issue_url' do - it 'handles trailing slashes' do - expect(service.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue!default.jspa') + it 'returns the correct URL' do + expect(integration.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue!default.jspa') end end end diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/integrations/jira_tracker_data_spec.rb index 72bdbe40a74..5430dd2eb52 100644 --- a/spec/models/project_services/jira_tracker_data_spec.rb +++ b/spec/models/integrations/jira_tracker_data_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraTrackerData do +RSpec.describe Integrations::JiraTrackerData do describe 'associations' do it { is_expected.to belong_to(:integration) } end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index 87befdd4303..c8a6584591c 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe MattermostSlashCommandsService do - it_behaves_like "chat slash commands service" +RSpec.describe Integrations::MattermostSlashCommands do + it_behaves_like Integrations::BaseSlashCommands context 'Mattermost API' do let(:project) { create(:project) } @@ -11,10 +11,10 @@ RSpec.describe MattermostSlashCommandsService do let(:user) { create(:user) } before do - session = Mattermost::Session.new(nil) + session = ::Mattermost::Session.new(nil) session.base_uri = 'http://mattermost.example.com' - allow_any_instance_of(Mattermost::Client).to receive(:with_session) + allow_any_instance_of(::Mattermost::Client).to receive(:with_session) .and_yield(session) end diff --git a/spec/models/integrations/mattermost_spec.rb b/spec/models/integrations/mattermost_spec.rb new file mode 100644 index 00000000000..f7702846b6c --- /dev/null +++ b/spec/models/integrations/mattermost_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::Mattermost do + it_behaves_like Integrations::SlackMattermostNotifier, "Mattermost" +end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index 5f3a94a5b99..2f1be233eb2 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MicrosoftTeamsService do +RSpec.describe Integrations::MicrosoftTeams do let(:chat_service) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } @@ -64,7 +64,7 @@ RSpec.describe MicrosoftTeamsService do end it 'specifies the webhook when it is configured' do - expect(MicrosoftTeams::Notifier).to receive(:new).with(webhook_url).and_return(double(:microsoft_teams_service).as_null_object) + expect(::MicrosoftTeams::Notifier).to receive(:new).with(webhook_url).and_return(double(:microsoft_teams_service).as_null_object) chat_service.execute(push_sample_data) end diff --git a/spec/models/project_services/open_project_service_spec.rb b/spec/models/integrations/open_project_spec.rb index 1abaab0ceff..e5b976dc91d 100644 --- a/spec/models/project_services/open_project_service_spec.rb +++ b/spec/models/integrations/open_project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe OpenProjectService do +RSpec.describe Integrations::OpenProject do describe 'Validations' do context 'when service is active' do before do diff --git a/spec/models/project_services/open_project_tracker_data_spec.rb b/spec/models/integrations/open_project_tracker_data_spec.rb index 1f7f01cfea4..41c913f978c 100644 --- a/spec/models/project_services/open_project_tracker_data_spec.rb +++ b/spec/models/integrations/open_project_tracker_data_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe OpenProjectTrackerData do +RSpec.describe Integrations::OpenProjectTrackerData do describe 'associations' do it { is_expected.to belong_to(:integration) } end diff --git a/spec/models/project_services/packagist_service_spec.rb b/spec/models/integrations/packagist_spec.rb index 33b5c9809c7..48f7e81adca 100644 --- a/spec/models/project_services/packagist_service_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PackagistService do +RSpec.describe Integrations::Packagist do let(:packagist_params) do { active: true, diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index 21cc5d44558..90055b04bb8 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PipelinesEmailService, :mailer do +RSpec.describe Integrations::PipelinesEmail, :mailer do let(:pipeline) do create(:ci_pipeline, :failed, project: project, diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/integrations/pivotaltracker_spec.rb index 8de85cc7fa5..2ce90b6f739 100644 --- a/spec/models/project_services/pivotaltracker_service_spec.rb +++ b/spec/models/integrations/pivotaltracker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PivotaltrackerService do +RSpec.describe Integrations::Pivotaltracker do include StubRequests describe 'Associations' do @@ -35,7 +35,7 @@ RSpec.describe PivotaltrackerService do end end - let(:url) { PivotaltrackerService::API_ENDPOINT } + let(:url) { described_class::API_ENDPOINT } def push_data(branch: 'master') { diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/integrations/pushover_spec.rb index b7d3b8987b8..be8dc5634a0 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/integrations/pushover_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PushoverService do +RSpec.describe Integrations::Pushover do include StubRequests describe 'Associations' do diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/integrations/redmine_spec.rb index b9be3940d34..083585d4fed 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/integrations/redmine_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RedmineService do +RSpec.describe Integrations::Redmine do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/project_services/slack_slash_commands_service_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index 95c87ef01bc..a9d3c820a3c 100644 --- a/spec/models/project_services/slack_slash_commands_service_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe SlackSlashCommandsService do - it_behaves_like "chat slash commands service" +RSpec.describe Integrations::SlackSlashCommands do + it_behaves_like Integrations::BaseSlashCommands describe '#trigger' do context 'when an auth url is generated' do diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/integrations/slack_spec.rb index 2e2c1c666d9..e598c528967 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe SlackService do - it_behaves_like "slack or mattermost notifications", 'Slack' +RSpec.describe Integrations::Slack do + it_behaves_like Integrations::SlackMattermostNotifier, "Slack" describe '#execute' do before do diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/integrations/teamcity_spec.rb index f71dad86a08..b88a4722ad4 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe TeamcityService, :use_clean_rails_memory_store_caching do +RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers include StubRequests diff --git a/spec/models/project_services/unify_circuit_service_spec.rb b/spec/models/integrations/unify_circuit_spec.rb index 0c749322e07..7a91b2d3c11 100644 --- a/spec/models/project_services/unify_circuit_service_spec.rb +++ b/spec/models/integrations/unify_circuit_spec.rb @@ -2,8 +2,8 @@ require "spec_helper" -RSpec.describe UnifyCircuitService do - it_behaves_like "chat service", "Unify Circuit" do +RSpec.describe Integrations::UnifyCircuit do + it_behaves_like "chat integration", "Unify Circuit" do let(:client_arguments) { webhook_url } let(:payload) do { diff --git a/spec/models/project_services/webex_teams_service_spec.rb b/spec/models/integrations/webex_teams_spec.rb index ed63f5bc48c..b5cba6762aa 100644 --- a/spec/models/project_services/webex_teams_service_spec.rb +++ b/spec/models/integrations/webex_teams_spec.rb @@ -2,8 +2,8 @@ require "spec_helper" -RSpec.describe WebexTeamsService do - it_behaves_like "chat service", "Webex Teams" do +RSpec.describe Integrations::WebexTeams do + it_behaves_like "chat integration", "Webex Teams" do let(:client_arguments) { webhook_url } let(:payload) do { diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/integrations/youtrack_spec.rb index 4339b44e1de..314204f6fb4 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/integrations/youtrack_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe YoutrackService do +RSpec.describe Integrations::Youtrack do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 884c476932e..edb93ecf4b6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Issue do include ExternalAuthorizationServiceHelpers + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } let_it_be(:reusable_project) { create(:project) } @@ -1287,15 +1289,33 @@ RSpec.describe Issue do end end - let(:project) { build_stubbed(:project_empty_repo) } - let(:issue) { build_stubbed(:issue, relative_position: 100, project: project) } + shared_examples 'schedules issues rebalancing' do + let(:issue) { build_stubbed(:issue, relative_position: 100, project: project) } + + it 'schedules rebalancing if we time-out when moving' do + lhs = build_stubbed(:issue, relative_position: 99, project: project) + to_move = build(:issue, project: project) + expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project_id, namespace_id) + + expect { to_move.move_between(lhs, issue) }.to raise_error(ActiveRecord::QueryCanceled) + end + end + + context 'when project in user namespace' do + let(:project) { build_stubbed(:project_empty_repo) } + let(:project_id) { project.id } + let(:namespace_id) { nil } + + it_behaves_like 'schedules issues rebalancing' + end - it 'schedules rebalancing if we time-out when moving' do - lhs = build_stubbed(:issue, relative_position: 99, project: project) - to_move = build(:issue, project: project) - expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project.id) + context 'when project in a group namespace' do + let(:group) { create(:group) } + let(:project) { build_stubbed(:project_empty_repo, group: group) } + let(:project_id) { nil } + let(:namespace_id) { group.id } - expect { to_move.move_between(lhs, issue) }.to raise_error(ActiveRecord::QueryCanceled) + it_behaves_like 'schedules issues rebalancing' end end @@ -1315,6 +1335,26 @@ RSpec.describe Issue do end end + describe '#supports_time_tracking?' do + let_it_be(:project) { create(:project) } + let_it_be_with_refind(:issue) { create(:incident, project: project) } + + where(:issue_type, :supports_time_tracking) do + :issue | true + :incident | true + end + + with_them do + before do + issue.update!(issue_type: issue_type) + end + + it do + expect(issue.supports_time_tracking?).to eq(supports_time_tracking) + end + end + end + describe '#email_participants_emails' do let_it_be(:issue) { create(:issue) } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 0cb20efcb0a..7468c1b9f0a 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -85,9 +85,9 @@ RSpec.describe Key, :mailer do let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) } let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) } - describe '.expired_today_and_not_notified' do - it 'returns keys that expire today' do - expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified) + describe '.expired_and_not_notified' do + it 'returns keys that expire today and in the past' do + expect(described_class.expired_and_not_notified).to contain_exactly(expired_today_not_notified, expired_yesterday) end end @@ -139,7 +139,7 @@ RSpec.describe Key, :mailer do end with_them do - let!(:key) { create(factory) } + let!(:key) { create(factory) } # rubocop:disable Rails/SaveBang let!(:original_fingerprint) { key.fingerprint } let!(:original_fingerprint_sha256) { key.fingerprint_sha256 } @@ -224,7 +224,7 @@ RSpec.describe Key, :mailer do expect(AuthorizedKeysWorker).to receive(:perform_async).with(:remove_key, key.shell_id) - key.destroy + key.destroy! end end @@ -244,7 +244,7 @@ RSpec.describe Key, :mailer do expect(AuthorizedKeysWorker).not_to receive(:perform_async) - key.destroy + key.destroy! end end end diff --git a/spec/models/label_link_spec.rb b/spec/models/label_link_spec.rb index e5753b34e72..c6bec215145 100644 --- a/spec/models/label_link_spec.rb +++ b/spec/models/label_link_spec.rb @@ -13,27 +13,14 @@ RSpec.describe LabelLink do let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end - describe 'scopes' do - describe '.for_target' do - it 'returns the label links for a given target' do - label_link = create(:label_link, target: create(:merge_request)) + describe '.for_target' do + it 'returns the label links for a given target' do + label_link = create(:label_link, target: create(:merge_request)) - create(:label_link, target: create(:issue)) + create(:label_link, target: create(:issue)) - expect(described_class.for_target(label_link.target_id, label_link.target_type)) - .to contain_exactly(label_link) - end - end - - describe '.with_remove_on_close_labels' do - it 'responds with label_links that can be removed when an issue is closed' do - issue = create(:issue) - removable_label = create(:label, project: issue.project, remove_on_close: true) - create(:label_link, target: issue) - removable_issue_label_link = create(:label_link, label: removable_label, target: issue) - - expect(described_class.with_remove_on_close_labels).to contain_exactly(removable_issue_label_link) - end + expect(described_class.for_target(label_link.target_id, label_link.target_type)) + .to contain_exactly(label_link) end end end diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index a0f633218b0..5210709a468 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -178,4 +178,34 @@ RSpec.describe LfsObject do expect(described_class.calculate_oid(path)).to eq expected end end + + context 'when an lfs object is associated with a project' do + let!(:lfs_object) { create(:lfs_object) } + let!(:lfs_object_project) { create(:lfs_objects_project, lfs_object: lfs_object) } + + it 'cannot be deleted' do + expect { lfs_object.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey) + + lfs_object_project.destroy! + + expect { lfs_object.destroy! }.not_to raise_error + end + end + + describe '.unreferenced_in_batches' do + let!(:unreferenced_lfs_object1) { create(:lfs_object, oid: '1') } + let!(:referenced_lfs_object) { create(:lfs_objects_project).lfs_object } + let!(:unreferenced_lfs_object2) { create(:lfs_object, oid: '2') } + + it 'returns lfs objects in batches' do + stub_const('LfsObject::BATCH_SIZE', 1) + + batches = [] + described_class.unreferenced_in_batches { |batch| batches << batch } + + expect(batches.size).to eq(2) + expect(batches.first).to eq([unreferenced_lfs_object2]) + expect(batches.last).to eq([unreferenced_lfs_object1]) + end + end end diff --git a/spec/models/lfs_objects_project_spec.rb b/spec/models/lfs_objects_project_spec.rb index 71009a6f28f..df49b60c4fa 100644 --- a/spec/models/lfs_objects_project_spec.rb +++ b/spec/models/lfs_objects_project_spec.rb @@ -39,7 +39,7 @@ RSpec.describe LfsObjectsProject do expect(ProjectCacheWorker).to receive(:perform_async) .with(project.id, [], [:lfs_objects_size]) - subject.destroy + subject.destroy! end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 247be7654d8..372fc40afcc 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -408,6 +408,20 @@ RSpec.describe Member do it { is_expected.not_to include @member_with_minimal_access } end + describe '.without_invites_and_requests' do + subject { described_class.without_invites_and_requests.to_a } + + it { is_expected.to include @owner } + it { is_expected.to include @maintainer } + it { is_expected.not_to include @invited_member } + it { is_expected.to include @accepted_invite_member } + it { is_expected.not_to include @requested_member } + it { is_expected.to include @accepted_request_member } + it { is_expected.to include @blocked_maintainer } + it { is_expected.to include @blocked_developer } + it { is_expected.not_to include @member_with_minimal_access } + end + describe '.connected_to_user' do subject { described_class.connected_to_user.to_a } @@ -594,6 +608,18 @@ RSpec.describe Member do end end + context 'when called with a known user secondary email' do + let(:secondary_email) { create(:email, email: 'secondary@example.com', user: user) } + + it 'adds the user as a member' do + expect(source.users).not_to include(user) + + described_class.add_user(source, secondary_email.email, :maintainer) + + expect(source.users.reload).to include(user) + end + end + context 'when called with an unknown user email' do it 'creates an invited member' do expect(source.users).not_to include(user) @@ -778,10 +804,27 @@ RSpec.describe Member do let(:invited_member) { create(:project_member, invite_email: "user@example.com", user: nil) } let(:requester) { create(:project_member, requested_at: Time.current.utc) } - it { expect(invited_member).to be_invite } + it { expect(invited_member).to be_pending } it { expect(requester).to be_pending } end + describe '#hook_prerequisites_met?' do + let(:member) { create(:project_member) } + + context 'when the member does not have an associated user' do + it 'returns false' do + member.update_column(:user_id, nil) + expect(member.reload.hook_prerequisites_met?).to eq(false) + end + end + + context 'when the member has an associated user' do + it 'returns true' do + expect(member.hook_prerequisites_met?).to eq(true) + end + end + end + describe "#accept_invite!" do let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } let(:user) { create(:user) } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 3a2db5d8516..8c942228059 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -37,6 +37,10 @@ RSpec.describe GroupMember do end end + describe 'delegations' do + it { is_expected.to delegate_method(:update_two_factor_requirement).to(:user).allow_nil } + end + describe '.access_level_roles' do it 'returns Gitlab::Access.options_with_owner' do expect(described_class.access_level_roles).to eq(Gitlab::Access.options_with_owner) @@ -93,6 +97,18 @@ RSpec.describe GroupMember do end end + describe '#destroy' do + context 'for an orphaned member' do + let!(:orphaned_group_member) do + create(:group_member).tap { |member| member.update_column(:user_id, nil) } + end + + it 'does not raise an error' do + expect { orphaned_group_member.destroy! }.not_to raise_error + end + end + end + describe '#after_accept_invite' do it 'calls #update_two_factor_requirement' do email = 'foo@email.com' diff --git a/spec/models/members/last_group_owner_assigner_spec.rb b/spec/models/members/last_group_owner_assigner_spec.rb index 3c9a7a11555..bb0f751e7d5 100644 --- a/spec/models/members/last_group_owner_assigner_spec.rb +++ b/spec/models/members/last_group_owner_assigner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Members::LastGroupOwnerAssigner do +RSpec.describe LastGroupOwnerAssigner do describe "#execute" do let_it_be(:user, reload: true) { create(:user) } let_it_be(:group) { create(:group) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index fa77e319c2c..b84b408cb4b 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -58,6 +58,16 @@ RSpec.describe ProjectMember do maintainer.destroy! expect(Event.recent.first).to be_left_action end + + context 'for an orphaned member' do + let!(:orphaned_project_member) do + owner.tap { |member| member.update_column(:user_id, nil) } + end + + it 'does not raise an error' do + expect { orphaned_project_member.destroy! }.not_to raise_error + end + end end describe '.import_team' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 4075eb96fc2..3741e01e99a 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -418,8 +418,8 @@ RSpec.describe MergeRequestDiff do shared_examples_for 'fetching full diffs' do it 'returns diffs from repository comparison' do expect_next_instance_of(Compare) do |comparison| - expect(comparison).to receive(:diffs_in_batch) - .with(1, 10, diff_options: diff_options) + expect(comparison).to receive(:diffs) + .with(diff_options) .and_call_original end @@ -448,13 +448,13 @@ RSpec.describe MergeRequestDiff do end it_behaves_like 'fetching full diffs' - end - context 'when diff_options include ignore_whitespace_change' do - it_behaves_like 'fetching full diffs' do + context 'when diff_options include ignore_whitespace_change' do let(:diff_options) do { ignore_whitespace_change: true } end + + it_behaves_like 'fetching full diffs' end end @@ -485,6 +485,51 @@ RSpec.describe MergeRequestDiff do 'files/whitespace' ]) end + + context 'when diff_options include ignore_whitespace_change' do + let(:diff_options) do + { ignore_whitespace_change: true } + end + + it 'returns a Gitlab::Diff::FileCollection::Compare with paginated diffs' do + diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare) + expect(diffs.diff_files.size).to eq 10 + expect(diffs.pagination_data).to eq(current_page: 1, next_page: 2, total_pages: 2) + end + + it 'returns an empty MergeRequestBatch with empty pagination data when the batch is empty' do + diffs = diff_with_commits.diffs_in_batch(3, 10, diff_options: diff_options) + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch) + expect(diffs.diff_files.size).to eq 0 + expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: nil) + end + + context 'with gradual load enabled' do + before do + stub_feature_flags(diffs_gradual_load: true) + end + + it 'returns pagination data from MergeRequestDiffBatch' do + diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) + file_count = diff_with_commits.merge_request_diff_files.count + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare) + expect(diffs.diff_files.size).to eq 10 + expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: file_count) + end + + it 'returns an empty MergeRequestBatch with empty pagination data when the batch is empty' do + diffs = diff_with_commits.diffs_in_batch(30, 10, diff_options: diff_options) + + expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch) + expect(diffs.diff_files.size).to eq 0 + expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: nil) + end + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a77ca1e9a51..94b4c1901b8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -99,16 +99,17 @@ RSpec.describe MergeRequest, factory_default: :keep do let_it_be(:merge_request1) { create(:merge_request, :unique_branches, reviewers: [user1])} let_it_be(:merge_request2) { create(:merge_request, :unique_branches, reviewers: [user2])} let_it_be(:merge_request3) { create(:merge_request, :unique_branches, reviewers: [])} + let_it_be(:merge_request4) { create(:merge_request, :draft_merge_request)} describe '.review_requested' do - it 'returns MRs that has any review requests' do + it 'returns MRs that have any review requests' do expect(described_class.review_requested).to eq([merge_request1, merge_request2]) end end describe '.no_review_requested' do - it 'returns MRs that has no review requests' do - expect(described_class.no_review_requested).to eq([merge_request3]) + it 'returns MRs that have no review requests' do + expect(described_class.no_review_requested).to eq([merge_request3, merge_request4]) end end @@ -119,8 +120,15 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe '.no_review_requested_to' do - it 'returns MRs that the user has been requested to review' do - expect(described_class.no_review_requested_to(user1)).to eq([merge_request2, merge_request3]) + it 'returns MRs that the user has not been requested to review' do + expect(described_class.no_review_requested_to(user1)) + .to eq([merge_request2, merge_request3, merge_request4]) + end + end + + describe '.drafts' do + it 'returns MRs where draft == true' do + expect(described_class.drafts).to eq([merge_request4]) end end end @@ -296,7 +304,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'does not create duplicated metrics records when MR is concurrently updated' do - merge_request.metrics.destroy + merge_request.metrics.destroy! instance1 = MergeRequest.find(merge_request.id) instance2 = MergeRequest.find(merge_request.id) @@ -317,6 +325,38 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(merge_request.target_project_id).to eq(merge_request.metrics.target_project_id) end end + + describe '#set_draft_status' do + let(:merge_request) { create(:merge_request) } + + context 'MR is a draft' do + before do + expect(merge_request.draft).to be_falsy + + merge_request.title = "Draft: #{merge_request.title}" + end + + it 'sets draft to true' do + merge_request.save! + + expect(merge_request.draft).to be_truthy + end + end + + context 'MR is not a draft' do + before do + expect(merge_request.draft).to be_falsey + + merge_request.title = "This is not a draft" + end + + it 'sets draft to true' do + merge_request.save! + + expect(merge_request.draft).to be_falsey + end + end + end end describe 'respond to' do @@ -347,7 +387,7 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } it 'returns empty requests' do - latest_merge_request_diff = merge_request.merge_request_diffs.create + latest_merge_request_diff = merge_request.merge_request_diffs.create! MergeRequestDiffCommit.where( merge_request_diff_id: latest_merge_request_diff, @@ -459,7 +499,7 @@ RSpec.describe MergeRequest, factory_default: :keep do } create(:merge_request, params).tap do |mr| - diffs.times { mr.merge_request_diffs.create } + diffs.times { mr.merge_request_diffs.create! } mr.create_merge_head_diff end end @@ -891,7 +931,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when there are MR diffs' do it 'delegates to the MR diffs' do - merge_request.save + merge_request.save! expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)).and_call_original @@ -1036,20 +1076,20 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when there are MR diffs' do it 'returns the correct count' do - merge_request.save + merge_request.save! expect(merge_request.diff_size).to eq('105') end it 'returns the correct overflow count' do allow(Commit).to receive(:max_diff_options).and_return(max_files: 2) - merge_request.save + merge_request.save! expect(merge_request.diff_size).to eq('2+') end it 'does not perform highlighting' do - merge_request.save + merge_request.save! expect(Gitlab::Diff::Highlight).not_to receive(:new) @@ -1470,7 +1510,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it "can't remove a root ref" do - subject.update(source_branch: 'master', target_branch: 'feature') + subject.update!(source_branch: 'master', target_branch: 'feature') expect(subject.can_remove_source_branch?(user)).to be_falsey end @@ -2501,7 +2541,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'with a completely different branch' do before do - subject.update(target_branch: 'csv') + subject.update!(target_branch: 'csv') end it_behaves_like 'returning all SHA' @@ -2509,7 +2549,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'with a branch having no difference' do before do - subject.update(target_branch: 'branch-merged') + subject.update!(target_branch: 'branch-merged') subject.reload # make sure commits were not cached end @@ -3207,7 +3247,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'and a failed pipeline is associated' do before do - pipeline.update(status: 'failed', sha: subject.diff_head_sha) + pipeline.update!(status: 'failed', sha: subject.diff_head_sha) allow(subject).to receive(:head_pipeline) { pipeline } end @@ -3216,7 +3256,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'and a successful pipeline is associated' do before do - pipeline.update(status: 'success', sha: subject.diff_head_sha) + pipeline.update!(status: 'success', sha: subject.diff_head_sha) allow(subject).to receive(:head_pipeline) { pipeline } end @@ -3225,7 +3265,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'and a skipped pipeline is associated' do before do - pipeline.update(status: 'skipped', sha: subject.diff_head_sha) + pipeline.update!(status: 'skipped', sha: subject.diff_head_sha) allow(subject).to receive(:head_pipeline).and_return(pipeline) end @@ -3530,7 +3570,7 @@ RSpec.describe MergeRequest, factory_default: :keep do before do # Update merge_request_diff so that #diff_refs will return commit.diff_refs allow(subject).to receive(:create_merge_request_diff) do - subject.merge_request_diffs.create( + subject.merge_request_diffs.create!( base_commit_sha: commit.parent_id, start_commit_sha: commit.parent_id, head_commit_sha: commit.sha @@ -3800,7 +3840,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'returns false if the merge request is merged' do - merge_request.update(state: 'merged') + merge_request.update!(state: 'merged') expect(merge_request.reload.reopenable?).to be_falsey end @@ -3880,14 +3920,6 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:service_class) { 'Ci::CompareMetricsReportsService' } it { is_expected.to be_truthy } - - context 'with the metrics report flag disabled' do - before do - stub_feature_flags(merge_base_pipeline_for_metrics_comparison: false) - end - - it { is_expected.to be_falsey } - end end context 'when service class is Ci::CompareCodequalityReportsService' do @@ -4029,9 +4061,9 @@ RSpec.describe MergeRequest, factory_default: :keep do subject { create(:merge_request, importing: true, source_project: project) } - let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } - let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:merge_request_diff1) { subject.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { subject.merge_request_diffs.create!(head_commit_sha: nil) } + let!(:merge_request_diff3) { subject.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'with diff refs' do it 'returns the diffs' do @@ -4062,9 +4094,9 @@ RSpec.describe MergeRequest, factory_default: :keep do subject { create(:merge_request, importing: true, source_project: project) } - let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } - let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:merge_request_diff1) { subject.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { subject.merge_request_diffs.create!(head_commit_sha: nil) } + let!(:merge_request_diff3) { subject.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'when the diff refs are for an older merge request version' do let(:diff_refs) { merge_request_diff1.diff_refs } @@ -4108,7 +4140,7 @@ RSpec.describe MergeRequest, factory_default: :keep do it 'refreshes the number of open merge requests of the target project' do project = subject.target_project - expect { subject.destroy } + expect { subject.destroy! } .to change { project.open_merge_requests_count }.from(1).to(0) end end @@ -4874,7 +4906,6 @@ RSpec.describe MergeRequest, factory_default: :keep do subject { merge_request.enabled_reports[report_type] } before do - stub_feature_flags(drop_license_management_artifact: false) stub_licensed_features({ feature => true }) end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 20dee288052..7cf7c360dff 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -8,7 +8,46 @@ RSpec.describe Milestone do let(:milestone) { create(:milestone, project: project) } let(:project) { create(:project, :public) } - it_behaves_like 'a timebox', :milestone + it_behaves_like 'a timebox', :milestone do + describe "#uniqueness_of_title" do + context "per project" do + it "does not accept the same title in a project twice" do + new_timebox = timebox.dup + expect(new_timebox).not_to be_valid + end + + it "accepts the same title in another project" do + project = create(:project) + new_timebox = timebox.dup + new_timebox.project = project + + expect(new_timebox).to be_valid + end + end + + context "per group" do + let(:timebox) { create(:milestone, *timebox_args, group: group) } + + before do + project.update!(group: group) + end + + it "does not accept the same title in a group twice" do + new_timebox = described_class.new(group: group, title: timebox.title) + + expect(new_timebox).not_to be_valid + end + + it "does not accept the same title of a child project timebox" do + create(:milestone, *timebox_args, project: group.projects.first) + + new_timebox = described_class.new(group: group, title: timebox.title) + + expect(new_timebox).not_to be_valid + end + end + end + end describe 'MilestoneStruct#serializable_hash' do let(:predefined_milestone) { described_class::TimeboxStruct.new('Test Milestone', '#test', 1) } @@ -158,7 +197,7 @@ RSpec.describe Milestone do it 'returns false if milestone active and not all nested issues closed' do issue.milestone = milestone - issue.save + issue.save! expect(milestone.can_be_closed?).to be_falsey end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 56afe49e15f..2c514593de8 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Namespace do include ProjectForksHelper include GitHelpers + include ReloadHelpers let!(:namespace) { create(:namespace, :with_namespace_settings) } let(:gitlab_shell) { Gitlab::Shell.new } @@ -199,6 +200,8 @@ RSpec.describe Namespace do it { is_expected.to include_module(Namespaces::Traversal::Linear) } end + it_behaves_like 'linear namespace traversal' + context 'traversal_ids on create' do context 'default traversal_ids' do let(:namespace) { build(:namespace) } @@ -804,9 +807,9 @@ RSpec.describe Namespace do end it 'updates the project storage location' do - repository_project_in_parent_group = create(:project_repository, project: project_in_parent_group) - repository_hashed_project_in_subgroup = create(:project_repository, project: hashed_project_in_subgroup) - repository_legacy_project_in_subgroup = create(:project_repository, project: legacy_project_in_subgroup) + repository_project_in_parent_group = project_in_parent_group.project_repository + repository_hashed_project_in_subgroup = hashed_project_in_subgroup.project_repository + repository_legacy_project_in_subgroup = legacy_project_in_subgroup.project_repository parent.update(path: 'mygroup_moved') @@ -1010,47 +1013,52 @@ RSpec.describe Namespace do end end - describe '#all_projects' do + shared_examples '#all_projects' do context 'when namespace is a group' do - let(:namespace) { create(:group) } - let(:child) { create(:group, parent: namespace) } - let!(:project1) { create(:project_empty_repo, namespace: namespace) } - let!(:project2) { create(:project_empty_repo, namespace: child) } + let_it_be(:namespace) { create(:group) } + let_it_be(:child) { create(:group, parent: namespace) } + let_it_be(:project1) { create(:project_empty_repo, namespace: namespace) } + let_it_be(:project2) { create(:project_empty_repo, namespace: child) } + let_it_be(:other_project) { create(:project_empty_repo) } + + before do + reload_models(namespace, child) + end it { expect(namespace.all_projects.to_a).to match_array([project2, project1]) } it { expect(child.all_projects.to_a).to match_array([project2]) } - - it 'queries for the namespace and its descendants' do - expect(Project).to receive(:where).with(namespace: [namespace, child]) - - namespace.all_projects - end end context 'when namespace is a user namespace' do let_it_be(:user) { create(:user) } let_it_be(:user_namespace) { create(:namespace, owner: user) } let_it_be(:project) { create(:project, namespace: user_namespace) } + let_it_be(:other_project) { create(:project_empty_repo) } - it { expect(user_namespace.all_projects.to_a).to match_array([project]) } + before do + reload_models(user_namespace) + end - it 'only queries for the namespace itself' do - expect(Project).to receive(:where).with(namespace: user_namespace) + it { expect(user_namespace.all_projects.to_a).to match_array([project]) } + end + end - user_namespace.all_projects + describe '#all_projects' do + context 'with use_traversal_ids feature flag enabled' do + before do + stub_feature_flags(use_traversal_ids: true) end + + include_examples '#all_projects' end - end - describe '#all_pipelines' do - let(:group) { create(:group) } - let(:child) { create(:group, parent: group) } - let!(:project1) { create(:project_empty_repo, namespace: group) } - let!(:project2) { create(:project_empty_repo, namespace: child) } - let!(:pipeline1) { create(:ci_empty_pipeline, project: project1) } - let!(:pipeline2) { create(:ci_empty_pipeline, project: project2) } + context 'with use_traversal_ids feature flag disabled' do + before do + stub_feature_flags(use_traversal_ids: false) + end - it { expect(group.all_pipelines.to_a).to match_array([pipeline1, pipeline2]) } + include_examples '#all_projects' + end end describe '#share_with_group_lock with subgroups' do @@ -1379,36 +1387,14 @@ RSpec.describe Namespace do describe '#pages_virtual_domain' do let(:project) { create(:project, namespace: namespace) } - context 'when there are pages deployed for the project' do - context 'but pages metadata is not migrated' do - before do - generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') - generic_commit_status.update!(project: project) - project.pages_metadatum.destroy! - end - - it 'migrates pages metadata and returns the virual domain' do - virtual_domain = namespace.pages_virtual_domain + it 'returns the virual domain' do + project.mark_pages_as_deployed + project.update_pages_deployment!(create(:pages_deployment, project: project)) - expect(project.reload.pages_metadatum.deployed).to eq(true) + virtual_domain = namespace.pages_virtual_domain - expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) - expect(virtual_domain.lookup_paths).not_to be_empty - end - end - - context 'and pages metadata is migrated' do - before do - project.mark_pages_as_deployed - end - - it 'returns the virual domain' do - virtual_domain = namespace.pages_virtual_domain - - expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) - expect(virtual_domain.lookup_paths).not_to be_empty - end - end + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 4eabc266b40..d9f566f9383 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1522,4 +1522,16 @@ RSpec.describe Note do it { is_expected.to be_truthy } end end + + describe '#attachment' do + it 'is cleaned up correctly when project is destroyed' do + note = create(:note_on_issue, :with_attachment) + + attachment = note.attachment + + note.project.destroy! + + expect(attachment).not_to be_exist + end + end end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding_progress_spec.rb index 779312c9fa0..deac8d29196 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding_progress_spec.rb @@ -211,4 +211,26 @@ RSpec.describe OnboardingProgress do it { is_expected.to eq(:subscription_created_at) } end + + describe '#number_of_completed_actions' do + subject { build(:onboarding_progress, actions.map { |x| { x => Time.current } }.inject(:merge)).number_of_completed_actions } + + context '0 completed actions' do + let(:actions) { [:created_at, :updated_at] } + + it { is_expected.to eq(0) } + end + + context '1 completed action' do + let(:actions) { [:created_at, :subscription_created_at] } + + it { is_expected.to eq(1) } + end + + context '2 completed actions' do + let(:actions) { [:subscription_created_at, :git_write_at] } + + it { is_expected.to eq(2) } + end + end end diff --git a/spec/models/operations/feature_flag_scope_spec.rb b/spec/models/operations/feature_flag_scope_spec.rb index 29d338d8b29..dc83789fade 100644 --- a/spec/models/operations/feature_flag_scope_spec.rb +++ b/spec/models/operations/feature_flag_scope_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Operations::FeatureFlagScope do end context 'when environment scope of a default scope is updated' do - let!(:feature_flag) { create(:operations_feature_flag) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) } let!(:scope_default) { feature_flag.default_scope } it 'keeps default scope intact' do @@ -41,7 +41,7 @@ RSpec.describe Operations::FeatureFlagScope do end context 'when a default scope is destroyed' do - let!(:feature_flag) { create(:operations_feature_flag) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) } let!(:scope_default) { feature_flag.default_scope } it 'prevents from destroying the default scope' do diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index d5b3c7a8582..55682e12642 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -181,7 +181,7 @@ RSpec.describe Operations::FeatureFlag do end context 'when the feature flag is active and all scopes are inactive' do - let!(:feature_flag) { create(:operations_feature_flag, active: true) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) } it 'returns the flag' do feature_flag.default_scope.update!(active: false) @@ -199,7 +199,7 @@ RSpec.describe Operations::FeatureFlag do end context 'when the feature flag is inactive and all scopes are active' do - let!(:feature_flag) { create(:operations_feature_flag, active: false) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) } it 'does not return the flag' do feature_flag.default_scope.update!(active: true) @@ -221,7 +221,7 @@ RSpec.describe Operations::FeatureFlag do end context 'when the feature flag is active and all scopes are inactive' do - let!(:feature_flag) { create(:operations_feature_flag, active: true) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) } it 'does not return the flag' do feature_flag.default_scope.update!(active: false) @@ -239,7 +239,7 @@ RSpec.describe Operations::FeatureFlag do end context 'when the feature flag is inactive and all scopes are active' do - let!(:feature_flag) { create(:operations_feature_flag, active: false) } + let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) } it 'returns the flag' do feature_flag.default_scope.update!(active: true) diff --git a/spec/models/packages/debian/file_entry_spec.rb b/spec/models/packages/debian/file_entry_spec.rb index 7aa16bc0cce..e981adf69bc 100644 --- a/spec/models/packages/debian/file_entry_spec.rb +++ b/spec/models/packages/debian/file_entry_spec.rb @@ -7,11 +7,11 @@ RSpec.describe Packages::Debian::FileEntry, type: :model do let(:filename) { 'sample_1.2.3~alpha2.dsc' } let(:size) { 671 } - let(:md5sum) { '3b0817804f669e16cdefac583ad88f0e' } + let(:md5sum) { package_file.file_md5 } let(:section) { 'libs' } let(:priority) { 'optional' } - let(:sha1sum) { '32ecbd674f0bfd310df68484d87752490685a8d6' } - let(:sha256sum) { '844f79825b7e8aaa191e514b58a81f9ac1e58e2180134b0c9512fa66d896d7ba' } + let(:sha1sum) { package_file.file_sha1 } + let(:sha256sum) { package_file.file_sha256 } let(:file_entry) do described_class.new( @@ -42,7 +42,7 @@ RSpec.describe Packages::Debian::FileEntry, type: :model do describe '#md5sum' do it { is_expected.to validate_presence_of(:md5sum) } - it { is_expected.not_to allow_value('12345678901234567890123456789012').for(:md5sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 3b0817804f669e16cdefac583ad88f0e != 12345678901234567890123456789012') } + it { is_expected.not_to allow_value('12345678901234567890123456789012').for(:md5sum).with_message("mismatch for sample_1.2.3~alpha2.dsc: #{package_file.file_md5} != 12345678901234567890123456789012") } end describe '#section' do @@ -55,12 +55,12 @@ RSpec.describe Packages::Debian::FileEntry, type: :model do describe '#sha1sum' do it { is_expected.to validate_presence_of(:sha1sum) } - it { is_expected.not_to allow_value('1234567890123456789012345678901234567890').for(:sha1sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 32ecbd674f0bfd310df68484d87752490685a8d6 != 1234567890123456789012345678901234567890') } + it { is_expected.not_to allow_value('1234567890123456789012345678901234567890').for(:sha1sum).with_message("mismatch for sample_1.2.3~alpha2.dsc: #{package_file.file_sha1} != 1234567890123456789012345678901234567890") } end describe '#sha256sum' do it { is_expected.to validate_presence_of(:sha256sum) } - it { is_expected.not_to allow_value('1234567890123456789012345678901234567890123456789012345678901234').for(:sha256sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 844f79825b7e8aaa191e514b58a81f9ac1e58e2180134b0c9512fa66d896d7ba != 1234567890123456789012345678901234567890123456789012345678901234') } + it { is_expected.not_to allow_value('1234567890123456789012345678901234567890123456789012345678901234').for(:sha256sum).with_message("mismatch for sample_1.2.3~alpha2.dsc: #{package_file.file_sha256} != 1234567890123456789012345678901234567890123456789012345678901234") } end describe '#package_file' do diff --git a/spec/models/packages/debian/group_distribution_key_spec.rb b/spec/models/packages/debian/group_distribution_key_spec.rb new file mode 100644 index 00000000000..9ba163012b0 --- /dev/null +++ b/spec/models/packages/debian/group_distribution_key_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::GroupDistributionKey do + it_behaves_like 'Debian Distribution Key', :group +end diff --git a/spec/models/packages/debian/project_distribution_key_spec.rb b/spec/models/packages/debian/project_distribution_key_spec.rb new file mode 100644 index 00000000000..3dd723423f1 --- /dev/null +++ b/spec/models/packages/debian/project_distribution_key_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::ProjectDistributionKey do + it_behaves_like 'Debian Distribution Key', :project +end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index f8ddd59ddc8..7f2f22c815c 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -116,6 +116,22 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '.for_helm_with_channel' do + let_it_be(:project) { create(:project) } + let_it_be(:non_helm_package) { create(:nuget_package, project: project, package_type: :nuget) } + let_it_be(:helm_package1) { create(:helm_package, project: project, package_type: :helm) } + let_it_be(:helm_package2) { create(:helm_package, project: project, package_type: :helm) } + let_it_be(:channel) { 'some-channel' } + + let_it_be(:non_helm_file) { create(:package_file, :nuget, package: non_helm_package) } + let_it_be(:helm_file1) { create(:helm_package_file, package: helm_package1) } + let_it_be(:helm_file2) { create(:helm_package_file, package: helm_package2, channel: channel) } + + it 'returns the matching file only for Helm packages' do + expect(described_class.for_helm_with_channel(project, channel)).to contain_exactly(helm_file2) + end + end + describe '#update_file_store callback' do let_it_be(:package_file) { build(:package_file, :nuget, size: nil) } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 52ef61e3d44..1e44327c089 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -404,7 +404,8 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.not_to allow_value(nil).for(:version) } it { is_expected.not_to allow_value('').for(:version) } it { is_expected.to allow_value('v1.2.3').for(:version) } - it { is_expected.not_to allow_value('1.2.3').for(:version) } + it { is_expected.to allow_value('1.2.3').for(:version) } + it { is_expected.not_to allow_value('v1.2').for(:version) } end it_behaves_like 'validating version to be SemVer compliant for', :npm_package @@ -729,6 +730,38 @@ RSpec.describe Packages::Package, type: :model do end end + context 'sorting' do + let_it_be(:project) { create(:project, name: 'aaa' ) } + let_it_be(:project2) { create(:project, name: 'bbb' ) } + let_it_be(:package1) { create(:package, project: project ) } + let_it_be(:package2) { create(:package, project: project2 ) } + let_it_be(:package3) { create(:package, project: project2 ) } + let_it_be(:package4) { create(:package, project: project ) } + + it 'orders packages by their projects name ascending' do + expect(Packages::Package.order_project_name).to eq([package1, package4, package2, package3]) + end + + it 'orders packages by their projects name descending' do + expect(Packages::Package.order_project_name_desc).to eq([package2, package3, package1, package4]) + end + + shared_examples 'order_project_path scope' do + it 'orders packages by their projects path asc, then package id asc' do + expect(Packages::Package.order_project_path).to eq([package1, package4, package2, package3]) + end + end + + shared_examples 'order_project_path_desc scope' do + it 'orders packages by their projects path desc, then package id desc' do + expect(Packages::Package.order_project_path_desc).to eq([package3, package2, package4, package1]) + end + end + + it_behaves_like 'order_project_path scope' + it_behaves_like 'order_project_path_desc scope' + end + describe '.order_by_package_file' do let_it_be(:project) { create(:project) } let_it_be(:package1) { create(:maven_package, project: project) } @@ -743,6 +776,33 @@ RSpec.describe Packages::Package, type: :model do end end + describe '.keyset_pagination_order' do + let(:join_class) { nil } + let(:column_name) { nil } + let(:direction) { nil } + + subject { described_class.keyset_pagination_order(join_class: join_class, column_name: column_name, direction: direction) } + + it { expect { subject }.to raise_error(NoMethodError) } + + context 'with valid params' do + let(:join_class) { Project } + let(:column_name) { :name } + + context 'ascending direction' do + let(:direction) { :asc } + + it { is_expected.to eq('projects.name asc NULLS LAST, "packages_packages"."id" ASC') } + end + + context 'descending direction' do + let(:direction) { :desc } + + it { is_expected.to eq('projects.name desc NULLS FIRST, "packages_packages"."id" DESC') } + end + end + end + describe '#versions' do let_it_be(:project) { create(:project) } let_it_be(:package) { create(:maven_package, project: project) } @@ -838,6 +898,26 @@ RSpec.describe Packages::Package, type: :model do end end + describe '#infrastructure_package?' do + let(:package) { create(:package) } + + subject { package.infrastructure_package? } + + it { is_expected.to eq(false) } + + context 'with generic package' do + let(:package) { create(:generic_package) } + + it { is_expected.to eq(false) } + end + + context 'with terraform module package' do + let(:package) { create(:terraform_module_package) } + + it { is_expected.to eq(true) } + end + end + describe 'plan_limits' do Packages::Package.package_types.keys.without('composer').each do |pt| plan_limit_name = if pt == 'generic' diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 735f2225c21..2d7ee8ba3be 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -47,14 +47,7 @@ RSpec.describe Pages::LookupPath do describe '#source' do let(:source) { lookup_path.source } - it 'uses disk storage', :aggregate_failures do - expect(source[:type]).to eq('file') - expect(source[:path]).to eq(project.full_path + "/public/") - end - - it 'return nil when local storage is disabled and there is no deployment' do - allow(Settings.pages.local_store).to receive(:enabled).and_return(false) - + it 'returns nil' do expect(source).to eq(nil) end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 1d5369e608e..7b997f0d4e1 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -12,6 +12,15 @@ RSpec.describe PagesDomain do it { is_expected.to have_many(:serverless_domain_clusters) } end + describe '.for_project' do + it 'returns domains assigned to project' do + domain = create(:pages_domain, :with_project) + create(:pages_domain) # unrelated domain + + expect(described_class.for_project(domain.project)).to eq([domain]) + end + end + describe 'validate domain' do subject(:pages_domain) { build(:pages_domain, domain: domain) } @@ -655,25 +664,16 @@ RSpec.describe PagesDomain do end end - context 'when there are pages deployed for the project' do - before do - generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') - generic_commit_status.update!(project: project) - project.pages_metadatum.destroy! - project.reload - end + it 'returns the virual domain when there are pages deployed for the project' do + project.mark_pages_as_deployed + project.update_pages_deployment!(create(:pages_deployment, project: project)) - it 'returns the virual domain' do - expect(Pages::VirtualDomain).to receive(:new).with([project], domain: pages_domain).and_call_original + expect(Pages::VirtualDomain).to receive(:new).with([project], domain: pages_domain).and_call_original - expect(pages_domain.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain) - end + virtual_domain = pages_domain.pages_virtual_domain - it 'migrates project pages metadata' do - expect { pages_domain.pages_virtual_domain }.to change { - project.reload.pages_metadatum&.deployed - }.from(nil).to(true) - end + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index b8c723b3847..cf8e30023eb 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -211,6 +211,7 @@ RSpec.describe PlanLimits do storage_size_limit daily_invites web_hook_calls + ci_daily_pipeline_schedule_triggers ] + disabled_max_artifact_size_columns end diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index 02a4d783b84..4bad8a3f0c0 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -24,6 +24,10 @@ RSpec.describe Postgresql::ReplicationSlot do expect(described_class).to receive(:in_use?).and_return(true) end + it 'does not raise an exception' do + expect { described_class.lag_too_great? }.not_to raise_error + end + it 'returns true when replication lag is too great' do expect(described_class) .to receive(:pluck) diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 406485d8cc8..c206ba27ec1 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -21,6 +21,12 @@ RSpec.describe ProjectCiCdSetting do end end + describe '#job_token_scope_enabled' do + it 'is true by default' do + expect(described_class.new.job_token_scope_enabled).to be_truthy + end + end + describe '#default_git_depth' do let(:default_value) { described_class::DEFAULT_GIT_DEPTH } diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb index 4baa59535e4..6ef407432b0 100644 --- a/spec/models/project_feature_usage_spec.rb +++ b/spec/models/project_feature_usage_spec.rb @@ -126,4 +126,54 @@ RSpec.describe ProjectFeatureUsage, type: :model do end end end + + context 'ProjectFeatureUsage with DB Load Balancing', :request_store do + include_context 'clear DB Load Balancing configuration' + + describe '#log_jira_dvcs_integration_usage' do + let!(:project) { create(:project) } + + subject { project.feature_usage } + + context 'database load balancing is configured' do + before do + # Do not pollute AR for other tests, but rather simulate effect of configure_proxy. + allow(ActiveRecord::Base.singleton_class).to receive(:prepend) + ::Gitlab::Database::LoadBalancing.configure_proxy + allow(ActiveRecord::Base).to receive(:connection).and_return(::Gitlab::Database::LoadBalancing.proxy) + ::Gitlab::Database::LoadBalancing::Session.clear_session + end + + it 'logs Jira DVCS Cloud last sync' do + freeze_time do + subject.log_jira_dvcs_integration_usage + + expect(subject.jira_dvcs_server_last_sync_at).to be_nil + expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) + end + end + + it 'does not stick to primary' do + expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_performed_write + expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_using_primary + + subject.log_jira_dvcs_integration_usage + + expect(::Gitlab::Database::LoadBalancing::Session.current).to be_performed_write + expect(::Gitlab::Database::LoadBalancing::Session.current).not_to be_using_primary + end + end + + context 'database load balancing is not cofigured' do + it 'logs Jira DVCS Cloud last sync' do + freeze_time do + subject.log_jira_dvcs_integration_usage + + expect(subject.jira_dvcs_server_last_sync_at).to be_nil + expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) + end + end + end + end + end end diff --git a/spec/models/project_repository_storage_move_spec.rb b/spec/models/project_repository_storage_move_spec.rb deleted file mode 100644 index eb193a44680..00000000000 --- a/spec/models/project_repository_storage_move_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ProjectRepositoryStorageMove, type: :model do - let_it_be_with_refind(:project) { create(:project) } - - it_behaves_like 'handles repository moves' do - let(:container) { project } - let(:repository_storage_factory_key) { :project_repository_storage_move } - let(:error_key) { :project } - let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker } - end - - describe 'state transitions' do - let(:storage) { 'test_second_storage' } - - before do - stub_storage_settings(storage => { 'path' => 'tmp/tests/extra_storage' }) - end - - context 'when started' do - subject(:storage_move) { create(:project_repository_storage_move, :started, container: project, destination_storage_name: storage) } - - context 'and transits to replicated' do - it 'sets the repository storage and marks the container as writable' do - storage_move.finish_replication! - - expect(project.repository_storage).to eq(storage) - expect(project).not_to be_repository_read_only - end - end - end - end -end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb deleted file mode 100644 index 42368c31ba0..00000000000 --- a/spec/models/project_services/hipchat_service_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# HipchatService is partially removed and it will be remove completely -# after the deletion of all the database records. -# https://gitlab.com/gitlab-org/gitlab/-/issues/27954 -RSpec.describe HipchatService do - let_it_be(:project) { create(:project) } - - subject(:service) { described_class.new(project: project) } - - it { is_expected.to be_valid } - - describe '#to_param' do - subject { service.to_param } - - it { is_expected.to eq('hipchat') } - end - - describe '#supported_events' do - subject { service.supported_events } - - it { is_expected.to be_empty } - end - - describe '#save' do - it 'prevents records from being created or updated' do - expect(service.save).to be_falsey - - expect(service.errors.full_messages).to include( - 'HipChat endpoint is deprecated and should not be created or modified.' - ) - end - end -end diff --git a/spec/models/project_services/mattermost_service_spec.rb b/spec/models/project_services/mattermost_service_spec.rb deleted file mode 100644 index af1944ea77d..00000000000 --- a/spec/models/project_services/mattermost_service_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MattermostService do - it_behaves_like "slack or mattermost notifications", "Mattermost" -end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 37a6d49ff74..a2025388fab 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -323,9 +323,9 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl end describe '#prometheus_available?' do - context 'clusters with installed prometheus' do + context 'clusters with enabled prometheus' do before do - create(:clusters_applications_prometheus, :installed, cluster: cluster) + create(:clusters_integrations_prometheus, cluster: cluster) end context 'cluster belongs to project' do @@ -340,7 +340,7 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl let_it_be(:group) { create(:group) } let(:project) { create(:prometheus_project, group: group) } - let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) } + let(:cluster) { create(:cluster_for_group, groups: [group]) } it 'returns true' do expect(service.prometheus_available?).to be(true) @@ -349,8 +349,8 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl it 'avoids N+1 queries' do service 5.times do |i| - other_cluster = create(:cluster_for_group, :with_installed_helm, groups: [group], environment_scope: i) - create(:clusters_applications_prometheus, :installing, cluster: other_cluster) + other_cluster = create(:cluster_for_group, groups: [group], environment_scope: i) + create(:clusters_integrations_prometheus, cluster: other_cluster) end expect { service.prometheus_available? }.not_to exceed_query_limit(1) end @@ -365,18 +365,9 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl end end - context 'clusters with updated prometheus' do - let!(:cluster) { create(:cluster, projects: [project]) } - let!(:prometheus) { create(:clusters_applications_prometheus, :updated, cluster: cluster) } - - it 'returns true' do - expect(service.prometheus_available?).to be(true) - end - end - - context 'clusters without prometheus installed' do + context 'clusters with prometheus disabled' do let(:cluster) { create(:cluster, projects: [project]) } - let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } + let!(:prometheus) { create(:clusters_integrations_prometheus, :disabled, cluster: cluster) } it 'returns false' do expect(service.prometheus_available?).to be(false) @@ -491,13 +482,13 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl expect(service.editable?).to be(true) end - context 'when cluster exists with prometheus installed' do + context 'when cluster exists with prometheus enabled' do let(:cluster) { create(:cluster, projects: [project]) } before do service.update!(manual_configuration: false) - create(:clusters_applications_prometheus, :installed, cluster: cluster) + create(:clusters_integrations_prometheus, cluster: cluster) end it 'remains editable' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c57c2792f87..78e32571d7d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:project_members).dependent(:delete_all) } it { is_expected.to have_many(:users).through(:project_members) } it { is_expected.to have_many(:requesters).dependent(:delete_all) } - it { is_expected.to have_many(:notes) } + it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:snippets).class_name('ProjectSnippet') } it { is_expected.to have_many(:deploy_keys_projects) } it { is_expected.to have_many(:deploy_keys) } @@ -43,31 +43,31 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:webex_teams_service) } it { is_expected.to have_one(:packagist_service) } it { is_expected.to have_one(:pushover_service) } - it { is_expected.to have_one(:asana_service) } + it { is_expected.to have_one(:asana_integration) } it { is_expected.to have_many(:boards) } - it { is_expected.to have_one(:campfire_service) } - it { is_expected.to have_one(:datadog_service) } - it { is_expected.to have_one(:discord_service) } - it { is_expected.to have_one(:drone_ci_service) } + it { is_expected.to have_one(:campfire_integration) } + it { is_expected.to have_one(:datadog_integration) } + it { is_expected.to have_one(:discord_integration) } + it { is_expected.to have_one(:drone_ci_integration) } it { is_expected.to have_one(:emails_on_push_service) } it { is_expected.to have_one(:pipelines_email_service) } it { is_expected.to have_one(:irker_service) } it { is_expected.to have_one(:pivotaltracker_service) } it { is_expected.to have_one(:flowdock_service) } - it { is_expected.to have_one(:assembla_service) } + it { is_expected.to have_one(:assembla_integration) } it { is_expected.to have_one(:slack_slash_commands_service) } it { is_expected.to have_one(:mattermost_slash_commands_service) } - it { is_expected.to have_one(:buildkite_service) } - it { is_expected.to have_one(:bamboo_service) } + it { is_expected.to have_one(:buildkite_integration) } + it { is_expected.to have_one(:bamboo_integration) } it { is_expected.to have_one(:teamcity_service) } it { is_expected.to have_one(:jira_service) } it { is_expected.to have_one(:redmine_service) } it { is_expected.to have_one(:youtrack_service) } - it { is_expected.to have_one(:custom_issue_tracker_service) } - it { is_expected.to have_one(:bugzilla_service) } + it { is_expected.to have_one(:custom_issue_tracker_integration) } + it { is_expected.to have_one(:bugzilla_integration) } it { is_expected.to have_one(:ewm_service) } it { is_expected.to have_one(:external_wiki_service) } - it { is_expected.to have_one(:confluence_service) } + it { is_expected.to have_one(:confluence_integration) } it { is_expected.to have_one(:project_feature) } it { is_expected.to have_one(:project_repository) } it { is_expected.to have_one(:container_expiration_policy) } @@ -472,6 +472,23 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#merge_requests_author_approval' do + where(:attribute_value, :return_value) do + true | true + false | false + nil | false + end + + with_them do + let(:project) { create(:project, merge_requests_author_approval: attribute_value) } + + it 'returns expected value' do + expect(project.merge_requests_author_approval).to eq(return_value) + expect(project.merge_requests_author_approval?).to eq(return_value) + end + end + end + describe '#all_pipelines' do let_it_be(:project) { create(:project) } @@ -636,6 +653,16 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) } + it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) } + it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } + + context 'when read_container_registry_access_level is disabled' do + before do + stub_feature_flags(read_container_registry_access_level: false) + end + + it { is_expected.not_to delegate_method(:container_registry_enabled?).to(:project_feature) } + end end describe 'reference methods' do @@ -967,6 +994,39 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#open_issues_count', :aggregate_failures do + let(:project) { build(:project) } + + it 'provides the issue count' do + expect(project.open_issues_count).to eq 0 + end + + it 'invokes the count service with current_user' do + user = build(:user) + count_service = instance_double(Projects::OpenIssuesCountService) + expect(Projects::OpenIssuesCountService).to receive(:new).with(project, user).and_return(count_service) + expect(count_service).to receive(:count) + + project.open_issues_count(user) + end + + it 'invokes the count service with no current_user' do + count_service = instance_double(Projects::OpenIssuesCountService) + expect(Projects::OpenIssuesCountService).to receive(:new).with(project, nil).and_return(count_service) + expect(count_service).to receive(:count) + + project.open_issues_count + end + end + + describe '#open_merge_requests_count' do + it 'provides the merge request count' do + project = build(:project) + + expect(project.open_merge_requests_count).to eq 0 + end + end + describe '#issue_exists?' do let_it_be(:project) { create(:project) } @@ -1086,7 +1146,7 @@ RSpec.describe Project, factory_default: :keep do project = create(:redmine_project) expect(project).to receive(:integrations).once.and_call_original - 2.times { expect(project.external_issue_tracker).to be_a_kind_of(RedmineService) } + 2.times { expect(project.external_issue_tracker).to be_a_kind_of(Integrations::Redmine) } end end @@ -1158,7 +1218,7 @@ RSpec.describe Project, factory_default: :keep do it 'returns an active external wiki' do create(:service, project: project, type: 'ExternalWikiService', active: true) - is_expected.to be_kind_of(ExternalWikiService) + is_expected.to be_kind_of(Integrations::ExternalWiki) end it 'does not return an inactive external wiki' do @@ -1584,19 +1644,20 @@ RSpec.describe Project, factory_default: :keep do end end - describe '.find_by_service_desk_project_key' do - it 'returns the correct project' do + describe '.with_service_desk_key' do + it 'returns projects with given key' do project1 = create(:project) project2 = create(:project) create(:service_desk_setting, project: project1, project_key: 'key1') - create(:service_desk_setting, project: project2, project_key: 'key2') + create(:service_desk_setting, project: project2, project_key: 'key1') + create(:service_desk_setting, project_key: 'key2') + create(:service_desk_setting) - expect(Project.find_by_service_desk_project_key('key1')).to eq(project1) - expect(Project.find_by_service_desk_project_key('key2')).to eq(project2) + expect(Project.with_service_desk_key('key1')).to contain_exactly(project1, project2) end - it 'returns nil if there is no project with the key' do - expect(Project.find_by_service_desk_project_key('some_key')).to be_nil + it 'returns empty if there is no project with the key' do + expect(Project.with_service_desk_key('key1')).to be_empty end end @@ -1632,112 +1693,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#any_active_runners?' do - subject { project.any_active_runners? } - - context 'shared runners' do - let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } - let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } - let(:shared_runner) { create(:ci_runner, :instance) } - - context 'for shared runners disabled' do - let(:shared_runners_enabled) { false } - - it 'has no runners available' do - is_expected.to be_falsey - end - - it 'has a specific runner' do - specific_runner - - is_expected.to be_truthy - end - - it 'has a shared runner, but they are prohibited to use' do - shared_runner - - is_expected.to be_falsey - end - - it 'checks the presence of specific runner' do - specific_runner - - expect(project.any_active_runners? { |runner| runner == specific_runner }).to be_truthy - end - - it 'returns false if match cannot be found' do - specific_runner - - expect(project.any_active_runners? { false }).to be_falsey - end - end - - context 'for shared runners enabled' do - let(:shared_runners_enabled) { true } - - it 'has a shared runner' do - shared_runner - - is_expected.to be_truthy - end - - it 'checks the presence of shared runner' do - shared_runner - - expect(project.any_active_runners? { |runner| runner == shared_runner }).to be_truthy - end - - it 'returns false if match cannot be found' do - shared_runner - - expect(project.any_active_runners? { false }).to be_falsey - end - end - end - - context 'group runners' do - let(:project) { create(:project, group_runners_enabled: group_runners_enabled) } - let(:group) { create(:group, projects: [project]) } - let(:group_runner) { create(:ci_runner, :group, groups: [group]) } - - context 'for group runners disabled' do - let(:group_runners_enabled) { false } - - it 'has no runners available' do - is_expected.to be_falsey - end - - it 'has a group runner, but they are prohibited to use' do - group_runner - - is_expected.to be_falsey - end - end - - context 'for group runners enabled' do - let(:group_runners_enabled) { true } - - it 'has a group runner' do - group_runner - - is_expected.to be_truthy - end - - it 'checks the presence of group runner' do - group_runner - - expect(project.any_active_runners? { |runner| runner == group_runner }).to be_truthy - end - - it 'returns false if match cannot be found' do - group_runner - - expect(project.any_active_runners? { false }).to be_falsey - end - end - end - end - describe '#any_online_runners?' do subject { project.any_online_runners? } @@ -1858,13 +1813,13 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#shared_runners' do - let!(:runner) { create(:ci_runner, :instance) } + shared_examples 'shared_runners' do + let_it_be(:runner) { create(:ci_runner, :instance) } subject { project.shared_runners } context 'when shared runners are enabled for project' do - let!(:project) { create(:project, shared_runners_enabled: true) } + let(:project) { build_stubbed(:project, shared_runners_enabled: true) } it "returns a list of shared runners" do is_expected.to eq([runner]) @@ -1872,7 +1827,7 @@ RSpec.describe Project, factory_default: :keep do end context 'when shared runners are disabled for project' do - let!(:project) { create(:project, shared_runners_enabled: false) } + let(:project) { build_stubbed(:project, shared_runners_enabled: false) } it "returns a empty list" do is_expected.to be_empty @@ -1880,6 +1835,16 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#shared_runners' do + it_behaves_like 'shared_runners' + end + + describe '#available_shared_runners' do + it_behaves_like 'shared_runners' do + subject { project.available_shared_runners } + end + end + describe '#visibility_level' do let(:project) { build(:project) } @@ -2238,13 +2203,13 @@ RSpec.describe Project, factory_default: :keep do end context 'with projects on legacy storage' do - let(:project) { create(:project, :repository, :legacy_storage) } + let(:project) { create(:project, :legacy_storage) } it_behaves_like 'tracks storage location' end context 'with projects on hashed storage' do - let(:project) { create(:project, :repository) } + let(:project) { create(:project) } it_behaves_like 'tracks storage location' end @@ -2363,35 +2328,55 @@ RSpec.describe Project, factory_default: :keep do it 'updates project_feature', :aggregate_failures do # Simulate an existing project that has container_registry enabled project.update_column(:container_registry_enabled, true) - project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) - - expect(project.container_registry_enabled).to eq(true) - expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED) project.update!(container_registry_enabled: false) - expect(project.container_registry_enabled).to eq(false) + expect(project.read_attribute(:container_registry_enabled)).to eq(false) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED) project.update!(container_registry_enabled: true) - expect(project.container_registry_enabled).to eq(true) + expect(project.read_attribute(:container_registry_enabled)).to eq(true) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::ENABLED) end it 'rollsback both projects and project_features row in case of error', :aggregate_failures do project.update_column(:container_registry_enabled, true) - project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) - - expect(project.container_registry_enabled).to eq(true) - expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED) allow(project).to receive(:valid?).and_return(false) expect { project.update!(container_registry_enabled: false) }.to raise_error(ActiveRecord::RecordInvalid) - expect(project.reload.container_registry_enabled).to eq(true) - expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::DISABLED) + expect(project.reload.read_attribute(:container_registry_enabled)).to eq(true) + expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::ENABLED) + end + end + + describe '#container_registry_enabled' do + let_it_be_with_reload(:project) { create(:project) } + + it 'delegates to project_feature', :aggregate_failures do + project.update_column(:container_registry_enabled, true) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) + + expect(project.container_registry_enabled).to eq(false) + expect(project.container_registry_enabled?).to eq(false) + end + + context 'with read_container_registry_access_level disabled' do + before do + stub_feature_flags(read_container_registry_access_level: false) + end + + it 'reads project.container_registry_enabled' do + project.update_column(:container_registry_enabled, true) + project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) + + expect(project.container_registry_enabled).to eq(true) + expect(project.container_registry_enabled?).to eq(true) + end end end @@ -2932,6 +2917,16 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#mark_primary_write_location' do + let(:project) { create(:project) } + + it 'marks the location with project ID' do + expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:mark_primary_write_location).with(:project, project.id) + + project.mark_primary_write_location + end + end + describe '#mark_stuck_remote_mirrors_as_failed!' do it 'fails stuck remote mirrors' do project = create(:project, :repository, :remote_mirror) @@ -4414,6 +4409,18 @@ RSpec.describe Project, factory_default: :keep do end end + context 'with export' do + let(:project) { create(:project, :with_export) } + + it '#export_file_exists? returns true' do + expect(project.export_file_exists?).to be true + end + + it '#export_archive_exists? returns false' do + expect(project.export_archive_exists?).to be true + end + end + describe '#forks_count' do it 'returns the number of forks' do project = build(:project) @@ -4692,7 +4699,6 @@ RSpec.describe Project, factory_default: :keep do specify do expect(subject).to include [ - { key: 'CI_PROJECT_CONFIG_PATH', value: Ci::Pipeline::DEFAULT_CONFIG_PATH, public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: Ci::Pipeline::DEFAULT_CONFIG_PATH, public: true, masked: false } ] end @@ -4705,7 +4711,6 @@ RSpec.describe Project, factory_default: :keep do it do expect(subject).to include [ - { key: 'CI_PROJECT_CONFIG_PATH', value: 'random.yml', public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: 'random.yml', public: true, masked: false } ] end @@ -5328,7 +5333,7 @@ RSpec.describe Project, factory_default: :keep do it 'executes services with the specified scope' do data = 'any data' - expect_next_found_instance_of(SlackService) do |instance| + expect_next_found_instance_of(Integrations::Slack) do |instance| expect(instance).to receive(:async_execute).with(data).once end @@ -5336,7 +5341,7 @@ RSpec.describe Project, factory_default: :keep do end it 'does not execute services that don\'t match the specified scope' do - expect(SlackService).not_to receive(:allocate).and_wrap_original do |method| + expect(Integrations::Slack).not_to receive(:allocate).and_wrap_original do |method| method.call.tap do |instance| expect(instance).not_to receive(:async_execute) end @@ -5379,7 +5384,7 @@ RSpec.describe Project, factory_default: :keep do it { expect(project.has_active_services?).to be_falsey } it 'returns true when a matching service exists' do - create(:custom_issue_tracker_service, push_events: true, merge_requests_events: false, project: project) + create(:custom_issue_tracker_integration, push_events: true, merge_requests_events: false, project: project) expect(project.has_active_services?(:merge_request_hooks)).to be_falsey expect(project.has_active_services?).to be_truthy @@ -6307,14 +6312,16 @@ RSpec.describe Project, factory_default: :keep do let_it_be(:project) { create(:project, group: create(:group, :public)) } it 'returns a maximum of ten maintainers of the project in recent_sign_in descending order' do - users = create_list(:user, 12, :with_sign_ins) + limit = 2 + stub_const("Member::ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT", limit) + users = create_list(:user, limit + 1, :with_sign_ins) active_maintainers = users.map do |user| create(:project_member, :maintainer, user: user, project: project) end active_maintainers_in_recent_sign_in_desc_order = project.members_and_requesters .id_in(active_maintainers) - .order_recent_sign_in.limit(10) + .order_recent_sign_in.limit(limit) expect(project.access_request_approvers_to_be_notified).to eq(active_maintainers_in_recent_sign_in_desc_order) end @@ -6558,6 +6565,14 @@ RSpec.describe Project, factory_default: :keep do expect(subject).to eq([lfs_object.oid]) end end + + it 'lfs_objects_projects associations are deleted along with project' do + expect { project.delete }.to change(LfsObjectsProject, :count).by(-2) + end + + it 'lfs_objects associations are unchanged when the assicated project is removed' do + expect { project.delete }.not_to change(LfsObject, :count) + end end context 'when project has no associated LFS objects' do @@ -6666,7 +6681,7 @@ RSpec.describe Project, factory_default: :keep do context 'when project export is completed' do before do finish_job(project_export_job) - allow(project).to receive(:export_file).and_return(double(ImportExportUploader, file: 'exists.zip')) + allow(project).to receive(:export_file_exists?).and_return(true) end it { expect(project.export_status).to eq :finished } @@ -6677,7 +6692,7 @@ RSpec.describe Project, factory_default: :keep do before do finish_job(project_export_job) - allow(project).to receive(:export_file).and_return(double(ImportExportUploader, file: 'exists.zip')) + allow(project).to receive(:export_file_exists?).and_return(true) end it { expect(project.export_status).to eq :regeneration_in_progress } @@ -6973,7 +6988,7 @@ RSpec.describe Project, factory_default: :keep do end describe 'topics' do - let_it_be(:project) { create(:project, tag_list: 'topic1, topic2, topic3') } + let_it_be(:project) { create(:project, topic_list: 'topic1, topic2, topic3') } it 'topic_list returns correct string array' do expect(project.topic_list).to match_array(%w[topic1 topic2 topic3]) @@ -6983,44 +6998,69 @@ RSpec.describe Project, factory_default: :keep do expect(project.topics.first.class.name).to eq('ActsAsTaggableOn::Tag') expect(project.topics.map(&:name)).to match_array(%w[topic1 topic2 topic3]) end + end - context 'aliases' do - it 'tag_list returns correct string array' do - expect(project.tag_list).to match_array(%w[topic1 topic2 topic3]) + shared_examples 'all_runners' do + let_it_be_with_refind(:project) { create(:project, group: create(:group)) } + let_it_be(:instance_runner) { create(:ci_runner, :instance) } + let_it_be(:group_runner) { create(:ci_runner, :group, groups: [project.group]) } + let_it_be(:other_group_runner) { create(:ci_runner, :group) } + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:other_project_runner) { create(:ci_runner, :project) } + + subject { project.all_runners } + + context 'when shared runners are enabled for project' do + before do + project.update!(shared_runners_enabled: true) end - it 'tags returns correct tag records' do - expect(project.tags.first.class.name).to eq('ActsAsTaggableOn::Tag') - expect(project.tags.map(&:name)).to match_array(%w[topic1 topic2 topic3]) + it 'returns a list with all runners' do + is_expected.to match_array([instance_runner, group_runner, project_runner]) end end - context 'intermediate state during background migration' do + context 'when shared runners are disabled for project' do before do - project.taggings.first.update!(context: 'tags') - project.instance_variable_set("@tag_list", nil) - project.reload + project.update!(shared_runners_enabled: false) end - it 'tag_list returns string array including old and new topics' do - expect(project.tag_list).to match_array(%w[topic1 topic2 topic3]) + it 'returns a list without shared runners' do + is_expected.to match_array([group_runner, project_runner]) + end + end + + context 'when group runners are enabled for project' do + before do + project.update!(group_runners_enabled: true) end - it 'tags returns old and new tag records' do - expect(project.tags.first.class.name).to eq('ActsAsTaggableOn::Tag') - expect(project.tags.map(&:name)).to match_array(%w[topic1 topic2 topic3]) - expect(project.taggings.map(&:context)).to match_array(%w[tags topics topics]) + it 'returns a list with all runners' do + is_expected.to match_array([instance_runner, group_runner, project_runner]) end + end - it 'update tag_list adds new topics and removes old topics' do - project.update!(tag_list: 'topic1, topic2, topic3, topic4') + context 'when group runners are disabled for project' do + before do + project.update!(group_runners_enabled: false) + end - expect(project.tags.map(&:name)).to match_array(%w[topic1 topic2 topic3 topic4]) - expect(project.taggings.map(&:context)).to match_array(%w[topics topics topics topics]) + it 'returns a list without group runners' do + is_expected.to match_array([instance_runner, project_runner]) end end end + describe '#all_runners' do + it_behaves_like 'all_runners' + end + + describe '#all_available_runners' do + it_behaves_like 'all_runners' do + subject { project.all_available_runners } + end + end + def finish_job(export_job) export_job.start export_job.finish diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index b4dff4c33ff..a5441e2f47b 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -67,12 +67,12 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do expect(subject[:next_page]).to eq(2) end - it 'parses the body as markdown and returns html' do - expect(subject[:items].first['body']).to match("<h2 id=\"bright-and-sunshinin-day\">bright and sunshinin’ day</h2>") + it 'parses the body as markdown and returns html, and links are target="_blank"' do + expect(subject[:items].first['body']).to match('<p data-sourcepos="1:1-1:62" dir="auto">bright and sunshinin\' <a href="https://en.wikipedia.org/wiki/Day" rel="nofollow noreferrer noopener" target="_blank">day</a></p>') end it 'logs an error if theres an error parsing markdown for an item, and skips it' do - allow(Kramdown::Document).to receive(:new).and_raise + allow(Banzai).to receive(:render).and_raise expect(Gitlab::ErrorTracking).to receive(:track_exception) expect(subject[:items]).to be_empty diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7748846f6a5..b6f09babb4b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1123,6 +1123,70 @@ RSpec.describe Repository do end end + describe '#fetch_as_mirror' do + let(:url) { "http://example.com" } + + context 'when :fetch_remote_params is enabled' do + let(:remote_name) { "remote-name" } + + before do + stub_feature_flags(fetch_remote_params: true) + end + + it 'fetches the URL without creating a remote' do + expect(repository).not_to receive(:add_remote) + expect(repository) + .to receive(:fetch_remote) + .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs) + .and_return(nil) + + repository.fetch_as_mirror(url, remote_name: remote_name) + end + end + + context 'when :fetch_remote_params is disabled' do + before do + stub_feature_flags(fetch_remote_params: false) + end + + shared_examples 'a fetch' do + it 'adds and fetches a remote' do + expect(repository) + .to receive(:add_remote) + .with(expected_remote, url, mirror_refmap: :all_refs) + .and_return(nil) + expect(repository) + .to receive(:fetch_remote) + .with(expected_remote, forced: false, prune: true) + .and_return(nil) + + repository.fetch_as_mirror(url, remote_name: remote_name) + end + end + + context 'with temporary remote' do + let(:remote_name) { nil } + let(:expected_remote_suffix) { "123456" } + let(:expected_remote) { "tmp-#{expected_remote_suffix}" } + + before do + expect(repository) + .to receive(:async_remove_remote).with(expected_remote).and_return(nil) + allow(SecureRandom).to receive(:hex).and_return(expected_remote_suffix) + end + + it_behaves_like 'a fetch' + end + + context 'with remote name' do + let(:remote_name) { "foo" } + let(:expected_remote) { "foo" } + + it_behaves_like 'a fetch' + end + end + end + describe '#fetch_ref' do let(:broken_repository) { create(:project, :broken_storage).repository } diff --git a/spec/models/service_desk_setting_spec.rb b/spec/models/service_desk_setting_spec.rb index ca57a5d4087..8ccbd983ba1 100644 --- a/spec/models/service_desk_setting_spec.rb +++ b/spec/models/service_desk_setting_spec.rb @@ -31,6 +31,37 @@ RSpec.describe ServiceDeskSetting do end end + describe '.valid_project_key' do + # Creates two projects with same full path slug + # group1/test/one and group1/test-one will both have 'group-test-one' slug + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group, name: 'test') } + let_it_be(:project1) { create(:project, name: 'test-one', group: group) } + let_it_be(:project2) { create(:project, name: 'one', group: subgroup) } + let_it_be(:project_key) { 'key' } + + before_all do + create(:service_desk_setting, project: project1, project_key: project_key) + end + + context 'when project_key is unique for every project slug' do + it 'does not add error' do + settings = build(:service_desk_setting, project: project2, project_key: 'otherkey') + + expect(settings).to be_valid + end + end + + context 'when project with same slug and settings project_key exists' do + it 'adds error' do + settings = build(:service_desk_setting, project: project2, project_key: project_key) + + expect(settings).to be_invalid + expect(settings.errors[:project_key].first).to eq('already in use for another service desk address.') + end + end + end + describe 'associations' do it { is_expected.to belong_to(:project) } end diff --git a/spec/models/snippet_repository_storage_move_spec.rb b/spec/models/snippet_repository_storage_move_spec.rb deleted file mode 100644 index f5ad837fb36..00000000000 --- a/spec/models/snippet_repository_storage_move_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe SnippetRepositoryStorageMove, type: :model do - it_behaves_like 'handles repository moves' do - let_it_be_with_refind(:container) { create(:snippet) } - - let(:repository_storage_factory_key) { :snippet_repository_storage_move } - let(:error_key) { :snippet } - let(:repository_storage_worker) { Snippets::UpdateRepositoryStorageWorker } - end -end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 41991821922..06e9899c0bd 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -750,7 +750,7 @@ RSpec.describe Snippet do end it 'returns an empty array' do - expect(subject).to eq [] + expect(subject).to be_empty end end end diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index c3432907112..bc042f7a639 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -64,25 +64,57 @@ RSpec.describe Timelog do let_it_be(:subgroup_issue) { create(:issue, project: subgroup_project) } let_it_be(:subgroup_merge_request) { create(:merge_request, source_project: subgroup_project) } - let_it_be(:timelog) { create(:issue_timelog, spent_at: 65.days.ago) } - let_it_be(:timelog1) { create(:issue_timelog, spent_at: 15.days.ago, issue: group_issue) } - let_it_be(:timelog2) { create(:issue_timelog, spent_at: 5.days.ago, issue: subgroup_issue) } - let_it_be(:timelog3) { create(:merge_request_timelog, spent_at: 65.days.ago) } - let_it_be(:timelog4) { create(:merge_request_timelog, spent_at: 15.days.ago, merge_request: group_merge_request) } - let_it_be(:timelog5) { create(:merge_request_timelog, spent_at: 5.days.ago, merge_request: subgroup_merge_request) } - - describe 'in_group' do + let_it_be(:short_time_ago) { 5.days.ago } + let_it_be(:medium_time_ago) { 15.days.ago } + let_it_be(:long_time_ago) { 65.days.ago } + + let_it_be(:timelog) { create(:issue_timelog, spent_at: long_time_ago) } + let_it_be(:timelog1) { create(:issue_timelog, spent_at: medium_time_ago, issue: group_issue) } + let_it_be(:timelog2) { create(:issue_timelog, spent_at: short_time_ago, issue: subgroup_issue) } + let_it_be(:timelog3) { create(:merge_request_timelog, spent_at: long_time_ago) } + let_it_be(:timelog4) { create(:merge_request_timelog, spent_at: medium_time_ago, merge_request: group_merge_request) } + let_it_be(:timelog5) { create(:merge_request_timelog, spent_at: short_time_ago, merge_request: subgroup_merge_request) } + + describe '.in_group' do it 'return timelogs created for group issues and merge requests' do expect(described_class.in_group(group)).to contain_exactly(timelog1, timelog2, timelog4, timelog5) end end - describe 'between_times' do - it 'returns collection of timelogs within given times' do - timelogs = described_class.between_times(20.days.ago, 10.days.ago) + describe '.at_or_after' do + it 'returns timelogs at the time limit' do + timelogs = described_class.at_or_after(short_time_ago) - expect(timelogs).to contain_exactly(timelog1, timelog4) + expect(timelogs).to contain_exactly(timelog2, timelog5) end + + it 'returns timelogs after given time' do + timelogs = described_class.at_or_after(just_before(short_time_ago)) + + expect(timelogs).to contain_exactly(timelog2, timelog5) + end + end + + describe '.at_or_before' do + it 'returns timelogs at the time limit' do + timelogs = described_class.at_or_before(long_time_ago) + + expect(timelogs).to contain_exactly(timelog, timelog3) + end + + it 'returns timelogs before given time' do + timelogs = described_class.at_or_before(just_after(long_time_ago)) + + expect(timelogs).to contain_exactly(timelog, timelog3) + end + end + + def just_before(time) + time - 1.day + end + + def just_after(time) + time + 1.day end end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index caa0a886abf..651e2cf273f 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -376,6 +376,18 @@ RSpec.describe Todo do end end + describe '.for_note' do + it 'returns todos that belongs to notes' do + note_1 = create(:note, noteable: issue, project: issue.project) + note_2 = create(:note, noteable: issue, project: issue.project) + todo_1 = create(:todo, note: note_1) + todo_2 = create(:todo, note: note_2) + create(:todo, note: create(:note)) + + expect(described_class.for_note([note_1, note_2])).to contain_exactly(todo_1, todo_2) + end + end + describe '.group_by_user_id_and_state' do let_it_be(:user1) { create(:user) } let_it_be(:user2) { create(:user) } diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 041af5b9c31..c2d9b916a1c 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -11,6 +11,11 @@ RSpec.describe UserDetail do it { is_expected.to validate_length_of(:job_title).is_at_most(200) } end + describe '#pronouns' do + it { is_expected.not_to validate_presence_of(:pronouns) } + it { is_expected.to validate_length_of(:pronouns).is_at_most(50) } + end + describe '#bio' do it { is_expected.to validate_length_of(:bio).is_at_most(255) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cb34917f073..f1c30a646f5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -22,6 +22,8 @@ RSpec.describe User do describe 'constants' do it { expect(described_class::COUNT_CACHE_VALIDITY_PERIOD).to be_a(Integer) } + it { expect(described_class::MAX_USERNAME_LENGTH).to be_a(Integer) } + it { expect(described_class::MIN_USERNAME_LENGTH).to be_a(Integer) } end describe 'delegations' do @@ -72,6 +74,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:job_title).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:job_title=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:pronouns).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:pronouns=).to(:user_detail).with_arguments(:args).allow_nil } + it { is_expected.to delegate_method(:bio).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:bio=).to(:user_detail).with_arguments(:args).allow_nil } it { is_expected.to delegate_method(:bio_html).to(:user_detail).allow_nil } @@ -90,7 +95,7 @@ RSpec.describe User do it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } - it { is_expected.to have_many(:expired_today_and_unnotified_keys) } + it { is_expected.to have_many(:expired_and_unnotified_keys) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:events).dependent(:delete_all) } @@ -134,6 +139,12 @@ RSpec.describe User do expect(user.bio).to eq(user.user_detail.bio) end + it 'delegates `pronouns` to `user_detail`' do + user = create(:user, pronouns: 'they/them') + + expect(user.pronouns).to eq(user.user_detail.pronouns) + end + it 'creates `user_detail` when `bio` is first updated' do user = create(:user) @@ -1025,12 +1036,6 @@ RSpec.describe User do let_it_be(:expiring_soon_not_notified) { create(:key, expires_at: 2.days.from_now, user: user2) } let_it_be(:expiring_soon_notified) { create(:key, expires_at: 2.days.from_now, user: user1, before_expiry_notification_delivered_at: Time.current) } - describe '.with_ssh_key_expired_today' do - it 'returns users whose key has expired today' do - expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1) - end - end - describe '.with_ssh_key_expiring_soon' do it 'returns users whose keys will expire soon' do expect(described_class.with_ssh_key_expiring_soon).to contain_exactly(user2) @@ -4258,45 +4263,16 @@ RSpec.describe User do end describe '#invalidate_issue_cache_counts' do - let_it_be(:user) { create(:user) } - - subject do - user.invalidate_issue_cache_counts - user.save! - end - - shared_examples 'invalidates the cached value' do - it 'invalidates cache for issue counter' do - expect(Rails.cache).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count']) - - subject - end - end - - it_behaves_like 'invalidates the cached value' - - context 'if feature flag assigned_open_issues_cache is enabled' do - it 'calls the recalculate worker' do - expect(Users::UpdateOpenIssueCountWorker).to receive(:perform_async).with(user.id) - - subject - end - - it_behaves_like 'invalidates the cached value' - end + let(:user) { build_stubbed(:user) } - context 'if feature flag assigned_open_issues_cache is disabled' do - before do - stub_feature_flags(assigned_open_issues_cache: false) - end + it 'invalidates cache for issue counter' do + cache_mock = double - it 'does not call the recalculate worker' do - expect(Users::UpdateOpenIssueCountWorker).not_to receive(:perform_async).with(user.id) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count']) - subject - end + allow(Rails).to receive(:cache).and_return(cache_mock) - it_behaves_like 'invalidates the cached value' + user.invalidate_issue_cache_counts end end @@ -5272,9 +5248,10 @@ RSpec.describe User do let_it_be(:user3) { create(:user, :ghost) } let_it_be(:user4) { create(:user, user_type: :support_bot) } let_it_be(:user5) { create(:user, state: 'blocked', user_type: :support_bot) } + let_it_be(:user6) { create(:user, user_type: :automation_bot) } it 'returns all active users including active bots but ghost users' do - expect(described_class.active_without_ghosts).to match_array([user1, user4]) + expect(described_class.active_without_ghosts).to match_array([user1, user4, user6]) end end @@ -5409,7 +5386,8 @@ RSpec.describe User do { user_type: :ghost }, { user_type: :alert_bot }, { user_type: :support_bot }, - { user_type: :security_bot } + { user_type: :security_bot }, + { user_type: :automation_bot } ] end @@ -5465,6 +5443,7 @@ RSpec.describe User do 'alert_bot' | false 'support_bot' | false 'security_bot' | false + 'automation_bot' | false end with_them do @@ -5612,10 +5591,12 @@ RSpec.describe User do it_behaves_like 'bot users', :migration_bot it_behaves_like 'bot users', :security_bot it_behaves_like 'bot users', :ghost + it_behaves_like 'bot users', :automation_bot it_behaves_like 'bot user avatars', :alert_bot, 'alert-bot.png' it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' + it_behaves_like 'bot user avatars', :automation_bot, 'support-bot.png' context 'when bot is the support_bot' do subject { described_class.support_bot } |