diff options
Diffstat (limited to 'spec/models')
89 files changed, 2894 insertions, 1540 deletions
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 16e1d8fbc4d..b5f153e7add 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -116,12 +116,20 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_presence_of(:max_yaml_depth) } it { is_expected.to validate_numericality_of(:max_yaml_depth).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_pages_size) } + it { is_expected.to validate_presence_of(:max_pages_custom_domains_per_project) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) .is_less_than(::Gitlab::Pages::MAX_SIZE / 1.megabyte) end + it 'ensures max_pages_custom_domains_per_project is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do + is_expected + .to validate_numericality_of(:max_pages_custom_domains_per_project) + .only_integer + .is_greater_than_or_equal_to(0) + end + it { is_expected.to validate_presence_of(:jobs_per_stage_page_size) } it { is_expected.to validate_numericality_of(:jobs_per_stage_page_size).only_integer.is_greater_than_or_equal_to(0) } diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index 737348765d9..1dd0386060d 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -13,10 +13,15 @@ RSpec.describe Ci::BuildDependencies do status: 'success') end - let!(:build) { create(:ci_build, pipeline: pipeline, name: 'build', stage_idx: 0, stage: 'build') } - let!(:rspec_test) { create(:ci_build, :success, pipeline: pipeline, name: 'rspec', stage_idx: 1, stage: 'test') } - let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, stage: 'test') } - let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, stage: 'deploy') } + let(:build_stage) { create(:ci_stage, name: 'build', pipeline: pipeline) } + let(:test_stage) { create(:ci_stage, name: 'test', pipeline: pipeline) } + let(:deploy_stage) { create(:ci_stage, name: 'deploy', pipeline: pipeline) } + let!(:build) { create(:ci_build, pipeline: pipeline, name: 'build', stage_idx: 0, ci_stage: build_stage) } + let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, ci_stage: test_stage) } + let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, ci_stage: deploy_stage) } + let!(:rspec_test) do + create(:ci_build, :success, pipeline: pipeline, name: 'rspec', stage_idx: 1, ci_stage: test_stage) + end context 'for local dependencies' do subject { described_class.new(job).all } @@ -63,7 +68,7 @@ RSpec.describe Ci::BuildDependencies do name: 'dag_job', scheduling_type: :dag, stage_idx: 2, - stage: 'deploy' + ci_stage: deploy_stage ) end @@ -87,7 +92,7 @@ RSpec.describe Ci::BuildDependencies do name: 'final', scheduling_type: scheduling_type, stage_idx: 3, - stage: 'deploy', + ci_stage: deploy_stage, options: { dependencies: dependencies } ) end @@ -218,12 +223,12 @@ RSpec.describe Ci::BuildDependencies do cross_pipeline_limit.times do |index| create(:ci_build, :success, pipeline: parent_pipeline, name: "dependency-#{index}", - stage_idx: 1, stage: 'build', user: user + stage_idx: 1, ci_stage: build_stage, user: user ) create(:ci_build, :success, pipeline: sibling_pipeline, name: "dependency-#{index}", - stage_idx: 1, stage: 'build', user: user + stage_idx: 1, ci_stage: build_stage, user: user ) end end @@ -355,7 +360,7 @@ RSpec.describe Ci::BuildDependencies do describe '#all' do let!(:job) do - create(:ci_build, pipeline: pipeline, name: 'deploy', stage_idx: 3, stage: 'deploy') + create(:ci_build, pipeline: pipeline, name: 'deploy', stage_idx: 3, ci_stage: deploy_stage) end let(:dependencies) { described_class.new(job) } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b865688d370..7ee381b29ea 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -67,31 +67,6 @@ RSpec.describe Ci::Build do create(:ci_build) end - - context 'when the execute_build_hooks_inline flag is disabled' do - before do - stub_feature_flags(execute_build_hooks_inline: false) - end - - it 'uses the old job hooks worker' do - expect(::BuildHooksWorker).to receive(:perform_async).with(Ci::Build) - - create(:ci_build) - end - end - - context 'when the execute_build_hooks_inline flag is enabled for a project' do - before do - stub_feature_flags(execute_build_hooks_inline: project) - end - - it 'executes hooks inline' do - expect(::BuildHooksWorker).not_to receive(:perform_async) - expect_next(described_class).to receive(:execute_hooks) - - create(:ci_build, project: project) - end - end end end @@ -594,6 +569,51 @@ RSpec.describe Ci::Build do end end + describe '#prevent_rollback_deployment?' do + subject { build.prevent_rollback_deployment? } + + let(:build) { create(:ci_build, :created, :with_deployment, project: project, environment: 'production') } + + context 'when build has no environment' do + let(:build) { create(:ci_build, :created, project: project, environment: nil) } + + it { expect(subject).to be_falsey } + end + + context 'when project has forward deployment disabled' do + before do + project.ci_cd_settings.update!(forward_deployment_enabled: false) + end + + it { expect(subject).to be_falsey } + end + + context 'when deployment cannot rollback' do + before do + expect(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(false) + end + + it { expect(subject).to be_falsey } + end + + context 'when prevent_outdated_deployment_jobs FF is disabled' do + before do + stub_feature_flags(prevent_outdated_deployment_jobs: false) + expect(build.deployment).not_to receive(:rollback?) + end + + it { expect(subject).to be_falsey } + end + + context 'when build can prevent rollback deployment' do + before do + expect(build.deployment).to receive(:older_than_last_successful_deployment?).and_return(true) + end + + it { expect(subject).to be_truthy } + end + end + describe '#schedulable?' do subject { build.schedulable? } @@ -1250,70 +1270,6 @@ RSpec.describe Ci::Build do end end - describe '#has_old_trace?' do - subject { build.has_old_trace? } - - context 'when old trace exists' do - before do - build.update_column(:trace, 'old trace') - end - - it { is_expected.to be_truthy } - end - - context 'when old trace does not exist' do - it { is_expected.to be_falsy } - end - end - - describe '#trace=' do - it "expect to fail trace=" do - expect { build.trace = "new" }.to raise_error(NotImplementedError) - end - end - - describe '#old_trace' do - subject { build.old_trace } - - before do - build.update_column(:trace, 'old trace') - end - - it "expect to receive data from database" do - is_expected.to eq('old trace') - end - end - - describe '#erase_old_trace!' do - subject { build.erase_old_trace! } - - context 'when old trace exists' do - before do - build.update_column(:trace, 'old trace') - end - - it "erases old trace" do - subject - - expect(build.old_trace).to be_nil - end - - it "executes UPDATE query" do - recorded = ActiveRecord::QueryRecorder.new { subject } - - expect(recorded.log.count { |l| l.match?(/UPDATE.*ci_builds/) }).to eq(1) - end - end - - context 'when old trace does not exist' do - it 'does not execute UPDATE query' do - recorded = ActiveRecord::QueryRecorder.new { subject } - - expect(recorded.log.count { |l| l.match?(/UPDATE.*ci_builds/) }).to eq(0) - end - end - end - describe '#hide_secrets' do let(:metrics) { spy('metrics') } let(:subject) { build.hide_secrets(data) } @@ -1370,13 +1326,12 @@ RSpec.describe Ci::Build do subject { build.send(event) } - where(:ff_enabled, :state, :report_count, :trait) do - true | :success! | 1 | :sast - true | :cancel! | 1 | :sast - true | :drop! | 2 | :multiple_report_artifacts - true | :success! | 0 | :allowed_to_fail - true | :skip! | 0 | :pending - false | :success! | 0 | :sast + where(:state, :report_count, :trait) do + :success! | 1 | :sast + :cancel! | 1 | :sast + :drop! | 2 | :multiple_report_artifacts + :success! | 0 | :allowed_to_fail + :skip! | 0 | :pending end with_them do @@ -1386,7 +1341,6 @@ RSpec.describe Ci::Build do context "when transitioning to #{params[:state]}" do before do allow(Gitlab).to receive(:com?).and_return(true) - stub_feature_flags(report_artifact_build_completed_metrics_on_build_completion: ff_enabled) end it 'increments build_completed_report_type metric' do @@ -1645,32 +1599,6 @@ RSpec.describe Ci::Build do end end - describe '#count_user_verification?' do - subject { build.count_user_verification? } - - context 'when build is the verify action for the environment' do - let(:build) do - create(:ci_build, - ref: 'master', - environment: 'staging', - options: { environment: { action: 'verify' } }) - end - - it { is_expected.to be_truthy } - end - - context 'when build is not the verify action for the environment' do - let(:build) do - create(:ci_build, - ref: 'master', - environment: 'staging', - options: { environment: { action: 'start' } }) - end - - it { is_expected.to be_falsey } - end - end - describe '#expanded_environment_name' do subject { build.expanded_environment_name } @@ -1873,12 +1801,6 @@ RSpec.describe Ci::Build do context 'build is not erasable' do let!(:build) { create(:ci_build) } - describe '#erase' do - subject { build.erase } - - it { is_expected.to be false } - end - describe '#erasable?' do subject { build.erasable? } @@ -1887,71 +1809,9 @@ RSpec.describe Ci::Build do end context 'build is erasable' do - context 'logging erase' do - let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) } - - it 'logs erased artifacts' do - expect(Gitlab::Ci::Artifacts::Logger) - .to receive(:log_deleted) - .with( - match_array(build.job_artifacts.to_a), - 'Ci::Build#erase' - ) - - build.erase - end - end - - context 'when project is undergoing stats refresh' do - let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) } - - describe '#erase' do - before do - allow(build.project).to receive(:refreshing_build_artifacts_size?).and_return(true) - end - - it 'logs and continues with deleting the artifacts' do - expect(Gitlab::ProjectStatsRefreshConflictsLogger).to receive(:warn_artifact_deletion_during_stats_refresh).with( - method: 'Ci::Build#erase', - project_id: build.project.id - ) - - build.erase - - expect(build.job_artifacts.count).to eq(0) - end - end - end - context 'new artifacts' do let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) } - describe '#erase' do - before do - build.erase(erased_by: erased_by) - end - - context 'erased by user' do - let!(:erased_by) { create(:user, username: 'eraser') } - - include_examples 'erasable' - - it 'records user who erased a build' do - expect(build.erased_by).to eq erased_by - end - end - - context 'erased by system' do - let(:erased_by) { nil } - - include_examples 'erasable' - - it 'does not set user who erased a build' do - expect(build.erased_by).to be_nil - end - end - end - describe '#erasable?' do subject { build.erasable? } @@ -1969,76 +1829,12 @@ RSpec.describe Ci::Build do context 'job has been erased' do before do - build.erase + build.update!(erased_at: 1.minute.ago) end it { is_expected.to be_truthy } end end - - context 'metadata and build trace are not available' do - let!(:build) { create(:ci_build, :success, :artifacts) } - - before do - build.erase_erasable_artifacts! - end - - describe '#erase' do - it 'does not raise error' do - expect { build.erase }.not_to raise_error - end - end - end - end - end - end - - describe '#erase_erasable_artifacts!' do - let!(:build) { create(:ci_build, :success) } - - subject { build.erase_erasable_artifacts! } - - before do - Ci::JobArtifact.file_types.keys.each do |file_type| - create(:ci_job_artifact, job: build, file_type: file_type, file_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS[file_type.to_sym]) - end - end - - it "erases erasable artifacts and logs them" do - expect(Gitlab::Ci::Artifacts::Logger) - .to receive(:log_deleted) - .with( - match_array(build.job_artifacts.erasable.to_a), - 'Ci::Build#erase_erasable_artifacts!' - ) - - subject - - expect(build.job_artifacts.erasable).to be_empty - end - - it "keeps non erasable artifacts" do - subject - - Ci::JobArtifact::NON_ERASABLE_FILE_TYPES.each do |file_type| - expect(build.send("job_artifacts_#{file_type}")).not_to be_nil - end - end - - context 'when the project is undergoing stats refresh' do - before do - allow(build.project).to receive(:refreshing_build_artifacts_size?).and_return(true) - end - - it 'logs and continues with deleting the artifacts' do - expect(Gitlab::ProjectStatsRefreshConflictsLogger).to receive(:warn_artifact_deletion_during_stats_refresh).with( - method: 'Ci::Build#erase_erasable_artifacts!', - project_id: build.project.id - ) - - subject - - expect(build.job_artifacts.erasable).to be_empty end end end @@ -2689,17 +2485,17 @@ RSpec.describe Ci::Build do describe '#ref_slug' do { - 'master' => 'master', - '1-foo' => '1-foo', - 'fix/1-foo' => 'fix-1-foo', - 'fix-1-foo' => 'fix-1-foo', - 'a' * 63 => 'a' * 63, - 'a' * 64 => 'a' * 63, - 'FOO' => 'foo', - '-' + 'a' * 61 + '-' => 'a' * 61, - '-' + 'a' * 62 + '-' => 'a' * 62, - '-' + 'a' * 63 + '-' => 'a' * 62, - 'a' * 62 + ' ' => 'a' * 62 + 'master' => 'master', + '1-foo' => '1-foo', + 'fix/1-foo' => 'fix-1-foo', + 'fix-1-foo' => 'fix-1-foo', + 'a' * 63 => 'a' * 63, + 'a' * 64 => 'a' * 63, + 'FOO' => 'foo', + '-' + 'a' * 61 + '-' => 'a' * 61, + '-' + 'a' * 62 + '-' => 'a' * 62, + '-' + 'a' * 63 + '-' => 'a' * 62, + 'a' * 62 + ' ' => 'a' * 62 }.each do |ref, slug| it "transforms #{ref} to #{slug}" do build.ref = ref @@ -3634,17 +3430,6 @@ RSpec.describe Ci::Build do it 'includes deploy token variables' do is_expected.to include(*deploy_token_variables) end - - context 'when the FF ci_variable_for_group_gitlab_deploy_token is disabled' do - before do - stub_feature_flags(ci_variable_for_group_gitlab_deploy_token: false) - end - - it 'does not include deploy token variables' do - expect(subject.find { |v| v[:key] == 'CI_DEPLOY_USER' }).to be_nil - expect(subject.find { |v| v[:key] == 'CI_DEPLOY_PASSWORD' }).to be_nil - end - end end end end @@ -3921,18 +3706,6 @@ RSpec.describe Ci::Build do build.enqueue end - context 'when the execute_build_hooks_inline flag is disabled' do - before do - stub_feature_flags(execute_build_hooks_inline: false) - end - - it 'queues BuildHooksWorker' do - expect(BuildHooksWorker).to receive(:perform_async).with(build) - - build.enqueue - end - end - it 'executes hooks' do expect(build).to receive(:execute_hooks) @@ -4048,10 +3821,6 @@ RSpec.describe Ci::Build do context 'when artifacts of depended job has been erased' do let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } - before do - pre_stage_job.erase - end - it { expect(job).not_to have_valid_build_dependencies } end end @@ -4072,10 +3841,6 @@ RSpec.describe Ci::Build do context 'when artifacts of depended job has been erased' do let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0, erased_at: 1.minute.ago) } - before do - pre_stage_job.erase - end - it { expect(job).to have_valid_build_dependencies } end end @@ -4405,9 +4170,7 @@ RSpec.describe Ci::Build do end describe '#collect_test_reports!' do - subject { build.collect_test_reports!(test_reports) } - - let(:test_reports) { Gitlab::Ci::Reports::TestReport.new } + subject(:test_reports) { build.collect_test_reports!(Gitlab::Ci::Reports::TestReport.new) } it { expect(test_reports.get_suite(build.name).total_count).to eq(0) } @@ -4455,56 +4218,6 @@ RSpec.describe Ci::Build do end end end - - context 'when build is part of parallel build' do - let(:build_1) { create(:ci_build, name: 'build 1/2') } - let(:test_report) { Gitlab::Ci::Reports::TestReport.new } - - before do - build_1.collect_test_reports!(test_report) - end - - it 'uses the group name for test suite name' do - expect(test_report.test_suites.keys).to contain_exactly('build') - end - - context 'when there are more than one parallel builds' do - let(:build_2) { create(:ci_build, name: 'build 2/2') } - - before do - build_2.collect_test_reports!(test_report) - end - - it 'merges the test suite from parallel builds' do - expect(test_report.test_suites.keys).to contain_exactly('build') - end - end - end - - context 'when build is part of matrix build' do - let(:test_report) { Gitlab::Ci::Reports::TestReport.new } - let(:matrix_build_1) { create(:ci_build, :matrix) } - - before do - matrix_build_1.collect_test_reports!(test_report) - end - - it 'uses the job name for the test suite' do - expect(test_report.test_suites.keys).to contain_exactly(matrix_build_1.name) - end - - context 'when there are more than one matrix builds' do - let(:matrix_build_2) { create(:ci_build, :matrix) } - - before do - matrix_build_2.collect_test_reports!(test_report) - end - - it 'keeps separate test suites' do - expect(test_report.test_suites.keys).to match_array([matrix_build_1.name, matrix_build_2.name]) - end - end - end end describe '#collect_accessibility_reports!' do @@ -5620,7 +5333,7 @@ RSpec.describe Ci::Build do end end - describe '#runner_features' do + describe '#runtime_runner_features' do subject do build.save! build.cancel_gracefully? @@ -5701,4 +5414,28 @@ RSpec.describe Ci::Build do end end end + + describe '#test_suite_name' do + let(:build) { create(:ci_build, name: 'test') } + + it 'uses the group name for test suite name' do + expect(build.test_suite_name).to eq('test') + end + + context 'when build is part of parallel build' do + let(:build) { create(:ci_build, name: 'build 1/2') } + + it 'uses the group name for test suite name' do + expect(build.test_suite_name).to eq('build') + end + end + + context 'when build is part of matrix build' do + let!(:matrix_build) { create(:ci_build, :matrix) } + + it 'uses the job name for the test suite' do + expect(matrix_build.test_suite_name).to eq(matrix_build.name) + end + end + end end diff --git a/spec/models/ci/freeze_period_status_spec.rb b/spec/models/ci/freeze_period_status_spec.rb index f51381f7a5f..ecbb7af64f7 100644 --- a/spec/models/ci/freeze_period_status_spec.rb +++ b/spec/models/ci/freeze_period_status_spec.rb @@ -59,4 +59,13 @@ RSpec.describe Ci::FreezePeriodStatus do it_behaves_like 'outside freeze period', Time.utc(2020, 4, 13, 8, 1) end + + # https://gitlab.com/gitlab-org/gitlab/-/issues/370472 + context 'when period overlaps with itself' do + let!(:freeze_period) { create(:ci_freeze_period, project: project, freeze_start: '* * * 8 *', freeze_end: '* * * 10 *') } + + it_behaves_like 'within freeze period', Time.utc(2020, 8, 11, 0, 0) + + it_behaves_like 'outside freeze period', Time.utc(2020, 10, 11, 0, 0) + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index b996bf84529..098f8bd4514 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -8,6 +8,8 @@ RSpec.describe Ci::JobArtifact do describe "Associations" do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:job) } + it { is_expected.to validate_presence_of(:job) } + it { is_expected.to validate_presence_of(:partition_id) } end it { is_expected.to respond_to(:file) } @@ -48,82 +50,86 @@ RSpec.describe Ci::JobArtifact do end end - describe '.test_reports' do - subject { described_class.test_reports } + describe '.of_report_type' do + subject { described_class.of_report_type(report_type) } - context 'when there is a test report' do - let!(:artifact) { create(:ci_job_artifact, :junit) } + describe 'test_reports' do + let(:report_type) { :test } - it { is_expected.to eq([artifact]) } - end + context 'when there is a test report' do + let!(:artifact) { create(:ci_job_artifact, :junit) } - context 'when there are no test reports' do - let!(:artifact) { create(:ci_job_artifact, :archive) } + it { is_expected.to eq([artifact]) } + end - it { is_expected.to be_empty } + context 'when there are no test reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } + + it { is_expected.to be_empty } + end end - end - describe '.accessibility_reports' do - subject { described_class.accessibility_reports } + describe 'accessibility_reports' do + let(:report_type) { :accessibility } - context 'when there is an accessibility report' do - let(:artifact) { create(:ci_job_artifact, :accessibility) } + context 'when there is an accessibility report' do + let(:artifact) { create(:ci_job_artifact, :accessibility) } - it { is_expected.to eq([artifact]) } - end + it { is_expected.to eq([artifact]) } + end - context 'when there are no accessibility report' do - let(:artifact) { create(:ci_job_artifact, :archive) } + context 'when there are no accessibility report' do + let(:artifact) { create(:ci_job_artifact, :archive) } - it { is_expected.to be_empty } + it { is_expected.to be_empty } + end end - end - describe '.coverage_reports' do - subject { described_class.coverage_reports } + describe 'coverage_reports' do + let(:report_type) { :coverage } - context 'when there is a coverage report' do - let!(:artifact) { create(:ci_job_artifact, :cobertura) } + context 'when there is a coverage report' do + let!(:artifact) { create(:ci_job_artifact, :cobertura) } - it { is_expected.to eq([artifact]) } - end + it { is_expected.to eq([artifact]) } + end - context 'when there are no coverage reports' do - let!(:artifact) { create(:ci_job_artifact, :archive) } + context 'when there are no coverage reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } - it { is_expected.to be_empty } + it { is_expected.to be_empty } + end end - end - describe '.codequality_reports' do - subject { described_class.codequality_reports } + describe 'codequality_reports' do + let(:report_type) { :codequality } - context 'when there is a codequality report' do - let!(:artifact) { create(:ci_job_artifact, :codequality) } + context 'when there is a codequality report' do + let!(:artifact) { create(:ci_job_artifact, :codequality) } - it { is_expected.to eq([artifact]) } - end + it { is_expected.to eq([artifact]) } + end - context 'when there are no codequality reports' do - let!(:artifact) { create(:ci_job_artifact, :archive) } + context 'when there are no codequality reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } - it { is_expected.to be_empty } + it { is_expected.to be_empty } + end end - end - describe '.terraform_reports' do - context 'when there is a terraform report' do - it 'return the job artifact' do - artifact = create(:ci_job_artifact, :terraform) + describe 'terraform_reports' do + let(:report_type) { :terraform } + + context 'when there is a terraform report' do + let!(:artifact) { create(:ci_job_artifact, :terraform) } - expect(described_class.terraform_reports).to eq([artifact]) + it { is_expected.to eq([artifact]) } end - end - context 'when there are no terraform reports' do - it 'return the an empty array' do - expect(described_class.terraform_reports).to eq([]) + context 'when there are no terraform reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } + + it { is_expected.to be_empty } end end end @@ -135,7 +141,7 @@ RSpec.describe Ci::JobArtifact do context 'when given an unrecognized report type' do it 'raises error' do - expect { described_class.file_types_for_report(:blah) }.to raise_error(KeyError, /blah/) + expect { described_class.file_types_for_report(:blah) }.to raise_error(ArgumentError, "Unrecognized report type: blah") end end end @@ -146,8 +152,8 @@ RSpec.describe Ci::JobArtifact do subject { Ci::JobArtifact.associated_file_types_for(file_type) } where(:file_type, :result) do - 'codequality' | %w(codequality) - 'quality' | nil + 'codequality' | %w(codequality) + 'quality' | nil end with_them do @@ -754,4 +760,26 @@ RSpec.describe Ci::JobArtifact do let!(:model) { create(:ci_job_artifact, project: parent) } end end + + describe 'partitioning' do + let(:job) { build(:ci_build, partition_id: 123) } + let(:artifact) { build(:ci_job_artifact, job: job, partition_id: nil) } + + it 'copies the partition_id from job' do + expect { artifact.valid? }.to change(artifact, :partition_id).from(nil).to(123) + end + + context 'when the job is missing' do + let(:artifact) do + build(:ci_job_artifact, + project: build_stubbed(:project), + job: nil, + partition_id: nil) + end + + it 'does not change the partition_id value' do + expect { artifact.valid? }.not_to change(artifact, :partition_id) + end + end + end end diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index 3e77c349ccb..29447cbc89d 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -16,7 +16,9 @@ RSpec.describe Ci::NamespaceMirror do expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id]) expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id]) expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) - expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id]) + expect(group4.reload.ci_namespace_mirror).to have_attributes( + traversal_ids: [group1.id, group2.id, group3.id, group4.id] + ) end context 'scopes' do @@ -103,6 +105,8 @@ RSpec.describe Ci::NamespaceMirror do describe '.sync!' do subject(:sync) { described_class.sync!(Namespaces::SyncEvent.last) } + let(:expected_traversal_ids) { [group1.id, group2.id, group3.id] } + context 'when namespace mirror does not exist in the first place' do let(:namespace) { group3 } @@ -114,7 +118,7 @@ RSpec.describe Ci::NamespaceMirror do it 'creates the mirror' do expect { sync }.to change { described_class.count }.from(3).to(4) - expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) + expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: expected_traversal_ids) end end @@ -128,36 +132,8 @@ RSpec.describe Ci::NamespaceMirror do it 'updates the mirror' do expect { sync }.not_to change { described_class.count } - expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id]) - end - end - - shared_context 'changing the middle namespace' do - let(:namespace) { group2 } - - before do - group2.update!(parent: nil) # creates a sync event - end - - it 'updates traversal_ids for the base and descendants' do - expect { sync }.not_to change { described_class.count } - - expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id]) - expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id]) - expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id]) - expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id, group4.id]) - end - end - - it_behaves_like 'changing the middle namespace' - - context 'when the FFs use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do - before do - stub_feature_flags(use_traversal_ids: false, - use_traversal_ids_for_ancestors: false) + expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: expected_traversal_ids) end - - it_behaves_like 'changing the middle namespace' end end end diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index b051f646bd4..3038cdc944b 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -227,6 +227,19 @@ RSpec.describe Ci::PipelineArtifact, type: :model do expect(subject.size).to eq(size) expect(subject.file_format).to eq(Ci::PipelineArtifact::REPORT_TYPES[file_type].to_s) expect(subject.expire_at).to eq(Ci::PipelineArtifact::EXPIRATION_DATE.from_now) + expect(subject.locked).to eq('unknown') + end + + it "creates a new pipeline artifact with pipeline's locked state" do + artifact = Ci::PipelineArtifact.create_or_replace_for_pipeline!( + pipeline: pipeline, + file_type: file_type, + file: file, + size: size, + locked: pipeline.locked + ) + + expect(artifact.locked).to eq(pipeline.locked) end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0c28c99c113..ec03030a4b8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -466,6 +466,48 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.jobs_count_in_alive_pipelines' do + before do + ::Ci::HasStatus::ALIVE_STATUSES.each do |status| + alive_pipeline = create(:ci_pipeline, status: status, project: project) + create(:ci_build, pipeline: alive_pipeline) + create(:ci_bridge, pipeline: alive_pipeline) + end + + completed_pipeline = create(:ci_pipeline, :success, project: project) + create(:ci_build, pipeline: completed_pipeline) + + old_pipeline = create(:ci_pipeline, :running, project: project, created_at: 2.days.ago) + create(:ci_build, pipeline: old_pipeline) + end + + it 'includes all jobs in alive pipelines created in the last 24 hours' do + expect(described_class.jobs_count_in_alive_pipelines) + .to eq(::Ci::HasStatus::ALIVE_STATUSES.count * 2) + end + end + + describe '.builds_count_in_alive_pipelines' do + before do + ::Ci::HasStatus::ALIVE_STATUSES.each do |status| + alive_pipeline = create(:ci_pipeline, status: status, project: project) + create(:ci_build, pipeline: alive_pipeline) + create(:ci_bridge, pipeline: alive_pipeline) + end + + completed_pipeline = create(:ci_pipeline, :success, project: project) + create(:ci_build, pipeline: completed_pipeline) + + old_pipeline = create(:ci_pipeline, :running, project: project, created_at: 2.days.ago) + create(:ci_build, pipeline: old_pipeline) + end + + it 'includes all builds in alive pipelines created in the last 24 hours' do + expect(described_class.builds_count_in_alive_pipelines) + .to eq(::Ci::HasStatus::ALIVE_STATUSES.count) + end + end + describe '#merge_request?' do let_it_be(:merge_request) { create(:merge_request) } let_it_be_with_reload(:pipeline) do @@ -686,7 +728,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do describe '.with_reports' do context 'when pipeline has a test report' do - subject { described_class.with_reports(Ci::JobArtifact.test_reports) } + subject { described_class.with_reports(Ci::JobArtifact.of_report_type(:test)) } let!(:pipeline_with_report) { create(:ci_pipeline, :with_test_reports) } @@ -696,7 +738,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline has a coverage report' do - subject { described_class.with_reports(Ci::JobArtifact.coverage_reports) } + subject { described_class.with_reports(Ci::JobArtifact.of_report_type(:coverage)) } let!(:pipeline_with_report) { create(:ci_pipeline, :with_coverage_reports) } @@ -706,7 +748,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline has an accessibility report' do - subject { described_class.with_reports(Ci::JobArtifact.accessibility_reports) } + subject { described_class.with_reports(Ci::JobArtifact.of_report_type(:accessibility)) } let(:pipeline_with_report) { create(:ci_pipeline, :with_accessibility_reports) } @@ -716,7 +758,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end context 'when pipeline has a codequality report' do - subject { described_class.with_reports(Ci::JobArtifact.codequality_reports) } + subject { described_class.with_reports(Ci::JobArtifact.of_report_type(:codequality)) } let(:pipeline_with_report) { create(:ci_pipeline, :with_codequality_reports) } @@ -729,14 +771,14 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it 'selects the pipeline' do pipeline_with_report = create(:ci_pipeline, :with_terraform_reports) - expect(described_class.with_reports(Ci::JobArtifact.terraform_reports)).to eq( + expect(described_class.with_reports(Ci::JobArtifact.of_report_type(:terraform))).to eq( [pipeline_with_report] ) end end context 'when pipeline does not have metrics reports' do - subject { described_class.with_reports(Ci::JobArtifact.test_reports) } + subject { described_class.with_reports(Ci::JobArtifact.of_report_type(:test)) } let!(:pipeline_without_report) { create(:ci_empty_pipeline) } @@ -1375,32 +1417,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let(:pipeline) { build(:ci_empty_pipeline, :created) } before do - create(:ci_stage, project: project, - pipeline: pipeline, - position: 4, - name: 'deploy') - - create(:ci_build, project: project, - pipeline: pipeline, - stage: 'test', - stage_idx: 3, - name: 'test') - - create(:ci_build, project: project, - pipeline: pipeline, - stage: 'build', - stage_idx: 2, - name: 'build') - - create(:ci_stage, project: project, - pipeline: pipeline, - position: 1, - name: 'sanity') - - create(:ci_stage, project: project, - pipeline: pipeline, - position: 5, - name: 'cleanup') + create(:ci_stage, project: project, pipeline: pipeline, position: 4, name: 'deploy') + create(:ci_build, project: project, pipeline: pipeline, stage: 'test', stage_idx: 3, name: 'test') + create(:ci_build, project: project, pipeline: pipeline, stage: 'build', stage_idx: 2, name: 'build') + create(:ci_stage, project: project, pipeline: pipeline, position: 1, name: 'sanity') + create(:ci_stage, project: project, pipeline: pipeline, position: 5, name: 'cleanup') end subject { pipeline.stages } @@ -1577,6 +1598,42 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe 'track artifact report' do + let(:pipeline) { create(:ci_pipeline, :running, :with_test_reports, status: :running, user: create(:user)) } + + context 'when transitioning to completed status' do + %i[drop! skip! succeed! cancel!].each do |command| + it "performs worker on transition to #{command}" do + expect(Ci::JobArtifacts::TrackArtifactReportWorker).to receive(:perform_async).with(pipeline.id) + pipeline.send(command) + end + end + end + + context 'when pipeline retried from failed to success', :clean_gitlab_redis_shared_state do + let(:test_event_name) { 'i_testing_test_report_uploaded' } + let(:start_time) { 1.week.ago } + let(:end_time) { 1.week.from_now } + + it 'counts only one report' do + expect(Ci::JobArtifacts::TrackArtifactReportWorker).to receive(:perform_async).with(pipeline.id).twice.and_call_original + + Sidekiq::Testing.inline! do + pipeline.drop! + pipeline.run! + pipeline.succeed! + end + + unique_pipeline_pass = Gitlab::UsageDataCounters::HLLRedisCounter.unique_events( + event_names: test_event_name, + start_date: start_time, + end_date: end_time + ) + expect(unique_pipeline_pass).to eq(1) + end + end + end + describe 'merge request metrics' do let(:pipeline) { create(:ci_empty_pipeline, status: from_status) } @@ -1649,9 +1706,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when auto merge is enabled' do let_it_be_with_reload(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } let_it_be_with_reload(:pipeline) do - create(:ci_pipeline, :running, project: merge_request.source_project, - ref: merge_request.source_branch, - sha: merge_request.diff_head_sha) + create(:ci_pipeline, :running, + project: merge_request.source_project, ref: merge_request.source_branch, sha: merge_request.diff_head_sha) end before_all do @@ -3615,8 +3671,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#environments_in_self_and_descendants' do - subject { pipeline.environments_in_self_and_descendants } + describe '#environments_in_self_and_project_descendants' do + subject { pipeline.environments_in_self_and_project_descendants } context 'when pipeline is not child nor parent' do let_it_be(:pipeline) { create(:ci_pipeline, :created) } @@ -4022,13 +4078,13 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#self_and_descendants_complete?' do + describe '#self_and_project_descendants_complete?' do let_it_be(:pipeline) { create(:ci_pipeline, :success) } let_it_be(:child_pipeline) { create(:ci_pipeline, :success, child_of: pipeline) } let_it_be_with_reload(:grandchild_pipeline) { create(:ci_pipeline, :success, child_of: child_pipeline) } context 'when all pipelines in the hierarchy is complete' do - it { expect(pipeline.self_and_descendants_complete?).to be(true) } + it { expect(pipeline.self_and_project_descendants_complete?).to be(true) } end context 'when a pipeline in the hierarchy is not complete' do @@ -4036,12 +4092,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do grandchild_pipeline.update!(status: :running) end - it { expect(pipeline.self_and_descendants_complete?).to be(false) } + it { expect(pipeline.self_and_project_descendants_complete?).to be(false) } end end - describe '#builds_in_self_and_descendants' do - subject(:builds) { pipeline.builds_in_self_and_descendants } + describe '#builds_in_self_and_project_descendants' do + subject(:builds) { pipeline.builds_in_self_and_project_descendants } let(:pipeline) { create(:ci_pipeline) } let!(:build) { create(:ci_build, pipeline: pipeline) } @@ -4073,7 +4129,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#build_with_artifacts_in_self_and_descendants' do + describe '#build_with_artifacts_in_self_and_project_descendants' do let_it_be(:pipeline) { create(:ci_pipeline) } let!(:build) { create(:ci_build, name: 'test', pipeline: pipeline) } @@ -4081,14 +4137,14 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let!(:child_build) { create(:ci_build, :artifacts, name: 'test', pipeline: child_pipeline) } it 'returns the build with a given name, having artifacts' do - expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(child_build) + expect(pipeline.build_with_artifacts_in_self_and_project_descendants('test')).to eq(child_build) end context 'when same job name is present in both parent and child pipeline' do let!(:build) { create(:ci_build, :artifacts, name: 'test', pipeline: pipeline) } it 'returns the job in the parent pipeline' do - expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(build) + expect(pipeline.build_with_artifacts_in_self_and_project_descendants('test')).to eq(build) end end end @@ -4158,7 +4214,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do test_build = create(:ci_build, :test_reports, pipeline: pipeline) create(:ci_build, :coverage_reports, pipeline: pipeline) - expect(pipeline.latest_report_builds(Ci::JobArtifact.test_reports)).to contain_exactly(test_build) + expect(pipeline.latest_report_builds(Ci::JobArtifact.of_report_type(:test))).to contain_exactly(test_build) end it 'only returns not retried builds' do @@ -4169,7 +4225,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#latest_report_builds_in_self_and_descendants' do + describe '#latest_report_builds_in_self_and_project_descendants' do let_it_be(:pipeline) { create(:ci_pipeline, project: project) } let_it_be(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } let_it_be(:grandchild_pipeline) { create(:ci_pipeline, child_of: child_pipeline) } @@ -4179,26 +4235,57 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do child_build = create(:ci_build, :coverage_reports, pipeline: child_pipeline) grandchild_build = create(:ci_build, :codequality_reports, pipeline: grandchild_pipeline) - expect(pipeline.latest_report_builds_in_self_and_descendants).to contain_exactly(parent_build, child_build, grandchild_build) + expect(pipeline.latest_report_builds_in_self_and_project_descendants).to contain_exactly(parent_build, child_build, grandchild_build) end it 'filters builds by scope' do create(:ci_build, :test_reports, pipeline: pipeline) grandchild_build = create(:ci_build, :codequality_reports, pipeline: grandchild_pipeline) - expect(pipeline.latest_report_builds_in_self_and_descendants(Ci::JobArtifact.codequality_reports)).to contain_exactly(grandchild_build) + expect(pipeline.latest_report_builds_in_self_and_project_descendants(Ci::JobArtifact.of_report_type(:codequality))).to contain_exactly(grandchild_build) end it 'only returns builds that are not retried' do create(:ci_build, :codequality_reports, :retried, pipeline: grandchild_pipeline) grandchild_build = create(:ci_build, :codequality_reports, pipeline: grandchild_pipeline) - expect(pipeline.latest_report_builds_in_self_and_descendants).to contain_exactly(grandchild_build) + expect(pipeline.latest_report_builds_in_self_and_project_descendants).to contain_exactly(grandchild_build) end end describe '#has_reports?' do - subject { pipeline.has_reports?(Ci::JobArtifact.test_reports) } + subject { pipeline.has_reports?(Ci::JobArtifact.of_report_type(:test)) } + + let(:pipeline) { create(:ci_pipeline, :running) } + + context 'when pipeline has builds with test reports' do + before do + create(:ci_build, :test_reports, pipeline: pipeline) + end + + it { is_expected.to be_truthy } + end + + context 'when pipeline does not have builds with test reports' do + before do + create(:ci_build, :artifacts, pipeline: pipeline) + end + + it { is_expected.to be_falsey } + end + + context 'when retried build has test reports but latest one has none' do + before do + create(:ci_build, :retried, :test_reports, pipeline: pipeline) + create(:ci_build, :artifacts, pipeline: pipeline) + end + + it { is_expected.to be_falsey } + end + end + + describe '#complete_and_has_reports?' do + subject { pipeline.complete_and_has_reports?(Ci::JobArtifact.of_report_type(:test)) } context 'when pipeline has builds with test reports' do before do @@ -4370,6 +4457,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do create(:ci_job_artifact, :junit_with_ant, job: build_java) end + it 'has a test suite for each job' do + expect(subject.test_suites.keys).to contain_exactly('rspec', 'java') + end + it 'returns test reports with collected data' do expect(subject.total_count).to be(7) expect(subject.success_count).to be(5) @@ -4388,6 +4479,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + context 'when the pipeline has parallel builds with test reports' do + let!(:parallel_build_1) { create(:ci_build, name: 'build 1/2', pipeline: pipeline) } + let!(:parallel_build_2) { create(:ci_build, name: 'build 2/2', pipeline: pipeline) } + + before do + create(:ci_job_artifact, :junit, job: parallel_build_1) + create(:ci_job_artifact, :junit, job: parallel_build_2) + end + + it 'merges the test suite from parallel builds' do + expect(subject.test_suites.keys).to contain_exactly('build') + end + end + + context 'the pipeline has matrix builds with test reports' do + let!(:matrix_build_1) { create(:ci_build, :matrix, pipeline: pipeline) } + let!(:matrix_build_2) { create(:ci_build, :matrix, pipeline: pipeline) } + + before do + create(:ci_job_artifact, :junit, job: matrix_build_1) + create(:ci_job_artifact, :junit, job: matrix_build_2) + end + + it 'keeps separate test suites for each matrix build' do + expect(subject.test_suites.keys).to contain_exactly(matrix_build_1.name, matrix_build_2.name) + end + end + context 'when pipeline does not have any builds with test reports' do it 'returns empty test reports' do expect(subject.total_count).to be(0) @@ -4472,10 +4591,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep 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) + context 'when the processable is a bridge' do + it 'returns true' do + create(:ci_bridge, pipeline: pipeline, scheduling_type: :dag) + + expect(pipeline.uses_needs?).to eq(true) + end + end - expect(pipeline.uses_needs?).to eq(true) + context 'when the processable is a build' do + it 'returns true' do + create(:ci_build, pipeline: pipeline, scheduling_type: :dag) + + expect(pipeline.uses_needs?).to eq(true) + end end end @@ -4911,8 +5040,58 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end - describe '#self_and_ancestors' do - subject(:self_and_ancestors) { pipeline.self_and_ancestors } + describe '#self_and_downstreams' do + subject(:self_and_downstreams) { pipeline.self_and_downstreams } + + let(:pipeline) { create(:ci_pipeline, :created) } + + context 'when pipeline is not child nor parent' do + it 'returns just the pipeline itself' do + expect(self_and_downstreams).to contain_exactly(pipeline) + end + end + + context 'when pipeline is child' do + let(:parent) { create(:ci_pipeline) } + let!(:pipeline) { create(:ci_pipeline, child_of: parent) } + + it 'returns self and no ancestors' do + expect(self_and_downstreams).to contain_exactly(pipeline) + end + end + + context 'when pipeline is parent' do + let(:child) { create(:ci_pipeline, child_of: pipeline) } + + it 'returns self and child' do + expect(self_and_downstreams).to contain_exactly(pipeline, child) + end + end + + context 'when pipeline is a grandparent pipeline' do + let(:child) { create(:ci_pipeline, child_of: pipeline) } + let(:grandchild) { create(:ci_pipeline, child_of: child) } + + it 'returns self, child, and grandchild' do + expect(self_and_downstreams).to contain_exactly(pipeline, child, grandchild) + end + end + + context 'when pipeline is a triggered pipeline from a different project' do + let(:downstream) { create(:ci_pipeline) } + + before do + create_source_pipeline(pipeline, downstream) + end + + it 'returns self and cross-project downstream' do + expect(self_and_downstreams).to contain_exactly(pipeline, downstream) + end + end + end + + describe '#self_and_project_ancestors' do + subject(:self_and_project_ancestors) { pipeline.self_and_project_ancestors } context 'when pipeline is child' do let(:pipeline) { create(:ci_pipeline, :created) } @@ -4925,7 +5104,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it 'returns parent and self' do - expect(self_and_ancestors).to contain_exactly(parent, pipeline) + expect(self_and_project_ancestors).to contain_exactly(parent, pipeline) end end @@ -4939,7 +5118,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end it 'returns only self' do - expect(self_and_ancestors).to contain_exactly(pipeline) + expect(self_and_project_ancestors).to contain_exactly(pipeline) + end + end + end + + describe '#complete_hierarchy_count' do + context 'with a combination of ancestor, descendant and sibling pipelines' do + let!(:pipeline) { create(:ci_pipeline) } + let!(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:sibling_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:grandchild_pipeline) { create(:ci_pipeline, child_of: child_pipeline) } + + it 'counts the whole tree' do + expect(sibling_pipeline.complete_hierarchy_count).to eq(4) end end end @@ -5165,16 +5357,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to be_falsey } end - context 'when the pipeline is still running' do - let(:pipeline) { create(:ci_pipeline, :running) } + context 'when the pipeline is still running and with test reports' do + let(:pipeline) { create(:ci_pipeline, :running, :with_test_reports) } - it { is_expected.to be_falsey } - end - - context 'when the pipeline is completed without test reports' do - let(:pipeline) { create(:ci_pipeline, :success) } - - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end end @@ -5298,4 +5484,36 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + describe 'partitioning' do + let(:pipeline) { build(:ci_pipeline) } + + before do + allow(described_class).to receive(:current_partition_value) { 123 } + end + + it 'sets partition_id to the current partition value' do + expect { pipeline.valid? }.to change(pipeline, :partition_id).to(123) + end + + context 'when it is already set' do + let(:pipeline) { build(:ci_pipeline, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { pipeline.valid? }.not_to change(pipeline, :partition_id) + end + end + + context 'without current partition value' do + before do + allow(described_class).to receive(:current_partition_value) {} + end + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { pipeline.valid? }.not_to change(pipeline, :partition_id) + end + end + end end diff --git a/spec/models/ci/pipeline_variable_spec.rb b/spec/models/ci/pipeline_variable_spec.rb index 4e8d49585d0..fdcec0e96af 100644 --- a/spec/models/ci/pipeline_variable_spec.rb +++ b/spec/models/ci/pipeline_variable_spec.rb @@ -17,4 +17,25 @@ RSpec.describe Ci::PipelineVariable do it { is_expected.to be_a(Hash) } it { is_expected.to eq({ key: 'foo', value: 'bar' }) } end + + describe 'partitioning' do + context 'with pipeline' do + let(:pipeline) { build(:ci_pipeline, partition_id: 123) } + let(:variable) { build(:ci_pipeline_variable, pipeline: pipeline, partition_id: nil) } + + it 'copies the partition_id from pipeline' do + expect { variable.valid? }.to change(variable, :partition_id).from(nil).to(123) + end + end + + context 'without pipeline' do + subject(:variable) { build(:ci_pipeline_variable, pipeline: nil, partition_id: nil) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { variable.valid? }.not_to change(variable, :partition_id) + end + end + end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 127a1417d9e..61e2864a518 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -30,10 +30,8 @@ RSpec.describe Ci::Processable do let_it_be(:downstream_project) { create(:project, :repository) } let_it_be_with_refind(:processable) do - create( - :ci_bridge, :success, pipeline: pipeline, downstream: downstream_project, - description: 'a trigger job', stage_id: stage.id - ) + create(:ci_bridge, :success, + pipeline: pipeline, downstream: downstream_project, description: 'a trigger job', stage_id: stage.id) end let(:clone_accessors) { ::Ci::Bridge.clone_accessors } @@ -57,8 +55,7 @@ RSpec.describe Ci::Processable do let(:clone_accessors) { ::Ci::Build.clone_accessors.without(::Ci::Build.extra_accessors) } let(:reject_accessors) do - %i[id status user token_encrypted coverage trace runner - artifacts_expire_at + %i[id status user token_encrypted coverage runner artifacts_expire_at created_at updated_at started_at finished_at queued_at erased_by erased_at auto_canceled_by job_artifacts job_artifacts_archive job_artifacts_metadata job_artifacts_trace job_artifacts_junit @@ -86,7 +83,7 @@ RSpec.describe Ci::Processable do resource resource_group_id processed security_scans author pipeline_id report_results pending_state pages_deployments queuing_entry runtime_metadata trace_metadata - dast_site_profile dast_scanner_profile].freeze + dast_site_profile dast_scanner_profile stage_id].freeze end before_all do @@ -208,10 +205,11 @@ RSpec.describe Ci::Processable do let(:environment_name) { 'review/$CI_COMMIT_REF_SLUG-$GITLAB_USER_ID' } let!(:processable) do - create(:ci_build, :with_deployment, environment: environment_name, - options: { environment: { name: environment_name } }, - pipeline: pipeline, stage_id: stage.id, project: project, - user: other_developer) + create(:ci_build, :with_deployment, + environment: environment_name, + options: { environment: { name: environment_name } }, + pipeline: pipeline, stage_id: stage.id, project: project, + user: other_developer) end it 're-uses the previous persisted environment' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ae8748f8ae3..181351222c1 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -266,13 +266,13 @@ RSpec.describe Ci::Runner do end shared_examples '.belonging_to_parent_group_of_project' do - let!(:group1) { create(:group) } - let!(:project1) { create(:project, group: group1) } - let!(:runner1) { create(:ci_runner, :group, groups: [group1]) } + let_it_be(:group1) { create(:group) } + let_it_be(:project1) { create(:project, group: group1) } + let_it_be(:runner1) { create(:ci_runner, :group, groups: [group1]) } - let!(:group2) { create(:group) } - let!(:project2) { create(:project, group: group2) } - let!(:runner2) { create(:ci_runner, :group, groups: [group2]) } + let_it_be(:group2) { create(:group) } + let_it_be(:project2) { create(:project, group: group2) } + let_it_be(:runner2) { create(:ci_runner, :group, groups: [group2]) } let(:project_id) { project1.id } @@ -495,8 +495,8 @@ RSpec.describe Ci::Runner do describe '.active' do subject { described_class.active(active_value) } - let!(:runner1) { create(:ci_runner, :instance, active: false) } - let!(:runner2) { create(:ci_runner, :instance) } + let_it_be(:runner1) { create(:ci_runner, :instance, active: false) } + let_it_be(:runner2) { create(:ci_runner, :instance) } context 'with active_value set to false' do let(:active_value) { false } @@ -544,7 +544,7 @@ RSpec.describe Ci::Runner do end describe '#stale?', :clean_gitlab_redis_cache do - let(:runner) { create(:ci_runner, :instance) } + let(:runner) { build(:ci_runner, :instance) } subject { runner.stale? } @@ -619,7 +619,7 @@ RSpec.describe Ci::Runner do end describe '#online?', :clean_gitlab_redis_cache do - let(:runner) { create(:ci_runner, :instance) } + let(:runner) { build(:ci_runner, :instance) } subject { runner.online? } @@ -1016,7 +1016,7 @@ RSpec.describe Ci::Runner do let!(:last_update) { runner.ensure_runner_queue_value } before do - Ci::Runners::UpdateRunnerService.new(runner).update(description: 'new runner') # rubocop: disable Rails/SaveBang + Ci::Runners::UpdateRunnerService.new(runner).execute(description: 'new runner') end it 'sets a new last_update value' do @@ -1162,13 +1162,13 @@ RSpec.describe Ci::Runner do end describe '.assignable_for' do - let(:project) { create(:project) } - let(:group) { create(:group) } - let(:another_project) { create(:project) } - let!(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) } - let!(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) } - let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } - let!(:instance_runner) { create(:ci_runner, :instance) } + let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:another_project) { create(:project) } + let_it_be(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) } + let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:instance_runner) { create(:ci_runner, :instance) } context 'with already assigned project' do subject { described_class.assignable_for(project) } @@ -1186,59 +1186,74 @@ RSpec.describe Ci::Runner do end end - describe "belongs_to_one_project?" do - it "returns false if there are two projects runner assigned to" do - project1 = create(:project) - project2 = create(:project) - runner = create(:ci_runner, :project, projects: [project1, project2]) + context 'Project-related queries' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + + describe '#owner_project' do + subject(:owner_project) { project_runner.owner_project } + + context 'with project1 as first project associated with runner' do + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project1, project2]) } + + it { is_expected.to eq project1 } + end + + context 'with project2 as first project associated with runner' do + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project2, project1]) } - expect(runner.belongs_to_one_project?).to be_falsey + it { is_expected.to eq project2 } + end end - it "returns true" do - project = create(:project) - runner = create(:ci_runner, :project, projects: [project]) + describe "belongs_to_one_project?" do + it "returns false if there are two projects runner is assigned to" do + runner = create(:ci_runner, :project, projects: [project1, project2]) + + expect(runner.belongs_to_one_project?).to be_falsey + end - expect(runner.belongs_to_one_project?).to be_truthy + it "returns true if there is only one project runner is assigned to" do + runner = create(:ci_runner, :project, projects: [project1]) + + expect(runner.belongs_to_one_project?).to be_truthy + end end - end - describe '#belongs_to_more_than_one_project?' do - context 'project runner' do - let(:project1) { create(:project) } - let(:project2) { create(:project) } + describe '#belongs_to_more_than_one_project?' do + context 'project runner' do + context 'two projects assigned to runner' do + let(:runner) { create(:ci_runner, :project, projects: [project1, project2]) } + + it 'returns true' do + expect(runner.belongs_to_more_than_one_project?).to be_truthy + end + end - context 'two projects assigned to runner' do - let(:runner) { create(:ci_runner, :project, projects: [project1, project2]) } + context 'one project assigned to runner' do + let(:runner) { create(:ci_runner, :project, projects: [project1]) } - it 'returns true' do - expect(runner.belongs_to_more_than_one_project?).to be_truthy + it 'returns false' do + expect(runner.belongs_to_more_than_one_project?).to be_falsey + end end end - context 'one project assigned to runner' do - let(:runner) { create(:ci_runner, :project, projects: [project1]) } + context 'group runner' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'returns false' do expect(runner.belongs_to_more_than_one_project?).to be_falsey end end - end - - context 'group runner' do - let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :group, groups: [group]) } - - it 'returns false' do - expect(runner.belongs_to_more_than_one_project?).to be_falsey - end - end - context 'shared runner' do - let(:runner) { create(:ci_runner, :instance) } + context 'shared runner' do + let(:runner) { create(:ci_runner, :instance) } - it 'returns false' do - expect(runner.belongs_to_more_than_one_project?).to be_falsey + it 'returns false' do + expect(runner.belongs_to_more_than_one_project?).to be_falsey + end end end end @@ -1299,7 +1314,7 @@ RSpec.describe Ci::Runner do end describe '.search' do - let(:runner) { create(:ci_runner, token: '123abc', description: 'test runner') } + let_it_be(:runner) { create(:ci_runner, token: '123abc', description: 'test runner') } it 'returns runners with a matching token' do expect(described_class.search(runner.token)).to eq([runner]) @@ -1326,57 +1341,10 @@ RSpec.describe Ci::Runner do end end - describe '#assigned_to_group?' do - subject { runner.assigned_to_group? } - - context 'when project runner' do - let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } - let(:project) { create(:project) } - - it { is_expected.to be_falsey } - end - - context 'when shared runner' do - let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } - - it { is_expected.to be_falsey } - end - - context 'when group runner' do - let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } - - it { is_expected.to be_truthy } - end - end - - describe '#assigned_to_project?' do - subject { runner.assigned_to_project? } - - context 'when group runner' do - let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } - let(:group) { create(:group) } - - it { is_expected.to be_falsey } - end - - context 'when shared runner' do - let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } - - it { is_expected.to be_falsey } - end - - context 'when project runner' do - let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } - let(:project) { create(:project) } - - it { is_expected.to be_truthy } - end - end - describe '#pick_build!' do + let_it_be(:runner) { create(:ci_runner) } + let(:build) { create(:ci_build) } - let(:runner) { create(:ci_runner) } context 'runner can pick the build' do it 'calls #tick_runner_queue' do @@ -1413,26 +1381,26 @@ RSpec.describe Ci::Runner do end describe '.order_by' do + let_it_be(:runner1) { create(:ci_runner, created_at: 1.year.ago, contacted_at: 1.year.ago) } + let_it_be(:runner2) { create(:ci_runner, created_at: 1.month.ago, contacted_at: 1.month.ago) } + + before do + runner1.update!(token_expires_at: 1.year.from_now) + end + it 'supports ordering by the contact date' do - runner1 = create(:ci_runner, contacted_at: 1.year.ago) - runner2 = create(:ci_runner, contacted_at: 1.month.ago) runners = described_class.order_by('contacted_asc') expect(runners).to eq([runner1, runner2]) end it 'supports ordering by the creation date' do - runner1 = create(:ci_runner, created_at: 1.year.ago) - runner2 = create(:ci_runner, created_at: 1.month.ago) runners = described_class.order_by('created_asc') expect(runners).to eq([runner2, runner1]) end it 'supports ordering by the token expiration' do - runner1 = create(:ci_runner) - runner1.update!(token_expires_at: 1.year.from_now) - runner2 = create(:ci_runner) runner3 = create(:ci_runner) runner3.update!(token_expires_at: 1.month.from_now) diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index d55a8509a98..dd9af33a562 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -369,4 +369,33 @@ RSpec.describe Ci::Stage, :models do let!(:model) { create(:ci_stage, project: parent) } end end + + describe 'partitioning' do + context 'with pipeline' do + let(:pipeline) { build(:ci_pipeline, partition_id: 123) } + let(:stage) { build(:ci_stage, pipeline: pipeline) } + + it 'copies the partition_id from pipeline' do + expect { stage.valid? }.to change(stage, :partition_id).to(123) + end + + context 'when it is already set' do + let(:stage) { build(:ci_stage, pipeline: pipeline, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { stage.valid? }.not_to change(stage, :partition_id) + end + end + end + + context 'without pipeline' do + subject(:stage) { build(:ci_stage, pipeline: nil) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { stage.valid? }.not_to change(stage, :partition_id) + end + end + end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 4ac8720780c..8517e583ec7 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -20,6 +20,7 @@ RSpec.describe Ci::Trigger do trigger = create(:ci_trigger_without_token, project: project) expect(trigger.token).not_to be_nil + expect(trigger.token).to start_with(Ci::Trigger::TRIGGER_TOKEN_PREFIX) end it 'does not set a random token if one provided' do @@ -30,12 +31,22 @@ RSpec.describe Ci::Trigger do end describe '#short_token' do - let(:trigger) { create(:ci_trigger, token: '12345678') } + let(:trigger) { create(:ci_trigger) } subject { trigger.short_token } - it 'returns shortened token' do - is_expected.to eq('1234') + it 'returns shortened token without prefix' do + is_expected.not_to start_with(Ci::Trigger::TRIGGER_TOKEN_PREFIX) + end + + context 'token does not have a prefix' do + before do + trigger.token = '12345678' + end + + it 'returns shortened token' do + is_expected.to eq('1234') + end end end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index a0ede9fb0d9..7b9ff409edd 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -605,15 +605,15 @@ RSpec.describe Clusters::Platforms::Kubernetes do { 'app.gitlab.com/app' => project.full_path_slug, 'app.gitlab.com/env' => 'env-000000' } ]) expect(rollout_status.instances).to eq([{ pod_name: "kube-pod", - stable: true, - status: "pending", - tooltip: "kube-pod (Pending)", - track: "stable" }, + stable: true, + status: "pending", + tooltip: "kube-pod (Pending)", + track: "stable" }, { pod_name: "Not provided", - stable: true, - status: "pending", - tooltip: "Not provided (Pending)", - track: "stable" }]) + stable: true, + status: "pending", + tooltip: "Not provided (Pending)", + track: "stable" }]) end context 'with canary ingress' do diff --git a/spec/models/commit_signatures/gpg_signature_spec.rb b/spec/models/commit_signatures/gpg_signature_spec.rb index 6ae2a202b72..605ad725dd7 100644 --- a/spec/models/commit_signatures/gpg_signature_spec.rb +++ b/spec/models/commit_signatures/gpg_signature_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe CommitSignatures::GpgSignature do # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test # For instructions on how to add more seed data, see the project README - let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - let!(:project) { create(:project, :repository, path: 'sample-project') } - let!(:commit) { create(:commit, project: project, sha: commit_sha) } - let(:signature) { create(:gpg_signature, commit_sha: commit_sha) } - let(:gpg_key) { create(:gpg_key) } - let(:gpg_key_subkey) { create(:gpg_key_subkey) } + let_it_be(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let_it_be(:project) { create(:project, :repository, path: 'sample-project') } + let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) } + let_it_be(:gpg_key) { create(:gpg_key) } + let_it_be(:gpg_key_subkey) { create(:gpg_key_subkey, gpg_key: gpg_key) } + + let(:signature) { create(:gpg_signature, commit_sha: commit_sha, gpg_key: gpg_key) } + let(:attributes) do { commit_sha: commit_sha, @@ -35,8 +37,7 @@ RSpec.describe CommitSignatures::GpgSignature do end describe '.by_commit_sha scope' do - let(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) } - let!(:another_gpg_signature) { create(:gpg_signature, gpg_key: gpg_key) } + let_it_be(:another_gpg_signature) { create(:gpg_signature, gpg_key: gpg_key) } it 'returns all gpg signatures by sha' do expect(described_class.by_commit_sha(commit_sha)).to match_array([signature]) diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb index 64d95fe3a71..08530bf6964 100644 --- a/spec/models/commit_signatures/ssh_signature_spec.rb +++ b/spec/models/commit_signatures/ssh_signature_spec.rb @@ -5,11 +5,11 @@ require 'spec_helper' RSpec.describe CommitSignatures::SshSignature do # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test # For instructions on how to add more seed data, see the project README - let(:commit_sha) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' } - let!(:project) { create(:project, :repository, path: 'sample-project') } - let!(:commit) { create(:commit, project: project, sha: commit_sha) } - let(:signature) { create(:ssh_signature, commit_sha: commit_sha) } - let(:ssh_key) { create(:ed25519_key_256) } + let_it_be(:commit_sha) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' } + let_it_be(:project) { create(:project, :repository, path: 'sample-project') } + let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) } + let_it_be(:ssh_key) { create(:ed25519_key_256) } + let(:attributes) do { commit_sha: commit_sha, @@ -18,6 +18,8 @@ RSpec.describe CommitSignatures::SshSignature do } end + let(:signature) { create(:ssh_signature, commit_sha: commit_sha, key: ssh_key) } + it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' diff --git a/spec/models/commit_signatures/x509_commit_signature_spec.rb b/spec/models/commit_signatures/x509_commit_signature_spec.rb index beb101cdd89..b971fd078e2 100644 --- a/spec/models/commit_signatures/x509_commit_signature_spec.rb +++ b/spec/models/commit_signatures/x509_commit_signature_spec.rb @@ -5,11 +5,10 @@ require 'spec_helper' RSpec.describe CommitSignatures::X509CommitSignature do # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test # For instructions on how to add more seed data, see the project README - let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } - let(:project) { create(:project, :public, :repository) } - let!(:commit) { create(:commit, project: project, sha: commit_sha) } - let(:x509_certificate) { create(:x509_certificate) } - let(:signature) { create(:x509_commit_signature, commit_sha: commit_sha) } + let_it_be(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) } + let_it_be(:x509_certificate) { create(:x509_certificate) } let(:attributes) do { @@ -20,6 +19,8 @@ RSpec.describe CommitSignatures::X509CommitSignature do } end + let(:signature) { create(:x509_commit_signature, commit_sha: commit_sha, x509_certificate: x509_certificate) } + it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 08d770a1beb..bab6247d4f9 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -84,6 +84,22 @@ RSpec.describe Commit do end end + describe '.build_from_sidekiq_hash' do + it 'returns a Commit' do + commit = described_class.build_from_sidekiq_hash(project, id: '123') + + expect(commit).to be_an_instance_of(Commit) + end + + it 'parses date strings into Time instances' do + commit = described_class.build_from_sidekiq_hash(project, + id: '123', + authored_date: Time.current.to_s) + + expect(commit.authored_date).to be_a_kind_of(Time) + end + end + describe '#diff_refs' do it 'is equal to itself' do expect(commit.diff_refs).to eq(commit.diff_refs) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 78d4d9de84e..adbd20b6730 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -836,17 +836,11 @@ RSpec.describe CommitStatus do context 'when commit status does not have stage but it exists' do let!(:stage) do - create(:ci_stage, project: project, - pipeline: pipeline, - name: 'test') + create(:ci_stage, project: project, pipeline: pipeline, name: 'test') end let(:commit_status) do - create(:commit_status, project: project, - pipeline: pipeline, - name: 'rspec', - stage: 'test', - status: :success) + create(:commit_status, project: project, pipeline: pipeline, name: 'rspec', stage: 'test', status: :success) end it 'uses existing stage', :sidekiq_might_not_need_inline do @@ -1008,4 +1002,53 @@ RSpec.describe CommitStatus do let!(:model) { create(:ci_build, runner: parent) } end end + + describe '.stage_name' do + subject(:stage_name) { commit_status.stage_name } + + it 'returns the stage name' do + expect(stage_name).to eq('test') + end + + context 'when ci stage is not present' do + before do + commit_status.ci_stage = nil + end + + it { is_expected.to be_nil } + end + end + + describe 'partitioning' do + context 'with pipeline' do + let(:pipeline) { build(:ci_pipeline, partition_id: 123) } + let(:status) { build(:commit_status, pipeline: pipeline) } + + it 'copies the partition_id from pipeline' do + expect { status.valid? }.to change(status, :partition_id).to(123) + end + + context 'when it is already set' do + let(:status) { build(:commit_status, pipeline: pipeline, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { status.valid? }.not_to change(status, :partition_id) + end + end + end + + context 'without pipeline' do + subject(:status) do + build(:commit_status, + project: build_stubbed(:project), + pipeline: nil) + end + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { status.valid? }.not_to change(status, :partition_id) + end + end + end end diff --git a/spec/models/concerns/approvable_base_spec.rb b/spec/models/concerns/approvable_spec.rb index 2bf6a98a64d..1ddd9b3edca 100644 --- a/spec/models/concerns/approvable_base_spec.rb +++ b/spec/models/concerns/approvable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ApprovableBase do +RSpec.describe Approvable do let(:merge_request) { create(:merge_request) } let(:user) { create(:user) } diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index 64691165e21..6af244a5a0f 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -46,30 +46,8 @@ RSpec.describe Ci::Artifactable do end end - context 'when file format is zip' do - context 'when artifact contains one file' do - let(:artifact) { build(:ci_job_artifact, :zip_with_single_file) } - - it 'iterates blob once' do - expect { |b| artifact.each_blob(&b) }.to yield_control.once - end - end - - context 'when artifact contains two files' do - let(:artifact) { build(:ci_job_artifact, :zip_with_multiple_files) } - - it 'iterates blob two times' do - expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(2).times - end - end - end - context 'when there are no adapters for the file format' do - let(:artifact) { build(:ci_job_artifact, :junit) } - - before do - allow(artifact).to receive(:file_format).and_return(:unknown) - end + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } it 'raises an error' do expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError) diff --git a/spec/models/concerns/ci/has_deployment_name_spec.rb b/spec/models/concerns/ci/has_deployment_name_spec.rb deleted file mode 100644 index 8c7338638b1..00000000000 --- a/spec/models/concerns/ci/has_deployment_name_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::HasDeploymentName do - describe 'deployment_name?' do - let(:build) { create(:ci_build) } - - subject { build.branch? } - - it 'does detect deployment names' do - build.name = 'deployment' - - expect(build.deployment_name?).to be_truthy - end - - it 'does detect partial deployment names' do - build.name = 'do a really cool deploy' - - expect(build.deployment_name?).to be_truthy - end - - it 'does not detect non-deployment names' do - build.name = 'testing' - - expect(build.deployment_name?).to be_falsy - end - - it 'is case insensitive' do - build.name = 'DEPLOY' - expect(build.deployment_name?).to be_truthy - end - end -end diff --git a/spec/models/concerns/ci/track_environment_usage_spec.rb b/spec/models/concerns/ci/track_environment_usage_spec.rb new file mode 100644 index 00000000000..d75972c49b5 --- /dev/null +++ b/spec/models/concerns/ci/track_environment_usage_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::TrackEnvironmentUsage do + describe '#verifies_environment?' do + subject { build.verifies_environment? } + + context 'when build is the verify action for the environment' do + let(:build) do + build_stubbed(:ci_build, + ref: 'master', + environment: 'staging', + options: { environment: { action: 'verify' } }) + end + + it { is_expected.to be_truthy } + end + + context 'when build is not the verify action for the environment' do + let(:build) do + build_stubbed(:ci_build, + ref: 'master', + environment: 'staging', + options: { environment: { action: 'start' } }) + end + + it { is_expected.to be_falsey } + end + end + + describe 'deployment_name?' do + let(:build) { create(:ci_build) } + + subject { build.branch? } + + it 'does detect deployment names' do + build.name = 'deployment' + + expect(build).to be_deployment_name + end + + it 'does detect partial deployment names' do + build.name = 'do a really cool deploy' + + expect(build).to be_deployment_name + end + + it 'does not detect non-deployment names' do + build.name = 'testing' + + expect(build).not_to be_deployment_name + end + + it 'is case insensitive' do + build.name = 'DEPLOY' + + expect(build).to be_deployment_name + end + end +end diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index 8d32ef14f47..2dd70188740 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -79,4 +79,14 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ end end end + + describe '.counter_attribute_enabled?' do + it 'is true when counter attribute is defined' do + expect(CounterAttributeModel.counter_attribute_enabled?(:build_artifacts_size)).to be_truthy + end + + it 'is false when counter attribute is not defined' do + expect(CounterAttributeModel.counter_attribute_enabled?(:nope)).to be_falsey + end + end end diff --git a/spec/models/concerns/from_set_operator_spec.rb b/spec/models/concerns/from_set_operator_spec.rb index 8ebbb5550c9..1ef0d1adb08 100644 --- a/spec/models/concerns/from_set_operator_spec.rb +++ b/spec/models/concerns/from_set_operator_spec.rb @@ -3,7 +3,22 @@ require 'spec_helper' RSpec.describe FromSetOperator do - describe 'when set operator method already exists' do + let_it_be(:from_set_operator) do + Class.new do + extend FromSetOperator + define_set_operator Gitlab::SQL::Union + + def table_name + 'groups' + end + + def from(*args) + '' + end + end + end + + context 'when set operator method already exists' do let(:redefine_method) do Class.new do def self.from_union @@ -17,4 +32,38 @@ RSpec.describe FromSetOperator do it { expect { redefine_method }.to raise_exception(RuntimeError) } end + + context 'with members' do + let_it_be(:group1) { create :group } + let_it_be(:group2) { create :group } + let_it_be(:groups) do + [ + Group.where(id: group1), + Group.where(id: group2) + ] + end + + shared_examples 'set operator called with correct members' do + it do + expect(Gitlab::SQL::Union).to receive(:new).with(groups, anything).and_call_original + subject + end + end + + context 'as array' do + subject { from_set_operator.new.from_union(groups) } + + it_behaves_like 'set operator called with correct members' + + it { expect { subject }.not_to make_queries } + end + + context 'as multiple parameters' do + subject { from_set_operator.new.from_union(*groups) } + + it_behaves_like 'set operator called with correct members' + + it { expect { subject }.not_to make_queries } + end + end end diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 55e3caf3c4c..3e42a3504ac 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -96,6 +96,7 @@ RSpec.describe PgFullTextSearchable do it 'ignores accents' do expect(model_class.pg_full_text_search('jurgen')).to contain_exactly(with_accent) + expect(model_class.pg_full_text_search('Jürgen')).to contain_exactly(with_accent) end it 'does not support searching by non-Latin characters' do diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index b49b9ce8a2a..89f34834aa4 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -8,6 +8,7 @@ RSpec.describe ProjectFeaturesCompatibility do let(:features) do features_enabled + %w( repository pages operations container_registry package_registry environments feature_flags releases + monitor ) end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index cb9bb676ede..039b9e574fe 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -281,7 +281,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do end it 'does not delete the value key' do - expect(Rails.cache).to receive(:delete).with(cache_key).never + expect(Rails.cache).not_to receive(:delete).with(cache_key) go! end @@ -338,7 +338,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do context 'when lifetime is exceeded' do it 'skips the calculation' do - expect(instance).to receive(:calculate_reactive_cache).never + expect(instance).not_to receive(:calculate_reactive_cache) go! end @@ -354,7 +354,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do it 'skips the calculation' do stub_exclusive_lease_taken(cache_key) - expect(instance).to receive(:calculate_reactive_cache).never + expect(instance).not_to receive(:calculate_reactive_cache) go! end diff --git a/spec/models/container_registry/event_spec.rb b/spec/models/container_registry/event_spec.rb index 799d9d4fd1c..c2c494c49fb 100644 --- a/spec/models/container_registry/event_spec.rb +++ b/spec/models/container_registry/event_spec.rb @@ -144,7 +144,7 @@ RSpec.describe ContainerRegistry::Event do let(:target) do { 'mediaType' => ContainerRegistry::Client::DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE, - 'repository' => repository_path, + 'repository' => repository_path, 'tag' => 'latest' } end diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb index 1833fcf5385..d19a0bdf6c7 100644 --- a/spec/models/customer_relations/organization_spec.rb +++ b/spec/models/customer_relations/organization_spec.rb @@ -146,15 +146,43 @@ RSpec.describe CustomerRelations::Organization, type: :model do end end - describe '.sort_by_name' do - let_it_be(:organization_a) { create(:organization, group: group, name: "c") } + describe '.counts_by_state' do + before do + create_list(:organization, 3, group: group) + create_list(:organization, 2, group: group, state: 'inactive') + end + + it 'returns correct organization counts' do + counts = group.organizations.counts_by_state + + expect(counts['active']).to be(3) + expect(counts['inactive']).to be(2) + end + + it 'returns 0 with no results' do + counts = group.organizations.where(id: non_existing_record_id).counts_by_state + + expect(counts['active']).to be(0) + expect(counts['inactive']).to be(0) + end + end + + describe 'sorting' do + let_it_be(:organization_a) { create(:organization, group: group, name: "c", description: "1") } let_it_be(:organization_b) { create(:organization, group: group, name: "a") } - let_it_be(:organization_c) { create(:organization, group: group, name: "b") } + let_it_be(:organization_c) { create(:organization, group: group, name: "b", description: "2") } - context 'when sorting the organizations' do + describe '.sort_by_name' do it 'sorts them by name in ascendent order' do expect(group.organizations.sort_by_name).to eq([organization_b, organization_c, organization_a]) end end + + describe '.sort_by_field' do + it 'sorts them by description in descending order' do + expect(group.organizations.sort_by_field('description', :desc)) + .to eq([organization_c, organization_a, organization_b]) + end + end end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 0a4ee73f3d3..87fa5289795 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -74,6 +74,27 @@ RSpec.describe Deployment do end end + describe '.for_iid' do + subject { described_class.for_iid(project, iid) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:deployment) { create(:deployment, project: project) } + + let(:iid) { deployment.iid } + + it 'finds the deployment' do + is_expected.to contain_exactly(deployment) + end + + context 'when iid does not match' do + let(:iid) { non_existing_record_id } + + it 'does not find the deployment' do + is_expected.to be_empty + end + end + end + describe '.for_environment_name' do subject { described_class.for_environment_name(project, environment_name) } @@ -142,8 +163,8 @@ RSpec.describe Deployment do it 'executes Deployments::HooksWorker asynchronously' do freeze_time do expect(Deployments::HooksWorker) - .to receive(:perform_async).with(deployment_id: deployment.id, status: 'running', - status_changed_at: Time.current) + .to receive(:perform_async) + .with(deployment_id: deployment.id, status: 'running', status_changed_at: Time.current) deployment.run! end @@ -179,8 +200,8 @@ RSpec.describe Deployment do it 'executes Deployments::HooksWorker asynchronously' do freeze_time do expect(Deployments::HooksWorker) - .to receive(:perform_async).with(deployment_id: deployment.id, status: 'success', - status_changed_at: Time.current) + .to receive(:perform_async) + .with(deployment_id: deployment.id, status: 'success', status_changed_at: Time.current) deployment.succeed! end @@ -209,8 +230,8 @@ RSpec.describe Deployment do it 'executes Deployments::HooksWorker asynchronously' do freeze_time do expect(Deployments::HooksWorker) - .to receive(:perform_async).with(deployment_id: deployment.id, status: 'failed', - status_changed_at: Time.current) + .to receive(:perform_async) + .with(deployment_id: deployment.id, status: 'failed', status_changed_at: Time.current) deployment.drop! end @@ -239,8 +260,8 @@ RSpec.describe Deployment do it 'executes Deployments::HooksWorker asynchronously' do freeze_time do expect(Deployments::HooksWorker) - .to receive(:perform_async).with(deployment_id: deployment.id, status: 'canceled', - status_changed_at: Time.current) + .to receive(:perform_async) + .with(deployment_id: deployment.id, status: 'canceled', status_changed_at: Time.current) deployment.cancel! end @@ -343,6 +364,31 @@ RSpec.describe Deployment do end end + describe '#older_than_last_successful_deployment?' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:environment) { create(:environment, project: project) } + + subject { deployment.older_than_last_successful_deployment? } + + context 'when deployment is current deployment' do + let(:deployment) { create(:deployment, :success, project: project) } + + it { is_expected.to be_falsey } + end + + context 'when deployment is behind current deployment' do + let!(:deployment) do + create(:deployment, :success, project: project, environment: environment, finished_at: 1.year.ago) + end + + let!(:last_deployment) do + create(:deployment, :success, project: project, environment: environment) + end + + it { is_expected.to be_truthy } + end + end + describe '#success?' do subject { deployment.success? } @@ -517,6 +563,16 @@ RSpec.describe Deployment do end end + describe '.ordered_as_upcoming' do + let!(:deployment1) { create(:deployment, status: :running) } + let!(:deployment2) { create(:deployment, status: :blocked) } + let!(:deployment3) { create(:deployment, status: :created) } + + it 'sorts by ID DESC' do + expect(described_class.ordered_as_upcoming).to eq([deployment3, deployment2, deployment1]) + end + end + describe 'visible' do subject { described_class.visible } @@ -876,6 +932,22 @@ RSpec.describe Deployment do end end + describe '#build' do + let!(:deployment) { create(:deployment) } + + subject { deployment.build } + + it 'retrieves build for the deployment' do + is_expected.to eq(deployment.deployable) + end + + it 'returns nil when the associated build is not found' do + deployment.update!(deployable_id: nil, deployable_type: nil) + + is_expected.to be_nil + end + end + describe '#previous_deployment' do using RSpec::Parameterized::TableSyntax @@ -1233,6 +1305,19 @@ RSpec.describe Deployment do end end + describe '#tags' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:deployment) { create(:deployment, project: project) } + + subject { deployment.tags } + + it 'will return tags related to this deployment' do + expect(project.repository).to receive(:tag_names_contains).with(deployment.sha, limit: 100).and_return(['test']) + + is_expected.to match_array(['test']) + end + end + describe '#valid_sha' do it 'does not add errors for a valid SHA' do project = create(:project, :repository) diff --git a/spec/models/design_management/version_spec.rb b/spec/models/design_management/version_spec.rb index 519ba3c67b4..44ecae82174 100644 --- a/spec/models/design_management/version_spec.rb +++ b/spec/models/design_management/version_spec.rb @@ -256,7 +256,7 @@ RSpec.describe DesignManagement::Version do it 'puts them in the right buckets' do expect(version.designs_by_event).to match( a_hash_including( - 'creation' => have_attributes(size: 3), + 'creation' => have_attributes(size: 3), 'modification' => have_attributes(size: 4), 'deletion' => have_attributes(size: 5) ) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 3f4372dafd0..1e15b09a069 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -17,6 +17,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do it { is_expected.to nullify_if_blank(:external_url) } it { is_expected.to belong_to(:project).required } + it { is_expected.to belong_to(:merge_request).optional } + it { is_expected.to have_many(:deployments) } it { is_expected.to have_many(:metrics_dashboard_annotations) } it { is_expected.to have_many(:alert_management_alerts) } @@ -40,6 +42,26 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do expect(environment).to be_valid end + + context 'does not allow changes to merge_request' do + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'for an environment that has no merge request associated' do + environment = create(:environment) + + environment.merge_request = merge_request + + expect(environment).not_to be_valid + end + + it 'for an environment that has a merge request associated' do + environment = create(:environment, merge_request: merge_request) + + environment.merge_request = nil + + expect(environment).not_to be_valid + end + end end describe 'validate and sanitize external url' do @@ -318,6 +340,16 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '.for_type' do + it 'filters by type' do + create(:environment) + create(:environment, name: 'type1/prod') + env = create(:environment, name: 'type2/prod') + + expect(described_class.for_type('type2')).to contain_exactly(env) + end + end + describe '#guess_tier' do using RSpec::Parameterized::TableSyntax @@ -938,6 +970,26 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe 'Last deployment relations' do + Deployment::FINISHED_STATUSES.each do |status| + it "returns the last #{status} deployment" do + create(:deployment, status.to_sym, environment: environment, finished_at: 1.day.ago) + expected = create(:deployment, status.to_sym, environment: environment, finished_at: Time.current) + + expect(environment.public_send(:"last_#{status}_deployment")).to eq(expected) + end + end + + Deployment::UPCOMING_STATUSES.each do |status| + it "returns the last #{status} deployment" do + create(:deployment, status.to_sym, environment: environment) + expected = create(:deployment, status.to_sym, environment: environment) + + expect(environment.public_send(:"last_#{status}_deployment")).to eq(expected) + end + end + end + describe '#last_deployable' do subject { environment.last_deployable } @@ -1573,11 +1625,38 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do expect(environment.auto_stop_in).to eq(expected_result) else + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + an_instance_of(expected_result), + project_id: environment.project_id, + environment_id: environment.id + ) + expect { subject }.to raise_error(expected_result) end end end end + + context 'resets earlier value' do + let(:environment) { create(:environment, auto_stop_at: 1.day.since.round) } + + where(:value, :expected_result) do + '2 days' | 2.days.to_i + '1 week' | 1.week.to_i + '2h20min' | 2.hours.to_i + 20.minutes.to_i + '' | nil + 'never' | nil + end + with_them do + it 'assigns new value' do + freeze_time do + subject + + expect(environment.auto_stop_in).to eq(expected_result) + end + end + end + end end describe '.for_id_and_slug' do diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 0685144dea6..30e73d84cfb 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -187,9 +187,38 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end + describe '#reactive_cache_limit_enabled?' do + subject { setting.reactive_cache_limit_enabled? } + + it { is_expected.to eq(true) } + + context 'when feature flag disabled' do + before do + stub_feature_flags(error_tracking_sentry_limit: false) + end + + it { is_expected.to eq(false) } + end + end + describe '#sentry_client' do - it 'returns sentry client' do - expect(subject.sentry_client).to be_a(ErrorTracking::SentryClient) + subject { setting.sentry_client } + + it { is_expected.to be_a(ErrorTracking::SentryClient) } + it { is_expected.to have_attributes(url: setting.api_url, token: setting.token) } + + describe '#validate_size_guarded_by_feature_flag?' do + subject { setting.sentry_client.validate_size_guarded_by_feature_flag? } + + it { is_expected.to eq(true) } + + context 'when feature flag disabled' do + before do + stub_feature_flags(error_tracking_sentry_limit: false) + end + + it { is_expected.to eq(false) } + end end end @@ -222,70 +251,39 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'when sentry client raises ErrorTracking::SentryClient::Error' do - before do - synchronous_reactive_cache(subject) - - allow(subject).to receive(:sentry_client).and_return(sentry_client) - allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(ErrorTracking::SentryClient::Error, 'error message') - end - - it 'returns error' do - expect(result).to eq( - error: 'error message', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE - ) - end - end - - context 'when sentry client raises ErrorTracking::SentryClient::MissingKeysError' do - before do - synchronous_reactive_cache(subject) - - allow(subject).to receive(:sentry_client).and_return(sentry_client) - allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(ErrorTracking::SentryClient::MissingKeysError, - 'Sentry API response is missing keys. key not found: "id"') - end - - it 'returns error' do - expect(result).to eq( - error: 'Sentry API response is missing keys. key not found: "id"', - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS - ) - end - end + describe 'client errors' do + using RSpec::Parameterized::TableSyntax - context 'when sentry client raises ErrorTracking::SentryClient::ResponseInvalidSizeError' do - let(:error_msg) { "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}." } + sc = ErrorTracking::SentryClient + pets = described_class + msg = 'something' before do synchronous_reactive_cache(subject) allow(subject).to receive(:sentry_client).and_return(sentry_client) - allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(ErrorTracking::SentryClient::ResponseInvalidSizeError, error_msg) end - it 'returns error' do - expect(result).to eq( - error: error_msg, - error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_INVALID_SIZE - ) + where(:exception, :error_type, :error_message) do + sc::Error | pets::SENTRY_API_ERROR_TYPE_NON_20X_RESPONSE | msg + sc::MissingKeysError | pets::SENTRY_API_ERROR_TYPE_MISSING_KEYS | msg + sc::ResponseInvalidSizeError | pets::SENTRY_API_ERROR_INVALID_SIZE | msg + sc::BadRequestError | pets::SENTRY_API_ERROR_TYPE_BAD_REQUEST | msg + StandardError | nil | 'Unexpected Error' end - end - context 'when sentry client raises StandardError' do - before do - synchronous_reactive_cache(subject) + with_them do + it 'returns an error' do + allow(sentry_client).to receive(:list_issues).with(opts) + .and_raise(exception, msg) - allow(subject).to receive(:sentry_client).and_return(sentry_client) - allow(sentry_client).to receive(:list_issues).with(opts).and_raise(StandardError) - end + expected_result = { + error: error_message, + error_type: error_type + }.compact - it 'returns error' do - expect(result).to eq(error: 'Unexpected Error') + expect(result).to eq(expected_result) + end end end end diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb index 969987c7e64..eec8fe0ef71 100644 --- a/spec/models/group_group_link_spec.rb +++ b/spec/models/group_group_link_spec.rb @@ -44,6 +44,17 @@ RSpec.describe GroupGroupLink do end end + describe '.with_owner_access' do + let_it_be(:group_group_link_maintainer) { create :group_group_link, :maintainer } + let_it_be(:group_group_link_owner) { create :group_group_link, :owner } + let_it_be(:group_group_link_reporter) { create :group_group_link, :reporter } + let_it_be(:group_group_link_guest) { create :group_group_link, :guest } + + it 'returns all records which have OWNER access' do + expect(described_class.with_owner_access).to match_array([group_group_link_owner]) + end + end + context 'for access via group shares' do let_it_be(:shared_with_group_1) { create(:group) } let_it_be(:shared_with_group_2) { create(:group) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 61662411ac8..2ce75fb1290 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -707,7 +707,8 @@ RSpec.describe Group do end describe '.public_or_visible_to_user' do - let!(:private_group) { create(:group, :private) } + let!(:private_group) { create(:group, :private) } + let!(:private_subgroup) { create(:group, :private, parent: private_group) } let!(:internal_group) { create(:group, :internal) } subject { described_class.public_or_visible_to_user(user) } @@ -731,6 +732,10 @@ RSpec.describe Group do end it { is_expected.to match_array([private_group, internal_group, group]) } + + it 'does not have access to subgroups (see accessible_to_user scope)' do + is_expected.not_to include(private_subgroup) + end end context 'when user is a member of private subgroup' do @@ -839,6 +844,36 @@ RSpec.describe Group do expect(described_class.by_ids_or_paths([new_group.id], [group_path])).to match_array([group, new_group]) end end + + describe 'accessible_to_user' do + subject { described_class.accessible_to_user(user) } + + let_it_be(:public_group) { create(:group, :public) } + let_it_be(:unaccessible_group) { create(:group, :private) } + let_it_be(:unaccessible_subgroup) { create(:group, :private, parent: unaccessible_group) } + let_it_be(:accessible_group) { create(:group, :private) } + let_it_be(:accessible_subgroup) { create(:group, :private, parent: accessible_group) } + + context 'when user is nil' do + let(:user) { nil } + + it { is_expected.to match_array([group, public_group]) } + end + + context 'when user is present' do + let(:user) { create(:user) } + + it { is_expected.to match_array([group, internal_group, public_group]) } + + context 'when user has access to accessible group' do + before do + accessible_group.add_developer(user) + end + + it { is_expected.to match_array([group, internal_group, public_group, accessible_group, accessible_subgroup]) } + end + end + end end describe '#to_reference' do @@ -1857,56 +1892,31 @@ RSpec.describe Group do end end - describe '#update_two_factor_requirement' do - let(:user) { create(:user) } + describe '#update_two_factor_requirement_for_members' do + let_it_be_with_reload(:user) { create(:user) } context 'group membership' do - before do + it 'enables two_factor_requirement for group members' do group.add_member(user, GroupMember::OWNER) - end - - it 'is called when require_two_factor_authentication is changed' do - expect_any_instance_of(User).to receive(:update_two_factor_requirement) - group.update!(require_two_factor_authentication: true) - end - - it 'is called when two_factor_grace_period is changed' do - expect_any_instance_of(User).to receive(:update_two_factor_requirement) - - group.update!(two_factor_grace_period: 23) - end - it 'is not called when other attributes are changed' do - expect_any_instance_of(User).not_to receive(:update_two_factor_requirement) + group.update_two_factor_requirement_for_members - group.update!(description: 'foobar') + expect(user.reload.require_two_factor_authentication_from_group).to be_truthy end - it 'calls #update_two_factor_requirement on each group member' do - other_user = create(:user) - group.add_member(other_user, GroupMember::OWNER) - - calls = 0 - allow_any_instance_of(User).to receive(:update_two_factor_requirement) do - calls += 1 - end + it 'disables two_factor_requirement for group members' do + user.update!(require_two_factor_authentication_from_group: true) + group.add_member(user, GroupMember::OWNER) + group.update!(require_two_factor_authentication: false) - group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23) + group.update_two_factor_requirement_for_members - expect(calls).to eq 2 + expect(user.reload.require_two_factor_authentication_from_group).to be_falsey end end context 'sub groups and projects' do - it 'enables two_factor_requirement for group member' do - group.add_member(user, GroupMember::OWNER) - - group.update!(require_two_factor_authentication: true) - - expect(user.reload.require_two_factor_authentication_from_group).to be_truthy - end - context 'expanded group members' do let(:indirect_user) { create(:user) } @@ -1915,9 +1925,10 @@ RSpec.describe Group do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) subgroup.add_member(indirect_user, GroupMember::OWNER) - group.update!(require_two_factor_authentication: true) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy end end @@ -1926,9 +1937,10 @@ RSpec.describe Group do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group, require_two_factor_authentication: true) subgroup.add_member(indirect_user, GroupMember::OWNER) - group.update!(require_two_factor_authentication: false) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy end @@ -1936,9 +1948,10 @@ RSpec.describe Group do ancestor_group = create(:group) ancestor_group.add_member(indirect_user, GroupMember::OWNER) group.update!(parent: ancestor_group) - group.update!(require_two_factor_authentication: true) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy end end @@ -1949,9 +1962,10 @@ RSpec.describe Group do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) subgroup.add_member(indirect_user, GroupMember::OWNER) - group.update!(require_two_factor_authentication: true) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy end end @@ -1960,9 +1974,10 @@ RSpec.describe Group do it 'disables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) subgroup.add_member(indirect_user, GroupMember::OWNER) - group.update!(require_two_factor_authentication: false) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_falsey end @@ -1970,9 +1985,10 @@ RSpec.describe Group do ancestor_group = create(:group, require_two_factor_authentication: false) indirect_user.update!(require_two_factor_authentication_from_group: true) ancestor_group.add_member(indirect_user, GroupMember::OWNER) - group.update!(require_two_factor_authentication: false) + group.update_two_factor_requirement_for_members + expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_falsey end end @@ -1983,9 +1999,10 @@ RSpec.describe Group do it 'does not enable two_factor_requirement for child project member' do project = create(:project, group: group) project.add_maintainer(user) - group.update!(require_two_factor_authentication: true) + group.update_two_factor_requirement_for_members + expect(user.reload.require_two_factor_authentication_from_group).to be_falsey end @@ -1993,15 +2010,36 @@ RSpec.describe Group do subgroup = create(:group, :nested, parent: group) project = create(:project, group: subgroup) project.add_maintainer(user) - group.update!(require_two_factor_authentication: true) + group.update_two_factor_requirement_for_members + expect(user.reload.require_two_factor_authentication_from_group).to be_falsey end end end end + describe '#update_two_factor_requirement' do + it 'enqueues a job when require_two_factor_authentication is changed' do + expect(Groups::UpdateTwoFactorRequirementForMembersWorker).to receive(:perform_async).with(group.id) + + group.update!(require_two_factor_authentication: true) + end + + it 'enqueues a job when two_factor_grace_period is changed' do + expect(Groups::UpdateTwoFactorRequirementForMembersWorker).to receive(:perform_async).with(group.id) + + group.update!(two_factor_grace_period: 23) + end + + it 'does not enqueue a job when other attributes are changed' do + expect(Groups::UpdateTwoFactorRequirementForMembersWorker).not_to receive(:perform_async).with(group.id) + + group.update!(description: 'foobar') + end + end + describe '#path_changed_hook' do let(:system_hook_service) { SystemHooksService.new } @@ -2043,201 +2081,6 @@ RSpec.describe Group do end end - describe '#ci_variables_for' do - let(:project) { create(:project, group: group) } - let(:environment_scope) { '*' } - - let!(:ci_variable) do - create(:ci_group_variable, value: 'secret', group: group, environment_scope: environment_scope) - end - - let!(:protected_variable) do - create(:ci_group_variable, :protected, value: 'protected', group: group) - end - - subject { group.ci_variables_for('ref', project) } - - it 'memoizes the result by ref and environment', :request_store do - scoped_variable = create(:ci_group_variable, value: 'secret', group: group, environment_scope: 'scoped') - - expect(project).to receive(:protected_for?).with('ref').once.and_return(true) - expect(project).to receive(:protected_for?).with('other').twice.and_return(false) - - 2.times do - expect(group.ci_variables_for('ref', project, environment: 'production')).to contain_exactly(ci_variable, protected_variable) - expect(group.ci_variables_for('other', project)).to contain_exactly(ci_variable) - expect(group.ci_variables_for('other', project, environment: 'scoped')).to contain_exactly(ci_variable, scoped_variable) - end - end - - shared_examples 'ref is protected' do - it 'contains all the variables' do - is_expected.to contain_exactly(ci_variable, protected_variable) - end - end - - context 'when the ref is not protected' do - before do - stub_application_setting( - default_branch_protection: Gitlab::Access::PROTECTION_NONE) - end - - it 'contains only the CI variables' do - is_expected.to contain_exactly(ci_variable) - end - end - - context 'when the ref is a protected branch' do - before do - allow(project).to receive(:protected_for?).with('ref').and_return(true) - end - - it_behaves_like 'ref is protected' - end - - context 'when the ref is a protected tag' do - before do - allow(project).to receive(:protected_for?).with('ref').and_return(true) - end - - it_behaves_like 'ref is protected' - end - - context 'when environment name is specified' do - let(:environment) { 'review/name' } - - subject do - group.ci_variables_for('ref', project, environment: environment) - end - - context 'when environment scope is exactly matched' do - let(:environment_scope) { 'review/name' } - - it { is_expected.to contain_exactly(ci_variable) } - end - - context 'when environment scope is matched by wildcard' do - let(:environment_scope) { 'review/*' } - - it { is_expected.to contain_exactly(ci_variable) } - end - - context 'when environment scope does not match' do - let(:environment_scope) { 'review/*/special' } - - it { is_expected.not_to contain_exactly(ci_variable) } - end - - context 'when environment scope has _' do - let(:environment_scope) { '*_*' } - - it 'does not treat it as wildcard' do - is_expected.not_to contain_exactly(ci_variable) - end - - context 'when environment name contains underscore' do - let(:environment) { 'foo_bar/test' } - let(:environment_scope) { 'foo_bar/*' } - - it 'matches literally for _' do - is_expected.to contain_exactly(ci_variable) - end - end - end - - # The environment name and scope cannot have % at the moment, - # but we're considering relaxing it and we should also make sure - # it doesn't break in case some data sneaked in somehow as we're - # not checking this integrity in database level. - context 'when environment scope has %' do - it 'does not treat it as wildcard' do - ci_variable.update_attribute(:environment_scope, '*%*') - - is_expected.not_to contain_exactly(ci_variable) - end - - context 'when environment name contains a percent' do - let(:environment) { 'foo%bar/test' } - - it 'matches literally for %' do - ci_variable.update_attribute(:environment_scope, 'foo%bar/*') - - is_expected.to contain_exactly(ci_variable) - end - end - end - - context 'when variables with the same name have different environment scopes' do - let!(:partially_matched_variable) do - create(:ci_group_variable, - key: ci_variable.key, - value: 'partial', - environment_scope: 'review/*', - group: group) - end - - let!(:perfectly_matched_variable) do - create(:ci_group_variable, - key: ci_variable.key, - value: 'prefect', - environment_scope: 'review/name', - group: group) - end - - it 'puts variables matching environment scope more in the end' do - is_expected.to eq( - [ci_variable, - partially_matched_variable, - perfectly_matched_variable]) - end - end - end - - context 'when group has children' do - let(:group_child) { create(:group, parent: group) } - let(:group_child_2) { create(:group, parent: group_child) } - let(:group_child_3) { create(:group, parent: group_child_2) } - let(:variable_child) { create(:ci_group_variable, group: group_child) } - let(:variable_child_2) { create(:ci_group_variable, group: group_child_2) } - let(:variable_child_3) { create(:ci_group_variable, group: group_child_3) } - - before do - allow(project).to receive(:protected_for?).with('ref').and_return(true) - end - - context 'traversal queries' do - shared_examples 'correct ancestor order' do - it 'returns all variables belong to the group and parent groups' do - expected_array1 = [protected_variable, ci_variable] - expected_array2 = [variable_child, variable_child_2, variable_child_3] - got_array = group_child_3.ci_variables_for('ref', project).to_a - - expect(got_array.shift(2)).to contain_exactly(*expected_array1) - expect(got_array).to eq(expected_array2) - end - end - - context 'recursive' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - include_examples 'correct ancestor order' - end - - context 'linear' do - before do - stub_feature_flags(use_traversal_ids: true) - - group_child_3.reload # make sure traversal_ids are reloaded - end - - include_examples 'correct ancestor order' - end - end - end - end - describe '#highest_group_member' do let(:nested_group) { create(:group, parent: group) } let(:nested_group_2) { create(:group, parent: nested_group) } @@ -2331,8 +2174,7 @@ RSpec.describe Group do let(:another_shared_with_group) { create(:group, parent: group) } before do - create(:group_group_link, shared_group: nested_group, - shared_with_group: another_shared_with_group) + create(:group_group_link, shared_group: nested_group, shared_with_group: another_shared_with_group) end it 'returns all shared with group ids' do @@ -3465,6 +3307,23 @@ RSpec.describe Group do end end + describe '#packages_policy_subject' do + it 'returns wrapper' do + expect(group.packages_policy_subject).to be_a(Packages::Policies::Group) + expect(group.packages_policy_subject.group).to eq(group) + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(read_package_policy_rule: false) + end + + it 'returns group' do + expect(group.packages_policy_subject).to eq(group) + end + end + end + describe '#gitlab_deploy_token' do subject(:gitlab_deploy_token) { group.gitlab_deploy_token } diff --git a/spec/models/incident_management/timeline_event_spec.rb b/spec/models/incident_management/timeline_event_spec.rb index 9f4011fe6a7..fea391acda3 100644 --- a/spec/models/incident_management/timeline_event_spec.rb +++ b/spec/models/incident_management/timeline_event_spec.rb @@ -29,7 +29,7 @@ RSpec.describe IncidentManagement::TimelineEvent do it { is_expected.to validate_length_of(:action).is_at_most(128) } end - describe '.order_occurred_at_asc' do + describe '.order_occurred_at_asc_id_asc' do let_it_be(:occurred_3mins_ago) do create(:incident_management_timeline_event, project: project, occurred_at: 3.minutes.ago) end @@ -38,11 +38,23 @@ RSpec.describe IncidentManagement::TimelineEvent do create(:incident_management_timeline_event, project: project, occurred_at: 2.minutes.ago) end - subject(:order) { described_class.order_occurred_at_asc } + subject(:order) { described_class.order_occurred_at_asc_id_asc } it 'sorts timeline events by occurred_at' do is_expected.to eq([occurred_3mins_ago, occurred_2mins_ago, timeline_event]) end + + context 'when two events occured at the same time' do + let_it_be(:also_occurred_2mins_ago) do + create(:incident_management_timeline_event, project: project, occurred_at: occurred_2mins_ago.occurred_at) + end + + it 'sorts timeline events by occurred_at then sorts by id' do + occurred_2mins_ago.touch # Interact with record of earlier id to switch default DB ordering + + is_expected.to eq([occurred_3mins_ago, occurred_2mins_ago, also_occurred_2mins_ago, timeline_event]) + end + end end describe '#cache_markdown_field' do diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index 68ef0ccb2e4..a63cc0b6d83 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -49,8 +49,8 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do allow(test_project).to receive(:avatar_url).with(only_path: false).and_return(args[:project][:avatar_url]) allow(Project).to receive(:find) { test_project } - test_pipeline = double("A test pipeline", has_yaml_errors?: has_yaml_errors, - yaml_errors: "yaml error description here") + test_pipeline = double("A test pipeline", + has_yaml_errors?: has_yaml_errors, yaml_errors: "yaml error description here") allow(Ci::Pipeline).to receive(:find) { test_pipeline } allow(Gitlab::UrlBuilder).to receive(:build).with(test_commit).and_return("http://example.com/commit") diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index cfc44b22a84..4ac684e8ff0 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -203,13 +203,10 @@ RSpec.describe Integrations::Datadog do end before do - stub_feature_flags(datadog_integration_logs_collection: enable_logs_collection) stub_request(:post, expected_hook_url) saved_instance.execute(data) end - let(:enable_logs_collection) { true } - context 'with pipeline data' do let(:data) { pipeline_data } let(:expected_headers) { { ::Gitlab::WebHooks::GITLAB_EVENT_HEADER => 'Pipeline Hook' } } @@ -232,12 +229,6 @@ RSpec.describe Integrations::Datadog do let(:expected_body) { data.to_json } it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers, body: expected_body)).to have_been_made } - - context 'but feature flag disabled' do - let(:enable_logs_collection) { false } - - it { expect(a_request(:post, expected_hook_url)).not_to have_been_made } - end end end end diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index b85620782c1..eb90acc73be 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -23,10 +23,10 @@ RSpec.describe Integrations::Discord do describe '#execute' do include StubRequests + let_it_be(:project) { create(:project, :repository) } + let(:user) { create(:user) } - let(:project) { create(:project, :repository) } let(:webhook_url) { "https://example.gitlab.com/" } - let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 8a51f8a0705..905fee075ad 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -175,9 +175,9 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end { - "killed" => :canceled, + "killed" => :canceled, "failure" => :failed, - "error" => :failed, + "error" => :failed, "success" => "success" }.each do |drone_status, our_status| it "sets commit status to #{our_status.inspect} when returned status is #{drone_status.inspect}" do diff --git a/spec/models/integrations/hangouts_chat_spec.rb b/spec/models/integrations/hangouts_chat_spec.rb index 17b40c484f5..828bcdf5d8f 100644 --- a/spec/models/integrations/hangouts_chat_spec.rb +++ b/spec/models/integrations/hangouts_chat_spec.rb @@ -12,4 +12,175 @@ RSpec.describe Integrations::HangoutsChat do } end end + + let(:chat_integration) { described_class.new } + let(:webhook_url) { 'https://example.gitlab.com/' } + let(:webhook_url_regex) { /\A#{webhook_url}.*/ } + + describe "#execute" do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, :wiki_repo) } + + before do + allow(chat_integration).to receive_messages( + project: project, + project_id: project.id, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url_regex) + end + + context 'with push events' do + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + it "adds thread key for push events" do + expect(chat_integration.execute(push_sample_data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /push .*?/ })) + .once + end + end + + context 'with issue events' do + let(:issues_sample_data) { create(:issue).to_hook_data(user) } + + it "adds thread key for issue events" do + expect(chat_integration.execute(issues_sample_data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /issue .*?/ })) + .once + end + end + + context 'with merge events' do + let(:merge_sample_data) { create(:merge_request).to_hook_data(user) } + + it "adds thread key for merge events" do + expect(chat_integration.execute(merge_sample_data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /merge request .*?/ })) + .once + end + end + + context 'with wiki page events' do + let(:wiki_page_sample_data) do + Gitlab::DataBuilder::WikiPage.build(create(:wiki_page, message: 'foo'), user, 'create') + end + + it "adds thread key for wiki page events" do + expect(chat_integration.execute(wiki_page_sample_data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /wiki_page .*?/ })) + .once + end + end + + context 'with pipeline events' do + let(:pipeline) do + create(:ci_pipeline, :failed, project: project, sha: project.commit.sha, ref: project.default_branch) + end + + let(:pipeline_sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + it "adds thread key for pipeline events" do + expect(chat_integration.execute(pipeline_sample_data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /pipeline .*?/ })) + .once + end + end + end + + describe "Note events" do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, creator: user) } + + before do + allow(chat_integration).to receive_messages( + project: project, + project_id: project.id, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url_regex) + end + + context 'when commit comment event executed' do + let(:commit_note) do + create(:note_on_commit, author: user, + project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "adds thread key" do + data = Gitlab::DataBuilder::Note.build(commit_note, user) + + expect(chat_integration.execute(data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /commit .*?/ })) + .once + end + end + + context 'when merge request comment event executed' do + let(:merge_request_note) do + create(:note_on_merge_request, project: project, + note: "merge request note") + end + + it "adds thread key" do + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) + + expect(chat_integration.execute(data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /merge request .*?/ })) + .once + end + end + + context 'when issue comment event executed' do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it "adds thread key" do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + + expect(chat_integration.execute(data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /issue .*?/ })) + .once + end + end + + context 'when snippet comment event executed' do + let(:snippet_note) do + create(:note_on_project_snippet, project: project, + note: "snippet note") + end + + it "adds thread key" do + data = Gitlab::DataBuilder::Note.build(snippet_note, user) + + expect(chat_integration.execute(data)).to be(true) + + expect(WebMock).to have_requested(:post, webhook_url) + .with(query: hash_including({ "threadKey" => /snippet .*?/ })) + .once + end + end + end end diff --git a/spec/models/integrations/harbor_spec.rb b/spec/models/integrations/harbor_spec.rb index 3952495119a..26b43fa3313 100644 --- a/spec/models/integrations/harbor_spec.rb +++ b/spec/models/integrations/harbor_spec.rb @@ -27,6 +27,12 @@ RSpec.describe Integrations::Harbor do it { is_expected.to allow_value('https://demo.goharbor.io').for(:url) } end + describe 'hostname' do + it 'returns the host of the integration url' do + expect(harbor_integration.hostname).to eq('demo.goharbor.io') + end + end + describe '#fields' do it 'returns custom fields' do expect(harbor_integration.fields.pluck(:name)).to eq(%w[url project_name username password]) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index af4c48775ec..17c3cd17364 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -150,8 +150,8 @@ RSpec.describe Issue do issue.confidential = false expect(issue).not_to be_valid - expect(issue.errors[:confidential]) - .to include('associated parent is confidential and can not have non-confidential children.') + expect(issue.errors[:base]) + .to include(_('A non-confidential issue cannot have a confidential parent.')) end it 'allows to make parent not-confidential' do @@ -172,8 +172,8 @@ RSpec.describe Issue do issue.confidential = true expect(issue).not_to be_valid - expect(issue.errors[:confidential]) - .to include('confidential parent can not be used if there are non-confidential children.') + expect(issue.errors[:base]) + .to include(_('A confidential issue cannot have a parent that already has non-confidential children.')) end it 'allows to make child confidential' do @@ -283,6 +283,14 @@ RSpec.describe Issue do create(:issue) end + + it_behaves_like 'issue_edit snowplow tracking' do + let(:issue) { create(:issue) } + let(:project) { issue.project } + let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:user) { issue.author } + subject(:service_action) { issue } + end end context 'issue namespace' do @@ -1782,20 +1790,4 @@ RSpec.describe Issue do end end end - - describe '#full_search' do - context 'when searching non-english terms' do - [ - 'abc 中文語', - '中文語cn', - '中文語' - ].each do |term| - it 'adds extra where clause to match partial index' do - expect(described_class.full_search(term).to_sql).to include( - "AND (issues.title NOT SIMILAR TO '[\\u0000-\\u218F]*' OR issues.description NOT SIMILAR TO '[\\u0000-\\u218F]*')" - ) - end - end - end - end end diff --git a/spec/models/jira_connect_installation_spec.rb b/spec/models/jira_connect_installation_spec.rb index 3d1095845aa..9c1f7c678a9 100644 --- a/spec/models/jira_connect_installation_spec.rb +++ b/spec/models/jira_connect_installation_spec.rb @@ -45,4 +45,30 @@ RSpec.describe JiraConnectInstallation do expect(subject).to contain_exactly(subscription.installation) end end + + describe '#oauth_authorization_url' do + let_it_be(:installation) { create(:jira_connect_installation) } + + subject { installation.oauth_authorization_url } + + before do + allow(Gitlab).to receive_message_chain('config.gitlab.url') { 'http://test.host' } + end + + it { is_expected.to eq('http://test.host') } + + context 'with instance_url' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://gitlab.example.com') } + + it { is_expected.to eq('https://gitlab.example.com') } + + context 'and jira_connect_oauth_self_managed feature is disabled' do + before do + stub_feature_flags(jira_connect_oauth_self_managed: false) + end + + it { is_expected.to eq('http://test.host') } + end + end + end end diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb index 9ee5b7340f3..a909252a78c 100644 --- a/spec/models/loose_foreign_keys/deleted_record_spec.rb +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -66,7 +66,7 @@ RSpec.describe LooseForeignKeys::DeletedRecord, type: :model do let(:partition_manager) { Gitlab::Database::Partitioning::PartitionManager.new(described_class) } describe 'next_partition_if callback' do - let(:active_partition) { described_class.partitioning_strategy.active_partition.value } + let(:active_partition) { described_class.partitioning_strategy.active_partition } subject(:value) { described_class.partitioning_strategy.next_partition_if.call(active_partition) } @@ -98,7 +98,7 @@ RSpec.describe LooseForeignKeys::DeletedRecord, type: :model do end describe 'detach_partition_if callback' do - let(:active_partition) { described_class.partitioning_strategy.active_partition.value } + let(:active_partition) { described_class.partitioning_strategy.active_partition } subject(:value) { described_class.partitioning_strategy.detach_partition_if.call(active_partition) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 2716244b7f3..7b75a6ee1c2 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -195,6 +195,15 @@ RSpec.describe Member do expect(member).not_to be_valid end end + + context 'access_level cannot be changed' do + it 'is invalid' do + member.access_level = Gitlab::Access::MAINTAINER + + expect(member).not_to be_valid + expect(member.errors.full_messages).to include( "Access level cannot be changed since member is associated with a custom role") + end + end end end end @@ -880,7 +889,8 @@ RSpec.describe Member do end describe 'generate invite token on create' do - let!(:member) { build(:project_member, invite_email: "user@example.com") } + let(:project) { create(:project) } + let!(:member) { build(:project_member, invite_email: "user@example.com", project: project) } it 'sets the invite token' do expect { member.save! }.to change { member.invite_token }.to(kind_of(String)) diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index c6266f15340..363830d21dd 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -74,7 +74,8 @@ RSpec.describe GroupMember do describe '#update_two_factor_requirement' do it 'is called after creation and deletion' do user = build :user - group_member = build :group_member, user: user + group = create :group + group_member = build :group_member, user: user, group: group expect(user).to receive(:update_two_factor_requirement) @@ -151,8 +152,8 @@ RSpec.describe GroupMember do context 'when importing' do it 'does not refresh' do expect(UserProjectAccessChangedService).not_to receive(:new) - - member = build(:group_member) + group = create(:group) + member = build(:group_member, group: group) member.importing = true member.save! end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 99fc5dc14df..ad6f3ca5428 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -201,7 +201,7 @@ RSpec.describe ProjectMember do it 'does not refresh' do expect(AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker).not_to receive(:bulk_perform_and_wait) - member = build(:project_member) + member = build(:project_member, project: project) member.importing = true member.save! end diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 387d17d7823..73bf7d02468 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -38,26 +38,4 @@ RSpec.describe MergeRequestAssignee do end end end - - it_behaves_like 'having unique enum values' - - describe '#attention_requested_by' do - let(:current_user) { create(:user) } - - before do - subject.update!(updated_state_by: current_user, state: :attention_requested) - end - - context 'attention requested' do - it { expect(subject.attention_requested_by).to eq(current_user) } - end - - context 'attention requested' do - before do - subject.update!(state: :reviewed) - end - - it { expect(subject.attention_requested_by).to eq(nil) } - end - end end diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index 4df2dba3a7d..5a29966e4b9 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -14,24 +14,4 @@ RSpec.describe MergeRequestReviewer do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } end - - describe '#attention_requested_by' do - let(:current_user) { create(:user) } - - before do - subject.update!(updated_state_by: current_user, state: :attention_requested) - end - - context 'attention requested' do - it { expect(subject.attention_requested_by).to eq(current_user) } - end - - context 'attention requested' do - before do - subject.update!(state: :reviewed) - end - - it { expect(subject.attention_requested_by).to eq(nil) } - end - end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 19026a4772d..f27f3b749b1 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -31,6 +31,7 @@ RSpec.describe MergeRequest, factory_default: :keep do it { is_expected.to have_many(:draft_notes) } it { is_expected.to have_many(:reviews).inverse_of(:merge_request) } it { is_expected.to have_one(:cleanup_schedule).inverse_of(:merge_request) } + it { is_expected.to have_many(:created_environments).class_name('Environment').inverse_of(:merge_request) } context 'for forks' do let!(:project) { create(:project) } @@ -144,22 +145,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '.attention' do - let_it_be(:merge_request5) { create(:merge_request, :unique_branches, assignees: [user2]) } - let_it_be(:merge_request6) { create(:merge_request, :unique_branches, assignees: [user2]) } - - before do - assignee = merge_request6.find_assignee(user2) - assignee.update!(state: :reviewed) - merge_request2.find_reviewer(user2).update!(state: :attention_requested) - merge_request5.find_assignee(user2).update!(state: :attention_requested) - end - - it 'returns MRs that have any attention requests' do - expect(described_class.attention(user2)).to eq([merge_request2, merge_request5]) - end - end - describe '.drafts' do it 'returns MRs where draft == true' do expect(described_class.drafts).to eq([merge_request4]) @@ -884,6 +869,16 @@ RSpec.describe MergeRequest, factory_default: :keep do expect { subject.cache_merge_request_closes_issues!(subject.author) } .not_to change(subject.merge_requests_closing_issues, :count) end + + it 'caches issues from another project with issues enabled' do + project = create(:project, :public, issues_enabled: true) + issue = create(:issue, project: project) + commit = double('commit1', safe_message: "Fixes #{issue.to_reference(full: true)}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues!(subject.author) } + .to change(subject.merge_requests_closing_issues, :count).by(1) + end end end @@ -3232,73 +3227,9 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '#detailed_merge_status' do - subject(:detailed_merge_status) { merge_request.detailed_merge_status } - - context 'when merge status is cannot_be_merged_rechecking' do - let(:merge_request) { create(:merge_request, merge_status: :cannot_be_merged_rechecking) } - - it 'returns :checking' do - expect(detailed_merge_status).to eq(:checking) - end - end - - context 'when merge status is preparing' do - let(:merge_request) { create(:merge_request, merge_status: :preparing) } - - it 'returns :checking' do - expect(detailed_merge_status).to eq(:checking) - end - end - - context 'when merge status is checking' do - let(:merge_request) { create(:merge_request, merge_status: :checking) } - - it 'returns :checking' do - expect(detailed_merge_status).to eq(:checking) - end - end - - context 'when merge status is unchecked' do - let(:merge_request) { create(:merge_request, merge_status: :unchecked) } - - it 'returns :unchecked' do - expect(detailed_merge_status).to eq(:unchecked) - end - end - - context 'when merge checks are a success' do - let(:merge_request) { create(:merge_request) } - - it 'returns :mergeable' do - expect(detailed_merge_status).to eq(:mergeable) - end - end - - context 'when merge status have a failure' do - let(:merge_request) { create(:merge_request) } - - before do - merge_request.close! - end - - it 'returns the failure reason' do - expect(detailed_merge_status).to eq(:not_open) - end - end - end - describe '#mergeable_state?' do it_behaves_like 'for mergeable_state' - context 'when improved_mergeability_checks is off' do - before do - stub_feature_flags(improved_mergeability_checks: false) - end - - it_behaves_like 'for mergeable_state' - end - context 'when merge state caching is off' do before do stub_feature_flags(mergeability_caching: false) @@ -3743,9 +3674,9 @@ RSpec.describe MergeRequest, factory_default: :keep do let(:expected_diff_refs) do Gitlab::Diff::DiffRefs.new( - base_sha: subject.merge_request_diff.base_commit_sha, + base_sha: subject.merge_request_diff.base_commit_sha, start_sha: subject.merge_request_diff.start_commit_sha, - head_sha: subject.merge_request_diff.head_commit_sha + head_sha: subject.merge_request_diff.head_commit_sha ) end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index a48e291fa55..f58d30f81a0 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -9,4 +9,35 @@ RSpec.describe Ml::Candidate do it { is_expected.to have_many(:params) } it { is_expected.to have_many(:metrics) } end + + describe '#new' do + it 'iid is not null' do + expect(create(:ml_candidates).iid).not_to be_nil + end + end + + describe 'by_project_id_and_iid' do + let_it_be(:candidate) { create(:ml_candidates) } + + let(:project_id) { candidate.experiment.project_id } + let(:iid) { candidate.iid } + + subject { described_class.with_project_id_and_iid(project_id, iid) } + + context 'when iid exists', 'and belongs to project' do + it { is_expected.to eq(candidate) } + end + + context 'when iid exists', 'and does not belong to project' do + let(:project_id) { non_existing_record_id } + + it { is_expected.to be_nil } + end + + context 'when iid does not exist' do + let(:iid) { 'a' } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index dca5280a8fe..e300f82d290 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -8,4 +8,55 @@ RSpec.describe Ml::Experiment do it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:candidates) } end + + describe '#by_project_id_and_iid?' do + let(:exp) { create(:ml_experiments) } + let(:iid) { exp.iid } + + subject { described_class.by_project_id_and_iid(exp.project_id, iid) } + + context 'if exists' do + it { is_expected.to eq(exp) } + end + + context 'if does not exist' do + let(:iid) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + end + + describe '#by_project_id_and_name?' do + let(:exp) { create(:ml_experiments) } + let(:exp_name) { exp.name } + + subject { described_class.by_project_id_and_name(exp.project_id, exp_name) } + + context 'if exists' do + it { is_expected.to eq(exp) } + end + + context 'if does not exist' do + let(:exp_name) { 'hello' } + + it { is_expected.to be_nil } + end + end + + describe '#has_record?' do + let(:exp) { create(:ml_experiments) } + let(:exp_name) { exp.name } + + subject { described_class.has_record?(exp.project_id, exp_name) } + + context 'if exists' do + it { is_expected.to be_truthy } + end + + context 'if does not exist' do + let(:exp_name) { 'hello' } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 25234db5734..4ac248802b8 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -127,4 +127,54 @@ RSpec.describe NamespaceSetting, type: :model do end end end + + describe '#show_diff_preview_in_email?' do + context 'when not a subgroup' do + context 'when :show_diff_preview_in_email is false' do + it 'returns false' do + settings = create(:namespace_settings, show_diff_preview_in_email: false) + group = create(:group, namespace_settings: settings ) + + expect(group.show_diff_preview_in_email?).to be_falsey + end + end + + context 'when :show_diff_preview_in_email is true' do + it 'returns true' do + settings = create(:namespace_settings, show_diff_preview_in_email: true) + group = create(:group, namespace_settings: settings ) + + expect(group.show_diff_preview_in_email?).to be_truthy + end + end + + it 'does not query the db when there is no parent group' do + group = create(:group) + + expect { group.show_diff_preview_in_email? }.not_to exceed_query_limit(0) + end + end + + context 'when a group has parent groups' do + let(:grandparent) { create(:group, namespace_settings: settings) } + let(:parent) { create(:group, parent: grandparent) } + let!(:group) { create(:group, parent: parent) } + + context "when a parent group has disabled diff previews" do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: false) } + + it 'returns false' do + expect(group.show_diff_preview_in_email?).to be_falsey + end + end + + context 'when all parent groups have enabled diff previews' do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + + it 'returns true' do + expect(group.show_diff_preview_in_email?).to be_truthy + end + end + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 71ce3afda44..2e8d22cb9db 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -336,6 +336,26 @@ RSpec.describe Namespace do expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent]) end end + + describe '.with_shared_runners_enabled' do + subject { described_class.with_shared_runners_enabled } + + context 'when shared runners are enabled for namespace' do + let!(:namespace_inheriting_shared_runners) { create(:namespace, shared_runners_enabled: true) } + + it "returns a namespace inheriting shared runners" do + is_expected.to include(namespace_inheriting_shared_runners) + end + end + + context 'when shared runners are disabled for namespace' do + let!(:namespace_not_inheriting_shared_runners) { create(:namespace, shared_runners_enabled: false) } + + it "does not return a namespace not inheriting shared runners" do + is_expected.not_to include(namespace_not_inheriting_shared_runners) + end + end + end end describe 'delegate' do @@ -439,19 +459,54 @@ RSpec.describe Namespace do context 'traversal_ids on create' do shared_examples 'default traversal_ids' do - let(:namespace) { build(:namespace) } - - before do - namespace.save! - namespace.reload - end + let!(:namespace) { create(:group) } + let!(:child_namespace) { create(:group, parent: namespace) } - it { expect(namespace.traversal_ids).to eq [namespace.id] } + it { expect(namespace.reload.traversal_ids).to eq [namespace.id] } + it { expect(child_namespace.reload.traversal_ids).to eq [namespace.id, child_namespace.id] } + it { expect(namespace.sync_events.count).to eq 1 } + it { expect(child_namespace.sync_events.count).to eq 1 } end it_behaves_like 'default traversal_ids' end + context 'traversal_ids on update' do + let!(:namespace1) { create(:group) } + let!(:namespace2) { create(:group) } + + it 'updates the traversal_ids when the parent_id is changed' do + expect do + namespace1.update!(parent: namespace2) + end.to change { namespace1.reload.traversal_ids }.from([namespace1.id]).to([namespace2.id, namespace1.id]) + end + + it 'creates a Namespaces::SyncEvent using triggers' do + Namespaces::SyncEvent.delete_all + namespace1.update!(parent: namespace2) + expect(namespace1.reload.sync_events.count).to eq(1) + end + + it 'creates sync_events using database trigger on the table' do + expect { Group.update_all(traversal_ids: [-1]) }.to change(Namespaces::SyncEvent, :count).by(2) + end + + it 'does not create sync_events using database trigger on the table when only the parent_id has changed' do + expect { Group.update_all(parent_id: -1) }.not_to change(Namespaces::SyncEvent, :count) + end + + it 'triggers the callback sync_traversal_ids on the namespace' do + allow(namespace1).to receive(:run_callbacks).and_call_original + expect(namespace1).to receive(:run_callbacks).with(:sync_traversal_ids) + namespace1.update!(parent: namespace2) + end + + it 'calls schedule_sync_event_worker on the updated namespace' do + expect(namespace1).to receive(:schedule_sync_event_worker) + namespace1.update!(parent: namespace2) + end + end + describe "after_commit :expire_child_caches" do let(:namespace) { create(:group) } @@ -675,6 +730,24 @@ RSpec.describe Namespace do end end + describe '#any_project_with_shared_runners_enabled?' do + subject { namespace.any_project_with_shared_runners_enabled? } + + let!(:project_not_inheriting_shared_runners) do + create(:project, namespace: namespace, shared_runners_enabled: false) + end + + context 'when a child project has shared runners enabled' do + let!(:project_inheriting_shared_runners) { create(:project, namespace: namespace, shared_runners_enabled: true) } + + it { is_expected.to eq true } + end + + context 'when all child projects have shared runners disabled' do + it { is_expected.to eq false } + end + end + describe '.search' do let_it_be(:first_group) { create(:group, name: 'my first namespace', path: 'old-path') } let_it_be(:parent_group) { create(:group, name: 'my parent namespace', path: 'parent-path') } @@ -744,30 +817,30 @@ RSpec.describe Namespace do create(:project, namespace: namespace, statistics: build(:project_statistics, - namespace: namespace, - repository_size: 101, - wiki_size: 505, - lfs_objects_size: 202, - build_artifacts_size: 303, + namespace: namespace, + repository_size: 101, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, pipeline_artifacts_size: 707, - packages_size: 404, - snippets_size: 605, - uploads_size: 808)) + packages_size: 404, + snippets_size: 605, + uploads_size: 808)) end let(:project2) do create(:project, namespace: namespace, statistics: build(:project_statistics, - namespace: namespace, - repository_size: 10, - wiki_size: 50, - lfs_objects_size: 20, - build_artifacts_size: 30, + namespace: namespace, + repository_size: 10, + wiki_size: 50, + lfs_objects_size: 20, + build_artifacts_size: 30, pipeline_artifacts_size: 70, - packages_size: 40, - snippets_size: 60, - uploads_size: 80)) + packages_size: 40, + snippets_size: 60, + uploads_size: 80)) end it "sums all project storage counters in the namespace" do @@ -2216,6 +2289,20 @@ RSpec.describe Namespace do expect(namespace.sync_events.count).to eq(2) end + it 'creates a namespaces_sync_event for the parent and all the descendent namespaces' do + children_namespaces = create_list(:group, 2, parent_id: namespace.id) + grand_children_namespaces = create_list(:group, 2, parent_id: children_namespaces.first.id) + expect(Namespaces::ProcessSyncEventsWorker).to receive(:perform_async).exactly(:once) + Namespaces::SyncEvent.delete_all + + expect do + namespace.update!(parent_id: new_namespace1.id) + end.to change(Namespaces::SyncEvent, :count).by(5) + + expected_ids = [namespace.id] + children_namespaces.map(&:id) + grand_children_namespaces.map(&:id) + expect(Namespaces::SyncEvent.pluck(:namespace_id)).to match_array(expected_ids) + end + it 'enqueues ProcessSyncEventsWorker' do expect(Namespaces::ProcessSyncEventsWorker).to receive(:perform_async) diff --git a/spec/models/namespaces/sync_event_spec.rb b/spec/models/namespaces/sync_event_spec.rb new file mode 100644 index 00000000000..a3a90ba9aff --- /dev/null +++ b/spec/models/namespaces/sync_event_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::SyncEvent, type: :model do + describe '.enqueue_worker' do + it 'schedules Namespaces::ProcessSyncEventsWorker job' do + expect(::Namespaces::ProcessSyncEventsWorker).to receive(:perform_async) + described_class.enqueue_worker + end + end + + describe '.upper_bound_count' do + it 'returns 0 when there are no records in the table' do + expect(described_class.upper_bound_count).to eq(0) + end + + it 'returns an estimated number of the records in the database' do + create_list(:namespace, 3) + expect(described_class.upper_bound_count).to eq(3) + end + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index ca558848cb0..1fce1f97dcb 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -280,6 +280,32 @@ RSpec.describe Note do expect { note.destroy! }.not_to raise_error end end + + describe 'sets internal flag' do + subject(:internal) { note.reload.internal } + + let(:note) { create(:note, confidential: confidential, project: issue.project, noteable: issue) } + + let_it_be(:issue) { create(:issue) } + + context 'when confidential is `true`' do + let(:confidential) { true } + + it { is_expected.to be true } + end + + context 'when confidential is `false`' do + let(:confidential) { false } + + it { is_expected.to be false } + end + + context 'when confidential is `nil`' do + let(:confidential) { nil } + + it { is_expected.to be false } + end + end end describe "Commit notes" do diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 4debda0621c..8105262aada 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -39,6 +39,56 @@ RSpec.describe NotificationRecipient do expect(recipient.notifiable?).to eq true end end + + context 'when recipient email is blocked', :clean_gitlab_redis_rate_limiting do + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) + .and_return( + temporary_email_failure: { threshold: 1, interval: 1.minute }, + permanent_email_failure: { threshold: 1, interval: 1.minute } + ) + end + + context 'with permanent failures' do + before do + 2.times { Gitlab::ApplicationRateLimiter.throttled?(:permanent_email_failure, scope: user.email) } + end + + it 'returns false' do + expect(recipient.notifiable?).to eq(false) + end + + context 'when block_emails_with_failures is disabled' do + before do + stub_feature_flags(block_emails_with_failures: false) + end + + it 'returns true' do + expect(recipient.notifiable?).to eq(true) + end + end + end + + context 'with temporary failures' do + before do + 2.times { Gitlab::ApplicationRateLimiter.throttled?(:temporary_email_failure, scope: user.email) } + end + + it 'returns false' do + expect(recipient.notifiable?).to eq(false) + end + + context 'when block_emails_with_failures is disabled' do + before do + stub_feature_flags(block_emails_with_failures: false) + end + + it 'returns true' do + expect(recipient.notifiable?).to eq(true) + end + end + end + end end describe '#has_access?' do diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index 544f6643712..a4540ac95bc 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' RSpec.describe OauthAccessToken do - let(:user) { create(:user) } let(:app_one) { create(:oauth_application) } let(:app_two) { create(:oauth_application) } let(:app_three) { create(:oauth_application) } @@ -69,4 +68,20 @@ RSpec.describe OauthAccessToken do end end end + + describe '.matching_token_for' do + it 'does not find existing tokens' do + expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_nil + end + + context 'when hash oauth tokens is disabled' do + before do + stub_feature_flags(hash_oauth_tokens: false) + end + + it 'finds an existing token' do + expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_present + end + end + end end diff --git a/spec/models/onboarding/completion_spec.rb b/spec/models/onboarding/completion_spec.rb new file mode 100644 index 00000000000..e1fad4255bc --- /dev/null +++ b/spec/models/onboarding/completion_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Onboarding::Completion do + describe '#percentage' do + let(:completed_actions) { {} } + let!(:onboarding_progress) { create(:onboarding_progress, namespace: namespace, **completed_actions) } + let(:tracked_action_columns) do + [ + *described_class::ACTION_ISSUE_IDS.keys, + *described_class::ACTION_PATHS, + :security_scan_enabled + ].map { |key| ::Onboarding::Progress.column_name(key) } + end + + let_it_be(:namespace) { create(:namespace) } + + subject { described_class.new(namespace).percentage } + + context 'when no onboarding_progress exists' do + subject { described_class.new(build(:namespace)).percentage } + + it { is_expected.to eq(0) } + end + + context 'when no action has been completed' do + it { is_expected.to eq(0) } + end + + context 'when all tracked actions have been completed' do + let(:completed_actions) do + tracked_action_columns.index_with { Time.current } + end + + it { is_expected.to eq(100) } + end + + context 'with security_actions_continuous_onboarding experiment' do + let(:completed_actions) { Hash[tracked_action_columns.first, Time.current] } + + context 'when control' do + before do + stub_experiments(security_actions_continuous_onboarding: :control) + end + + it { is_expected.to eq(11) } + end + + context 'when candidate' do + before do + stub_experiments(security_actions_continuous_onboarding: :candidate) + end + + it { is_expected.to eq(9) } + end + end + end +end diff --git a/spec/models/onboarding/learn_gitlab_spec.rb b/spec/models/onboarding/learn_gitlab_spec.rb new file mode 100644 index 00000000000..5e3e1f9c304 --- /dev/null +++ b/spec/models/onboarding/learn_gitlab_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Onboarding::LearnGitlab do + let_it_be(:current_user) { create(:user) } + let_it_be(:learn_gitlab_project) { create(:project, name: described_class::PROJECT_NAME) } + let_it_be(:learn_gitlab_board) { create(:board, project: learn_gitlab_project, name: described_class::BOARD_NAME) } + let_it_be(:learn_gitlab_label) { create(:label, project: learn_gitlab_project, name: described_class::LABEL_NAME) } + + before do + learn_gitlab_project.add_developer(current_user) + end + + describe '#available?' do + using RSpec::Parameterized::TableSyntax + + where(:project, :board, :label, :expected_result) do + nil | nil | nil | nil + nil | nil | true | nil + nil | true | nil | nil + nil | true | true | nil + true | nil | nil | nil + true | nil | true | nil + true | true | nil | nil + true | true | true | true + end + + with_them do + before do + allow_next_instance_of(described_class) do |learn_gitlab| + allow(learn_gitlab).to receive(:project).and_return(project) + allow(learn_gitlab).to receive(:board).and_return(board) + allow(learn_gitlab).to receive(:label).and_return(label) + end + end + + subject { described_class.new(current_user).available? } + + it { is_expected.to be expected_result } + end + end + + describe '#project' do + subject { described_class.new(current_user).project } + + it { is_expected.to eq learn_gitlab_project } + + context 'when it is created during trial signup' do + let_it_be(:learn_gitlab_project) do + create(:project, name: described_class::PROJECT_NAME_ULTIMATE_TRIAL, path: 'learn-gitlab-ultimate-trial') + end + + it { is_expected.to eq learn_gitlab_project } + end + end + + describe '#board' do + subject { described_class.new(current_user).board } + + it { is_expected.to eq learn_gitlab_board } + end + + describe '#label' do + subject { described_class.new(current_user).label } + + it { is_expected.to eq learn_gitlab_label } + end +end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding/progress_spec.rb index 9688dd01c71..9d91af2487a 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding/progress_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe OnboardingProgress do +RSpec.describe Onboarding::Progress do let(:namespace) { create(:namespace) } let(:action) { :subscription_created } @@ -34,7 +34,9 @@ RSpec.describe OnboardingProgress do subject { described_class.incomplete_actions(actions) } let!(:no_actions_completed) { create(:onboarding_progress) } - let!(:one_action_completed_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => Time.current) } + let!(:one_action_completed_one_action_incompleted) do + create(:onboarding_progress, "#{action}_at" => Time.current) + end context 'when given one action' do let(:actions) { action } @@ -52,8 +54,13 @@ RSpec.describe OnboardingProgress do describe '.completed_actions' do subject { described_class.completed_actions(actions) } - let!(:one_action_completed_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => Time.current) } - let!(:both_actions_completed) { create(:onboarding_progress, "#{action}_at" => Time.current, git_write_at: Time.current) } + let!(:one_action_completed_one_action_incompleted) do + create(:onboarding_progress, "#{action}_at" => Time.current) + end + + let!(:both_actions_completed) do + create(:onboarding_progress, "#{action}_at" => Time.current, git_write_at: Time.current) + end context 'when given one action' do let(:actions) { action } @@ -69,12 +76,23 @@ RSpec.describe OnboardingProgress do end describe '.completed_actions_with_latest_in_range' do - subject { described_class.completed_actions_with_latest_in_range(actions, 1.day.ago.beginning_of_day..1.day.ago.end_of_day) } + subject do + described_class.completed_actions_with_latest_in_range(actions, + 1.day.ago.beginning_of_day..1.day.ago.end_of_day) + end + + let!(:one_action_completed_in_range_one_action_incompleted) do + create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day) + end - let!(:one_action_completed_in_range_one_action_incompleted) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day) } let!(:git_write_action_completed_in_range) { create(:onboarding_progress, git_write_at: 1.day.ago.middle_of_day) } - let!(:both_actions_completed_latest_action_out_of_range) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: Time.current) } - let!(:both_actions_completed_latest_action_in_range) { create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: 2.days.ago.middle_of_day) } + let!(:both_actions_completed_latest_action_out_of_range) do + create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: Time.current) + end + + let!(:both_actions_completed_latest_action_in_range) do + create(:onboarding_progress, "#{action}_at" => 1.day.ago.middle_of_day, git_write_at: 2.days.ago.middle_of_day) + end context 'when given one action' do let(:actions) { :git_write } @@ -147,7 +165,9 @@ RSpec.describe OnboardingProgress do expect(described_class.find_by_namespace_id(namespace.id).subscription_created_at).to be_nil register_action expect(described_class.find_by_namespace_id(namespace.id).subscription_created_at).not_to be_nil - expect { described_class.register(namespace, action) }.not_to change { described_class.find_by_namespace_id(namespace.id).subscription_created_at } + expect do + described_class.register(namespace, action) + end.not_to change { described_class.find_by_namespace_id(namespace.id).subscription_created_at } end context 'when the action does not exist' do @@ -209,7 +229,9 @@ RSpec.describe OnboardingProgress do context 'when the namespace was not onboarded' do it 'does not register the action for the namespace' do expect { register_action }.not_to change { described_class.completed?(namespace, action1) }.from(false) - expect { described_class.register(namespace, action) }.not_to change { described_class.completed?(namespace, action2) }.from(false) + expect do + described_class.register(namespace, action) + end.not_to change { described_class.completed?(namespace, action2) }.from(false) end end end @@ -270,21 +292,23 @@ RSpec.describe OnboardingProgress do end describe '#number_of_completed_actions' do - subject { build(:onboarding_progress, actions.map { |x| { x => Time.current } }.inject(:merge)).number_of_completed_actions } + subject do + build(:onboarding_progress, actions.map { |x| { x => Time.current } }.inject(:merge)).number_of_completed_actions + end - context '0 completed actions' do + context 'with 0 completed actions' do let(:actions) { [:created_at, :updated_at] } it { is_expected.to eq(0) } end - context '1 completed action' do + context 'with 1 completed action' do let(:actions) { [:created_at, :subscription_created_at] } it { is_expected.to eq(1) } end - context '2 completed actions' do + context 'with 2 completed actions' do let(:actions) { [:subscription_created_at, :git_write_at] } it { is_expected.to eq(2) } diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index e709470b312..85a475f5c53 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -55,7 +55,7 @@ RSpec.describe Operations::FeatureFlag do it 'is valid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) feature_flag = described_class.create!({ name: 'test', project: project, version: 2, - strategies_attributes: [{ name: 'default', parameters: {} }] }) + strategies_attributes: [{ name: 'default', parameters: {} }] }) expect(feature_flag).to be_valid end @@ -114,13 +114,11 @@ RSpec.describe Operations::FeatureFlag do let_it_be(:project) { create(:project) } let!(:feature_flag) do - create(:operations_feature_flag, project: project, - name: 'feature1', active: true, version: 2) + create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 2) end let!(:strategy) do - create(:operations_strategy, feature_flag: feature_flag, - name: 'default', parameters: {}) + create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) end it 'matches wild cards in the scope' do @@ -141,10 +139,8 @@ RSpec.describe Operations::FeatureFlag do it 'returns feature flags ordered by id' do create(:operations_scope, strategy: strategy, environment_scope: 'production') - feature_flag_b = create(:operations_feature_flag, project: project, - name: 'feature2', active: true, version: 2) - strategy_b = create(:operations_strategy, feature_flag: feature_flag_b, - name: 'default', parameters: {}) + feature_flag_b = create(:operations_feature_flag, project: project, name: 'feature2', active: true, version: 2) + strategy_b = create(:operations_strategy, feature_flag: feature_flag_b, name: 'default', parameters: {}) create(:operations_scope, strategy: strategy_b, environment_scope: '*') flags = described_class.for_unleash_client(project, 'production') diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 526c57d08b0..fb88dbb4212 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_one(:nuget_metadatum).inverse_of(:package) } it { is_expected.to have_one(:rubygems_metadatum).inverse_of(:package) } it { is_expected.to have_one(:npm_metadatum).inverse_of(:package) } + it { is_expected.to have_one(:rpm_metadatum).inverse_of(:package) } end describe '.with_debian_codename' do @@ -1356,4 +1357,16 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to eq(normalized_name) } end end + + describe '#touch_last_downloaded_at' do + let_it_be(:package) { create(:package) } + + subject { package.touch_last_downloaded_at } + + it 'updates the downloaded_at' do + expect(::Gitlab::Database::LoadBalancing::Session).to receive(:without_sticky_writes).and_call_original + expect { subject } + .to change(package, :last_downloaded_at).from(nil).to(instance_of(ActiveSupport::TimeWithZone)) + end + end end diff --git a/spec/models/packages/rpm/metadatum_spec.rb b/spec/models/packages/rpm/metadatum_spec.rb new file mode 100644 index 00000000000..0e7817fdf86 --- /dev/null +++ b/spec/models/packages/rpm/metadatum_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Rpm::Metadatum, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:package) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:package) } + it { is_expected.to validate_presence_of(:epoch) } + it { is_expected.to validate_presence_of(:release) } + it { is_expected.to validate_presence_of(:summary) } + it { is_expected.to validate_presence_of(:description) } + it { is_expected.to validate_presence_of(:arch) } + + it { is_expected.to validate_numericality_of(:epoch).only_integer.is_greater_than_or_equal_to(0) } + + it { is_expected.to validate_length_of(:release).is_at_most(128) } + it { is_expected.to validate_length_of(:summary).is_at_most(1000) } + it { is_expected.to validate_length_of(:description).is_at_most(5000) } + it { is_expected.to validate_length_of(:arch).is_at_most(255) } + it { is_expected.to validate_length_of(:license).is_at_most(1000) } + it { is_expected.to validate_length_of(:url).is_at_most(1000) } + + describe '#rpm_package_type' do + it 'will not allow a package with a different package_type' do + package = build('conan_package') + rpm_metadatum = build('rpm_metadatum', package: package) + + expect(rpm_metadatum).not_to be_valid + expect(rpm_metadatum.errors.to_a).to include('Package type must be RPM') + end + end + end +end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 4e463b1194c..b50bfaed528 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -21,6 +21,15 @@ RSpec.describe PagesDomain do end end + describe '.verified' do + let!(:verified) { create(:pages_domain) } + let!(:unverified) { create(:pages_domain, :unverified) } + + it 'finds verified' do + expect(described_class.verified).to match_array(verified) + end + end + describe 'validate domain' do subject(:pages_domain) { build(:pages_domain, domain: domain) } @@ -32,17 +41,17 @@ RSpec.describe PagesDomain do describe "hostname" do { - 'my.domain.com' => true, - '123.456.789' => true, - '0x12345.com' => true, - '0123123' => true, - 'a-reserved.com' => true, + 'my.domain.com' => true, + '123.456.789' => true, + '0x12345.com' => true, + '0123123' => true, + 'a-reserved.com' => true, 'a.b-reserved.com' => true, - 'reserved.com' => true, - '_foo.com' => false, - 'a.reserved.com' => false, + 'reserved.com' => true, + '_foo.com' => false, + 'a.reserved.com' => false, 'a.b.reserved.com' => false, - nil => false + nil => false }.each do |value, validity| context "domain #{value.inspect} validity" do before do @@ -62,12 +71,11 @@ RSpec.describe PagesDomain do let(:domain) { 'my.domain.com' } let(:project) do - instance_double(Project, pages_https_only?: pages_https_only) + instance_double(Project, pages_https_only?: pages_https_only, can_create_custom_domains?: true) end let(:pages_domain) do - build(:pages_domain, certificate: certificate, key: key, - auto_ssl_enabled: auto_ssl_enabled).tap do |pd| + build(:pages_domain, certificate: certificate, key: key, auto_ssl_enabled: auto_ssl_enabled).tap do |pd| allow(pd).to receive(:project).and_return(project) pd.valid? end @@ -572,6 +580,32 @@ RSpec.describe PagesDomain do end end + describe '#validate_custom_domain_count_per_project' do + let_it_be(:project) { create(:project) } + + context 'when max custom domain setting is set to 0' do + it 'returns without an error' do + pages_domain = create(:pages_domain, project: project) + + expect(pages_domain).to be_valid + end + end + + context 'when max custom domain setting is not set to 0' do + it 'returns with an error for extra domains' do + Gitlab::CurrentSettings.update!(max_pages_custom_domains_per_project: 1) + + pages_domain = create(:pages_domain, project: project) + expect(pages_domain).to be_valid + + pages_domain = build(:pages_domain, project: project) + expect(pages_domain).not_to be_valid + expect(pages_domain.errors.full_messages) + .to contain_exactly('This project reached the limit of custom domains. (Max 1)') + end + end + end + describe '.find_by_domain_case_insensitive' do it 'lookup is case-insensitive' do pages_domain = create(:pages_domain, domain: "Pages.IO") diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index f3ef347121e..5bce6a2cc3f 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -64,6 +64,81 @@ RSpec.describe PersonalAccessToken do expect(described_class.for_users([user_1, user_2])).to contain_exactly(token_of_user_1, token_of_user_2) end end + + describe '.created_before' do + let(:last_used_at) { 1.month.ago.beginning_of_hour } + let!(:new_used_token) do + create(:personal_access_token, + created_at: last_used_at + 1.minute, + last_used_at: last_used_at + 1.minute + ) + end + + let!(:old_unused_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute + ) + end + + let!(:old_formerly_used_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute, + last_used_at: last_used_at - 1.minute + ) + end + + let!(:old_still_used_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute, + last_used_at: 1.minute.ago + ) + end + + subject { described_class.created_before(last_used_at) } + + it do + is_expected.to contain_exactly( + old_unused_token, + old_formerly_used_token, + old_still_used_token + ) + end + end + + describe '.last_used_before_or_unused' do + let(:last_used_at) { 1.month.ago.beginning_of_hour } + let!(:unused_token) { create(:personal_access_token) } + let!(:used_token) do + create(:personal_access_token, + created_at: last_used_at + 1.minute, + last_used_at: last_used_at + 1.minute + ) + end + + let!(:old_unused_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute + ) + end + + let!(:old_formerly_used_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute, + last_used_at: last_used_at - 1.minute + ) + end + + let!(:old_still_used_token) do + create(:personal_access_token, + created_at: last_used_at - 1.minute, + last_used_at: 1.minute.ago + ) + end + + subject { described_class.last_used_before_or_unused(last_used_at) } + + it { is_expected.to contain_exactly(old_unused_token, old_formerly_used_token) } + end end describe ".active?" do diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index 447b7b2e0a2..bf88e941540 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -43,6 +43,15 @@ RSpec.describe PoolRepository do end end + context 'when skipping disconnect' do + it 'does not change the alternates file' do + before = File.read(alternates_file) + pool.unlink_repository(pool.source_project.repository, disconnect: false) + + expect(File.read(alternates_file)).to eq(before) + end + end + context 'when the second member leaves' do it 'does not schedule pool removal' do other_project = create(:project, :repository, pool_repository: pool) diff --git a/spec/models/preloaders/project_policy_preloader_spec.rb b/spec/models/preloaders/project_policy_preloader_spec.rb new file mode 100644 index 00000000000..79f232f5ce2 --- /dev/null +++ b/spec/models/preloaders/project_policy_preloader_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::ProjectPolicyPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:guest_project) { create(:project, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_project) do + create(:project, :private, name: 'b private maintainer', path: 'b-private-maintainer', namespace: root_parent) + end + + let_it_be(:private_developer_project) do + create(:project, :private, name: 'c public developer', path: 'c-public-developer') + end + + let_it_be(:public_maintainer_project) do + create(:project, :private, name: 'a public maintainer', path: 'a-public-maintainer') + end + + let(:base_projects) do + Project.where(id: [guest_project, private_maintainer_project, private_developer_project, public_maintainer_project]) + end + + before_all do + guest_project.add_guest(user) + private_maintainer_project.add_maintainer(user) + private_developer_project.add_developer(user) + public_maintainer_project.add_maintainer(user) + end + + it 'avoids N+1 queries when authorizing a list of projects', :request_store do + preload_projects_for_policy(user) + control = ActiveRecord::QueryRecorder.new { authorize_all_projects(user) } + + new_project1 = create(:project, :private).tap { |project| project.add_maintainer(user) } + new_project2 = create(:project, :private, namespace: root_parent) + + another_root = create(:group, :private, name: 'root-3', path: 'root-3') + new_project3 = create(:project, :private, namespace: another_root).tap { |project| project.add_maintainer(user) } + + pristine_projects = Project.where(id: base_projects + [new_project1, new_project2, new_project3]) + + preload_projects_for_policy(user, pristine_projects) + expect { authorize_all_projects(user, pristine_projects) }.not_to exceed_query_limit(control) + end + + def authorize_all_projects(current_user, project_list = base_projects) + project_list.each { |project| current_user.can?(:read_project, project) } + end + + def preload_projects_for_policy(current_user, project_list = base_projects) + described_class.new(project_list, current_user).execute + end +end diff --git a/spec/models/preloaders/project_root_ancestor_preloader_spec.rb b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb new file mode 100644 index 00000000000..30036a6a033 --- /dev/null +++ b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::ProjectRootAncestorPreloader do + let_it_be(:root_parent1) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:root_parent2) { create(:group, :private, name: 'root-2', path: 'root-2') } + let_it_be(:guest_project) { create(:project, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_project) do + create(:project, :private, name: 'b private maintainer', path: 'b-private-maintainer', namespace: root_parent1) + end + + let_it_be(:private_developer_project) do + create(:project, :private, name: 'c public developer', path: 'c-public-developer') + end + + let_it_be(:public_maintainer_project) do + create(:project, :private, name: 'a public maintainer', path: 'a-public-maintainer', namespace: root_parent2) + end + + let(:root_query_regex) { /\ASELECT.+FROM "namespaces" WHERE "namespaces"."id" = \d+/ } + let(:additional_preloads) { [] } + let(:projects) { [guest_project, private_maintainer_project, private_developer_project, public_maintainer_project] } + let(:pristine_projects) { Project.where(id: projects) } + + shared_examples 'executes N matching DB queries' do |expected_query_count, query_method = nil| + it 'executes the specified root_ancestor queries' do + expect do + pristine_projects.each do |project| + root_ancestor = project.root_ancestor + + root_ancestor.public_send(query_method) if query_method.present? + end + end.to make_queries_matching(root_query_regex, expected_query_count) + end + + it 'strong_memoizes the correct root_ancestor' do + pristine_projects.each do |project| + expected_parent_id = project.root_ancestor&.id + + expect(project.parent_id).to eq(expected_parent_id) + end + end + end + + context 'when use_traversal_ids FF is enabled' do + context 'when the preloader is used' do + before do + preload_ancestors + end + + context 'when no additional preloads are provided' do + it_behaves_like 'executes N matching DB queries', 0 + end + + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + + it_behaves_like 'executes N matching DB queries', 0, :full_path + end + end + + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 4 + end + end + + context 'when use_traversal_ids FF is disabled' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + context 'when the preloader is used' do + before do + preload_ancestors + end + + context 'when no additional preloads are provided' do + it_behaves_like 'executes N matching DB queries', 4 + end + + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + + it_behaves_like 'executes N matching DB queries', 4, :full_path + end + end + + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 4 + end + end + + def preload_ancestors + described_class.new(pristine_projects, :namespace, additional_preloads).execute + end +end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index fb1601a5f9c..a09ae7ec7ae 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -63,4 +63,51 @@ RSpec.describe ProjectSetting, type: :model do target_platforms.permutation(n).to_a end end + + describe '#show_diff_preview_in_email?' do + context 'when a project is a top-level namespace' do + let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: false) } + let(:project) { create(:project, project_setting: project_settings) } + + context 'when show_diff_preview_in_email is disabled' do + it 'returns false' do + expect(project).not_to be_show_diff_preview_in_email + end + end + + context 'when show_diff_preview_in_email is enabled' do + let(:project_settings ) { create(:project_setting, show_diff_preview_in_email: true) } + + it 'returns true' do + settings = create(:project_setting, show_diff_preview_in_email: true) + project = create(:project, project_setting: settings) + + expect(project).to be_show_diff_preview_in_email + end + end + end + + context 'when a parent group has a parent group' do + let(:namespace_settings) { create(:namespace_settings, show_diff_preview_in_email: false) } + let(:project_settings) { create(:project_setting, show_diff_preview_in_email: true) } + let(:group) { create(:group, namespace_settings: namespace_settings) } + let!(:project) { create(:project, namespace_id: group.id, project_setting: project_settings) } + + context 'when show_diff_preview_in_email is disabled for the parent group' do + it 'returns false' do + expect(project).not_to be_show_diff_preview_in_email + end + end + + context 'when all ancestors have enabled diff previews' do + let(:namespace_settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + + it 'returns true' do + group.update_attribute(:show_diff_preview_in_email, true) + + expect(project).to be_show_diff_preview_in_email + end + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 98b202299a8..99b984ff547 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -366,12 +366,35 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to include_module(Sortable) } end + describe 'before_validation' do + context 'with removal of leading spaces' do + subject(:project) { build(:project, name: ' space first', path: 'some_path') } + + it 'removes the leading space' do + expect(project.name).to eq ' space first' + + expect(project).to be_valid # triggers before_validation and assures we automatically handle the bad format + + expect(project.name).to eq 'space first' + end + + context 'when name is nil' do + it 'falls through to the presence validation' do + project.name = nil + + expect(project).not_to be_valid + end + end + end + end + describe 'validation' do let!(:project) { create(:project) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) } it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to allow_value('space last ').for(:name) } it { is_expected.not_to allow_value('colon:in:path').for(:path) } # This is to validate that a specially crafted name cannot bypass a pattern match. See !72555 it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_most(255) } @@ -1736,8 +1759,8 @@ RSpec.describe Project, factory_default: :keep do end end - describe '.with_shared_runners' do - subject { described_class.with_shared_runners } + describe '.with_shared_runners_enabled' do + subject { described_class.with_shared_runners_enabled } context 'when shared runners are enabled for project' do let!(:project) { create(:project, shared_runners_enabled: true) } @@ -3925,162 +3948,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#ci_variables_for' do - let_it_be(:project) { create(:project) } - - let(:environment_scope) { '*' } - - let!(:ci_variable) do - create(:ci_variable, value: 'secret', project: project, environment_scope: environment_scope) - end - - let!(:protected_variable) do - create(:ci_variable, :protected, value: 'protected', project: project) - end - - subject { project.reload.ci_variables_for(ref: 'ref') } - - before do - stub_application_setting( - default_branch_protection: Gitlab::Access::PROTECTION_NONE) - end - - shared_examples 'ref is protected' do - it 'contains all the variables' do - is_expected.to contain_exactly(ci_variable, protected_variable) - end - end - - it 'memoizes the result by ref and environment', :request_store do - scoped_variable = create(:ci_variable, value: 'secret', project: project, environment_scope: 'scoped') - - expect(project).to receive(:protected_for?).with('ref').once.and_return(true) - expect(project).to receive(:protected_for?).with('other').twice.and_return(false) - - 2.times do - expect(project.reload.ci_variables_for(ref: 'ref', environment: 'production')).to contain_exactly(ci_variable, protected_variable) - expect(project.reload.ci_variables_for(ref: 'other')).to contain_exactly(ci_variable) - expect(project.reload.ci_variables_for(ref: 'other', environment: 'scoped')).to contain_exactly(ci_variable, scoped_variable) - end - end - - context 'when the ref is not protected' do - before do - allow(project).to receive(:protected_for?).with('ref').and_return(false) - end - - it 'contains only the CI variables' do - is_expected.to contain_exactly(ci_variable) - end - end - - context 'when the ref is a protected branch' do - before do - allow(project).to receive(:protected_for?).with('ref').and_return(true) - end - - it_behaves_like 'ref is protected' - end - - context 'when the ref is a protected tag' do - before do - allow(project).to receive(:protected_for?).with('ref').and_return(true) - end - - it_behaves_like 'ref is protected' - end - - context 'when environment name is specified' do - let(:environment) { 'review/name' } - - subject do - project.ci_variables_for(ref: 'ref', environment: environment) - end - - context 'when environment scope is exactly matched' do - let(:environment_scope) { 'review/name' } - - it { is_expected.to contain_exactly(ci_variable) } - end - - context 'when environment scope is matched by wildcard' do - let(:environment_scope) { 'review/*' } - - it { is_expected.to contain_exactly(ci_variable) } - end - - context 'when environment scope does not match' do - let(:environment_scope) { 'review/*/special' } - - it { is_expected.not_to contain_exactly(ci_variable) } - end - - context 'when environment scope has _' do - let(:environment_scope) { '*_*' } - - it 'does not treat it as wildcard' do - is_expected.not_to contain_exactly(ci_variable) - end - - context 'when environment name contains underscore' do - let(:environment) { 'foo_bar/test' } - let(:environment_scope) { 'foo_bar/*' } - - it 'matches literally for _' do - is_expected.to contain_exactly(ci_variable) - end - end - end - - # The environment name and scope cannot have % at the moment, - # but we're considering relaxing it and we should also make sure - # it doesn't break in case some data sneaked in somehow as we're - # not checking this integrity in database level. - context 'when environment scope has %' do - it 'does not treat it as wildcard' do - ci_variable.update_attribute(:environment_scope, '*%*') - - is_expected.not_to contain_exactly(ci_variable) - end - - context 'when environment name contains a percent' do - let(:environment) { 'foo%bar/test' } - - it 'matches literally for _' do - ci_variable.environment_scope = 'foo%bar/*' - - is_expected.to contain_exactly(ci_variable) - end - end - end - - context 'when variables with the same name have different environment scopes' do - let!(:partially_matched_variable) do - create(:ci_variable, - key: ci_variable.key, - value: 'partial', - environment_scope: 'review/*', - project: project) - end - - let!(:perfectly_matched_variable) do - create(:ci_variable, - key: ci_variable.key, - value: 'prefect', - environment_scope: 'review/name', - project: project) - end - - it 'puts variables matching environment scope more in the end' do - is_expected.to eq( - [ci_variable, - partially_matched_variable, - perfectly_matched_variable]) - end - end - end - end - describe '#any_lfs_file_locks?', :request_store do let_it_be(:project) { create(:project) } @@ -6290,14 +6157,6 @@ RSpec.describe Project, factory_default: :keep do let!(:deploy_token) { create(:deploy_token, :gitlab_deploy_token, :group, groups: [group]) } it { is_expected.to eq(deploy_token) } - - context 'when the FF ci_variable_for_group_gitlab_deploy_token is disabled' do - before do - stub_feature_flags(ci_variable_for_group_gitlab_deploy_token: false) - end - - it { is_expected.to be_nil } - end end context 'when the project and its group has a gitlab deploy token associated' do @@ -6307,14 +6166,6 @@ RSpec.describe Project, factory_default: :keep do let!(:group_deploy_token) { create(:deploy_token, :gitlab_deploy_token, :group, groups: [group]) } it { is_expected.to eq(project_deploy_token) } - - context 'when the FF ci_variable_for_group_gitlab_deploy_token is disabled' do - before do - stub_feature_flags(ci_variable_for_group_gitlab_deploy_token: false) - end - - it { is_expected.to eq(project_deploy_token) } - end end end @@ -6624,11 +6475,27 @@ RSpec.describe Project, factory_default: :keep do let(:pool) { create(:pool_repository) } let(:project) { create(:project, :repository, pool_repository: pool) } - it 'removes the membership' do - project.leave_pool_repository + subject { project.leave_pool_repository } + + it 'removes the membership and disconnects alternates' do + expect(pool).to receive(:unlink_repository).with(project.repository, disconnect: true).and_call_original + + subject expect(pool.member_projects.reload).not_to include(project) end + + context 'when the project is pending delete' do + it 'removes the membership and does not disconnect alternates' do + project.pending_delete = true + + expect(pool).to receive(:unlink_repository).with(project.repository, disconnect: false).and_call_original + + subject + + expect(pool.member_projects.reload).not_to include(project) + end + end end describe '#check_personal_projects_limit' do @@ -8434,6 +8301,25 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#packages_policy_subject' do + let_it_be(:project) { create(:project) } + + it 'returns wrapper' do + expect(project.packages_policy_subject).to be_a(Packages::Policies::Project) + expect(project.packages_policy_subject.project).to eq(project) + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(read_package_policy_rule: false) + end + + it 'returns project' do + expect(project.packages_policy_subject).to eq(project) + end + end + end + describe '#destroy_deployment_by_id' do let(:project) { create(:project, :repository) } @@ -8451,6 +8337,25 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#can_create_custom_domains?' do + let_it_be(:project) { create(:project) } + let_it_be(:pages_domain) { create(:pages_domain, project: project) } + + subject { project.can_create_custom_domains? } + + context 'when max custom domain setting is set to 0' do + it { is_expected.to be true } + end + + context 'when max custom domain setting is not set to 0' do + before do + Gitlab::CurrentSettings.update!(max_pages_custom_domains_per_project: 1) + end + + it { is_expected.to be false } + end + end + private def finish_job(export_job) diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index f4edc68457b..b2158baa670 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -407,6 +407,25 @@ RSpec.describe ProjectStatistics do end end + describe '#refresh_storage_size!' do + it 'recalculates storage size from its components and save it' do + statistics.update_columns( + repository_size: 2, + wiki_size: 4, + lfs_objects_size: 3, + snippets_size: 2, + pipeline_artifacts_size: 3, + build_artifacts_size: 3, + packages_size: 6, + uploads_size: 5, + + storage_size: 0 + ) + + expect { statistics.refresh_storage_size! }.to change { statistics.storage_size }.from(0).to(28) + end + end + describe '.increment_statistic' do shared_examples 'a statistic that increases storage_size' do it 'increases the statistic by that amount' do @@ -432,16 +451,15 @@ RSpec.describe ProjectStatistics do end end - it 'schedules a worker to update the statistic and storage_size async' do + it 'schedules a worker to update the statistic and storage_size async', :sidekiq_inline do expect(FlushCounterIncrementsWorker) .to receive(:perform_in) .with(CounterAttribute::WORKER_DELAY, described_class.name, statistics.id, stat) + .and_call_original - expect(FlushCounterIncrementsWorker) - .to receive(:perform_in) - .with(CounterAttribute::WORKER_DELAY, described_class.name, statistics.id, :storage_size) - - described_class.increment_statistic(project, stat, 20) + expect { described_class.increment_statistic(project, stat, 20) } + .to change { statistics.reload.send(stat) }.by(20) + .and change { statistics.reload.send(:storage_size) }.by(20) end end diff --git a/spec/models/projects/build_artifacts_size_refresh_spec.rb b/spec/models/projects/build_artifacts_size_refresh_spec.rb index 052e654af76..21cd8e0b9d4 100644 --- a/spec/models/projects/build_artifacts_size_refresh_spec.rb +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -265,4 +265,16 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do it { is_expected.to eq(result) } end end + + describe 'callbacks' do + context 'when destroyed' do + it 'enqueues a Namespaces::ScheduleAggregationWorker' do + refresh = create(:project_build_artifacts_size_refresh) + + expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async).with(refresh.project.namespace_id) + + refresh.destroy! + end + end + end end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 3936e7127b8..54a90ca6049 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -171,8 +171,8 @@ RSpec.describe ProtectedBranch do let_it_be(:project) { create(:project, :repository) } let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "“jawn”") } - let(:feature_flag) { true } - let(:dry_run) { true } + let(:use_new_cache_implementation) { true } + let(:rely_on_new_cache) { true } shared_examples_for 'hash based cache implementation' do it 'calls only hash based cache implementation' do @@ -182,19 +182,22 @@ RSpec.describe ProtectedBranch do expect(Rails.cache).not_to receive(:fetch) - described_class.protected?(project, 'missing-branch', dry_run: dry_run) + described_class.protected?(project, 'missing-branch') end end before do - stub_feature_flags(hash_based_cache_for_protected_branches: feature_flag) + stub_feature_flags(hash_based_cache_for_protected_branches: use_new_cache_implementation) + stub_feature_flags(rely_on_protected_branches_cache: rely_on_new_cache) allow(described_class).to receive(:matching).and_call_original # the original call works and warms the cache - described_class.protected?(project, protected_branch.name, dry_run: dry_run) + described_class.protected?(project, protected_branch.name) end - context 'Dry-run: true' do + context 'Dry-run: true (rely_on_protected_branches_cache is off, new hash-based is used)' do + let(:rely_on_new_cache) { false } + it 'recalculates a fresh value every time in order to check the cache is not returning stale data' do expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).twice @@ -204,21 +207,21 @@ RSpec.describe ProtectedBranch do it_behaves_like 'hash based cache implementation' end - context 'Dry-run: false' do - let(:dry_run) { false } + context 'Dry-run: false (rely_on_protected_branches_cache is enabled, new hash-based cache is used)' do + let(:rely_on_new_cache) { true } it 'correctly invalidates a cache' do expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).exactly(3).times.and_call_original create_params = { name: 'bar', merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }] } branch = ProtectedBranches::CreateService.new(project, project.owner, create_params).execute - expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) ProtectedBranches::UpdateService.new(project, project.owner, name: 'ber').execute(branch) - expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) ProtectedBranches::DestroyService.new(project, project.owner).execute(branch) - expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) end it_behaves_like 'hash based cache implementation' @@ -229,7 +232,7 @@ RSpec.describe ProtectedBranch do project.touch - described_class.protected?(project, protected_branch.name, dry_run: dry_run) + described_class.protected?(project, protected_branch.name) end end @@ -240,19 +243,19 @@ RSpec.describe ProtectedBranch do another_project = create(:project) ProtectedBranches::CreateService.new(another_project, another_project.owner, name: 'bar').execute - described_class.protected?(project, protected_branch.name, dry_run: dry_run) + described_class.protected?(project, protected_branch.name) end end it 'correctly uses the cached version' do expect(described_class).not_to receive(:matching) - expect(described_class.protected?(project, protected_branch.name, dry_run: dry_run)).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) end end context 'when feature flag hash_based_cache_for_protected_branches is off' do - let(:feature_flag) { false } + let(:use_new_cache_implementation) { false } it 'does not call hash based cache implementation' do expect(ProtectedBranches::CacheService).not_to receive(:new) diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 429ad550626..adb4777ae90 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -359,10 +359,10 @@ RSpec.describe RemoteMirror, :mailer do it 'resets all the columns when URL changes' do remote_mirror.update!(last_error: Time.current, - last_update_at: Time.current, - last_successful_update_at: Time.current, - update_status: 'started', - error_notification_sent: true) + last_update_at: Time.current, + last_successful_update_at: Time.current, + update_status: 'started', + error_notification_sent: true) expect { remote_mirror.update_attribute(:url, 'http://new.example.com') } .to change { remote_mirror.last_error }.to(nil) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 47532ed1216..4e386bf584f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -161,6 +161,33 @@ RSpec.describe Repository do end end + context 'semantic versioning sort' do + let(:version_two) { 'v2.0.0' } + let(:version_ten) { 'v10.0.0' } + + before do + repository.add_tag(user, version_two, repository.commit.id) + repository.add_tag(user, version_ten, repository.commit.id) + end + + after do + repository.rm_tag(user, version_two) + repository.rm_tag(user, version_ten) + end + + context 'desc' do + subject { repository.tags_sorted_by('version_desc').map(&:name) & (tags_to_compare + [version_two, version_ten]) } + + it { is_expected.to eq([version_ten, version_two, 'v1.1.0', 'v1.0.0']) } + end + + context 'asc' do + subject { repository.tags_sorted_by('version_asc').map(&:name) & (tags_to_compare + [version_two, version_ten]) } + + it { is_expected.to eq(['v1.0.0', 'v1.1.0', version_two, version_ten]) } + end + end + context 'unknown option' do subject { repository.tags_sorted_by('unknown_desc').map(&:name) & tags_to_compare } @@ -518,6 +545,54 @@ RSpec.describe Repository do end end + describe '#list_commits_by' do + it 'returns commits with messages containing a given string' do + commit_ids = repository.list_commits_by('test text', 'master').map(&:id) + + expect(commit_ids).to include( + 'b83d6e391c22777fca1ed3012fce84f633d7fed0', + '498214de67004b1da3d820901307bed2a68a8ef6' + ) + expect(commit_ids).not_to include('c84ff944ff4529a70788a5e9003c2b7feae29047') + end + + it 'is case insensitive' do + commit_ids = repository.list_commits_by('TEST TEXT', 'master').map(&:id) + + expect(commit_ids).to include('b83d6e391c22777fca1ed3012fce84f633d7fed0') + end + + it 'returns commits based in before filter' do + commit_ids = repository.list_commits_by('test text', 'master', before: 1474828200).map(&:id) + expect(commit_ids).to include( + '498214de67004b1da3d820901307bed2a68a8ef6' + ) + expect(commit_ids).not_to include('b83d6e391c22777fca1ed3012fce84f633d7fed0') + end + + it 'returns commits based in after filter' do + commit_ids = repository.list_commits_by('test text', 'master', after: 1474828200).map(&:id) + expect(commit_ids).to include( + 'b83d6e391c22777fca1ed3012fce84f633d7fed0' + ) + expect(commit_ids).not_to include('498214de67004b1da3d820901307bed2a68a8ef6') + end + + it 'returns commits based in author filter' do + commit_ids = repository.list_commits_by('test text', 'master', author: 'Job van der Voort').map(&:id) + expect(commit_ids).to include( + 'b83d6e391c22777fca1ed3012fce84f633d7fed0' + ) + expect(commit_ids).not_to include('498214de67004b1da3d820901307bed2a68a8ef6') + end + + describe 'when storage is broken', :broken_storage do + it 'raises a storage error' do + expect_to_raise_storage_error { broken_repository.list_commits_by('s') } + end + end + end + describe '#blob_at' do context 'blank sha' do subject { repository.blob_at(Gitlab::Git::BLANK_SHA, '.gitignore') } @@ -3306,7 +3381,7 @@ RSpec.describe Repository do before do storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'), - 'picked' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories') + 'picked' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories') } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) diff --git a/spec/models/resource_state_event_spec.rb b/spec/models/resource_state_event_spec.rb index 2b4898b750a..f84634bd220 100644 --- a/spec/models/resource_state_event_spec.rb +++ b/spec/models/resource_state_event_spec.rb @@ -42,16 +42,44 @@ RSpec.describe ResourceStateEvent, type: :model do context 'callbacks' do describe '#issue_usage_metrics' do - it 'tracks closed issues' do - expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_closed_action) - - create(described_class.name.underscore.to_sym, issue: issue, state: described_class.states[:closed]) + describe 'when an issue is closed' do + subject(:close_issue) do + create(described_class.name.underscore.to_sym, issue: issue, + state: described_class.states[:closed]) + end + + it 'tracks closed issues' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_closed_action) + + close_issue + end + + it_behaves_like 'issue_edit snowplow tracking' do + let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CLOSED } + let(:project) { issue.project } + let(:user) { issue.author } + subject(:service_action) { close_issue } + end end - it 'tracks reopened issues' do - expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_reopened_action) + describe 'when an issue is reopened' do + subject(:reopen_issue) do + create(described_class.name.underscore.to_sym, issue: issue, + state: described_class.states[:reopened]) + end + + it 'tracks reopened issues' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_reopened_action) + + reopen_issue + end - create(described_class.name.underscore.to_sym, issue: issue, state: described_class.states[:reopened]) + it_behaves_like 'issue_edit snowplow tracking' do + let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_REOPENED } + let(:project) { issue.project } + let(:user) { issue.author } + subject(:service_action) { reopen_issue } + end end it 'does not track merge requests' do diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index e8a933d2277..655cfad57c9 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -39,7 +39,7 @@ RSpec.describe SnippetRepository do let(:data) { [new_file, move_file, update_file] } it 'returns nil when files argument is empty' do - expect(snippet.repository).not_to receive(:multi_action) + expect(snippet.repository).not_to receive(:commit_files) operation = snippet_repository.multi_files_action(user, [], **commit_opts) @@ -47,7 +47,7 @@ RSpec.describe SnippetRepository do end it 'returns nil when files argument is nil' do - expect(snippet.repository).not_to receive(:multi_action) + expect(snippet.repository).not_to receive(:commit_files) operation = snippet_repository.multi_files_action(user, nil, **commit_opts) @@ -119,7 +119,7 @@ RSpec.describe SnippetRepository do end it 'infers the commit action based on the parameters if not present' do - expect(repo).to receive(:multi_action).with(user, hash_including(actions: result)) + expect(repo).to receive(:commit_files).with(user, hash_including(actions: result)) snippet_repository.multi_files_action(user, data, **commit_opts) end @@ -131,7 +131,7 @@ RSpec.describe SnippetRepository do specify do expect(repo).to( - receive(:multi_action).with( + receive(:commit_files).with( user, hash_including(actions: array_including(hash_including(action: expected_action))))) diff --git a/spec/models/spam_log_spec.rb b/spec/models/spam_log_spec.rb index 97a0dc27f17..a40c7c5c892 100644 --- a/spec/models/spam_log_spec.rb +++ b/spec/models/spam_log_spec.rb @@ -21,15 +21,37 @@ RSpec.describe SpamLog do end context 'when admin mode is enabled', :enable_admin_mode do - it 'removes the user', :sidekiq_might_not_need_inline do - spam_log = build(:spam_log) - user = spam_log.user + context 'when user_destroy_with_limited_execution_time_worker is enabled' do + it 'initiates user removal', :sidekiq_inline do + spam_log = build(:spam_log) + user = spam_log.user + + perform_enqueued_jobs do + spam_log.remove_user(deleted_by: admin) + end + + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + ).to be_exists + end + end - perform_enqueued_jobs do - spam_log.remove_user(deleted_by: admin) + context 'when user_destroy_with_limited_execution_time_worker is disabled' do + before do + stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) end - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + it 'removes the user', :sidekiq_inline do + spam_log = build(:spam_log) + user = spam_log.user + + perform_enqueued_jobs do + spam_log.remove_user(deleted_by: admin) + end + + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 69cd51137b5..04f2c7f9176 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -334,6 +334,58 @@ RSpec.describe User do end end end + + context 'check_password_weakness' do + let(:weak_password) { "qwertyuiop" } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(block_weak_passwords: false) + end + + it 'does not add an error when password is weak' do + expect(Security::WeakPasswords).not_to receive(:weak_for_user?) + + user.password = weak_password + expect(user).to be_valid + end + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(block_weak_passwords: true) + end + + it 'checks for password weakness when password changes' do + expect(Security::WeakPasswords).to receive(:weak_for_user?) + .with(weak_password, user).and_call_original + user.password = weak_password + expect(user).not_to be_valid + end + + it 'adds an error when password is weak' do + user.password = weak_password + expect(user).not_to be_valid + expect(user.errors).to be_of_kind(:password, 'must not contain commonly used combinations of words and letters') + end + + it 'is valid when password is not weak' do + user.password = ::User.random_password + expect(user).to be_valid + end + + it 'is valid when weak password was already set' do + user = build(:user, password: weak_password) + user.save!(validate: false) + + expect(Security::WeakPasswords).not_to receive(:weak_for_user?) + + # Change an unrelated value + user.name = "Example McExampleFace" + expect(user).to be_valid + end + end + end end describe 'name' do @@ -2071,11 +2123,12 @@ RSpec.describe User do context 'user has existing U2F registration' do it 'returns false' do device = U2F::FakeU2F.new(FFaker::BaconIpsum.characters(5)) - create(:u2f_registration, name: 'my u2f device', - user: user, - certificate: Base64.strict_encode64(device.cert_raw), - key_handle: U2F.urlsafe_encode64(device.key_handle_raw), - public_key: Base64.strict_encode64(device.origin_public_key_raw)) + create(:u2f_registration, + name: 'my u2f device', + user: user, + certificate: Base64.strict_encode64(device.cert_raw), + key_handle: U2F.urlsafe_encode64(device.key_handle_raw), + public_key: Base64.strict_encode64(device.origin_public_key_raw)) expect(user.two_factor_u2f_enabled?).to eq(false) end @@ -2094,11 +2147,12 @@ RSpec.describe User do context 'user has existing U2F registration' do it 'returns true' do device = U2F::FakeU2F.new(FFaker::BaconIpsum.characters(5)) - create(:u2f_registration, name: 'my u2f device', - user: user, - certificate: Base64.strict_encode64(device.cert_raw), - key_handle: U2F.urlsafe_encode64(device.key_handle_raw), - public_key: Base64.strict_encode64(device.origin_public_key_raw)) + create(:u2f_registration, + name: 'my u2f device', + user: user, + certificate: Base64.strict_encode64(device.cert_raw), + key_handle: U2F.urlsafe_encode64(device.key_handle_raw), + public_key: Base64.strict_encode64(device.origin_public_key_raw)) expect(user.two_factor_u2f_enabled?).to eq(true) end @@ -3601,15 +3655,15 @@ RSpec.describe User do user = create :user project = create(:project, :public) - expect(user.starred?(project)).to be_falsey - - user.toggle_star(project) - - expect(user.starred?(project)).to be_truthy - - user.toggle_star(project) + # starring + expect { user.toggle_star(project) } + .to change { user.starred?(project) }.from(false).to(true) + .and not_change { project.reload.updated_at } - expect(user.starred?(project)).to be_falsey + # unstarring + expect { user.toggle_star(project) } + .to change { user.starred?(project) }.from(true).to(false) + .and not_change { project.reload.updated_at } end end @@ -3810,8 +3864,8 @@ RSpec.describe User do describe '#can_be_deactivated?' do let(:activity) { {} } let(:user) { create(:user, name: 'John Smith', **activity) } - let(:day_within_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.pred.days.ago } - let(:day_outside_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.next.days.ago } + let(:day_within_minium_inactive_days_threshold) { Gitlab::CurrentSettings.deactivate_dormant_users_period.pred.days.ago } + let(:day_outside_minium_inactive_days_threshold) { Gitlab::CurrentSettings.deactivate_dormant_users_period.next.days.ago } shared_examples 'not eligible for deactivation' do it 'returns false' do @@ -7193,8 +7247,8 @@ RSpec.describe User do describe '.dormant' do it 'returns dormant users' do freeze_time do - not_that_long_ago = (described_class::MINIMUM_INACTIVE_DAYS - 1).days.ago.to_date - too_long_ago = described_class::MINIMUM_INACTIVE_DAYS.days.ago.to_date + not_that_long_ago = (Gitlab::CurrentSettings.deactivate_dormant_users_period - 1).days.ago.to_date + too_long_ago = Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date create(:user, :deactivated, last_activity_on: too_long_ago) @@ -7214,8 +7268,8 @@ RSpec.describe User do describe '.with_no_activity' do it 'returns users with no activity' do freeze_time do - active_not_that_long_ago = (described_class::MINIMUM_INACTIVE_DAYS - 1).days.ago.to_date - active_too_long_ago = described_class::MINIMUM_INACTIVE_DAYS.days.ago.to_date + active_not_that_long_ago = (Gitlab::CurrentSettings.deactivate_dormant_users_period - 1).days.ago.to_date + active_too_long_ago = Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date created_recently = (described_class::MINIMUM_DAYS_CREATED - 1).days.ago.to_date created_not_recently = described_class::MINIMUM_DAYS_CREATED.days.ago.to_date @@ -7396,25 +7450,6 @@ RSpec.describe User do let(:factory_name) { :user } end - describe 'mr_attention_requests_enabled?' do - let(:user) { create(:user) } - - before do - stub_feature_flags(mr_attention_requests: false) - end - - it { expect(user.mr_attention_requests_enabled?).to be(false) } - - it 'feature flag is enabled for user' do - stub_feature_flags(mr_attention_requests: user) - - another_user = create(:user) - - expect(user.mr_attention_requests_enabled?).to be(true) - expect(another_user.mr_attention_requests_enabled?).to be(false) - end - end - describe 'user age' do let(:user) { create(:user, created_at: Date.yesterday) } diff --git a/spec/models/user_status_spec.rb b/spec/models/user_status_spec.rb index 663df9712ab..289e1ce1856 100644 --- a/spec/models/user_status_spec.rb +++ b/spec/models/user_status_spec.rb @@ -18,6 +18,14 @@ RSpec.describe UserStatus do expect { status.user.destroy! }.to change { described_class.count }.from(1).to(0) end + describe '#clear_status_after' do + it 'is an alias of #clear_status_at', :freeze_time do + status = build(:user_status, clear_status_at: 8.hours.from_now) + + expect(status.clear_status_after).to be_like_time(8.hours.from_now) + end + end + describe '#clear_status_after=' do it 'sets clear_status_at' do status = build(:user_status) diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index 34cfd500c26..58b529ff18a 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -28,4 +28,27 @@ RSpec.describe Users::CreditCardValidation do expect(subject.similar_records).to eq([match2, match1, subject]) end end + + describe '#similar_holder_names_count' do + subject!(:credit_card_validation) { create(:credit_card_validation, holder_name: holder_name) } + + context 'when holder_name is present' do + let(:holder_name) { 'ALICE M SMITH' } + + let!(:match) { create(:credit_card_validation, holder_name: 'Alice M Smith') } + let!(:non_match) { create(:credit_card_validation, holder_name: 'Bob B Brown') } + + it 'returns the count of cards with similar case insensitive holder names' do + expect(subject.similar_holder_names_count).to eq(2) + end + end + + context 'when holder_name is nil' do + let(:holder_name) { nil } + + it 'returns 0' do + expect(subject.similar_holder_names_count).to eq(0) + end + end + end end diff --git a/spec/models/users/ghost_user_migration_spec.rb b/spec/models/users/ghost_user_migration_spec.rb new file mode 100644 index 00000000000..d4a0657c3be --- /dev/null +++ b/spec/models/users/ghost_user_migration_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::GhostUserMigration do + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:initiator_user) } + end + + describe 'validation' do + it { is_expected.to validate_presence_of(:user_id) } + end +end diff --git a/spec/models/users/merge_request_interaction_spec.rb b/spec/models/users/merge_request_interaction_spec.rb index a499a7c68e8..0b1888bd9a6 100644 --- a/spec/models/users/merge_request_interaction_spec.rb +++ b/spec/models/users/merge_request_interaction_spec.rb @@ -59,11 +59,8 @@ RSpec.describe ::Users::MergeRequestInteraction do context 'when the user has been asked to review the MR' do before do merge_request.reviewers << user - merge_request.find_reviewer(user).update!(state: :attention_requested) end - it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUESTED'].value) } - it 'implies not reviewed' do expect(interaction).not_to be_reviewed end diff --git a/spec/models/users_star_project_spec.rb b/spec/models/users_star_project_spec.rb new file mode 100644 index 00000000000..e41519a2b69 --- /dev/null +++ b/spec/models/users_star_project_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe UsersStarProject, type: :model do + it { is_expected.to belong_to(:project).touch(false) } +end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index e2240c225a9..341f9a9c60f 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -59,6 +59,14 @@ RSpec.describe WorkItem do create(:work_item) end + + it_behaves_like 'issue_edit snowplow tracking' do + let(:work_item) { create(:work_item) } + let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:project) { work_item.project } + let(:user) { work_item.author } + subject(:service_action) { work_item } + end end context 'work item namespace' do @@ -123,8 +131,8 @@ RSpec.describe WorkItem do child.confidential = false expect(child).not_to be_valid - expect(child.errors[:confidential]) - .to include('associated parent is confidential and can not have non-confidential children.') + expect(child.errors[:base]) + .to include(_('A non-confidential work item cannot have a confidential parent.')) end it 'allows to make parent non-confidential' do @@ -143,8 +151,9 @@ RSpec.describe WorkItem do parent.confidential = true expect(parent).not_to be_valid - expect(parent.errors[:confidential]) - .to include('confidential parent can not be used if there are non-confidential children.') + expect(parent.errors[:base]).to include( + _('A confidential work item cannot have a parent that already has non-confidential children.') + ) end it 'allows to make child confidential' do @@ -161,8 +170,8 @@ RSpec.describe WorkItem do child.work_item_parent = create(:work_item, confidential: true, project: project) expect(child).not_to be_valid - expect(child.errors[:confidential]) - .to include('associated parent is confidential and can not have non-confidential children.') + expect(child.errors[:base]) + .to include('A non-confidential work item cannot have a confidential parent.') end end end diff --git a/spec/models/work_items/widgets/description_spec.rb b/spec/models/work_items/widgets/description_spec.rb index 8359db31bff..c24dc9cfb9c 100644 --- a/spec/models/work_items/widgets/description_spec.rb +++ b/spec/models/work_items/widgets/description_spec.rb @@ -3,7 +3,10 @@ require 'spec_helper' RSpec.describe WorkItems::Widgets::Description do - let_it_be(:work_item) { create(:work_item, description: '# Title') } + let_it_be(:user) { create(:user) } + let_it_be(:work_item, refind: true) do + create(:work_item, description: 'Title', last_edited_at: 10.days.ago, last_edited_by: user) + end describe '.type' do subject { described_class.type } @@ -22,4 +25,42 @@ RSpec.describe WorkItems::Widgets::Description do it { is_expected.to eq(work_item.description) } end + + describe '#edited?' do + subject { described_class.new(work_item).edited? } + + it { is_expected.to be_truthy } + end + + describe '#last_edited_at' do + subject { described_class.new(work_item).last_edited_at } + + it { is_expected.to eq(work_item.last_edited_at) } + end + + describe '#last_edited_by' do + subject { described_class.new(work_item).last_edited_by } + + context 'when the work item is edited' do + context 'when last edited user still exists in the DB' do + it { is_expected.to eq(user) } + end + + context 'when last edited user no longer exists' do + before do + work_item.update!(last_edited_by: nil) + end + + it { is_expected.to eq(User.ghost) } + end + end + + context 'when the work item is not edited yet' do + before do + work_item.update!(last_edited_at: nil) + end + + it { is_expected.to be_nil } + end + end end |