diff options
Diffstat (limited to 'spec/models')
60 files changed, 2598 insertions, 1240 deletions
diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb index ddd65e723eb..f88a66a7c27 100644 --- a/spec/models/alert_management/http_integration_spec.rb +++ b/spec/models/alert_management/http_integration_spec.rb @@ -81,6 +81,32 @@ RSpec.describe AlertManagement::HttpIntegration do end end + describe 'before validation' do + describe '#ensure_payload_example_not_nil' do + subject(:integration) { build(:alert_management_http_integration, payload_example: payload_example) } + + context 'when the payload_example is nil' do + let(:payload_example) { nil } + + it 'sets the payload_example to empty JSON' do + integration.valid? + + expect(integration.payload_example).to eq({}) + end + end + + context 'when the payload_example is not nil' do + let(:payload_example) { { 'key' => 'value' } } + + it 'sets the payload_example to specified value' do + integration.valid? + + expect(integration.payload_example).to eq(payload_example) + end + end + end + end + describe '#token' do subject { integration.token } diff --git a/spec/models/analytics/instance_statistics/measurement_spec.rb b/spec/models/analytics/usage_trends/measurement_spec.rb index dbb16c5ffbe..d9a6b70c87a 100644 --- a/spec/models/analytics/instance_statistics/measurement_spec.rb +++ b/spec/models/analytics/usage_trends/measurement_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do +RSpec.describe Analytics::UsageTrends::Measurement, type: :model do describe 'validation' do - let!(:measurement) { create(:instance_statistics_measurement) } + let!(:measurement) { create(:usage_trends_measurement) } it { is_expected.to validate_presence_of(:recorded_at) } it { is_expected.to validate_presence_of(:identifier) } @@ -33,9 +33,9 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do end describe 'scopes' do - let_it_be(:measurement_1) { create(:instance_statistics_measurement, :project_count, recorded_at: 10.days.ago) } - let_it_be(:measurement_2) { create(:instance_statistics_measurement, :project_count, recorded_at: 2.days.ago) } - let_it_be(:measurement_3) { create(:instance_statistics_measurement, :group_count, recorded_at: 5.days.ago) } + let_it_be(:measurement_1) { create(:usage_trends_measurement, :project_count, recorded_at: 10.days.ago) } + let_it_be(:measurement_2) { create(:usage_trends_measurement, :project_count, recorded_at: 2.days.ago) } + let_it_be(:measurement_3) { create(:usage_trends_measurement, :group_count, recorded_at: 5.days.ago) } describe '.order_by_latest' do subject { described_class.order_by_latest } @@ -101,15 +101,15 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do describe '.find_latest_or_fallback' do subject(:count) { described_class.find_latest_or_fallback(:pipelines_skipped).count } - context 'with instance statistics' do - let!(:measurement) { create(:instance_statistics_measurement, :pipelines_skipped_count) } + context 'with usage statistics' do + let!(:measurement) { create(:usage_trends_measurement, :pipelines_skipped_count) } it 'returns the latest stored measurement' do expect(count).to eq measurement.count end end - context 'without instance statistics' do + context 'without usage statistics' do it 'returns the realtime query of the measurement' do expect(count).to eq 0 end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 5658057f588..808932ce7e4 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -105,14 +105,14 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value(false).for(:hashed_storage_enabled) } - it { is_expected.not_to allow_value(101).for(:repository_storages_weighted_default) } - it { is_expected.to allow_value('90').for(:repository_storages_weighted_default) } - it { is_expected.not_to allow_value(-1).for(:repository_storages_weighted_default) } - it { is_expected.to allow_value(100).for(:repository_storages_weighted_default) } - it { is_expected.to allow_value(0).for(:repository_storages_weighted_default) } - it { is_expected.to allow_value(50).for(:repository_storages_weighted_default) } - it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) } - it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) } + it { is_expected.to allow_value('default' => 0).for(:repository_storages_weighted) } + it { is_expected.to allow_value('default' => 50).for(:repository_storages_weighted) } + it { is_expected.to allow_value('default' => 100).for(:repository_storages_weighted) } + it { is_expected.to allow_value('default' => '90').for(:repository_storages_weighted) } + it { is_expected.to allow_value('default' => nil).for(:repository_storages_weighted) } + it { is_expected.not_to allow_value('default' => -1).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } + it { is_expected.not_to allow_value('default' => 101).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } + it { is_expected.not_to allow_value('default' => 100, shouldntexist: 50).for(:repository_storages_weighted).with_message("can't include: shouldntexist") } it { is_expected.to allow_value(400).for(:notes_create_limit) } it { is_expected.not_to allow_value('two').for(:notes_create_limit) } @@ -377,7 +377,7 @@ RSpec.describe ApplicationSetting do end end - it_behaves_like 'an object with email-formated attributes', :abuse_notification_email do + it_behaves_like 'an object with email-formatted attributes', :abuse_notification_email do subject { setting } end @@ -984,12 +984,6 @@ RSpec.describe ApplicationSetting do it_behaves_like 'application settings examples' - describe 'repository_storages_weighted_attributes' do - it 'returns the keys for repository_storages_weighted' do - expect(subject.class.repository_storages_weighted_attributes).to eq([:repository_storages_weighted_default]) - end - end - describe 'kroki_format_supported?' do it 'returns true when Excalidraw is enabled' do subject.kroki_formats_excalidraw = true @@ -1033,11 +1027,4 @@ RSpec.describe ApplicationSetting do expect(subject.kroki_formats_excalidraw).to eq(true) end end - - it 'does not allow to set weight for non existing storage' do - setting.repository_storages_weighted = { invalid_storage: 100 } - - expect(setting).not_to be_valid - expect(setting.errors.messages[:repository_storages_weighted]).to match_array(["can't include: invalid_storage"]) - end end diff --git a/spec/models/board_spec.rb b/spec/models/board_spec.rb index d309b4dbdb9..c8a9504d4fc 100644 --- a/spec/models/board_spec.rb +++ b/spec/models/board_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Board do end describe 'validations' do + it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:project) } end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index e5ab96ca514..17ab4d5954c 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -14,7 +14,6 @@ RSpec.describe BulkImports::Entity, type: :model do it { is_expected.to validate_presence_of(:source_type) } it { is_expected.to validate_presence_of(:source_full_path) } it { is_expected.to validate_presence_of(:destination_name) } - it { is_expected.to validate_presence_of(:destination_namespace) } it { is_expected.to define_enum_for(:source_type).with_values(%i[group_entity project_entity]) } @@ -38,7 +37,11 @@ RSpec.describe BulkImports::Entity, type: :model do context 'when associated with a group and no project' do it 'is valid as a group_entity' do entity = build(:bulk_import_entity, :group_entity, group: build(:group), project: nil) + expect(entity).to be_valid + end + it 'is valid when destination_namespace is empty' do + entity = build(:bulk_import_entity, :group_entity, group: build(:group), project: nil, destination_namespace: '') expect(entity).to be_valid end @@ -57,6 +60,12 @@ RSpec.describe BulkImports::Entity, type: :model do expect(entity).to be_valid end + it 'is invalid when destination_namespace is nil' do + entity = build(:bulk_import_entity, :group_entity, group: build(:group), project: nil, destination_namespace: nil) + expect(entity).not_to be_valid + expect(entity.errors).to include(:destination_namespace) + end + it 'is invalid as a project_entity' do entity = build(:bulk_import_entity, :group_entity, group: nil, project: build(:project)) @@ -94,7 +103,9 @@ RSpec.describe BulkImports::Entity, type: :model do ) expect(entity).not_to be_valid - expect(entity.errors).to include(:destination_namespace) + expect(entity.errors).to include(:base) + expect(entity.errors[:base]) + .to include('Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again.') end it 'is invalid if destination namespace is a descendant of the source' do @@ -109,7 +120,8 @@ RSpec.describe BulkImports::Entity, type: :model do ) expect(entity).not_to be_valid - expect(entity.errors).to include(:destination_namespace) + expect(entity.errors[:base]) + .to include('Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again.') end end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 8eb5a6c27dd..77896105959 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -15,6 +15,8 @@ RSpec.describe BulkImports::Tracker, type: :model do it { is_expected.to validate_presence_of(:relation) } it { is_expected.to validate_uniqueness_of(:relation).scoped_to(:bulk_import_entity_id) } + it { is_expected.to validate_presence_of(:stage) } + context 'when has_next_page is true' do it "validates presence of `next_page`" do tracker = build(:bulk_import_tracker, has_next_page: true) diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index b50e4204e0a..f3029598b02 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -41,7 +41,7 @@ RSpec.describe Ci::Bridge do end end - describe '#scoped_variables_hash' do + describe '#scoped_variables' do it 'returns a hash representing variables' do variables = %w[ CI_JOB_NAME CI_JOB_STAGE CI_COMMIT_SHA CI_COMMIT_SHORT_SHA @@ -53,7 +53,7 @@ RSpec.describe Ci::Bridge do CI_COMMIT_TIMESTAMP ] - expect(bridge.scoped_variables_hash.keys).to include(*variables) + expect(bridge.scoped_variables.map { |v| v[:key] }).to include(*variables) end context 'when bridge has dependency which has dotenv variable' do @@ -63,7 +63,7 @@ RSpec.describe Ci::Bridge do let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: test) } it 'includes inherited variable' do - expect(bridge.scoped_variables_hash).to include(job_variable.key => job_variable.value) + expect(bridge.scoped_variables.to_hash).to include(job_variable.key => job_variable.value) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 4ad7ce70a44..5b07bd8923f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -581,7 +581,7 @@ RSpec.describe Ci::Build do end it 'that cannot handle build' do - expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false) + expect_any_instance_of(Ci::Runner).to receive(:matches_build?).with(build).and_return(false) is_expected.to be_falsey end end @@ -817,7 +817,48 @@ RSpec.describe Ci::Build do end describe '#cache' do - let(:options) { { cache: { key: "key", paths: ["public"], policy: "pull-push" } } } + let(:options) do + { cache: [{ key: "key", paths: ["public"], policy: "pull-push" }] } + end + + context 'with multiple_cache_per_job FF disabled' do + before do + stub_feature_flags(multiple_cache_per_job: false) + end + let(:options) { { cache: { key: "key", paths: ["public"], policy: "pull-push" } } } + + subject { build.cache } + + context 'when build has cache' do + before do + allow(build).to receive(:options).and_return(options) + end + + context 'when project has jobs_cache_index' do + before do + allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) + end + + it { is_expected.to be_an(Array).and all(include(key: "key-1")) } + end + + context 'when project does not have jobs_cache_index' do + before do + allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil) + end + + it { is_expected.to eq([options[:cache]]) } + end + end + + context 'when build does not have cache' do + before do + allow(build).to receive(:options).and_return({}) + end + + it { is_expected.to eq([]) } + end + end subject { build.cache } @@ -826,6 +867,21 @@ RSpec.describe Ci::Build do allow(build).to receive(:options).and_return(options) end + context 'when build has multiple caches' do + let(:options) do + { cache: [ + { key: "key", paths: ["public"], policy: "pull-push" }, + { key: "key2", paths: ["public"], policy: "pull-push" } + ] } + end + + before do + allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) + end + + it { is_expected.to match([a_hash_including(key: "key-1"), a_hash_including(key: "key2-1")]) } + end + context 'when project has jobs_cache_index' do before do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) @@ -839,7 +895,7 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil) end - it { is_expected.to eq([options[:cache]]) } + it { is_expected.to eq(options[:cache]) } end end @@ -848,7 +904,7 @@ RSpec.describe Ci::Build do allow(build).to receive(:options).and_return({}) end - it { is_expected.to eq([nil]) } + it { is_expected.to be_empty } end end @@ -1205,6 +1261,21 @@ RSpec.describe Ci::Build do end end + describe '#environment_deployment_tier' do + subject { build.environment_deployment_tier } + + let(:build) { described_class.new(options: options) } + let(:options) { { environment: { deployment_tier: 'production' } } } + + it { is_expected.to eq('production') } + + context 'when options does not include deployment_tier' do + let(:options) { { environment: { name: 'production' } } } + + it { is_expected.to be_nil } + end + end + describe 'deployment' do describe '#outdated_deployment?' do subject { build.outdated_deployment? } @@ -2367,6 +2438,7 @@ RSpec.describe Ci::Build do { key: 'CI_JOB_ID', value: build.id.to_s, public: true, masked: false }, { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true, masked: false }, { key: 'CI_JOB_TOKEN', value: 'my-token', public: false, masked: true }, + { key: 'CI_JOB_STARTED_AT', value: build.started_at&.iso8601, public: true, masked: false }, { key: 'CI_BUILD_ID', value: build.id.to_s, public: true, masked: false }, { key: 'CI_BUILD_TOKEN', value: 'my-token', public: false, masked: true }, { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true, masked: false }, @@ -2405,17 +2477,18 @@ RSpec.describe Ci::Build do { 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 }, - { key: 'CI_DEPENDENCY_PROXY_SERVER', value: "#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}", public: true, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_SERVER', value: Gitlab.host_with_port, public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX', - value: "#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/#{project.namespace.root_ancestor.path}#{DependencyProxy::URL_SUFFIX}", + value: "#{Gitlab.host_with_port}/#{project.namespace.root_ancestor.path.downcase}#{DependencyProxy::URL_SUFFIX}", public: true, masked: false }, { key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true, masked: false }, { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true, masked: false }, { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true, masked: false }, - { key: 'CI_CONFIG_PATH', value: pipeline.config_path, public: true, masked: false }, + { key: 'CI_PIPELINE_CREATED_AT', value: pipeline.created_at.iso8601, public: true, masked: false }, { key: 'CI_COMMIT_SHA', value: build.sha, public: true, masked: false }, { key: 'CI_COMMIT_SHORT_SHA', value: build.short_sha, public: true, masked: false }, { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true, masked: false }, @@ -2440,7 +2513,8 @@ RSpec.describe Ci::Build do build.yaml_variables = [] end - it { is_expected.to eq(predefined_variables) } + it { is_expected.to be_instance_of(Gitlab::Ci::Variables::Collection) } + it { expect(subject.to_runner_variables).to eq(predefined_variables) } context 'when ci_job_jwt feature flag is disabled' do before do @@ -2495,7 +2569,7 @@ RSpec.describe Ci::Build do end it 'returns variables in order depending on resource hierarchy' do - is_expected.to eq( + expect(subject.to_runner_variables).to eq( [dependency_proxy_var, job_jwt_var, build_pre_var, @@ -2525,7 +2599,7 @@ RSpec.describe Ci::Build do end it 'matches explicit variables ordering' do - received_variables = subject.map { |variable| variable.fetch(:key) } + received_variables = subject.map { |variable| variable[:key] } expect(received_variables).to eq expected_variables end @@ -2584,14 +2658,14 @@ RSpec.describe Ci::Build do end shared_examples 'containing environment variables' do - it { environment_variables.each { |v| is_expected.to include(v) } } + it { is_expected.to include(*environment_variables) } end context 'when no URL was set' do it_behaves_like 'containing environment variables' it 'does not have CI_ENVIRONMENT_URL' do - keys = subject.map { |var| var[:key] } + keys = subject.pluck(:key) expect(keys).not_to include('CI_ENVIRONMENT_URL') end @@ -2618,7 +2692,7 @@ RSpec.describe Ci::Build do it_behaves_like 'containing environment variables' it 'puts $CI_ENVIRONMENT_URL in the last so all other variables are available to be used when runners are trying to expand it' do - expect(subject.last).to eq(environment_variables.last) + expect(subject.to_runner_variables.last).to eq(environment_variables.last) end end end @@ -2951,7 +3025,7 @@ RSpec.describe Ci::Build do end it 'overrides YAML variable using a pipeline variable' do - variables = subject.reverse.uniq { |variable| variable[:key] }.reverse + variables = subject.to_runner_variables.reverse.uniq { |variable| variable[:key] }.reverse expect(variables) .not_to include(key: 'MYVAR', value: 'myvar', public: true, masked: false) @@ -3248,47 +3322,6 @@ RSpec.describe Ci::Build do end end - describe '#scoped_variables_hash' do - context 'when overriding CI variables' do - before do - project.variables.create!(key: 'MY_VAR', value: 'my value 1') - pipeline.variables.create!(key: 'MY_VAR', value: 'my value 2') - end - - it 'returns a regular hash created using valid ordering' do - expect(build.scoped_variables_hash).to include('MY_VAR': 'my value 2') - expect(build.scoped_variables_hash).not_to include('MY_VAR': 'my value 1') - end - end - - context 'when overriding user-provided variables' do - let(:build) do - create(:ci_build, pipeline: pipeline, yaml_variables: [{ key: 'MY_VAR', value: 'myvar', public: true }]) - end - - before do - pipeline.variables.build(key: 'MY_VAR', value: 'pipeline value') - end - - it 'returns a hash including variable with higher precedence' do - expect(build.scoped_variables_hash).to include('MY_VAR': 'pipeline value') - expect(build.scoped_variables_hash).not_to include('MY_VAR': 'myvar') - end - end - - context 'when overriding CI instance variables' do - before do - create(:ci_instance_variable, key: 'MY_VAR', value: 'my value 1') - group.variables.create!(key: 'MY_VAR', value: 'my value 2') - end - - it 'returns a regular hash created using valid ordering' do - expect(build.scoped_variables_hash).to include('MY_VAR': 'my value 2') - expect(build.scoped_variables_hash).not_to include('MY_VAR': 'my value 1') - end - end - end - describe '#any_unmet_prerequisites?' do let(:build) { create(:ci_build, :created) } diff --git a/spec/models/ci/daily_build_group_report_result_spec.rb b/spec/models/ci/daily_build_group_report_result_spec.rb index f6e6a6a5e02..4e96ec7cecb 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -162,39 +162,5 @@ RSpec.describe Ci::DailyBuildGroupReportResult do end end end - - describe '.by_date' do - subject(:coverages) { described_class.by_date(start_date) } - - let!(:coverage_1) { create(:ci_daily_build_group_report_result, date: 1.week.ago) } - - context 'when project has several coverage' do - let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 2.weeks.ago) } - let(:start_date) { 1.week.ago.to_date.to_s } - - it 'returns the coverage from the start_date' do - expect(coverages).to contain_exactly(coverage_1) - end - end - - context 'when start_date is over 90 days' do - let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 90.days.ago) } - let!(:coverage_3) { create(:ci_daily_build_group_report_result, date: 91.days.ago) } - let(:start_date) { 1.year.ago.to_date.to_s } - - it 'returns the coverage in the last 90 days' do - expect(coverages).to contain_exactly(coverage_1, coverage_2) - end - end - - context 'when start_date is not a string' do - let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 90.days.ago) } - let(:start_date) { 1.week.ago } - - it 'returns the coverage in the last 90 days' do - expect(coverages).to contain_exactly(coverage_1, coverage_2) - end - end - end end end diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index c8eac4d8765..f0eec549da7 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -9,7 +9,17 @@ RSpec.describe Ci::GroupVariable do it { is_expected.to include_module(Presentable) } it { is_expected.to include_module(Ci::Maskable) } - it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id).with_message(/\(\w+\) has already been taken/) } + it { is_expected.to include_module(HasEnvironmentScope) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to([:group_id, :environment_scope]).with_message(/\(\w+\) has already been taken/) } + + describe '.by_environment_scope' do + let!(:matching_variable) { create(:ci_group_variable, environment_scope: 'production ') } + let!(:non_matching_variable) { create(:ci_group_variable, environment_scope: 'staging') } + + subject { Ci::GroupVariable.by_environment_scope('production') } + + it { is_expected.to contain_exactly(matching_variable) } + end describe '.unprotected' do subject { described_class.unprotected } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 94943fb3644..d57a39d133f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -8,12 +8,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do include Ci::SourcePipelineHelpers let_it_be(:user) { create(:user) } - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:project) { create_default(:project, :repository) } - - let(:pipeline) do - create(:ci_empty_pipeline, status: :created, project: project) - end + let_it_be(:namespace) { create_default(:namespace).freeze } + let_it_be(:project) { create_default(:project, :repository).freeze } it_behaves_like 'having unique enum values' @@ -53,6 +49,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'associations' do + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + it 'has a bidirectional relationship with projects' do expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:all_pipelines) expect(Project.reflect_on_association(:all_pipelines).has_inverse?).to eq(:project) @@ -82,6 +80,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#set_status' do + let(:pipeline) { build(:ci_empty_pipeline, :created) } + where(:from_status, :to_status) do from_status_names = described_class.state_machines[:status].states.map(&:name) to_status_names = from_status_names - [:created] # we never want to transition into created @@ -105,6 +105,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '.processables' do + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + before do create(:ci_build, name: 'build', pipeline: pipeline) create(:ci_bridge, name: 'bridge', pipeline: pipeline) @@ -142,7 +144,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { described_class.for_sha(sha) } let(:sha) { 'abc' } - let!(:pipeline) { create(:ci_pipeline, sha: 'abc') } + + let_it_be(:pipeline) { create(:ci_pipeline, sha: 'abc') } it 'returns the pipeline' do is_expected.to contain_exactly(pipeline) @@ -170,7 +173,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { described_class.for_source_sha(source_sha) } let(:source_sha) { 'abc' } - let!(:pipeline) { create(:ci_pipeline, source_sha: 'abc') } + + let_it_be(:pipeline) { create(:ci_pipeline, source_sha: 'abc') } it 'returns the pipeline' do is_expected.to contain_exactly(pipeline) @@ -228,7 +232,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { described_class.for_branch(branch) } let(:branch) { 'master' } - let!(:pipeline) { create(:ci_pipeline, ref: 'master') } + + let_it_be(:pipeline) { create(:ci_pipeline, ref: 'master') } it 'returns the pipeline' do is_expected.to contain_exactly(pipeline) @@ -247,13 +252,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '.ci_sources' do subject { described_class.ci_sources } - let!(:push_pipeline) { create(:ci_pipeline, source: :push) } - let!(:web_pipeline) { create(:ci_pipeline, source: :web) } - let!(:api_pipeline) { create(:ci_pipeline, source: :api) } - let!(:webide_pipeline) { create(:ci_pipeline, source: :webide) } - let!(:child_pipeline) { create(:ci_pipeline, source: :parent_pipeline) } + let(:push_pipeline) { build(:ci_pipeline, source: :push) } + let(:web_pipeline) { build(:ci_pipeline, source: :web) } + let(:api_pipeline) { build(:ci_pipeline, source: :api) } + let(:webide_pipeline) { build(:ci_pipeline, source: :webide) } + let(:child_pipeline) { build(:ci_pipeline, source: :parent_pipeline) } + let(:pipelines) { [push_pipeline, web_pipeline, api_pipeline, webide_pipeline, child_pipeline] } it 'contains pipelines having CI only sources' do + pipelines.map(&:save!) + expect(subject).to contain_exactly(push_pipeline, web_pipeline, api_pipeline) end @@ -365,8 +373,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#merge_request_pipeline?' do - subject { pipeline.merge_request_pipeline? } + describe '#merged_result_pipeline?' do + subject { pipeline.merged_result_pipeline? } let!(:pipeline) do create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, target_sha: target_sha) @@ -387,6 +395,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#merge_request_ref?' do subject { pipeline.merge_request_ref? } + let(:pipeline) { build(:ci_empty_pipeline, :created) } + it 'calls MergeRequest#merge_request_ref?' do expect(MergeRequest).to receive(:merge_request_ref?).with(pipeline.ref) @@ -606,7 +616,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#source' do context 'when creating new pipeline' do let(:pipeline) do - build(:ci_empty_pipeline, status: :created, project: project, source: nil) + build(:ci_empty_pipeline, :created, project: project, source: nil) end it "prevents from creating an object" do @@ -615,17 +625,21 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when updating existing pipeline' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + before do pipeline.update_attribute(:source, nil) end - it "object is valid" do + it 'object is valid' do expect(pipeline).to be_valid end end end describe '#block' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + it 'changes pipeline status to manual' do expect(pipeline.block).to be true expect(pipeline.reload).to be_manual @@ -636,7 +650,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#delay' do subject { pipeline.delay } - let(:pipeline) { build(:ci_pipeline, status: :created) } + let(:pipeline) { build(:ci_pipeline, :created) } it 'changes pipeline status to schedule' do subject @@ -646,6 +660,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#valid_commit_sha' do + let(:pipeline) { build_stubbed(:ci_empty_pipeline, :created, project: project) } + context 'commit.sha can not start with 00000000' do before do pipeline.sha = '0' * 40 @@ -659,6 +675,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#short_sha' do subject { pipeline.short_sha } + let(:pipeline) { build_stubbed(:ci_empty_pipeline, :created) } + it 'has 8 items' do expect(subject.size).to eq(8) end @@ -668,49 +686,58 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#retried' do subject { pipeline.retried } + let(:pipeline) { create(:ci_empty_pipeline, :created, project: project) } + let!(:build1) { create(:ci_build, pipeline: pipeline, name: 'deploy', retried: true) } + before do - @build1 = create(:ci_build, pipeline: pipeline, name: 'deploy', retried: true) - @build2 = create(:ci_build, pipeline: pipeline, name: 'deploy') + create(:ci_build, pipeline: pipeline, name: 'deploy') end it 'returns old builds' do - is_expected.to contain_exactly(@build1) + is_expected.to contain_exactly(build1) end end describe '#coverage' do - let(:project) { create(:project, build_coverage_regex: "/.*/") } - let(:pipeline) { create(:ci_empty_pipeline, project: project) } + let_it_be_with_reload(:pipeline) { create(:ci_empty_pipeline) } - it "calculates average when there are two builds with coverage" do - create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline) - create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline) - expect(pipeline.coverage).to eq("35.00") - end + context 'with multiple pipelines' do + before_all do + create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline) + create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline) + end - it "calculates average when there are two builds with coverage and one with nil" do - create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline) - create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline) - create(:ci_build, pipeline: pipeline) - expect(pipeline.coverage).to eq("35.00") - end + it "calculates average when there are two builds with coverage" do + expect(pipeline.coverage).to eq("35.00") + end + + it "calculates average when there are two builds with coverage and one with nil" do + create(:ci_build, pipeline: pipeline) + + expect(pipeline.coverage).to eq("35.00") + end - it "calculates average when there are two builds with coverage and one is retried" do - create(:ci_build, name: "rspec", coverage: 30, pipeline: pipeline) - create(:ci_build, name: "rubocop", coverage: 30, pipeline: pipeline, retried: true) - create(:ci_build, name: "rubocop", coverage: 40, pipeline: pipeline) - expect(pipeline.coverage).to eq("35.00") + it "calculates average when there are two builds with coverage and one is retried" do + create(:ci_build, name: "rubocop", coverage: 30, pipeline: pipeline, retried: true) + + expect(pipeline.coverage).to eq("35.00") + end end - it "calculates average when there is one build without coverage" do - FactoryBot.create(:ci_build, pipeline: pipeline) - expect(pipeline.coverage).to be_nil + context 'when there is one build without coverage' do + it "calculates average to nil" do + create(:ci_build, pipeline: pipeline) + + expect(pipeline.coverage).to be_nil + end end end describe '#retryable?' do subject { pipeline.retryable? } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created, project: project) } + context 'no failed builds' do before do create_build('rspec', 'success') @@ -772,13 +799,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#predefined_variables' do subject { pipeline.predefined_variables } + let(:pipeline) { build(:ci_empty_pipeline, :created) } + it 'includes all predefined variables in a valid order' do keys = subject.map { |variable| variable[:key] } expect(keys).to eq %w[ CI_PIPELINE_IID CI_PIPELINE_SOURCE - CI_CONFIG_PATH + CI_PIPELINE_CREATED_AT CI_COMMIT_SHA CI_COMMIT_SHORT_SHA CI_COMMIT_BEFORE_SHA @@ -798,21 +827,18 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when merge request is present' do + let_it_be(:assignees) { create_list(:user, 2) } + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:labels) { create_list(:label, 2) } let(:merge_request) do - create(:merge_request, + create(:merge_request, :simple, source_project: project, - source_branch: 'feature', target_project: project, - target_branch: 'master', assignees: assignees, milestone: milestone, labels: labels) end - let(:assignees) { create_list(:user, 2) } - let(:milestone) { create(:milestone, project: project) } - let(:labels) { create_list(:label, 2) } - context 'when pipeline for merge request is created' do let(:pipeline) do create(:ci_pipeline, :detached_merge_request_pipeline, @@ -998,9 +1024,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#protected_ref?' do - before do - pipeline.project = create(:project, :repository) - end + let(:pipeline) { build(:ci_empty_pipeline, :created) } it 'delegates method to project' do expect(pipeline).not_to be_protected_ref @@ -1008,11 +1032,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#legacy_trigger' do - let(:trigger_request) { create(:ci_trigger_request) } - - before do - pipeline.trigger_requests << trigger_request - end + let(:trigger_request) { build(:ci_trigger_request) } + let(:pipeline) { build(:ci_empty_pipeline, :created, trigger_requests: [trigger_request]) } it 'returns first trigger request' do expect(pipeline.legacy_trigger).to eq trigger_request @@ -1022,6 +1043,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#auto_canceled?' do subject { pipeline.auto_canceled? } + let(:pipeline) { build(:ci_empty_pipeline, :created) } + context 'when it is canceled' do before do pipeline.cancel @@ -1029,7 +1052,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when there is auto_canceled_by' do before do - pipeline.update!(auto_canceled_by: create(:ci_empty_pipeline)) + pipeline.auto_canceled_by = create(:ci_empty_pipeline) end it 'is auto canceled' do @@ -1057,6 +1080,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'pipeline stages' do + let(:pipeline) { build(:ci_empty_pipeline, :created) } + describe 'legacy stages' do before do create(:commit_status, pipeline: pipeline, @@ -1107,22 +1132,28 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when commit status is retried' do - before do + let!(:old_commit_status) do create(:commit_status, pipeline: pipeline, - stage: 'build', - name: 'mac', - stage_idx: 0, - status: 'success') - - Ci::ProcessPipelineService - .new(pipeline) - .execute + stage: 'build', + name: 'mac', + stage_idx: 0, + status: 'success') end - it 'ignores the previous state' do - expect(statuses).to eq([%w(build success), - %w(test success), - %w(deploy running)]) + context 'when FF ci_remove_update_retried_from_process_pipeline is disabled' do + before do + stub_feature_flags(ci_remove_update_retried_from_process_pipeline: false) + + Ci::ProcessPipelineService + .new(pipeline) + .execute + end + + it 'ignores the previous state' do + expect(statuses).to eq([%w(build success), + %w(test success), + %w(deploy running)]) + end end end end @@ -1162,6 +1193,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#legacy_stage' do subject { pipeline.legacy_stage('test') } + let(:pipeline) { build(:ci_empty_pipeline, :created) } + context 'with status in stage' do before do create(:commit_status, pipeline: pipeline, stage: 'test') @@ -1184,6 +1217,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#stages' do + let(:pipeline) { build(:ci_empty_pipeline, :created) } + before do create(:ci_stage_entity, project: project, pipeline: pipeline, @@ -1238,6 +1273,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'state machine' do + let_it_be_with_reload(:pipeline) { create(:ci_empty_pipeline, :created) } let(:current) { Time.current.change(usec: 0) } let(:build) { create_build('build1', queued_at: 0) } let(:build_b) { create_build('build2', queued_at: 0) } @@ -1401,28 +1437,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'pipeline caching' do - context 'if pipeline is cacheable' do - before do - pipeline.source = 'push' - end - - it 'performs ExpirePipelinesCacheWorker' do - expect(ExpirePipelineCacheWorker).to receive(:perform_async).with(pipeline.id) + it 'performs ExpirePipelinesCacheWorker' do + expect(ExpirePipelineCacheWorker).to receive(:perform_async).with(pipeline.id) - pipeline.cancel - end - end - - context 'if pipeline is not cacheable' do - before do - pipeline.source = 'webide' - end - - it 'deos not perform ExpirePipelinesCacheWorker' do - expect(ExpirePipelineCacheWorker).not_to receive(:perform_async) - - pipeline.cancel - end + pipeline.cancel end end @@ -1441,24 +1459,25 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'auto merge' do - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - - let(:pipeline) do - create(:ci_pipeline, :running, project: merge_request.source_project, - ref: merge_request.source_branch, - sha: merge_request.diff_head_sha) - end + context 'when auto merge is enabled' do + let_it_be_with_reload(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let_it_be_with_reload(:pipeline) do + create(:ci_pipeline, :running, project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end - before do - merge_request.update_head_pipeline - end + before_all do + merge_request.update_head_pipeline + end - %w[succeed! drop! cancel! skip!].each do |action| - context "when the pipeline recieved #{action} event" do - it 'performs AutoMergeProcessWorker' do - expect(AutoMergeProcessWorker).to receive(:perform_async).with(merge_request.id) + %w[succeed! drop! cancel! skip!].each do |action| + context "when the pipeline recieved #{action} event" do + it 'performs AutoMergeProcessWorker' do + expect(AutoMergeProcessWorker).to receive(:perform_async).with(merge_request.id) - pipeline.public_send(action) + pipeline.public_send(action) + end end end end @@ -1610,15 +1629,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'multi-project pipelines' do let!(:downstream_project) { create(:project, :repository) } - let!(:upstream_pipeline) { create(:ci_pipeline, project: project) } + let!(:upstream_pipeline) { create(:ci_pipeline) } let!(:downstream_pipeline) { create(:ci_pipeline, :with_job, project: downstream_project) } it_behaves_like 'upstream downstream pipeline' end context 'parent-child pipelines' do - let!(:upstream_pipeline) { create(:ci_pipeline, project: project) } - let!(:downstream_pipeline) { create(:ci_pipeline, :with_job, project: project) } + let!(:upstream_pipeline) { create(:ci_pipeline) } + let!(:downstream_pipeline) { create(:ci_pipeline, :with_job) } it_behaves_like 'upstream downstream pipeline' end @@ -1637,6 +1656,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#branch?' do subject { pipeline.branch? } + let(:pipeline) { build(:ci_empty_pipeline, :created) } + context 'when ref is not a tag' do before do pipeline.tag = false @@ -1647,16 +1668,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is merge request' do - let(:pipeline) do - create(:ci_pipeline, merge_request: merge_request) - end + let(:pipeline) { build(:ci_pipeline, merge_request: merge_request) } let(:merge_request) do - create(:merge_request, + create(:merge_request, :simple, source_project: project, - source_branch: 'feature', - target_project: project, - target_branch: 'master') + target_project: project) end it 'returns false' do @@ -1720,6 +1737,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when repository exists' do using RSpec::Parameterized::TableSyntax + let_it_be(:pipeline, refind: true) { create(:ci_empty_pipeline) } + where(:tag, :ref, :result) do false | 'master' | true false | 'non-existent-branch' | false @@ -1728,8 +1747,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end with_them do - let(:pipeline) do - create(:ci_empty_pipeline, project: project, tag: tag, ref: ref) + before do + pipeline.update!(tag: tag, ref: ref) end it "correctly detects ref" do @@ -1739,10 +1758,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when repository does not exist' do - let(:project) { create(:project) } - let(:pipeline) do - create(:ci_empty_pipeline, project: project, ref: 'master') - end + let(:pipeline) { build(:ci_empty_pipeline, ref: 'master', project: build(:project)) } it 'always returns false' do expect(pipeline.ref_exists?).to eq false @@ -1753,7 +1769,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'with non-empty project' do let(:pipeline) do create(:ci_pipeline, - project: project, ref: project.default_branch, sha: project.commit.sha) end @@ -1761,14 +1776,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#lazy_ref_commit' do let(:another) do create(:ci_pipeline, - project: project, ref: 'feature', sha: project.commit('feature').sha) end let(:unicode) do create(:ci_pipeline, - project: project, ref: 'ü/unicode/multi-byte') end @@ -1827,6 +1840,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#manual_actions' do subject { pipeline.manual_actions } + let(:pipeline) { create(:ci_empty_pipeline, :created) } + it 'when none defined' do is_expected.to be_empty end @@ -1853,9 +1868,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#branch_updated?' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + context 'when pipeline has before SHA' do before do - pipeline.update_column(:before_sha, 'a1b2c3d4') + pipeline.update!(before_sha: 'a1b2c3d4') end it 'runs on a branch update push' do @@ -1866,7 +1883,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline does not have before SHA' do before do - pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA) + pipeline.update!(before_sha: Gitlab::Git::BLANK_SHA) end it 'does not run on a branch updating push' do @@ -1876,6 +1893,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#modified_paths' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + context 'when old and new revisions are set' do before do pipeline.update!(before_sha: '1234abcd', sha: '2345bcde') @@ -1892,7 +1911,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when either old or new revision is missing' do before do - pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA) + pipeline.update!(before_sha: Gitlab::Git::BLANK_SHA) end it 'returns nil' do @@ -1906,11 +1925,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end let(:merge_request) do - create(:merge_request, + create(:merge_request, :simple, source_project: project, - source_branch: 'feature', - target_project: project, - target_branch: 'master') + target_project: project) end it 'returns merge request modified paths' do @@ -1944,6 +1961,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#has_kubernetes_active?' do + let(:pipeline) { create(:ci_empty_pipeline, :created, project: project) } + context 'when kubernetes is active' do context 'when user configured kubernetes from CI/CD > Clusters' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } @@ -1965,6 +1984,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#has_warnings?' do subject { pipeline.has_warnings? } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + context 'build which is allowed to fail fails' do before do create :ci_build, :success, pipeline: pipeline, name: 'rspec' @@ -2021,6 +2042,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#number_of_warnings' do + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + it 'returns the number of warnings' do create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') create(:ci_bridge, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') @@ -2029,7 +2052,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it 'supports eager loading of the number of warnings' do - pipeline2 = create(:ci_empty_pipeline, status: :created, project: project) + pipeline2 = create(:ci_empty_pipeline, :created) create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop') @@ -2053,6 +2076,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { pipeline.needs_processing? } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + where(:processed, :result) do nil | true false | true @@ -2072,122 +2097,107 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - shared_context 'with some outdated pipelines' do - before do - create_pipeline(:canceled, 'ref', 'A', project) - create_pipeline(:success, 'ref', 'A', project) - create_pipeline(:failed, 'ref', 'B', project) - create_pipeline(:skipped, 'feature', 'C', project) + context 'with outdated pipelines' do + before_all do + create_pipeline(:canceled, 'ref', 'A') + create_pipeline(:success, 'ref', 'A') + create_pipeline(:failed, 'ref', 'B') + create_pipeline(:skipped, 'feature', 'C') end - def create_pipeline(status, ref, sha, project) + def create_pipeline(status, ref, sha) create( :ci_empty_pipeline, status: status, ref: ref, - sha: sha, - project: project + sha: sha ) end - end - describe '.newest_first' do - include_context 'with some outdated pipelines' - - it 'returns the pipelines from new to old' do - expect(described_class.newest_first.pluck(:status)) - .to eq(%w[skipped failed success canceled]) - end + describe '.newest_first' do + it 'returns the pipelines from new to old' do + expect(described_class.newest_first.pluck(:status)) + .to eq(%w[skipped failed success canceled]) + end - it 'searches limited backlog' do - expect(described_class.newest_first(limit: 1).pluck(:status)) - .to eq(%w[skipped]) + it 'searches limited backlog' do + expect(described_class.newest_first(limit: 1).pluck(:status)) + .to eq(%w[skipped]) + end end - end - describe '.latest_status' do - include_context 'with some outdated pipelines' - - context 'when no ref is specified' do - it 'returns the status of the latest pipeline' do - expect(described_class.latest_status).to eq('skipped') + describe '.latest_status' do + context 'when no ref is specified' do + it 'returns the status of the latest pipeline' do + expect(described_class.latest_status).to eq('skipped') + end end - end - context 'when ref is specified' do - it 'returns the status of the latest pipeline for the given ref' do - expect(described_class.latest_status('ref')).to eq('failed') + context 'when ref is specified' do + it 'returns the status of the latest pipeline for the given ref' do + expect(described_class.latest_status('ref')).to eq('failed') + end end end - end - describe '.latest_successful_for_ref' do - include_context 'with some outdated pipelines' - - let!(:latest_successful_pipeline) do - create_pipeline(:success, 'ref', 'D', project) - end + describe '.latest_successful_for_ref' do + let!(:latest_successful_pipeline) do + create_pipeline(:success, 'ref', 'D') + end - it 'returns the latest successful pipeline' do - expect(described_class.latest_successful_for_ref('ref')) - .to eq(latest_successful_pipeline) + it 'returns the latest successful pipeline' do + expect(described_class.latest_successful_for_ref('ref')) + .to eq(latest_successful_pipeline) + end end - end - describe '.latest_running_for_ref' do - include_context 'with some outdated pipelines' - - let!(:latest_running_pipeline) do - create_pipeline(:running, 'ref', 'D', project) - end + describe '.latest_running_for_ref' do + let!(:latest_running_pipeline) do + create_pipeline(:running, 'ref', 'D') + end - it 'returns the latest running pipeline' do - expect(described_class.latest_running_for_ref('ref')) - .to eq(latest_running_pipeline) + it 'returns the latest running pipeline' do + expect(described_class.latest_running_for_ref('ref')) + .to eq(latest_running_pipeline) + end end - end - - describe '.latest_failed_for_ref' do - include_context 'with some outdated pipelines' - let!(:latest_failed_pipeline) do - create_pipeline(:failed, 'ref', 'D', project) - end + describe '.latest_failed_for_ref' do + let!(:latest_failed_pipeline) do + create_pipeline(:failed, 'ref', 'D') + end - it 'returns the latest failed pipeline' do - expect(described_class.latest_failed_for_ref('ref')) - .to eq(latest_failed_pipeline) + it 'returns the latest failed pipeline' do + expect(described_class.latest_failed_for_ref('ref')) + .to eq(latest_failed_pipeline) + end end - end - - describe '.latest_successful_for_sha' do - include_context 'with some outdated pipelines' - let!(:latest_successful_pipeline) do - create_pipeline(:success, 'ref', 'awesomesha', project) - end + describe '.latest_successful_for_sha' do + let!(:latest_successful_pipeline) do + create_pipeline(:success, 'ref', 'awesomesha') + end - it 'returns the latest successful pipeline' do - expect(described_class.latest_successful_for_sha('awesomesha')) - .to eq(latest_successful_pipeline) + it 'returns the latest successful pipeline' do + expect(described_class.latest_successful_for_sha('awesomesha')) + .to eq(latest_successful_pipeline) + end end - end - - describe '.latest_successful_for_refs' do - include_context 'with some outdated pipelines' - let!(:latest_successful_pipeline1) do - create_pipeline(:success, 'ref1', 'D', project) - end + describe '.latest_successful_for_refs' do + let!(:latest_successful_pipeline1) do + create_pipeline(:success, 'ref1', 'D') + end - let!(:latest_successful_pipeline2) do - create_pipeline(:success, 'ref2', 'D', project) - end + let!(:latest_successful_pipeline2) do + create_pipeline(:success, 'ref2', 'D') + end - it 'returns the latest successful pipeline for both refs' do - refs = %w(ref1 ref2 ref3) + it 'returns the latest successful pipeline for both refs' do + refs = %w(ref1 ref2 ref3) - expect(described_class.latest_successful_for_refs(refs)).to eq({ 'ref1' => latest_successful_pipeline1, 'ref2' => latest_successful_pipeline2 }) + expect(described_class.latest_successful_for_refs(refs)).to eq({ 'ref1' => latest_successful_pipeline1, 'ref2' => latest_successful_pipeline2 }) + end end end @@ -2197,8 +2207,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do :ci_empty_pipeline, status: 'success', ref: 'master', - sha: '123', - project: project + sha: '123' ) end @@ -2207,8 +2216,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do :ci_empty_pipeline, status: 'success', ref: 'develop', - sha: '123', - project: project + sha: '123' ) end @@ -2217,8 +2225,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do :ci_empty_pipeline, status: 'success', ref: 'test', - sha: '456', - project: project + sha: '456' ) end @@ -2315,12 +2322,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#status', :sidekiq_inline do - let(:build) do - create(:ci_build, :created, pipeline: pipeline, name: 'test') - end - subject { pipeline.reload.status } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + let(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } + context 'on waiting for resource' do before do allow(build).to receive(:with_resource_group?) { true } @@ -2412,8 +2418,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#detailed_status' do subject { pipeline.detailed_status(user) } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + context 'when pipeline is created' do - let(:pipeline) { create(:ci_pipeline, status: :created) } + let(:pipeline) { create(:ci_pipeline, :created) } it 'returns detailed status for created pipeline' do expect(subject.text).to eq s_('CiStatusText|created') @@ -2490,6 +2498,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#cancelable?' do + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + %i[created running pending].each do |status0| context "when there is a build #{status0}" do before do @@ -2581,7 +2591,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#cancel_running' do - let(:latest_status) { pipeline.statuses.pluck(:status) } + subject(:latest_status) { pipeline.statuses.pluck(:status) } + + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } context 'when there is a running external job and a regular job' do before do @@ -2624,7 +2636,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#retry_failed' do - let(:latest_status) { pipeline.latest_statuses.pluck(:status) } + subject(:latest_status) { pipeline.latest_statuses.pluck(:status) } + + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } before do stub_not_protect_default_branch @@ -2673,11 +2687,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#execute_hooks' do + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } let!(:build_a) { create_build('a', 0) } let!(:build_b) { create_build('b', 0) } let!(:hook) do - create(:project_hook, project: project, pipeline_events: enabled) + create(:project_hook, pipeline_events: enabled) end before do @@ -2703,7 +2718,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it 'builds hook data once' do - create(:pipelines_email_service, project: project) + create(:pipelines_email_service) expect(Gitlab::DataBuilder::Pipeline).to receive(:build).once.and_call_original @@ -2789,7 +2804,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe "#merge_requests_as_head_pipeline" do - let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: 'a288a022a53a5a944fae87bcec6efc87b7061808') } + let_it_be_with_reload(:pipeline) { create(:ci_empty_pipeline, status: 'created', ref: 'master', sha: 'a288a022a53a5a944fae87bcec6efc87b7061808') } it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do allow_next_instance_of(MergeRequest) do |instance| @@ -2801,7 +2816,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do - create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') + create(:merge_request, :simple, source_project: project) expect(pipeline.merge_requests_as_head_pipeline).to be_empty end @@ -2817,7 +2832,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#all_merge_requests' do - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created, project: project) } shared_examples 'a method that returns all merge requests for a given pipeline' do let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: pipeline_project, ref: 'master') } @@ -2911,10 +2927,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#related_merge_requests' do - let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'stable') } - let(:branch_pipeline) { create(:ci_pipeline, project: project, ref: 'feature') } + let(:branch_pipeline) { create(:ci_pipeline, ref: 'feature') } let(:merge_pipeline) { create(:ci_pipeline, :detached_merge_request_pipeline, merge_request: merge_request) } context 'for a branch pipeline' do @@ -2951,9 +2966,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#open_merge_requests_refs' do - let(:project) { create(:project) } - let(:user) { create(:user) } - let!(:pipeline) { create(:ci_pipeline, user: user, project: project, ref: 'feature') } + let!(:pipeline) { create(:ci_pipeline, user: user, ref: 'feature') } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } subject { pipeline.open_merge_requests_refs } @@ -3000,6 +3013,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#same_family_pipeline_ids' do subject { pipeline.same_family_pipeline_ids.map(&:id) } + let_it_be(:pipeline) { create(:ci_empty_pipeline, :created) } + context 'when pipeline is not child nor parent' do it 'returns just the pipeline id' do expect(subject).to contain_exactly(pipeline.id) @@ -3007,7 +3022,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is child' do - let(:parent) { create(:ci_pipeline, project: project) } + let(:parent) { create(:ci_pipeline) } let!(:pipeline) { create(:ci_pipeline, child_of: parent) } let!(:sibling) { create(:ci_pipeline, child_of: parent) } @@ -3025,7 +3040,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is a child of a child pipeline' do - let(:ancestor) { create(:ci_pipeline, project: project) } + let(:ancestor) { create(:ci_pipeline) } let!(:parent) { create(:ci_pipeline, child_of: ancestor) } let!(:pipeline) { create(:ci_pipeline, child_of: parent) } let!(:cousin_parent) { create(:ci_pipeline, child_of: ancestor) } @@ -3050,10 +3065,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#root_ancestor' do subject { pipeline.root_ancestor } - let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:pipeline) { create(:ci_pipeline) } context 'when pipeline is child of child pipeline' do - let!(:root_ancestor) { create(:ci_pipeline, project: project) } + let!(:root_ancestor) { create(:ci_pipeline) } let!(:parent_pipeline) { create(:ci_pipeline, child_of: root_ancestor) } let!(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) } @@ -3088,6 +3103,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#stuck?' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + before do create(:ci_build, :pending, pipeline: pipeline) end @@ -3132,6 +3149,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#has_yaml_errors?' do + let(:pipeline) { build_stubbed(:ci_pipeline) } + context 'when yaml_errors is set' do before do pipeline.yaml_errors = 'File not found' @@ -3201,7 +3220,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline is not the latest' do before do - create(:ci_pipeline, :success, project: project, ci_ref: pipeline.ci_ref) + create(:ci_pipeline, :success, ci_ref: pipeline.ci_ref) end it 'does not pass ref_status' do @@ -3302,7 +3321,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#builds_in_self_and_descendants' do subject(:builds) { pipeline.builds_in_self_and_descendants } - let(:pipeline) { create(:ci_pipeline, project: project) } + let(:pipeline) { create(:ci_pipeline) } let!(:build) { create(:ci_build, pipeline: pipeline) } context 'when pipeline is standalone' do @@ -3333,6 +3352,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#build_with_artifacts_in_self_and_descendants' do + let_it_be(:pipeline) { create(:ci_pipeline) } let!(:build) { create(:ci_build, name: 'test', pipeline: pipeline) } let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } let!(:child_build) { create(:ci_build, :artifacts, name: 'test', pipeline: child_pipeline) } @@ -3351,6 +3371,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#find_job_with_archive_artifacts' do + let(:pipeline) { create(:ci_pipeline) } let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) } let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) } let!(:expected_job) { create(:ci_build, :artifacts, name: 'rspec', pipeline: pipeline ) } @@ -3364,6 +3385,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#latest_builds_with_artifacts' do + let(:pipeline) { create(:ci_pipeline) } let!(:fresh_build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } let!(:stale_build) { create(:ci_build, :success, :expired, :artifacts, pipeline: pipeline) } @@ -3390,7 +3412,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#batch_lookup_report_artifact_for_file_type' do context 'with code quality report artifact' do - let(:pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) } + let(:pipeline) { create(:ci_pipeline, :with_codequality_reports) } it "returns the code quality artifact" do expect(pipeline.batch_lookup_report_artifact_for_file_type(:codequality)).to eq(pipeline.job_artifacts.sample) @@ -3399,24 +3421,26 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#latest_report_builds' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + it 'returns build with test artifacts' do - test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) - coverage_build = create(:ci_build, :coverage_reports, pipeline: pipeline, project: project) + test_build = create(:ci_build, :test_reports, pipeline: pipeline) + coverage_build = create(:ci_build, :coverage_reports, pipeline: pipeline) create(:ci_build, :artifacts, pipeline: pipeline, project: project) expect(pipeline.latest_report_builds).to contain_exactly(test_build, coverage_build) end it 'filters builds by scope' do - test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) - create(:ci_build, :coverage_reports, pipeline: pipeline, project: project) + test_build = create(:ci_build, :test_reports, pipeline: pipeline) + create(:ci_build, :coverage_reports, pipeline: pipeline) expect(pipeline.latest_report_builds(Ci::JobArtifact.test_reports)).to contain_exactly(test_build) end it 'only returns not retried builds' do - test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) - create(:ci_build, :test_reports, :retried, pipeline: pipeline, project: project) + test_build = create(:ci_build, :test_reports, pipeline: pipeline) + create(:ci_build, :test_reports, :retried, pipeline: pipeline) expect(pipeline.latest_report_builds).to contain_exactly(test_build) end @@ -3427,17 +3451,17 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline has builds with test reports' do before do - create(:ci_build, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :test_reports, pipeline: pipeline) end context 'when pipeline status is running' do - let(:pipeline) { create(:ci_pipeline, :running, project: project) } + let(:pipeline) { create(:ci_pipeline, :running) } it { is_expected.to be_falsey } end context 'when pipeline status is success' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { is_expected.to be_truthy } end @@ -3445,20 +3469,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline does not have builds with test reports' do before do - create(:ci_build, :artifacts, pipeline: pipeline, project: project) + create(:ci_build, :artifacts, pipeline: pipeline) end - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { is_expected.to be_falsey } end context 'when retried build has test reports' do before do - create(:ci_build, :retried, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :retried, :test_reports, pipeline: pipeline) end - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { is_expected.to be_falsey } end @@ -3468,13 +3492,13 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { pipeline.has_coverage_reports? } context 'when pipeline has a code coverage artifact' do - let(:pipeline) { create(:ci_pipeline, :with_coverage_report_artifact, :running, project: project) } + let(:pipeline) { create(:ci_pipeline, :with_coverage_report_artifact, :running) } it { expect(subject).to be_truthy } end context 'when pipeline does not have a code coverage artifact' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { expect(subject).to be_falsey } end @@ -3485,17 +3509,17 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline has builds with coverage reports' do before do - create(:ci_build, :coverage_reports, pipeline: pipeline, project: project) + create(:ci_build, :coverage_reports, pipeline: pipeline) end context 'when pipeline status is running' do - let(:pipeline) { create(:ci_pipeline, :running, project: project) } + let(:pipeline) { create(:ci_pipeline, :running) } it { expect(subject).to be_falsey } end context 'when pipeline status is success' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { expect(subject).to be_truthy } end @@ -3503,10 +3527,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline does not have builds with coverage reports' do before do - create(:ci_build, :artifacts, pipeline: pipeline, project: project) + create(:ci_build, :artifacts, pipeline: pipeline) end - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { expect(subject).to be_falsey } end @@ -3516,13 +3540,13 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { pipeline.has_codequality_mr_diff_report? } context 'when pipeline has a codequality mr diff report' do - let(:pipeline) { create(:ci_pipeline, :with_codequality_mr_diff_report, :running, project: project) } + let(:pipeline) { create(:ci_pipeline, :with_codequality_mr_diff_report, :running) } it { expect(subject).to be_truthy } end context 'when pipeline does not have a codequality mr diff report' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { expect(subject).to be_falsey } end @@ -3533,17 +3557,17 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline has builds with codequality reports' do before do - create(:ci_build, :codequality_reports, pipeline: pipeline, project: project) + create(:ci_build, :codequality_reports, pipeline: pipeline) end context 'when pipeline status is running' do - let(:pipeline) { create(:ci_pipeline, :running, project: project) } + let(:pipeline) { create(:ci_pipeline, :running) } it { expect(subject).to be_falsey } end context 'when pipeline status is success' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it 'can generate a codequality report' do expect(subject).to be_truthy @@ -3563,10 +3587,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline does not have builds with codequality reports' do before do - create(:ci_build, :artifacts, pipeline: pipeline, project: project) + create(:ci_build, :artifacts, pipeline: pipeline) end - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } it { expect(subject).to be_falsey } end @@ -3575,12 +3599,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#test_report_summary' do subject { pipeline.test_report_summary } - context 'when pipeline has multiple builds with report results' do - let(:pipeline) { create(:ci_pipeline, :success, project: project) } + let(:pipeline) { create(:ci_pipeline, :success) } + context 'when pipeline has multiple builds with report results' do before do - create(:ci_build, :success, :report_results, name: 'rspec', pipeline: pipeline, project: project) - create(:ci_build, :success, :report_results, name: 'java', pipeline: pipeline, project: project) + create(:ci_build, :success, :report_results, name: 'rspec', pipeline: pipeline) + create(:ci_build, :success, :report_results, name: 'java', pipeline: pipeline) end it 'returns test report summary with collected data' do @@ -3598,13 +3622,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#test_reports' do subject { pipeline.test_reports } + let_it_be(:pipeline) { create(:ci_pipeline) } + context 'when pipeline has multiple builds with test reports' do - let!(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project) } - let!(:build_java) { create(:ci_build, :success, name: 'java', pipeline: pipeline, project: project) } + let!(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline) } + let!(:build_java) { create(:ci_build, :success, name: 'java', pipeline: pipeline) } before do - create(:ci_job_artifact, :junit, job: build_rspec, project: project) - create(:ci_job_artifact, :junit_with_ant, job: build_java, project: project) + create(:ci_job_artifact, :junit, job: build_rspec) + create(:ci_job_artifact, :junit_with_ant, job: build_java) end it 'returns test reports with collected data' do @@ -3614,8 +3640,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when builds are retried' do - let!(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline, project: project) } - let!(:build_java) { create(:ci_build, :retried, :success, name: 'java', pipeline: pipeline, project: project) } + let!(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline) } + let!(:build_java) { create(:ci_build, :retried, :success, name: 'java', pipeline: pipeline) } it 'does not take retried builds into account' do expect(subject.total_count).to be(0) @@ -3635,13 +3661,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#accessibility_reports' do subject { pipeline.accessibility_reports } + let_it_be(:pipeline) { create(:ci_pipeline) } + context 'when pipeline has multiple builds with accessibility reports' do - let(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project) } - let(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline, project: project) } + let(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline) } + let(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline) } before do - create(:ci_job_artifact, :accessibility, job: build_rspec, project: project) - create(:ci_job_artifact, :accessibility_without_errors, job: build_golang, project: project) + create(:ci_job_artifact, :accessibility, job: build_rspec) + create(:ci_job_artifact, :accessibility_without_errors, job: build_golang) end it 'returns accessibility report with collected data' do @@ -3652,8 +3680,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when builds are retried' do - let(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline, project: project) } - let(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline, project: project) } + let(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline) } + let(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline) } it 'returns empty urls for accessibility reports' do expect(subject.urls).to be_empty @@ -3671,13 +3699,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#coverage_reports' do subject { pipeline.coverage_reports } + let_it_be(:pipeline) { create(:ci_pipeline) } + context 'when pipeline has multiple builds with coverage reports' do - let!(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project) } - let!(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline, project: project) } + let!(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline) } + let!(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline) } before do - create(:ci_job_artifact, :cobertura, job: build_rspec, project: project) - create(:ci_job_artifact, :coverage_gocov_xml, job: build_golang, project: project) + create(:ci_job_artifact, :cobertura, job: build_rspec) + create(:ci_job_artifact, :coverage_gocov_xml, job: build_golang) end it 'returns coverage reports with collected data' do @@ -3689,8 +3719,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it 'does not execute N+1 queries' do - single_build_pipeline = create(:ci_empty_pipeline, status: :created, project: project) - single_rspec = create(:ci_build, :success, name: 'rspec', pipeline: single_build_pipeline, project: project) + single_build_pipeline = create(:ci_empty_pipeline, :created) + single_rspec = create(:ci_build, :success, name: 'rspec', pipeline: single_build_pipeline) create(:ci_job_artifact, :cobertura, job: single_rspec, project: project) control = ActiveRecord::QueryRecorder.new { single_build_pipeline.coverage_reports } @@ -3699,8 +3729,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when builds are retried' do - let!(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline, project: project) } - let!(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline, project: project) } + let!(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline) } + let!(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline) } it 'does not take retried builds into account' do expect(subject.files).to eql({}) @@ -3718,13 +3748,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#codequality_reports' do subject(:codequality_reports) { pipeline.codequality_reports } + let_it_be(:pipeline) { create(:ci_pipeline) } + context 'when pipeline has multiple builds with codequality reports' do - let(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline, project: project) } - let(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline, project: project) } + let(:build_rspec) { create(:ci_build, :success, name: 'rspec', pipeline: pipeline) } + let(:build_golang) { create(:ci_build, :success, name: 'golang', pipeline: pipeline) } before do - create(:ci_job_artifact, :codequality, job: build_rspec, project: project) - create(:ci_job_artifact, :codequality_without_errors, job: build_golang, project: project) + create(:ci_job_artifact, :codequality, job: build_rspec) + create(:ci_job_artifact, :codequality_without_errors, job: build_golang) end it 'returns codequality report with collected data' do @@ -3732,8 +3764,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when builds are retried' do - let(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline, project: project) } - let(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline, project: project) } + let(:build_rspec) { create(:ci_build, :retried, :success, name: 'rspec', pipeline: pipeline) } + let(:build_golang) { create(:ci_build, :retried, :success, name: 'golang', pipeline: pipeline) } it 'returns a codequality reports without degradations' do expect(codequality_reports.degradations).to be_empty @@ -3749,6 +3781,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#total_size' do + let(:pipeline) { create(:ci_pipeline) } let!(:build_job1) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } let!(:build_job2) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } let!(:test_job_failed_and_retried) { create(:ci_build, :failed, :retried, pipeline: pipeline, stage_idx: 1) } @@ -3785,17 +3818,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#default_branch?' do - let(:default_branch) { 'master'} - subject { pipeline.default_branch? } - before do - allow(project).to receive(:default_branch).and_return(default_branch) - end - context 'when pipeline ref is the default branch of the project' do let(:pipeline) do - build(:ci_empty_pipeline, status: :created, project: project, ref: default_branch) + build(:ci_empty_pipeline, :created, project: project, ref: project.default_branch) end it "returns true" do @@ -3805,7 +3832,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline ref is not the default branch of the project' do let(:pipeline) do - build(:ci_empty_pipeline, status: :created, project: project, ref: 'another_branch') + build(:ci_empty_pipeline, :created, project: project, ref: 'another_branch') end it "returns false" do @@ -3815,7 +3842,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#find_stage_by_name' do - let(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline) { create(:ci_pipeline) } let(:stage_name) { 'test' } let(:stage) do @@ -3895,10 +3922,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#parent_pipeline' do - let_it_be(:project) { create(:project) } + let_it_be_with_reload(:pipeline) { create(:ci_pipeline) } context 'when pipeline is triggered by a pipeline from the same project' do - let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:upstream_pipeline) { create(:ci_pipeline) } let_it_be(:pipeline) { create(:ci_pipeline, child_of: upstream_pipeline) } it 'returns the parent pipeline' do @@ -3911,7 +3938,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is triggered by a pipeline from another project' do - let(:pipeline) { create(:ci_pipeline, project: project) } + let(:pipeline) { create(:ci_pipeline) } let!(:upstream_pipeline) { create(:ci_pipeline, project: create(:project), upstream_of: pipeline) } it 'returns nil' do @@ -3938,7 +3965,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#child_pipelines' do let_it_be(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, project: project) } context 'when pipeline triggered other pipelines on same project' do let(:downstream_pipeline) { create(:ci_pipeline, project: pipeline.project) } @@ -3992,6 +4019,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'upstream status interactions' do + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, :created) } + context 'when a pipeline has an upstream status' do context 'when an upstream status is a bridge' do let(:bridge) { create(:ci_bridge, status: :pending) } @@ -4050,6 +4079,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#source_ref_path' do subject { pipeline.source_ref_path } + let(:pipeline) { create(:ci_pipeline, :created) } + context 'when pipeline is for a branch' do it { is_expected.to eq(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.source_ref.to_s) } end @@ -4062,13 +4093,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is for a tag' do - let(:pipeline) { create(:ci_pipeline, project: project, tag: true) } + let(:pipeline) { create(:ci_pipeline, tag: true) } it { is_expected.to eq(Gitlab::Git::TAG_REF_PREFIX + pipeline.source_ref.to_s) } end end - describe "#builds_with_coverage" do + describe '#builds_with_coverage' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + it 'returns builds with coverage only' do rspec = create(:ci_build, name: 'rspec', coverage: 97.1, pipeline: pipeline) jest = create(:ci_build, name: 'jest', coverage: 94.1, pipeline: pipeline) @@ -4092,10 +4125,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#base_and_ancestors' do - let(:same_project) { false } - subject { pipeline.base_and_ancestors(same_project: same_project) } + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + let(:same_project) { false } + context 'when pipeline is not child nor parent' do it 'returns just the pipeline itself' do expect(subject).to contain_exactly(pipeline) @@ -4103,8 +4137,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is child' do - let(:parent) { create(:ci_pipeline, project: pipeline.project) } - let(:sibling) { create(:ci_pipeline, project: pipeline.project) } + let(:parent) { create(:ci_pipeline) } + let(:sibling) { create(:ci_pipeline) } before do create_source_pipeline(parent, pipeline) @@ -4117,7 +4151,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is parent' do - let(:child) { create(:ci_pipeline, project: pipeline.project) } + let(:child) { create(:ci_pipeline) } before do create_source_pipeline(pipeline, child) @@ -4129,8 +4163,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is a child of a child pipeline' do - let(:ancestor) { create(:ci_pipeline, project: pipeline.project) } - let(:parent) { create(:ci_pipeline, project: pipeline.project) } + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + let(:ancestor) { create(:ci_pipeline) } + let(:parent) { create(:ci_pipeline) } before do create_source_pipeline(ancestor, parent) @@ -4143,6 +4178,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline is a triggered pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } let(:upstream) { create(:ci_pipeline, project: create(:project)) } before do @@ -4166,8 +4202,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'reset_ancestor_bridges!' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + context 'when the pipeline is a child pipeline and the bridge is depended' do - let!(:parent_pipeline) { create(:ci_pipeline, project: project) } + let!(:parent_pipeline) { create(:ci_pipeline) } let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) } it 'marks source bridge as pending' do @@ -4191,7 +4229,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when the pipeline is a child pipeline and the bridge is not depended' do - let!(:parent_pipeline) { create(:ci_pipeline, project: project) } + let!(:parent_pipeline) { create(:ci_pipeline) } let!(:bridge) { create_bridge(parent_pipeline, pipeline, false) } it 'does not touch source bridge' do @@ -4227,6 +4265,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe 'test failure history processing' do + let(:pipeline) { build(:ci_pipeline, :created) } + it 'performs the service asynchronously when the pipeline is completed' do service = double @@ -4238,21 +4278,23 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#latest_test_report_builds' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + it 'returns pipeline builds with test report artifacts' do - test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) + test_build = create(:ci_build, :test_reports, pipeline: pipeline) create(:ci_build, :artifacts, pipeline: pipeline, project: project) expect(pipeline.latest_test_report_builds).to contain_exactly(test_build) end it 'preloads project on each build to avoid N+1 queries' do - create(:ci_build, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :test_reports, pipeline: pipeline) control_count = ActiveRecord::QueryRecorder.new do pipeline.latest_test_report_builds.map(&:project).map(&:full_path) end - multi_build_pipeline = create(:ci_empty_pipeline, status: :created, project: project) + multi_build_pipeline = create(:ci_empty_pipeline, :created) create(:ci_build, :test_reports, pipeline: multi_build_pipeline, project: project) create(:ci_build, :test_reports, pipeline: multi_build_pipeline, project: project) @@ -4262,30 +4304,32 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#builds_with_failed_tests' do + let_it_be(:pipeline) { create(:ci_pipeline, :created) } + it 'returns pipeline builds with test report artifacts' do - failed_build = create(:ci_build, :failed, :test_reports, pipeline: pipeline, project: project) - create(:ci_build, :success, :test_reports, pipeline: pipeline, project: project) + failed_build = create(:ci_build, :failed, :test_reports, pipeline: pipeline) + create(:ci_build, :success, :test_reports, pipeline: pipeline) expect(pipeline.builds_with_failed_tests).to contain_exactly(failed_build) end it 'supports limiting the number of builds to fetch' do - create(:ci_build, :failed, :test_reports, pipeline: pipeline, project: project) - create(:ci_build, :failed, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :failed, :test_reports, pipeline: pipeline) + create(:ci_build, :failed, :test_reports, pipeline: pipeline) expect(pipeline.builds_with_failed_tests(limit: 1).count).to eq(1) end it 'preloads project on each build to avoid N+1 queries' do - create(:ci_build, :failed, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :failed, :test_reports, pipeline: pipeline) control_count = ActiveRecord::QueryRecorder.new do pipeline.builds_with_failed_tests.map(&:project).map(&:full_path) end - multi_build_pipeline = create(:ci_empty_pipeline, status: :created, project: project) - create(:ci_build, :failed, :test_reports, pipeline: multi_build_pipeline, project: project) - create(:ci_build, :failed, :test_reports, pipeline: multi_build_pipeline, project: project) + multi_build_pipeline = create(:ci_empty_pipeline, :created) + create(:ci_build, :failed, :test_reports, pipeline: multi_build_pipeline) + create(:ci_build, :failed, :test_reports, pipeline: multi_build_pipeline) expect { multi_build_pipeline.builds_with_failed_tests.map(&:project).map(&:full_path) } .not_to exceed_query_limit(control_count) diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 6290f4aef16..0a43f785598 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -112,8 +112,8 @@ RSpec.describe Ci::Processable do it 'returns all needs attributes' do is_expected.to contain_exactly( - { 'artifacts' => true, 'name' => 'test1' }, - { 'artifacts' => true, 'name' => 'test2' } + { 'artifacts' => true, 'name' => 'test1', 'optional' => false }, + { 'artifacts' => true, 'name' => 'test2', 'optional' => false } ) end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 3e5d068d780..ff3551d2a18 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -40,41 +40,39 @@ RSpec.describe Ci::Runner do context 'runner_type validations' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project) } - let(:group_runner) { create(:ci_runner, :group, groups: [group]) } - let(:project_runner) { create(:ci_runner, :project, projects: [project]) } - let(:instance_runner) { create(:ci_runner, :instance) } it 'disallows assigning group to project_type runner' do - project_runner.groups << build(:group) + project_runner = build(:ci_runner, :project, groups: [group]) expect(project_runner).not_to be_valid expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned') end it 'disallows assigning group to instance_type runner' do - instance_runner.groups << build(:group) + instance_runner = build(:ci_runner, :instance, groups: [group]) expect(instance_runner).not_to be_valid expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned') end it 'disallows assigning project to group_type runner' do - group_runner.projects << build(:project) + group_runner = build(:ci_runner, :instance, projects: [project]) expect(group_runner).not_to be_valid expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned') end it 'disallows assigning project to instance_type runner' do - instance_runner.projects << build(:project) + instance_runner = build(:ci_runner, :instance, projects: [project]) expect(instance_runner).not_to be_valid expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned') end it 'fails to save a group assigned to a project runner even if the runner is already saved' do - group.runners << project_runner - expect { group.save! } + project_runner = create(:ci_runner, :project, projects: [project]) + + expect { create(:group, runners: [project_runner]) } .to raise_error(ActiveRecord::RecordInvalid) end end @@ -352,6 +350,8 @@ RSpec.describe Ci::Runner do end describe '#can_pick?' do + using RSpec::Parameterized::TableSyntax + let_it_be(:pipeline) { create(:ci_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } let(:runner_project) { build.project } @@ -365,6 +365,11 @@ RSpec.describe Ci::Runner do let(:other_project) { create(:project) } let(:other_runner) { create(:ci_runner, :project, projects: [other_project], tag_list: tag_list, run_untagged: run_untagged) } + before do + # `can_pick?` is not used outside the runners available for the project + stub_feature_flags(ci_runners_short_circuit_assignable_for: false) + end + it 'cannot handle builds' do expect(other_runner.can_pick?(build)).to be_falsey end @@ -432,9 +437,32 @@ RSpec.describe Ci::Runner do expect(runner.can_pick?(build)).to be_truthy end end + + it 'does not query for owned or instance runners' do + expect(described_class).not_to receive(:owned_or_instance_wide) + + runner.can_pick?(build) + end + + context 'when feature flag ci_runners_short_circuit_assignable_for is disabled' do + before do + stub_feature_flags(ci_runners_short_circuit_assignable_for: false) + end + + it 'does not query for owned or instance runners' do + expect(described_class).to receive(:owned_or_instance_wide).and_call_original + + runner.can_pick?(build) + end + end end context 'when runner is not shared' do + before do + # `can_pick?` is not used outside the runners available for the project + stub_feature_flags(ci_runners_short_circuit_assignable_for: false) + end + context 'when runner is assigned to a project' do it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -502,6 +530,29 @@ RSpec.describe Ci::Runner do it { is_expected.to be_falsey } end end + + context 'matches tags' do + where(:run_untagged, :runner_tags, :build_tags, :result) do + true | [] | [] | true + true | [] | ['a'] | false + true | %w[a b] | ['a'] | true + true | ['a'] | %w[a b] | false + true | ['a'] | ['a'] | true + false | ['a'] | ['a'] | true + false | ['b'] | ['a'] | false + false | %w[a b] | ['a'] | true + end + + with_them do + let(:tag_list) { runner_tags } + + before do + build.tag_list = build_tags + end + + it { is_expected.to eq(result) } + end + end end describe '#status' do @@ -844,27 +895,50 @@ RSpec.describe Ci::Runner do end describe '#pick_build!' do + let(:build) { create(:ci_build) } + let(:runner) { create(:ci_runner) } + context 'runner can pick the build' do it 'calls #tick_runner_queue' do - ci_build = build(:ci_build) - runner = build(:ci_runner) - allow(runner).to receive(:can_pick?).with(ci_build).and_return(true) - expect(runner).to receive(:tick_runner_queue) - runner.pick_build!(ci_build) + runner.pick_build!(build) end end context 'runner cannot pick the build' do - it 'does not call #tick_runner_queue' do - ci_build = build(:ci_build) - runner = build(:ci_runner) - allow(runner).to receive(:can_pick?).with(ci_build).and_return(false) + before do + build.tag_list = [:docker] + end + it 'does not call #tick_runner_queue' do expect(runner).not_to receive(:tick_runner_queue) - runner.pick_build!(ci_build) + runner.pick_build!(build) + end + end + + context 'build picking improvement enabled' do + before do + stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: true) + end + + 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 diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 26a7a2596af..93a24ba9157 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -14,6 +14,15 @@ RSpec.describe Ci::Variable do it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) } end + describe '.by_environment_scope' do + let!(:matching_variable) { create(:ci_variable, environment_scope: 'production ') } + let!(:non_matching_variable) { create(:ci_variable, environment_scope: 'staging') } + + subject { Ci::Variable.by_environment_scope('production') } + + it { is_expected.to contain_exactly(matching_variable) } + end + describe '.unprotected' do subject { described_class.unprotected } diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index 5cb84ee131a..a1b45df1970 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -3,8 +3,11 @@ require 'spec_helper' RSpec.describe Clusters::AgentToken do - it { is_expected.to belong_to(:agent).class_name('Clusters::Agent') } + it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required } it { is_expected.to belong_to(:created_by_user).class_name('User').optional } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_presence_of(:name) } describe '#token' do it 'is generated on save' do diff --git a/spec/models/concerns/ci/has_variable_spec.rb b/spec/models/concerns/ci/has_variable_spec.rb index b5390281064..e917ec6b802 100644 --- a/spec/models/concerns/ci/has_variable_spec.rb +++ b/spec/models/concerns/ci/has_variable_spec.rb @@ -11,6 +11,17 @@ RSpec.describe Ci::HasVariable do it { is_expected.not_to allow_value('foo bar').for(:key) } it { is_expected.not_to allow_value('foo/bar').for(:key) } + describe 'scopes' do + describe '.by_key' do + let!(:matching_variable) { create(:ci_variable, key: 'example') } + let!(:non_matching_variable) { create(:ci_variable, key: 'other') } + + subject { Ci::Variable.by_key('example') } + + it { is_expected.to contain_exactly(matching_variable) } + end + end + describe '#key=' do context 'when the new key is nil' do it 'strips leading and trailing whitespaces' do diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 2059e170446..62c9a041a85 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' RSpec.describe ProjectFeaturesCompatibility do let(:project) { create(:project) } - let(:features_enabled) { %w(issues wiki builds merge_requests snippets) } - let(:features) { features_enabled + %w(repository pages operations) } + let(:features_enabled) { %w(issues wiki builds merge_requests snippets security_and_compliance) } + let(:features) { features_enabled + %w(repository pages operations container_registry) } # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table # All those fields got moved to a new table called project_feature and are now integers instead of booleans diff --git a/spec/models/custom_emoji_spec.rb b/spec/models/custom_emoji_spec.rb index 41ce480b02f..e34934d393a 100644 --- a/spec/models/custom_emoji_spec.rb +++ b/spec/models/custom_emoji_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' RSpec.describe CustomEmoji do describe 'Associations' do - it { is_expected.to belong_to(:namespace) } + it { is_expected.to belong_to(:namespace).inverse_of(:custom_emoji) } + it { is_expected.to belong_to(:creator).inverse_of(:created_custom_emoji) } it { is_expected.to have_db_column(:file) } + it { is_expected.to validate_presence_of(:creator) } it { is_expected.to validate_length_of(:name).is_at_most(36) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to have_db_column(:external) } @@ -36,7 +38,7 @@ RSpec.describe CustomEmoji do new_emoji = build(:custom_emoji, name: old_emoji.name, namespace: old_emoji.namespace, group: group) expect(new_emoji).not_to be_valid - expect(new_emoji.errors.messages).to eq(name: ["has already been taken"]) + expect(new_emoji.errors.messages).to include(name: ["has already been taken"]) end it 'disallows non http and https file value' do diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index aa2e73356dd..4203644c003 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -29,24 +29,32 @@ RSpec.describe DependencyProxy::Manifest, type: :model do end end - describe '.find_or_initialize_by_file_name' do - subject { DependencyProxy::Manifest.find_or_initialize_by_file_name(file_name) } + describe '.find_or_initialize_by_file_name_or_digest' do + let_it_be(:file_name) { 'foo' } + let_it_be(:digest) { 'bar' } - context 'no manifest exists' do - let_it_be(:file_name) { 'foo' } + subject { DependencyProxy::Manifest.find_or_initialize_by_file_name_or_digest(file_name: file_name, digest: digest) } + context 'no manifest exists' do it 'initializes a manifest' do - expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name) + expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name, digest: digest) subject end end - context 'manifest exists' do + context 'manifest exists and matches file_name' do let_it_be(:dependency_proxy_manifest) { create(:dependency_proxy_manifest) } let_it_be(:file_name) { dependency_proxy_manifest.file_name } it { is_expected.to eq(dependency_proxy_manifest) } end + + context 'manifest exists and matches digest' do + let_it_be(:dependency_proxy_manifest) { create(:dependency_proxy_manifest) } + let_it_be(:digest) { dependency_proxy_manifest.digest } + + it { is_expected.to eq(dependency_proxy_manifest) } + end end end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index ffdc621dd4c..62f2a53ab3c 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Email do end describe 'validations' do - it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :email do + it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do subject { build(:email) } end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 0c7d8e2969d..e021a6cf6d3 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -34,6 +34,27 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do it { is_expected.to validate_length_of(:external_url).is_at_most(255) } + describe '.before_save' do + it 'ensures environment tier when a new object is created' do + environment = build(:environment, name: 'gprd', tier: nil) + + expect { environment.save }.to change { environment.tier }.from(nil).to('production') + end + + it 'ensures environment tier when an existing object is updated' do + environment = create(:environment, name: 'gprd') + environment.update_column(:tier, nil) + + expect { environment.stop! }.to change { environment.reload.tier }.from(nil).to('production') + end + + it 'does not overwrite the existing environment tier' do + environment = create(:environment, name: 'gprd', tier: :production) + + expect { environment.update!(name: 'gstg') }.not_to change { environment.reload.tier } + end + end + describe '.order_by_last_deployed_at' do let!(:environment1) { create(:environment, project: project) } let!(:environment2) { create(:environment, project: project) } @@ -51,6 +72,62 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe ".stopped_review_apps" do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:old_stopped_review_env) { create(:environment, :with_review_app, :stopped, created_at: 31.days.ago, project: project) } + let_it_be(:new_stopped_review_env) { create(:environment, :with_review_app, :stopped, project: project) } + let_it_be(:old_active_review_env) { create(:environment, :with_review_app, :available, created_at: 31.days.ago, project: project) } + let_it_be(:old_stopped_other_env) { create(:environment, :stopped, created_at: 31.days.ago, project: project) } + let_it_be(:new_stopped_other_env) { create(:environment, :stopped, project: project) } + let_it_be(:old_active_other_env) { create(:environment, :available, created_at: 31.days.ago, project: project) } + + let(:before) { 30.days.ago } + let(:limit) { 1000 } + + subject { project.environments.stopped_review_apps(before, limit) } # rubocop: disable RSpec/SingleLineHook + + it { is_expected.to contain_exactly(old_stopped_review_env) } + + context "current timestamp" do + let(:before) { Time.zone.now } + + it { is_expected.to contain_exactly(old_stopped_review_env, new_stopped_review_env) } + end + end + + describe "scheduled deletion" do + let_it_be(:deletable_environment) { create(:environment, auto_delete_at: Time.zone.now) } + let_it_be(:undeletable_environment) { create(:environment, auto_delete_at: nil) } + + describe ".scheduled_for_deletion" do + subject { described_class.scheduled_for_deletion } + + it { is_expected.to contain_exactly(deletable_environment) } + end + + describe ".not_scheduled_for_deletion" do + subject { described_class.not_scheduled_for_deletion } + + it { is_expected.to contain_exactly(undeletable_environment) } + end + + describe ".schedule_to_delete" do + subject { described_class.for_id(deletable_environment).schedule_to_delete } + + it "schedules the record for deletion" do + freeze_time do + subject + + deletable_environment.reload + undeletable_environment.reload + + expect(deletable_environment.auto_delete_at).to eq(1.week.from_now) + expect(undeletable_environment.auto_delete_at).to be_nil + end + end + end + end + describe 'state machine' do it 'invalidates the cache after a change' do expect(environment).to receive(:expire_etag_cache) @@ -195,6 +272,62 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '.for_tier' do + let_it_be(:environment) { create(:environment, :production) } + + it 'returns the production environment when searching for production tier' do + expect(described_class.for_tier(:production)).to eq([environment]) + end + + it 'returns nothing when searching for staging tier' do + expect(described_class.for_tier(:staging)).to be_empty + end + end + + describe '#guess_tier' do + using RSpec::Parameterized::TableSyntax + + subject { environment.send(:guess_tier) } + + let(:environment) { build(:environment, name: name) } + + where(:name, :tier) do + 'review/feature' | described_class.tiers[:development] + 'review/product' | described_class.tiers[:development] + 'DEV' | described_class.tiers[:development] + 'development' | described_class.tiers[:development] + 'trunk' | described_class.tiers[:development] + 'test' | described_class.tiers[:testing] + 'TEST' | described_class.tiers[:testing] + 'testing' | described_class.tiers[:testing] + 'testing-prd' | described_class.tiers[:testing] + 'acceptance-testing' | described_class.tiers[:testing] + 'QC' | described_class.tiers[:testing] + 'gstg' | described_class.tiers[:staging] + 'staging' | described_class.tiers[:staging] + 'stage' | described_class.tiers[:staging] + 'Model' | described_class.tiers[:staging] + 'MODL' | described_class.tiers[:staging] + 'Pre-production' | described_class.tiers[:staging] + 'pre' | described_class.tiers[:staging] + 'Demo' | described_class.tiers[:staging] + 'gprd' | described_class.tiers[:production] + 'gprd-cny' | described_class.tiers[:production] + 'production' | described_class.tiers[:production] + 'Production' | described_class.tiers[:production] + 'prod' | described_class.tiers[:production] + 'PROD' | described_class.tiers[:production] + 'Live' | described_class.tiers[:production] + 'canary' | described_class.tiers[:other] + 'other' | described_class.tiers[:other] + 'EXP' | described_class.tiers[:other] + end + + with_them do + it { is_expected.to eq(tier) } + end + end + describe '#expire_etag_cache' do let(:store) { Gitlab::EtagCaching::Store.new } diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 72ed11f6c74..3ae0666f7d0 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -111,7 +111,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '#sentry_client' do it 'returns sentry client' do - expect(subject.sentry_client).to be_a(Sentry::Client) + expect(subject.sentry_client).to be_a(ErrorTracking::SentryClient) end end @@ -152,7 +152,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'when sentry client raises Sentry::Client::Error' do + context 'when sentry client raises ErrorTracking::SentryClient::Error' do let(:sentry_client) { spy(:sentry_client) } before do @@ -160,7 +160,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(Sentry::Client::Error, 'error message') + .and_raise(ErrorTracking::SentryClient::Error, 'error message') end it 'returns error' do @@ -171,7 +171,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'when sentry client raises Sentry::Client::MissingKeysError' do + context 'when sentry client raises ErrorTracking::SentryClient::MissingKeysError' do let(:sentry_client) { spy(:sentry_client) } before do @@ -179,7 +179,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') + .and_raise(ErrorTracking::SentryClient::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') end it 'returns error' do @@ -190,7 +190,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'when sentry client raises Sentry::Client::ResponseInvalidSizeError' do + context 'when sentry client raises ErrorTracking::SentryClient::ResponseInvalidSizeError' do let(:sentry_client) { spy(:sentry_client) } let(:error_msg) {"Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."} @@ -199,7 +199,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(Sentry::Client::ResponseInvalidSizeError, error_msg) + .and_raise(ErrorTracking::SentryClient::ResponseInvalidSizeError, error_msg) end it 'returns error' do diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 22bbf2df8fd..09dd1766acc 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -98,10 +98,11 @@ RSpec.describe Experiment do describe '.record_conversion_event' do let_it_be(:user) { build(:user) } + let_it_be(:context) { { a: 42 } } let(:experiment_key) { :test_experiment } - subject(:record_conversion_event) { described_class.record_conversion_event(experiment_key, user) } + subject(:record_conversion_event) { described_class.record_conversion_event(experiment_key, user, context) } context 'when no matching experiment exists' do it 'creates the experiment and uses it' do @@ -127,22 +128,79 @@ RSpec.describe Experiment do it 'sends record_conversion_event_for_user to the experiment instance' do expect_next_found_instance_of(described_class) do |experiment| - expect(experiment).to receive(:record_conversion_event_for_user).with(user) + expect(experiment).to receive(:record_conversion_event_for_user).with(user, context) end record_conversion_event end end end + shared_examples 'experiment user with context' do + let_it_be(:context) { { a: 42, 'b' => 34, 'c': { c1: 100, c2: 'c2', e: :e }, d: [1, 3] } } + let_it_be(:initial_expected_context) { { 'a' => 42, 'b' => 34, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [1, 3] } } + + before do + subject + experiment.record_user_and_group(user, :experimental, {}) + end + + it 'has an initial context with stringified keys' do + expect(ExperimentUser.last.context).to eq(initial_expected_context) + end + + context 'when updated' do + before do + subject + experiment.record_user_and_group(user, :experimental, new_context) + end + + context 'with an empty context' do + let_it_be(:new_context) { {} } + + it 'keeps the initial context' do + expect(ExperimentUser.last.context).to eq(initial_expected_context) + end + end + + context 'with string keys' do + let_it_be(:new_context) { { f: :some_symbol } } + + it 'adds new symbols stringified' do + expected_context = initial_expected_context.merge('f' => 'some_symbol') + expect(ExperimentUser.last.context).to eq(expected_context) + end + end + + context 'with atomic values or array values' do + let_it_be(:new_context) { { b: 97, d: [99] } } + + it 'overrides the values' do + expected_context = { 'a' => 42, 'b' => 97, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [99] } + expect(ExperimentUser.last.context).to eq(expected_context) + end + end + + context 'with nested hashes' do + let_it_be(:new_context) { { c: { g: 107 } } } + + it 'inserts nested additional values in the same keys' do + expected_context = initial_expected_context.deep_merge('c' => { 'g' => 107 }) + expect(ExperimentUser.last.context).to eq(expected_context) + end + end + end + end + describe '#record_conversion_event_for_user' do let_it_be(:user) { create(:user) } let_it_be(:experiment) { create(:experiment) } + let_it_be(:context) { { a: 42 } } - subject(:record_conversion_event_for_user) { experiment.record_conversion_event_for_user(user) } + subject { experiment.record_conversion_event_for_user(user, context) } context 'when no existing experiment_user record exists for the given user' do it 'does not update or create an experiment_user record' do - expect { record_conversion_event_for_user }.not_to change { ExperimentUser.all.to_a } + expect { subject }.not_to change { ExperimentUser.all.to_a } end end @@ -151,7 +209,13 @@ RSpec.describe Experiment do let!(:experiment_user) { create(:experiment_user, experiment: experiment, user: user, converted_at: 2.days.ago) } it 'does not update the converted_at value' do - expect { record_conversion_event_for_user }.not_to change { experiment_user.converted_at } + expect { subject }.not_to change { experiment_user.converted_at } + end + + it_behaves_like 'experiment user with context' do + before do + experiment.record_user_and_group(user, :experimental, context) + end end end @@ -159,7 +223,13 @@ RSpec.describe Experiment do let(:experiment_user) { create(:experiment_user, experiment: experiment, user: user) } it 'updates the converted_at value' do - expect { record_conversion_event_for_user }.to change { experiment_user.reload.converted_at } + expect { subject }.to change { experiment_user.reload.converted_at } + end + + it_behaves_like 'experiment user with context' do + before do + experiment.record_user_and_group(user, :experimental, context) + end end end end @@ -196,24 +266,25 @@ RSpec.describe Experiment do describe '#record_user_and_group' do let_it_be(:experiment) { create(:experiment) } let_it_be(:user) { create(:user) } + let_it_be(:group) { :control } + let_it_be(:context) { { a: 42 } } - let(:group) { :control } - let(:context) { { a: 42 } } - - subject(:record_user_and_group) { experiment.record_user_and_group(user, group, context) } + subject { experiment.record_user_and_group(user, group, context) } context 'when an experiment_user does not yet exist for the given user' do it 'creates a new experiment_user record' do - expect { record_user_and_group }.to change(ExperimentUser, :count).by(1) + expect { subject }.to change(ExperimentUser, :count).by(1) end it 'assigns the correct group_type to the experiment_user' do - record_user_and_group + subject + expect(ExperimentUser.last.group_type).to eq('control') end it 'adds the correct context to the experiment_user' do - record_user_and_group + subject + expect(ExperimentUser.last.context).to eq({ 'a' => 42 }) end end @@ -225,72 +296,18 @@ RSpec.describe Experiment do end it 'does not create a new experiment_user record' do - expect { record_user_and_group }.not_to change(ExperimentUser, :count) + expect { subject }.not_to change(ExperimentUser, :count) end context 'but the group_type and context has changed' do let(:group) { :experimental } it 'updates the existing experiment_user record with group_type' do - expect { record_user_and_group }.to change { ExperimentUser.last.group_type } + expect { subject }.to change { ExperimentUser.last.group_type } end end - end - - context 'when a context already exists' do - let_it_be(:context) { { a: 42, 'b' => 34, 'c': { c1: 100, c2: 'c2', e: :e }, d: [1, 3] } } - let_it_be(:initial_expected_context) { { 'a' => 42, 'b' => 34, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [1, 3] } } - - before do - record_user_and_group - experiment.record_user_and_group(user, :control, {}) - end - - it 'has an initial context with stringified keys' do - expect(ExperimentUser.last.context).to eq(initial_expected_context) - end - - context 'when updated' do - before do - record_user_and_group - experiment.record_user_and_group(user, :control, new_context) - end - - context 'with an empty context' do - let_it_be(:new_context) { {} } - it 'keeps the initial context' do - expect(ExperimentUser.last.context).to eq(initial_expected_context) - end - end - - context 'with string keys' do - let_it_be(:new_context) { { f: :some_symbol } } - - it 'adds new symbols stringified' do - expected_context = initial_expected_context.merge('f' => 'some_symbol') - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - - context 'with atomic values or array values' do - let_it_be(:new_context) { { b: 97, d: [99] } } - - it 'overrides the values' do - expected_context = { 'a' => 42, 'b' => 97, 'c' => { 'c1' => 100, 'c2' => 'c2', 'e' => 'e' }, 'd' => [99] } - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - - context 'with nested hashes' do - let_it_be(:new_context) { { c: { g: 107 } } } - - it 'inserts nested additional values in the same keys' do - expected_context = initial_expected_context.deep_merge('c' => { 'g' => 107 }) - expect(ExperimentUser.last.context).to eq(expected_context) - end - end - end + it_behaves_like 'experiment user with context' end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e79b54b4674..24d09d1c035 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -25,7 +25,6 @@ RSpec.describe Group do it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') } it { is_expected.to have_many(:container_repositories) } it { is_expected.to have_many(:milestones) } - it { is_expected.to have_many(:iterations) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:services) } it { is_expected.to have_one(:dependency_proxy_setting) } @@ -65,6 +64,59 @@ RSpec.describe Group do it { is_expected.to validate_presence_of :two_factor_grace_period } it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) } + context 'validating the parent of a group' do + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'does not allow a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('a group cannot have a user namespace as its parent') + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + + context 'when the feature flag `validate_namespace_parent_type` is disabled' do + before do + stub_feature_flags(validate_namespace_parent_type: false) + end + + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'allows a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).to be_valid + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + end + end + describe 'path validation' do it 'rejects paths reserved on the root namespace when the group has no parent' do group = build(:group, path: 'api') @@ -513,6 +565,42 @@ RSpec.describe Group do end end + describe '#last_blocked_owner?' do + let(:blocked_user) { create(:user, :blocked) } + + before do + group.add_user(blocked_user, GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_truthy } + + context 'with another active owner' do + before do + group.add_user(create(:user), GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + + context 'with 2 blocked owners' do + before do + group.add_user(create(:user, :blocked), GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + + context 'with owners from a parent' do + before do + parent_group = create(:group) + create(:group_member, :owner, group: parent_group) + group.update(parent: parent_group) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + end + describe '#lfs_enabled?' do context 'LFS enabled globally' do before do @@ -729,8 +817,16 @@ RSpec.describe Group do context 'evaluating admin access level' do let_it_be(:admin) { create(:admin) } - it 'returns OWNER by default' do - expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) + 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 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 'returns NO_ACCESS when only concrete membership should be considered' do @@ -740,6 +836,33 @@ RSpec.describe Group do end end + describe '#direct_members' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } + let_it_be(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + + it 'does not return members of the parent' do + expect(group.direct_members).not_to include(maintainer) + end + + it 'returns the direct member of the group' do + expect(group.direct_members).to include(developer) + end + + context 'group sharing' do + let!(:shared_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'does not return members of the shared_with group' do + expect(shared_group.direct_members).not_to( + include(developer)) + end + end + end + describe '#members_with_parents' do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } @@ -932,6 +1055,65 @@ RSpec.describe Group do end end + describe '#refresh_members_authorized_projects' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:group_user) { create(:user) } + + before do + group.parent.add_maintainer(parent_group_user) + group.add_developer(group_user) + end + + context 'users for which authorizations refresh is executed' do + it 'processes authorizations refresh for all members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id, parent_group_user.id)).and_call_original + + group.refresh_members_authorized_projects + end + + context 'when explicitly specified to run only for direct members' do + it 'processes authorizations refresh only for direct members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original + + group.refresh_members_authorized_projects(direct_members_only: true) + end + end + end + end + + describe '#users_ids_of_direct_members' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:group_user) { create(:user) } + + before do + group.parent.add_maintainer(parent_group_user) + group.add_developer(group_user) + end + + it 'does not return user ids of the members of the parent' do + expect(group.users_ids_of_direct_members).not_to include(parent_group_user.id) + end + + it 'returns the user ids of the direct member of the group' do + expect(group.users_ids_of_direct_members).to include(group_user.id) + end + + context 'group sharing' do + let!(:shared_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'does not return the user ids of members of the shared_with group' do + expect(shared_group.users_ids_of_direct_members).not_to( + include(group_user.id)) + end + end + end + describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do maintainer = create(:user) @@ -959,6 +1141,29 @@ RSpec.describe Group do include(group_user.id)) end end + + context 'distinct user ids' do + let_it_be(:subgroup) { create(:group, :nested) } + let_it_be(:user) { create(:user) } + let_it_be(:shared_with_group) { create(:group) } + let_it_be(:other_subgroup_user) { create(:user) } + + before do + create(:group_group_link, shared_group: subgroup, shared_with_group: shared_with_group) + subgroup.add_maintainer(other_subgroup_user) + + # `user` is added as a direct member of the parent group, the subgroup + # and another group shared with the subgroup. + subgroup.parent.add_maintainer(user) + subgroup.add_developer(user) + shared_with_group.add_guest(user) + end + + it 'returns only distinct user ids of users for which to refresh authorizations' do + expect(subgroup.user_ids_for_project_authorizations).to( + contain_exactly(user.id, other_subgroup_user.id)) + end + end end describe '#update_two_factor_requirement' do @@ -1149,9 +1354,10 @@ RSpec.describe Group do describe '#ci_variables_for' do let(:project) { create(:project, group: group) } + let(:environment_scope) { '*' } let!(:ci_variable) do - create(:ci_group_variable, value: 'secret', group: group) + create(:ci_group_variable, value: 'secret', group: group, environment_scope: environment_scope) end let!(:protected_variable) do @@ -1160,13 +1366,16 @@ RSpec.describe Group do subject { group.ci_variables_for('ref', project) } - it 'memoizes the result by ref', :request_store do + it 'memoizes the result by ref and environment', :request_store do + scoped_variable = create(:ci_group_variable, value: 'secret', group: group, environment_scope: 'scoped') + expect(project).to receive(:protected_for?).with('ref').once.and_return(true) - expect(project).to receive(:protected_for?).with('other').once.and_return(false) + expect(project).to receive(:protected_for?).with('other').twice.and_return(false) 2.times do - expect(group.ci_variables_for('ref', project)).to contain_exactly(ci_variable, protected_variable) + expect(group.ci_variables_for('ref', project, environment: 'production')).to contain_exactly(ci_variable, protected_variable) expect(group.ci_variables_for('other', project)).to contain_exactly(ci_variable) + expect(group.ci_variables_for('other', project, environment: 'scoped')).to contain_exactly(ci_variable, scoped_variable) end end @@ -1203,6 +1412,120 @@ RSpec.describe Group do it_behaves_like 'ref is protected' end + context 'when environment name is specified' do + let(:environment) { 'review/name' } + + subject do + group.ci_variables_for('ref', project, environment: environment) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is matched by wildcard' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.not_to contain_exactly(ci_variable) } + end + + context 'when environment scope has _' do + let(:environment_scope) { '*_*' } + + it 'does not treat it as wildcard' do + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains underscore' do + let(:environment) { 'foo_bar/test' } + let(:environment_scope) { 'foo_bar/*' } + + it 'matches literally for _' do + is_expected.to contain_exactly(ci_variable) + end + end + end + + # The environment name and scope cannot have % at the moment, + # but we're considering relaxing it and we should also make sure + # it doesn't break in case some data sneaked in somehow as we're + # not checking this integrity in database level. + context 'when environment scope has %' do + it 'does not treat it as wildcard' do + ci_variable.update_attribute(:environment_scope, '*%*') + + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains a percent' do + let(:environment) { 'foo%bar/test' } + + it 'matches literally for %' do + ci_variable.update(environment_scope: 'foo%bar/*') + + is_expected.to contain_exactly(ci_variable) + end + end + end + + context 'when variables with the same name have different environment scopes' do + let!(:partially_matched_variable) do + create(:ci_group_variable, + key: ci_variable.key, + value: 'partial', + environment_scope: 'review/*', + group: group) + end + + let!(:perfectly_matched_variable) do + create(:ci_group_variable, + key: ci_variable.key, + value: 'prefect', + environment_scope: 'review/name', + group: group) + end + + it 'puts variables matching environment scope more in the end' do + is_expected.to eq( + [ci_variable, + partially_matched_variable, + perfectly_matched_variable]) + end + end + + context 'when :scoped_group_variables feature flag is disabled' do + before do + stub_feature_flags(scoped_group_variables: false) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is partially matched' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.to contain_exactly(ci_variable) } + end + end + end + context 'when group has children' do let(:group_child) { create(:group, parent: group) } let(:group_child_2) { create(:group, parent: group_child) } diff --git a/spec/models/issue_email_participant_spec.rb b/spec/models/issue_email_participant_spec.rb index f19e65e31f3..09c231bbfda 100644 --- a/spec/models/issue_email_participant_spec.rb +++ b/spec/models/issue_email_participant_spec.rb @@ -11,9 +11,14 @@ RSpec.describe IssueEmailParticipant do subject { build(:issue_email_participant) } it { is_expected.to validate_presence_of(:issue) } - it { is_expected.to validate_presence_of(:email) } - it { is_expected.to validate_uniqueness_of(:email).scoped_to([:issue_id]) } + it { is_expected.to validate_uniqueness_of(:email).scoped_to([:issue_id]).ignoring_case_sensitivity } - it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :email + it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email + + it 'is invalid if the email is nil' do + subject.email = nil + + expect(subject).to be_invalid + end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 969d897e551..a3e245f4def 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1258,4 +1258,23 @@ RSpec.describe Issue do expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError) end end + + describe '#email_participants_emails' do + let_it_be(:issue) { create(:issue) } + + it 'returns a list of emails' do + participant1 = issue.issue_email_participants.create(email: 'a@gitlab.com') + participant2 = issue.issue_email_participants.create(email: 'b@gitlab.com') + + expect(issue.email_participants_emails).to contain_exactly(participant1.email, participant2.email) + end + end + + describe '#email_participants_downcase' do + it 'returns a list of emails with all uppercase letters replaced with their lowercase counterparts' do + participant = create(:issue_email_participant, email: 'SomEoNe@ExamPLe.com') + + expect(participant.issue.email_participants_emails_downcase).to match([participant.email.downcase]) + end + end end diff --git a/spec/models/iteration_spec.rb b/spec/models/iteration_spec.rb deleted file mode 100644 index e7ec5de0ef1..00000000000 --- a/spec/models/iteration_spec.rb +++ /dev/null @@ -1,335 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Iteration do - let_it_be(:project) { create(:project) } - let_it_be(:group) { create(:group) } - - describe "#iid" do - it "is properly scoped on project and group" do - iteration1 = create(:iteration, :skip_project_validation, project: project) - iteration2 = create(:iteration, :skip_project_validation, project: project) - iteration3 = create(:iteration, group: group) - iteration4 = create(:iteration, group: group) - iteration5 = create(:iteration, :skip_project_validation, project: project) - - want = { - iteration1: 1, - iteration2: 2, - iteration3: 1, - iteration4: 2, - iteration5: 3 - } - got = { - iteration1: iteration1.iid, - iteration2: iteration2.iid, - iteration3: iteration3.iid, - iteration4: iteration4.iid, - iteration5: iteration5.iid - } - expect(got).to eq(want) - end - end - - describe '.filter_by_state' do - let_it_be(:closed_iteration) { create(:iteration, :closed, :skip_future_date_validation, group: group, start_date: 8.days.ago, due_date: 2.days.ago) } - let_it_be(:started_iteration) { create(:iteration, :started, :skip_future_date_validation, group: group, start_date: 1.day.ago, due_date: 6.days.from_now) } - let_it_be(:upcoming_iteration) { create(:iteration, :upcoming, group: group, start_date: 1.week.from_now, due_date: 2.weeks.from_now) } - - shared_examples_for 'filter_by_state' do - it 'filters by the given state' do - expect(described_class.filter_by_state(Iteration.all, state)).to match(expected_iterations) - end - end - - context 'filtering by closed iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'closed' } - let(:expected_iterations) { [closed_iteration] } - end - end - - context 'filtering by started iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'started' } - let(:expected_iterations) { [started_iteration] } - end - end - - context 'filtering by opened iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'opened' } - let(:expected_iterations) { [started_iteration, upcoming_iteration] } - end - end - - context 'filtering by upcoming iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'upcoming' } - let(:expected_iterations) { [upcoming_iteration] } - end - end - - context 'filtering by "all"' do - it_behaves_like 'filter_by_state' do - let(:state) { 'all' } - let(:expected_iterations) { [closed_iteration, started_iteration, upcoming_iteration] } - end - end - - context 'filtering by nonexistent filter' do - it 'raises ArgumentError' do - expect { described_class.filter_by_state(Iteration.none, 'unknown') }.to raise_error(ArgumentError, 'Unknown state filter: unknown') - end - end - end - - context 'Validations' do - subject { build(:iteration, group: group, start_date: start_date, due_date: due_date) } - - describe '#not_belonging_to_project' do - subject { build(:iteration, project: project, start_date: Time.current, due_date: 1.day.from_now) } - - it 'is invalid' do - expect(subject).not_to be_valid - expect(subject.errors[:project_id]).to include('is not allowed. We do not currently support project-level iterations') - end - end - - describe '#dates_do_not_overlap' do - let_it_be(:existing_iteration) { create(:iteration, group: group, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - context 'when no Iteration dates overlap' do - let(:start_date) { 2.weeks.from_now } - let(:due_date) { 3.weeks.from_now } - - it { is_expected.to be_valid } - end - - context 'when updated iteration dates overlap with its own dates' do - it 'is valid' do - existing_iteration.start_date = 5.days.from_now - - expect(existing_iteration).to be_valid - end - end - - context 'when dates overlap' do - let(:start_date) { 5.days.from_now } - let(:due_date) { 6.days.from_now } - - shared_examples_for 'overlapping dates' do |skip_constraint_test: false| - context 'when start_date is in range' do - let(:start_date) { 5.days.from_now } - let(:due_date) { 3.weeks.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - - context 'when end_date is in range' do - let(:start_date) { Time.current } - let(:due_date) { 6.days.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - - context 'when both overlap' do - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - end - - context 'group' do - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_daterange_group_id_constraint' } - end - - context 'different group' do - let(:group) { create(:group) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - - context 'sub-group' do - let(:subgroup) { create(:group, parent: group) } - - subject { build(:iteration, group: subgroup, start_date: start_date, due_date: due_date) } - - it_behaves_like 'overlapping dates', skip_constraint_test: true - end - end - - context 'project' do - let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) } - - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' } - end - - context 'different project' do - let(:project) { create(:project) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - - context 'in a group' do - let(:group) { create(:group) } - - subject { build(:iteration, group: group, start_date: start_date, due_date: due_date) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - end - - context 'project in a group' do - let_it_be(:project) { create(:project, group: create(:group)) } - let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) } - - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' } - end - end - end - end - - describe '#future_date' do - context 'when dates are in the future' do - let(:start_date) { Time.current } - let(:due_date) { 1.week.from_now } - - it { is_expected.to be_valid } - end - - context 'when start_date is in the past' do - let(:start_date) { 1.week.ago } - let(:due_date) { 1.week.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:start_date]).to include('cannot be in the past') - end - end - - context 'when due_date is in the past' do - let(:start_date) { Time.current } - let(:due_date) { 1.week.ago } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:due_date]).to include('cannot be in the past') - end - end - - context 'when start_date is over 500 years in the future' do - let(:start_date) { 501.years.from_now } - let(:due_date) { Time.current } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:start_date]).to include('cannot be more than 500 years in the future') - end - end - - context 'when due_date is over 500 years in the future' do - let(:start_date) { Time.current } - let(:due_date) { 501.years.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:due_date]).to include('cannot be more than 500 years in the future') - end - end - end - end - - context 'time scopes' do - let_it_be(:project) { create(:project, :empty_repo) } - let_it_be(:iteration_1) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 3.days.ago, due_date: 1.day.from_now) } - let_it_be(:iteration_2) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 10.days.ago, due_date: 4.days.ago) } - let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - describe 'start_date_passed' do - it 'returns iterations where start_date is in the past but due_date is in the future' do - expect(described_class.start_date_passed).to contain_exactly(iteration_1) - end - end - - describe 'due_date_passed' do - it 'returns iterations where due date is in the past' do - expect(described_class.due_date_passed).to contain_exactly(iteration_2) - end - end - end - - describe '.within_timeframe' do - let_it_be(:now) { Time.current } - let_it_be(:project) { create(:project, :empty_repo) } - let_it_be(:iteration_1) { create(:iteration, :skip_project_validation, project: project, start_date: now, due_date: 1.day.from_now) } - let_it_be(:iteration_2) { create(:iteration, :skip_project_validation, project: project, start_date: 2.days.from_now, due_date: 3.days.from_now) } - let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - it 'returns iterations with start_date and/or end_date between timeframe' do - iterations = described_class.within_timeframe(2.days.from_now, 3.days.from_now) - - expect(iterations).to match_array([iteration_2]) - end - - it 'returns iterations which starts before the timeframe' do - iterations = described_class.within_timeframe(1.day.from_now, 3.days.from_now) - - expect(iterations).to match_array([iteration_1, iteration_2]) - end - - it 'returns iterations which ends after the timeframe' do - iterations = described_class.within_timeframe(3.days.from_now, 5.days.from_now) - - expect(iterations).to match_array([iteration_2, iteration_3]) - end - end -end diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index f0b1bc33e84..ad07ee1115b 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe List do it_behaves_like 'having unique enum values' it_behaves_like 'boards listable model', :list + it_behaves_like 'list_preferences_for user', :list, :list_id describe 'relationships' do it { is_expected.to belong_to(:board) } @@ -17,70 +18,16 @@ RSpec.describe List do it { is_expected.to validate_presence_of(:list_type) } end - describe '#update_preferences_for' do - let(:user) { create(:user) } - let(:list) { create(:list) } + describe '.without_types' do + it 'exclude lists of given types' do + board = create(:list, list_type: :label).board + # closed list is created by default + backlog_list = create(:list, list_type: :backlog, board: board) - context 'when user is present' do - context 'when there are no preferences for user' do - it 'creates new user preferences' do - expect { list.update_preferences_for(user, collapsed: true) }.to change { ListUserPreference.count }.by(1) - expect(list.preferences_for(user).collapsed).to eq(true) - end - end + exclude_type = [described_class.list_types[:label], described_class.list_types[:closed]] - context 'when there are preferences for user' do - it 'updates user preferences' do - list.update_preferences_for(user, collapsed: false) - - expect { list.update_preferences_for(user, collapsed: true) }.not_to change { ListUserPreference.count } - expect(list.preferences_for(user).collapsed).to eq(true) - end - end - - context 'when user is nil' do - it 'does not create user preferences' do - expect { list.update_preferences_for(nil, collapsed: true) }.not_to change { ListUserPreference.count } - end - end - end - end - - describe '#preferences_for' do - let(:user) { create(:user) } - let(:list) { create(:list) } - - context 'when user is nil' do - it 'returns not persisted preferences' do - preferences = list.preferences_for(nil) - - expect(preferences.persisted?).to eq(false) - expect(preferences.list_id).to eq(list.id) - expect(preferences.user_id).to be_nil - end - end - - context 'when a user preference already exists' do - before do - list.update_preferences_for(user, collapsed: true) - end - - it 'loads preference for user' do - preferences = list.preferences_for(user) - - expect(preferences).to be_persisted - expect(preferences.collapsed).to eq(true) - end - end - - context 'when preferences for user does not exist' do - it 'returns not persisted preferences' do - preferences = list.preferences_for(user) - - expect(preferences.persisted?).to eq(false) - expect(preferences.user_id).to eq(user.id) - expect(preferences.list_id).to eq(list.id) - end + lists = described_class.without_types(exclude_type) + expect(lists.where(board: board)).to match_array([backlog_list]) end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index b60af7abade..c41f466456f 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Member do it { is_expected.to allow_value(nil).for(:expires_at) } end - it_behaves_like 'an object with email-formated attributes', :invite_email do + it_behaves_like 'an object with email-formatted attributes', :invite_email do subject { build(:project_member) } end @@ -130,14 +130,18 @@ RSpec.describe Member do @maintainer_user = create(:user).tap { |u| project.add_maintainer(u) } @maintainer = project.members.find_by(user_id: @maintainer_user.id) - @blocked_user = create(:user).tap do |u| + @blocked_maintainer_user = create(:user).tap do |u| project.add_maintainer(u) + + u.block! + end + @blocked_developer_user = create(:user).tap do |u| project.add_developer(u) u.block! end - @blocked_maintainer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MAINTAINER) - @blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER) + @blocked_maintainer = project.members.find_by(user_id: @blocked_maintainer_user.id, access_level: Gitlab::Access::MAINTAINER) + @blocked_developer = project.members.find_by(user_id: @blocked_developer_user.id, access_level: Gitlab::Access::DEVELOPER) @invited_member = create(:project_member, :developer, project: project, @@ -161,7 +165,7 @@ RSpec.describe Member do describe '.access_for_user_ids' do it 'returns the right access levels' do - users = [@owner_user.id, @maintainer_user.id, @blocked_user.id] + users = [@owner_user.id, @maintainer_user.id, @blocked_maintainer_user.id] expected = { @owner_user.id => Gitlab::Access::OWNER, @maintainer_user.id => Gitlab::Access::MAINTAINER @@ -382,6 +386,20 @@ RSpec.describe Member do it { is_expected.not_to include @member_with_minimal_access } end + describe '.blocked' do + subject { described_class.blocked.to_a } + + it { is_expected.not_to include @owner } + it { is_expected.not_to include @maintainer } + it { is_expected.not_to include @invited_member } + it { is_expected.not_to include @accepted_invite_member } + it { is_expected.not_to include @requested_member } + it { is_expected.not_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 '.active_without_invites_and_requests' do subject { described_class.active_without_invites_and_requests.to_a } @@ -425,12 +443,10 @@ RSpec.describe Member do end context 'when admin mode is disabled' do - # Skipped because `Group#max_member_access_for_user` needs to be migrated to use admin mode - # https://gitlab.com/gitlab-org/gitlab/-/issues/207950 - xit 'rejects setting members.created_by to the given admin current_user' do + it 'rejects setting members.created_by to the given admin current_user' do member = described_class.add_user(source, user, :maintainer, current_user: admin) - expect(member.created_by).not_to be_persisted + expect(member.created_by).to be_nil end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ebe2cd2ac03..8c7289adbcc 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -9,8 +9,8 @@ RSpec.describe MergeRequest, factory_default: :keep do using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:project, refind: true) { create_default(:project, :repository) } + let_it_be(:namespace) { create_default(:namespace).freeze } + let_it_be(:project, refind: true) { create_default(:project, :repository).freeze } subject { create(:merge_request) } @@ -1366,6 +1366,10 @@ RSpec.describe MergeRequest, factory_default: :keep do it "doesn't detect WIP by default" do expect(subject.work_in_progress?).to eq false end + + it "is aliased to #draft?" do + expect(subject.method(:work_in_progress?)).to eq(subject.method(:draft?)) + end end describe "#wipless_title" do @@ -2895,6 +2899,14 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.mergeable?).to be_truthy end + it 'return true if #mergeable_state? is true and the MR #can_be_merged? is false' do + allow(subject).to receive(:mergeable_state?) { true } + expect(subject).to receive(:check_mergeability) + expect(subject).to receive(:can_be_merged?) { false } + + expect(subject.mergeable?).to be_falsey + end + context 'with skip_ci_check option' do before do allow(subject).to receive_messages(check_mergeability: nil, @@ -3072,6 +3084,7 @@ RSpec.describe MergeRequest, factory_default: :keep do where(:status, :public_status) do 'cannot_be_merged_rechecking' | 'checking' + 'preparing' | 'checking' 'checking' | 'checking' 'cannot_be_merged' | 'cannot_be_merged' end @@ -3082,32 +3095,83 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe "#head_pipeline_active? " do - it do - is_expected - .to delegate_method(:active?) - .to(:head_pipeline) - .with_prefix - .with_arguments(allow_nil: true) + context 'when project lacks a head_pipeline relation' do + before do + subject.head_pipeline = nil + end + + it 'returns false' do + expect(subject.head_pipeline_active?).to be false + end + end + + context 'when project has a head_pipeline relation' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + allow(subject).to receive(:head_pipeline) { pipeline } + end + + it 'accesses the value from the head_pipeline' do + expect(subject.head_pipeline) + .to receive(:active?) + + subject.head_pipeline_active? + end end end describe "#actual_head_pipeline_success? " do - it do - is_expected - .to delegate_method(:success?) - .to(:actual_head_pipeline) - .with_prefix - .with_arguments(allow_nil: true) + context 'when project lacks an actual_head_pipeline relation' do + before do + allow(subject).to receive(:actual_head_pipeline) { nil } + end + + it 'returns false' do + expect(subject.actual_head_pipeline_success?).to be false + end + end + + context 'when project has a actual_head_pipeline relation' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + allow(subject).to receive(:actual_head_pipeline) { pipeline } + end + + it 'accesses the value from the actual_head_pipeline' do + expect(subject.actual_head_pipeline) + .to receive(:success?) + + subject.actual_head_pipeline_success? + end end end describe "#actual_head_pipeline_active? " do - it do - is_expected - .to delegate_method(:active?) - .to(:actual_head_pipeline) - .with_prefix - .with_arguments(allow_nil: true) + context 'when project lacks an actual_head_pipeline relation' do + before do + allow(subject).to receive(:actual_head_pipeline) { nil } + end + + it 'returns false' do + expect(subject.actual_head_pipeline_active?).to be false + end + end + + context 'when project has a actual_head_pipeline relation' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + allow(subject).to receive(:actual_head_pipeline) { pipeline } + end + + it 'accesses the value from the actual_head_pipeline' do + expect(subject.actual_head_pipeline) + .to receive(:active?) + + subject.actual_head_pipeline_active? + end end end @@ -3784,6 +3848,87 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#use_merge_base_pipeline_for_comparison?' do + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) } + + subject { merge_request.use_merge_base_pipeline_for_comparison?(service_class) } + + context 'when service class is Ci::CompareCodequalityReportsService' do + let(:service_class) { 'Ci::CompareCodequalityReportsService' } + + context 'when feature flag is enabled' do + it { is_expected.to be_truthy } + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(codequality_backend_comparison: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'when service class is different' do + let(:service_class) { 'Ci::GenerateCoverageReportsService' } + + it { is_expected.to be_falsey } + end + end + + describe '#comparison_base_pipeline' do + subject(:pipeline) { merge_request.comparison_base_pipeline(service_class) } + + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) } + let!(:base_pipeline) do + create(:ci_pipeline, + :with_test_reports, + project: project, + ref: merge_request.target_branch, + sha: merge_request.diff_base_sha + ) + end + + context 'when service class is Ci::CompareCodequalityReportsService' do + let(:service_class) { 'Ci::CompareCodequalityReportsService' } + + context 'when merge request has a merge request pipeline' do + let(:merge_request) do + create(:merge_request, :with_merge_request_pipeline) + end + + let(:merge_base_pipeline) do + create(:ci_pipeline, ref: merge_request.target_branch, sha: merge_request.target_branch_sha) + end + + before do + merge_base_pipeline + merge_request.update_head_pipeline + end + + it 'returns the merge_base_pipeline' do + expect(pipeline).to eq(merge_base_pipeline) + end + end + + context 'when merge does not have a merge request pipeline' do + it 'returns the base_pipeline' do + expect(pipeline).to eq(base_pipeline) + end + end + end + + context 'when service_class is different' do + let(:service_class) { 'Ci::GenerateCoverageReportsService' } + + it 'returns the base_pipeline' do + expect(pipeline).to eq(base_pipeline) + end + end + end + describe '#base_pipeline' do let(:pipeline_arguments) do { @@ -3963,6 +4108,65 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#mark_as_unchecked' do + subject { create(:merge_request, source_project: project, merge_status: merge_status) } + + shared_examples 'for an invalid state transition' do + it 'is not a valid state transition' do + expect { subject.mark_as_unchecked! }.to raise_error(StateMachines::InvalidTransition) + end + end + + shared_examples 'for an valid state transition' do + it 'is a valid state transition' do + expect { subject.mark_as_unchecked! } + .to change { subject.merge_status } + .from(merge_status.to_s) + .to(expected_merge_status) + end + end + + context 'when the status is unchecked' do + let(:merge_status) { :unchecked } + + include_examples 'for an invalid state transition' + end + + context 'when the status is checking' do + let(:merge_status) { :checking } + let(:expected_merge_status) { 'unchecked' } + + include_examples 'for an valid state transition' + end + + context 'when the status is can_be_merged' do + let(:merge_status) { :can_be_merged } + let(:expected_merge_status) { 'unchecked' } + + include_examples 'for an valid state transition' + end + + context 'when the status is cannot_be_merged_recheck' do + let(:merge_status) { :cannot_be_merged_recheck } + + include_examples 'for an invalid state transition' + end + + context 'when the status is cannot_be_merged' do + let(:merge_status) { :cannot_be_merged } + let(:expected_merge_status) { 'cannot_be_merged_recheck' } + + include_examples 'for an valid state transition' + end + + context 'when the status is cannot_be_merged' do + let(:merge_status) { :cannot_be_merged } + let(:expected_merge_status) { 'cannot_be_merged_recheck' } + + include_examples 'for an valid state transition' + end + end + describe 'transition to cannot_be_merged' do let(:notification_service) { double(:notification_service) } let(:todo_service) { double(:todo_service) } @@ -4661,4 +4865,33 @@ RSpec.describe MergeRequest, factory_default: :keep do end end end + + describe '#includes_ci_config?' do + let(:merge_request) { build(:merge_request) } + let(:project) { merge_request.project } + + subject(:result) { merge_request.includes_ci_config? } + + before do + allow(merge_request).to receive(:diff_stats).and_return(diff_stats) + end + + context 'when diff_stats is nil' do + let(:diff_stats) {} + + it { is_expected.to eq(false) } + end + + context 'when diff_stats does not include the ci config path of the project' do + let(:diff_stats) { [double(path: 'abc.txt')] } + + it { is_expected.to eq(false) } + end + + context 'when diff_stats includes the ci config path of the project' do + let(:diff_stats) { [double(path: '.gitlab-ci.yml')] } + + it { is_expected.to eq(true) } + end + end end diff --git a/spec/models/namespace/traversal_hierarchy_spec.rb b/spec/models/namespace/traversal_hierarchy_spec.rb index 71b0e974106..83e6d704640 100644 --- a/spec/models/namespace/traversal_hierarchy_spec.rb +++ b/spec/models/namespace/traversal_hierarchy_spec.rb @@ -3,41 +3,41 @@ require 'spec_helper' RSpec.describe Namespace::TraversalHierarchy, type: :model do - let_it_be(:root, reload: true) { create(:namespace, :with_hierarchy) } + let_it_be(:root, reload: true) { create(:group, :with_hierarchy) } describe '.for_namespace' do - let(:hierarchy) { described_class.for_namespace(namespace) } + let(:hierarchy) { described_class.for_namespace(group) } context 'with root group' do - let(:namespace) { root } + let(:group) { root } it { expect(hierarchy.root).to eq root } end context 'with child group' do - let(:namespace) { root.children.first.children.first } + let(:group) { root.children.first.children.first } it { expect(hierarchy.root).to eq root } end context 'with group outside of hierarchy' do - let(:namespace) { create(:namespace) } + let(:group) { create(:namespace) } it { expect(hierarchy.root).not_to eq root } end end describe '.new' do - let(:hierarchy) { described_class.new(namespace) } + let(:hierarchy) { described_class.new(group) } context 'with root group' do - let(:namespace) { root } + let(:group) { root } it { expect(hierarchy.root).to eq root } end context 'with child group' do - let(:namespace) { root.children.first } + let(:group) { root.children.first } it { expect { hierarchy }.to raise_error(StandardError, 'Must specify a root node') } end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 647e279bf83..65d787d334b 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Namespace do include ProjectForksHelper include GitHelpers - let!(:namespace) { create(:namespace) } + let!(:namespace) { create(:namespace, :with_namespace_settings) } let(:gitlab_shell) { Gitlab::Shell.new } let(:repository_storage) { 'default' } @@ -32,14 +32,68 @@ RSpec.describe Namespace do it { is_expected.to validate_presence_of(:owner) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } + context 'validating the parent of a namespace' do + context 'when the namespace has no parent' do + it 'allows a namespace to have no parent associated with it' do + namespace = build(:namespace) + + expect(namespace).to be_valid + end + end + + context 'when the namespace has a parent' do + it 'does not allow a namespace to have a group as its parent' do + namespace = build(:namespace, parent: build(:group)) + + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + end + + it 'does not allow a namespace to have another namespace as its parent' do + namespace = build(:namespace, parent: build(:namespace)) + + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + end + end + + context 'when the feature flag `validate_namespace_parent_type` is disabled' do + before do + stub_feature_flags(validate_namespace_parent_type: false) + end + + context 'when the namespace has no parent' do + it 'allows a namespace to have no parent associated with it' do + namespace = build(:namespace) + + expect(namespace).to be_valid + end + end + + context 'when the namespace has a parent' do + it 'allows a namespace to have a group as its parent' do + namespace = build(:namespace, parent: build(:group)) + + expect(namespace).to be_valid + end + + it 'allows a namespace to have another namespace as its parent' do + namespace = build(:namespace, parent: build(:namespace)) + + expect(namespace).to be_valid + end + end + end + end + it 'does not allow too deep nesting' do ancestors = (1..21).to_a - nested = build(:namespace, parent: namespace) + group = build(:group) - allow(nested).to receive(:ancestors).and_return(ancestors) + allow(group).to receive(:ancestors).and_return(ancestors) - expect(nested).not_to be_valid - expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting') + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('has too deep level of nesting') end describe 'reserved path validation' do @@ -116,6 +170,28 @@ RSpec.describe Namespace do it { is_expected.to include_module(Namespaces::Traversal::Recursive) } end + describe 'callbacks' do + describe 'before_save :ensure_delayed_project_removal_assigned_to_namespace_settings' do + it 'sets the matching value in namespace_settings' do + expect { namespace.update!(delayed_project_removal: true) }.to change { + namespace.namespace_settings.delayed_project_removal + }.from(false).to(true) + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(migrate_delayed_project_removal: false) + end + + it 'does not set the matching value in namespace_settings' do + expect { namespace.update!(delayed_project_removal: true) }.not_to change { + namespace.namespace_settings.delayed_project_removal + } + end + end + end + end + describe '#visibility_level_field' do it { expect(namespace.visibility_level_field).to eq(:visibility_level) } end @@ -150,45 +226,45 @@ RSpec.describe Namespace do end describe '.search' do - let_it_be(:first_namespace) { build(:namespace, name: 'my first namespace', path: 'old-path').tap(&:save!) } - let_it_be(:parent_namespace) { build(:namespace, name: 'my parent namespace', path: 'parent-path').tap(&:save!) } - let_it_be(:second_namespace) { build(:namespace, name: 'my second namespace', path: 'new-path', parent: parent_namespace).tap(&:save!) } - let_it_be(:project_with_same_path) { create(:project, id: second_namespace.id, path: first_namespace.path) } + let_it_be(:first_group) { build(:group, name: 'my first namespace', path: 'old-path').tap(&:save!) } + let_it_be(:parent_group) { build(:group, name: 'my parent namespace', path: 'parent-path').tap(&:save!) } + let_it_be(:second_group) { build(:group, name: 'my second namespace', path: 'new-path', parent: parent_group).tap(&:save!) } + let_it_be(:project_with_same_path) { create(:project, id: second_group.id, path: first_group.path) } it 'returns namespaces with a matching name' do - expect(described_class.search('my first namespace')).to eq([first_namespace]) + expect(described_class.search('my first namespace')).to eq([first_group]) end it 'returns namespaces with a partially matching name' do - expect(described_class.search('first')).to eq([first_namespace]) + expect(described_class.search('first')).to eq([first_group]) end it 'returns namespaces with a matching name regardless of the casing' do - expect(described_class.search('MY FIRST NAMESPACE')).to eq([first_namespace]) + expect(described_class.search('MY FIRST NAMESPACE')).to eq([first_group]) end it 'returns namespaces with a matching path' do - expect(described_class.search('old-path')).to eq([first_namespace]) + expect(described_class.search('old-path')).to eq([first_group]) end it 'returns namespaces with a partially matching path' do - expect(described_class.search('old')).to eq([first_namespace]) + expect(described_class.search('old')).to eq([first_group]) end it 'returns namespaces with a matching path regardless of the casing' do - expect(described_class.search('OLD-PATH')).to eq([first_namespace]) + expect(described_class.search('OLD-PATH')).to eq([first_group]) end it 'returns namespaces with a matching route path' do - expect(described_class.search('parent-path/new-path', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('parent-path/new-path', include_parents: true)).to eq([second_group]) end it 'returns namespaces with a partially matching route path' do - expect(described_class.search('parent-path/new', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('parent-path/new', include_parents: true)).to eq([second_group]) end it 'returns namespaces with a matching route path regardless of the casing' do - expect(described_class.search('PARENT-PATH/NEW-PATH', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('PARENT-PATH/NEW-PATH', include_parents: true)).to eq([second_group]) end end @@ -285,6 +361,18 @@ RSpec.describe Namespace do end end + describe '.top_most' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + subject { described_class.top_most.ids } + + it 'only contains root namespaces' do + is_expected.to contain_exactly(group.id, namespace.id) + end + end + describe '#ancestors_upto' do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } @@ -800,7 +888,7 @@ RSpec.describe Namespace do end describe '#all_projects' do - shared_examples 'all projects for a group' 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) } @@ -808,49 +896,39 @@ RSpec.describe Namespace do it { expect(namespace.all_projects.to_a).to match_array([project2, project1]) } it { expect(child.all_projects.to_a).to match_array([project2]) } - end - - shared_examples 'all projects for personal 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) } - it { expect(user_namespace.all_projects.to_a).to match_array([project]) } - end - - context 'with recursive approach' do - context 'when namespace is a group' do - include_examples 'all projects for a group' + context 'when recursive_namespace_lookup_as_inner_join feature flag is on' do + before do + stub_feature_flags(recursive_namespace_lookup_as_inner_join: true) + end it 'queries for the namespace and its descendants' do - expect(Project).to receive(:where).with(namespace: [namespace, child]) - - namespace.all_projects + expect(namespace.all_projects).to match_array([project1, project2]) end end - context 'when namespace is a user namespace' do - include_examples 'all projects for personal namespace' - - it 'only queries for the namespace itself' do - expect(Project).to receive(:where).with(namespace: user_namespace) + context 'when recursive_namespace_lookup_as_inner_join feature flag is off' do + before do + stub_feature_flags(recursive_namespace_lookup_as_inner_join: false) + end - user_namespace.all_projects + it 'queries for the namespace and its descendants' do + expect(namespace.all_projects).to match_array([project1, project2]) end end end - context 'with route path wildcard approach' do - before do - stub_feature_flags(recursive_approach_for_all_projects: false) - 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) } - context 'when namespace is a group' do - include_examples 'all projects for a group' - end + it { expect(user_namespace.all_projects.to_a).to match_array([project]) } + + it 'only queries for the namespace itself' do + expect(Project).to receive(:where).with(namespace: user_namespace) - context 'when namespace is a user namespace' do - include_examples 'all projects for personal namespace' + user_namespace.all_projects end end end @@ -1250,14 +1328,14 @@ RSpec.describe Namespace do using RSpec::Parameterized::TableSyntax shared_examples_for 'fetching closest setting' do - let!(:root_namespace) { create(:namespace) } - let!(:namespace) { create(:namespace, parent: root_namespace) } + let!(:parent) { create(:group) } + let!(:group) { create(:group, parent: parent) } - let(:setting) { namespace.closest_setting(setting_name) } + let(:setting) { group.closest_setting(setting_name) } before do - root_namespace.update_attribute(setting_name, root_setting) - namespace.update_attribute(setting_name, child_setting) + parent.update_attribute(setting_name, root_setting) + group.update_attribute(setting_name, child_setting) end it 'returns closest non-nil value' do @@ -1348,30 +1426,30 @@ RSpec.describe Namespace do context 'with a parent' do context 'when parent has shared runners disabled' do - let(:parent) { create(:namespace, :shared_runners_disabled) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is invalid' do - expect(sub_namespace).to be_invalid - expect(sub_namespace.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') + expect(group).to be_invalid + expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') end end context 'when parent has shared runners disabled but allows override' do - let(:parent) { create(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end context 'when parent has shared runners enabled' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end end @@ -1401,30 +1479,30 @@ RSpec.describe Namespace do context 'with a parent' do context 'when parent does not allow shared runners' do - let(:parent) { create(:namespace, :shared_runners_disabled) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } it 'is invalid' do - expect(sub_namespace).to be_invalid - expect(sub_namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') + expect(group).to be_invalid + expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') end end context 'when parent allows shared runners and setting to true' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end context 'when parent allows shared runners and setting to false' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end end @@ -1449,4 +1527,24 @@ RSpec.describe Namespace do end end end + + describe '#recent?' do + subject { namespace.recent? } + + context 'when created more than 90 days ago' do + before do + namespace.update_attribute(:created_at, 91.days.ago) + end + + it { is_expected.to be(false) } + end + + context 'when created less than 90 days ago' do + before do + namespace.update_attribute(:created_at, 89.days.ago) + end + + it { is_expected.to be(true) } + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 364b80e8601..590acfc0ac1 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -336,6 +336,25 @@ RSpec.describe Note do end end + describe "last_edited_at" do + let(:timestamp) { Time.current } + let(:note) { build(:note, last_edited_at: nil, created_at: timestamp, updated_at: timestamp + 5.hours) } + + context "with last_edited_at" do + it "returns last_edited_at" do + note.last_edited_at = timestamp + + expect(note.last_edited_at).to eq(timestamp) + end + end + + context "without last_edited_at" do + it "returns updated_at" do + expect(note.last_edited_at).to eq(timestamp + 5.hours) + end + end + end + describe "edited?" do let(:note) { build(:note, updated_by_id: nil, created_at: Time.current, updated_at: Time.current + 5.hours) } diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 8429f577dc6..4debda0621c 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -337,6 +337,39 @@ RSpec.describe NotificationRecipient do expect(recipient.suitable_notification_level?).to eq true end end + + context 'with merge_when_pipeline_succeeds' do + let(:notification_setting) { user.notification_settings_for(project) } + let(:recipient) do + described_class.new( + user, + :watch, + custom_action: :merge_when_pipeline_succeeds, + target: target, + project: project + ) + end + + context 'custom event enabled' do + before do + notification_setting.update!(merge_when_pipeline_succeeds: true) + end + + it 'returns true' do + expect(recipient.suitable_notification_level?).to eq true + end + end + + context 'custom event disabled' do + before do + notification_setting.update!(merge_when_pipeline_succeeds: false) + end + + it 'returns false' do + expect(recipient.suitable_notification_level?).to eq false + end + end + end end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index bc50e2af373..4ef5ab7af48 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -180,7 +180,8 @@ RSpec.describe NotificationSetting do :failed_pipeline, :success_pipeline, :fixed_pipeline, - :moved_project + :moved_project, + :merge_when_pipeline_succeeds ) end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding_progress_spec.rb index 0aa19345a25..779312c9fa0 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding_progress_spec.rb @@ -106,7 +106,7 @@ RSpec.describe OnboardingProgress do end context 'when not given a root namespace' do - let(:namespace) { create(:namespace, parent: build(:namespace)) } + let(:namespace) { create(:group, parent: build(:group)) } it 'does not add a record for the namespace' do expect { onboard }.not_to change(described_class, :count).from(0) @@ -182,6 +182,30 @@ RSpec.describe OnboardingProgress do end end + describe '.not_completed?' do + subject { described_class.not_completed?(namespace.id, action) } + + context 'when the namespace has not yet been onboarded' do + it { is_expected.to be(false) } + end + + context 'when the namespace has been onboarded but not registered the action yet' do + before do + described_class.onboard(namespace) + end + + it { is_expected.to be(true) } + + context 'when the action has been registered' do + before do + described_class.register(namespace, action) + end + + it { is_expected.to be(false) } + end + end + end + describe '.column_name' do subject { described_class.column_name(action) } diff --git a/spec/models/packages/maven/metadatum_spec.rb b/spec/models/packages/maven/metadatum_spec.rb index 16f6929d710..94a0e558985 100644 --- a/spec/models/packages/maven/metadatum_spec.rb +++ b/spec/models/packages/maven/metadatum_spec.rb @@ -36,5 +36,38 @@ RSpec.describe Packages::Maven::Metadatum, type: :model do expect(maven_metadatum.errors.to_a).to include('Package type must be Maven') end end + + context 'with a package' do + let_it_be(:package) { create(:package) } + + describe '.for_package_ids' do + let_it_be(:metadata) { create_list(:maven_metadatum, 3, package: package) } + + subject { Packages::Maven::Metadatum.for_package_ids(package.id) } + + it { is_expected.to match_array(metadata) } + end + + describe '.order_created' do + let_it_be(:metadatum1) { create(:maven_metadatum, package: package) } + let_it_be(:metadatum2) { create(:maven_metadatum, package: package) } + let_it_be(:metadatum3) { create(:maven_metadatum, package: package) } + let_it_be(:metadatum4) { create(:maven_metadatum, package: package) } + + subject { Packages::Maven::Metadatum.for_package_ids(package.id).order_created } + + it { is_expected.to eq([metadatum1, metadatum2, metadatum3, metadatum4]) } + end + + describe '.pluck_app_name' do + let_it_be(:metadatum1) { create(:maven_metadatum, package: package, app_name: 'one') } + let_it_be(:metadatum2) { create(:maven_metadatum, package: package, app_name: 'two') } + let_it_be(:metadatum3) { create(:maven_metadatum, package: package, app_name: 'three') } + + subject { Packages::Maven::Metadatum.for_package_ids(package.id).pluck_app_name } + + it { is_expected.to match_array([metadatum1, metadatum2, metadatum3].map(&:app_name)) } + end + end end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index ebb10e991ad..9cf998a0639 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -62,6 +62,21 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '.for_rubygem_with_file_name' do + let_it_be(:project) { create(:project) } + let_it_be(:non_ruby_package) { create(:nuget_package, project: project, package_type: :nuget) } + let_it_be(:ruby_package) { create(:rubygems_package, project: project, package_type: :rubygems) } + let_it_be(:file_name) { 'other.gem' } + + let_it_be(:non_ruby_file) { create(:package_file, :nuget, package: non_ruby_package, file_name: file_name) } + let_it_be(:gem_file1) { create(:package_file, :gem, package: ruby_package) } + let_it_be(:gem_file2) { create(:package_file, :gem, package: ruby_package, file_name: file_name) } + + it 'returns the matching gem file only for ruby packages' do + expect(described_class.for_rubygem_with_file_name(project, file_name)).to contain_exactly(gem_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 6c55d37b95f..82997acee3f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -22,6 +22,14 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_one(:rubygems_metadatum).inverse_of(:package) } end + describe '.with_debian_codename' do + let_it_be(:publication) { create(:debian_publication) } + + subject { described_class.with_debian_codename(publication.distribution.codename).to_a } + + it { is_expected.to contain_exactly(publication.package) } + end + describe '.with_composer_target' do let!(:package1) { create(:composer_package, :with_metadatum, sha: '123') } let!(:package2) { create(:composer_package, :with_metadatum, sha: '123') } @@ -162,6 +170,18 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.not_to allow_value('../../../my_package').for(:name) } it { is_expected.not_to allow_value('%2e%2e%2fmy_package').for(:name) } end + + context 'npm package' do + subject { build_stubbed(:npm_package) } + + it { is_expected.to allow_value("@group-1/package").for(:name) } + it { is_expected.to allow_value("@any-scope/package").for(:name) } + it { is_expected.to allow_value("unscoped-package").for(:name) } + it { is_expected.not_to allow_value("@inv@lid-scope/package").for(:name) } + it { is_expected.not_to allow_value("@scope/../../package").for(:name) } + it { is_expected.not_to allow_value("@scope%2e%2e%fpackage").for(:name) } + it { is_expected.not_to allow_value("@scope/sub/package").for(:name) } + end end describe '#version' do @@ -342,16 +362,6 @@ RSpec.describe Packages::Package, type: :model do end describe '#package_already_taken' do - context 'npm package' do - let!(:package) { create(:npm_package) } - - it 'will not allow a package of the same name' do - new_package = build(:npm_package, project: create(:project), name: package.name) - - expect(new_package).not_to be_valid - end - end - context 'maven package' do let!(:package) { create(:maven_package) } @@ -511,7 +521,7 @@ RSpec.describe Packages::Package, type: :model do describe '.without_nuget_temporary_name' do let!(:package1) { create(:nuget_package) } - let!(:package2) { create(:nuget_package, name: Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME) } + let!(:package2) { create(:nuget_package, name: Packages::Nuget::TEMPORARY_PACKAGE_NAME) } subject { described_class.without_nuget_temporary_name } @@ -530,7 +540,7 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to match_array([package1, package2, package3]) } context 'with temporary packages' do - let!(:package1) { create(:nuget_package, name: Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME) } + let!(:package1) { create(:nuget_package, name: Packages::Nuget::TEMPORARY_PACKAGE_NAME) } it { is_expected.to match_array([package2, package3]) } end @@ -803,4 +813,63 @@ RSpec.describe Packages::Package, type: :model do expect(package.package_settings).to eq(group.package_settings) end end + + describe '#sync_maven_metadata' do + let_it_be(:user) { create(:user) } + let_it_be(:package) { create(:maven_package) } + + subject { package.sync_maven_metadata(user) } + + shared_examples 'not enqueuing a sync worker job' do + it 'does not enqueue a sync worker job' do + expect(::Packages::Maven::Metadata::SyncWorker) + .not_to receive(:perform_async) + + subject + end + end + + it 'enqueues a sync worker job' do + expect(::Packages::Maven::Metadata::SyncWorker) + .to receive(:perform_async).with(user.id, package.project.id, package.name) + + subject + end + + context 'with no user' do + let(:user) { nil } + + it_behaves_like 'not enqueuing a sync worker job' + end + + context 'with a versionless maven package' do + let_it_be(:package) { create(:maven_package, version: nil) } + + it_behaves_like 'not enqueuing a sync worker job' + end + + context 'with a non maven package' do + let_it_be(:package) { create(:npm_package) } + + it_behaves_like 'not enqueuing a sync worker job' + end + end + + context 'destroying a composer package' do + let_it_be(:package_name) { 'composer-package-name' } + let_it_be(:json) { { 'name' => package_name } } + let_it_be(:project) { create(:project, :custom_repo, files: { 'composer.json' => json.to_json } ) } + let!(:package) { create(:composer_package, :with_metadatum, project: project, name: package_name, version: '1.0.0', json: json) } + + before do + Gitlab::Composer::Cache.new(project: project, name: package_name).execute + package.composer_metadatum.reload + end + + it 'schedule the update job' do + expect(::Packages::Composer::CacheUpdateWorker).to receive(:perform_async).with(project.id, package_name, package.composer_metadatum.version_cache_sha) + + package.destroy! + end + end end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 0a2b04f1a7c..9e65635da91 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -117,14 +117,6 @@ RSpec.describe Pages::LookupPath do end end - context 'when pages_serve_from_deployments feature flag is disabled' do - before do - stub_feature_flags(pages_serve_from_deployments: false) - end - - include_examples 'uses disk storage' - end - context 'when deployment were created during migration' do before do allow(deployment).to receive(:migrated?).and_return(true) diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 37402fa04c4..a56018f0fee 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -40,7 +40,7 @@ RSpec.describe ProjectFeature do end context 'public features' do - features = %w(issues wiki builds merge_requests snippets repository metrics_dashboard operations) + features = ProjectFeature::FEATURES - %i(pages) features.each do |feature| it "does not allow public access level for #{feature}" do @@ -187,4 +187,30 @@ RSpec.describe ProjectFeature do expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST) end end + + describe 'container_registry_access_level' do + context 'when the project is created with container_registry_enabled false' do + it 'creates project with DISABLED container_registry_access_level' do + project = create(:project, container_registry_enabled: false) + + expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + end + end + + context 'when the project is created with container_registry_enabled true' do + it 'creates project with ENABLED container_registry_access_level' do + project = create(:project, container_registry_enabled: true) + + expect(project.project_feature.container_registry_access_level).to eq(described_class::ENABLED) + end + end + + context 'when the project is created with container_registry_enabled nil' do + it 'creates project with DISABLED container_registry_access_level' do + project = create(:project, container_registry_enabled: nil) + + expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED) + end + end + end end diff --git a/spec/models/project_repository_storage_move_spec.rb b/spec/models/project_repository_storage_move_spec.rb index 88535f6dd6e..eb193a44680 100644 --- a/spec/models/project_repository_storage_move_spec.rb +++ b/spec/models/project_repository_storage_move_spec.rb @@ -9,7 +9,7 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do let(:container) { project } let(:repository_storage_factory_key) { :project_repository_storage_move } let(:error_key) { :project } - let(:repository_storage_worker) { ProjectUpdateRepositoryStorageWorker } + let(:repository_storage_worker) { Projects::UpdateRepositoryStorageWorker } end describe 'state transitions' do diff --git a/spec/models/project_services/discord_service_spec.rb b/spec/models/project_services/discord_service_spec.rb index d4bd08ddeb6..ffe0a36dcdc 100644 --- a/spec/models/project_services/discord_service_spec.rb +++ b/spec/models/project_services/discord_service_spec.rb @@ -6,7 +6,16 @@ RSpec.describe DiscordService do it_behaves_like "chat service", "Discord notifications" do let(:client) { Discordrb::Webhooks::Client } let(:client_arguments) { { url: webhook_url } } - let(:content_key) { :content } + let(:payload) do + { + embeds: [ + include( + author: include(name: be_present), + description: be_present + ) + ] + } + end end describe '#execute' do @@ -58,5 +67,16 @@ RSpec.describe DiscordService do expect { subject.execute(sample_data) }.to raise_error(ArgumentError, /is blocked/) end end + + context 'when the Discord request fails' do + before do + WebMock.stub_request(:post, webhook_url).to_return(status: 400) + end + + it 'logs an error and returns false' do + expect(subject).to receive(:log_error).with('400 Bad Request') + expect(subject.execute(sample_data)).to be(false) + end + end end end diff --git a/spec/models/project_services/hangouts_chat_service_spec.rb b/spec/models/project_services/hangouts_chat_service_spec.rb index 042e32439d1..9d3bd457fc8 100644 --- a/spec/models/project_services/hangouts_chat_service_spec.rb +++ b/spec/models/project_services/hangouts_chat_service_spec.rb @@ -6,6 +6,10 @@ RSpec.describe HangoutsChatService do it_behaves_like "chat service", "Hangouts Chat" do let(:client) { HangoutsChat::Sender } let(:client_arguments) { webhook_url } - let(:content_key) { :text } + let(:payload) do + { + text: be_present + } + end end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 78bd0e91208..3fc39fd3266 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -474,21 +474,32 @@ RSpec.describe JiraService do let(:custom_base_url) { 'http://custom_url' } shared_examples 'close_issue' do + let(:issue_key) { 'JIRA-123' } + let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}" } + let(:transitions_url) { "#{issue_url}/transitions" } + let(:comment_url) { "#{issue_url}/comment" } + let(:remote_link_url) { "#{issue_url}/remotelink" } + let(:transitions) { nil } + + let(:issue_fields) do + { + id: issue_key, + self: issue_url, + transitions: transitions + } + end + + subject(:close_issue) do + jira_service.close_issue(resource, ExternalIssue.new(issue_key, project)) + end + before do - @jira_service = described_class.new - allow(@jira_service).to receive_messages( - project_id: project.id, - project: project, - url: 'http://jira.example.com', - username: 'gitlab_jira_username', - password: 'gitlab_jira_password', - jira_issue_transition_id: '999' - ) + allow(jira_service).to receive_messages(jira_issue_transition_id: '999') # These stubs are needed to test JiraService#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: { 'id' => 'JIRA-123' }) + open_issue = JIRA::Resource::Issue.new(jira_service.client, attrs: issue_fields.deep_stringify_keys) closed_issue = open_issue.dup allow(open_issue).to receive(:resolution).and_return(false) allow(closed_issue).to receive(:resolution).and_return(true) @@ -497,29 +508,22 @@ RSpec.describe JiraService do allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return('JIRA-123') allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) - @jira_service.save! - - project_issues_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123' - @transitions_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123/transitions' - @comment_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123/comment' - @remote_link_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123/remotelink' - - WebMock.stub_request(:get, project_issues_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) - WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) - WebMock.stub_request(:post, @comment_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) - WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)) + WebMock.stub_request(:get, issue_url).with(basic_auth: %w(jira-username jira-password)) + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)) + WebMock.stub_request(:post, comment_url).with(basic_auth: %w(jira-username jira-password)) + WebMock.stub_request(:post, remote_link_url).with(basic_auth: %w(jira-username jira-password)) end let(:external_issue) { ExternalIssue.new('JIRA-123', project) } def close_issue - @jira_service.close_issue(resource, external_issue, current_user) + jira_service.close_issue(resource, external_issue, current_user) end it 'calls Jira API' do close_issue - expect(WebMock).to have_requested(:post, @comment_url).with( + expect(WebMock).to have_requested(:post, comment_url).with( body: /Issue solved with/ ).once end @@ -546,9 +550,9 @@ RSpec.describe JiraService do favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}" # Creates comment - expect(WebMock).to have_requested(:post, @comment_url) + expect(WebMock).to have_requested(:post, comment_url) # Creates Remote Link in Jira issue fields - expect(WebMock).to have_requested(:post, @remote_link_url).with( + expect(WebMock).to have_requested(:post, remote_link_url).with( body: hash_including( GlobalID: 'GitLab', relationship: 'mentioned on', @@ -564,11 +568,11 @@ RSpec.describe JiraService do context 'when "comment_on_event_enabled" is set to false' do it 'creates Remote Link reference but does not create comment' do - allow(@jira_service).to receive_messages(comment_on_event_enabled: false) + allow(jira_service).to receive_messages(comment_on_event_enabled: false) close_issue - expect(WebMock).not_to have_requested(:post, @comment_url) - expect(WebMock).to have_requested(:post, @remote_link_url) + expect(WebMock).not_to have_requested(:post, comment_url) + expect(WebMock).to have_requested(:post, remote_link_url) end end @@ -589,7 +593,7 @@ RSpec.describe JiraService do close_issue - expect(WebMock).not_to have_requested(:post, @comment_url) + expect(WebMock).not_to have_requested(:post, comment_url) end end @@ -598,8 +602,8 @@ RSpec.describe JiraService do close_issue - expect(WebMock).not_to have_requested(:post, @comment_url) - expect(WebMock).not_to have_requested(:post, @remote_link_url) + expect(WebMock).not_to have_requested(:post, comment_url) + expect(WebMock).not_to have_requested(:post, remote_link_url) end it 'does not send comment or remote links to issues with unknown resolution' do @@ -607,8 +611,8 @@ RSpec.describe JiraService do close_issue - expect(WebMock).not_to have_requested(:post, @comment_url) - expect(WebMock).not_to have_requested(:post, @remote_link_url) + expect(WebMock).not_to have_requested(:post, comment_url) + expect(WebMock).not_to have_requested(:post, remote_link_url) end it 'references the GitLab commit' do @@ -616,7 +620,7 @@ RSpec.describe JiraService do close_issue - expect(WebMock).to have_requested(:post, @comment_url).with( + expect(WebMock).to have_requested(:post, comment_url).with( body: %r{#{custom_base_url}/#{project.full_path}/-/commit/#{commit_id}} ).once end @@ -631,18 +635,18 @@ RSpec.describe JiraService do close_issue - expect(WebMock).to have_requested(:post, @comment_url).with( + expect(WebMock).to have_requested(:post, comment_url).with( body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}} ).once end it 'logs exception when transition id is not valid' do - allow(@jira_service).to receive(:log_error) - WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise("Bad Request") + allow(jira_service).to receive(:log_error) + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)).and_raise("Bad Request") close_issue - expect(@jira_service).to have_received(:log_error).with( + expect(jira_service).to have_received(:log_error).with( "Issue transition failed", error: hash_including( exception_class: 'StandardError', @@ -655,34 +659,64 @@ RSpec.describe JiraService do it 'calls the api with jira_issue_transition_id' do close_issue - expect(WebMock).to have_requested(:post, @transitions_url).with( - body: /999/ + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"999"/ ).once end - context 'when have multiple transition ids' do - it 'calls the api with transition ids separated by comma' do - allow(@jira_service).to receive_messages(jira_issue_transition_id: '1,2,3') + context 'when using multiple transition ids' do + before do + allow(jira_service).to receive_messages(jira_issue_transition_id: '1,2,3') + end + it 'calls the api with transition ids separated by comma' do close_issue 1.upto(3) do |transition_id| - expect(WebMock).to have_requested(:post, @transitions_url).with( - body: /#{transition_id}/ + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"#{transition_id}"/ ).once end + + expect(WebMock).to have_requested(:post, comment_url) end it 'calls the api with transition ids separated by semicolon' do - allow(@jira_service).to receive_messages(jira_issue_transition_id: '1;2;3') + allow(jira_service).to receive_messages(jira_issue_transition_id: '1;2;3') close_issue 1.upto(3) do |transition_id| - expect(WebMock).to have_requested(:post, @transitions_url).with( - body: /#{transition_id}/ + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"#{transition_id}"/ ).once end + + expect(WebMock).to have_requested(:post, comment_url) + end + + context 'when a transition fails' do + before do + WebMock.stub_request(:post, transitions_url).with(basic_auth: %w(jira-username jira-password)).to_return do |request| + { status: request.body.include?('"id":"2"') ? 500 : 200 } + end + end + + it 'stops the sequence' do + close_issue + + 1.upto(2) do |transition_id| + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"#{transition_id}"/ + ) + end + + expect(WebMock).not_to have_requested(:post, transitions_url).with( + body: /"id":"3"/ + ) + + expect(WebMock).not_to have_requested(:post, comment_url) + end end end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index ea63406e615..366c3f68e1d 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -511,20 +511,23 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl type: 'checkbox', name: 'manual_configuration', title: s_('PrometheusService|Active'), + help: s_('PrometheusService|Select this checkbox to override the auto configuration settings with your own settings.'), required: true }, { type: 'text', name: 'api_url', title: 'API URL', - placeholder: s_('PrometheusService|Prometheus API Base URL, like http://prometheus.example.com/'), + placeholder: s_('PrometheusService|https://prometheus.example.com/'), + help: s_('PrometheusService|The Prometheus API base URL.'), required: true }, { type: 'text', name: 'google_iap_audience_client_id', title: 'Google IAP Audience Client ID', - placeholder: s_('PrometheusService|Client ID of the IAP secured resource (looks like IAP_CLIENT_ID.apps.googleusercontent.com)'), + placeholder: s_('PrometheusService|IAP_CLIENT_ID.apps.googleusercontent.com'), + help: s_('PrometheusService|PrometheusService|The ID of the IAP-secured resource.'), autocomplete: 'off', required: false }, @@ -532,7 +535,8 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl type: 'textarea', name: 'google_iap_service_account_json', title: 'Google IAP Service Account JSON', - placeholder: s_('PrometheusService|Contents of the credentials.json file of your service account, like: { "type": "service_account", "project_id": ... }'), + placeholder: s_('PrometheusService|{ "type": "service_account", "project_id": ... }'), + help: s_('PrometheusService|The contents of the credentials.json file of your service account.'), required: false } ] diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 0b35b9e7b30..aa5d92e5c61 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -4,4 +4,116 @@ require 'spec_helper' RSpec.describe SlackService do it_behaves_like "slack or mattermost notifications", 'Slack' + + describe '#execute' do + before do + stub_request(:post, "https://slack.service.url/") + end + + let_it_be(:slack_service) { create(:slack_service, branches_to_be_notified: 'all') } + + it 'uses only known events', :aggregate_failures do + described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| + expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true + end + end + + context 'hook data includes a user object' do + let_it_be(:user) { create_default(:user) } + let_it_be(:project) { create_default(:project, :repository, :wiki_repo) } + + shared_examples 'increases the usage data counter' do |event_name| + it 'increases the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(event_name, values: user.id).and_call_original + + slack_service.execute(data) + end + end + + context 'event is not supported for usage log' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + it 'does not increase the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with('i_ecosystem_slack_service_pipeline_notification', values: user.id) + + slack_service.execute(data) + end + end + + context 'issue notification' do + let_it_be(:issue) { create(:issue) } + let(:data) { issue.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_issue_notification' + end + + context 'push notification' do + let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_push_notification' + end + + context 'deployment notification' do + let_it_be(:deployment) { create(:deployment, user: user) } + let(:data) { Gitlab::DataBuilder::Deployment.build(deployment) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_deployment_notification' + end + + context 'wiki_page notification' do + let_it_be(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') } + let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_wiki_page_notification' + end + + context 'merge_request notification' do + let_it_be(:merge_request) { create(:merge_request) } + let(:data) { merge_request.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_merge_request_notification' + end + + context 'note notification' do + let_it_be(:issue_note) { create(:note_on_issue, note: 'issue note') } + let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_note_notification' + end + + context 'tag_push notification' do + let(:oldrev) { Gitlab::Git::BLANK_SHA } + let(:newrev) { '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b' } # gitlab-test: git rev-parse refs/tags/v1.1.0 + let(:ref) { 'refs/tags/v1.1.0' } + let(:data) { Git::TagHooksService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }).send(:push_data) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_tag_push_notification' + end + + context 'confidential note notification' do + let_it_be(:confidential_issue_note) { create(:note_on_issue, note: 'issue note', confidential: true) } + let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_note_notification' + end + + context 'confidential issue notification' do + let_it_be(:issue) { create(:issue, confidential: true) } + let(:data) { issue.to_hook_data(user) } + + it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_confidential_issue_notification' + end + end + + context 'hook data does not include a user' do + let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline)) } + + it 'does not increase the usage data counter' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + slack_service.execute(data) + end + end + end end diff --git a/spec/models/project_services/unify_circuit_service_spec.rb b/spec/models/project_services/unify_circuit_service_spec.rb index 73702aa8471..0c749322e07 100644 --- a/spec/models/project_services/unify_circuit_service_spec.rb +++ b/spec/models/project_services/unify_circuit_service_spec.rb @@ -5,6 +5,12 @@ require "spec_helper" RSpec.describe UnifyCircuitService do it_behaves_like "chat service", "Unify Circuit" do let(:client_arguments) { webhook_url } - let(:content_key) { :subject } + let(:payload) do + { + subject: project.full_name, + text: be_present, + markdown: true + } + end end end diff --git a/spec/models/project_services/webex_teams_service_spec.rb b/spec/models/project_services/webex_teams_service_spec.rb index bd73d0c93b8..ed63f5bc48c 100644 --- a/spec/models/project_services/webex_teams_service_spec.rb +++ b/spec/models/project_services/webex_teams_service_spec.rb @@ -5,6 +5,10 @@ require "spec_helper" RSpec.describe WebexTeamsService do it_behaves_like "chat service", "Webex Teams" do let(:client_arguments) { webhook_url } - let(:content_key) { :markdown } + let(:payload) do + { + markdown: be_present + } + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fd7975bf65d..1cee494989d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Project, factory_default: :keep do include ExternalAuthorizationServiceHelpers using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create_default(:namespace) } + let_it_be(:namespace) { create_default(:namespace).freeze } it_behaves_like 'having unique enum values' @@ -145,7 +145,7 @@ RSpec.describe Project, factory_default: :keep do end it_behaves_like 'model with wiki' do - let_it_be(:container) { create(:project, :wiki_repo) } + let_it_be(:container) { create(:project, :wiki_repo, namespace: create(:group)) } let(:container_without_wiki) { create(:project) } end @@ -1599,7 +1599,7 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#any_runners?' do + describe '#any_active_runners?' do context 'shared runners' do let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } @@ -1609,31 +1609,31 @@ RSpec.describe Project, factory_default: :keep do let(:shared_runners_enabled) { false } it 'has no runners available' do - expect(project.any_runners?).to be_falsey + expect(project.any_active_runners?).to be_falsey end it 'has a specific runner' do specific_runner - expect(project.any_runners?).to be_truthy + expect(project.any_active_runners?).to be_truthy end it 'has a shared runner, but they are prohibited to use' do shared_runner - expect(project.any_runners?).to be_falsey + expect(project.any_active_runners?).to be_falsey end it 'checks the presence of specific runner' do specific_runner - expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + 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_runners? { false }).to be_falsey + expect(project.any_active_runners? { false }).to be_falsey end end @@ -1643,19 +1643,19 @@ RSpec.describe Project, factory_default: :keep do it 'has a shared runner' do shared_runner - expect(project.any_runners?).to be_truthy + expect(project.any_active_runners?).to be_truthy end it 'checks the presence of shared runner' do shared_runner - expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + 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_runners? { false }).to be_falsey + expect(project.any_active_runners? { false }).to be_falsey end end end @@ -1669,13 +1669,13 @@ RSpec.describe Project, factory_default: :keep do let(:group_runners_enabled) { false } it 'has no runners available' do - expect(project.any_runners?).to be_falsey + expect(project.any_active_runners?).to be_falsey end it 'has a group runner, but they are prohibited to use' do group_runner - expect(project.any_runners?).to be_falsey + expect(project.any_active_runners?).to be_falsey end end @@ -1685,19 +1685,19 @@ RSpec.describe Project, factory_default: :keep do it 'has a group runner' do group_runner - expect(project.any_runners?).to be_truthy + expect(project.any_active_runners?).to be_truthy end it 'checks the presence of group runner' do group_runner - expect(project.any_runners? { |runner| runner == group_runner }).to be_truthy + 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_runners? { false }).to be_falsey + expect(project.any_active_runners? { false }).to be_falsey end end end @@ -1799,7 +1799,8 @@ RSpec.describe Project, factory_default: :keep do describe '#default_branch_protected?' do using RSpec::Parameterized::TableSyntax - let_it_be(:project) { create(:project) } + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } subject { project.default_branch_protected? } @@ -2201,6 +2202,44 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#set_container_registry_access_level' do + let_it_be_with_reload(:project) { create(:project) } + + 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.update!(container_registry_enabled: false) + + expect(project.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.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) + + 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) + end + end + describe '#has_container_registry_tags?' do let(:project) { build(:project) } @@ -2802,7 +2841,8 @@ RSpec.describe Project, factory_default: :keep do end describe '#emails_disabled?' do - let(:project) { build(:project, emails_disabled: false) } + let_it_be(:namespace) { create(:namespace) } + let(:project) { build(:project, namespace: namespace, emails_disabled: false) } context 'emails disabled in group' do it 'returns true' do @@ -2830,7 +2870,8 @@ RSpec.describe Project, factory_default: :keep do end describe '#lfs_enabled?' do - let(:project) { build(:project) } + let(:namespace) { create(:namespace) } + let(:project) { build(:project, namespace: namespace) } shared_examples 'project overrides group' do it 'returns true when enabled in project' do @@ -4463,7 +4504,11 @@ RSpec.describe Project, factory_default: :keep do subject { project.predefined_project_variables.to_runner_variables } specify do - expect(subject).to include({ key: 'CI_PROJECT_CONFIG_PATH', value: Ci::Pipeline::DEFAULT_CONFIG_PATH, public: true, masked: false }) + 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 context 'when ci config path is overridden' do @@ -4471,7 +4516,41 @@ RSpec.describe Project, factory_default: :keep do project.update!(ci_config_path: 'random.yml') end - it { expect(subject).to include({ key: 'CI_PROJECT_CONFIG_PATH', value: 'random.yml', public: true, masked: false }) } + 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 + end + end + + describe '#dependency_proxy_variables' do + let_it_be(:namespace) { create(:namespace, path: 'NameWithUPPERcaseLetters') } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } + + subject { project.dependency_proxy_variables.to_runner_variables } + + context 'when dependency_proxy is enabled' do + before do + stub_config(dependency_proxy: { enabled: true }) + end + + it 'contains the downcased name' do + expect(subject).to include({ key: 'CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX', + value: "#{Gitlab.host_with_port}/namewithuppercaseletters#{DependencyProxy::URL_SUFFIX}", + public: true, + masked: false }) + end + end + + context 'when dependency_proxy is disabled' do + before do + stub_config(dependency_proxy: { enabled: false }) + end + + it { expect(subject).to be_empty } end end @@ -4877,7 +4956,8 @@ RSpec.describe Project, factory_default: :keep do end context 'branch protection' do - let(:project) { create(:project, :repository) } + let_it_be(:namespace) { create(:namespace) } + let(:project) { create(:project, :repository, namespace: namespace) } before do create(:import_state, :started, project: project) diff --git a/spec/models/projects/repository_storage_move_spec.rb b/spec/models/projects/repository_storage_move_spec.rb new file mode 100644 index 00000000000..ab0ad81f77a --- /dev/null +++ b/spec/models/projects/repository_storage_move_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::RepositoryStorageMove, 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/prometheus_alert_event_spec.rb b/spec/models/prometheus_alert_event_spec.rb index 913ca7db0be..6bff549bc4b 100644 --- a/spec/models/prometheus_alert_event_spec.rb +++ b/spec/models/prometheus_alert_event_spec.rb @@ -52,7 +52,7 @@ RSpec.describe PrometheusAlertEvent do let(:started_at) { Time.current } context 'when status is none' do - subject { build(:prometheus_alert_event, :none) } + subject { build(:prometheus_alert_event, status: nil, started_at: nil) } it 'fires an event' do result = subject.fire(started_at) diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index a89f8778780..a173ab48f17 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -207,6 +207,28 @@ RSpec.describe ProtectedBranch do end end + describe "#allow_force_push?" do + context "when the attr allow_force_push is true" do + let(:subject_branch) { create(:protected_branch, allow_force_push: true, name: "foo") } + + it "returns true" do + project = subject_branch.project + + expect(described_class.allow_force_push?(project, "foo")).to eq(true) + end + end + + context "when the attr allow_force_push is false" do + let(:subject_branch) { create(:protected_branch, allow_force_push: false, name: "foo") } + + it "returns false" do + project = subject_branch.project + + expect(described_class.allow_force_push?(project, "foo")).to eq(false) + end + end + end + describe '#any_protected?' do context 'existing project' do let(:project) { create(:project, :repository) } diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index cdbc1feefce..11196f06529 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -286,6 +286,7 @@ RSpec.describe SnippetRepository do context 'with git errors' do it_behaves_like 'snippet repository with git errors', 'invalid://path/here', described_class::InvalidPathError + it_behaves_like 'snippet repository with git errors', '.git/hooks/pre-commit', described_class::InvalidPathError it_behaves_like 'snippet repository with git errors', '../../path/traversal/here', described_class::InvalidPathError it_behaves_like 'snippet repository with git errors', 'README', described_class::CommitError diff --git a/spec/models/snippet_repository_storage_move_spec.rb b/spec/models/snippet_repository_storage_move_spec.rb index 357951f8859..f5ad837fb36 100644 --- a/spec/models/snippet_repository_storage_move_spec.rb +++ b/spec/models/snippet_repository_storage_move_spec.rb @@ -8,6 +8,6 @@ RSpec.describe SnippetRepositoryStorageMove, type: :model do let(:repository_storage_factory_key) { :snippet_repository_storage_move } let(:error_key) { :snippet } - let(:repository_storage_worker) { SnippetUpdateRepositoryStorageWorker } + let(:repository_storage_worker) { Snippets::UpdateRepositoryStorageWorker } end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 623767d19e0..09f9cf8e222 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Snippet do it { is_expected.to have_many(:user_mentions).class_name("SnippetUserMention") } it { is_expected.to have_one(:snippet_repository) } it { is_expected.to have_one(:statistics).class_name('SnippetStatistics').dependent(:destroy) } - it { is_expected.to have_many(:repository_storage_moves).class_name('SnippetRepositoryStorageMove').inverse_of(:container) } + it { is_expected.to have_many(:repository_storage_moves).class_name('Snippets::RepositoryStorageMove').inverse_of(:container) } end describe 'validation' do @@ -496,6 +496,16 @@ RSpec.describe Snippet do it 'returns array of blobs' do expect(snippet.blobs).to all(be_a(Blob)) end + + context 'when file does not exist' do + it 'removes nil values from the blobs array' do + allow(snippet).to receive(:list_files).and_return(%w(LICENSE non_existent_snippet_file)) + + blobs = snippet.blobs + expect(blobs.count).to eq 1 + expect(blobs.first.name).to eq 'LICENSE' + end + end end end diff --git a/spec/models/snippets/repository_storage_move_spec.rb b/spec/models/snippets/repository_storage_move_spec.rb new file mode 100644 index 00000000000..ed518faf6ff --- /dev/null +++ b/spec/models/snippets/repository_storage_move_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Snippets::RepositoryStorageMove, 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/todo_spec.rb b/spec/models/todo_spec.rb index a9c4c6680cd..855b1b0f3f7 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -363,23 +363,6 @@ RSpec.describe Todo do end end - describe '.for_ids' do - it 'returns the expected todos' do - todo1 = create(:todo) - todo2 = create(:todo) - todo3 = create(:todo) - create(:todo) - - expect(described_class.for_ids([todo2.id, todo1.id, todo3.id])).to contain_exactly(todo1, todo2, todo3) - end - - it 'returns an empty collection when no ids are given' do - create(:todo) - - expect(described_class.for_ids([])).to be_empty - end - end - describe '.for_user' do it 'returns the expected todos' do user1 = create(:user) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 18388b4cd83..6bac5e31435 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -221,7 +221,7 @@ RSpec.describe Upload do it 'does not send a message to Sentry' do upload = described_class.new(path: "#{__FILE__}-nope", store: ObjectStorage::Store::LOCAL) - expect(Raven).not_to receive(:capture_message) + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) upload.exist? end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 860c015e166..5f2842c9d16 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -41,6 +41,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:show_whitespace_in_diffs).to(:user_preference) } it { is_expected.to delegate_method(:show_whitespace_in_diffs=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:view_diffs_file_by_file).to(:user_preference) } + it { is_expected.to delegate_method(:view_diffs_file_by_file=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:tab_width).to(:user_preference) } it { is_expected.to delegate_method(:tab_width=).to(:user_preference).with_arguments(:args) } @@ -59,6 +62,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:experience_level).to(:user_preference) } it { is_expected.to delegate_method(:experience_level=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:markdown_surround_selection).to(:user_preference) } + it { is_expected.to delegate_method(:markdown_surround_selection=).to(:user_preference).with_arguments(:args) } + 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 } @@ -101,6 +107,7 @@ RSpec.describe User do it { is_expected.to have_many(:reviews).inverse_of(:author) } it { is_expected.to have_many(:merge_request_assignees).inverse_of(:assignee) } it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) } + it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) } describe "#user_detail" do it 'does not persist `user_detail` by default' do @@ -380,11 +387,11 @@ RSpec.describe User do it { is_expected.not_to allow_value(-1).for(:projects_limit) } it { is_expected.not_to allow_value(Gitlab::Database::MAX_INT_VALUE + 1).for(:projects_limit) } - it_behaves_like 'an object with email-formated attributes', :email do + it_behaves_like 'an object with email-formatted attributes', :email do subject { build(:user) } end - it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do + it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :public_email, :notification_email do subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } end @@ -1050,7 +1057,7 @@ RSpec.describe User do let(:user) { create(:user) } let(:external_user) { create(:user, external: true) } - it "sets other properties aswell" do + it "sets other properties as well" do expect(external_user.can_create_team).to be_falsey expect(external_user.can_create_group).to be_falsey expect(external_user.projects_limit).to be 0 @@ -1061,7 +1068,7 @@ RSpec.describe User do let(:user) { create(:user) } let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } - it 'allows a verfied secondary email to be used as the primary without needing reconfirmation' do + it 'allows a verified secondary email to be used as the primary without needing reconfirmation' do user.update!(email: secondary.email) user.reload expect(user.email).to eq secondary.email @@ -1827,7 +1834,7 @@ RSpec.describe User do end describe '.instance_access_request_approvers_to_be_notified' do - let_it_be(:admin_list) { create_list(:user, 12, :admin, :with_sign_ins) } + let_it_be(:admin_issue_board_list) { create_list(:user, 12, :admin, :with_sign_ins) } it 'returns up to the ten most recently active instance admins' do active_admins_in_recent_sign_in_desc_order = User.admins.active.order_recent_sign_in.limit(10) @@ -2492,6 +2499,38 @@ RSpec.describe User do end end + describe "#clear_avatar_caches" do + let(:user) { create(:user) } + + context "when :avatar_cache_for_email flag is enabled" do + before do + stub_feature_flags(avatar_cache_for_email: true) + end + + it "clears the avatar cache when saving" do + allow(user).to receive(:avatar_changed?).and_return(true) + + expect(Gitlab::AvatarCache).to receive(:delete_by_email).with(*user.verified_emails) + + user.update(avatar: fixture_file_upload('spec/fixtures/dk.png')) + end + end + + context "when :avatar_cache_for_email flag is disabled" do + before do + stub_feature_flags(avatar_cache_for_email: false) + end + + it "doesn't attempt to clear the avatar cache" do + allow(user).to receive(:avatar_changed?).and_return(true) + + expect(Gitlab::AvatarCache).not_to receive(:delete_by_email) + + user.update(avatar: fixture_file_upload('spec/fixtures/dk.png')) + end + end + end + describe '#accept_pending_invitations!' do let(:user) { create(:user, email: 'user@email.com') } let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) } @@ -3227,23 +3266,8 @@ RSpec.describe User do create(:group_group_link, shared_group: private_group, shared_with_group: other_group) end - context 'when shared_group_membership_auth is enabled' do - before do - stub_feature_flags(shared_group_membership_auth: user) - end - - it { is_expected.to include shared_group } - it { is_expected.not_to include other_group } - end - - context 'when shared_group_membership_auth is disabled' do - before do - stub_feature_flags(shared_group_membership_auth: false) - end - - it { is_expected.not_to include shared_group } - it { is_expected.not_to include other_group } - end + it { is_expected.to include shared_group } + it { is_expected.not_to include other_group } end end @@ -3937,6 +3961,37 @@ RSpec.describe User do end end + describe '#can_admin_all_resources?', :request_store do + it 'returns false for regular user' do + user = build_stubbed(:user) + + expect(user.can_admin_all_resources?).to be_falsy + end + + context 'for admin user' do + include_context 'custom session' + + let(:user) { build_stubbed(:user, :admin) } + + context 'when admin mode is disabled' do + it 'returns false' do + expect(user.can_admin_all_resources?).to be_falsy + end + end + + context 'when admin mode is enabled' do + before do + Gitlab::Auth::CurrentUserMode.new(user).request_admin_mode! + Gitlab::Auth::CurrentUserMode.new(user).enable_admin_mode!(password: user.password) + end + + it 'returns true' do + expect(user.can_admin_all_resources?).to be_truthy + end + end + end + end + describe '.ghost' do it "creates a ghost user if one isn't already present" do ghost = described_class.ghost @@ -5370,6 +5425,40 @@ RSpec.describe User do end end + describe 'can_trigger_notifications?' do + context 'when user is not confirmed' do + let_it_be(:user) { create(:user, :unconfirmed) } + + it 'returns false' do + expect(user.can_trigger_notifications?).to be(false) + end + end + + context 'when user is blocked' do + let_it_be(:user) { create(:user, :blocked) } + + it 'returns false' do + expect(user.can_trigger_notifications?).to be(false) + end + end + + context 'when user is a ghost' do + let_it_be(:user) { create(:user, :ghost) } + + it 'returns false' do + expect(user.can_trigger_notifications?).to be(false) + end + end + + context 'when user is confirmed and neither blocked or a ghost' do + let_it_be(:user) { create(:user) } + + it 'returns true' do + expect(user.can_trigger_notifications?).to be(true) + end + end + end + context 'bot users' do shared_examples 'bot users' do |bot_type| it 'creates the user if it does not exist' do @@ -5412,4 +5501,89 @@ RSpec.describe User do it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' end + + describe '#confirmation_required_on_sign_in?' do + subject { user.confirmation_required_on_sign_in? } + + context 'when user is confirmed' do + let(:user) { build_stubbed(:user) } + + it 'is falsey' do + expect(user.confirmed?).to be_truthy + expect(subject).to be_falsey + end + end + + context 'when user is not confirmed' do + let_it_be(:user) { build_stubbed(:user, :unconfirmed, confirmation_sent_at: Time.current) } + + it 'is truthy when soft_email_confirmation feature is disabled' do + stub_feature_flags(soft_email_confirmation: false) + expect(subject).to be_truthy + end + + context 'when soft_email_confirmation feature is enabled' do + before do + stub_feature_flags(soft_email_confirmation: true) + end + + it 'is falsey when confirmation period is valid' do + expect(subject).to be_falsey + end + + it 'is truthy when confirmation period is expired' do + travel_to(User.allow_unconfirmed_access_for.from_now + 1.day) do + expect(subject).to be_truthy + end + end + + context 'when user has no confirmation email sent' do + let(:user) { build(:user, :unconfirmed, confirmation_sent_at: nil) } + + it 'is truthy' do + expect(subject).to be_truthy + end + end + end + end + end + + describe '#find_or_initialize_callout' do + subject(:find_or_initialize_callout) { user.find_or_initialize_callout(feature_name) } + + let(:user) { create(:user) } + let(:feature_name) { UserCallout.feature_names.each_key.first } + + context 'when callout exists' do + let!(:callout) { create(:user_callout, user: user, feature_name: feature_name) } + + it 'returns existing callout' do + expect(find_or_initialize_callout).to eq(callout) + end + end + + context 'when callout does not exist' do + context 'when feature name is valid' do + it 'initializes a new callout' do + expect(find_or_initialize_callout).to be_a_new(UserCallout) + end + + it 'is valid' do + expect(find_or_initialize_callout).to be_valid + end + end + + context 'when feature name is not valid' do + let(:feature_name) { 'notvalid' } + + it 'initializes a new callout' do + expect(find_or_initialize_callout).to be_a_new(UserCallout) + end + + it 'is not valid' do + expect(find_or_initialize_callout).not_to be_valid + end + end + end + end end |