diff options
Diffstat (limited to 'spec/models')
112 files changed, 4082 insertions, 1133 deletions
diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 6a0f2290b4c..7e6ac351e68 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -93,13 +93,6 @@ RSpec.describe ApplicationRecord do end end - describe '.at_most' do - it 'limits the number of records returned' do - create_list(:user, 3) - expect(User.at_most(2).count).to eq(2) - end - end - describe '.where_exists' do it 'produces a WHERE EXISTS query' do user = create(:user) @@ -107,4 +100,33 @@ RSpec.describe ApplicationRecord do expect(User.where_exists(User.limit(1))).to eq([user]) end end + + describe '.with_fast_read_statement_timeout' do + context 'when the query runs faster than configured timeout' do + it 'executes the query without error' do + result = nil + + expect do + described_class.with_fast_read_statement_timeout(100) do + result = described_class.connection.exec_query('SELECT 1') + end + end.not_to raise_error + + expect(result).not_to be_nil + end + end + + # This query hangs for 10ms and then gets cancelled. As there is no + # other way to test the timeout for sure, 10ms of waiting seems to be + # reasonable! + context 'when the query runs longer than configured timeout' do + it 'cancels the query and raises an exception' do + expect do + described_class.with_fast_read_statement_timeout(10) do + described_class.connection.exec_query('SELECT pg_sleep(0.1)') + end + end.to raise_error(ActiveRecord::QueryCanceled) + end + end + end end diff --git a/spec/models/audit_event_archived_spec.rb b/spec/models/audit_event_archived_spec.rb deleted file mode 100644 index 43a2e8434b0..00000000000 --- a/spec/models/audit_event_archived_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe AuditEventArchived do - let(:source_table) { AuditEvent } - let(:destination_table) { described_class } - - it 'has the same columns as the source table' do - column_names_from_source_table = column_names(source_table) - column_names_from_destination_table = column_names(destination_table) - - expect(column_names_from_destination_table).to match_array(column_names_from_source_table) - end - - it 'has the same null constraints as the source table' do - constraints_from_source_table = null_constraints(source_table) - constraints_from_destination_table = null_constraints(destination_table) - - expect(constraints_from_destination_table.to_a).to match_array(constraints_from_source_table.to_a) - end - - it 'inserts the same record as the one in the source table', :aggregate_failures do - expect { create(:audit_event) }.to change { destination_table.count }.by(1) - - event_from_source_table = source_table.connection.select_one( - "SELECT * FROM #{source_table.table_name} ORDER BY created_at desc LIMIT 1" - ) - event_from_destination_table = destination_table.connection.select_one( - "SELECT * FROM #{destination_table.table_name} ORDER BY created_at desc LIMIT 1" - ) - - expect(event_from_destination_table).to eq(event_from_source_table) - end - - def column_names(table) - table.connection.select_all(<<~SQL) - SELECT c.column_name - FROM information_schema.columns c - WHERE c.table_name = '#{table.table_name}' - SQL - end - - def null_constraints(table) - table.connection.select_all(<<~SQL) - SELECT c.column_name, c.is_nullable - FROM information_schema.columns c - WHERE c.table_name = '#{table.table_name}' - AND c.column_name != 'created_at' - SQL - end -end diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index cd885d312dc..803614d90a5 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -8,6 +8,7 @@ RSpec.describe BlobViewer::GitlabCiYml do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } + let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) } let(:sha) { sample_commit.id } diff --git a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb index 84dfc5186a8..8d5c7ce84f6 100644 --- a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb +++ b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb @@ -7,6 +7,7 @@ RSpec.describe BlobViewer::MetricsDashboardYml do include RepoHelpers let_it_be(:project) { create(:project, :repository) } + let(:blob) { fake_blob(path: '.gitlab/dashboards/custom-dashboard.yml', data: data) } let(:sha) { sample_commit.id } let(:data) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 17ab4d5954c..652ea431696 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -125,68 +125,4 @@ RSpec.describe BulkImports::Entity, type: :model do end end end - - describe "#update_tracker_for" do - let(:entity) { create(:bulk_import_entity) } - - it "inserts new tracker when it does not exist" do - expect do - entity.update_tracker_for(relation: :relation, has_next_page: false) - end.to change(BulkImports::Tracker, :count).by(1) - - tracker = entity.trackers.last - - expect(tracker.relation).to eq('relation') - expect(tracker.has_next_page).to eq(false) - expect(tracker.next_page).to eq(nil) - end - - it "updates the tracker if it already exist" do - create( - :bulk_import_tracker, - relation: :relation, - has_next_page: false, - entity: entity - ) - - expect do - entity.update_tracker_for(relation: :relation, has_next_page: true, next_page: 'nextPage') - end.not_to change(BulkImports::Tracker, :count) - - tracker = entity.trackers.last - - expect(tracker.relation).to eq('relation') - expect(tracker.has_next_page).to eq(true) - expect(tracker.next_page).to eq('nextPage') - end - end - - describe "#has_next_page?" do - it "queries for the given relation if it has more pages to be fetched" do - entity = create(:bulk_import_entity) - create( - :bulk_import_tracker, - relation: :relation, - has_next_page: false, - entity: entity - ) - - expect(entity.has_next_page?(:relation)).to eq(false) - end - end - - describe "#next_page_for" do - it "queries for the next page of the given relation" do - entity = create(:bulk_import_entity) - create( - :bulk_import_tracker, - relation: :relation, - has_next_page: false, - next_page: 'nextPage', - entity: entity - ) - - expect(entity.next_page_for(:relation)).to eq('nextPage') - end - end end diff --git a/spec/models/bulk_imports/stage_spec.rb b/spec/models/bulk_imports/stage_spec.rb new file mode 100644 index 00000000000..7765fd4c5c4 --- /dev/null +++ b/spec/models/bulk_imports/stage_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Stage do + let(:pipelines) do + if Gitlab.ee? + [ + [0, BulkImports::Groups::Pipelines::GroupPipeline], + [1, BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline], + [1, BulkImports::Groups::Pipelines::MembersPipeline], + [1, BulkImports::Groups::Pipelines::LabelsPipeline], + [1, BulkImports::Groups::Pipelines::MilestonesPipeline], + [1, BulkImports::Groups::Pipelines::BadgesPipeline], + [1, 'BulkImports::Groups::Pipelines::IterationsPipeline'.constantize], + [2, 'BulkImports::Groups::Pipelines::EpicsPipeline'.constantize], + [3, 'BulkImports::Groups::Pipelines::EpicAwardEmojiPipeline'.constantize], + [3, 'BulkImports::Groups::Pipelines::EpicEventsPipeline'.constantize], + [4, BulkImports::Groups::Pipelines::EntityFinisher] + ] + else + [ + [0, BulkImports::Groups::Pipelines::GroupPipeline], + [1, BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline], + [1, BulkImports::Groups::Pipelines::MembersPipeline], + [1, BulkImports::Groups::Pipelines::LabelsPipeline], + [1, BulkImports::Groups::Pipelines::MilestonesPipeline], + [1, BulkImports::Groups::Pipelines::BadgesPipeline], + [2, BulkImports::Groups::Pipelines::EntityFinisher] + ] + end + end + + describe '.pipelines' do + it 'list all the pipelines with their stage number, ordered by stage' do + expect(described_class.pipelines).to match_array(pipelines) + end + end + + describe '.pipeline_exists?' do + it 'returns true when the given pipeline name exists in the pipelines list' do + expect(described_class.pipeline_exists?(BulkImports::Groups::Pipelines::GroupPipeline)).to eq(true) + expect(described_class.pipeline_exists?('BulkImports::Groups::Pipelines::GroupPipeline')).to eq(true) + end + + it 'returns false when the given pipeline name exists in the pipelines list' do + expect(described_class.pipeline_exists?('BulkImports::Groups::Pipelines::InexistentPipeline')).to eq(false) + end + end +end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 77896105959..0f00aeb9c1d 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -26,4 +26,60 @@ RSpec.describe BulkImports::Tracker, type: :model do end end end + + describe '.stage_running?' do + it 'returns true if there is any unfinished pipeline in the given stage' do + tracker = create(:bulk_import_tracker) + + expect(described_class.stage_running?(tracker.entity.id, 0)) + .to eq(true) + end + + it 'returns false if there are no unfinished pipeline in the given stage' do + tracker = create(:bulk_import_tracker, :finished) + + expect(described_class.stage_running?(tracker.entity.id, 0)) + .to eq(false) + end + end + + describe '.next_pipeline_trackers_for' do + let_it_be(:entity) { create(:bulk_import_entity) } + let_it_be(:stage_0_tracker) { create(:bulk_import_tracker, :finished, entity: entity) } + + it 'returns empty when all the stages pipelines are finished' do + expect(described_class.next_pipeline_trackers_for(entity.id)) + .to eq([]) + end + + it 'returns the not started pipeline trackers from the minimum stage number' do + stage_1_tracker = create(:bulk_import_tracker, entity: entity, stage: 1) + stage_2_tracker = create(:bulk_import_tracker, entity: entity, stage: 2) + + expect(described_class.next_pipeline_trackers_for(entity.id)) + .to include(stage_1_tracker) + + expect(described_class.next_pipeline_trackers_for(entity.id)) + .not_to include(stage_2_tracker) + end + end + + describe '#pipeline_class' do + it 'returns the pipeline class' do + pipeline_class = BulkImports::Stage.pipelines.first[1] + tracker = create(:bulk_import_tracker, pipeline_name: pipeline_class) + + expect(tracker.pipeline_class).to eq(pipeline_class) + end + + it 'raises an error when the pipeline is not valid' do + tracker = create(:bulk_import_tracker, pipeline_name: 'InexistingPipeline') + + expect { tracker.pipeline_class } + .to raise_error( + NameError, + "'InexistingPipeline' is not a valid BulkImport Pipeline" + ) + end + end end diff --git a/spec/models/ci/artifact_blob_spec.rb b/spec/models/ci/artifact_blob_spec.rb index 44f895cc1c5..c00f46683b9 100644 --- a/spec/models/ci/artifact_blob_spec.rb +++ b/spec/models/ci/artifact_blob_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Ci::ArtifactBlob do let_it_be(:project) { create(:project, :public) } let_it_be(:build) { create(:ci_build, :artifacts, project: project) } + let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/another-subdirectory/banana_sample.gif') } subject { described_class.new(entry) } diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index f3029598b02..db956b26b6b 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -50,7 +50,7 @@ RSpec.describe Ci::Bridge do CI_PROJECT_PATH_SLUG CI_PROJECT_NAMESPACE CI_PROJECT_ROOT_NAMESPACE CI_PIPELINE_IID CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION CI_COMMIT_REF_PROTECTED - CI_COMMIT_TIMESTAMP + CI_COMMIT_TIMESTAMP CI_COMMIT_AUTHOR ] expect(bridge.scoped_variables.map { |v| v[:key] }).to include(*variables) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5b07bd8923f..339dffa507f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -585,6 +585,68 @@ RSpec.describe Ci::Build do is_expected.to be_falsey end end + + context 'with runners_cached_states feature flag enabled' do + before do + stub_feature_flags(runners_cached_states: true) + end + + it 'caches the result in Redis' do + expect(Rails.cache).to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute) + + build.any_runners_online? + end + end + + context 'with runners_cached_states feature flag disabled' do + before do + stub_feature_flags(runners_cached_states: false) + end + + it 'does not cache' do + expect(Rails.cache).not_to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute) + + build.any_runners_online? + end + end + end + + describe '#any_runners_available?' do + subject { build.any_runners_available? } + + context 'when no runners' do + it { is_expected.to be_falsey } + end + + context 'when there are runners' do + let!(:runner) { create(:ci_runner, :project, projects: [build.project]) } + + it { is_expected.to be_truthy } + end + + context 'with runners_cached_states feature flag enabled' do + before do + stub_feature_flags(runners_cached_states: true) + end + + it 'caches the result in Redis' do + expect(Rails.cache).to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute) + + build.any_runners_available? + end + end + + context 'with runners_cached_states feature flag disabled' do + before do + stub_feature_flags(runners_cached_states: false) + end + + it 'does not cache' do + expect(Rails.cache).not_to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute) + + build.any_runners_available? + end + end end describe '#artifacts?' do @@ -821,45 +883,6 @@ RSpec.describe Ci::Build 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 } context 'when build has cache' do @@ -1174,6 +1197,8 @@ RSpec.describe Ci::Build do end describe 'state transition as a deployable' do + subject { build.send(event) } + let!(:build) { create(:ci_build, :with_deployment, :start_review_app, project: project, pipeline: pipeline) } let(:deployment) { build.deployment } let(:environment) { deployment.environment } @@ -1188,54 +1213,78 @@ RSpec.describe Ci::Build do expect(environment.name).to eq('review/master') end - context 'when transits to running' do - before do - build.run! + shared_examples_for 'avoid deadlock' do + it 'executes UPDATE in the right order' do + recorded = ActiveRecord::QueryRecorder.new { subject } + + index_for_build = recorded.log.index { |l| l.include?("UPDATE \"ci_builds\"") } + index_for_deployment = recorded.log.index { |l| l.include?("UPDATE \"deployments\"") } + + expect(index_for_build).to be < index_for_deployment end + end + + context 'when transits to running' do + let(:event) { :run! } + + it_behaves_like 'avoid deadlock' it 'transits deployment status to running' do + subject + expect(deployment).to be_running end end context 'when transits to success' do + let(:event) { :success! } + before do allow(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) - build.success! end + it_behaves_like 'avoid deadlock' + it 'transits deployment status to success' do + subject + expect(deployment).to be_success end end context 'when transits to failed' do - before do - build.drop! - end + let(:event) { :drop! } + + it_behaves_like 'avoid deadlock' it 'transits deployment status to failed' do + subject + expect(deployment).to be_failed end end context 'when transits to skipped' do - before do - build.skip! - end + let(:event) { :skip! } + + it_behaves_like 'avoid deadlock' it 'transits deployment status to skipped' do + subject + expect(deployment).to be_skipped end end context 'when transits to canceled' do - before do - build.cancel! - end + let(:event) { :cancel! } + + it_behaves_like 'avoid deadlock' it 'transits deployment status to canceled' do + subject + expect(deployment).to be_canceled end end @@ -2500,6 +2549,7 @@ RSpec.describe Ci::Build do { key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true, masked: false }, { key: 'CI_COMMIT_REF_PROTECTED', value: (!!pipeline.protected_ref?).to_s, public: true, masked: false }, { key: 'CI_COMMIT_TIMESTAMP', value: pipeline.git_commit_timestamp, public: true, masked: false }, + { key: 'CI_COMMIT_AUTHOR', value: pipeline.git_author_full_text, public: true, masked: false }, { key: 'CI_BUILD_REF', value: build.sha, public: true, masked: false }, { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true, masked: false }, { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true, masked: false }, @@ -3620,10 +3670,10 @@ RSpec.describe Ci::Build do end describe 'state transition when build fails' do - let(:service) { MergeRequests::AddTodoWhenBuildFailsService.new(project, user) } + let(:service) { ::MergeRequests::AddTodoWhenBuildFailsService.new(project, user) } before do - allow(MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).and_return(service) + allow(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).and_return(service) allow(service).to receive(:close) end @@ -3708,7 +3758,7 @@ RSpec.describe Ci::Build do subject.drop! end - it 'creates a todo' do + it 'creates a todo async', :sidekiq_inline do project.add_developer(user) expect_next_instance_of(TodoService) do |todo_service| @@ -3741,6 +3791,7 @@ RSpec.describe Ci::Build do describe '.matches_tag_ids' do let_it_be(:build, reload: true) { create(:ci_build, project: project, user: user) } + let(:tag_ids) { ::ActsAsTaggableOn::Tag.named_any(tag_list).ids } subject { described_class.where(id: build).matches_tag_ids(tag_ids) } @@ -4192,6 +4243,7 @@ RSpec.describe Ci::Build do describe '#artifacts_metadata_entry' do let_it_be(:build) { create(:ci_build, project: project) } + let(:path) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' } around do |example| diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 3d728b9335e..12bc5d9aa3c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers let_it_be(:build) { create(:ci_build, :running) } + let(:chunk_index) { 0 } let(:data_store) { :redis } let(:raw_data) { nil } 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 4e96ec7cecb..acc87c61036 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -86,6 +86,7 @@ RSpec.describe Ci::DailyBuildGroupReportResult do describe 'scopes' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } + let(:recent_build_group_report_result) { create(:ci_daily_build_group_report_result, project: project, group: group) } let(:old_build_group_report_result) do create(:ci_daily_build_group_report_result, date: 1.week.ago, project: project) diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 796947be4c8..cdb123573f1 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -195,6 +195,22 @@ RSpec.describe Ci::JobArtifact do end end + describe '#archived_trace_exists?' do + subject { artifact.archived_trace_exists? } + + context 'when the file exists' do + it { is_expected.to be_truthy } + end + + context 'when the file does not exist' do + before do + artifact.file.remove! + end + + it { is_expected.to be_falsy } + end + end + describe '.for_sha' do let(:first_pipeline) { create(:ci_pipeline) } let(:second_pipeline) { create(:ci_pipeline, project: first_pipeline.project, sha: Digest::SHA1.hexdigest(SecureRandom.hex)) } diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index cec3b544e50..3e5fbbfe823 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -90,6 +90,18 @@ RSpec.describe Ci::PipelineSchedule do end end + describe '.owned_by' do + let(:user) { create(:user) } + let!(:owned_pipeline_schedule) { create(:ci_pipeline_schedule, owner: user) } + let!(:other_pipeline_schedule) { create(:ci_pipeline_schedule) } + + subject { described_class.owned_by(user) } + + it 'returns owned pipeline schedules' do + is_expected.to eq([owned_pipeline_schedule]) + end + end + describe '#set_next_run_at' do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) } let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_from, Time.zone.now) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d57a39d133f..b7f5811e945 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -40,6 +40,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to respond_to :git_author_name } it { is_expected.to respond_to :git_author_email } + it { is_expected.to respond_to :git_author_full_text } it { is_expected.to respond_to :short_sha } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } @@ -426,6 +427,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject { pipeline.legacy_detached_merge_request_pipeline? } let_it_be(:merge_request) { create(:merge_request) } + let(:ref) { 'feature' } let(:target_sha) { nil } @@ -819,6 +821,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do CI_COMMIT_DESCRIPTION CI_COMMIT_REF_PROTECTED CI_COMMIT_TIMESTAMP + CI_COMMIT_AUTHOR CI_BUILD_REF CI_BUILD_BEFORE_SHA CI_BUILD_REF_NAME @@ -830,6 +833,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep 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, :simple, source_project: project, @@ -1274,6 +1278,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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) } @@ -2277,6 +2282,35 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ) end end + + context 'when method is scoped' do + let!(:commit_123_ref_master_parent_pipeline) do + create( + :ci_pipeline, + sha: '123', + ref: 'master', + project: project + ) + end + + let!(:commit_123_ref_master_child_pipeline) do + create( + :ci_pipeline, + sha: '123', + ref: 'master', + project: project, + child_of: commit_123_ref_master_parent_pipeline + ) + end + + it 'returns the latest pipeline after applying the scope' do + result = described_class.ci_sources.latest_pipeline_per_commit(%w[123], 'master') + + expect(result).to match( + '123' => commit_123_ref_master_parent_pipeline + ) + end + end end describe '.latest_successful_ids_per_project' do @@ -2325,6 +2359,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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 @@ -2633,6 +2668,37 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do expect(latest_status).to eq %w(canceled canceled) end end + + context 'preloading relations' do + let(:pipeline1) { create(:ci_empty_pipeline, :created) } + let(:pipeline2) { create(:ci_empty_pipeline, :created) } + + before do + create(:ci_build, :pending, pipeline: pipeline1) + create(:generic_commit_status, :pending, pipeline: pipeline1) + + create(:ci_build, :pending, pipeline: pipeline2) + create(:ci_build, :pending, pipeline: pipeline2) + create(:generic_commit_status, :pending, pipeline: pipeline2) + create(:generic_commit_status, :pending, pipeline: pipeline2) + create(:generic_commit_status, :pending, pipeline: pipeline2) + end + + it 'preloads relations for each build to avoid N+1 queries' do + control1 = ActiveRecord::QueryRecorder.new do + pipeline1.cancel_running + end + + control2 = ActiveRecord::QueryRecorder.new do + pipeline2.cancel_running + end + + extra_update_queries = 3 # transition ... => :canceled + extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types + + expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries) + end + end end describe '#retry_failed' do @@ -2688,6 +2754,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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) } @@ -3353,6 +3420,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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) } @@ -3780,6 +3848,26 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#uses_needs?' do + let_it_be(:pipeline) { create(:ci_pipeline) } + + context 'when the scheduling type is `dag`' do + it 'returns true' do + create(:ci_build, pipeline: pipeline, scheduling_type: :dag) + + expect(pipeline.uses_needs?).to eq(true) + end + end + + context 'when the scheduling type is nil or stage' do + it 'returns false' do + create(:ci_build, pipeline: pipeline, scheduling_type: :stage) + + expect(pipeline.uses_needs?).to eq(false) + end + end + end + describe '#total_size' do let(:pipeline) { create(:ci_pipeline) } let!(:build_job1) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } @@ -3814,6 +3902,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do pipeline.drop end end + + context 'with failure_reason' do + let(:pipeline) { create(:ci_pipeline, :running) } + let(:failure_reason) { 'config_error' } + let(:counter) { Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') } + + it 'increments the counter with the failure_reason' do + expect { pipeline.drop!(failure_reason) }.to change { counter.get(reason: failure_reason) }.by(1) + end + end end end @@ -3843,6 +3941,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '#find_stage_by_name' do let_it_be(:pipeline) { create(:ci_pipeline) } + let(:stage_name) { 'test' } let(:stage) do @@ -4128,6 +4227,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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 @@ -4164,6 +4264,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when pipeline is a child of a child pipeline' do let_it_be(:pipeline) { create(:ci_pipeline, :created) } + let(:ancestor) { create(:ci_pipeline) } let(:parent) { create(:ci_pipeline) } @@ -4179,6 +4280,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do 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 diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ff3551d2a18..ffe0b0d0b19 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -353,6 +353,7 @@ RSpec.describe Ci::Runner do using RSpec::Parameterized::TableSyntax let_it_be(:pipeline) { create(:ci_pipeline) } + let(:build) { create(:ci_build, pipeline: pipeline) } let(:runner_project) { build.project } let(:runner) { create(:ci_runner, :project, projects: [runner_project], tag_list: tag_list, run_untagged: run_untagged) } diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 0afc491dc73..e46d9189c86 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Ci::Stage, :models do let_it_be(:pipeline) { create(:ci_empty_pipeline) } + let(:stage) { create(:ci_stage_entity, pipeline: pipeline, project: pipeline.project) } it_behaves_like 'having unique enum values' @@ -27,6 +28,18 @@ RSpec.describe Ci::Stage, :models do end end + describe '.by_name' do + it 'finds stages by name' do + a = create(:ci_stage_entity, name: 'a') + b = create(:ci_stage_entity, name: 'b') + c = create(:ci_stage_entity, name: 'c') + + expect(described_class.by_name('a')).to contain_exactly(a) + expect(described_class.by_name('b')).to contain_exactly(b) + expect(described_class.by_name(%w[a c])).to contain_exactly(a, c) + end + end + describe '#status' do context 'when stage is pending' do let(:stage) { create(:ci_stage_entity, status: 'pending') } diff --git a/spec/models/ci/test_case_failure_spec.rb b/spec/models/ci/test_case_failure_spec.rb deleted file mode 100644 index 34f89b663ed..00000000000 --- a/spec/models/ci/test_case_failure_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::TestCaseFailure do - describe 'relationships' do - it { is_expected.to belong_to(:build) } - it { is_expected.to belong_to(:test_case) } - end - - describe 'validations' do - subject { build(:ci_test_case_failure) } - - it { is_expected.to validate_presence_of(:test_case) } - it { is_expected.to validate_presence_of(:build) } - it { is_expected.to validate_presence_of(:failed_at) } - end - - describe '.recent_failures_count' do - let_it_be(:project) { create(:project) } - - subject(:recent_failures) do - described_class.recent_failures_count( - project: project, - test_case_keys: test_case_keys - ) - end - - context 'when test case failures are within the date range and are for the test case keys' do - let(:tc_1) { create(:ci_test_case, project: project) } - let(:tc_2) { create(:ci_test_case, project: project) } - let(:test_case_keys) { [tc_1.key_hash, tc_2.key_hash] } - - before do - create_list(:ci_test_case_failure, 3, test_case: tc_1, failed_at: 1.day.ago) - create_list(:ci_test_case_failure, 2, test_case: tc_2, failed_at: 3.days.ago) - end - - it 'returns the number of failures for each test case key hash for the past 14 days by default' do - expect(recent_failures).to eq( - tc_1.key_hash => 3, - tc_2.key_hash => 2 - ) - end - end - - context 'when test case failures are within the date range but are not for the test case keys' do - let(:tc) { create(:ci_test_case, project: project) } - let(:test_case_keys) { ['some-other-key-hash'] } - - before do - create(:ci_test_case_failure, test_case: tc, failed_at: 1.day.ago) - end - - it 'excludes them from the count' do - expect(recent_failures[tc.key_hash]).to be_nil - end - end - - context 'when test case failures are not within the date range but are for the test case keys' do - let(:tc) { create(:ci_test_case, project: project) } - let(:test_case_keys) { [tc.key_hash] } - - before do - create(:ci_test_case_failure, test_case: tc, failed_at: 15.days.ago) - end - - it 'excludes them from the count' do - expect(recent_failures[tc.key_hash]).to be_nil - end - end - end -end diff --git a/spec/models/ci/test_case_spec.rb b/spec/models/ci/test_case_spec.rb deleted file mode 100644 index 45311e285a6..00000000000 --- a/spec/models/ci/test_case_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::TestCase do - describe 'relationships' do - it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:test_case_failures) } - end - - describe 'validations' do - subject { build(:ci_test_case) } - - it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:key_hash) } - end - - describe '.find_or_create_by_batch' do - it 'finds or creates records for the given test case keys', :aggregate_failures do - project = create(:project) - existing_tc = create(:ci_test_case, project: project) - new_key = Digest::SHA256.hexdigest(SecureRandom.hex) - keys = [existing_tc.key_hash, new_key] - - result = described_class.find_or_create_by_batch(project, keys) - - expect(result.map(&:key_hash)).to match_array([existing_tc.key_hash, new_key]) - expect(result).to all(be_persisted) - end - end -end diff --git a/spec/models/ci/unit_test_failure_spec.rb b/spec/models/ci/unit_test_failure_spec.rb new file mode 100644 index 00000000000..f9b8c66b603 --- /dev/null +++ b/spec/models/ci/unit_test_failure_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::UnitTestFailure do + describe 'relationships' do + it { is_expected.to belong_to(:build) } + it { is_expected.to belong_to(:unit_test) } + end + + describe 'validations' do + subject { build(:ci_unit_test_failure) } + + it { is_expected.to validate_presence_of(:unit_test) } + it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:failed_at) } + end + + describe '.recent_failures_count' do + let_it_be(:project) { create(:project) } + + subject(:recent_failures) do + described_class.recent_failures_count( + project: project, + unit_test_keys: unit_test_keys + ) + end + + context 'when unit test failures are within the date range and are for the unit test keys' do + let(:test_1) { create(:ci_unit_test, project: project) } + let(:test_2) { create(:ci_unit_test, project: project) } + let(:unit_test_keys) { [test_1.key_hash, test_2.key_hash] } + + before do + create_list(:ci_unit_test_failure, 3, unit_test: test_1, failed_at: 1.day.ago) + create_list(:ci_unit_test_failure, 2, unit_test: test_2, failed_at: 3.days.ago) + end + + it 'returns the number of failures for each unit test key hash for the past 14 days by default' do + expect(recent_failures).to eq( + test_1.key_hash => 3, + test_2.key_hash => 2 + ) + end + end + + context 'when unit test failures are within the date range but are not for the unit test keys' do + let(:test) { create(:ci_unit_test, project: project) } + let(:unit_test_keys) { ['some-other-key-hash'] } + + before do + create(:ci_unit_test_failure, unit_test: test, failed_at: 1.day.ago) + end + + it 'excludes them from the count' do + expect(recent_failures[test.key_hash]).to be_nil + end + end + + context 'when unit test failures are not within the date range but are for the unit test keys' do + let(:test) { create(:ci_unit_test, project: project) } + let(:unit_test_keys) { [test.key_hash] } + + before do + create(:ci_unit_test_failure, unit_test: test, failed_at: 15.days.ago) + end + + it 'excludes them from the count' do + expect(recent_failures[test.key_hash]).to be_nil + end + end + end +end diff --git a/spec/models/ci/unit_test_spec.rb b/spec/models/ci/unit_test_spec.rb new file mode 100644 index 00000000000..2207a362be3 --- /dev/null +++ b/spec/models/ci/unit_test_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::UnitTest do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:unit_test_failures) } + end + + describe 'validations' do + subject { build(:ci_unit_test) } + + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:key_hash) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:suite_name) } + end + + describe '.find_or_create_by_batch' do + let(:project) { create(:project) } + + it 'finds or creates records for the given unit test keys', :aggregate_failures do + existing_test = create(:ci_unit_test, project: project, suite_name: 'rspec', name: 'Math#sum adds numbers') + new_key = Digest::SHA256.hexdigest(SecureRandom.hex) + attrs = [ + { + key_hash: existing_test.key_hash, + name: 'This new name will not apply', + suite_name: 'This new suite name will not apply' + }, + { + key_hash: new_key, + name: 'Component works', + suite_name: 'jest' + } + ] + + result = described_class.find_or_create_by_batch(project, attrs) + + expect(result).to match_array([ + have_attributes( + key_hash: existing_test.key_hash, + suite_name: 'rspec', + name: 'Math#sum adds numbers' + ), + have_attributes( + key_hash: new_key, + suite_name: 'jest', + name: 'Component works' + ) + ]) + + expect(result).to all(be_persisted) + end + + context 'when a given name or suite_name exceeds the string size limit' do + before do + stub_const("#{described_class}::MAX_NAME_SIZE", 6) + stub_const("#{described_class}::MAX_SUITE_NAME_SIZE", 6) + end + + it 'truncates the values before storing the information' do + new_key = Digest::SHA256.hexdigest(SecureRandom.hex) + attrs = [ + { + key_hash: new_key, + name: 'abcdefg', + suite_name: 'abcdefg' + } + ] + + result = described_class.find_or_create_by_batch(project, attrs) + + expect(result).to match_array([ + have_attributes( + key_hash: new_key, + suite_name: 'abc...', + name: 'abc...' + ) + ]) + + expect(result).to all(be_persisted) + end + end + end +end diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index a1b45df1970..680b351d24a 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -24,4 +24,53 @@ RSpec.describe Clusters::AgentToken do expect(agent_token.token.length).to be >= 50 end end + + describe '#track_usage', :clean_gitlab_redis_cache do + let(:agent_token) { create(:cluster_agent_token) } + + subject { agent_token.track_usage } + + context 'when last_used_at was updated recently' do + before do + agent_token.update!(last_used_at: 10.minutes.ago) + end + + it 'updates cache but not database' do + expect { subject }.not_to change { agent_token.reload.read_attribute(:last_used_at) } + + expect_redis_update + end + end + + context 'when last_used_at was not updated recently' do + it 'updates cache and database' do + does_db_update + expect_redis_update + end + + context 'with invalid token' do + before do + agent_token.description = SecureRandom.hex(2000) + end + + it 'still updates caches and database' do + expect(agent_token).to be_invalid + + does_db_update + expect_redis_update + end + end + end + + def expect_redis_update + Gitlab::Redis::Cache.with do |redis| + redis_key = "cache:#{described_class.name}:#{agent_token.id}:attributes" + expect(redis.get(redis_key)).to be_present + end + end + + def does_db_update + expect { subject }.to change { agent_token.reload.read_attribute(:last_used_at) } + end + end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 032de6aa7c2..5a0ccabd467 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -39,6 +39,19 @@ RSpec.describe Clusters::Applications::Prometheus do end end + describe 'transition to externally_installed' do + let(:project) { create(:project) } + let(:cluster) { create(:cluster, :with_installed_helm) } + let(:application) { create(:clusters_applications_prometheus, :installing, cluster: cluster) } + + it 'schedules post installation job' do + expect(Clusters::Applications::ActivateServiceWorker) + .to receive(:perform_async).with(cluster.id, 'prometheus') + + application.make_externally_installed! + end + end + describe 'transition to updating' do let(:project) { create(:project) } let(:cluster) { create(:cluster, projects: [project]) } @@ -61,85 +74,8 @@ RSpec.describe Clusters::Applications::Prometheus do end describe '#prometheus_client' do - shared_examples 'exception caught for prometheus client' do - before do - allow(kube_client).to receive(:proxy_url).and_raise(exception) - end - - it 'returns nil' do - expect(subject.prometheus_client).to be_nil - end - end - - context 'cluster is nil' do - it 'returns nil' do - expect(subject.cluster).to be_nil - expect(subject.prometheus_client).to be_nil - end - end - - context "cluster doesn't have kubeclient" do - let(:cluster) { create(:cluster) } - - subject { create(:clusters_applications_prometheus, cluster: cluster) } - - it 'returns nil' do - expect(subject.prometheus_client).to be_nil - end - end - - context 'cluster has kubeclient' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:kubernetes_url) { subject.cluster.platform_kubernetes.api_url } - let(:kube_client) { subject.cluster.kubeclient.core_client } - - subject { create(:clusters_applications_prometheus, cluster: cluster) } - - before do - subject.cluster.platform_kubernetes.namespace = 'a-namespace' - stub_kubeclient_discover(cluster.platform_kubernetes.api_url) - - create(:cluster_kubernetes_namespace, - cluster: cluster, - cluster_project: cluster.cluster_project, - project: cluster.cluster_project.project) - end - - it 'creates proxy prometheus_client' do - expect(subject.prometheus_client).to be_instance_of(Gitlab::PrometheusClient) - end - - it 'merges proxy_url, options and headers from kube client with prometheus_client options' do - expect(Gitlab::PrometheusClient) - .to(receive(:new)) - .with(a_valid_url, kube_client.rest_client.options.merge({ - headers: kube_client.headers, - timeout: PrometheusAdapter::DEFAULT_PROMETHEUS_REQUEST_TIMEOUT_SEC - })) - subject.prometheus_client - end - - context 'when cluster is not reachable' do - it_behaves_like 'exception caught for prometheus client' do - let(:exception) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) } - end - end - - context 'when there is a socket error while contacting cluster' do - it_behaves_like 'exception caught for prometheus client' do - let(:exception) { Errno::ECONNREFUSED } - end - - it_behaves_like 'exception caught for prometheus client' do - let(:exception) { Errno::ECONNRESET } - end - end - - context 'when the network is unreachable' do - it_behaves_like 'exception caught for prometheus client' do - let(:exception) { Errno::ENETUNREACH } - end - end + include_examples '#prometheus_client shared' do + let(:factory) { :clusters_applications_prometheus } end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index a8f81cba285..b2ed64fd9b0 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to have_one(:provider_gcp) } it { is_expected.to have_one(:provider_aws) } it { is_expected.to have_one(:platform_kubernetes) } + it { is_expected.to have_one(:integration_prometheus) } it { is_expected.to have_one(:application_helm) } it { is_expected.to have_one(:application_ingress) } it { is_expected.to have_one(:application_prometheus) } @@ -40,7 +41,6 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix } - it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_knative).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_elastic_stack).with_prefix } it { is_expected.to delegate_method(:external_ip).to(:application_ingress).with_prefix } @@ -1349,6 +1349,80 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end + describe '#application_prometheus_available?' do + let_it_be_with_reload(:cluster) { create(:cluster, :project) } + + subject { cluster.application_prometheus_available? } + + it { is_expected.to be_falsey } + + context 'has a integration_prometheus' do + let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } + + it { is_expected.to be_truthy } + + context 'disabled' do + before do + cluster.integration_prometheus.enabled = false + end + + it { is_expected.to be_falsey } + end + end + + context 'has a application_prometheus' do + let_it_be(:application) { create(:clusters_applications_prometheus, :installed, :no_helm_installed, cluster: cluster) } + + it { is_expected.to be_truthy } + + context 'errored' do + before do + cluster.application_prometheus.status = Clusters::Applications::Prometheus.state_machines[:status].states[:errored] + end + + it { is_expected.to be_falsey } + end + + context 'also has a integration_prometheus' do + let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } + + it { is_expected.to be_truthy } + end + end + end + + describe '#prometheus_adapter' do + let_it_be_with_reload(:cluster) { create(:cluster, :project) } + + it 'returns nothing' do + expect(cluster.prometheus_adapter).to be_nil + end + + context 'has a integration_prometheus' do + let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } + + it 'returns the integration' do + expect(cluster.prometheus_adapter).to eq(integration) + end + end + + context 'has a application_prometheus' do + let_it_be(:application) { create(:clusters_applications_prometheus, :no_helm_installed, cluster: cluster) } + + it 'returns the application' do + expect(cluster.prometheus_adapter).to eq(application) + end + + context 'also has a integration_prometheus' do + let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) } + + it 'returns the integration' do + expect(cluster.prometheus_adapter).to eq(integration) + end + end + end + end + describe '#delete_cached_resources!' do let!(:cluster) { create(:cluster, :project) } let!(:staging_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, namespace: 'staging') } diff --git a/spec/models/clusters/integrations/prometheus_spec.rb b/spec/models/clusters/integrations/prometheus_spec.rb new file mode 100644 index 00000000000..a7be1673ce2 --- /dev/null +++ b/spec/models/clusters/integrations/prometheus_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Integrations::Prometheus do + include KubernetesHelpers + include StubRequests + + describe 'associations' do + it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:cluster) } + it { is_expected.not_to allow_value(nil).for(:enabled) } + end + + describe '#prometheus_client' do + include_examples '#prometheus_client shared' do + let(:factory) { :clusters_integrations_prometheus } + end + end + + describe '#configured?' do + let(:prometheus) { create(:clusters_integrations_prometheus, cluster: cluster) } + + subject { prometheus.configured? } + + context 'when a kubenetes client is present' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + + it { is_expected.to be_truthy } + + context 'when it is disabled' do + let(:prometheus) { create(:clusters_integrations_prometheus, :disabled, cluster: cluster) } + + it { is_expected.to be_falsey } + end + + context 'when the kubernetes URL is blocked' do + before do + blocked_ip = '127.0.0.1' # localhost addresses are blocked by default + + stub_all_dns(cluster.platform.api_url, ip_address: blocked_ip) + end + + it { is_expected.to be_falsey } + end + end + + context 'when a kubenetes client is not present' do + let(:cluster) { create(:cluster) } + + it { is_expected.to be_falsy } + end + end +end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index a5f02b61132..7c00f367844 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -526,7 +526,7 @@ eos context 'that is found' do before do # Artificially mark as completed. - merge_request.update(merge_commit_sha: merge_commit.id) + merge_request.update!(merge_commit_sha: merge_commit.id) end it do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 01da379e001..e64dee2d26f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -213,12 +213,12 @@ RSpec.describe CommitStatus do context 'when it is canceled' do before do - commit_status.update(status: 'canceled') + commit_status.update!(status: 'canceled') end context 'when there is auto_canceled_by' do before do - commit_status.update(auto_canceled_by: create(:ci_empty_pipeline)) + commit_status.update!(auto_canceled_by: create(:ci_empty_pipeline)) end it 'is auto canceled' do @@ -510,10 +510,6 @@ RSpec.describe CommitStatus do end describe '#group_name' do - before do - stub_feature_flags(simplified_commit_status_group_name: false) - end - using RSpec::Parameterized::TableSyntax let(:commit_status) do @@ -528,18 +524,24 @@ RSpec.describe CommitStatus do 'rspec1 0/2' | 'rspec1' 'rspec:windows' | 'rspec:windows' 'rspec:windows 0' | 'rspec:windows 0' + 'rspec:windows 0 2/2' | 'rspec:windows 0' 'rspec:windows 0 test' | 'rspec:windows 0 test' - 'rspec:windows 0 1' | 'rspec:windows' - 'rspec:windows 0 1 name' | 'rspec:windows name' + 'rspec:windows 0 test 2/2' | 'rspec:windows 0 test' + 'rspec:windows 0 1 2/2' | 'rspec:windows' + 'rspec:windows 0 1 [aws] 2/2' | 'rspec:windows' + 'rspec:windows 0 1 name [aws] 2/2' | 'rspec:windows 0 1 name' + 'rspec:windows 0 1 name' | 'rspec:windows 0 1 name' + 'rspec:windows 0 1 name 1/2' | 'rspec:windows 0 1 name' 'rspec:windows 0/1' | 'rspec:windows' - 'rspec:windows 0/1 name' | 'rspec:windows name' + 'rspec:windows 0/1 name' | 'rspec:windows 0/1 name' + 'rspec:windows 0/1 name 1/2' | 'rspec:windows 0/1 name' 'rspec:windows 0:1' | 'rspec:windows' - 'rspec:windows 0:1 name' | 'rspec:windows name' + 'rspec:windows 0:1 name' | 'rspec:windows 0:1 name' 'rspec:windows 10000 20000' | 'rspec:windows' 'rspec:windows 0 : / 1' | 'rspec:windows' - 'rspec:windows 0 : / 1 name' | 'rspec:windows name' - '0 1 name ruby' | 'name ruby' - '0 :/ 1 name ruby' | 'name ruby' + 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' + '0 1 name ruby' | '0 1 name ruby' + '0 :/ 1 name ruby' | '0 :/ 1 name ruby' 'rspec: [aws]' | 'rspec' 'rspec: [aws] 0/1' | 'rspec' 'rspec: [aws, max memory]' | 'rspec' @@ -561,58 +563,6 @@ RSpec.describe CommitStatus do is_expected.to eq(group_name) end end - - context 'with simplified_commit_status_group_name' do - before do - stub_feature_flags(simplified_commit_status_group_name: true) - end - - where(:name, :group_name) do - 'rspec1' | 'rspec1' - 'rspec1 0 1' | 'rspec1' - 'rspec1 0/2' | 'rspec1' - 'rspec:windows' | 'rspec:windows' - 'rspec:windows 0' | 'rspec:windows 0' - 'rspec:windows 0 2/2' | 'rspec:windows 0' - 'rspec:windows 0 test' | 'rspec:windows 0 test' - 'rspec:windows 0 test 2/2' | 'rspec:windows 0 test' - 'rspec:windows 0 1 2/2' | 'rspec:windows' - 'rspec:windows 0 1 [aws] 2/2' | 'rspec:windows' - 'rspec:windows 0 1 name [aws] 2/2' | 'rspec:windows 0 1 name' - 'rspec:windows 0 1 name' | 'rspec:windows 0 1 name' - 'rspec:windows 0 1 name 1/2' | 'rspec:windows 0 1 name' - 'rspec:windows 0/1' | 'rspec:windows' - 'rspec:windows 0/1 name' | 'rspec:windows 0/1 name' - 'rspec:windows 0/1 name 1/2' | 'rspec:windows 0/1 name' - 'rspec:windows 0:1' | 'rspec:windows' - 'rspec:windows 0:1 name' | 'rspec:windows 0:1 name' - 'rspec:windows 10000 20000' | 'rspec:windows' - 'rspec:windows 0 : / 1' | 'rspec:windows' - 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' - '0 1 name ruby' | '0 1 name ruby' - '0 :/ 1 name ruby' | '0 :/ 1 name ruby' - 'rspec: [aws]' | 'rspec' - 'rspec: [aws] 0/1' | 'rspec' - 'rspec: [aws, max memory]' | 'rspec' - 'rspec:linux: [aws, max memory, data]' | 'rspec:linux' - 'rspec: [inception: [something, other thing], value]' | 'rspec' - 'rspec:windows 0/1: [name, other]' | 'rspec:windows' - 'rspec:windows: [name, other] 0/1' | 'rspec:windows' - 'rspec:windows: [name, 0/1] 0/1' | 'rspec:windows' - 'rspec:windows: [0/1, name]' | 'rspec:windows' - 'rspec:windows: [, ]' | 'rspec:windows' - 'rspec:windows: [name]' | 'rspec:windows' - 'rspec:windows: [name,other]' | 'rspec:windows' - end - - with_them do - it "#{params[:name]} puts in #{params[:group_name]}" do - commit_status.name = name - - is_expected.to eq(group_name) - end - end - end end describe '#detailed_status' do @@ -660,7 +610,7 @@ RSpec.describe CommitStatus do end it "raise exception when trying to update" do - expect { commit_status.save }.to raise_error(ActiveRecord::StaleObjectError) + expect { commit_status.save! }.to raise_error(ActiveRecord::StaleObjectError) end end @@ -679,30 +629,45 @@ RSpec.describe CommitStatus do end end - describe 'set failure_reason when drop' do + describe '#drop' do let(:commit_status) { create(:commit_status, :created) } + let(:counter) { Gitlab::Metrics.counter(:gitlab_ci_job_failure_reasons, 'desc') } + let(:failure_reason) { reason.to_s } subject do commit_status.drop!(reason) commit_status end + shared_examples 'incrementing failure reason counter' do + it 'increments the counter with the failure_reason' do + expect { subject }.to change { counter.get(reason: failure_reason) }.by(1) + end + end + context 'when failure_reason is nil' do let(:reason) { } + let(:failure_reason) { 'unknown_failure' } it { is_expected.to be_unknown_failure } + + it_behaves_like 'incrementing failure reason counter' end context 'when failure_reason is script_failure' do let(:reason) { :script_failure } it { is_expected.to be_script_failure } + + it_behaves_like 'incrementing failure reason counter' end context 'when failure_reason is unmet_prerequisites' do let(:reason) { :unmet_prerequisites } it { is_expected.to be_unmet_prerequisites } + + it_behaves_like 'incrementing failure reason counter' end end @@ -870,4 +835,23 @@ RSpec.describe CommitStatus do it { is_expected.to eq(false) } end end + + describe '#update_older_statuses_retried!' do + let!(:build_old) { create_status(name: 'build') } + let!(:build_new) { create_status(name: 'build') } + let!(:test) { create_status(name: 'test') } + let!(:build_from_other_pipeline) do + new_pipeline = create(:ci_pipeline, project: project, sha: project.commit.id) + create_status(name: 'build', pipeline: new_pipeline) + end + + it "updates 'retried' and 'status' columns of the latest status with the same name in the same pipeline" do + build_new.update_older_statuses_retried! + + expect(build_new.reload).to have_attributes(retried: false, processed: false) + expect(build_old.reload).to have_attributes(retried: true, processed: true) + expect(test.reload).to have_attributes(retried: false, processed: false) + expect(build_from_other_pipeline.reload).to have_attributes(retried: false, processed: false) + end + end end diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index a8fcb714c64..993afd47a57 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -26,6 +26,7 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:project) { create(:project) } let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) } + let!(:todos) { create(:todo, project: project) } it 'destroys multiple builds' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 6e62d4ef31b..33a4c8eac41 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -17,9 +17,13 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do include CacheMarkdownField def initialize(args = {}) - @title, @description, @cached_markdown_version = args[:title], args[:description], args[:cached_markdown_version] - @title_html, @description_html = args[:title_html], args[:description_html] - @author, @project = args[:author], args[:project] + @title = args[:title] + @description = args[:description] + @cached_markdown_version = args[:cached_markdown_version] + @title_html = args[:title_html] + @description_html = args[:description_html] + @author = args[:author] + @project = args[:project] @parent_user = args[:parent_user] end diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index f2877bed9cf..dc80e30216a 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -205,7 +205,7 @@ RSpec.describe CacheableAttributes do end end - it 'uses RequestStore in addition to process memory cache', :request_store do + it 'uses RequestStore in addition to process memory cache', :request_store, :do_not_mock_admin_mode_setting do # Warm up the cache create(:application_setting).cache! diff --git a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb new file mode 100644 index 00000000000..ddff9ce32b4 --- /dev/null +++ b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb @@ -0,0 +1,320 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do + let(:group) { create(:group) } + let(:subgroup) { create(:group, parent: group) } + + def group_settings + group.namespace_settings + end + + def subgroup_settings + subgroup.namespace_settings + end + + describe '#delayed_project_removal' do + subject(:delayed_project_removal) { subgroup_settings.delayed_project_removal } + + context 'when the feature is disabled' do + before do + stub_feature_flags(cascading_namespace_settings: false) + + group_settings.update!(delayed_project_removal: true) + end + + it 'does not cascade' do + expect(delayed_project_removal).to eq(nil) + end + end + + context 'when there is no parent' do + context 'and the value is not nil' do + before do + group_settings.update!(delayed_project_removal: true) + end + + it 'returns the local value' do + expect(group_settings.delayed_project_removal).to eq(true) + end + end + + context 'and the value is nil' do + before do + group_settings.update!(delayed_project_removal: nil) + stub_application_setting(delayed_project_removal: false) + end + + it 'returns the application settings value' do + expect(group_settings.delayed_project_removal).to eq(false) + end + end + end + + context 'when parent does not lock the attribute' do + context 'and value is not nil' do + before do + group_settings.update!(delayed_project_removal: false) + end + + it 'returns local setting when present' do + subgroup_settings.update!(delayed_project_removal: true) + + expect(delayed_project_removal).to eq(true) + end + + it 'returns the parent value when local value is nil' do + subgroup_settings.update!(delayed_project_removal: nil) + + expect(delayed_project_removal).to eq(false) + end + + it 'returns the correct dirty value' do + subgroup_settings.delayed_project_removal = true + + expect(delayed_project_removal).to eq(true) + end + + it 'does not return the application setting value when parent value is false' do + stub_application_setting(delayed_project_removal: true) + + expect(delayed_project_removal).to eq(false) + end + end + + context 'and the value is nil' do + before do + group_settings.update!(delayed_project_removal: nil, lock_delayed_project_removal: false) + subgroup_settings.update!(delayed_project_removal: nil) + + subgroup_settings.clear_memoization(:delayed_project_removal) + end + + it 'cascades to the application settings value' do + expect(delayed_project_removal).to eq(false) + end + end + + context 'when multiple ancestors set a value' do + let(:third_level_subgroup) { create(:group, parent: subgroup) } + + before do + group_settings.update!(delayed_project_removal: true) + subgroup_settings.update!(delayed_project_removal: false) + end + + it 'returns the closest ancestor value' do + expect(third_level_subgroup.namespace_settings.delayed_project_removal).to eq(false) + end + end + end + + context 'when parent locks the attribute' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'returns the parent value' do + expect(delayed_project_removal).to eq(false) + end + + it 'does not allow the local value to be saved' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when the application settings locks the attribute' do + before do + subgroup_settings.update!(delayed_project_removal: true) + stub_application_setting(lock_delayed_project_removal: true, delayed_project_removal: true) + end + + it 'returns the application setting value' do + expect(delayed_project_removal).to eq(true) + end + + it 'does not allow the local value to be saved' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + end + + describe '#delayed_project_removal?' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'aliases the method when the attribute is a boolean' do + expect(subgroup_settings.delayed_project_removal?).to eq(subgroup_settings.delayed_project_removal) + end + end + + describe '#delayed_project_removal_locked?' do + shared_examples 'not locked' do + it 'is not locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false) + end + + it 'is not locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false) + end + + it 'does not return a locked namespace' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil + end + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(cascading_namespace_settings: false) + + group_settings.update!(delayed_project_removal: true) + end + + it_behaves_like 'not locked' + end + + context 'when parent does not lock the attribute' do + it_behaves_like 'not locked' + end + + context 'when parent locks the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'is locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(true) + end + + it 'is not locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false) + end + + it 'returns a locked namespace settings object' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor.namespace_id).to eq(group_settings.namespace_id) + end + end + + context 'when not locked by application settings' do + before do + stub_application_setting(lock_delayed_project_removal: false) + end + + it_behaves_like 'not locked' + end + + context 'when locked by application settings' do + before do + stub_application_setting(lock_delayed_project_removal: true) + end + + it 'is not locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false) + end + + it 'is locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(true) + end + + it 'does not return a locked namespace' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil + end + end + end + + describe '#lock_delayed_project_removal=' do + context 'when parent locks the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'does not allow the attribute to be saved' do + subgroup_settings.lock_delayed_project_removal = true + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when parent does not lock the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: false) + + subgroup_settings.lock_delayed_project_removal = true + end + + it 'allows the lock to be set when the attribute is not nil' do + subgroup_settings.delayed_project_removal = true + + expect(subgroup_settings.save).to eq(true) + end + + it 'does not allow the lock to be saved when the attribute is nil' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be nil when locking the attribute/) + end + end + + context 'when application settings locks the attribute' do + before do + stub_application_setting(lock_delayed_project_removal: true) + end + + it 'does not allow the attribute to be saved' do + subgroup_settings.lock_delayed_project_removal = true + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when application_settings does not lock the attribute' do + before do + stub_application_setting(lock_delayed_project_removal: false) + end + + it 'allows the attribute to be saved' do + subgroup_settings.delayed_project_removal = true + subgroup_settings.lock_delayed_project_removal = true + + expect(subgroup_settings.save).to eq(true) + end + end + end + + describe 'after update callback' do + before do + subgroup_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + end + + it 'clears descendant locks' do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: true) + + expect(subgroup_settings.reload.lock_delayed_project_removal).to eq(false) + end + end +end diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index ebc838e86a6..62fc689a9ca 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -72,5 +72,33 @@ RSpec.describe Ci::Artifactable do expect(Ci::JobArtifact.expired(1).order_id_asc).to eq([recently_expired_artifact]) end end + + describe '.with_files_stored_locally' do + it 'returns artifacts stored locally' do + expect(Ci::JobArtifact.with_files_stored_locally).to contain_exactly(recently_expired_artifact, later_expired_artifact, not_expired_artifact) + end + end + + describe '.with_files_stored_remotely' do + let(:remote_artifact) { create(:ci_job_artifact, :remote_store) } + + before do + stub_artifacts_object_storage + end + + it 'returns artifacts stored remotely' do + expect(Ci::JobArtifact.with_files_stored_remotely).to contain_exactly(remote_artifact) + end + end + + describe '.project_id_in' do + context 'when artifacts belongs to projects' do + let(:project_ids) { [recently_expired_artifact.project.id, not_expired_artifact.project.id, non_existing_record_id] } + + it 'returns artifacts belonging to projects' do + expect(Ci::JobArtifact.project_id_in(project_ids)).to contain_exactly(recently_expired_artifact, not_expired_artifact) + end + end + end end end diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index b550d22f686..295f3523dd5 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Featurable do let_it_be(:user) { create(:user) } + let(:project) { create(:project) } let(:feature_class) { subject.class } let(:features) { feature_class::FEATURES } @@ -163,7 +164,7 @@ RSpec.describe Featurable do end def update_all_project_features(project, features, value) - project_feature_attributes = features.map { |f| ["#{f}_access_level", value] }.to_h + project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] } project.project_feature.update!(project_feature_attributes) end end diff --git a/spec/models/concerns/has_timelogs_report_spec.rb b/spec/models/concerns/has_timelogs_report_spec.rb new file mode 100644 index 00000000000..f694fc350ee --- /dev/null +++ b/spec/models/concerns/has_timelogs_report_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe HasTimelogsReport do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:issue) { create(:issue, project: create(:project, :public, group: group)) } + + describe '#timelogs' do + let!(:timelog1) { create_timelog(15.days.ago) } + let!(:timelog2) { create_timelog(10.days.ago) } + let!(:timelog3) { create_timelog(5.days.ago) } + let(:start_time) { 20.days.ago } + let(:end_time) { 8.days.ago } + + before do + group.add_developer(user) + end + + it 'returns collection of timelogs between given times' do + expect(group.timelogs(start_time, end_time).to_a).to match_array([timelog1, timelog2]) + end + + it 'returns empty collection if times are not present' do + expect(group.timelogs(nil, nil)).to be_empty + end + + it 'returns empty collection if time range is invalid' do + expect(group.timelogs(end_time, start_time)).to be_empty + end + end + + describe '#user_can_access_group_timelogs?' do + it 'returns true if user can access group timelogs' do + group.add_developer(user) + + expect(group).to be_user_can_access_group_timelogs(user) + end + + it 'returns false if user has insufficient permissions' do + group.add_guest(user) + + expect(group).not_to be_user_can_access_group_timelogs(user) + end + end + + def create_timelog(time) + create(:timelog, issue: issue, user: user, spent_at: time) + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 3545c8e9686..14db9b530db 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Issuable do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author) } it { is_expected.to have_many(:notes).dependent(:destroy) } - it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:todos) } it { is_expected.to have_many(:labels) } it { is_expected.to have_many(:note_authors).through(:notes) } @@ -65,6 +65,23 @@ RSpec.describe Issuable do it { expect(issuable_class).to respond_to(:opened) } it { expect(issuable_class).to respond_to(:closed) } it { expect(issuable_class).to respond_to(:assigned) } + + describe '.includes_for_bulk_update' do + before do + stub_const('Example', Class.new(ActiveRecord::Base)) + + Example.class_eval do + include Issuable # adds :labels and :metrics, among others + + belongs_to :author + has_many :assignees + end + end + + it 'includes available associations' do + expect(Example.includes_for_bulk_update.includes_values).to eq([:author, :assignees, :labels, :metrics]) + end + end end describe 'author_name' do @@ -380,7 +397,7 @@ RSpec.describe Issuable do context 'user is a participant in the issue' do before do - allow(issue).to receive(:participants).with(user).and_return([user]) + allow(issue).to receive(:participant?).with(user).and_return(true) end it 'returns false when no subcription exists' do diff --git a/spec/models/concerns/milestoneable_spec.rb b/spec/models/concerns/milestoneable_spec.rb index 5fb3b39f734..961eac4710d 100644 --- a/spec/models/concerns/milestoneable_spec.rb +++ b/spec/models/concerns/milestoneable_spec.rb @@ -50,13 +50,13 @@ RSpec.describe Milestoneable do it 'returns true with a milestone from the issue project' do milestone = create(:milestone, project: project) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) end it 'returns true with a milestone from the issue project group' do milestone = create(:milestone, group: group) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) end it 'returns true with a milestone from the the parent of the issue project group' do @@ -64,19 +64,23 @@ RSpec.describe Milestoneable do group.update!(parent: parent) milestone = create(:milestone, group: parent) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) + end + + it 'returns true with a blank milestone' do + expect(build_milestoneable('').milestone_available?).to be(true) end it 'returns false with a milestone from another project' do milestone = create(:milestone) - expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey + expect(build_milestoneable(milestone.id).milestone_available?).to be(false) end it 'returns false with a milestone from another group' do milestone = create(:milestone, group: create(:group)) - expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey + expect(build_milestoneable(milestone.id).milestone_available?).to be(false) end end end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 3b8fc465421..46a876f34e9 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -2,30 +2,28 @@ require 'spec_helper' -RSpec.describe Milestone, 'Milestoneish' do - let(:author) { create(:user) } - let(:assignee) { create(:user) } - let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:guest) { create(:user) } - let(:admin) { create(:admin) } - let(:project) { create(:project, :public) } - let(:milestone) { create(:milestone, project: project) } - let(:label1) { create(:label, project: project) } - let(:label2) { create(:label, project: project) } - let!(:issue) { create(:issue, project: project, milestone: milestone, assignees: [member], labels: [label1]) } - let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone, labels: [label2]) } - let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) } - let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) } - let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) } - let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } - let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } - let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } - let(:label_1) { create(:label, title: 'label_1', project: project, priority: 1) } - let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) } - let(:label_3) { create(:label, title: 'label_3', project: project) } +RSpec.describe Milestone, 'Milestoneish', factory_default: :keep do + let_it_be(:author) { create(:user) } + let_it_be(:assignee) { create(:user) } + let_it_be(:non_member) { create(:user) } + let_it_be(:member) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:project, reload: true) { create_default(:project, :public, :empty_repo).freeze } + let_it_be(:milestone, refind: true) { create_default(:milestone, project: project) } + let_it_be(:label1) { create(:label) } + let_it_be(:label2) { create(:label) } + let_it_be(:issue, reload: true) { create(:issue, milestone: milestone, assignees: [member], labels: [label1]) } + let_it_be(:security_issue_1, reload: true) { create(:issue, :confidential, author: author, milestone: milestone, labels: [label2]) } + let_it_be(:security_issue_2, reload: true) { create(:issue, :confidential, assignees: [assignee], milestone: milestone) } + let_it_be(:closed_issue_1, reload: true) { create(:issue, :closed, milestone: milestone) } + let_it_be(:closed_issue_2, reload: true) { create(:issue, :closed, milestone: milestone) } + let_it_be(:closed_security_issue_1, reload: true) { create(:issue, :confidential, :closed, author: author, milestone: milestone) } + let_it_be(:closed_security_issue_2, reload: true) { create(:issue, :confidential, :closed, assignees: [assignee], milestone: milestone) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } + let_it_be(:label_1) { create(:label, title: 'label_1', priority: 1) } + let_it_be(:label_2) { create(:label, title: 'label_2', priority: 2) } + let_it_be(:label_3) { create(:label, title: 'label_3') } before do project.add_developer(member) @@ -63,7 +61,7 @@ RSpec.describe Milestone, 'Milestoneish' do end end - context 'attributes visibility' do + context 'with attributes visibility' do using RSpec::Parameterized::TableSyntax let(:users) do @@ -167,8 +165,6 @@ RSpec.describe Milestone, 'Milestoneish' do end describe '#merge_requests_visible_to_user' do - let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } - context 'when project is private' do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -211,10 +207,11 @@ RSpec.describe Milestone, 'Milestoneish' do end context 'when milestone is at parent level group' do - let(:parent_group) { create(:group) } - let(:group) { create(:group, parent: parent_group) } - let(:project) { create(:project, namespace: group) } - let(:milestone) { create(:milestone, group: parent_group) } + let_it_be(:parent_group) { create(:group) } + let_it_be(:group) { create(:group, parent: parent_group) } + let_it_be(:project) { create(:project, :empty_repo, namespace: group) } + let_it_be(:milestone) { create(:milestone, group: parent_group) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } it 'does not return any merge request for a non member' do merge_requests = milestone.merge_requests_visible_to_user(non_member) @@ -243,7 +240,7 @@ RSpec.describe Milestone, 'Milestoneish' do end describe '#percent_complete', :use_clean_rails_memory_store_caching do - context 'division by zero' do + context 'with division by zero' do let(:new_milestone) { build_stubbed(:milestone) } it { expect(new_milestone.percent_complete).to eq(0) } @@ -252,13 +249,19 @@ RSpec.describe Milestone, 'Milestoneish' do describe '#closed_issues_count' do it 'counts all closed issues including confidential' do - expect(milestone.closed_issues_count).to eq 6 + expect(milestone.closed_issues_count).to eq 4 end end describe '#total_issues_count' do it 'counts all issues including confidential' do - expect(milestone.total_issues_count).to eq 9 + expect(milestone.total_issues_count).to eq 7 + end + end + + describe '#total_merge_requests_count' do + it 'counts merge requests' do + expect(milestone.total_merge_requests_count).to eq 1 end end diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb index 3376e337dc9..903c7ae16b6 100644 --- a/spec/models/concerns/participable_spec.rb +++ b/spec/models/concerns/participable_spec.rb @@ -39,11 +39,12 @@ RSpec.describe Participable do expect(participants).to include(user3) end - it 'caches the raw list of participants' do + it 'caches the list of filtered participants' do instance = model.new user1 = build(:user) - expect(instance).to receive(:raw_participants).once + expect(instance).to receive(:all_participants_hash).once.and_return({}) + expect(instance).to receive(:filter_by_ability).once instance.participants(user1) instance.participants(user1) @@ -91,5 +92,71 @@ RSpec.describe Participable do expect(ext_arg).to be_an_instance_of(Gitlab::ReferenceExtractor) end end + + context 'participable is a personal snippet' do + let(:model) { PersonalSnippet } + let(:instance) { model.new(author: user1) } + + let(:user1) { build(:user) } + let(:user2) { build(:user) } + let(:user3) { build(:user) } + + before do + allow(model).to receive(:participant_attrs).and_return([:foo, :bar]) + end + + it 'returns the list of participants' do + expect(instance).to receive(:foo).and_return(user1) + expect(instance).to receive(:bar).and_return(user2) + + participants = instance.participants(user1) + expect(participants).to contain_exactly(user1) + end + end + end + + describe '#participant?' do + let(:instance) { model.new } + + let(:user1) { build(:user) } + let(:user2) { build(:user) } + let(:user3) { build(:user) } + let(:project) { build(:project, :public) } + + before do + allow(model).to receive(:participant_attrs).and_return([:foo, :bar]) + end + + it 'returns whether the user is a participant' do + allow(instance).to receive(:foo).and_return(user2) + allow(instance).to receive(:bar).and_return(user3) + allow(instance).to receive(:project).and_return(project) + + expect(instance.participant?(user1)).to be false + expect(instance.participant?(user2)).to be true + expect(instance.participant?(user3)).to be true + end + + it 'caches the list of raw participants' do + expect(instance).to receive(:raw_participants).once.and_return([]) + expect(instance).to receive(:project).twice.and_return(project) + + instance.participant?(user1) + instance.participant?(user1) + end + + context 'participable is a personal snippet' do + let(:model) { PersonalSnippet } + let(:instance) { model.new(author: user1) } + + it 'returns whether the user is a participant' do + allow(instance).to receive(:foo).and_return(user1) + allow(instance).to receive(:bar).and_return(user2) + + expect(instance.participant?(user1)).to be true + expect(instance.participant?(user2)).to be false + expect(instance.participant?(user3)).to be false + end + end end end diff --git a/spec/models/concerns/safe_url_spec.rb b/spec/models/concerns/safe_url_spec.rb index 3d38c05bf11..c298e56b1b1 100644 --- a/spec/models/concerns/safe_url_spec.rb +++ b/spec/models/concerns/safe_url_spec.rb @@ -26,14 +26,16 @@ RSpec.describe SafeUrl do context 'when URL contains credentials' do let(:url) { 'http://foo:bar@example.com' } - it { is_expected.to eq('http://*****:*****@example.com')} + it 'masks username and password' do + is_expected.to eq('http://*****:*****@example.com') + end - context 'when username is whitelisted' do - subject { test_class.safe_url(usernames_whitelist: usernames_whitelist) } + context 'when username is allowed' do + subject { test_class.safe_url(allowed_usernames: usernames) } - let(:usernames_whitelist) { %w[foo] } + let(:usernames) { %w[foo] } - it 'does expect the whitelisted username not to be masked' do + it 'masks the password, but not the username' do is_expected.to eq('http://foo:*****@example.com') end end diff --git a/spec/models/concerns/sidebars/container_with_html_options_spec.rb b/spec/models/concerns/sidebars/container_with_html_options_spec.rb new file mode 100644 index 00000000000..c555945e121 --- /dev/null +++ b/spec/models/concerns/sidebars/container_with_html_options_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::ContainerWithHtmlOptions do + subject do + Class.new do + include Sidebars::ContainerWithHtmlOptions + + def title + 'Foo' + end + end.new + end + + describe '#container_html_options' do + it 'includes title attribute' do + expect(subject.container_html_options).to eq(title: 'Foo') + end + end +end diff --git a/spec/models/concerns/sidebars/positionable_list_spec.rb b/spec/models/concerns/sidebars/positionable_list_spec.rb new file mode 100644 index 00000000000..231aa5295dd --- /dev/null +++ b/spec/models/concerns/sidebars/positionable_list_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::PositionableList do + subject do + Class.new do + include Sidebars::PositionableList + end.new + end + + describe '#add_element' do + it 'adds the element to the last position of the list' do + list = [1, 2] + + subject.add_element(list, 3) + + expect(list).to eq([1, 2, 3]) + end + end + + describe '#insert_element_before' do + let(:user) { build(:user) } + let(:list) { [1, user] } + + it 'adds element before the specific element class' do + subject.insert_element_before(list, User, 2) + + expect(list).to eq [1, 2, user] + end + + context 'when reference element does not exist' do + it 'adds the element to the top of the list' do + subject.insert_element_before(list, Project, 2) + + expect(list).to eq [2, 1, user] + end + end + end + + describe '#insert_element_after' do + let(:user) { build(:user) } + let(:list) { [1, user] } + + it 'adds element after the specific element class' do + subject.insert_element_after(list, Integer, 2) + + expect(list).to eq [1, 2, user] + end + + context 'when reference element does not exist' do + it 'adds the element to the end of the list' do + subject.insert_element_after(list, Project, 2) + + expect(list).to eq [1, user, 2] + end + end + end +end diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb index bbfdaeec64c..cfa00bab025 100644 --- a/spec/models/concerns/sortable_spec.rb +++ b/spec/models/concerns/sortable_spec.rb @@ -3,6 +3,31 @@ require 'spec_helper' RSpec.describe Sortable do + describe 'scopes' do + describe 'secondary ordering by id' do + let(:sorted_relation) { Group.all.order_created_asc } + + def arel_orders(relation) + relation.arel.orders + end + + it 'allows secondary ordering by id ascending' do + orders = arel_orders(sorted_relation.with_order_id_asc) + + expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders).to all(be_kind_of(Arel::Nodes::Ascending)) + end + + it 'allows secondary ordering by id descending' do + orders = arel_orders(sorted_relation.with_order_id_desc) + + expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders.first).to be_kind_of(Arel::Nodes::Ascending) + expect(orders.last).to be_kind_of(Arel::Nodes::Descending) + end + end + end + describe '.order_by' do let(:arel_table) { Group.arel_table } let(:relation) { Group.all } diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index 3e52ca5cf63..a60a0a5e26d 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -7,50 +7,54 @@ RSpec.describe Subscribable, 'Subscribable' do let(:resource) { create(:issue, project: project) } let(:user_1) { create(:user) } - describe '#subscribed?' do + shared_examples 'returns expected values' do |method| context 'without user' do it 'returns false' do - expect(resource.subscribed?(nil, project)).to be_falsey + expect(resource.public_send(method, nil, project)).to be_falsey end end context 'without project' do it 'returns false when no subscription exists' do - expect(resource.subscribed?(user_1)).to be_falsey + expect(resource.public_send(method, user_1)).to be_falsey end - it 'returns true when a subcription exists and subscribed is true' do + it 'returns true when a subscription exists and subscribed is true' do resource.subscriptions.create!(user: user_1, subscribed: true) - expect(resource.subscribed?(user_1)).to be_truthy + expect(resource.public_send(method, user_1)).to be_truthy end - it 'returns false when a subcription exists and subscribed is false' do + it 'returns false when a subscription exists and subscribed is false' do resource.subscriptions.create!(user: user_1, subscribed: false) - expect(resource.subscribed?(user_1)).to be_falsey + expect(resource.public_send(method, user_1)).to be_falsey end end context 'with project' do it 'returns false when no subscription exists' do - expect(resource.subscribed?(user_1, project)).to be_falsey + expect(resource.public_send(method, user_1, project)).to be_falsey end - it 'returns true when a subcription exists and subscribed is true' do + it 'returns true when a subscription exists and subscribed is true' do resource.subscriptions.create!(user: user_1, project: project, subscribed: true) - expect(resource.subscribed?(user_1, project)).to be_truthy + expect(resource.public_send(method, user_1, project)).to be_truthy end - it 'returns false when a subcription exists and subscribed is false' do + it 'returns false when a subscription exists and subscribed is false' do resource.subscriptions.create!(user: user_1, project: project, subscribed: false) - expect(resource.subscribed?(user_1, project)).to be_falsey + expect(resource.public_send(method, user_1, project)).to be_falsey end end end + describe '#subscribed?' do + it_behaves_like 'returns expected values', :subscribed? + end + describe '#subscribers' do it 'returns [] when no subcribers exists' do expect(resource.subscribers(project)).to be_empty @@ -189,4 +193,27 @@ RSpec.describe Subscribable, 'Subscribable' do it_behaves_like 'setting subscriptions' end end + + describe '#lazy_subscription' do + let(:labels) { create_list(:group_label, 5) } + + before do + labels.each do |label| + create(:subscription, :group_label, user: user_1, subscribable: label) + end + end + + it 'executes only one SQL query' do + lazy_queries = ActiveRecord::QueryRecorder.new do + labels.each { |label| label.lazy_subscription(user_1) } + end + + preloaded_queries = ActiveRecord::QueryRecorder.new do + labels.each { |label| label.lazy_subscription(user_1)&.subscribed? } + end + + expect(lazy_queries.count).to eq(0) + expect(preloaded_queries.count).to eq(1) + end + end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 2df76684d71..4bdb3e0a32a 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -54,7 +54,7 @@ RSpec.describe ApplicationSetting, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do expect(subject).to eq settings.reload.runners_registration_token expect(settings.read_attribute('runners_registration_token_encrypted')) - .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) + .to eq TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token(subject) expect(settings).to be_persisted end @@ -243,7 +243,7 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do build.ensure_token! - encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) + encrypted = TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token(build.token) expect(build.read_attribute('token_encrypted')).to eq encrypted end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index 1e1cd97e410..b311e302a31 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -7,6 +7,10 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do let(:instance) { double(:instance) } let(:encrypted) do + TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token('my-value') + end + + let(:encrypted_with_static_iv) do Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') end @@ -15,12 +19,25 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#find_token_authenticatable' do - context 'when using optional strategy' do + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'encrypted resource' + end + end + + context 'when encryption is optional' do let(:options) { { encrypted: :optional } } it 'finds the encrypted resource by cleartext' do allow(model).to receive(:find_by) - .with('some_field_encrypted' => encrypted) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return('encrypted resource') expect(subject.find_token_authenticatable('my-value')) @@ -33,7 +50,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do .and_return('plaintext resource') allow(model).to receive(:find_by) - .with('some_field_encrypted' => encrypted) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return(nil) expect(subject.find_token_authenticatable('my-value')) @@ -41,7 +58,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'finds the cleartext resource by cleartext' do @@ -65,12 +82,28 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#get_token' do - context 'when using optional strategy' do - let(:options) { { encrypted: :optional } } + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'returns decrypted token when an encrypted with static iv token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(Gitlab::CryptoHelper.aes256_gcm_encrypt('my-test-value')) + + expect(subject.get_token(instance)).to eq 'my-test-value' + end + + it 'returns decrypted token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) - before do - stub_feature_flags(dynamic_nonce_creation: false) + expect(subject.get_token(instance)).to eq 'my-value' end + end + + context 'when encryption is optional' do + let(:options) { { encrypted: :optional } } it 'returns decrypted token when an encrypted token is present' do allow(instance).to receive(:read_attribute) @@ -80,6 +113,14 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.get_token(instance)).to eq 'my-value' end + it 'returns decrypted token when an encrypted with static iv token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(Gitlab::CryptoHelper.aes256_gcm_encrypt('my-test-value')) + + expect(subject.get_token(instance)).to eq 'my-test-value' + end + it 'returns the plaintext token when encrypted token is not present' do allow(instance).to receive(:read_attribute) .with('some_field_encrypted') @@ -93,7 +134,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'returns cleartext token when an encrypted token is present' do @@ -123,12 +164,22 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#set_token' do - context 'when using optional strategy' do + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'writes encrypted token and returns it' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + context 'when encryption is optional' do let(:options) { { encrypted: :optional } } it 'writes encrypted token and removes plaintext token and returns it' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', any_args) + .with('some_field_encrypted', encrypted) expect(instance).to receive(:[]=) .with('some_field', nil) @@ -136,12 +187,12 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'writes encrypted token and writes plaintext token' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', any_args) + .with('some_field_encrypted', encrypted) expect(instance).to receive(:[]=) .with('some_field', 'my-value') diff --git a/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb new file mode 100644 index 00000000000..6f322a32a3b --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe TokenAuthenticatableStrategies::EncryptionHelper do + let(:encrypted_token) { described_class.encrypt_token('my-value') } + + describe '.encrypt_token' do + it 'encrypts token' do + expect(encrypted_token).not_to eq('my-value') + end + end + + describe '.decrypt_token' do + it 'decrypts token with static iv' do + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + + it 'decrypts token with dynamic iv' do + iv = ::Digest::SHA256.hexdigest('my-value').bytes.take(described_class::NONCE_SIZE).pack('c*') + token = Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value', nonce: iv) + encrypted_token = "#{described_class::DYNAMIC_NONCE_IDENTIFIER}#{token}#{iv}" + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + end +end diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb index ccc2c64e02c..60f4b9c55b1 100644 --- a/spec/models/deploy_keys_project_spec.rb +++ b/spec/models/deploy_keys_project_spec.rb @@ -26,7 +26,7 @@ RSpec.describe DeployKeysProject do end it "doesn't destroy the deploy key" do - subject.destroy + subject.destroy! expect { deploy_key.reload }.not_to raise_error end @@ -34,7 +34,7 @@ RSpec.describe DeployKeysProject do context "when the deploy key is private" do it "destroys the deploy key" do - subject.destroy + subject.destroy! expect { deploy_key.reload }.to raise_error(ActiveRecord::RecordNotFound) end @@ -49,7 +49,7 @@ RSpec.describe DeployKeysProject do end it "doesn't destroy the deploy key" do - subject.destroy + subject.destroy! expect { deploy_key.reload }.not_to raise_error end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index c7e1d5fc0d5..c8917a7dd65 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -47,7 +47,7 @@ RSpec.describe DeployToken do describe '#ensure_token' do it 'ensures a token' do deploy_token.token = nil - deploy_token.save + deploy_token.save! expect(deploy_token.token).not_to be_empty end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 68d12f51d4b..c9544569ad6 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -161,9 +161,9 @@ RSpec.describe Deployment do end end - it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + it 'does not execute Deployments::LinkMergeRequestWorker' do expect(Deployments::LinkMergeRequestWorker) - .to receive(:perform_async).with(deployment.id) + .not_to receive(:perform_async).with(deployment.id) deployment.drop! end @@ -188,9 +188,9 @@ RSpec.describe Deployment do end end - it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + it 'does not execute Deployments::LinkMergeRequestWorker' do expect(Deployments::LinkMergeRequestWorker) - .to receive(:perform_async).with(deployment.id) + .not_to receive(:perform_async).with(deployment.id) deployment.cancel! end @@ -497,7 +497,7 @@ RSpec.describe Deployment do context 'when the SHA for the deployment does not exist in the repo' do it 'returns false' do - deployment.update(sha: Gitlab::Git::BLANK_SHA) + deployment.update!(sha: Gitlab::Git::BLANK_SHA) commit = project.commit expect(deployment.includes_commit?(commit)).to be false @@ -573,15 +573,39 @@ RSpec.describe Deployment do end describe '#previous_deployment' do - it 'returns the previous deployment' do - deploy1 = create(:deployment) - deploy2 = create( - :deployment, - project: deploy1.project, - environment: deploy1.environment - ) + using RSpec::Parameterized::TableSyntax - expect(deploy2.previous_deployment).to eq(deploy1) + let_it_be(:project) { create(:project, :repository) } + let_it_be(:production) { create(:environment, :production, project: project) } + let_it_be(:staging) { create(:environment, :staging, project: project) } + let_it_be(:production_deployment_1) { create(:deployment, :success, project: project, environment: production) } + let_it_be(:production_deployment_2) { create(:deployment, :success, project: project, environment: production) } + let_it_be(:production_deployment_3) { create(:deployment, :failed, project: project, environment: production) } + let_it_be(:production_deployment_4) { create(:deployment, :canceled, project: project, environment: production) } + let_it_be(:staging_deployment_1) { create(:deployment, :failed, project: project, environment: staging) } + let_it_be(:staging_deployment_2) { create(:deployment, :success, project: project, environment: staging) } + let_it_be(:production_deployment_5) { create(:deployment, :success, project: project, environment: production) } + let_it_be(:staging_deployment_3) { create(:deployment, :success, project: project, environment: staging) } + + where(:pointer, :expected_previous_deployment) do + 'production_deployment_1' | nil + 'production_deployment_2' | 'production_deployment_1' + 'production_deployment_3' | 'production_deployment_2' + 'production_deployment_4' | 'production_deployment_2' + 'staging_deployment_1' | nil + 'staging_deployment_2' | nil + 'production_deployment_5' | 'production_deployment_2' + 'staging_deployment_3' | 'staging_deployment_2' + end + + with_them do + it 'returns the previous deployment' do + if expected_previous_deployment.nil? + expect(send(pointer).previous_deployment).to eq(expected_previous_deployment) + else + expect(send(pointer).previous_deployment).to eq(send(expected_previous_deployment)) + end + end end end @@ -631,45 +655,6 @@ RSpec.describe Deployment do end end - describe '#previous_environment_deployment' do - it 'returns the previous deployment of the same environment' do - deploy1 = create(:deployment, :success) - deploy2 = create( - :deployment, - :success, - project: deploy1.project, - environment: deploy1.environment - ) - - expect(deploy2.previous_environment_deployment).to eq(deploy1) - end - - it 'ignores deployments that were not successful' do - deploy1 = create(:deployment, :failed) - deploy2 = create( - :deployment, - :success, - project: deploy1.project, - environment: deploy1.environment - ) - - expect(deploy2.previous_environment_deployment).to be_nil - end - - it 'ignores deployments for different environments' do - deploy1 = create(:deployment, :success) - preprod = create(:environment, project: deploy1.project, name: 'preprod') - deploy2 = create( - :deployment, - :success, - project: deploy1.project, - environment: preprod - ) - - expect(deploy2.previous_environment_deployment).to be_nil - end - end - describe '#create_ref' do let(:deployment) { build(:deployment) } @@ -796,4 +781,30 @@ RSpec.describe Deployment do end end end + + describe '#update_merge_request_metrics!' do + let_it_be(:project) { create(:project, :repository) } + let(:environment) { build(:environment, environment_tier, project: project) } + let!(:deployment) { create(:deployment, :success, project: project, environment: environment) } + let!(:merge_request) { create(:merge_request, :simple, :merged_last_month, project: project) } + + context 'with production environment' do + let(:environment_tier) { :production } + + it 'updates merge request metrics for production-grade environment' do + expect { deployment.update_merge_request_metrics! } + .to change { merge_request.reload.metrics.first_deployed_to_production_at } + .from(nil).to(deployment.reload.finished_at) + end + end + + context 'with staging environment' do + let(:environment_tier) { :staging } + + it 'updates merge request metrics for production-grade environment' do + expect { deployment.update_merge_request_metrics! } + .not_to change { merge_request.reload.metrics.first_deployed_to_production_at } + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e021a6cf6d3..759bb080172 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -302,6 +302,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'testing' | described_class.tiers[:testing] 'testing-prd' | described_class.tiers[:testing] 'acceptance-testing' | described_class.tiers[:testing] + 'production-test' | described_class.tiers[:testing] + 'test-production' | described_class.tiers[:testing] 'QC' | described_class.tiers[:testing] 'gstg' | described_class.tiers[:staging] 'staging' | described_class.tiers[:staging] @@ -315,6 +317,12 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do 'gprd-cny' | described_class.tiers[:production] 'production' | described_class.tiers[:production] 'Production' | described_class.tiers[:production] + 'PRODUCTION' | described_class.tiers[:production] + 'Production/eu' | described_class.tiers[:production] + 'production/eu' | described_class.tiers[:production] + 'PRODUCTION/EU' | described_class.tiers[:production] + 'productioneu' | described_class.tiers[:production] + 'production/www.gitlab.com' | described_class.tiers[:production] 'prod' | described_class.tiers[:production] 'PROD' | described_class.tiers[:production] 'Live' | described_class.tiers[:production] @@ -444,31 +452,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#update_merge_request_metrics?' do - { - 'gprd' => false, - 'prod' => true, - 'prod-test' => false, - 'PROD' => true, - 'production' => true, - 'production-test' => false, - 'PRODUCTION' => true, - 'production/eu' => true, - 'PRODUCTION/EU' => true, - 'production/www.gitlab.com' => true, - 'productioneu' => false, - 'Production' => true, - 'Production/eu' => true, - 'test-production' => false - }.each do |name, expected_value| - it "returns #{expected_value} for #{name}" do - env = create(:environment, name: name) - - expect(env.update_merge_request_metrics?).to eq(expected_value), "Expected the name '#{name}' to result in #{expected_value}, but it didn't." - end - end - end - describe '#environment_type' do subject { environment.environment_type } diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 47148c4febc..949e8ec0a72 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -299,11 +299,11 @@ RSpec.describe Event do end def visible_to_none_except(*roles) - visible_to_none.merge(roles.map { |role| [role, true] }.to_h) + visible_to_none.merge(roles.to_h { |role| [role, true] }) end def visible_to_all_except(*roles) - visible_to_all.merge(roles.map { |role| [role, false] }.to_h) + visible_to_all.merge(roles.to_h { |role| [role, false] }) end shared_examples 'visibility examples' do @@ -723,7 +723,7 @@ RSpec.describe Event do note_on_design: true, note_on_commit: true } - valid_target_factories.map do |kind, needs_project| + valid_target_factories.to_h do |kind, needs_project| extra_data = if kind == :merge_request { source_project: project } elsif needs_project @@ -735,7 +735,7 @@ RSpec.describe Event do target = kind == :project ? nil : build(kind, **extra_data) [kind, build(:event, :created, author: project.owner, project: project, target: target)] - end.to_h + end end it 'passes a sanity check', :aggregate_failures do diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 09dd1766acc..1517f426fa3 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -244,18 +244,27 @@ RSpec.describe Experiment do context 'when no existing experiment_subject record exists for the given group' do it 'creates an experiment_subject record' do - expect_next(ExperimentSubject).to receive(:update!).with(variant: variant).and_call_original - expect { record_group_and_variant! }.to change(ExperimentSubject, :count).by(1) + expect(ExperimentSubject.last.variant).to eq(variant.to_s) end end context 'when an existing experiment_subject exists for the given group' do - context 'but it belonged to a different variant' do - let!(:experiment_subject) do - create(:experiment_subject, experiment: experiment, group: group, user: nil, variant: :experimental) + let_it_be(:experiment_subject) do + create(:experiment_subject, experiment: experiment, group: group, user: nil, variant: :experimental) + end + + context 'when it belongs to the same variant' do + let(:variant) { :experimental } + + it 'does not initiate a transaction' do + expect(ActiveRecord::Base.connection).not_to receive(:transaction) + + subject end + end + context 'but it belonged to a different variant' do it 'updates the variant value' do expect { record_group_and_variant! }.to change { experiment_subject.reload.variant }.to('control') end @@ -299,6 +308,16 @@ RSpec.describe Experiment do expect { subject }.not_to change(ExperimentUser, :count) end + context 'when group type or context did not change' do + let(:context) { {} } + + it 'does not initiate a transaction' do + expect(ActiveRecord::Base.connection).not_to receive(:transaction) + + subject + end + end + context 'but the group_type and context has changed' do let(:group) { :experimental } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 24d09d1c035..2f82d8a0bbe 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Group do + include ReloadHelpers + let!(:group) { create(:group) } describe 'associations' do @@ -281,7 +283,7 @@ RSpec.describe Group do end describe '#two_factor_authentication_allowed' do - let_it_be(:group) { create(:group) } + let_it_be_with_reload(:group) { create(:group) } context 'for a parent group' do it 'is valid' do @@ -311,6 +313,120 @@ RSpec.describe Group do end end + context 'traversal_ids on create' do + context 'default traversal_ids' do + let(:group) { build(:group) } + + before do + group.save! + group.reload + end + + it { expect(group.traversal_ids).to eq [group.id] } + end + + context 'has a parent' do + let(:parent) { create(:group) } + let(:group) { build(:group, parent: parent) } + + before do + group.save! + reload_models(parent, group) + end + + it { expect(parent.traversal_ids).to eq [parent.id] } + it { expect(group.traversal_ids).to eq [parent.id, group.id] } + end + + context 'has a parent update before save' do + let(:parent) { create(:group) } + let(:group) { build(:group, parent: parent) } + let!(:new_grandparent) { create(:group) } + + before do + parent.update!(parent: new_grandparent) + group.save! + reload_models(parent, group) + end + + it 'avoid traversal_ids race condition' do + expect(parent.traversal_ids).to eq [new_grandparent.id, parent.id] + expect(group.traversal_ids).to eq [new_grandparent.id, parent.id, group.id] + end + end + end + + context 'traversal_ids on update' do + context 'parent is updated' do + let(:new_parent) { create(:group) } + + subject {group.update!(parent: new_parent, name: 'new name') } + + it_behaves_like 'update on column', :traversal_ids + end + + context 'parent is not updated' do + subject { group.update!(name: 'new name') } + + it_behaves_like 'no update on column', :traversal_ids + end + end + + context 'traversal_ids on ancestral update' do + context 'update multiple ancestors before save' do + let(:parent) { create(:group) } + let(:group) { create(:group, parent: parent) } + let!(:new_grandparent) { create(:group) } + let!(:new_parent) { create(:group) } + + before do + group.parent = new_parent + new_parent.update!(parent: new_grandparent) + + group.save! + reload_models(parent, group, new_grandparent, new_parent) + end + + it 'avoids traversal_ids race condition' do + expect(parent.traversal_ids).to eq [parent.id] + expect(group.traversal_ids).to eq [new_grandparent.id, new_parent.id, group.id] + expect(new_grandparent.traversal_ids).to eq [new_grandparent.id] + expect(new_parent.traversal_ids).to eq [new_grandparent.id, new_parent.id] + end + end + + context 'assigning a new parent' do + let!(:old_parent) { create(:group) } + let!(:new_parent) { create(:group) } + let!(:group) { create(:group, parent: old_parent) } + + before do + group.update(parent: new_parent) + reload_models(old_parent, new_parent, group) + end + + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [new_parent.id, group.id] + end + end + + context 'assigning a new grandparent' do + let!(:old_grandparent) { create(:group) } + let!(:new_grandparent) { create(:group) } + let!(:parent_group) { create(:group, parent: old_grandparent) } + let!(:group) { create(:group, parent: parent_group) } + + before do + parent_group.update(parent: new_grandparent) + end + + it 'updates traversal_ids for all descendants' do + expect(parent_group.reload.traversal_ids).to eq [new_grandparent.id, parent_group.id] + expect(group.reload.traversal_ids).to eq [new_grandparent.id, parent_group.id, group.id] + end + end + end + describe '.without_integration' do let(:another_group) { create(:group) } let(:instance_integration) { build(:jira_service, :instance) } @@ -565,39 +681,178 @@ RSpec.describe Group do end end - describe '#last_blocked_owner?' do - let(:blocked_user) { create(:user, :blocked) } + describe '#member_last_blocked_owner?' do + let_it_be(:blocked_user) { create(:user, :blocked) } + + let(:member) { blocked_user.group_members.last } before do group.add_user(blocked_user, GroupMember::OWNER) end - it { expect(group.last_blocked_owner?(blocked_user)).to be_truthy } + context 'when last_blocked_owner is set' do + before do + expect(group).not_to receive(:members_with_parents) + end + + it 'returns true' do + member.last_blocked_owner = true + + expect(group.member_last_blocked_owner?(member)).to be(true) + end + + it 'returns false' do + member.last_blocked_owner = false + + expect(group.member_last_blocked_owner?(member)).to be(false) + end + end + + context 'when last_blocked_owner is not set' do + it { expect(group.member_last_blocked_owner?(member)).to be(true) } + + context 'with another active owner' do + before do + group.add_user(create(:user), GroupMember::OWNER) + end + + it { expect(group.member_last_blocked_owner?(member)).to be(false) } + end + + context 'with 2 blocked owners' do + before do + group.add_user(create(:user, :blocked), GroupMember::OWNER) + end + + it { expect(group.member_last_blocked_owner?(member)).to be(false) } + 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.member_last_blocked_owner?(member)).to be(false) } + end + end + end + + context 'when analyzing blocked owners' do + let_it_be(:blocked_user) { create(:user, :blocked) } + + describe '#single_blocked_owner?' do + context 'when there is only one blocked owner' do + before do + group.add_user(blocked_user, GroupMember::OWNER) + end + + it 'returns true' do + expect(group.single_blocked_owner?).to eq(true) + end + end + + context 'when there are multiple blocked owners' do + let_it_be(:blocked_user_2) { create(:user, :blocked) } + + before do + group.add_user(blocked_user, GroupMember::OWNER) + group.add_user(blocked_user_2, GroupMember::OWNER) + end + + it 'returns true' do + expect(group.single_blocked_owner?).to eq(false) + end + end + + context 'when there are no blocked owners' do + it 'returns false' do + expect(group.single_blocked_owner?).to eq(false) + end + end + end + + describe '#blocked_owners' do + let_it_be(:user) { create(:user) } - context 'with another active owner' do before do - group.add_user(create(:user), GroupMember::OWNER) + group.add_user(blocked_user, GroupMember::OWNER) + group.add_user(user, GroupMember::OWNER) end - it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + it 'has only blocked owners' do + expect(group.blocked_owners.map(&:user)).to match([blocked_user]) + end end + end + + describe '#single_owner?' do + let_it_be(:user) { create(:user) } - context 'with 2 blocked owners' do + context 'when there is only one owner' do before do - group.add_user(create(:user, :blocked), GroupMember::OWNER) + group.add_user(user, GroupMember::OWNER) end - it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + it 'returns true' do + expect(group.single_owner?).to eq(true) + end end - context 'with owners from a parent' do + context 'when there are multiple owners' do + let_it_be(:user_2) { create(:user) } + before do - parent_group = create(:group) - create(:group_member, :owner, group: parent_group) - group.update(parent: parent_group) + group.add_user(user, GroupMember::OWNER) + group.add_user(user_2, GroupMember::OWNER) end - it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + it 'returns true' do + expect(group.single_owner?).to eq(false) + end + end + + context 'when there are no owners' do + it 'returns false' do + expect(group.single_owner?).to eq(false) + end + end + end + + describe '#member_last_owner?' do + let_it_be(:user) { create(:user) } + + let(:member) { group.members.last } + + before do + group.add_user(user, GroupMember::OWNER) + end + + context 'when last_owner is set' do + before do + expect(group).not_to receive(:last_owner?) + end + + it 'returns true' do + member.last_owner = true + + expect(group.member_last_owner?(member)).to be(true) + end + + it 'returns false' do + member.last_owner = false + + expect(group.member_last_owner?(member)).to be(false) + end + end + + context 'when last_owner is not set' do + it 'returns true' do + expect(group).to receive(:last_owner?).and_call_original + + expect(group.member_last_owner?(member)).to be(true) + end end end @@ -1500,30 +1755,6 @@ RSpec.describe Group do 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 @@ -1838,24 +2069,28 @@ RSpec.describe Group do end end - def subject_and_reload(*models) - subject - models.map(&:reload) - end - describe '#update_shared_runners_setting!' do context 'enabled' do subject { group.update_shared_runners_setting!('enabled') } context 'group that its ancestors have shared runners disabled' do - let_it_be(:parent) { create(:group, :shared_runners_disabled) } - let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled) } + let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project, reload: true) { create(:project, shared_runners_enabled: false, group: group) } - it 'raises error and does not enable shared Runners' do - expect { subject_and_reload(parent, group, project) } + it 'raises exception' do + expect { subject } .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Shared runners enabled cannot be enabled because parent group has shared Runners disabled') - .and not_change { parent.shared_runners_enabled } + end + + it 'does not enable shared runners' do + expect do + subject rescue nil + + parent.reload + group.reload + project.reload + end.to not_change { parent.shared_runners_enabled } .and not_change { group.shared_runners_enabled } .and not_change { project.shared_runners_enabled } end @@ -1941,13 +2176,21 @@ RSpec.describe Group do end context 'when parent does not allow' do - let_it_be(:parent) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } - let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } + let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } - it 'raises error and does not allow descendants to override' do - expect { subject_and_reload(parent, group) } + it 'raises exception' do + expect { subject } .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') - .and not_change { parent.allow_descendants_override_disabled_shared_runners } + end + + it 'does not allow descendants to override' do + expect do + subject rescue nil + + parent.reload + group.reload + end.to not_change { parent.allow_descendants_override_disabled_shared_runners } .and not_change { parent.shared_runners_enabled } .and not_change { group.allow_descendants_override_disabled_shared_runners } .and not_change { group.shared_runners_enabled } @@ -2104,4 +2347,18 @@ RSpec.describe Group do it_behaves_like 'model with Debian distributions' end + + describe '.ids_with_disabled_email' do + let!(:parent_1) { create(:group, emails_disabled: true) } + let!(:child_1) { create(:group, parent: parent_1) } + + let!(:parent_2) { create(:group, emails_disabled: false) } + let!(:child_2) { create(:group, parent: parent_2) } + + let!(:other_group) { create(:group, emails_disabled: false) } + + subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) } + + it { is_expected.to eq(Set.new([child_1.id])) } + end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index e56d08c1847..02e630cbf27 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -56,7 +56,7 @@ RSpec.describe SystemHook do end it "user_destroy hook" do - user.destroy + user.destroy! expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_destroy/, @@ -102,7 +102,7 @@ RSpec.describe SystemHook do end it 'group destroy hook' do - group.destroy + group.destroy! expect(WebMock).to have_requested(:post, system_hook.url).with( body: /group_destroy/, diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 3fc1ad6eb0d..413e69fb071 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -26,7 +26,7 @@ RSpec.describe WebHook do it 'strips :url before saving it' do hook.url = ' https://example.com ' - hook.save + hook.save! expect(hook.url).to eq('https://example.com') end @@ -45,14 +45,14 @@ RSpec.describe WebHook do it 'gets rid of whitespace' do hook.push_events_branch_filter = ' branch ' - hook.save + hook.save! expect(hook.push_events_branch_filter).to eq('branch') end it 'stores whitespace only as empty' do hook.push_events_branch_filter = ' ' - hook.save + hook.save! expect(hook.push_events_branch_filter).to eq('') end @@ -91,7 +91,7 @@ RSpec.describe WebHook do web_hook = create(:project_hook) create_list(:web_hook_log, 3, web_hook: web_hook) - expect { web_hook.destroy }.to change(web_hook.web_hook_logs, :count).by(-3) + expect { web_hook.destroy! }.to change(web_hook.web_hook_logs, :count).by(-3) end end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index d89b323f525..781e2aece56 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -6,23 +6,28 @@ RSpec.describe Integration do let_it_be(:project_1) { create(:project) } let_it_be(:project_2) { create(:project) } let_it_be(:project_3) { create(:project) } + let_it_be(:project_4) { create(:project) } let_it_be(:instance_integration) { create(:jira_service, :instance) } before do create(:jira_service, project: project_1, inherit_from_id: instance_integration.id) create(:jira_service, project: project_2, inherit_from_id: nil) - create(:slack_service, project: project_3, inherit_from_id: nil) + create(:jira_service, group: create(:group), project: nil, inherit_from_id: nil) + create(:jira_service, project: project_3, inherit_from_id: nil) + create(:slack_service, project: project_4, inherit_from_id: nil) end describe '.with_custom_integration_for' do it 'returns projects with custom integrations' do - expect(Project.with_custom_integration_for(instance_integration)).to contain_exactly(project_2) + # We use pagination to verify that the group is excluded from the query + expect(Project.with_custom_integration_for(instance_integration, 0, 2)).to contain_exactly(project_2, project_3) + expect(Project.with_custom_integration_for(instance_integration)).to contain_exactly(project_2, project_3) end end describe '.without_integration' do it 'returns projects without integration' do - expect(Project.without_integration(instance_integration)).to contain_exactly(project_3) + expect(Project.without_integration(instance_integration)).to contain_exactly(project_4) end end end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 07f62b9de55..981245627af 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -97,6 +97,25 @@ RSpec.describe InternalId do expect(subject).to eq(1) end end + + context 'when executed outside of transaction' do + it 'increments counter with in_transaction: "false"' do + expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original + + subject + end + end + + context 'when executed within transaction' do + it 'increments counter with in_transaction: "true"' do + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original + + InternalId.transaction { subject } + end + end end describe '.reset' do @@ -134,6 +153,29 @@ RSpec.describe InternalId do described_class.generate_next(issue, scope, usage, init) end end + + context 'when executed outside of transaction' do + let(:value) { 2 } + + it 'increments counter with in_transaction: "false"' do + expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original + + subject + end + end + + context 'when executed within transaction' do + let(:value) { 2 } + + it 'increments counter with in_transaction: "true"' do + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original + + InternalId.transaction { subject } + end + end end describe '.track_greatest' do @@ -183,6 +225,25 @@ RSpec.describe InternalId do expect(subject).to eq(value) end end + + context 'when executed outside of transaction' do + it 'increments counter with in_transaction: "false"' do + expect(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original + + subject + end + end + + context 'when executed within transaction' do + it 'increments counter with in_transaction: "true"' do + expect(InternalId::InternalIdGenerator.internal_id_transactions_total).to receive(:increment) + .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original + + InternalId.transaction { subject } + end + end end describe '#increment_and_save!' do diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 1d3c09a48b7..18b0a46c928 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Issue::Metrics do context "milestones" do it "records the first time an issue is associated with a milestone" do time = Time.current - travel_to(time) { subject.update(milestone: create(:milestone, project: project)) } + travel_to(time) { subject.update!(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present @@ -47,9 +47,9 @@ RSpec.describe Issue::Metrics do it "does not record the second time an issue is associated with a milestone" do time = Time.current - travel_to(time) { subject.update(milestone: create(:milestone, project: project)) } - travel_to(time + 2.hours) { subject.update(milestone: nil) } - travel_to(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) } + travel_to(time) { subject.update!(milestone: create(:milestone, project: project)) } + travel_to(time + 2.hours) { subject.update!(milestone: nil) } + travel_to(time + 6.hours) { subject.update!(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present @@ -61,7 +61,7 @@ RSpec.describe Issue::Metrics do it "records the first time an issue is associated with a list label" do list_label = create(:list).label time = Time.current - travel_to(time) { subject.update(label_ids: [list_label.id]) } + travel_to(time) { subject.update!(label_ids: [list_label.id]) } metrics = subject.metrics expect(metrics).to be_present @@ -71,9 +71,9 @@ RSpec.describe Issue::Metrics do it "does not record the second time an issue is associated with a list label" do time = Time.current first_list_label = create(:list).label - travel_to(time) { subject.update(label_ids: [first_list_label.id]) } + travel_to(time) { subject.update!(label_ids: [first_list_label.id]) } second_list_label = create(:list).label - travel_to(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } + travel_to(time + 5.hours) { subject.update!(label_ids: [second_list_label.id]) } metrics = subject.metrics expect(metrics).to be_present diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index a3e245f4def..23caf3647c3 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -85,18 +85,14 @@ RSpec.describe Issue do describe 'callbacks' do describe '#ensure_metrics' do it 'creates metrics after saving' do - issue = create(:issue, project: reusable_project) - - expect(issue.metrics).to be_persisted + expect(subject.metrics).to be_persisted expect(Issue::Metrics.count).to eq(1) end it 'does not create duplicate metrics for an issue' do - issue = create(:issue, project: reusable_project) + subject.close! - issue.close! - - expect(issue.metrics).to be_persisted + expect(subject.metrics).to be_persisted expect(Issue::Metrics.count).to eq(1) end @@ -105,6 +101,20 @@ RSpec.describe Issue do create(:issue, project: reusable_project) end + + context 'when metrics record is missing' do + before do + subject.metrics.delete + subject.reload + subject.metrics # make sure metrics association is cached (currently nil) + end + + it 'creates the metrics record' do + subject.update!(title: 'title') + + expect(subject.metrics).to be_present + end + end end describe '#record_create_action' do @@ -327,7 +337,7 @@ RSpec.describe Issue do end it 'returns true for a user that is the author of an issue' do - issue.update(author: user) + issue.update!(author: user) expect(issue.assignee_or_author?(user)).to be_truthy end @@ -665,7 +675,7 @@ RSpec.describe Issue do expect(user2.assigned_open_issues_count).to eq(0) issue.assignees = [user2] - issue.save + issue.save! expect(user1.assigned_open_issues_count).to eq(0) expect(user2.assigned_open_issues_count).to eq(1) @@ -897,7 +907,7 @@ RSpec.describe Issue do let(:private_project) { build(:project, :private)} before do - issue.update(project: private_project) # move issue to private project + issue.update!(project: private_project) # move issue to private project end shared_examples 'issue visible if user has guest access' do @@ -1034,7 +1044,7 @@ RSpec.describe Issue do with_them do it 'checks for spam on issues that can be seen anonymously' do project = reusable_project - project.update(visibility_level: visibility_level) + project.update!(visibility_level: visibility_level) issue = create(:issue, project: project, confidential: confidential, description: 'original description') issue.assign_attributes(new_attributes) @@ -1048,7 +1058,7 @@ RSpec.describe Issue do it 'refreshes the number of open issues of the project' do project = subject.project - expect { subject.destroy } + expect { subject.destroy! } .to change { project.open_issues_count }.from(1).to(0) end end @@ -1263,8 +1273,8 @@ RSpec.describe Issue 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') + 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 diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 3d33a39d353..0cb20efcb0a 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -75,6 +75,28 @@ RSpec.describe Key, :mailer do .to eq([key_3, key_1, key_2]) end end + + context 'expiration scopes' do + let_it_be(:user) { create(:user) } + let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user) } + let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user, expiry_notification_delivered_at: Time.current) } + let_it_be(:expired_yesterday) { create(:key, expires_at: 1.day.ago, user: user) } + let_it_be(:expiring_soon_unotified) { create(:key, expires_at: 3.days.from_now, user: user) } + let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) } + let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) } + + describe '.expired_today_and_not_notified' do + it 'returns keys that expire today' do + expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified) + end + end + + describe '.expiring_soon_and_not_notified' do + it 'returns keys that will expire soon' do + expect(described_class.expiring_soon_and_not_notified).to contain_exactly(expiring_soon_unotified) + end + end + end end context "validation of uniqueness (based on fingerprint uniqueness)" do diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index e1abfd9d8e5..14acaf11ca4 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Label do + let_it_be(:project) { create(:project) } + describe 'modules' do it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Subscribable) } @@ -44,6 +46,22 @@ RSpec.describe Label do end end + describe 'scopes' do + describe '.on_board' do + let(:board) { create(:board, project: project) } + let!(:list1) { create(:list, board: board, label: development) } + let!(:list2) { create(:list, board: board, label: testing) } + + let!(:development) { create(:label, project: project, name: 'Development') } + let!(:testing) { create(:label, project: project, name: 'Testing') } + let!(:regression) { create(:label, project: project, name: 'Regression') } + + it 'returns only the board labels' do + expect(described_class.on_board(board.id)).to match_array([development, testing]) + end + end + end + describe '#color' do it 'strips color' do label = described_class.new(color: ' #abcdef ') @@ -92,9 +110,7 @@ RSpec.describe Label do end describe 'priorization' do - subject(:label) { create(:label) } - - let(:project) { label.project } + subject(:label) { create(:label, project: project) } describe '#prioritize!' do context 'when label is not prioritized' do diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index ad07ee1115b..67a76c305f6 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -17,17 +17,4 @@ RSpec.describe List do it { is_expected.to validate_presence_of(:label) } it { is_expected.to validate_presence_of(:list_type) } end - - 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) - - exclude_type = [described_class.list_types[:label], described_class.list_types[:closed]] - - 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 c41f466456f..5f3a67b52ba 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -413,6 +413,24 @@ RSpec.describe Member do it { is_expected.not_to include @blocked_developer } it { is_expected.not_to include @member_with_minimal_access } end + + describe '.distinct_on_user_with_max_access_level' do + let_it_be(:other_group) { create(:group) } + let_it_be(:member_with_lower_access_level) { create(:group_member, :developer, group: other_group, user: @owner_user) } + + subject { described_class.default_scoped.distinct_on_user_with_max_access_level.to_a } + + it { is_expected.not_to include member_with_lower_access_level } + it { is_expected.to include @owner } + it { is_expected.to include @maintainer } + it { is_expected.to include @invited_member } + it { is_expected.to include @accepted_invite_member } + it { is_expected.to include @requested_member } + it { is_expected.to include @accepted_request_member } + it { is_expected.to include @blocked_maintainer } + it { is_expected.to include @blocked_developer } + it { is_expected.to include @member_with_minimal_access } + end end describe "Delegate methods" do @@ -420,6 +438,16 @@ RSpec.describe Member do it { is_expected.to respond_to(:user_email) } end + describe '.valid_email?' do + it 'is a valid email format' do + expect(described_class.valid_email?('foo')).to eq(false) + end + + it 'is not a valid email format' do + expect(described_class.valid_email?('foo@example.com')).to eq(true) + end + end + describe '.add_user' do %w[project group].each do |source_type| context "when source is a #{source_type}" do diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 3d3ed6fc54a..908bb9f91a3 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -66,6 +66,12 @@ RSpec.describe GroupMember do it_behaves_like 'members notifications', :group + describe '#namespace_id' do + subject { build(:group_member, source_id: 1).namespace_id } + + it { is_expected.to eq 1 } + end + describe '#real_source_type' do subject { create(:group_member).real_source_type } diff --git a/spec/models/members/last_group_owner_assigner_spec.rb b/spec/models/members/last_group_owner_assigner_spec.rb new file mode 100644 index 00000000000..3c9a7a11555 --- /dev/null +++ b/spec/models/members/last_group_owner_assigner_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::LastGroupOwnerAssigner do + describe "#execute" do + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:group) { create(:group) } + + let(:group_member) { user.members.last } + + subject(:assigner) { described_class.new(group, [group_member]) } + + before do + group.add_owner(user) + end + + it "avoids extra database queries utilizing memoization", :aggregate_failures do + control = ActiveRecord::QueryRecorder.new { assigner.execute } + count_queries = control.occurrences_by_line_method.first[1][:occurrences].find_all { |i| i.include?('SELECT COUNT') } + + expect(control.count).to be <= 5 + expect(count_queries.count).to eq(0) + end + + context "when there are unblocked owners" do + context "with one unblocked owner" do + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(true) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + end + + context "with multiple unblocked owners" do + let_it_be(:unblocked_owner_member) { create(:group_member, :owner, source: group) } + + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + + it "has many members passed" do + assigner = described_class.new(group, [unblocked_owner_member, group_member]) + + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + .and change(unblocked_owner_member, :last_owner) + .from(nil).to(false) + .and change(unblocked_owner_member, :last_blocked_owner) + .from(nil).to(false) + end + end + end + + context "when there are blocked owners" do + before do + user.block! + end + + context "with one blocked owner" do + specify do + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(true) + end + end + + context "with multiple unblocked owners" do + specify do + create_list(:group_member, 2, :owner, source: group) + + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + end + + context "with multiple blocked owners" do + specify do + create(:group_member, :owner, :blocked, source: group) + + expect { assigner.execute }.to change(group_member, :last_owner) + .from(nil).to(false) + .and change(group_member, :last_blocked_owner) + .from(nil).to(false) + end + end + end + end +end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 388d04c8012..ce3e86f964d 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -13,6 +13,10 @@ RSpec.describe ProjectMember do it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end + describe 'delegations' do + it { is_expected.to delegate_method(:namespace_id).to(:project) } + end + describe '.access_level_roles' do it 'returns Gitlab::Access.options' do expect(described_class.access_level_roles).to eq(Gitlab::Access.options) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8c7289adbcc..4b46c98117f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -186,39 +186,7 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:multiline_commits) { subject.commits.select(&is_multiline) } let(:singleline_commits) { subject.commits.reject(&is_multiline) } - context 'when the total number of commits is safe' do - it 'returns the oldest multiline commit message' do - expect(subject.default_squash_commit_message).to eq(multiline_commits.last.message) - end - end - - context 'when the total number of commits is big' do - let(:safe_number) { 20 } - - before do - stub_const('MergeRequestDiff::COMMITS_SAFE_SIZE', safe_number) - end - - it 'returns the oldest multiline commit message from safe number of commits' do - expect(subject.default_squash_commit_message).to eq( - "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n" - ) - end - end - - it 'returns the merge request title if there are no multiline commits' do - expect(subject).to receive(:commits).and_return( - CommitCollection.new(project, singleline_commits) - ) - - expect(subject.default_squash_commit_message).to eq(subject.title) - end - - it 'does not return commit messages from multiline merge commits' do - collection = CommitCollection.new(project, multiline_commits).enrich! - - expect(collection.commits).to all( receive(:merge_commit?).and_return(true) ) - expect(subject).to receive(:commits).and_return(collection) + it 'returns the merge request title' do expect(subject.default_squash_commit_message).to eq(subject.title) end end @@ -420,6 +388,19 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '.by_merge_or_squash_commit_sha' do + subject { described_class.by_merge_or_squash_commit_sha([sha1, sha2]) } + + let(:sha1) { '123abc' } + let(:sha2) { '456abc' } + let(:mr1) { create(:merge_request, :merged, squash_commit_sha: sha1) } + let(:mr2) { create(:merge_request, :merged, merge_commit_sha: sha2) } + + it 'returns merge requests that match the given squash and merge commits' do + is_expected.to include(mr1, mr2) + end + end + describe '.by_related_commit_sha' do subject { described_class.by_related_commit_sha(sha) } @@ -462,16 +443,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '.by_cherry_pick_sha' do - it 'returns merge requests that match the given merge commit' do - note = create(:track_mr_picking_note, commit_id: '456abc') - - create(:track_mr_picking_note, project: create(:project), commit_id: '456def') - - expect(described_class.by_cherry_pick_sha('456abc')).to eq([note.noteable]) - end - end - describe '.in_projects' do it 'returns the merge requests for a set of projects' do expect(described_class.in_projects(Project.all)).to eq([subject]) @@ -1353,6 +1324,24 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.work_in_progress?).to eq false end + it 'does not detect Draft: in the middle of the title' do + subject.title = 'Something with Draft: in the middle' + + expect(subject.work_in_progress?).to eq false + end + + it 'does not detect WIP at the end of the title' do + subject.title = 'Something ends with WIP' + + expect(subject.work_in_progress?).to eq false + end + + it 'does not detect Draft at the end of the title' do + subject.title = 'Something ends with Draft' + + expect(subject.work_in_progress?).to eq false + end + it "doesn't detect WIP for words starting with WIP" do subject.title = "Wipwap #{subject.title}" expect(subject.work_in_progress?).to eq false @@ -1363,6 +1352,11 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.work_in_progress?).to eq false end + it "doesn't detect draft for words containing with draft" do + subject.title = "Drafting #{subject.title}" + expect(subject.work_in_progress?).to eq false + end + it "doesn't detect WIP by default" do expect(subject.work_in_progress?).to eq false end @@ -1393,6 +1387,42 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(subject.work_in_progress?).to eq false end end + + it 'removes only WIP prefix from the MR title' do + subject.title = 'WIP: Implement feature called WIP' + + expect(subject.wipless_title).to eq 'Implement feature called WIP' + end + + it 'removes only draft prefix from the MR title' do + subject.title = 'Draft: Implement feature called draft' + + expect(subject.wipless_title).to eq 'Implement feature called draft' + end + + it 'does not remove WIP in the middle of the title' do + subject.title = 'Something with WIP in the middle' + + expect(subject.wipless_title).to eq subject.title + end + + it 'does not remove Draft in the middle of the title' do + subject.title = 'Something with Draft in the middle' + + expect(subject.wipless_title).to eq subject.title + end + + it 'does not remove WIP at the end of the title' do + subject.title = 'Something ends with WIP' + + expect(subject.wipless_title).to eq subject.title + end + + it 'does not remove Draft at the end of the title' do + subject.title = 'Something ends with Draft' + + expect(subject.wipless_title).to eq subject.title + end end describe "#wip_title" do @@ -2023,14 +2053,6 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) } it { is_expected.to be_truthy } - - 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 head pipeline does not have a codequality report' do @@ -3857,17 +3879,7 @@ RSpec.describe MergeRequest, factory_default: :keep do 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 + it { is_expected.to be_truthy } end context 'when service class is different' do diff --git a/spec/models/namespace/admin_note_spec.rb b/spec/models/namespace/admin_note_spec.rb new file mode 100644 index 00000000000..65ba1f61416 --- /dev/null +++ b/spec/models/namespace/admin_note_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespace::AdminNote, type: :model do + let!(:namespace) { create(:namespace) } + + describe 'associations' do + it { is_expected.to belong_to :namespace } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_length_of(:note).is_at_most(1000) } + end +end diff --git a/spec/models/namespace/traversal_hierarchy_spec.rb b/spec/models/namespace/traversal_hierarchy_spec.rb index 83e6d704640..b166d541171 100644 --- a/spec/models/namespace/traversal_hierarchy_spec.rb +++ b/spec/models/namespace/traversal_hierarchy_spec.rb @@ -43,21 +43,63 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model do end end + shared_examples 'locked update query' do + it 'locks query with FOR UPDATE' do + qr = ActiveRecord::QueryRecorder.new do + subject + end + expect(qr.count).to eq 1 + expect(qr.log.first).to match /FOR UPDATE/ + end + end + describe '#incorrect_traversal_ids' do - subject { described_class.new(root).incorrect_traversal_ids } + let!(:hierarchy) { described_class.new(root) } + + subject { hierarchy.incorrect_traversal_ids } + + before do + Namespace.update_all(traversal_ids: []) + end it { is_expected.to match_array Namespace.all } + + context 'when lock is true' do + subject { hierarchy.incorrect_traversal_ids(lock: true).load } + + it_behaves_like 'locked update query' + end end describe '#sync_traversal_ids!' do - let(:hierarchy) { described_class.new(root) } + let!(:hierarchy) { described_class.new(root) } - before do - hierarchy.sync_traversal_ids! - root.reload - end + subject { hierarchy.sync_traversal_ids! } - it_behaves_like 'hierarchy with traversal_ids' it { expect(hierarchy.incorrect_traversal_ids).to be_empty } + + it_behaves_like 'hierarchy with traversal_ids' + it_behaves_like 'locked update query' + + context 'when deadlocked' do + before do + connection_double = double(:connection) + + allow(Namespace).to receive(:connection).and_return(connection_double) + allow(connection_double).to receive(:exec_query) { raise ActiveRecord::Deadlocked.new } + end + + it { expect { subject }.to raise_error(ActiveRecord::Deadlocked) } + + it 'increment db_deadlock counter' do + expect { subject rescue nil }.to change { db_deadlock_total('Namespace#sync_traversal_ids!') }.by(1) + end + end + end + + def db_deadlock_total(source) + Gitlab::Metrics + .counter(:db_deadlock, 'Counts the times we have deadlocked in the database') + .get(source: source) end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 59b7510051f..14d28be8d43 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -66,5 +66,36 @@ RSpec.describe NamespaceSetting, type: :model do end end end + + describe '#allow_resource_access_token_creation_for_group' do + let(:settings) { group.namespace_settings } + + context 'group is top-level group' do + let(:group) { create(:group) } + + it 'is valid' do + settings.resource_access_token_creation_allowed = false + + expect(settings).to be_valid + end + end + + context 'group is a subgroup' do + let(:group) { create(:group, parent: create(:group)) } + + it 'is invalid when resource access token creation is not enabled' do + settings.resource_access_token_creation_allowed = false + + expect(settings).to be_invalid + expect(group.namespace_settings.errors.messages[:resource_access_token_creation_allowed]).to include("is not allowed since the group is not top-level group.") + end + + it 'is valid when resource access tokens are enabled' do + settings.resource_access_token_creation_allowed = true + + expect(settings).to be_valid + end + end + end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 65d787d334b..96ecc9836d4 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Namespace do it { is_expected.to have_many :custom_emoji } it { is_expected.to have_one :package_setting_relation } it { is_expected.to have_one :onboarding_progress } + it { is_expected.to have_one :admin_note } end describe 'validations' do @@ -154,6 +155,33 @@ RSpec.describe Namespace do end end + describe 'scopes' do + let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } + let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } + let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace1) } + let_it_be(:namespace2sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace2) } + + describe '.by_parent' do + it 'includes correct namespaces' do + expect(described_class.by_parent(namespace1.id)).to eq([namespace1sub]) + expect(described_class.by_parent(namespace2.id)).to eq([namespace2sub]) + expect(described_class.by_parent(nil)).to match_array([namespace, namespace1, namespace2]) + end + end + + describe '.filter_by_path' do + it 'includes correct namespaces' do + expect(described_class.filter_by_path(namespace1.path)).to eq([namespace1]) + expect(described_class.filter_by_path(namespace2.path)).to eq([namespace2]) + expect(described_class.filter_by_path('sub-namespace')).to match_array([namespace1sub, namespace2sub]) + end + + it 'filters case-insensitive' do + expect(described_class.filter_by_path(namespace1.path.upcase)).to eq([namespace1]) + end + end + end + describe 'delegate' do it { is_expected.to delegate_method(:name).to(:owner).with_prefix.with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:avatar_url).to(:owner).with_arguments(allow_nil: true) } @@ -168,27 +196,19 @@ RSpec.describe Namespace do describe 'inclusions' do it { is_expected.to include_module(Gitlab::VisibilityLevel) } it { is_expected.to include_module(Namespaces::Traversal::Recursive) } + it { is_expected.to include_module(Namespaces::Traversal::Linear) } 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 + context 'traversal_ids on create' do + context 'default traversal_ids' do + let(:namespace) { build(:namespace) } - 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 + before do + namespace.save! + namespace.reload end + + it { expect(namespace.traversal_ids).to eq [namespace.id] } end end @@ -859,7 +879,51 @@ RSpec.describe Namespace do end end - it_behaves_like 'recursive namespace traversal' + describe '#use_traversal_ids?' do + let_it_be(:namespace) { build(:namespace) } + + subject { namespace.use_traversal_ids? } + + context 'when use_traversal_ids feature flag is true' do + before do + stub_feature_flags(use_traversal_ids: true) + end + + it { is_expected.to eq true } + end + + context 'when use_traversal_ids feature flag is false' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + it { is_expected.to eq false } + end + end + + context 'when use_traversal_ids feature flag is true' do + it_behaves_like 'namespace traversal' + + describe '#self_and_descendants' do + subject { namespace.self_and_descendants } + + it { expect(subject.to_sql).to include 'traversal_ids @>' } + end + end + + context 'when use_traversal_ids feature flag is false' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + it_behaves_like 'namespace traversal' + + describe '#self_and_descendants' do + subject { namespace.self_and_descendants } + + it { expect(subject.to_sql).not_to include 'traversal_ids @>' } + end + end describe '#users_with_descendants' do let(:user_a) { create(:user) } @@ -897,24 +961,10 @@ 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]) } - 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]) - it 'queries for the namespace and its descendants' do - expect(namespace.all_projects).to match_array([project1, project2]) - end - end - - 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 - - it 'queries for the namespace and its descendants' do - expect(namespace.all_projects).to match_array([project1, project2]) - end + namespace.all_projects end end @@ -1085,21 +1135,42 @@ RSpec.describe Namespace do end describe '#root_ancestor' do - let!(:root_group) { create(:group) } + context 'with persisted root group' do + let!(:root_group) { create(:group) } - it 'returns root_ancestor for root group without a query' do - expect { root_group.root_ancestor }.not_to exceed_query_limit(0) + it 'returns root_ancestor for root group without a query' do + expect { root_group.root_ancestor }.not_to exceed_query_limit(0) + end + + it 'returns the top most ancestor' do + nested_group = create(:group, parent: root_group) + deep_nested_group = create(:group, parent: nested_group) + very_deep_nested_group = create(:group, parent: deep_nested_group) + + expect(root_group.root_ancestor).to eq(root_group) + expect(nested_group.root_ancestor).to eq(root_group) + expect(deep_nested_group.root_ancestor).to eq(root_group) + expect(very_deep_nested_group.root_ancestor).to eq(root_group) + end end - it 'returns the top most ancestor' do - nested_group = create(:group, parent: root_group) - deep_nested_group = create(:group, parent: nested_group) - very_deep_nested_group = create(:group, parent: deep_nested_group) + context 'with not persisted root group' do + let!(:root_group) { build(:group) } - expect(root_group.root_ancestor).to eq(root_group) - expect(nested_group.root_ancestor).to eq(root_group) - expect(deep_nested_group.root_ancestor).to eq(root_group) - expect(very_deep_nested_group.root_ancestor).to eq(root_group) + it 'returns root_ancestor for root group without a query' do + expect { root_group.root_ancestor }.not_to exceed_query_limit(0) + end + + it 'returns the top most ancestor' do + nested_group = build(:group, parent: root_group) + deep_nested_group = build(:group, parent: nested_group) + very_deep_nested_group = build(:group, parent: deep_nested_group) + + expect(root_group.root_ancestor).to eq(root_group) + expect(nested_group.root_ancestor).to eq(root_group) + expect(deep_nested_group.root_ancestor).to eq(root_group) + expect(very_deep_nested_group.root_ancestor).to eq(root_group) + end end end @@ -1372,6 +1443,12 @@ RSpec.describe Namespace do end end + describe '#paid?' do + it 'returns false for a root namespace with a free plan' do + expect(namespace.paid?).to eq(false) + end + end + describe '#shared_runners_setting' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 590acfc0ac1..992b2246f01 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -20,6 +20,7 @@ RSpec.describe Note do it { is_expected.to include_module(Participable) } it { is_expected.to include_module(Mentionable) } it { is_expected.to include_module(Awardable) } + it { is_expected.to include_module(Sortable) } end describe 'validation' do @@ -856,6 +857,22 @@ RSpec.describe Note do end end + describe '.simple_sorts' do + it 'does not contain name sorts' do + expect(described_class.simple_sorts.grep(/name/)).to be_empty + end + end + + describe '.cherry_picked_merge_requests' do + it 'returns merge requests that match the given merge commit' do + note = create(:track_mr_picking_note, commit_id: '456abc') + + create(:track_mr_picking_note, project: create(:project), commit_id: '456def') + + expect(MergeRequest.id_in(described_class.cherry_picked_merge_requests('456abc'))).to eq([note.noteable]) + end + end + describe '#for_project_snippet?' do it 'returns true for a project snippet note' do expect(build(:note_on_project_snippet).for_project_snippet?).to be true @@ -1322,7 +1339,7 @@ RSpec.describe Note do let_it_be(:note1) { create(:note, note: 'Test 345') } let_it_be(:note2) { create(:note, note: 'Test 789') } - describe '#for_note_or_capitalized_note' do + describe '.for_note_or_capitalized_note' do it 'returns the expected matching note' do notes = described_class.for_note_or_capitalized_note('Test 345') @@ -1344,7 +1361,7 @@ RSpec.describe Note do end end - describe '#like_note_or_capitalized_note' do + describe '.like_note_or_capitalized_note' do it 'returns the expected matching note' do notes = described_class.like_note_or_capitalized_note('Test 345') @@ -1367,69 +1384,69 @@ RSpec.describe Note do expect(notes.second.id).to eq(note2.id) end end + end - describe '#noteable_assignee_or_author' do - let(:user) { create(:user) } - let(:noteable) { create(:issue) } - let(:note) { create(:note, project: noteable.project, noteable: noteable) } + describe '#noteable_assignee_or_author?' do + let(:user) { create(:user) } + let(:noteable) { create(:issue) } + let(:note) { create(:note, project: noteable.project, noteable: noteable) } - subject { note.noteable_assignee_or_author?(user) } + subject { note.noteable_assignee_or_author?(user) } - shared_examples 'assignee check' do - context 'when the provided user is one of the assignees' do - before do - note.noteable.update(assignees: [user, create(:user)]) - end + shared_examples 'assignee check' do + context 'when the provided user is one of the assignees' do + before do + note.noteable.update(assignees: [user, create(:user)]) + end - it 'returns true' do - expect(subject).to be_truthy - end + it 'returns true' do + expect(subject).to be_truthy end end + end - shared_examples 'author check' do - context 'when the provided user is the author' do - before do - note.noteable.update(author: user) - end - - it 'returns true' do - expect(subject).to be_truthy - end + shared_examples 'author check' do + context 'when the provided user is the author' do + before do + note.noteable.update(author: user) end - context 'when the provided user is neither author nor assignee' do - it 'returns true' do - expect(subject).to be_falsey - end + it 'returns true' do + expect(subject).to be_truthy end end - context 'when user is nil' do - let(:user) { nil } - - it 'returns false' do + context 'when the provided user is neither author nor assignee' do + it 'returns true' do expect(subject).to be_falsey end end + end + + context 'when user is nil' do + let(:user) { nil } - context 'when noteable is an issue' do - it_behaves_like 'author check' - it_behaves_like 'assignee check' + it 'returns false' do + expect(subject).to be_falsey end + end - context 'when noteable is a merge request' do - let(:noteable) { create(:merge_request) } + context 'when noteable is an issue' do + it_behaves_like 'author check' + it_behaves_like 'assignee check' + end - it_behaves_like 'author check' - it_behaves_like 'assignee check' - end + context 'when noteable is a merge request' do + let(:noteable) { create(:merge_request) } - context 'when noteable is a snippet' do - let(:noteable) { create(:personal_snippet) } + it_behaves_like 'author check' + it_behaves_like 'assignee check' + end - it_behaves_like 'author check' - end + context 'when noteable is a snippet' do + let(:noteable) { create(:personal_snippet) } + + it_behaves_like 'author check' end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 4ef5ab7af48..010b7455f85 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -200,4 +200,18 @@ RSpec.describe NotificationSetting do subject.email_events end end + + describe '#order_by_id_asc' do + let_it_be(:project) { create(:project) } + let_it_be(:other_project) { create(:project) } + let_it_be(:notification_setting_1) { create(:notification_setting, project: project) } + let_it_be(:notification_setting_2) { create(:notification_setting, project: other_project) } + let_it_be(:notification_setting_3) { create(:notification_setting, project: project) } + + let(:ids) { [notification_setting_1, notification_setting_2, notification_setting_3].map(&:id) } + + subject(:ordered_records) { described_class.where(id: ids, source: project).order_by_id_asc } + + it { is_expected.to eq([notification_setting_1, notification_setting_3]) } + end end diff --git a/spec/models/packages/debian/file_entry_spec.rb b/spec/models/packages/debian/file_entry_spec.rb new file mode 100644 index 00000000000..7aa16bc0cce --- /dev/null +++ b/spec/models/packages/debian/file_entry_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::FileEntry, type: :model do + let_it_be(:package_file) { create(:debian_package_file, :dsc) } + + let(:filename) { 'sample_1.2.3~alpha2.dsc' } + let(:size) { 671 } + let(:md5sum) { '3b0817804f669e16cdefac583ad88f0e' } + let(:section) { 'libs' } + let(:priority) { 'optional' } + let(:sha1sum) { '32ecbd674f0bfd310df68484d87752490685a8d6' } + let(:sha256sum) { '844f79825b7e8aaa191e514b58a81f9ac1e58e2180134b0c9512fa66d896d7ba' } + + let(:file_entry) do + described_class.new( + filename: filename, + size: size, + md5sum: md5sum, + section: section, + priority: priority, + sha1sum: sha1sum, + sha256sum: sha256sum, + package_file: package_file + ) + end + + subject { file_entry } + + describe 'validations' do + it { is_expected.to be_valid } + + describe '#filename' do + it { is_expected.to validate_presence_of(:filename) } + it { is_expected.not_to allow_value('Hé').for(:filename) } + end + + describe '#size' do + it { is_expected.to validate_presence_of(:size) } + end + + describe '#md5sum' do + it { is_expected.to validate_presence_of(:md5sum) } + it { is_expected.not_to allow_value('12345678901234567890123456789012').for(:md5sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 3b0817804f669e16cdefac583ad88f0e != 12345678901234567890123456789012') } + end + + describe '#section' do + it { is_expected.to validate_presence_of(:section) } + end + + describe '#priority' do + it { is_expected.to validate_presence_of(:priority) } + end + + describe '#sha1sum' do + it { is_expected.to validate_presence_of(:sha1sum) } + it { is_expected.not_to allow_value('1234567890123456789012345678901234567890').for(:sha1sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 32ecbd674f0bfd310df68484d87752490685a8d6 != 1234567890123456789012345678901234567890') } + end + + describe '#sha256sum' do + it { is_expected.to validate_presence_of(:sha256sum) } + it { is_expected.not_to allow_value('1234567890123456789012345678901234567890123456789012345678901234').for(:sha256sum).with_message('mismatch for sample_1.2.3~alpha2.dsc: 844f79825b7e8aaa191e514b58a81f9ac1e58e2180134b0c9512fa66d896d7ba != 1234567890123456789012345678901234567890123456789012345678901234') } + end + + describe '#package_file' do + it { is_expected.to validate_presence_of(:package_file) } + end + end + + describe '#component' do + subject { file_entry.component } + + context 'without section' do + let(:section) { nil } + + it { is_expected.to eq 'main' } + end + + context 'with empty section' do + let(:section) { '' } + + it { is_expected.to eq 'main' } + end + + context 'with ruby section' do + let(:section) { 'ruby' } + + it { is_expected.to eq 'main' } + end + + context 'with contrib/ruby section' do + let(:section) { 'contrib/ruby' } + + it { is_expected.to eq 'contrib' } + end + end +end diff --git a/spec/models/packages/dependency_spec.rb b/spec/models/packages/dependency_spec.rb index fa6b0fd1848..4437cad46cd 100644 --- a/spec/models/packages/dependency_spec.rb +++ b/spec/models/packages/dependency_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Packages::Dependency, type: :model do context 'with too big parameter' do let(:size) { (Packages::Dependency::MAX_CHUNKED_QUERIES_COUNT * chunk_size) + 1 } - let(:names_and_version_patterns) { Hash[(1..size).map { |v| [v, v] }] } + let(:names_and_version_patterns) { (1..size).to_h { |v| [v, v] } } it { expect { subject }.to raise_error(ArgumentError, 'Too many names_and_version_patterns') } end diff --git a/spec/models/packages/go/module_version_spec.rb b/spec/models/packages/go/module_version_spec.rb index c4c6a07d9e9..7fa416d8537 100644 --- a/spec/models/packages/go/module_version_spec.rb +++ b/spec/models/packages/go/module_version_spec.rb @@ -3,19 +3,9 @@ require 'spec_helper' RSpec.describe Packages::Go::ModuleVersion, type: :model do - let_it_be(:user) { create :user } - let_it_be(:project) { create :project_empty_repo, creator: user, path: 'my-go-lib' } - let_it_be(:mod) { create :go_module, project: project } + include_context 'basic Go module' - before :all do - create :go_module_commit, :files, project: project, tag: 'v1.0.0', files: { 'README.md' => 'Hi' } - create :go_module_commit, :module, project: project, tag: 'v1.0.1' - create :go_module_commit, :package, project: project, tag: 'v1.0.2', path: 'pkg' - create :go_module_commit, :module, project: project, tag: 'v1.0.3', name: 'mod' - create :go_module_commit, :files, project: project, files: { 'y.go' => "package a\n" } - create :go_module_commit, :module, project: project, name: 'v2' - create :go_module_commit, :files, project: project, tag: 'v2.0.0', files: { 'v2/x.go' => "package a\n" } - end + let_it_be(:mod) { create :go_module, project: project } shared_examples '#files' do |desc, *entries| it "returns #{desc}" do diff --git a/spec/models/packages/maven/metadatum_spec.rb b/spec/models/packages/maven/metadatum_spec.rb index 94a0e558985..0000543cb18 100644 --- a/spec/models/packages/maven/metadatum_spec.rb +++ b/spec/models/packages/maven/metadatum_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Packages::Maven::Metadatum, type: :model do 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 } + subject { described_class.for_package_ids(package.id).order_created } it { is_expected.to eq([metadatum1, metadatum2, metadatum3, metadatum4]) } end @@ -64,10 +64,20 @@ RSpec.describe Packages::Maven::Metadatum, type: :model do 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 } + subject { described_class.for_package_ids(package.id).pluck_app_name } it { is_expected.to match_array([metadatum1, metadatum2, metadatum3].map(&:app_name)) } end + + describe '.with_path' do + let_it_be(:metadatum1) { create(:maven_metadatum, package: package, path: 'one') } + let_it_be(:metadatum2) { create(:maven_metadatum, package: package, path: 'two') } + let_it_be(:metadatum3) { create(:maven_metadatum, package: package, path: 'three') } + + subject { described_class.with_path('two') } + + it { is_expected.to match_array([metadatum2]) } + end end end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 82997acee3f..cf52749a186 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -99,6 +99,34 @@ RSpec.describe Packages::Package, type: :model do end end + describe '.for_projects' do + let_it_be(:package1) { create(:maven_package) } + let_it_be(:package2) { create(:maven_package) } + let_it_be(:package3) { create(:maven_package) } + + let(:projects) { ::Project.id_in([package1.project_id, package2.project_id]) } + + subject { described_class.for_projects(projects.select(:id)) } + + it 'returns package1 and package2' do + expect(projects).not_to receive(:any?) + + expect(subject).to match_array([package1, package2]) + end + + context 'with maven_packages_group_level_improvements disabled' do + before do + stub_feature_flags(maven_packages_group_level_improvements: false) + end + + it 'returns package1 and package2' do + expect(projects).to receive(:any?).and_call_original + + expect(subject).to match_array([package1, package2]) + end + end + end + describe 'validations' do subject { build(:package) } @@ -339,7 +367,14 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to validate_presence_of(:version) } it { is_expected.to allow_value('1.2.3').for(:version) } it { is_expected.to allow_value('1.3.350').for(:version) } - it { is_expected.not_to allow_value('1.3.350-20201230123456').for(:version) } + it { is_expected.to allow_value('1.3.350-20201230123456').for(:version) } + it { is_expected.to allow_value('1.2.3-rc1').for(:version) } + it { is_expected.to allow_value('1.2.3g').for(:version) } + it { is_expected.to allow_value('1.2').for(:version) } + it { is_expected.to allow_value('1.2.bananas').for(:version) } + it { is_expected.to allow_value('v1.2.4-build').for(:version) } + it { is_expected.to allow_value('d50d836eb3de6177ce6c7a5482f27f9c2c84b672').for(:version) } + it { is_expected.to allow_value('this_is_a_string_only').for(:version) } it { is_expected.not_to allow_value('..1.2.3').for(:version) } it { is_expected.not_to allow_value(' 1.2.3').for(:version) } it { is_expected.not_to allow_value("1.2.3 \r\t").for(:version) } @@ -621,10 +656,12 @@ RSpec.describe Packages::Package, type: :model do describe '.displayable' do let_it_be(:hidden_package) { create(:maven_package, :hidden) } let_it_be(:processing_package) { create(:maven_package, :processing) } + let_it_be(:error_package) { create(:maven_package, :error) } subject { described_class.displayable } - it 'does not include hidden packages', :aggregate_failures do + it 'does not include non-displayable packages', :aggregate_failures do + is_expected.to include(error_package) is_expected.not_to include(hidden_package) is_expected.not_to include(processing_package) end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 9e65635da91..f2659771a49 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Pages::LookupPath do it 'return nil when legacy storage is disabled and there is no deployment' do stub_feature_flags(pages_serve_from_legacy_storage: false) expect(Gitlab::ErrorTracking).to receive(:track_exception) - .with(described_class::LegacyStorageDisabledError) + .with(described_class::LegacyStorageDisabledError, project_id: project.id) .and_call_original expect(source).to eq(nil) @@ -136,14 +136,6 @@ RSpec.describe Pages::LookupPath do ) end end - - context 'when pages_serve_from_migrated_zip feature flag is disabled' do - before do - stub_feature_flags(pages_serve_from_migrated_zip: false) - end - - include_examples 'uses disk storage' - end end end end diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index 029eb8e513a..a27d836e2c2 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe PagesDeployment do + let_it_be(:project) { create(:project) } + describe 'associations' do it { is_expected.to belong_to(:project).required } it { is_expected.to belong_to(:ci_build).optional } @@ -28,7 +30,6 @@ RSpec.describe PagesDeployment do describe '.migrated_from_legacy_storage' do it 'only returns migrated deployments' do - project = create(:project) migrated_deployment = create_migrated_deployment(project) # create one other deployment create(:pages_deployment, project: project) @@ -37,6 +38,27 @@ RSpec.describe PagesDeployment do end end + context 'with deployments stored locally and remotely' do + before do + stub_pages_object_storage(::Pages::DeploymentUploader) + end + + let!(:remote_deployment) { create(:pages_deployment, project: project, file_store: ::ObjectStorage::Store::REMOTE) } + let!(:local_deployment) { create(:pages_deployment, project: project, file_store: ::ObjectStorage::Store::LOCAL) } + + describe '.with_files_stored_locally' do + it 'only returns deployments with files stored locally' do + expect(described_class.with_files_stored_locally).to contain_exactly(local_deployment) + end + end + + describe '.with_files_stored_remotely' do + it 'only returns deployments with files stored remotely' do + expect(described_class.with_files_stored_remotely).to contain_exactly(remote_deployment) + end + end + end + describe '#migrated?' do it 'returns false for normal deployment' do deployment = create(:pages_deployment) @@ -45,7 +67,6 @@ RSpec.describe PagesDeployment do end it 'returns true for migrated deployment' do - project = create(:project) deployment = create_migrated_deployment(project) expect(deployment.migrated?).to eq(true) @@ -67,7 +88,6 @@ RSpec.describe PagesDeployment do end describe 'default for file_store' do - let(:project) { create(:project) } let(:deployment) do filepath = Rails.root.join("spec/fixtures/pages.zip") diff --git a/spec/models/preloaders/labels_preloader_spec.rb b/spec/models/preloaders/labels_preloader_spec.rb new file mode 100644 index 00000000000..94de00bb94c --- /dev/null +++ b/spec/models/preloaders/labels_preloader_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::LabelsPreloader do + let_it_be(:user) { create(:user) } + + shared_examples 'an efficient database query' do + let(:subscriptions) { labels.each { |l| create(:subscription, subscribable: l, project: l.project, user: user) }} + + it 'does not make n+1 queries' do + first_label = labels_with_preloaded_data.first + clean_labels = labels_with_preloaded_data + + expect { access_data(clean_labels) }.to issue_same_number_of_queries_as { access_data([first_label]) } + end + end + + context 'project labels' do + let_it_be(:projects) { create_list(:project, 3, :public, :repository) } + let_it_be(:labels) { projects.each { |p| create(:label, project: p) } } + + it_behaves_like 'an efficient database query' + end + + context 'group labels' do + let_it_be(:groups) { create_list(:group, 3) } + let_it_be(:labels) { groups.each { |g| create(:group_label, group: g) } } + + it_behaves_like 'an efficient database query' + end + + private + + def labels_with_preloaded_data + l = Label.where(id: labels.map(&:id)) + described_class.new(l, user).preload_all + l + end + + def access_data(labels) + labels.each do |label| + if label.is_a?(ProjectLabel) + label.project.project_feature + label.lazy_subscription(user, label.project) + elsif label.is_a?(GroupLabel) + label.group.route + label.lazy_subscription(user) + end + end + end +end diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb new file mode 100644 index 00000000000..16e699b7e0e --- /dev/null +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + let_it_be(:project_3) { create(:project) } + + let(:projects) { [project_1, project_2, project_3] } + + before do + project_1.add_developer(user) + project_2.add_developer(user) + end + + context 'preload maximum access level to avoid querying project_authorizations', :request_store do + it 'avoids N+1 queries', :request_store do + Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, user).execute + + query_count = ActiveRecord::QueryRecorder.new do + projects.each { |project| user.can?(:read_project, project) } + end.count + + expect(query_count).to eq(0) + end + + it 'runs N queries without preloading' do + query_count = ActiveRecord::QueryRecorder.new do + projects.each { |project| user.can?(:read_project, project) } + end.count + + expect(query_count).to eq(projects.size) + end + end +end diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb index cd70602fc4d..4baa59535e4 100644 --- a/spec/models/project_feature_usage_spec.rb +++ b/spec/models/project_feature_usage_spec.rb @@ -24,21 +24,91 @@ RSpec.describe ProjectFeatureUsage, type: :model do subject { project.feature_usage } - it 'logs Jira DVCS Cloud last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage + context 'when the feature usage has not been created yet' do + it 'logs Jira DVCS Cloud last sync' do + freeze_time do + subject.log_jira_dvcs_integration_usage - expect(subject.jira_dvcs_server_last_sync_at).to be_nil - expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) + expect(subject.jira_dvcs_server_last_sync_at).to be_nil + expect(subject.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.current) + end + end + + it 'logs Jira DVCS Server last sync' do + freeze_time do + subject.log_jira_dvcs_integration_usage(cloud: false) + + expect(subject.jira_dvcs_server_last_sync_at).to be_like_time(Time.current) + expect(subject.jira_dvcs_cloud_last_sync_at).to be_nil + end end end - it 'logs Jira DVCS Server last sync' do - freeze_time do - subject.log_jira_dvcs_integration_usage(cloud: false) + context 'when the feature usage already exists' do + let(:today) { Time.current.beginning_of_day } + let(:project) { create(:project) } + + subject { project.feature_usage } - expect(subject.jira_dvcs_server_last_sync_at).to be_like_time(Time.current) - expect(subject.jira_dvcs_cloud_last_sync_at).to be_nil + where(:cloud, :timestamp_field) do + [ + [true, :jira_dvcs_cloud_last_sync_at], + [false, :jira_dvcs_server_last_sync_at] + ] + end + + with_them do + context 'when Jira DVCS Cloud last sync has not been logged' do + before do + travel_to today - 3.days do + subject.log_jira_dvcs_integration_usage(cloud: !cloud) + end + end + + it 'logs Jira DVCS Cloud last sync' do + freeze_time do + subject.log_jira_dvcs_integration_usage(cloud: cloud) + + expect(subject.reload.send(timestamp_field)).to be_like_time(Time.current) + end + end + end + + context 'when Jira DVCS Cloud last sync was logged today' do + let(:last_updated) { today + 1.hour } + + before do + travel_to last_updated do + subject.log_jira_dvcs_integration_usage(cloud: cloud) + end + end + + it 'does not log Jira DVCS Cloud last sync' do + travel_to today + 2.hours do + subject.log_jira_dvcs_integration_usage(cloud: cloud) + + expect(subject.reload.send(timestamp_field)).to be_like_time(last_updated) + end + end + end + + context 'when Jira DVCS Cloud last sync was logged yesterday' do + let(:last_updated) { today - 2.days } + + before do + travel_to last_updated do + subject.log_jira_dvcs_integration_usage(cloud: cloud) + end + end + + it 'logs Jira DVCS Cloud last sync' do + travel_to today + 1.hour do + subject.log_jira_dvcs_integration_usage(cloud: cloud) + + expect(subject.reload.send(timestamp_field)).to be_like_time(today + 1.hour) + end + end + end end end diff --git a/spec/models/project_services/chat_message/alert_message_spec.rb b/spec/models/project_services/chat_message/alert_message_spec.rb index 927c5dffe77..4d400990789 100644 --- a/spec/models/project_services/chat_message/alert_message_spec.rb +++ b/spec/models/project_services/chat_message/alert_message_spec.rb @@ -6,6 +6,7 @@ RSpec.describe ChatMessage::AlertMessage do subject { described_class.new(args) } let_it_be(:start_time) { Time.current } + let(:alert) { create(:alert_management_alert, started_at: start_time) } let(:args) do diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index 02b266e4fae..71cfe3ff45b 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -16,7 +16,7 @@ RSpec.describe ChatMessage::MergeMessage do project_url: 'http://somewhere.com', object_attributes: { - title: "Merge Request title\nSecond line", + title: "Merge request title\nSecond line", id: 10, iid: 100, assignee_id: 1, @@ -35,7 +35,7 @@ RSpec.describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') + 'Test User (test.user) opened merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -46,7 +46,7 @@ RSpec.describe ChatMessage::MergeMessage do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') + 'Test User (test.user) closed merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -60,12 +60,12 @@ RSpec.describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened merge request [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') + 'Test User (test.user) opened merge request [!100 *Merge request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ - title: 'Merge Request opened by Test User (test.user)', + title: 'Merge request opened by Test User (test.user)', subtitle: 'in [project_name](http://somewhere.com)', - text: '[!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100)', + text: '[!100 *Merge request title*](http://somewhere.com/-/merge_requests/100)', image: 'http://someavatar.com' }) end @@ -78,12 +78,12 @@ RSpec.describe ChatMessage::MergeMessage do it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed merge request [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') + 'Test User (test.user) closed merge request [!100 *Merge request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ - title: 'Merge Request closed by Test User (test.user)', + title: 'Merge request closed by Test User (test.user)', subtitle: 'in [project_name](http://somewhere.com)', - text: '[!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100)', + text: '[!100 *Merge request title*](http://somewhere.com/-/merge_requests/100)', image: 'http://someavatar.com' }) end @@ -97,7 +97,7 @@ RSpec.describe ChatMessage::MergeMessage do it 'returns a message regarding completed approval of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) approved merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> '\ + 'Test User (test.user) approved merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> '\ 'in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end @@ -110,7 +110,7 @@ RSpec.describe ChatMessage::MergeMessage do it 'returns a message regarding revocation of completed approval of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) unapproved merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> '\ + 'Test User (test.user) unapproved merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> '\ 'in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end @@ -123,7 +123,7 @@ RSpec.describe ChatMessage::MergeMessage do it 'returns a message regarding added approval of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) added their approval to merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> '\ + 'Test User (test.user) added their approval to merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> '\ 'in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end @@ -136,7 +136,7 @@ RSpec.describe ChatMessage::MergeMessage do it 'returns a message regarding revoking approval of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) removed their approval from merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> '\ + 'Test User (test.user) removed their approval from merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge request title*> '\ 'in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb index 6954a72f9c1..c5927503eec 100644 --- a/spec/models/project_services/emails_on_push_service_spec.rb +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe EmailsOnPushService do + let_it_be(:project) { create_default(:project).freeze } + describe 'Validations' do context 'when service is active' do before do @@ -19,6 +21,42 @@ RSpec.describe EmailsOnPushService do it { is_expected.not_to validate_presence_of(:recipients) } end + + describe 'validates number of recipients' do + before do + stub_const("#{described_class}::RECIPIENTS_LIMIT", 2) + end + + subject(:service) { described_class.new(project: project, recipients: recipients, active: true) } + + context 'valid number of recipients' do + let(:recipients) { 'foo@bar.com duplicate@example.com Duplicate@example.com invalid-email' } + + it 'does not count duplicates and invalid emails' do + is_expected.to be_valid + end + end + + context 'invalid number of recipients' do + let(:recipients) { 'foo@bar.com bar@foo.com bob@gitlab.com' } + + it { is_expected.not_to be_valid } + + it 'adds an error message' do + service.valid? + + expect(service.errors).to contain_exactly('Recipients can\'t exceed 2') + end + + context 'when service is not active' do + before do + service.active = false + end + + it { is_expected.to be_valid } + end + end + end end describe '.new' do @@ -39,6 +77,14 @@ RSpec.describe EmailsOnPushService do end end + describe '.valid_recipients' do + let(:recipients) { '<invalid> foobar Valid@recipient.com Dup@lica.te dup@lica.te Dup@Lica.te' } + + it 'removes invalid email addresses and removes duplicates by keeping the original capitalization' do + expect(described_class.valid_recipients(recipients)).to contain_exactly('Valid@recipient.com', 'Dup@lica.te') + end + end + describe '#execute' do let(:push_data) { { object_kind: 'push' } } let(:project) { create(:project, :repository) } diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 3fc39fd3266..b50fa1edbc3 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -82,11 +82,8 @@ RSpec.describe JiraService do subject(:fields) { service.fields } - it 'includes transition help link' do - transition_id_field = fields.find { |field| field[:name] == 'jira_issue_transition_id' } - - expect(transition_id_field[:title]).to eq('Jira workflow transition IDs') - expect(transition_id_field[:help]).to include('/help/user/project/integrations/jira') + it 'returns custom fields' do + expect(fields.pluck(:name)).to eq(%w[url api_url username password]) end end @@ -460,10 +457,10 @@ RSpec.describe JiraService do end context 'with options' do - let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}?expand=renderedFields" } + let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}?expand=renderedFields,transitions" } it 'calls the Jira API with the options to get the issue' do - jira_service.find_issue(issue_key, rendered_fields: true) + jira_service.find_issue(issue_key, rendered_fields: true, transitions: true) expect(WebMock).to have_requested(:get, issue_url) end @@ -494,7 +491,7 @@ RSpec.describe JiraService do end before do - allow(jira_service).to receive_messages(jira_issue_transition_id: '999') + jira_service.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. @@ -505,7 +502,7 @@ RSpec.describe JiraService do allow(closed_issue).to receive(:resolution).and_return(true) allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) - allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return('JIRA-123') + allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return(issue_key) allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) WebMock.stub_request(:get, issue_url).with(basic_auth: %w(jira-username jira-password)) @@ -664,6 +661,61 @@ RSpec.describe JiraService do ).once end + context 'when custom transition IDs are blank' do + before do + jira_service.jira_issue_transition_id = '' + end + + it 'does not transition the issue' do + close_issue + + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + + context 'when using automatic issue transitions' do + let(:transitions) do + [ + { id: '1' }, + { id: '2', to: { statusCategory: { key: 'new' } } }, + { id: '3', to: { statusCategory: { key: 'done' } } }, + { id: '4', to: { statusCategory: { key: 'done' } } } + ] + end + + before do + jira_service.jira_issue_transition_automatic = true + + close_issue + end + + it 'uses the next transition with a status category of done' do + expect(WebMock).to have_requested(:post, transitions_url).with( + body: /"id":"3"/ + ).once + end + + context 'when no done transition is available' do + let(:transitions) do + [ + { id: '1', to: { statusCategory: { key: 'new' } } } + ] + end + + it 'does not attempt to transition' do + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + + context 'when no valid transitions are returned' do + let(:transitions) { 'foo' } + + it 'does not attempt to transition' do + expect(WebMock).not_to have_requested(:post, transitions_url) + end + end + end + context 'when using multiple transition ids' do before do allow(jira_service).to receive_messages(jira_issue_transition_id: '1,2,3') @@ -738,6 +790,7 @@ RSpec.describe JiraService do describe '#create_cross_reference_note' do let_it_be(:user) { build_stubbed(:user) } + let(:jira_issue) { ExternalIssue.new('JIRA-123', project) } subject { jira_service.create_cross_reference_note(jira_issue, resource, user) } @@ -902,4 +955,22 @@ RSpec.describe JiraService do end end end + + describe '#issue_transition_enabled?' do + it 'returns true if automatic transitions are enabled' do + jira_service.jira_issue_transition_automatic = true + + expect(jira_service.issue_transition_enabled?).to be(true) + end + + it 'returns true if custom transitions are set' do + jira_service.jira_issue_transition_id = '1, 2, 3' + + expect(jira_service.issue_transition_enabled?).to be(true) + end + + it 'returns false if automatic and custom transitions are disabled' do + expect(jira_service.issue_transition_enabled?).to be(false) + end + end end diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb index 46194efcb3d..a698d3fce5f 100644 --- a/spec/models/project_services/jira_tracker_data_spec.rb +++ b/spec/models/project_services/jira_tracker_data_spec.rb @@ -11,20 +11,9 @@ RSpec.describe JiraTrackerData do it { is_expected.to define_enum_for(:deployment_type).with_values([:unknown, :server, :cloud]).with_prefix(:deployment) } end - describe 'proxy settings' do - it { is_expected.to validate_length_of(:proxy_address).is_at_most(2048) } - it { is_expected.to validate_length_of(:proxy_port).is_at_most(5) } - it { is_expected.to validate_length_of(:proxy_username).is_at_most(255) } - it { is_expected.to validate_length_of(:proxy_password).is_at_most(255) } - end - describe 'encrypted attributes' do subject { described_class.encrypted_attributes.keys } - it { - is_expected.to contain_exactly( - :api_url, :password, :proxy_address, :proxy_password, :proxy_port, :proxy_username, :url, :username - ) - } + it { is_expected.to contain_exactly(:api_url, :password, :url, :username) } end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 366c3f68e1d..37a6d49ff74 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -9,6 +9,7 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl include ReactiveCachingHelpers let_it_be_with_reload(:project) { create(:prometheus_project) } + let(:service) { project.prometheus_service } describe "Associations" do @@ -337,6 +338,7 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl context 'cluster belongs to projects group' do let_it_be(:group) { create(:group) } + let(:project) { create(:prometheus_project, group: group) } let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) } diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index aa5d92e5c61..688a59fcf09 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -32,6 +32,7 @@ RSpec.describe SlackService do 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 @@ -43,6 +44,7 @@ RSpec.describe SlackService do 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' @@ -56,6 +58,7 @@ RSpec.describe SlackService do 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' @@ -63,6 +66,7 @@ RSpec.describe SlackService do 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' @@ -70,6 +74,7 @@ RSpec.describe SlackService do 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' @@ -77,6 +82,7 @@ RSpec.describe SlackService do 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' @@ -93,6 +99,7 @@ RSpec.describe SlackService do 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' @@ -100,6 +107,7 @@ RSpec.describe SlackService do 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' diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 49d9fd56d70..12c17e699e3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -891,6 +891,7 @@ RSpec.describe Project, factory_default: :keep do describe '#get_issue' do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } + let!(:issue) { create(:issue, project: project) } before_all do @@ -1351,6 +1352,34 @@ RSpec.describe Project, factory_default: :keep do end end + describe '.with_remote_mirrors' do + let_it_be(:project) { create(:project, :repository) } + + subject { described_class.with_remote_mirrors } + + context 'when some remote mirrors are enabled for the project' do + let!(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } + + it "returns a project" do + is_expected.to eq([project]) + end + end + + context 'when some remote mirrors exists but disabled for the project' do + let!(:remote_mirror) { create(:remote_mirror, project: project, enabled: false) } + + it "returns a project" do + is_expected.to be_empty + end + end + + context 'when no remote mirrors exist for the project' do + it "returns an empty list" do + is_expected.to be_empty + end + end + end + describe '.with_active_jira_services' do it 'returns the correct project' do active_jira_service = create(:jira_service) @@ -1600,6 +1629,8 @@ RSpec.describe Project, factory_default: :keep do end describe '#any_active_runners?' do + subject { project.any_active_runners? } + context 'shared runners' do let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } @@ -1609,19 +1640,19 @@ RSpec.describe Project, factory_default: :keep do let(:shared_runners_enabled) { false } it 'has no runners available' do - expect(project.any_active_runners?).to be_falsey + is_expected.to be_falsey end it 'has a specific runner' do specific_runner - expect(project.any_active_runners?).to be_truthy + is_expected.to be_truthy end it 'has a shared runner, but they are prohibited to use' do shared_runner - expect(project.any_active_runners?).to be_falsey + is_expected.to be_falsey end it 'checks the presence of specific runner' do @@ -1643,7 +1674,7 @@ RSpec.describe Project, factory_default: :keep do it 'has a shared runner' do shared_runner - expect(project.any_active_runners?).to be_truthy + is_expected.to be_truthy end it 'checks the presence of shared runner' do @@ -1669,13 +1700,13 @@ RSpec.describe Project, factory_default: :keep do let(:group_runners_enabled) { false } it 'has no runners available' do - expect(project.any_active_runners?).to be_falsey + is_expected.to be_falsey end it 'has a group runner, but they are prohibited to use' do group_runner - expect(project.any_active_runners?).to be_falsey + is_expected.to be_falsey end end @@ -1685,7 +1716,7 @@ RSpec.describe Project, factory_default: :keep do it 'has a group runner' do group_runner - expect(project.any_active_runners?).to be_truthy + is_expected.to be_truthy end it 'checks the presence of group runner' do @@ -1703,6 +1734,126 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#any_online_runners?' do + subject { project.any_online_runners? } + + context 'shared runners' do + let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } + let(:specific_runner) { create(:ci_runner, :project, :online, projects: [project]) } + let(:shared_runner) { create(:ci_runner, :instance, :online) } + let(:offline_runner) { create(:ci_runner, :instance) } + + context 'for shared runners disabled' do + let(:shared_runners_enabled) { false } + + it 'has no runners available' do + is_expected.to be_falsey + end + + it 'has a specific runner' do + specific_runner + + is_expected.to be_truthy + end + + it 'has a shared runner, but they are prohibited to use' do + shared_runner + + is_expected.to be_falsey + end + + it 'checks the presence of specific runner' do + specific_runner + + expect(project.any_online_runners? { |runner| runner == specific_runner }).to be_truthy + end + + it 'returns false if match cannot be found' do + specific_runner + + expect(project.any_online_runners? { false }).to be_falsey + end + + it 'returns false if runner is offline' do + offline_runner + + is_expected.to be_falsey + end + end + + context 'for shared runners enabled' do + let(:shared_runners_enabled) { true } + + it 'has a shared runner' do + shared_runner + + is_expected.to be_truthy + end + + it 'checks the presence of shared runner' do + shared_runner + + expect(project.any_online_runners? { |runner| runner == shared_runner }).to be_truthy + end + + it 'returns false if match cannot be found' do + shared_runner + + expect(project.any_online_runners? { false }).to be_falsey + end + end + end + + context 'group runners' do + let(:project) { create(:project, group_runners_enabled: group_runners_enabled) } + let(:group) { create(:group, projects: [project]) } + let(:group_runner) { create(:ci_runner, :group, :online, groups: [group]) } + let(:offline_runner) { create(:ci_runner, :group, groups: [group]) } + + context 'for group runners disabled' do + let(:group_runners_enabled) { false } + + it 'has no runners available' do + is_expected.to be_falsey + end + + it 'has a group runner, but they are prohibited to use' do + group_runner + + is_expected.to be_falsey + end + end + + context 'for group runners enabled' do + let(:group_runners_enabled) { true } + + it 'has a group runner' do + group_runner + + is_expected.to be_truthy + end + + it 'has an offline group runner' do + offline_runner + + is_expected.to be_falsey + end + + it 'checks the presence of group runner' do + group_runner + + expect(project.any_online_runners? { |runner| runner == group_runner }).to be_truthy + end + + it 'returns false if match cannot be found' do + group_runner + + expect(project.any_online_runners? { false }).to be_falsey + end + end + end + end + describe '#shared_runners' do let!(:runner) { create(:ci_runner, :instance) } @@ -2378,6 +2529,7 @@ RSpec.describe Project, factory_default: :keep do describe '#latest_pipeline' do let_it_be(:project) { create(:project, :repository) } + let(:second_branch) { project.repository.branches[2] } let!(:pipeline_for_default_branch) do @@ -2842,6 +2994,7 @@ RSpec.describe Project, factory_default: :keep do describe '#emails_disabled?' do let_it_be(:namespace) { create(:namespace) } + let(:project) { build(:project, namespace: namespace, emails_disabled: false) } context 'emails disabled in group' do @@ -3162,6 +3315,7 @@ RSpec.describe Project, factory_default: :keep do describe '#ci_variables_for' do let_it_be(:project) { create(:project) } + let(:environment_scope) { '*' } let!(:ci_variable) do @@ -3649,49 +3803,74 @@ RSpec.describe Project, factory_default: :keep do end describe '#default_merge_request_target' do + let_it_be(:project) { create(:project, :public) } + + let!(:forked) { fork_project(project) } + + context 'when mr_default_target_self is set to true' do + it 'returns the current project' do + expect(forked.project_setting).to receive(:mr_default_target_self) + .and_return(true) + + expect(forked.default_merge_request_target).to eq(forked) + end + end + + context 'when merge request can not target upstream' do + it 'returns the current project' do + expect(forked).to receive(:mr_can_target_upstream?).and_return(false) + + expect(forked.default_merge_request_target).to eq(forked) + end + end + + context 'when merge request can target upstream' do + it 'returns the source project' do + expect(forked).to receive(:mr_can_target_upstream?).and_return(true) + + expect(forked.default_merge_request_target).to eq(project) + end + end + end + + describe '#mr_can_target_upstream?' do + let_it_be(:project) { create(:project, :public) } + + let!(:forked) { fork_project(project) } + context 'when forked from a more visible project' do - it 'returns the more restrictive project' do - project = create(:project, :public) - forked = fork_project(project) + it 'can not target the upstream project' do forked.visibility = Gitlab::VisibilityLevel::PRIVATE forked.save! expect(project.visibility).to eq 'public' expect(forked.visibility).to eq 'private' - expect(forked.default_merge_request_target).to eq(forked) + expect(forked.mr_can_target_upstream?).to be_falsey end end context 'when forked from a project with disabled merge requests' do - it 'returns the current project' do - project = create(:project, :merge_requests_disabled) - forked = fork_project(project) + it 'can not target the upstream project' do + project.project_feature + .update!(merge_requests_access_level: ProjectFeature::DISABLED) expect(forked.forked_from_project).to receive(:merge_requests_enabled?) .and_call_original - expect(forked.default_merge_request_target).to eq(forked) + expect(forked.mr_can_target_upstream?).to be_falsey end end context 'when forked from a project with enabled merge requests' do - it 'returns the source project' do - project = create(:project, :public) - forked = fork_project(project) - - expect(project.visibility).to eq 'public' - expect(forked.visibility).to eq 'public' - - expect(forked.default_merge_request_target).to eq(project) + it 'can target the upstream project' do + expect(forked.mr_can_target_upstream?).to be_truthy end end context 'when not forked' do - it 'returns the current project' do - project = build_stubbed(:project) - - expect(project.default_merge_request_target).to eq(project) + it 'can not target the upstream project' do + expect(project.mr_can_target_upstream?).to be_falsey end end end @@ -4007,6 +4186,7 @@ RSpec.describe Project, factory_default: :keep do include ProjectHelpers let_it_be(:group) { create(:group) } + let!(:project) { create(:project, project_level, namespace: group ) } let(:user) { create_user_from_membership(project, membership) } @@ -4109,7 +4289,7 @@ RSpec.describe Project, factory_default: :keep do subject { described_class.wrap_with_cte(projects) } it 'wrapped query matches original' do - expect(subject.to_sql).to match(/^WITH "projects_cte" AS/) + expect(subject.to_sql).to match(/^WITH "projects_cte" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/) expect(subject).to match_array(projects) end end @@ -4200,7 +4380,7 @@ RSpec.describe Project, factory_default: :keep do end it 'does nothing if updates on legacy storage are disabled' do - stub_feature_flags(pages_update_legacy_storage: false) + allow(Settings.pages.local_store).to receive(:enabled).and_return(false) expect(Gitlab::PagesTransfer).not_to receive(:new) expect(PagesWorker).not_to receive(:perform_in) @@ -4272,6 +4452,7 @@ RSpec.describe Project, factory_default: :keep do context 'legacy storage' do let_it_be(:project) { create(:project, :repository, :legacy_storage) } + let(:gitlab_shell) { Gitlab::Shell.new } let(:project_storage) { project.send(:storage) } @@ -4371,6 +4552,7 @@ RSpec.describe Project, factory_default: :keep do context 'hashed storage' do let_it_be(:project) { create(:project, :repository, skip_disk_validation: true) } + let(:gitlab_shell) { Gitlab::Shell.new } let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } @@ -4461,6 +4643,7 @@ RSpec.describe Project, factory_default: :keep do describe '#has_ci?' do let_it_be(:project, reload: true) { create(:project) } + let(:repository) { double } before do @@ -4957,6 +5140,7 @@ RSpec.describe Project, factory_default: :keep do context 'branch protection' do let_it_be(:namespace) { create(:namespace) } + let(:project) { create(:project, :repository, namespace: namespace) } before do @@ -5041,57 +5225,27 @@ RSpec.describe Project, factory_default: :keep do end describe '#default_branch' do - context 'with an empty repository' do - let_it_be(:project) { create(:project_empty_repo) } - - context 'group.default_branch_name is available' do - let(:project_group) { create(:group) } - let(:project) { create(:project, path: 'avatar', namespace: project_group) } + context 'with default_branch_name' do + let_it_be_with_refind(:root_group) { create(:group) } + let_it_be_with_refind(:project_group) { create(:group, parent: root_group) } + let_it_be_with_refind(:project) { create(:project, path: 'avatar', namespace: project_group) } - before do - expect(Gitlab::CurrentSettings) - .not_to receive(:default_branch_name) - - expect(project.group) - .to receive(:default_branch_name) - .and_return('example_branch') - end - - it 'returns the group default value' do - expect(project.default_branch).to eq('example_branch') - end + where(:instance_branch, :root_group_branch, :project_group_branch, :project_branch) do + '' | nil | nil | nil + nil | nil | nil | nil + 'main' | nil | nil | 'main' + 'main' | 'root_branch' | nil | 'root_branch' + 'main' | 'root_branch' | 'group_branch' | 'group_branch' end - context 'Gitlab::CurrentSettings.default_branch_name is available' do + with_them do before do - expect(Gitlab::CurrentSettings) - .to receive(:default_branch_name) - .and_return(example_branch_name) - end - - context 'is missing or nil' do - let(:example_branch_name) { nil } - - it "returns nil" do - expect(project.default_branch).to be_nil - end - end - - context 'is blank' do - let(:example_branch_name) { '' } - - it 'returns nil' do - expect(project.default_branch).to be_nil - end + allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return(instance_branch) + root_group.namespace_settings.update!(default_branch_name: root_group_branch) + project_group.namespace_settings.update!(default_branch_name: project_group_branch) end - context 'is present' do - let(:example_branch_name) { 'example_branch_name' } - - it 'returns the expected branch name' do - expect(project.default_branch).to eq(example_branch_name) - end - end + it { expect(project.default_branch).to eq(project_branch) } end end end @@ -5636,16 +5790,34 @@ RSpec.describe Project, factory_default: :keep do end describe '#find_or_initialize_services' do - before do - allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity]) - allow(subject).to receive(:disabled_services).and_return(%w[prometheus]) + let_it_be(:subject) { create(:project) } + + it 'avoids N+1 database queries' do + control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_services }.count + + expect(control_count).to be <= 4 end - it 'returns only enabled services' do - services = subject.find_or_initialize_services + it 'avoids N+1 database queries with more available services' do + allow(Service).to receive(:available_services_names).and_return(%w[pushover]) + control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_services } - expect(services.count).to eq(2) - expect(services.map(&:title)).to eq(['JetBrains TeamCity CI', 'Pushover']) + allow(Service).to receive(:available_services_names).and_call_original + expect { subject.find_or_initialize_services }.not_to exceed_query_limit(control_count) + end + + context 'with disabled services' do + before do + allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity]) + allow(subject).to receive(:disabled_services).and_return(%w[prometheus]) + end + + it 'returns only enabled services sorted' do + services = subject.find_or_initialize_services + + expect(services.size).to eq(2) + expect(services.map(&:title)).to eq(['JetBrains TeamCity', 'Pushover']) + end end end @@ -6074,12 +6246,15 @@ RSpec.describe Project, factory_default: :keep do project.set_first_pages_deployment!(deployment) expect(project.pages_metadatum.reload.pages_deployment).to eq(deployment) + expect(project.pages_metadatum.reload.deployed).to eq(true) end it "updates the existing metadara record with deployment" do expect do project.set_first_pages_deployment!(deployment) end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment) + + expect(project.pages_metadatum.reload.deployed).to eq(true) end it 'only updates metadata for this project' do @@ -6088,6 +6263,8 @@ RSpec.describe Project, factory_default: :keep do expect do project.set_first_pages_deployment!(deployment) end.not_to change { other_project.pages_metadatum.reload.pages_deployment }.from(nil) + + expect(other_project.pages_metadatum.reload.deployed).to eq(false) end it 'does nothing if metadata already references some deployment' do @@ -6098,6 +6275,14 @@ RSpec.describe Project, factory_default: :keep do project.set_first_pages_deployment!(deployment) end.not_to change { project.pages_metadatum.reload.pages_deployment }.from(existing_deployment) end + + it 'marks project as not deployed if deployment is nil' do + project.mark_pages_as_deployed + + expect do + project.set_first_pages_deployment!(nil) + end.to change { project.pages_metadatum.reload.deployed }.from(true).to(false) + end end describe '#has_pool_repsitory?' do @@ -6556,6 +6741,7 @@ RSpec.describe Project, factory_default: :keep do describe '#latest_jira_import' do let_it_be(:project) { create(:project) } + context 'when no jira imports' do it 'returns nil' do expect(project.latest_jira_import).to be nil diff --git a/spec/models/protected_tag_spec.rb b/spec/models/protected_tag_spec.rb index 7bc62b1d0e7..e5cee6f18cd 100644 --- a/spec/models/protected_tag_spec.rb +++ b/spec/models/protected_tag_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe ProtectedTag do describe 'Associations' do - it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:project).touch(true) } end describe 'Validation' do diff --git a/spec/models/raw_usage_data_spec.rb b/spec/models/raw_usage_data_spec.rb index 7acfb8c19af..6ff4c6eb19b 100644 --- a/spec/models/raw_usage_data_spec.rb +++ b/spec/models/raw_usage_data_spec.rb @@ -13,14 +13,20 @@ RSpec.describe RawUsageData do it { is_expected.to validate_uniqueness_of(:recorded_at) } end - describe '#update_sent_at!' do + describe '#update_version_metadata!' do let(:raw_usage_data) { create(:raw_usage_data) } it 'updates sent_at' do - raw_usage_data.update_sent_at! + raw_usage_data.update_version_metadata!(usage_data_id: 123) expect(raw_usage_data.sent_at).not_to be_nil end + + it 'updates version_usage_data_id_value' do + raw_usage_data.update_version_metadata!(usage_data_id: 123) + + expect(raw_usage_data.version_usage_data_id_value).not_to be_nil + end end end end diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index 60087278671..673451b5e76 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -13,26 +13,6 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do ReleaseHighlight.instance_variable_set(:@file_paths, nil) end - describe '.for_version' do - subject { ReleaseHighlight.for_version(version: version) } - - let(:version) { '1.1' } - - context 'with version param that exists' do - it 'returns items from that version' do - expect(subject.items.first['title']).to eq("It's gonna be a bright") - end - end - - context 'with version param that does NOT exist' do - let(:version) { '84.0' } - - it 'returns nil' do - expect(subject).to be_nil - end - end - end - describe '.paginated' do let(:dot_com) { false } @@ -143,28 +123,27 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do end end - describe '.versions' do - subject { described_class.versions } + describe '.most_recent_version_digest' do + subject { ReleaseHighlight.most_recent_version_digest } it 'uses process memory cache' do - expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:versions:#{Gitlab.revision}", { expires_in: described_class::CACHE_DURATION }) + expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:most_recent_version_digest:#{Gitlab.revision}", expires_in: described_class::CACHE_DURATION) subject end - it 'returns versions from the file paths' do - expect(subject).to eq(['1.5', '1.2', '1.1']) + context 'when recent release items exist' do + it 'returns a digest from the release of the first item of the most recent file' do + # this value is coming from fixture data + expect(subject).to eq(Digest::SHA256.hexdigest('01.05')) + end end - context 'when there are more than 12 versions' do - let(:file_paths) do - i = 0 - Array.new(20) { "20201225_01_#{i += 1}.yml" } - end + context 'when recent release items do NOT exist' do + it 'returns nil' do + allow(ReleaseHighlight).to receive(:paginated).and_return(nil) - it 'limits to 12 versions' do - allow(ReleaseHighlight).to receive(:file_paths).and_return(file_paths) - expect(subject.count).to eq(12) + expect(subject).to be_nil end end end diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 209ac471210..540a8068b20 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Release do it { expect(release).to be_valid } describe 'associations' do - it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:project).touch(true) } it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:links).class_name('Releases::Link') } it { is_expected.to have_many(:milestones) } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 4c3151f431c..d6951b5926e 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -263,6 +263,30 @@ RSpec.describe RemoteMirror, :mailer do end end + describe '#hard_retry!' do + let(:remote_mirror) { create(:remote_mirror).tap {|mirror| mirror.update_column(:url, 'invalid') } } + + it 'transitions an invalid mirror to the to_retry state' do + remote_mirror.hard_retry!('Invalid') + + expect(remote_mirror.update_status).to eq('to_retry') + expect(remote_mirror.last_error).to eq('Invalid') + end + end + + describe '#hard_fail!' do + let(:remote_mirror) { create(:remote_mirror).tap {|mirror| mirror.update_column(:url, 'invalid') } } + + it 'transitions an invalid mirror to the failed state' do + remote_mirror.hard_fail!('Invalid') + + expect(remote_mirror.update_status).to eq('failed') + expect(remote_mirror.last_error).to eq('Invalid') + expect(remote_mirror.last_update_at).not_to be_nil + expect(RemoteMirrorNotificationWorker.jobs).not_to be_empty + end + end + context 'when remote mirror gets destroyed' do it 'removes remote' do mirror = create_mirror(url: 'http://foo:bar@test.com') diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 84347ec2a51..a739f523008 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -170,6 +170,22 @@ RSpec.describe Repository do end end + describe '#search_branch_names' do + subject(:search_branch_names) { repository.search_branch_names('conflict-*') } + + it 'returns matching branch names' do + expect(search_branch_names).to contain_exactly( + 'conflict-binary-file', + 'conflict-resolvable', + 'conflict-contains-conflict-markers', + 'conflict-missing-side', + 'conflict-start', + 'conflict-non-utf8', + 'conflict-too-large' + ) + end + end + describe '#list_last_commits_for_tree' do let(:path_to_commit) do { @@ -977,6 +993,57 @@ RSpec.describe Repository do end end + describe '#search_files_by_wildcard_path' do + let(:ref) { 'master' } + + subject(:result) { repository.search_files_by_wildcard_path(path, ref) } + + context 'when specifying a normal path' do + let(:path) { 'files/images/logo-black.png' } + + it 'returns the path' do + expect(result).to eq(['files/images/logo-black.png']) + end + end + + context 'when specifying a path with wildcard' do + let(:path) { 'files/*/*.png' } + + it 'returns all files matching the path' do + expect(result).to contain_exactly('files/images/logo-black.png', + 'files/images/logo-white.png') + end + end + + context 'when specifying an extension with wildcard' do + let(:path) { '*.rb' } + + it 'returns all files matching the extension' do + expect(result).to contain_exactly('encoding/russian.rb', + 'files/ruby/popen.rb', + 'files/ruby/regex.rb', + 'files/ruby/version_info.rb') + end + end + + context 'when sending regexp' do + let(:path) { '.*\.rb' } + + it 'ignores the regexp and returns an empty array' do + expect(result).to eq([]) + end + end + + context 'when sending another ref' do + let(:path) { 'files' } + let(:ref) { 'other-branch' } + + it 'returns an empty array' do + expect(result).to eq([]) + end + end + end + describe '#async_remove_remote' do before do masterrev = repository.find_branch('master').dereferenced_target @@ -1036,7 +1103,8 @@ RSpec.describe Repository do describe '#create_ref' do it 'redirects the call to write_ref' do - ref, ref_path = '1', '2' + ref = '1' + ref_path = '2' expect(repository.raw_repository).to receive(:write_ref).with(ref_path, ref) @@ -1647,12 +1715,13 @@ RSpec.describe Repository do end it 'writes merge of source SHA and first parent ref to MR merge_ref_path' do - merge_commit_id = repository.merge_to_ref(user, - merge_request.diff_head_sha, - merge_request, - merge_request.merge_ref_path, - 'Custom message', - merge_request.target_branch_ref) + merge_commit_id = + repository.merge_to_ref(user, + source_sha: merge_request.diff_head_sha, + branch: merge_request.target_branch, + target_ref: merge_request.merge_ref_path, + message: 'Custom message', + first_parent_ref: merge_request.target_branch_ref) merge_commit = repository.commit(merge_commit_id) diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index aeafb49f8b5..aa515952c2b 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -72,8 +72,8 @@ RSpec.describe SentNotification do it_behaves_like 'a successful sent notification' - it 'does not set in_reply_to_discussion_id' do - expect(subject.in_reply_to_discussion_id).to be_nil + it 'sets in_reply_to_discussion_id' do + expect(subject.in_reply_to_discussion_id).to eq(note.discussion_id) end end end @@ -212,10 +212,10 @@ RSpec.describe SentNotification do subject { described_class.record_note(note, note.author.id) } - it 'creates a comment on the issue' do + it 'converts the comment to a discussion on the issue' do new_note = subject.create_reply('Test') expect(new_note.in_reply_to?(note)).to be_truthy - expect(new_note.discussion_id).not_to eq(note.discussion_id) + expect(new_note.discussion_id).to eq(note.discussion_id) end end @@ -247,10 +247,10 @@ RSpec.describe SentNotification do subject { described_class.record_note(note, note.author.id) } - it 'creates a comment on the merge request' do + it 'converts the comment to a discussion on the merge request' do new_note = subject.create_reply('Test') expect(new_note.in_reply_to?(note)).to be_truthy - expect(new_note.discussion_id).not_to eq(note.discussion_id) + expect(new_note.discussion_id).to eq(note.discussion_id) end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 9ffefd4bbf7..d8eb4ebc432 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -202,6 +202,16 @@ RSpec.describe Service do end end + describe '#project_level?' do + it 'is true when service has a project' do + expect(build(:service, project: project)).to be_project_level + end + + it 'is false when service has no project' do + expect(build(:service, project: nil)).not_to be_project_level + end + end + describe '.find_or_initialize_non_project_specific_integration' do let!(:service1) { create(:jira_service, project_id: nil, group_id: group.id) } let!(:service2) { create(:jira_service) } diff --git a/spec/models/sidebars/menu_spec.rb b/spec/models/sidebars/menu_spec.rb new file mode 100644 index 00000000000..320f5f1ad1e --- /dev/null +++ b/spec/models/sidebars/menu_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Menu do + let(:menu) { described_class.new(context) } + let(:context) { Sidebars::Context.new(current_user: nil, container: nil) } + + describe '#all_active_routes' do + it 'gathers all active routes of items and the current menu' do + menu_item1 = Sidebars::MenuItem.new(context) + menu_item2 = Sidebars::MenuItem.new(context) + menu_item3 = Sidebars::MenuItem.new(context) + menu.add_item(menu_item1) + menu.add_item(menu_item2) + menu.add_item(menu_item3) + + allow(menu).to receive(:active_routes).and_return({ path: 'foo' }) + allow(menu_item1).to receive(:active_routes).and_return({ path: %w(bar test) }) + allow(menu_item2).to receive(:active_routes).and_return({ controller: 'fooc' }) + allow(menu_item3).to receive(:active_routes).and_return({ controller: 'barc' }) + + expect(menu.all_active_routes).to eq({ path: %w(foo bar test), controller: %w(fooc barc) }) + end + + it 'does not include routes for non renderable items' do + menu_item = Sidebars::MenuItem.new(context) + menu.add_item(menu_item) + + allow(menu).to receive(:active_routes).and_return({ path: 'foo' }) + allow(menu_item).to receive(:render?).and_return(false) + allow(menu_item).to receive(:active_routes).and_return({ controller: 'bar' }) + + expect(menu.all_active_routes).to eq({ path: ['foo'] }) + end + end + + describe '#render?' do + context 'when the menus has no items' do + it 'returns true' do + expect(menu.render?).to be true + end + end + + context 'when the menu has items' do + let(:menu_item) { Sidebars::MenuItem.new(context) } + + before do + menu.add_item(menu_item) + end + + context 'when items are not renderable' do + it 'returns false' do + allow(menu_item).to receive(:render?).and_return(false) + + expect(menu.render?).to be false + end + end + + context 'when there are renderable items' do + it 'returns true' do + expect(menu.render?).to be true + end + end + end + end +end diff --git a/spec/models/sidebars/panel_spec.rb b/spec/models/sidebars/panel_spec.rb new file mode 100644 index 00000000000..0e539460810 --- /dev/null +++ b/spec/models/sidebars/panel_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Panel do + let(:context) { Sidebars::Context.new(current_user: nil, container: nil) } + let(:panel) { Sidebars::Panel.new(context) } + let(:menu1) { Sidebars::Menu.new(context) } + let(:menu2) { Sidebars::Menu.new(context) } + + describe '#renderable_menus' do + it 'returns only renderable menus' do + panel.add_menu(menu1) + panel.add_menu(menu2) + + allow(menu1).to receive(:render?).and_return(true) + allow(menu2).to receive(:render?).and_return(false) + + expect(panel.renderable_menus).to eq([menu1]) + end + end + + describe '#has_renderable_menus?' do + it 'returns false when no renderable menus' do + expect(panel.has_renderable_menus?).to be false + end + + it 'returns true when no renderable menus' do + panel.add_menu(menu1) + + expect(panel.has_renderable_menus?).to be true + end + end +end diff --git a/spec/models/sidebars/projects/context_spec.rb b/spec/models/sidebars/projects/context_spec.rb new file mode 100644 index 00000000000..44578ae1583 --- /dev/null +++ b/spec/models/sidebars/projects/context_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Projects::Context do + let(:project) { build(:project) } + + subject { described_class.new(current_user: nil, container: project) } + + it 'sets project attribute reader' do + expect(subject.project).to eq(project) + end +end diff --git a/spec/models/sidebars/projects/menus/learn_gitlab/menu_spec.rb b/spec/models/sidebars/projects/menus/learn_gitlab/menu_spec.rb new file mode 100644 index 00000000000..bc1815558d3 --- /dev/null +++ b/spec/models/sidebars/projects/menus/learn_gitlab/menu_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Projects::Menus::LearnGitlab::Menu do + let(:project) { build(:project) } + let(:experiment_enabled) { true } + let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project, learn_gitlab_experiment_enabled: experiment_enabled) } + + subject { described_class.new(context) } + + it 'does not contain any sub menu' do + expect(subject.instance_variable_get(:@items)).to be_empty + end + + describe '#render?' do + context 'when learn gitlab experiment is enabled' do + it 'returns true' do + expect(subject.render?).to eq true + end + end + + context 'when learn gitlab experiment is disabled' do + let(:experiment_enabled) { false } + + it 'returns false' do + expect(subject.render?).to eq false + end + end + end +end diff --git a/spec/models/sidebars/projects/menus/project_overview/menu_items/releases_spec.rb b/spec/models/sidebars/projects/menus/project_overview/menu_items/releases_spec.rb new file mode 100644 index 00000000000..db124c2252e --- /dev/null +++ b/spec/models/sidebars/projects/menus/project_overview/menu_items/releases_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Projects::Menus::ProjectOverview::MenuItems::Releases do + let_it_be(:project) { create(:project, :repository) } + + let(:user) { project.owner } + let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) } + + subject { described_class.new(context) } + + describe '#render?' do + context 'when project repository is empty' do + it 'returns false' do + allow(project).to receive(:empty_repo?).and_return(true) + + expect(subject.render?).to eq false + end + end + + context 'when project repository is not empty' do + context 'when user can read releases' do + it 'returns true' do + expect(subject.render?).to eq true + end + end + + context 'when user cannot read releases' do + let(:user) { nil } + + it 'returns false' do + expect(subject.render?).to eq false + end + end + end + end +end diff --git a/spec/models/sidebars/projects/menus/project_overview/menu_spec.rb b/spec/models/sidebars/projects/menus/project_overview/menu_spec.rb new file mode 100644 index 00000000000..105a28ce953 --- /dev/null +++ b/spec/models/sidebars/projects/menus/project_overview/menu_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Projects::Menus::ProjectOverview::Menu do + let(:project) { build(:project) } + let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project) } + + subject { described_class.new(context) } + + it 'has the required items' do + items = subject.instance_variable_get(:@items) + + expect(items[0]).to be_a(Sidebars::Projects::Menus::ProjectOverview::MenuItems::Details) + expect(items[1]).to be_a(Sidebars::Projects::Menus::ProjectOverview::MenuItems::Activity) + expect(items[2]).to be_a(Sidebars::Projects::Menus::ProjectOverview::MenuItems::Releases) + end +end diff --git a/spec/models/sidebars/projects/panel_spec.rb b/spec/models/sidebars/projects/panel_spec.rb new file mode 100644 index 00000000000..bad9b17bc83 --- /dev/null +++ b/spec/models/sidebars/projects/panel_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Projects::Panel do + let(:project) { build(:project) } + let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project) } + + subject { described_class.new(context) } + + it 'has a scope menu' do + expect(subject.scope_menu).to be_a(Sidebars::Projects::Menus::Scope::Menu) + end +end diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index e9019b55635..6a252b444f9 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -56,9 +56,9 @@ RSpec.describe Timelog do group = create(:group) subgroup = create(:group, parent: group) - create(:timelog, issue: create(:issue, project: create(:project))) - timelog1 = create(:timelog, issue: create(:issue, project: create(:project, group: group))) - timelog2 = create(:timelog, issue: create(:issue, project: create(:project, group: subgroup))) + create(:issue_timelog) + timelog1 = create(:issue_timelog, issue: create(:issue, project: create(:project, group: group))) + timelog2 = create(:issue_timelog, issue: create(:issue, project: create(:project, group: subgroup))) expect(described_class.for_issues_in_group(group)).to contain_exactly(timelog1, timelog2) end @@ -66,9 +66,9 @@ RSpec.describe Timelog do describe 'between_times' do it 'returns collection of timelogs within given times' do - create(:timelog, spent_at: 65.days.ago) - timelog1 = create(:timelog, spent_at: 15.days.ago) - timelog2 = create(:timelog, spent_at: 5.days.ago) + create(:issue_timelog, spent_at: 65.days.ago) + timelog1 = create(:issue_timelog, spent_at: 15.days.ago) + timelog2 = create(:issue_timelog, spent_at: 5.days.ago) timelogs = described_class.between_times(20.days.ago, 1.day.ago) expect(timelogs).to contain_exactly(timelog1, timelog2) diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 855b1b0f3f7..c4146b347d7 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -376,6 +376,22 @@ RSpec.describe Todo do end end + describe '.group_by_user_id_and_state' do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + + before do + create(:todo, user: user1, state: :pending) + create(:todo, user: user1, state: :pending) + create(:todo, user: user1, state: :done) + create(:todo, user: user2, state: :pending) + end + + specify do + expect(Todo.count_grouped_by_user_id_and_state).to eq({ [user1.id, "done"] => 1, [user1.id, "pending"] => 2, [user2.id, "pending"] => 1 }) + end + end + describe '.any_for_target?' do it 'returns true if there are todos for a given target' do todo = create(:todo) @@ -435,4 +451,12 @@ RSpec.describe Todo do end end end + + describe '.pluck_user_id' do + subject { described_class.pluck_user_id } + + let_it_be(:todo) { create(:todo) } + + it { is_expected.to eq([todo.user_id]) } + end end diff --git a/spec/models/user_callout_spec.rb b/spec/models/user_callout_spec.rb index cdf70dd5190..eb66f074293 100644 --- a/spec/models/user_callout_spec.rb +++ b/spec/models/user_callout_spec.rb @@ -18,36 +18,14 @@ RSpec.describe UserCallout do it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity } end - describe 'scopes' do - describe '.with_feature_name' do - let(:second_feature_name) { described_class.feature_names.keys.second } - let(:last_feature_name) { described_class.feature_names.keys.last } - - it 'returns callout for requested feature name only' do - callout1 = create(:user_callout, feature_name: second_feature_name ) - create(:user_callout, feature_name: last_feature_name ) - - callouts = described_class.with_feature_name(second_feature_name) - - expect(callouts).to match_array([callout1]) - end - end - - describe '.with_dismissed_after' do - let(:some_feature_name) { described_class.feature_names.keys.second } - let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )} - - it 'does not return callouts dismissed before specified date' do - callouts = described_class.with_dismissed_after(15.days.ago) - - expect(callouts).to match_array([]) - end - - it 'returns callouts dismissed after specified date' do - callouts = described_class.with_dismissed_after(2.months.ago) - - expect(callouts).to match_array([callout_dismissed_month_ago]) - end + describe '#dismissed_after?' do + let(:some_feature_name) { described_class.feature_names.keys.second } + let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )} + let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )} + + it 'returns whether a callout dismissed after specified date' do + expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false) + expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5f2842c9d16..3abf2a651a0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -85,6 +85,7 @@ RSpec.describe User do it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } + it { is_expected.to have_many(:expired_today_and_unnotified_keys) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:events).dependent(:delete_all) } @@ -108,6 +109,7 @@ RSpec.describe User do 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) } + it { is_expected.to have_many(:in_product_marketing_emails) } describe "#user_detail" do it 'does not persist `user_detail` by default' do @@ -999,6 +1001,27 @@ RSpec.describe User do end end + context 'SSH key expiration scopes' do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user1) } + let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user2, expiry_notification_delivered_at: Time.current) } + let_it_be(:expiring_soon_not_notified) { create(:key, expires_at: 2.days.from_now, user: user2) } + let_it_be(:expiring_soon_notified) { create(:key, expires_at: 2.days.from_now, user: user1, before_expiry_notification_delivered_at: Time.current) } + + describe '.with_ssh_key_expired_today' do + it 'returns users whose key has expired today' do + expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1) + end + end + + describe '.with_ssh_key_expiring_soon' do + it 'returns users whose keys will expire soon' do + expect(described_class.with_ssh_key_expiring_soon).to contain_exactly(user2) + end + end + end + describe '.active_without_ghosts' do let_it_be(:user1) { create(:user, :external) } let_it_be(:user2) { create(:user, state: 'blocked') } @@ -1766,7 +1789,7 @@ RSpec.describe User do end describe 'blocking user' do - let(:user) { create(:user, name: 'John Smith') } + let_it_be_with_refind(:user) { create(:user, name: 'John Smith') } it 'blocks user' do user.block @@ -1776,17 +1799,22 @@ RSpec.describe User do context 'when user has running CI pipelines' do let(:service) { double } + let(:pipelines) { build_list(:ci_pipeline, 3, :running) } - before do - pipeline = create(:ci_pipeline, :running, user: user) - create(:ci_build, :running, pipeline: pipeline) + it 'aborts all running pipelines and related jobs' do + expect(user).to receive(:pipelines).and_return(pipelines) + expect(Ci::DropPipelineService).to receive(:new).and_return(service) + expect(service).to receive(:execute_async_for_all).with(pipelines, :user_blocked, user) + + user.block end + end - it 'cancels all running pipelines and related jobs' do - expect(Ci::CancelUserPipelinesService).to receive(:new).and_return(service) - expect(service).to receive(:execute).with(user) + context 'when user has active CI pipeline schedules' do + let_it_be(:schedule) { create(:ci_pipeline_schedule, active: true, owner: user) } - user.block + it 'disables any pipeline schedules' do + expect { user.block }.to change { schedule.reload.active? }.to(false) end end end @@ -2502,32 +2530,12 @@ RSpec.describe User do 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) - 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) - 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 + user.update(avatar: fixture_file_upload('spec/fixtures/dk.png')) end end @@ -5500,6 +5508,12 @@ RSpec.describe User do it_behaves_like 'bot user avatars', :alert_bot, 'alert-bot.png' it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' + + context 'when bot is the support_bot' do + subject { described_class.support_bot } + + it { is_expected.to be_confirmed } + end end describe '#confirmation_required_on_sign_in?' do diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb new file mode 100644 index 00000000000..772d875d69e --- /dev/null +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::InProductMarketingEmail, type: :model do + let(:track) { :create } + let(:series) { 0 } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + subject { build(:in_product_marketing_email) } + + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:track) } + it { is_expected.to validate_presence_of(:series) } + it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:track, :series]).with_message('has already been sent') } + end + + describe '.without_track_and_series' do + let_it_be(:user) { create(:user) } + + subject(:without_track_and_series) { User.merge(described_class.without_track_and_series(track, series)) } + + before do + create(:in_product_marketing_email, track: :create, series: 0, user: user) + create(:in_product_marketing_email, track: :create, series: 1, user: user) + create(:in_product_marketing_email, track: :verify, series: 0, user: user) + end + + context 'when given track and series already exists' do + it { expect(without_track_and_series).to be_empty } + end + + context 'when track does not exist' do + let(:track) { :trial } + + it { expect(without_track_and_series).to eq [user] } + end + + context 'when series does not exist' do + let(:series) { 2 } + + it { expect(without_track_and_series).to eq [user] } + end + + context 'when no track or series for a user exists' do + let(:track) { :create } + let(:series) { 0 } + + before do + @other_user = create(:user) + end + + it { expect(without_track_and_series).to eq [@other_user] } + end + end + + describe '.for_user_with_track_and_series' do + let_it_be(:user) { create(:user) } + let_it_be(:in_product_marketing_email) { create(:in_product_marketing_email, series: 0, track: 0, user: user) } + + subject(:for_user_with_track_and_series) { described_class.for_user_with_track_and_series(user, track, series).first } + + context 'when record for user with given track and series exists' do + it { is_expected.to eq(in_product_marketing_email) } + end + + context 'when user is different' do + let(:user) { build_stubbed(:user) } + + it { is_expected.to be_nil } + end + + context 'when track is different' do + let(:track) { 1 } + + it { is_expected.to be_nil } + end + + context 'when series is different' do + let(:series) { 1 } + + it { is_expected.to be_nil } + end + end + + describe '.save_cta_click' do + let(:user) { create(:user) } + + subject(:save_cta_click) { described_class.save_cta_click(user, track, series) } + + context 'when there is no record' do + it 'does not error' do + expect { save_cta_click }.not_to raise_error + end + end + + context 'when there is no record for the track and series' do + it 'does not perform an update' do + other_email = create(:in_product_marketing_email, user: user, track: :verify, series: 2, cta_clicked_at: nil) + + expect { save_cta_click }.not_to change { other_email.reload } + end + end + + context 'when there is a record for the track and series' do + it 'saves the cta click date' do + email = create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: nil) + + freeze_time do + expect { save_cta_click }.to change { email.reload.cta_clicked_at }.from(nil).to(Time.zone.now) + end + end + + context 'cta_clicked_at is already set' do + it 'does not update' do + create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: Time.zone.now) + + expect_next_found_instance_of(described_class) do |record| + expect(record).not_to receive(:update) + end + + save_cta_click + end + end + end + end +end diff --git a/spec/models/users/merge_request_interaction_spec.rb b/spec/models/users/merge_request_interaction_spec.rb new file mode 100644 index 00000000000..d333577fa1a --- /dev/null +++ b/spec/models/users/merge_request_interaction_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Users::MergeRequestInteraction do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + subject(:interaction) do + ::Users::MergeRequestInteraction.new(user: user, merge_request: merge_request.reset) + end + + describe 'declarative policy delegation' do + it 'delegates to the merge request' do + expect(subject.declarative_policy_subject).to eq(merge_request) + end + end + + describe '#can_merge?' do + context 'when the user cannot merge' do + it { is_expected.not_to be_can_merge } + end + + context 'when the user can merge' do + before do + project.add_maintainer(user) + end + + it { is_expected.to be_can_merge } + end + end + + describe '#can_update?' do + context 'when the user cannot update the MR' do + it { is_expected.not_to be_can_update } + end + + context 'when the user can update the MR' do + before do + project.add_developer(user) + end + + it { is_expected.to be_can_update } + end + end + + describe '#review_state' do + subject { interaction.review_state } + + context 'when the user has not been asked to review the MR' do + it { is_expected.to be_nil } + + it 'implies not reviewed' do + expect(interaction).not_to be_reviewed + end + end + + context 'when the user has been asked to review the MR' do + before do + merge_request.reviewers << user + end + + it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) } + + it 'implies not reviewed' do + expect(interaction).not_to be_reviewed + end + end + + context 'when the user has provided a review' do + before do + merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed']) + end + + it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) } + + it 'implies reviewed' do + expect(interaction).to be_reviewed + end + end + end + + describe '#approved?' do + context 'when the user has not approved the MR' do + it { is_expected.not_to be_approved } + end + + context 'when the user has approved the MR' do + before do + merge_request.approved_by_users << user + end + + it { is_expected.to be_approved } + end + end +end |