diff options
Diffstat (limited to 'spec/models')
85 files changed, 2648 insertions, 683 deletions
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index d12f9b9100a..7bef3d30064 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -51,6 +51,18 @@ describe ApplicationSetting do it { is_expected.to allow_value(nil).for(:static_objects_external_storage_url) } it { is_expected.to allow_value(http).for(:static_objects_external_storage_url) } it { is_expected.to allow_value(https).for(:static_objects_external_storage_url) } + it { is_expected.to allow_value(['/example'] * 100).for(:protected_paths) } + it { is_expected.not_to allow_value(['/example'] * 101).for(:protected_paths) } + it { is_expected.not_to allow_value(nil).for(:protected_paths) } + it { is_expected.to allow_value([]).for(:protected_paths) } + + it { is_expected.to allow_value(3).for(:push_event_hooks_limit) } + it { is_expected.not_to allow_value('three').for(:push_event_hooks_limit) } + it { is_expected.not_to allow_value(nil).for(:push_event_hooks_limit) } + + it { is_expected.to allow_value(3).for(:push_event_activities_limit) } + it { is_expected.not_to allow_value('three').for(:push_event_activities_limit) } + it { is_expected.not_to allow_value(nil).for(:push_event_activities_limit) } context "when user accepted let's encrypt terms of service" do before do diff --git a/spec/models/aws/role_spec.rb b/spec/models/aws/role_spec.rb new file mode 100644 index 00000000000..c40752e40a6 --- /dev/null +++ b/spec/models/aws/role_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Aws::Role do + it { is_expected.to belong_to(:user) } + it { is_expected.to validate_length_of(:role_external_id).is_at_least(1).is_at_most(64) } + + describe 'custom validations' do + subject { role.valid? } + + context ':role_arn' do + let(:role) { build(:aws_role, role_arn: role_arn) } + + context 'length is zero' do + let(:role_arn) { '' } + + it { is_expected.to be_falsey } + end + + context 'length is longer than 2048' do + let(:role_arn) { '1' * 2049 } + + it { is_expected.to be_falsey } + end + + context 'ARN is valid' do + let(:role_arn) { 'arn:aws:iam::123456789012:role/test-role' } + + it { is_expected.to be_truthy } + end + end + end +end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 2efab3076d8..9e55fbcce20 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -320,6 +320,22 @@ describe Blob do expect(blob.rich_viewer).to be_a(BlobViewer::Markup) end end + + context 'when the blob is video' do + it 'returns a video viewer' do + blob = fake_blob(path: 'file.mp4', binary: true) + + expect(blob.rich_viewer).to be_a(BlobViewer::Video) + end + end + + context 'when the blob is audio' do + it 'returns an audio viewer' do + blob = fake_blob(path: 'file.wav', binary: true) + + expect(blob.rich_viewer).to be_a(BlobViewer::Audio) + end + end end describe '#auxiliary_viewer' do diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 67cd939b4c6..da95a2d30f5 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe Ci::BuildMetadata do set(:user) { create(:user) } - set(:group) { create(:group, :access_requestable) } + set(:group) { create(:group) } set(:project) { create(:project, :repository, group: group, build_timeout: 2000) } set(:pipeline) do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5c0f8bd392a..058305bc04e 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe Ci::Build do set(:user) { create(:user) } - set(:group) { create(:group, :access_requestable) } + set(:group) { create(:group) } set(:project) { create(:project, :repository, group: group) } set(:pipeline) do @@ -19,17 +19,24 @@ describe Ci::Build do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } + it { is_expected.to have_many(:trace_sections) } it { is_expected.to have_many(:needs) } + it { is_expected.to have_many(:sourced_pipelines) } + it { is_expected.to have_many(:job_variables) } + it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session) } - it { is_expected.to have_many(:job_variables) } + it { is_expected.to validate_presence_of(:ref) } + it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + it { is_expected.to delegate_method(:merge_request_event?).to(:pipeline) } it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } it { is_expected.to delegate_method(:legacy_detached_merge_request_pipeline?).to(:pipeline) } + it { is_expected.to include_module(Ci::PipelineDelegator) } describe 'associations' do @@ -567,6 +574,7 @@ describe Ci::Build do describe '#artifacts_metadata?' do subject { build.artifacts_metadata? } + context 'artifacts metadata does not exist' do it { is_expected.to be_falsy } end @@ -579,6 +587,7 @@ describe Ci::Build do describe '#artifacts_expire_in' do subject { build.artifacts_expire_in } + it { is_expected.to be_nil } context 'when artifacts_expire_at is specified' do @@ -957,7 +966,7 @@ describe Ci::Build do end describe 'state transition as a deployable' do - let!(:build) { create(:ci_build, :start_review_app) } + let!(:build) { create(:ci_build, :with_deployment, :start_review_app) } let(:deployment) { build.deployment } let(:environment) { deployment.environment } @@ -1043,20 +1052,6 @@ describe Ci::Build do end describe 'deployment' do - describe '#has_deployment?' do - subject { build.has_deployment? } - - context 'when build has a deployment' do - let!(:deployment) { create(:deployment, deployable: build) } - - it { is_expected.to be_truthy } - end - - context 'when build does not have a deployment' do - it { is_expected.to be_falsy } - end - end - describe '#outdated_deployment?' do subject { build.outdated_deployment? } @@ -1272,6 +1267,7 @@ describe Ci::Build do describe '#erasable?' do subject { build.erasable? } + it { is_expected.to be_truthy } end @@ -1955,7 +1951,7 @@ describe Ci::Build do end context 'when build has a start environment' do - let(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) } + let(:build) { create(:ci_build, :with_deployment, :deploy_to_production, pipeline: pipeline) } it 'does not expand environment name' do expect(build).not_to receive(:expanded_environment_name) @@ -2202,6 +2198,7 @@ describe Ci::Build do { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true, masked: false }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true, masked: false }, { key: 'CI_NODE_TOTAL', value: '1', public: true, masked: false }, + { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, { key: 'CI_BUILD_REF', value: build.sha, public: true, masked: false }, { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true, masked: false }, { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true, masked: false }, @@ -2210,6 +2207,7 @@ describe Ci::Build do { key: 'CI_BUILD_STAGE', value: 'test', public: true, masked: false }, { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true, masked: false }, { key: 'CI_PROJECT_NAME', value: project.path, public: true, masked: false }, + { key: 'CI_PROJECT_TITLE', value: project.title, public: true, masked: false }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true, masked: false }, { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true, masked: false }, { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true, masked: false }, @@ -3079,6 +3077,12 @@ describe Ci::Build do rescue StateMachines::InvalidTransition end + it 'ensures pipeline ref existence' do + expect(job.pipeline.persistent_ref).to receive(:create).once + + run_job_without_exception + end + shared_examples 'saves data on transition' do it 'saves timeout' do expect { job.run! }.to change { job.reload.ensure_metadata.timeout }.from(nil).to(expected_timeout) @@ -3918,4 +3922,14 @@ describe Ci::Build do end end end + + describe '#invalid_dependencies' do + let!(:pre_stage_job_valid) { create(:ci_build, :manual, pipeline: pipeline, name: 'test1', stage_idx: 0) } + let!(:pre_stage_job_invalid) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test2', stage_idx: 1) } + let!(:job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 2, options: { dependencies: %w(test1 test2) }) } + + it 'returns invalid dependencies' do + expect(job.invalid_dependencies).to eq([pre_stage_job_invalid]) + end + end end diff --git a/spec/models/ci/build_trace_spec.rb b/spec/models/ci/build_trace_spec.rb new file mode 100644 index 00000000000..2471a6fa827 --- /dev/null +++ b/spec/models/ci/build_trace_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::BuildTrace do + let(:build) { build_stubbed(:ci_build) } + let(:state) { nil } + let(:data) { StringIO.new('the-stream') } + + let(:stream) do + Gitlab::Ci::Trace::Stream.new { data } + end + + subject { described_class.new(build: build, stream: stream, state: state, content_format: content_format) } + + shared_examples 'delegates methods' do + it { is_expected.to delegate_method(:state).to(:trace) } + it { is_expected.to delegate_method(:append).to(:trace) } + it { is_expected.to delegate_method(:truncated).to(:trace) } + it { is_expected.to delegate_method(:offset).to(:trace) } + it { is_expected.to delegate_method(:size).to(:trace) } + it { is_expected.to delegate_method(:total).to(:trace) } + it { is_expected.to delegate_method(:id).to(:build).with_prefix } + it { is_expected.to delegate_method(:status).to(:build).with_prefix } + it { is_expected.to delegate_method(:complete?).to(:build).with_prefix } + end + + context 'with :json content format' do + let(:content_format) { :json } + + it_behaves_like 'delegates methods' + + it { is_expected.to be_json } + + it 'returns formatted trace' do + expect(subject.trace.lines).to eq([ + { offset: 0, content: [{ text: 'the-stream' }] } + ]) + end + end + + context 'with :html content format' do + let(:content_format) { :html } + + it_behaves_like 'delegates methods' + + it { is_expected.to be_html } + + it 'returns formatted trace' do + expect(subject.trace.html).to eq('<span>the-stream</span>') + end + end +end diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb index 36c65d92840..b3b158a111e 100644 --- a/spec/models/ci/group_spec.rb +++ b/spec/models/ci/group_spec.rb @@ -22,6 +22,32 @@ describe Ci::Group do end end + describe '#status' do + let(:jobs) do + [create(:ci_build, :failed)] + end + + context 'when ci_composite_status is enabled' do + before do + stub_feature_flags(ci_composite_status: true) + end + + it 'returns a failed status' do + expect(subject.status).to eq('failed') + end + end + + context 'when ci_composite_status is disabled' do + before do + stub_feature_flags(ci_composite_status: false) + end + + it 'returns a failed status' do + expect(subject.status).to eq('failed') + end + end + end + describe '#detailed_status' do context 'when there is only one item in the group' do it 'calls the status from the object itself' do diff --git a/spec/models/ci/legacy_stage_spec.rb b/spec/models/ci/legacy_stage_spec.rb index bb812cc0533..477f4036218 100644 --- a/spec/models/ci/legacy_stage_spec.rb +++ b/spec/models/ci/legacy_stage_spec.rb @@ -216,7 +216,7 @@ describe Ci::LegacyStage do context 'when stage has warnings' do context 'when using memoized warnings flag' do context 'when there are warnings' do - let(:stage) { build(:ci_stage, warnings: 2) } + let(:stage) { build(:ci_stage, warnings: true) } it 'returns true using memoized value' do expect(stage).not_to receive(:statuses) @@ -225,22 +225,13 @@ describe Ci::LegacyStage do end context 'when there are no warnings' do - let(:stage) { build(:ci_stage, warnings: 0) } + let(:stage) { build(:ci_stage, warnings: false) } it 'returns false using memoized value' do expect(stage).not_to receive(:statuses) expect(stage).not_to have_warnings end end - - context 'when number of warnings is not a valid value' do - let(:stage) { build(:ci_stage, warnings: true) } - - it 'calculates statuses using database queries' do - expect(stage).to receive(:statuses).and_call_original - expect(stage).not_to have_warnings - end - end end context 'when calculating warnings from statuses' do diff --git a/spec/models/ci/persistent_ref_spec.rb b/spec/models/ci/persistent_ref_spec.rb new file mode 100644 index 00000000000..be447476e2c --- /dev/null +++ b/spec/models/ci/persistent_ref_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::PersistentRef do + it 'cleans up persistent refs after pipeline finished' do + pipeline = create(:ci_pipeline, :running) + + expect(pipeline.persistent_ref).to receive(:delete).once + + pipeline.succeed! + end + + context '#exist?' do + subject { pipeline.persistent_ref.exist? } + + let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let(:project) { create(:project, :repository) } + let(:sha) { project.repository.commit.sha } + + context 'when a persistent ref does not exist' do + it { is_expected.to eq(false) } + end + + context 'when a persistent ref exists' do + before do + pipeline.persistent_ref.create + end + + it { is_expected.to eq(true) } + end + end + + context '#create' do + subject { pipeline.persistent_ref.create } + + let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let(:project) { create(:project, :repository) } + let(:sha) { project.repository.commit.sha } + + context 'when a persistent ref does not exist' do + it 'creates a persistent ref' do + subject + + expect(pipeline.persistent_ref).to be_exist + end + + context 'when sha does not exist in the repository' do + let(:sha) { 'not-exist' } + + it 'fails to create a persistent ref' do + subject + + expect(pipeline.persistent_ref).not_to be_exist + end + end + end + + context 'when a persistent ref already exists' do + before do + pipeline.persistent_ref.create + end + + it 'does not create a persistent ref' do + expect(project.repository).not_to receive(:create_ref) + + subject + end + end + end + + context '#delete' do + subject { pipeline.persistent_ref.delete } + + let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let(:project) { create(:project, :repository) } + let(:sha) { project.repository.commit.sha } + + context 'when a persistent ref exists' do + before do + pipeline.persistent_ref.create + end + + it 'deletes the ref' do + expect { subject }.to change { pipeline.persistent_ref.exist? } + .from(true).to(false) + end + end + + context 'when a persistent ref does not exist' do + it 'does not raise an error' do + expect { subject }.not_to raise_error + end + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d5ad70194cb..de0ce9932e8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -28,7 +28,13 @@ describe Ci::Pipeline, :mailer do it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:auto_canceled_pipelines) } it { is_expected.to have_many(:auto_canceled_jobs) } + it { is_expected.to have_many(:sourced_pipelines) } + it { is_expected.to have_many(:triggered_pipelines) } + it { is_expected.to have_one(:chat_data) } + it { is_expected.to have_one(:source_pipeline) } + it { is_expected.to have_one(:triggered_by_pipeline) } + it { is_expected.to have_one(:source_job) } it { is_expected.to validate_presence_of(:sha) } it { is_expected.to validate_presence_of(:status) } @@ -1136,59 +1142,71 @@ describe Ci::Pipeline, :mailer do end describe '#legacy_stages' do + using RSpec::Parameterized::TableSyntax + subject { pipeline.legacy_stages } - context 'stages list' do - it 'returns ordered list of stages' do - expect(subject.map(&:name)).to eq(%w[build test deploy]) - end + where(:ci_composite_status) do + [false, true] end - context 'stages with statuses' do - let(:statuses) do - subject.map { |stage| [stage.name, stage.status] } + with_them do + before do + stub_feature_flags(ci_composite_status: ci_composite_status) end - it 'returns list of stages with correct statuses' do - expect(statuses).to eq([%w(build failed), - %w(test success), - %w(deploy running)]) + context 'stages list' do + it 'returns ordered list of stages' do + expect(subject.map(&:name)).to eq(%w[build test deploy]) + end end - context 'when commit status is retried' do - before do - create(:commit_status, pipeline: pipeline, - stage: 'build', - name: 'mac', - stage_idx: 0, - status: 'success') - - pipeline.process! + context 'stages with statuses' do + let(:statuses) do + subject.map { |stage| [stage.name, stage.status] } end - it 'ignores the previous state' do - expect(statuses).to eq([%w(build success), + it 'returns list of stages with correct statuses' do + expect(statuses).to eq([%w(build failed), %w(test success), %w(deploy running)]) end - end - end - context 'when there is a stage with warnings' do - before do - create(:commit_status, pipeline: pipeline, - stage: 'deploy', - name: 'prod:2', - stage_idx: 2, - status: 'failed', - allow_failure: true) + context 'when commit status is retried' do + before do + create(:commit_status, pipeline: pipeline, + stage: 'build', + name: 'mac', + stage_idx: 0, + status: 'success') + + pipeline.process! + end + + it 'ignores the previous state' do + expect(statuses).to eq([%w(build success), + %w(test success), + %w(deploy running)]) + end + end end - it 'populates stage with correct number of warnings' do - deploy_stage = pipeline.legacy_stages.third + context 'when there is a stage with warnings' do + before do + create(:commit_status, pipeline: pipeline, + stage: 'deploy', + name: 'prod:2', + stage_idx: 2, + status: 'failed', + allow_failure: true) + end - expect(deploy_stage).not_to receive(:statuses) - expect(deploy_stage).to have_warnings + it 'populates stage with correct number of warnings' do + deploy_stage = pipeline.legacy_stages.third + + expect(deploy_stage).not_to receive(:statuses) + expect(deploy_stage).to have_warnings + end end end end @@ -1318,6 +1336,16 @@ describe Ci::Pipeline, :mailer do let(:build_b) { create_build('build2', queued_at: 0) } let(:build_c) { create_build('build3', queued_at: 0) } + %w[succeed! drop! cancel! skip!].each do |action| + context "when the pipeline recieved #{action} event" do + it 'deletes a persistent ref' do + expect(pipeline.persistent_ref).to receive(:delete).once + + pipeline.public_send(action) + end + end + end + describe '#duration' do context 'when multiple builds are finished' do before do @@ -1755,6 +1783,30 @@ describe Ci::Pipeline, :mailer do end end + describe '#all_worktree_paths' do + let(:files) { { 'main.go' => '', 'mocks/mocks.go' => '' } } + let(:project) { create(:project, :custom_repo, files: files) } + let(:pipeline) { build(:ci_pipeline, project: project, sha: project.repository.head_commit.sha) } + + it 'returns all file paths cached' do + expect(project.repository).to receive(:ls_files).with(pipeline.sha).once.and_call_original + expect(pipeline.all_worktree_paths).to eq(files.keys) + expect(pipeline.all_worktree_paths).to eq(files.keys) + end + end + + describe '#top_level_worktree_paths' do + let(:files) { { 'main.go' => '', 'mocks/mocks.go' => '' } } + let(:project) { create(:project, :custom_repo, files: files) } + let(:pipeline) { build(:ci_pipeline, project: project, sha: project.repository.head_commit.sha) } + + it 'returns top-level file paths cached' do + expect(project.repository).to receive(:tree).with(pipeline.sha).once.and_call_original + expect(pipeline.top_level_worktree_paths).to eq(['main.go']) + expect(pipeline.top_level_worktree_paths).to eq(['main.go']) + end + end + describe '#has_kubernetes_active?' do context 'when kubernetes is active' do context 'when user configured kubernetes from CI/CD > Clusters' do @@ -1932,40 +1984,57 @@ describe Ci::Pipeline, :mailer do end end - describe '.latest_status_per_commit' do + describe '.latest_pipeline_per_commit' do let(:project) { create(:project) } - before do - pairs = [ - %w[success ref1 123], - %w[manual master 123], - %w[failed ref 456] - ] - - pairs.each do |(status, ref, sha)| - create( - :ci_empty_pipeline, - status: status, - ref: ref, - sha: sha, - project: project - ) - end + let!(:commit_123_ref_master) do + create( + :ci_empty_pipeline, + status: 'success', + ref: 'master', + sha: '123', + project: project + ) + end + let!(:commit_123_ref_develop) do + create( + :ci_empty_pipeline, + status: 'success', + ref: 'develop', + sha: '123', + project: project + ) + end + let!(:commit_456_ref_test) do + create( + :ci_empty_pipeline, + status: 'success', + ref: 'test', + sha: '456', + project: project + ) end context 'without a ref' do - it 'returns a Hash containing the latest status per commit for all refs' do - expect(described_class.latest_status_per_commit(%w[123 456])) - .to eq({ '123' => 'manual', '456' => 'failed' }) + it 'returns a Hash containing the latest pipeline per commit for all refs' do + result = described_class.latest_pipeline_per_commit(%w[123 456]) + + expect(result).to match( + '123' => commit_123_ref_develop, + '456' => commit_456_ref_test + ) end - it 'only includes the status of the given commit SHAs' do - expect(described_class.latest_status_per_commit(%w[123])) - .to eq({ '123' => 'manual' }) + it 'only includes the latest pipeline of the given commit SHAs' do + result = described_class.latest_pipeline_per_commit(%w[123]) + + expect(result).to match( + '123' => commit_123_ref_develop + ) end context 'when there are two pipelines for a ref and SHA' do - it 'returns the status of the latest pipeline' do + let!(:commit_123_ref_master_latest) do create( :ci_empty_pipeline, status: 'failed', @@ -1973,17 +2042,25 @@ describe Ci::Pipeline, :mailer do sha: '123', project: project ) + end + + it 'returns the latest pipeline' do + result = described_class.latest_pipeline_per_commit(%w[123]) - expect(described_class.latest_status_per_commit(%w[123])) - .to eq({ '123' => 'failed' }) + expect(result).to match( + '123' => commit_123_ref_master_latest + ) end end end context 'with a ref' do it 'only includes the pipelines for the given ref' do - expect(described_class.latest_status_per_commit(%w[123 456], 'master')) - .to eq({ '123' => 'manual' }) + result = described_class.latest_pipeline_per_commit(%w[123 456], 'master') + + expect(result).to match( + '123' => commit_123_ref_master + ) end end end @@ -2267,36 +2344,38 @@ describe Ci::Pipeline, :mailer do describe '#update_status' do context 'when pipeline is empty' do it 'updates does not change pipeline status' do - expect(pipeline.statuses.latest.status).to be_nil + expect(pipeline.statuses.latest.slow_composite_status).to be_nil expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'skipped' + .to change { pipeline.reload.status } + .from('created') + .to('skipped') end end context 'when updating status to pending' do before do - allow(pipeline) - .to receive_message_chain(:statuses, :latest, :status) - .and_return(:running) + create(:ci_build, pipeline: pipeline, status: :running) end it 'updates pipeline status to running' do expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'running' + .to change { pipeline.reload.status } + .from('created') + .to('running') end end context 'when updating status to scheduled' do before do - allow(pipeline) - .to receive_message_chain(:statuses, :latest, :status) - .and_return(:scheduled) + create(:ci_build, pipeline: pipeline, status: :scheduled) end it 'updates pipeline status to scheduled' do expect { pipeline.update_status } - .to change { pipeline.reload.status }.to 'scheduled' + .to change { pipeline.reload.status } + .from('created') + .to('scheduled') end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 70ff3cf5dc4..ac438f7d473 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -686,11 +686,13 @@ describe Ci::Runner do describe '#has_tags?' do context 'when runner has tags' do subject { create(:ci_runner, tag_list: ['tag']) } + it { is_expected.to have_tags } end context 'when runner does not have tags' do subject { create(:ci_runner, tag_list: []) } + it { is_expected.not_to have_tags } end end diff --git a/spec/models/ci/sources/pipeline_spec.rb b/spec/models/ci/sources/pipeline_spec.rb new file mode 100644 index 00000000000..63bee5bfb55 --- /dev/null +++ b/spec/models/ci/sources/pipeline_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::Sources::Pipeline do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:pipeline) } + + it { is_expected.to belong_to(:source_project) } + it { is_expected.to belong_to(:source_job) } + it { is_expected.to belong_to(:source_pipeline) } + + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:pipeline) } + + it { is_expected.to validate_presence_of(:source_project) } + it { is_expected.to validate_presence_of(:source_job) } + it { is_expected.to validate_presence_of(:source_pipeline) } +end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 85cd32fb03a..8827509edda 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -130,7 +130,7 @@ describe Ci::Stage, :models do context 'when statuses status was not recognized' do before do allow(stage) - .to receive_message_chain(:statuses, :latest, :status) + .to receive(:latest_stage_status) .and_return(:unknown) end diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index bddc09decc3..c1933c578bc 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -54,7 +54,7 @@ describe Clusters::Applications::CertManager do 'kubectl label --overwrite namespace gitlab-managed-apps certmanager.k8s.io/disable-validation=true' ]) expect(subject.postinstall).to eq([ - 'for i in $(seq 1 30); do kubectl apply -f /data/helm/certmanager/config/cluster_issuer.yaml && break; sleep 1s; echo "Retrying ($i)..."; done' + "for i in $(seq 1 30); do kubectl apply -f /data/helm/certmanager/config/cluster_issuer.yaml && s=0 && break || s=$?; sleep 1s; echo \"Retrying ($i)...\"; done; (exit $s)" ]) end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 9672129bb1e..64f58155a66 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -134,4 +134,14 @@ describe Clusters::Applications::Helm do end end end + + describe '#post_uninstall' do + let(:helm) { create(:clusters_applications_helm, :installed) } + + it do + expect(helm.cluster.kubeclient).to receive(:delete_namespace).with('gitlab-managed-apps') + + helm.post_uninstall + end + end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index d9461ee8581..be0c6df7ad6 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -96,7 +96,7 @@ describe Clusters::Applications::Ingress do it 'is initialized with ingress arguments' do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') - expect(subject.version).to eq('1.1.2') + expect(subject.version).to eq('1.22.1') expect(subject).to be_rbac expect(subject.files).to eq(ingress.files) end @@ -113,7 +113,7 @@ describe Clusters::Applications::Ingress do let(:ingress) { create(:clusters_applications_ingress, :errored, version: 'nginx') } it 'is initialized with the locked version' do - expect(subject.version).to eq('1.1.2') + expect(subject.version).to eq('1.22.1') end end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index e1eee014567..0ec9333d6a7 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -91,7 +91,6 @@ describe Clusters::Applications::Jupyter do it 'includes valid values' do expect(values).to include('ingress') expect(values).to include('hub') - expect(values).to include('rbac') expect(values).to include('proxy') expect(values).to include('auth') expect(values).to include('singleuser') @@ -111,7 +110,6 @@ describe Clusters::Applications::Jupyter do it 'includes valid values' do expect(values).to include('ingress') expect(values).to include('hub') - expect(values).to include('rbac') expect(values).to include('proxy') expect(values).to include('auth') expect(values).to include('singleuser') diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 3825994b733..51c8a6bb68d 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -16,6 +16,13 @@ describe Clusters::Applications::Knative do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) end + describe 'when cloud run is enabled' do + let(:cluster) { create(:cluster, :provided_by_gcp, :cloud_run_enabled) } + let(:knative_cloud_run) { create(:clusters_applications_knative, cluster: cluster) } + + it { expect(knative_cloud_run).to be_not_installable } + end + describe 'when rbac is not enabled' do let(:cluster) { create(:cluster, :provided_by_gcp, :rbac_disabled) } let(:knative_no_rbac) { create(:clusters_applications_knative, cluster: cluster) } @@ -112,7 +119,7 @@ describe Clusters::Applications::Knative do subject { knative.install_command } it 'is initialized with latest version' do - expect(subject.version).to eq('0.6.0') + expect(subject.version).to eq('0.7.0') end it_behaves_like 'a command' diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 8fc3b7e4c40..2aeb7e5a990 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -175,6 +175,18 @@ describe Clusters::Applications::Prometheus do expect(subject).to be_rbac end + describe '#predelete' do + let(:knative) { create(:clusters_applications_knative, :updated ) } + let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } + + subject { prometheus.uninstall_command.predelete } + + it 'deletes knative metrics' do + metrics_config = Clusters::Applications::Knative::METRICS_CONFIG + is_expected.to include("kubectl delete -f #{metrics_config} --ignore-not-found") + end + end + context 'on a non rbac enabled cluster' do before do prometheus.cluster.platform_kubernetes.abac! diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 9afbe6328ca..48e3b4d6bae 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -11,11 +11,13 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do subject { build(:cluster) } it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:management_project).class_name('::Project') } it { is_expected.to have_many(:cluster_projects) } it { is_expected.to have_many(:projects) } it { is_expected.to have_many(:cluster_groups) } it { is_expected.to have_many(:groups) } it { is_expected.to have_one(:provider_gcp) } + it { is_expected.to have_one(:provider_aws) } it { is_expected.to have_one(:platform_kubernetes) } it { is_expected.to have_one(:application_helm) } it { is_expected.to have_one(:application_ingress) } @@ -38,6 +40,15 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to respond_to :project } + describe 'applications have inverse_of: :cluster option' do + let(:cluster) { create(:cluster) } + let!(:helm) { create(:clusters_applications_helm, cluster: cluster) } + + it 'does not do a third query when referencing cluster again' do + expect { cluster.application_helm.cluster }.not_to exceed_query_limit(2) + end + end + describe '.enabled' do subject { described_class.enabled } @@ -98,6 +109,31 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to contain_exactly(cluster) } end + describe '.aws_provided' do + subject { described_class.aws_provided } + + let!(:cluster) { create(:cluster, :provided_by_aws) } + + before do + create(:cluster, :provided_by_user) + end + + it { is_expected.to contain_exactly(cluster) } + end + + describe '.aws_installed' do + subject { described_class.aws_installed } + + let!(:cluster) { create(:cluster, :provided_by_aws) } + + before do + errored_cluster = create(:cluster, :provided_by_aws) + errored_cluster.provider.make_errored!("Error message") + end + + it { is_expected.to contain_exactly(cluster) } + end + describe '.managed' do subject do described_class.managed @@ -280,6 +316,20 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to be_valid } end end + + describe 'unique scope for management_project' do + let(:project) { create(:project) } + let!(:cluster_with_management_project) { create(:cluster, management_project: project) } + + context 'duplicate scopes for the same management project' do + let(:cluster) { build(:cluster, management_project: project) } + + it 'adds an error on environment_scope' do + expect(cluster).not_to be_valid + expect(cluster.errors[:environment_scope].first).to eq('cannot add duplicated environment scope') + end + end + end end describe '.ancestor_clusters_for_clusterable' do @@ -374,7 +424,14 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it 'returns a provider' do is_expected.to eq(cluster.provider_gcp) - expect(subject.class.name.deconstantize).to eq(Clusters::Providers.to_s) + end + end + + context 'when provider is aws' do + let(:cluster) { create(:cluster, :provided_by_aws) } + + it 'returns a provider' do + is_expected.to eq(cluster.provider_aws) end end @@ -537,7 +594,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do before do expect(Clusters::KubernetesNamespaceFinder).to receive(:new) - .with(cluster, project: environment.project, environment_slug: environment.slug) + .with(cluster, project: environment.project, environment_name: environment.name) .and_return(double(execute: persisted_namespace)) end @@ -747,4 +804,26 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end end + + describe '#knative_pre_installed?' do + subject { cluster.knative_pre_installed? } + + context 'with a GCP provider without cloud_run' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + + it { is_expected.to be_falsey } + end + + context 'with a GCP provider with cloud_run' do + let(:cluster) { create(:cluster, :provided_by_gcp, :cloud_run_enabled) } + + it { is_expected.to be_truthy } + end + + context 'with a user provider' do + let(:cluster) { create(:cluster, :provided_by_user) } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/models/clusters/clusters_hierarchy_spec.rb b/spec/models/clusters/clusters_hierarchy_spec.rb index 0470ebe17ea..fc35b8257e9 100644 --- a/spec/models/clusters/clusters_hierarchy_spec.rb +++ b/spec/models/clusters/clusters_hierarchy_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' describe Clusters::ClustersHierarchy do describe '#base_and_ancestors' do - def base_and_ancestors(clusterable) - described_class.new(clusterable).base_and_ancestors + def base_and_ancestors(clusterable, include_management_project: true) + described_class.new(clusterable, include_management_project: include_management_project).base_and_ancestors end context 'project in nested group with clusters at every level' do @@ -44,14 +44,44 @@ describe Clusters::ClustersHierarchy do end end + context 'cluster has management project' do + let!(:project_cluster) { create(:cluster, :project, projects: [project]) } + let!(:group_cluster) { create(:cluster, :group, groups: [group], management_project: management_project) } + + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:management_project) { create(:project) } + + it 'returns clusters for management_project' do + expect(base_and_ancestors(management_project)).to eq([group_cluster]) + end + + it 'returns nothing if include_management_project is false' do + expect(base_and_ancestors(management_project, include_management_project: false)).to be_empty + end + + it 'returns clusters for project' do + expect(base_and_ancestors(project)).to eq([project_cluster, group_cluster]) + end + + it 'returns clusters for group' do + expect(base_and_ancestors(group)).to eq([group_cluster]) + end + end + context 'project in nested group with clusters at some levels' do - let!(:child) { create(:cluster, :group, groups: [child_group]) } + let!(:child) { create(:cluster, :group, groups: [child_group], management_project: management_project) } let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) } let(:ancestor_group) { create(:group) } let(:parent_group) { create(:group, parent: ancestor_group) } let(:child_group) { create(:group, parent: parent_group) } let(:project) { create(:project, group: child_group) } + let(:management_project) { create(:project) } + + it 'returns clusters for management_project' do + expect(base_and_ancestors(management_project)).to eq([child]) + end it 'returns clusters for project' do expect(base_and_ancestors(project)).to eq([child, ancestor]) diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index d4e3a0ac84d..2920bbf2b58 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -24,13 +24,13 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do end end - describe '.with_environment_slug' do + describe '.with_environment_name' do let(:cluster) { create(:cluster, :group) } - let(:environment) { create(:environment, slug: slug) } + let(:environment) { create(:environment, name: name) } - let(:slug) { 'production' } + let(:name) { 'production' } - subject { described_class.with_environment_slug(slug) } + subject { described_class.with_environment_name(name) } context 'there is no associated environment' do let!(:namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: environment.project) } @@ -48,12 +48,12 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do ) end - context 'with a matching slug' do + context 'with a matching name' do it { is_expected.to eq [namespace] } end - context 'without a matching slug' do - let(:environment) { create(:environment, slug: 'staging') } + context 'without a matching name' do + let(:environment) { create(:environment, name: 'staging') } it { is_expected.to be_empty } end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 0c4cf291d20..d53fc32cfef 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -19,14 +19,23 @@ describe Clusters::Platforms::Kubernetes do it_behaves_like 'having unique enum values' describe 'before_validation' do + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } + context 'when namespace includes upper case' do - let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } let(:namespace) { 'ABC' } it 'converts to lower case' do expect(kubernetes.namespace).to eq('abc') end end + + context 'when namespace is blank' do + let(:namespace) { '' } + + it 'nullifies the namespace' do + expect(kubernetes.namespace).to be_nil + end + end end describe 'validation' do @@ -35,8 +44,8 @@ describe Clusters::Platforms::Kubernetes do context 'when validates namespace' do let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: namespace) } - context 'when namespace is blank' do - let(:namespace) { '' } + context 'when namespace is nil' do + let(:namespace) { nil } it { is_expected.to be_truthy } end @@ -218,7 +227,7 @@ describe Clusters::Platforms::Kubernetes do before do allow(Clusters::KubernetesNamespaceFinder).to receive(:new) - .with(cluster, project: project, environment_slug: environment_slug) + .with(cluster, project: project, environment_name: environment_name) .and_return(double(execute: persisted_namespace)) end @@ -232,6 +241,23 @@ describe Clusters::Platforms::Kubernetes do it { is_expected.to include(key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true) } end + context 'cluster is managed by project' do + before do + allow(Gitlab::Kubernetes::DefaultNamespace).to receive(:new) + .with(cluster, project: project).and_return(double(from_environment_name: namespace)) + + allow(platform).to receive(:kubeconfig).with(namespace).and_return('kubeconfig') + end + + let(:cluster) { create(:cluster, :group, platform_kubernetes: platform, management_project: project) } + let(:namespace) { 'kubernetes-namespace' } + let(:kubeconfig) { 'kubeconfig' } + + it { is_expected.to include(key: 'KUBE_TOKEN', value: platform.token, public: false, masked: true) } + it { is_expected.to include(key: 'KUBE_NAMESPACE', value: namespace) } + it { is_expected.to include(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) } + end + context 'kubernetes namespace exists' do let(:variable) { Hash(key: :fake_key, value: 'fake_value') } let(:namespace_variables) { Gitlab::Ci::Variables::Collection.new([variable]) } diff --git a/spec/models/clusters/providers/aws_spec.rb b/spec/models/clusters/providers/aws_spec.rb new file mode 100644 index 00000000000..ec8159a7ee0 --- /dev/null +++ b/spec/models/clusters/providers/aws_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::Providers::Aws do + it { is_expected.to belong_to(:cluster) } + it { is_expected.to belong_to(:created_by_user) } + + it { is_expected.to validate_length_of(:key_name).is_at_least(1).is_at_most(255) } + it { is_expected.to validate_length_of(:region).is_at_least(1).is_at_most(255) } + it { is_expected.to validate_length_of(:instance_type).is_at_least(1).is_at_most(255) } + it { is_expected.to validate_length_of(:security_group_id).is_at_least(1).is_at_most(255) } + it { is_expected.to validate_presence_of(:subnet_ids) } + + include_examples 'provider status', :cluster_provider_aws + + describe 'default_value_for' do + let(:provider) { build(:cluster_provider_aws) } + + it "sets default values" do + expect(provider.region).to eq('us-east-1') + expect(provider.num_nodes).to eq(3) + expect(provider.instance_type).to eq('m5.large') + end + end + + describe 'custom validations' do + subject { provider.valid? } + + context ':num_nodes' do + let(:provider) { build(:cluster_provider_aws, num_nodes: num_nodes) } + + context 'contains non-digit characters' do + let(:num_nodes) { 'A3' } + + it { is_expected.to be_falsey } + end + + context 'is blank' do + let(:num_nodes) { nil } + + it { is_expected.to be_falsey } + end + + context 'is less than 1' do + let(:num_nodes) { 0 } + + it { is_expected.to be_falsey } + end + + context 'is a positive integer' do + let(:num_nodes) { 3 } + + it { is_expected.to be_truthy } + end + end + end + + describe '#nullify_credentials' do + let(:provider) { create(:cluster_provider_aws, :scheduled) } + + subject { provider.nullify_credentials } + + before do + expect(provider.access_key_id).to be_present + expect(provider.secret_access_key).to be_present + end + + it 'removes access_key_id and secret_access_key' do + subject + + expect(provider.access_key_id).to be_nil + expect(provider.secret_access_key).to be_nil + end + end +end diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index 785db4febe0..15e152519b4 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -6,6 +6,8 @@ describe Clusters::Providers::Gcp do it { is_expected.to belong_to(:cluster) } it { is_expected.to validate_presence_of(:zone) } + include_examples 'provider status', :cluster_provider_gcp + describe 'default_value_for' do let(:gcp) { build(:cluster_provider_gcp) } @@ -84,86 +86,20 @@ describe Clusters::Providers::Gcp do it { is_expected.not_to be_legacy_abac } end - describe '#state_machine' do - context 'when any => [:created]' do - let(:gcp) { build(:cluster_provider_gcp, :creating) } - - before do - gcp.make_created - end - - it 'nullify access_token and operation_id' do - expect(gcp.access_token).to be_nil - expect(gcp.operation_id).to be_nil - expect(gcp).to be_created - end - end - - context 'when any => [:creating]' do - let(:gcp) { build(:cluster_provider_gcp) } - - context 'when operation_id is present' do - let(:operation_id) { 'operation-xxx' } - - before do - gcp.make_creating(operation_id) - end - - it 'sets operation_id' do - expect(gcp.operation_id).to eq(operation_id) - expect(gcp).to be_creating - end - end - - context 'when operation_id is nil' do - let(:operation_id) { nil } - - it 'raises an error' do - expect { gcp.make_creating(operation_id) } - .to raise_error('operation_id is required') - end - end - end - - context 'when any => [:errored]' do - let(:gcp) { build(:cluster_provider_gcp, :creating) } - let(:status_reason) { 'err msg' } - - it 'nullify access_token and operation_id' do - gcp.make_errored(status_reason) - - expect(gcp.access_token).to be_nil - expect(gcp.operation_id).to be_nil - expect(gcp.status_reason).to eq(status_reason) - expect(gcp).to be_errored - end - - context 'when status_reason is nil' do - let(:gcp) { build(:cluster_provider_gcp, :errored) } + describe '#knative_pre_installed?' do + subject { gcp.knative_pre_installed? } - it 'does not set status_reason' do - gcp.make_errored(nil) + context 'when cluster is cloud_run' do + let(:gcp) { create(:cluster_provider_gcp) } - expect(gcp.status_reason).not_to be_nil - end - end + it { is_expected.to be_falsey } end - end - describe '#on_creation?' do - subject { gcp.on_creation? } - - context 'when status is creating' do - let(:gcp) { create(:cluster_provider_gcp, :creating) } + context 'when cluster is not cloud_run' do + let(:gcp) { create(:cluster_provider_gcp, :cloud_run_enabled) } it { is_expected.to be_truthy } end - - context 'when status is created' do - let(:gcp) { create(:cluster_provider_gcp, :created) } - - it { is_expected.to be_falsey } - end end describe '#api_client' do @@ -190,4 +126,31 @@ describe Clusters::Providers::Gcp do it { is_expected.to be_nil } end end + + describe '#nullify_credentials' do + let(:provider) { create(:cluster_provider_gcp, :creating) } + + before do + expect(provider.access_token).to be_present + expect(provider.operation_id).to be_present + end + + it 'removes access_token and operation_id' do + provider.nullify_credentials + + expect(provider.access_token).to be_nil + expect(provider.operation_id).to be_nil + end + end + + describe '#assign_operation_id' do + let(:provider) { create(:cluster_provider_gcp, :scheduled) } + let(:operation_id) { 'operation-123' } + + it 'sets operation_id' do + provider.assign_operation_id(operation_id) + + expect(provider.operation_id).to eq(operation_id) + end + end end diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index 0bdf83fa90f..d49b71db5f8 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -51,6 +51,30 @@ describe CommitCollection do end end + describe '#with_latest_pipeline' do + let!(:pipeline) do + create( + :ci_empty_pipeline, + ref: 'master', + sha: commit.id, + status: 'success', + project: project + ) + end + let(:collection) { described_class.new(project, [commit]) } + + it 'sets the latest pipeline for every commit so no additional queries are necessary' do + commits = collection.with_latest_pipeline('master') + + recorder = ActiveRecord::QueryRecorder.new do + expect(commits.map { |c| c.latest_pipeline('master') }) + .to eq([pipeline]) + end + + expect(recorder.count).to be_zero + end + end + describe 'enrichment methods' do let(:gitaly_commit) { commit } let(:hash_commit) { Commit.from_hash(gitaly_commit.to_hash, project) } @@ -125,27 +149,17 @@ describe CommitCollection do collection.enrich! end - end - end - describe '#with_pipeline_status' do - it 'sets the pipeline status for every commit so no additional queries are necessary' do - create( - :ci_empty_pipeline, - ref: 'master', - sha: commit.id, - status: 'success', - project: project - ) + it 'returns the original commit if the commit could not be lazy loaded' do + collection = described_class.new(project, [hash_commit]) + unexisting_lazy_commit = Commit.lazy(project, Gitlab::Git::BLANK_SHA) - collection = described_class.new(project, [commit]) - collection.with_pipeline_status + expect(Commit).to receive(:lazy).with(project, hash_commit.id).and_return(unexisting_lazy_commit) - recorder = ActiveRecord::QueryRecorder.new do - expect(commit.status).to eq('success') - end + collection.enrich! - expect(recorder.count).to be_zero + expect(collection.commits).to contain_exactly(hash_commit) + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 5ef824b9950..839c4cadb5e 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -462,78 +462,6 @@ eos end end - describe '#last_pipeline' do - let!(:first_pipeline) do - create(:ci_empty_pipeline, - project: project, - sha: commit.sha, - status: 'success') - end - let!(:second_pipeline) do - create(:ci_empty_pipeline, - project: project, - sha: commit.sha, - status: 'success') - end - - it 'returns last pipeline' do - expect(commit.last_pipeline).to eq second_pipeline - end - end - - describe '#status' do - context 'without ref argument' do - before do - %w[success failed created pending].each do |status| - create(:ci_empty_pipeline, - project: project, - sha: commit.sha, - status: status) - end - end - - it 'gives compound status from latest pipelines' do - expect(commit.status).to eq(Ci::Pipeline.latest_status) - expect(commit.status).to eq('pending') - end - end - - context 'when a particular ref is specified' do - let!(:pipeline_from_master) do - create(:ci_empty_pipeline, - project: project, - sha: commit.sha, - ref: 'master', - status: 'failed') - end - - let!(:pipeline_from_fix) do - create(:ci_empty_pipeline, - project: project, - sha: commit.sha, - ref: 'fix', - status: 'success') - end - - it 'gives pipelines from a particular branch' do - expect(commit.status('master')).to eq(pipeline_from_master.status) - expect(commit.status('fix')).to eq(pipeline_from_fix.status) - end - - it 'gives compound status from latest pipelines if ref is nil' do - expect(commit.status(nil)).to eq(pipeline_from_fix.status) - end - end - end - - describe '#set_status_for_ref' do - it 'sets the status for a given reference' do - commit.set_status_for_ref('master', 'failed') - - expect(commit.status('master')).to eq('failed') - end - end - describe '#participants' do let(:user1) { build(:user) } let(:user2) { build(:user) } @@ -575,6 +503,8 @@ eos expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) expect(commit.uri_type('files/images/wm.svg')).to be(:raw) + expect(project.commit('audio').uri_type('files/audio/clip.mp3')).to be(:raw) + expect(project.commit('audio').uri_type('files/audio/sample.wav')).to be(:raw) expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) expect(commit.uri_type('files/js/application.js')).to be(:blob) end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 017cca0541e..95e9b0d0f92 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -321,7 +321,7 @@ describe CommitStatus do end it 'returns a correct compound status' do - expect(described_class.all.status).to eq 'running' + expect(described_class.all.slow_composite_status).to eq 'running' end end @@ -331,7 +331,7 @@ describe CommitStatus do end it 'returns status that indicates success' do - expect(described_class.all.status).to eq 'success' + expect(described_class.all.slow_composite_status).to eq 'success' end end @@ -342,7 +342,7 @@ describe CommitStatus do end it 'returns status according to the scope' do - expect(described_class.latest.status).to eq 'success' + expect(described_class.latest.slow_composite_status).to eq 'success' end end end diff --git a/spec/models/commit_with_pipeline_spec.rb b/spec/models/commit_with_pipeline_spec.rb new file mode 100644 index 00000000000..e0bb29fec7b --- /dev/null +++ b/spec/models/commit_with_pipeline_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe CommitWithPipeline do + let(:project) { create(:project, :public, :repository) } + let(:commit) { described_class.new(project.commit) } + + describe '#last_pipeline' do + let!(:first_pipeline) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + status: 'success') + end + let!(:second_pipeline) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + status: 'success') + end + + it 'returns last pipeline' do + expect(commit.last_pipeline).to eq second_pipeline + end + end + + describe '#latest_pipeline' do + let(:pipeline) { double } + + shared_examples_for 'fetching latest pipeline' do |ref| + it 'returns the latest pipeline for the project' do + expect(commit) + .to receive(:latest_pipeline_for_project) + .with(ref, project) + .and_return(pipeline) + + expect(result).to eq(pipeline) + end + + it "returns the memoized pipeline for the key of #{ref}" do + commit.set_latest_pipeline_for_ref(ref, pipeline) + + expect(commit) + .not_to receive(:latest_pipeline_for_project) + + expect(result).to eq(pipeline) + end + end + + context 'without ref argument' do + let(:result) { commit.latest_pipeline } + + it_behaves_like 'fetching latest pipeline', nil + end + + context 'when a particular ref is specified' do + let(:result) { commit.latest_pipeline('master') } + + it_behaves_like 'fetching latest pipeline', 'master' + end + end + + describe '#latest_pipeline_for_project' do + let(:project_pipelines) { double } + let(:pipeline_project) { double } + let(:pipeline) { double } + let(:ref) { 'master' } + let(:result) { commit.latest_pipeline_for_project(ref, pipeline_project) } + + before do + allow(pipeline_project).to receive(:ci_pipelines).and_return(project_pipelines) + end + + it 'returns the latest pipeline of the commit for the given ref and project' do + expect(project_pipelines) + .to receive(:latest_pipeline_per_commit) + .with(commit.id, ref) + .and_return(commit.id => pipeline) + + expect(result).to eq(pipeline) + end + end + + describe '#set_latest_pipeline_for_ref' do + let(:pipeline) { double } + + it 'sets the latest pipeline for a given reference' do + commit.set_latest_pipeline_for_ref('master', pipeline) + + expect(commit.latest_pipeline('master')).to eq(pipeline) + end + end + + describe "#status" do + it 'returns the status of the latest pipeline for the given ref' do + expect(commit) + .to receive(:latest_pipeline) + .with('master') + .and_return(double(status: 'success')) + + expect(commit.status('master')).to eq('success') + end + + it 'returns nil when latest pipeline is not present for the given ref' do + expect(commit) + .to receive(:latest_pipeline) + .with('master') + .and_return(nil) + + expect(commit.status('master')).to eq(nil) + end + + it 'returns the status of the latest pipeline when no ref is given' do + expect(commit) + .to receive(:latest_pipeline) + .with(nil) + .and_return(double(status: 'success')) + + expect(commit.status).to eq('success') + end + end +end diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb index de2bc3a387b..5c1694e3737 100644 --- a/spec/models/concerns/access_requestable_spec.rb +++ b/spec/models/concerns/access_requestable_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe AccessRequestable do describe 'Group' do describe '#request_access' do - let(:group) { create(:group, :public, :access_requestable) } + let(:group) { create(:group, :public) } let(:user) { create(:user) } it { expect(group.request_access(user)).to be_a(GroupMember) } @@ -13,7 +13,7 @@ describe AccessRequestable do end describe '#access_requested?' do - let(:group) { create(:group, :public, :access_requestable) } + let(:group) { create(:group, :public) } let(:user) { create(:user) } before do @@ -26,14 +26,14 @@ describe AccessRequestable do describe 'Project' do describe '#request_access' do - let(:project) { create(:project, :public, :access_requestable) } + let(:project) { create(:project, :public) } let(:user) { create(:user) } it { expect(project.request_access(user)).to be_a(ProjectMember) } end describe '#access_requested?' do - let(:project) { create(:project, :public, :access_requestable) } + let(:project) { create(:project, :public) } let(:user) { create(:user) } before do diff --git a/spec/models/concerns/atomic_internal_id_spec.rb b/spec/models/concerns/atomic_internal_id_spec.rb new file mode 100644 index 00000000000..0605392c0aa --- /dev/null +++ b/spec/models/concerns/atomic_internal_id_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AtomicInternalId do + let(:milestone) { build(:milestone) } + let(:iid) { double('iid', to_i: 42) } + let(:external_iid) { 100 } + let(:scope_attrs) { { project: milestone.project } } + let(:usage) { :milestones } + + describe '#track_project_iid!' do + subject { milestone.track_project_iid! } + + it 'tracks the present value' do + milestone.iid = external_iid + + expect(InternalId).to receive(:track_greatest).once.with(milestone, scope_attrs, usage, external_iid, anything) + expect(InternalId).not_to receive(:generate_next) + + subject + end + + context 'when value is set by ensure_project_iid!' do + it 'does not track the value' do + expect(InternalId).not_to receive(:track_greatest) + + milestone.ensure_project_iid! + subject + end + + it 'tracks the iid for the scope that is actually present' do + milestone.iid = external_iid + + expect(InternalId).to receive(:track_greatest).once.with(milestone, scope_attrs, usage, external_iid, anything) + expect(InternalId).not_to receive(:generate_next) + + # group scope is not present here, the milestone does not have a group + milestone.track_group_iid! + subject + end + end + end + + describe '#ensure_project_iid!' do + subject { milestone.ensure_project_iid! } + + it 'generates a new value if non is present' do + expect(InternalId).to receive(:generate_next).with(milestone, scope_attrs, usage, anything).and_return(iid) + + expect { subject }.to change { milestone.iid }.from(nil).to(iid.to_i) + end + + it 'generates a new value if first set with iid= but later set to nil' do + expect(InternalId).to receive(:generate_next).with(milestone, scope_attrs, usage, anything).and_return(iid) + + milestone.iid = external_iid + milestone.iid = nil + + expect { subject }.to change { milestone.iid }.from(nil).to(iid.to_i) + end + end +end diff --git a/spec/models/concerns/checksummable_spec.rb b/spec/models/concerns/checksummable_spec.rb new file mode 100644 index 00000000000..017077bd297 --- /dev/null +++ b/spec/models/concerns/checksummable_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Checksummable do + describe ".hexdigest" do + let(:fake_class) do + Class.new do + include Checksummable + end + end + + it 'returns the SHA256 sum of the file' do + expected = Digest::SHA256.file(__FILE__).hexdigest + + expect(fake_class.hexdigest(__FILE__)).to eq(expected) + end + end +end diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb deleted file mode 100644 index ad2c0770a2c..00000000000 --- a/spec/models/concerns/deployable_spec.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Deployable do - describe '#create_deployment' do - let(:deployment) { job.deployment } - let(:environment) { deployment&.environment } - - context 'when the deployable object will deploy to production' do - let!(:job) { create(:ci_build, :start_review_app) } - - it 'creates a deployment and environment record' do - expect(deployment.project).to eq(job.project) - expect(deployment.ref).to eq(job.ref) - expect(deployment.tag).to eq(job.tag) - expect(deployment.sha).to eq(job.sha) - expect(deployment.user).to eq(job.user) - expect(deployment.deployable).to eq(job) - expect(deployment.on_stop).to eq('stop_review_app') - expect(environment.name).to eq('review/master') - end - end - - context 'when the deployable object will deploy to a cluster' do - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } - let!(:job) { create(:ci_build, :start_review_app, project: project) } - - it 'creates a deployment with cluster association' do - expect(deployment.cluster).to eq(cluster) - end - end - - context 'when the deployable object will stop an environment' do - let!(:job) { create(:ci_build, :stop_review_app) } - - it 'does not create a deployment record' do - expect(deployment).to be_nil - end - end - - context 'when the deployable object has already had a deployment' do - let!(:job) { create(:ci_build, :start_review_app, deployment: race_deployment) } - let!(:race_deployment) { create(:deployment, :success) } - - it 'does not create a new deployment' do - expect(deployment).to eq(race_deployment) - end - end - - context 'when the deployable object will not deploy' do - let!(:job) { create(:ci_build) } - - it 'does not create a deployment and environment record' do - expect(deployment).to be_nil - expect(environment).to be_nil - end - end - - context 'when environment scope contains invalid character' do - let(:job) do - create( - :ci_build, - name: 'job:deploy-to-test-site', - environment: '$CI_JOB_NAME', - options: { - environment: { - name: '$CI_JOB_NAME', - url: 'http://staging.example.com/$CI_JOB_NAME', - on_stop: 'stop_review_app' - } - }) - end - - it 'does not create a deployment and environment record' do - expect(deployment).to be_nil - expect(environment).to be_nil - end - end - end -end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 220f244ad71..f99bf18768f 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -12,6 +12,26 @@ describe DeploymentPlatform do it { is_expected.to be_nil } end + context 'when project is the cluster\'s management project ' do + let!(:cluster_with_management_project) { create(:cluster, :provided_by_user, management_project: project) } + + context 'cluster_management_project feature is enabled' do + it 'returns the cluster with management project' do + is_expected.to eq(cluster_with_management_project.platform_kubernetes) + end + end + + context 'cluster_management_project feature is disabled' do + before do + stub_feature_flags(cluster_management_project: false) + end + + it 'returns nothing' do + is_expected.to be_nil + end + end + end + context 'when project has configured kubernetes from CI/CD > Clusters' do let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let(:platform_kubernetes) { cluster.platform_kubernetes } @@ -45,6 +65,35 @@ describe DeploymentPlatform do is_expected.to eq(group_cluster.platform_kubernetes) end + context 'when project is the cluster\'s management project ' do + let!(:cluster_with_management_project) { create(:cluster, :provided_by_user, management_project: project) } + + context 'cluster_management_project feature is enabled' do + it 'returns the cluster with management project' do + is_expected.to eq(cluster_with_management_project.platform_kubernetes) + end + end + + context 'cluster_management_project feature is disabled' do + before do + stub_feature_flags(cluster_management_project: false) + end + + it 'returns the group cluster' do + is_expected.to eq(group_cluster.platform_kubernetes) + end + end + end + + context 'when project is not the cluster\'s management project' do + let(:another_project) { create(:project, group: group) } + let!(:cluster_with_management_project) { create(:cluster, :provided_by_user, management_project: another_project) } + + it 'returns the group cluster' do + is_expected.to eq(group_cluster.platform_kubernetes) + end + end + context 'when child group has configured kubernetes cluster' do let(:child_group1) { create(:group, parent: group) } let!(:child_group1_cluster) { create(:cluster_for_group, groups: [child_group1]) } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 09fb2fff521..21e4dda6dab 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -3,12 +3,15 @@ require 'spec_helper' describe HasStatus do - describe '.status' do - subject { CommitStatus.status } + describe '.slow_composite_status' do + using RSpec::Parameterized::TableSyntax + + subject { CommitStatus.slow_composite_status } shared_examples 'build status summary' do context 'all successful' do let!(:statuses) { Array.new(2) { create(type, status: :success) } } + it { is_expected.to eq 'success' } end @@ -165,16 +168,26 @@ describe HasStatus do end end - context 'ci build statuses' do - let(:type) { :ci_build } - - it_behaves_like 'build status summary' + where(:ci_composite_status) do + [false, true] end - context 'generic commit statuses' do - let(:type) { :generic_commit_status } + with_them do + before do + stub_feature_flags(ci_composite_status: ci_composite_status) + end + + context 'ci build statuses' do + let(:type) { :ci_build } - it_behaves_like 'build status summary' + it_behaves_like 'build status summary' + end + + context 'generic commit statuses' do + let(:type) { :generic_commit_status } + + it_behaves_like 'build status summary' + end end end @@ -372,8 +385,8 @@ describe HasStatus do end end - describe '.status_sql' do - subject { Ci::Build.status_sql } + describe '.legacy_status_sql' do + subject { Ci::Build.legacy_status_sql } it 'returns SQL' do puts subject diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 65d41edc035..e8116f0a301 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -45,8 +45,11 @@ describe Issuable do it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_at_most(255) } - it { is_expected.to validate_length_of(:description).is_at_most(1_000_000) } + it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) } + it { is_expected.to validate_length_of(:description).is_at_most(described_class::DESCRIPTION_LENGTH_MAX).on(:create) } + + it_behaves_like 'validates description length with custom validation' + it_behaves_like 'truncates the description to its allowed maximum length on import' end describe 'milestone' do diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 929b5f52c7c..f823ac0165f 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -6,6 +6,7 @@ describe Noteable do let!(:active_diff_note1) { create(:diff_note_on_merge_request) } let(:project) { active_diff_note1.project } subject { active_diff_note1.noteable } + let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: subject, in_reply_to: active_diff_note1) } let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: active_position2) } let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: outdated_position) } diff --git a/spec/models/concerns/stepable_spec.rb b/spec/models/concerns/stepable_spec.rb index 5685de6a9bf..51356c3eaf6 100644 --- a/spec/models/concerns/stepable_spec.rb +++ b/spec/models/concerns/stepable_spec.rb @@ -7,6 +7,8 @@ describe Stepable do Class.new do include Stepable + attr_writer :return_non_success + steps :method1, :method2, :method3 def execute @@ -15,18 +17,18 @@ describe Stepable do private - def method1 + def method1(_result) { status: :success } end - def method2 - return { status: :error } unless @pass + def method2(result) + return { status: :not_a_success } if @return_non_success - { status: :success, variable1: 'var1' } + result.merge({ status: :success, variable1: 'var1', excluded_variable: 'a' }) end - def method3 - { status: :success, variable2: 'var2' } + def method3(result) + result.except(:excluded_variable).merge({ status: :success, variable2: 'var2' }) end end end @@ -41,8 +43,8 @@ describe Stepable do private - def appended_method1 - { status: :success } + def appended_method1(previous_result) + previous_result.merge({ status: :success }) end end end @@ -51,21 +53,19 @@ describe Stepable do described_class.prepend(prepended_module) end - it 'stops after the first error' do + it 'stops after the first non success status' do + subject.return_non_success = true + expect(subject).not_to receive(:method3) expect(subject).not_to receive(:appended_method1) expect(subject.execute).to eq( - status: :error, - failed_step: :method2 + status: :not_a_success, + last_step: :method2 ) end context 'when all methods return success' do - before do - subject.instance_variable_set(:@pass, true) - end - it 'calls all methods in order' do expect(subject).to receive(:method1).and_call_original.ordered expect(subject).to receive(:method2).and_call_original.ordered @@ -82,6 +82,10 @@ describe Stepable do variable2: 'var2' ) end + + it 'can modify results of previous steps' do + expect(subject.execute).not_to include(excluded_variable: 'a') + end end context 'with multiple stepable classes' do diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 51e28974ae0..43b894b5957 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -17,6 +17,7 @@ describe User, 'TokenAuthenticatable' do describe 'ensures authentication token' do subject { create(:user).send(token_field) } + it { is_expected.to be_a String } end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 935838ce294..eea539746a5 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -78,7 +78,7 @@ describe ContainerRepository do describe '#delete_tags!' do let(:repository) do create(:container_repository, name: 'my_image', - tags: %w[latest rc1], + tags: { latest: '123', rc1: '234' }, project: project) end @@ -86,6 +86,7 @@ describe ContainerRepository do it 'returns status that indicates success' do expect(repository.client) .to receive(:delete_repository_tag) + .twice .and_return(true) expect(repository.delete_tags!).to be_truthy @@ -96,6 +97,7 @@ describe ContainerRepository do it 'returns status that indicates failure' do expect(repository.client) .to receive(:delete_repository_tag) + .twice .and_return(false) expect(repository.delete_tags!).to be_falsey diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb index c137444763b..1dbae78a01d 100644 --- a/spec/models/deploy_keys_project_spec.rb +++ b/spec/models/deploy_keys_project_spec.rb @@ -16,6 +16,7 @@ describe DeployKeysProject do describe "Destroying" do let(:project) { create(:project) } subject { create(:deploy_keys_project, project: project) } + let(:deploy_key) { subject.deploy_key } context "when the deploy key is only used by this project" do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 51ed8e9421b..3a0b3c46ad0 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -348,4 +348,17 @@ describe Deployment do expect(deployment.deployed_by).to eq(build_user) end end + + describe '.find_successful_deployment!' do + it 'returns a successful deployment' do + deploy = create(:deployment, :success) + + expect(described_class.find_successful_deployment!(deploy.iid)).to eq(deploy) + end + + it 'raises when no deployment is found' do + expect { described_class.find_successful_deployment!(-1) } + .to raise_error(ActiveRecord::RecordNotFound) + end + end end diff --git a/spec/models/description_version_spec.rb b/spec/models/description_version_spec.rb new file mode 100644 index 00000000000..5ec34c0cde4 --- /dev/null +++ b/spec/models/description_version_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DescriptionVersion do + describe 'associations' do + it { is_expected.to belong_to :issue } + it { is_expected.to belong_to :merge_request } + end + + describe 'validations' do + describe 'exactly_one_issuable' do + using RSpec::Parameterized::TableSyntax + + subject { described_class.new(issue_id: issue_id, merge_request_id: merge_request_id).valid? } + + where(:issue_id, :merge_request_id, :valid?) do + nil | 1 | true + 1 | nil | true + nil | nil | false + 1 | 1 | false + end + + with_them do + it { is_expected.to eq(valid?) } + end + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 521c4704c87..786f3b832c4 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -882,4 +882,19 @@ describe Environment, :use_clean_rails_memory_store_caching do end end end + + describe '.find_or_create_by_name' do + it 'finds an existing environment if it exists' do + env = create(:environment) + + expect(described_class.find_or_create_by_name(env.name)).to eq(env) + end + + it 'creates an environment if it does not exist' do + env = project.environments.find_or_create_by_name('kittens') + + expect(env).to be_an_instance_of(described_class) + expect(env).to be_persisted + end + end end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index e2836420df9..01d331f518b 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -95,7 +95,7 @@ describe EnvironmentStatus do describe '.build_environments_status' do subject { described_class.send(:build_environments_status, merge_request, user, pipeline) } - let!(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) } + let!(:build) { create(:ci_build, :with_deployment, :deploy_to_production, pipeline: pipeline) } let(:environment) { build.deployment.environment } let(:user) { project.owner } diff --git a/spec/models/event_collection_spec.rb b/spec/models/event_collection_spec.rb index efe511042c3..c421ffa000d 100644 --- a/spec/models/event_collection_spec.rb +++ b/spec/models/event_collection_spec.rb @@ -4,50 +4,75 @@ require 'spec_helper' describe EventCollection do describe '#to_a' do - let(:project) { create(:project_empty_repo) } - let(:projects) { Project.where(id: project.id) } - let(:user) { create(:user) } + set(:group) { create(:group) } + set(:project) { create(:project_empty_repo, group: group) } + set(:projects) { Project.where(id: project.id) } + set(:user) { create(:user) } - before do - 20.times do - event = create(:push_event, project: project, author: user) + context 'with project events' do + before do + 20.times do + event = create(:push_event, project: project, author: user) - create(:push_event_payload, event: event) + create(:push_event_payload, event: event) + end + + create(:closed_issue_event, project: project, author: user) end - create(:closed_issue_event, project: project, author: user) - end + it 'returns an Array of events' do + events = described_class.new(projects).to_a - it 'returns an Array of events' do - events = described_class.new(projects).to_a + expect(events).to be_an_instance_of(Array) + end - expect(events).to be_an_instance_of(Array) - end + it 'applies a limit to the number of events' do + events = described_class.new(projects).to_a - it 'applies a limit to the number of events' do - events = described_class.new(projects).to_a + expect(events.length).to eq(20) + end - expect(events.length).to eq(20) - end + it 'can paginate through events' do + events = described_class.new(projects, offset: 20).to_a - it 'can paginate through events' do - events = described_class.new(projects, offset: 20).to_a + expect(events.length).to eq(1) + end - expect(events.length).to eq(1) - end + it 'returns an empty Array when crossing the maximum page number' do + events = described_class.new(projects, limit: 1, offset: 15).to_a - it 'returns an empty Array when crossing the maximum page number' do - events = described_class.new(projects, limit: 1, offset: 15).to_a + expect(events).to be_empty + end + + it 'allows filtering of events using an EventFilter' do + filter = EventFilter.new(EventFilter::ISSUE) + events = described_class.new(projects, filter: filter).to_a - expect(events).to be_empty + expect(events.length).to eq(1) + expect(events[0].action).to eq(Event::CLOSED) + end end - it 'allows filtering of events using an EventFilter' do - filter = EventFilter.new(EventFilter::ISSUE) - events = described_class.new(projects, filter: filter).to_a + context 'with group events' do + let(:groups) { group.self_and_descendants.public_or_visible_to_user(user) } + let(:subject) { described_class.new(projects, groups: groups).to_a } + + it 'includes also group events' do + subgroup = create(:group, parent: group) + event1 = create(:event, project: project, author: user) + event2 = create(:event, project: nil, group: group, author: user) + event3 = create(:event, project: nil, group: subgroup, author: user) - expect(events.length).to eq(1) - expect(events[0].action).to eq(Event::CLOSED) + expect(subject).to eq([event3, event2, event1]) + end + + it 'does not include events from inaccessible groups' do + subgroup = create(:group, :private, parent: group) + event1 = create(:event, project: nil, group: group, author: user) + create(:event, project: nil, group: subgroup, author: user) + + expect(subject).to eq([event1]) + end end end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 62663c247d1..ff2e1aa047e 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -100,26 +100,31 @@ describe Event do describe '#membership_changed?' do context "created" do subject { build(:event, :created).membership_changed? } + it { is_expected.to be_falsey } end context "updated" do subject { build(:event, :updated).membership_changed? } + it { is_expected.to be_falsey } end context "expired" do subject { build(:event, :expired).membership_changed? } + it { is_expected.to be_truthy } end context "left" do subject { build(:event, :left).membership_changed? } + it { is_expected.to be_truthy } end context "joined" do subject { build(:event, :joined).membership_changed? } + it { is_expected.to be_truthy } end end diff --git a/spec/models/evidence_spec.rb b/spec/models/evidence_spec.rb new file mode 100644 index 00000000000..00788c2c391 --- /dev/null +++ b/spec/models/evidence_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Evidence do + let_it_be(:project) { create(:project) } + let(:release) { create(:release, project: project) } + let(:schema_file) { 'evidences/evidence' } + let(:summary_json) { described_class.last.summary.to_json } + + describe 'associations' do + it { is_expected.to belong_to(:release) } + end + + describe 'summary_sha' do + it 'returns nil if summary is nil' do + expect(build(:evidence, summary: nil).summary_sha).to be_nil + end + end + + describe '#generate_summary_and_sha' do + before do + described_class.create!(release: release) + end + + context 'when a release name is not provided' do + let(:release) { create(:release, project: project, name: nil) } + + it 'creates a valid JSON object' do + expect(release.name).to be_nil + expect(summary_json).to match_schema(schema_file) + end + end + + context 'when a release is associated to a milestone' do + let(:milestone) { create(:milestone, project: project) } + let(:release) { create(:release, project: project, milestones: [milestone]) } + + context 'when a milestone has no issue associated with it' do + it 'creates a valid JSON object' do + expect(milestone.issues).to be_empty + expect(summary_json).to match_schema(schema_file) + end + end + + context 'when a milestone has no description' do + let(:milestone) { create(:milestone, project: project, description: nil) } + + it 'creates a valid JSON object' do + expect(milestone.description).to be_nil + expect(summary_json).to match_schema(schema_file) + end + end + + context 'when a milestone has no due_date' do + let(:milestone) { create(:milestone, project: project, due_date: nil) } + + it 'creates a valid JSON object' do + expect(milestone.due_date).to be_nil + expect(summary_json).to match_schema(schema_file) + end + end + + context 'when a milestone has an issue' do + context 'when the issue has no description' do + let(:issue) { create(:issue, project: project, description: nil, state: 'closed') } + + before do + milestone.issues << issue + end + + it 'creates a valid JSON object' do + expect(milestone.issues.first.description).to be_nil + expect(summary_json).to match_schema(schema_file) + end + end + end + end + + context 'when a release is not associated to any milestone' do + it 'creates a valid JSON object' do + expect(release.milestones).to be_empty + expect(summary_json).to match_schema(schema_file) + end + end + end +end diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index 4911375c962..a780b8bfdf5 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -20,6 +20,7 @@ RSpec.describe GpgSignature do describe 'validation' do subject { described_class.new } + it { is_expected.to validate_presence_of(:commit_sha) } it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:gpg_key_primary_keyid) } @@ -60,6 +61,18 @@ RSpec.describe GpgSignature do end 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) } + + it 'returns all gpg signatures by sha' do + expect(described_class.by_commit_sha(commit_sha)).to eq([gpg_signature]) + expect( + described_class.by_commit_sha([commit_sha, another_gpg_signature.commit_sha]) + ).to contain_exactly(gpg_signature, another_gpg_signature) + end + end + describe '#commit' do it 'fetches the commit through the project' do expect_any_instance_of(Project).to receive(:commit).with(commit_sha).and_return(commit) diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb new file mode 100644 index 00000000000..f8973097a40 --- /dev/null +++ b/spec/models/grafana_integration_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GrafanaIntegration do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:token) } + + it 'disallows invalid urls for grafana_url' do + unsafe_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>} + non_ascii_url = 'http://gitlab.com/api/0/projects/project1/something€' + blank_url = '' + excessively_long_url = 'https://grafan' + 'a' * 1024 + '.com' + + is_expected.not_to allow_values( + unsafe_url, + non_ascii_url, + blank_url, + excessively_long_url + ).for(:grafana_url) + end + + it 'allows valid urls for grafana_url' do + external_url = 'http://grafana.com/' + internal_url = 'http://192.168.1.1' + + is_expected.to allow_value( + external_url, + internal_url + ).for(:grafana_url) + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 796b6917fb2..520421ac5e3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Group do - let!(:group) { create(:group, :access_requestable) } + let!(:group) { create(:group) } describe 'associations' do it { is_expected.to have_many :projects } @@ -331,7 +331,7 @@ describe Group do end describe '#avatar_url' do - let!(:group) { create(:group, :access_requestable, :with_avatar) } + let!(:group) { create(:group, :with_avatar) } let(:user) { create(:user) } context 'when avatar file is uploaded' do @@ -880,22 +880,6 @@ describe Group do end end - describe '#has_parent?' do - context 'when the group has a parent' do - it 'is truthy' do - group = create(:group, :nested) - expect(group.has_parent?).to be_truthy - end - end - - context 'when the group has no parent' do - it 'is falsy' do - group = create(:group, parent: nil) - expect(group.has_parent?).to be_falsy - end - end - end - context 'with uploads' do it_behaves_like 'model with uploads', true do let(:model_object) { create(:group, :with_avatar) } @@ -1058,4 +1042,21 @@ describe Group do expect(group.access_request_approvers_to_be_notified).to eq(active_owners_in_recent_sign_in_desc_order) end end + + describe '.groups_including_descendants_by' do + it 'returns the expected groups for a group and its descendants' do + parent_group1 = create(:group) + child_group1 = create(:group, parent: parent_group1) + child_group2 = create(:group, parent: parent_group1) + + parent_group2 = create(:group) + child_group3 = create(:group, parent: parent_group2) + + create(:group) + + groups = described_class.groups_including_descendants_by([parent_group2.id, parent_group1.id]) + + expect(groups).to contain_exactly(parent_group1, parent_group2, child_group1, child_group2, child_group3) + end + end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index fe08dc4f5e6..025c11d6407 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -6,7 +6,7 @@ describe WebHook do let(:hook) { build(:project_hook) } describe 'associations' do - it { is_expected.to have_many(:web_hook_logs).dependent(:destroy) } + it { is_expected.to have_many(:web_hook_logs) } end describe 'validations' do @@ -85,4 +85,13 @@ describe WebHook do hook.async_execute(data, hook_name) end end + + describe '#destroy' do + it 'cascades to web_hook_logs' do + web_hook = create(:project_hook) + create_list(:web_hook_log, 3, web_hook: web_hook) + + expect { web_hook.destroy }.to change(web_hook.web_hook_logs, :count).by(-3) + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 9c58d307c4c..18a1a30eee5 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -138,7 +138,10 @@ describe Issue do end it 'changes the state to closed' do - expect { issue.close }.to change { issue.state }.from('opened').to('closed') + open_state = described_class.available_states[:opened] + closed_state = described_class.available_states[:closed] + + expect { issue.close }.to change { issue.state_id }.from(open_state).to(closed_state) end end @@ -155,7 +158,7 @@ describe Issue do end it 'changes the state to opened' do - expect { issue.reopen }.to change { issue.state }.from('closed').to('opened') + expect { issue.reopen }.to change { issue.state_id }.from(described_class.available_states[:closed]).to(described_class.available_states[:opened]) end end @@ -277,6 +280,7 @@ describe Issue do context 'checking destination project also' do subject { issue.can_move?(user, to_project) } + let(:to_project) { create(:project) } context 'destination project allowed' do @@ -899,4 +903,6 @@ describe Issue do let(:default_params) { { project: project } } end end + + it_behaves_like 'versioned description' end diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index 85bfc3f1387..47cae5cf197 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -31,6 +31,46 @@ describe LfsObject do end end + describe '#project_allowed_access?' do + set(:lfs_object) { create(:lfs_objects_project).lfs_object } + set(:project) { create(:project) } + + it 'returns true when project is linked' do + create(:lfs_objects_project, lfs_object: lfs_object, project: project) + + expect(lfs_object.project_allowed_access?(project)).to eq(true) + end + + it 'returns false when project is not linked' do + expect(lfs_object.project_allowed_access?(project)).to eq(false) + end + + context 'when project is a member of a fork network' do + set(:fork_network) { create(:fork_network) } + set(:fork_network_root_project) { fork_network.root_project } + set(:fork_network_membership) { create(:fork_network_member, project: project, fork_network: fork_network) } + + it 'returns true for all members when forked project is linked' do + create(:lfs_objects_project, lfs_object: lfs_object, project: project) + + expect(lfs_object.project_allowed_access?(project)).to eq(true) + expect(lfs_object.project_allowed_access?(fork_network_root_project)).to eq(true) + end + + it 'returns true for all members when root of network is linked' do + create(:lfs_objects_project, lfs_object: lfs_object, project: fork_network_root_project) + + expect(lfs_object.project_allowed_access?(project)).to eq(true) + expect(lfs_object.project_allowed_access?(fork_network_root_project)).to eq(true) + end + + it 'returns false when no member of fork network is linked' do + expect(lfs_object.project_allowed_access?(project)).to eq(false) + expect(lfs_object.project_allowed_access?(fork_network_root_project)).to eq(false) + end + end + end + describe '#schedule_background_upload' do before do stub_lfs_setting(enabled: true) @@ -116,4 +156,15 @@ describe LfsObject do end end end + + describe ".calculate_oid" do + let(:lfs_object) { create(:lfs_object, :with_file) } + + it 'returns SHA256 sum of the file' do + path = lfs_object.file.path + expected = Digest::SHA256.file(path).hexdigest + + expect(described_class.calculate_oid(path)).to eq expected + end + end end diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index dc28204d7aa..bc9124e73af 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -136,18 +136,6 @@ describe List do expect(preferences).to be_persisted expect(preferences.collapsed).to eq(true) end - - context 'when preferences are already loaded for user' do - it 'gets preloaded user preferences' do - fetched_list = described_class.where(id: list.id).with_preferences_for(user).first - - expect(fetched_list).to receive(:preloaded_preferences_for).with(user).and_call_original - - preferences = fetched_list.preferences_for(user) - - expect(preferences.collapsed).to eq(true) - end - end end context 'when preferences for user does not exist' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 2cb4f222ea4..e7f03226826 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -92,7 +92,7 @@ describe Member do describe 'Scopes & finders' do before do - project = create(:project, :public, :access_requestable) + project = create(:project, :public) group = create(:group) @owner_user = create(:user).tap { |u| group.add_owner(u) } @owner = group.members.find_by(user_id: @owner_user.id) @@ -230,7 +230,7 @@ describe Member do describe '.add_user' do %w[project group].each do |source_type| context "when source is a #{source_type}" do - let!(:source) { create(source_type, :public, :access_requestable) } + let!(:source) { create(source_type, :public) } let!(:user) { create(:user) } let!(:admin) { create(:admin) } @@ -437,7 +437,7 @@ describe Member do describe '.add_users' do %w[project group].each do |source_type| context "when source is a #{source_type}" do - let!(:source) { create(source_type, :public, :access_requestable) } + let!(:source) { create(source_type, :public) } let!(:admin) { create(:admin) } let(:user1) { create(:user) } let(:user2) { create(:user) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 65cc1a4bd6b..ad79bee8801 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -470,7 +470,7 @@ describe MergeRequest do commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") allow(subject).to receive(:commits).and_return([commit]) - allow(subject).to receive(:state).and_return("closed") + allow(subject).to receive(:state_id).and_return(described_class.available_states[:closed]) expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count) end @@ -479,7 +479,7 @@ describe MergeRequest do issue = create :issue, project: subject.project commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") allow(subject).to receive(:commits).and_return([commit]) - allow(subject).to receive(:state).and_return("merged") + allow(subject).to receive(:state_id).and_return(described_class.available_states[:merged]) expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count) end @@ -541,6 +541,7 @@ describe MergeRequest do context 'with diffs' do subject { create(:merge_request, :with_diffs) } + it 'returns the sha of the source branch last commit' do expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end @@ -548,6 +549,7 @@ describe MergeRequest do context 'without diffs' do subject { create(:merge_request, :without_diffs) } + it 'returns the sha of the source branch last commit' do expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end @@ -570,6 +572,7 @@ describe MergeRequest do context 'when the merge request is being created' do subject { build(:merge_request, source_branch: nil, compare_commits: []) } + it 'returns nil' do expect(subject.source_branch_sha).to be_nil end @@ -650,6 +653,46 @@ describe MergeRequest do end end + describe '#note_positions_for_paths' do + let(:merge_request) { create(:merge_request, :with_diffs) } + let(:project) { merge_request.project } + let!(:diff_note) do + create(:diff_note_on_merge_request, project: project, noteable: merge_request) + end + + let(:file_paths) { merge_request.diffs.diff_files.map(&:file_path) } + + subject do + merge_request.note_positions_for_paths(file_paths) + end + + it 'returns a Gitlab::Diff::PositionCollection' do + expect(subject).to be_a(Gitlab::Diff::PositionCollection) + end + + context 'within all diff files' do + it 'returns correct positions' do + expect(subject).to match_array([diff_note.position]) + end + end + + context 'within specific diff file' do + let(:file_paths) { [diff_note.position.file_path] } + + it 'returns correct positions' do + expect(subject).to match_array([diff_note.position]) + end + end + + context 'within no diff files' do + let(:file_paths) { [] } + + it 'returns no positions' do + expect(subject.to_a).to be_empty + end + end + end + describe '#discussions_diffs' do let(:merge_request) { create(:merge_request) } @@ -2032,7 +2075,7 @@ describe MergeRequest do end it 'refuses to enqueue a job if the MR is not open' do - merge_request.update_column(:state, 'foo') + merge_request.update_column(:state_id, 5) expect(RebaseWorker).not_to receive(:perform_async) @@ -2326,7 +2369,7 @@ describe MergeRequest do merge_requests_as_head_pipeline: [merge_request]) end - let!(:job) { create(:ci_build, :start_review_app, pipeline: pipeline, project: project) } + let!(:job) { create(:ci_build, :with_deployment, :start_review_app, pipeline: pipeline, project: project) } it 'returns environments' do is_expected.to eq(pipeline.environments) @@ -2455,6 +2498,7 @@ describe MergeRequest do describe "#diff_refs" do context "with diffs" do subject { create(:merge_request, :with_diffs) } + let(:expected_diff_refs) do Gitlab::Diff::DiffRefs.new( base_sha: subject.merge_request_diff.base_commit_sha, @@ -2527,32 +2571,32 @@ describe MergeRequest do describe '#merge_ongoing?' do it 'returns true when the merge request is locked' do - merge_request = build_stubbed(:merge_request, state: :locked) + merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:locked]) expect(merge_request.merge_ongoing?).to be(true) end it 'returns true when merge_id, MR is not merged and it has no running job' do - merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo') + merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: 'foo') allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true } expect(merge_request.merge_ongoing?).to be(true) end it 'returns false when merge_jid is nil' do - merge_request = build_stubbed(:merge_request, state: :open, merge_jid: nil) + merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: nil) expect(merge_request.merge_ongoing?).to be(false) end it 'returns false if MR is merged' do - merge_request = build_stubbed(:merge_request, state: :merged, merge_jid: 'foo') + merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:merged], merge_jid: 'foo') expect(merge_request.merge_ongoing?).to be(false) end it 'returns false if there is no merge job running' do - merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo') + merge_request = build_stubbed(:merge_request, state_id: described_class.available_states[:opened], merge_jid: 'foo') allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { false } expect(merge_request.merge_ongoing?).to be(false) @@ -2686,7 +2730,7 @@ describe MergeRequest do context 'closed MR' do before do - merge_request.update_attribute(:state, :closed) + merge_request.update_attribute(:state_id, described_class.available_states[:closed]) end it 'is not mergeable' do @@ -2800,6 +2844,7 @@ describe MergeRequest do describe '#merge_request_diff_for' do subject { create(:merge_request, importing: true) } + let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } @@ -2830,6 +2875,7 @@ describe MergeRequest do describe '#version_params_for' do subject { create(:merge_request, importing: true) } + let(:project) { subject.project } let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } @@ -3257,4 +3303,40 @@ describe MergeRequest do end end end + + describe '.with_open_merge_when_pipeline_succeeds' do + let!(:project) { create(:project) } + let!(:fork) { fork_project(project) } + let!(:merge_request1) do + create(:merge_request, + :merge_when_pipeline_succeeds, + target_project: project, + target_branch: 'master', + source_project: project, + source_branch: 'feature-1') + end + + let!(:merge_request2) do + create(:merge_request, + :merge_when_pipeline_succeeds, + target_project: project, + target_branch: 'master', + source_project: fork, + source_branch: 'fork-feature-1') + end + + let!(:merge_request4) do + create(:merge_request, + target_project: project, + target_branch: 'master', + source_project: fork, + source_branch: 'fork-feature-2') + end + + let(:query) { described_class.with_open_merge_when_pipeline_succeeds } + + it { expect(query).to contain_exactly(merge_request1, merge_request2) } + end + + it_behaves_like 'versioned description' end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 972f26ac745..1e06d0fd7b9 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -191,6 +191,16 @@ describe Namespace do end end + describe '.find_by_pages_host' do + it 'finds namespace by GitLab Pages host and is case-insensitive' do + namespace = create(:namespace, name: 'topnamespace') + create(:namespace, name: 'annother_namespace') + host = "TopNamespace.#{Settings.pages.host.upcase}" + + expect(described_class.find_by_pages_host(host)).to eq(namespace) + end + end + describe '#ancestors_upto' do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } @@ -240,7 +250,7 @@ describe Namespace do it "moves dir if path changed" do namespace.update(path: namespace.full_path + '_new') - expect(gitlab_shell.exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy end context 'when #write_projects_repository_config raises an error' do @@ -348,7 +358,7 @@ describe Namespace do namespace.update(path: namespace.full_path + '_new') expect(before_disk_path).to eq(project.disk_path) - expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy end end @@ -886,31 +896,110 @@ describe Namespace do end end end + end - context 'when :emails_disabled feature flag is off' do - before do - stub_feature_flags(emails_disabled: false) - end + describe '#pages_virtual_domain' do + let(:project) { create(:project, namespace: namespace) } + + context 'when there are pages deployed for the project' do + context 'but pages metadata is not migrated' do + before do + generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') + generic_commit_status.update!(project: project) + project.pages_metadatum.destroy! + end + + it 'migrates pages metadata and returns the virual domain' do + virtual_domain = namespace.pages_virtual_domain - context 'when not a subgroup' do - it 'returns false' do - group = create(:group, emails_disabled: true) + expect(project.reload.pages_metadatum.deployed).to eq(true) - expect(group.emails_disabled?).to be_falsey + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty end end - context 'when a subgroup and ancestor emails are disabled' do - let(:grandparent) { create(:group) } - let(:parent) { create(:group, parent: grandparent) } - let(:group) { create(:group, parent: parent) } + context 'and pages metadata is migrated' do + before do + project.mark_pages_as_deployed + end - it 'returns false' do - grandparent.update_attribute(:emails_disabled, true) + it 'returns the virual domain' do + virtual_domain = namespace.pages_virtual_domain - expect(group.emails_disabled?).to be_falsey + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths).not_to be_empty end end end end + + describe '#has_parent?' do + it 'returns true when the group has a parent' do + group = create(:group, :nested) + + expect(group.has_parent?).to be_truthy + end + + it 'returns true when the group has an unsaved parent' do + parent = build(:group) + group = build(:group, parent: parent) + + expect(group.has_parent?).to be_truthy + end + + it 'returns false when the group has no parent' do + group = create(:group, parent: nil) + + expect(group.has_parent?).to be_falsy + end + end + + describe '#closest_setting' do + using RSpec::Parameterized::TableSyntax + + shared_examples_for 'fetching closest setting' do + let!(:root_namespace) { create(:namespace) } + let!(:namespace) { create(:namespace, parent: root_namespace) } + + let(:setting) { namespace.closest_setting(setting_name) } + + before do + root_namespace.update_attribute(setting_name, root_setting) + namespace.update_attribute(setting_name, child_setting) + end + + it 'returns closest non-nil value' do + expect(setting).to eq(result) + end + end + + context 'when setting is of non-boolean type' do + where(:root_setting, :child_setting, :result) do + 100 | 200 | 200 + 100 | nil | 100 + nil | nil | nil + end + + with_them do + let(:setting_name) { :max_artifacts_size } + + it_behaves_like 'fetching closest setting' + end + end + + context 'when setting is of boolean type' do + where(:root_setting, :child_setting, :result) do + true | false | false + true | nil | true + nil | nil | nil + end + + with_them do + let(:setting_name) { :lfs_enabled } + + it_behaves_like 'fetching closest setting' + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 66e3c6d5e9d..4c320b4b145 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -55,11 +55,13 @@ describe Note do context 'when noteable and note project are the same' do subject { create(:note) } + it { is_expected.to be_valid } end context 'when project is missing for a project related note' do subject { build(:note, project: nil, noteable: build_stubbed(:issue)) } + it { is_expected.to be_invalid } end @@ -70,6 +72,37 @@ describe Note do is_expected.to be_valid end end + + describe 'max notes limit' do + let_it_be(:noteable) { create(:issue) } + let_it_be(:existing_note) { create(:note, project: noteable.project, noteable: noteable) } + + before do + stub_const('Noteable::MAX_NOTES_LIMIT', 1) + end + + context 'when creating a system note' do + subject { build(:system_note, project: noteable.project, noteable: noteable) } + + it { is_expected.to be_valid } + end + + context 'when creating a user note' do + subject { build(:note, project: noteable.project, noteable: noteable) } + + it { is_expected.not_to be_valid } + end + + context 'when updating an existing note on a noteable that already exceeds the limit' do + subject { existing_note } + + before do + create(:system_note, project: noteable.project, noteable: noteable) + end + + it { is_expected.to be_valid } + end + end end describe "Commit notes" do @@ -710,6 +743,7 @@ describe Note do describe '#to_discussion' do subject { create(:discussion_note_on_merge_request) } + let!(:note2) { create(:discussion_note_on_merge_request, project: subject.project, noteable: subject.noteable, in_reply_to: subject) } it "returns a discussion with just this note" do @@ -777,6 +811,7 @@ describe Note do context 'for a note' do context 'when part of a discussion' do subject { create(:discussion_note_on_issue) } + let(:note) { create(:discussion_note_on_issue, in_reply_to: subject) } it 'checks if the note is in reply to the other discussion' do @@ -790,6 +825,7 @@ describe Note do context 'when not part of a discussion' do subject { create(:note) } + let(:note) { create(:note, in_reply_to: subject) } it 'checks if the note is in reply to the other noteable' do @@ -804,6 +840,7 @@ describe Note do context 'for a discussion' do context 'when part of the same discussion' do subject { create(:diff_note_on_merge_request) } + let(:note) { create(:diff_note_on_merge_request, in_reply_to: subject) } it 'returns true' do @@ -813,6 +850,7 @@ describe Note do context 'when not part of the same discussion' do subject { create(:diff_note_on_merge_request) } + let(:note) { create(:diff_note_on_merge_request) } it 'returns false' do @@ -824,6 +862,7 @@ describe Note do context 'for a noteable' do context 'when a comment on the same noteable' do subject { create(:note) } + let(:note) { create(:note, in_reply_to: subject) } it 'returns true' do @@ -833,6 +872,7 @@ describe Note do context 'when not a comment on the same noteable' do subject { create(:note) } + let(:note) { create(:note) } it 'returns false' do @@ -856,6 +896,7 @@ describe Note do context 'when not part of a discussion' do subject { create(:note) } + let(:note) { create(:note, in_reply_to: subject) } it 'returns the noteable' do @@ -941,13 +982,64 @@ describe Note do project = create(:project) note = create(:note_on_issue, project: project) - expect(note.parent).to eq(project) + expect(note.resource_parent).to eq(project) end it 'returns nil for personal snippet note' do note = create(:note_on_personal_snippet) - expect(note.parent).to be_nil + expect(note.resource_parent).to be_nil + end + end + + describe 'scopes' do + let_it_be(:note1) { create(:note, note: 'Test 345') } + let_it_be(:note2) { create(:note, note: 'Test 789') } + + describe '#for_note_or_capitalized_note' do + it 'returns the expected matching note' do + notes = described_class.for_note_or_capitalized_note('Test 345') + + expect(notes.count).to eq(1) + expect(notes.first.id).to eq(note1.id) + end + + it 'returns the expected capitalized note' do + notes = described_class.for_note_or_capitalized_note('test 345') + + expect(notes.count).to eq(1) + expect(notes.first.id).to eq(note1.id) + end + + it 'does not support pattern matching' do + notes = described_class.for_note_or_capitalized_note('test%') + + expect(notes.count).to eq(0) + end + end + + describe '#like_note_or_capitalized_note' do + it 'returns the expected matching note' do + notes = described_class.like_note_or_capitalized_note('Test 345') + + expect(notes.count).to eq(1) + expect(notes.first.id).to eq(note1.id) + end + + it 'returns the expected capitalized note' do + notes = described_class.like_note_or_capitalized_note('test 345') + + expect(notes.count).to eq(1) + expect(notes.first.id).to eq(note1.id) + end + + it 'supports pattern matching' do + notes = described_class.like_note_or_capitalized_note('test%') + + expect(notes.count).to eq(2) + expect(notes.first.id).to eq(note1.id) + expect(notes.second.id).to eq(note2.id) + end end end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 820d233dbdc..094c60e3e09 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -98,6 +98,7 @@ RSpec.describe NotificationSetting do it 'returns email events' do expect(subject).to include( + :new_release, :new_note, :new_issue, :reopen_issue, diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 2146b0c9abd..c05d4c82634 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -57,8 +57,18 @@ describe Pages::LookupPath do end describe '#prefix' do - it 'returns "/"' do + it 'returns "/" for pages group root projects' do + project = instance_double(Project, pages_group_root?: true) + lookup_path = described_class.new(project, trim_prefix: 'mygroup') + expect(lookup_path.prefix).to eq('/') end + + it 'returns the project full path with the provided prefix removed' do + project = instance_double(Project, pages_group_root?: false, full_path: 'mygroup/myproject') + lookup_path = described_class.new(project, trim_prefix: 'mygroup') + + expect(lookup_path.prefix).to eq('/myproject/') + end end end diff --git a/spec/models/pages/virtual_domain_spec.rb b/spec/models/pages/virtual_domain_spec.rb index eaa57b7acd6..a5310738482 100644 --- a/spec/models/pages/virtual_domain_spec.rb +++ b/spec/models/pages/virtual_domain_spec.rb @@ -25,19 +25,33 @@ describe Pages::VirtualDomain do end describe '#lookup_paths' do - let(:domain) { instance_double(PagesDomain) } let(:project_a) { instance_double(Project) } let(:project_z) { instance_double(Project) } let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa') } let(:pages_lookup_path_z) { instance_double(Pages::LookupPath, prefix: 'zzz') } - subject(:virtual_domain) { described_class.new([project_a, project_z], domain: domain) } + context 'when there is pages domain provided' do + let(:domain) { instance_double(PagesDomain) } - it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do - expect(project_a).to receive(:pages_lookup_path).with(domain: domain).and_return(pages_lookup_path_a) - expect(project_z).to receive(:pages_lookup_path).with(domain: domain).and_return(pages_lookup_path_z) + subject(:virtual_domain) { described_class.new([project_a, project_z], domain: domain) } - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a]) + it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do + expect(project_a).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_a) + expect(project_z).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_z) + + expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a]) + end + end + + context 'when there is trim_prefix provided' do + subject(:virtual_domain) { described_class.new([project_a, project_z], trim_prefix: 'group/') } + + it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do + expect(project_a).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_a) + expect(project_z).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_z) + + expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a]) + end end end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 89a837dc812..4b65bf032d1 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -160,7 +160,7 @@ describe PagesDomain do end context 'when curve is set explicitly by parameters' do - it 'adds errors to private key', :quarantine do + it 'adds errors to private key' do domain = build(:pages_domain, :explicit_ecdsa) expect(domain).to be_invalid @@ -293,11 +293,13 @@ describe PagesDomain do describe "#https?" do context "when a certificate is present" do subject { build(:pages_domain) } + it { is_expected.to be_https } end context "when no certificate is present" do subject { build(:pages_domain, :without_certificate) } + it { is_expected.not_to be_https } end end @@ -557,15 +559,35 @@ describe PagesDomain do end end - describe '.pages_virtual_domain' do - let(:project) { build(:project) } + describe '#pages_virtual_domain' do + let(:project) { create(:project) } + let(:pages_domain) { create(:pages_domain, project: project) } + + context 'when there are no pages deployed for the project' do + it 'returns nil' do + expect(pages_domain.pages_virtual_domain).to be_nil + end + end + + context 'when there are pages deployed for the project' do + before do + generic_commit_status = create(:generic_commit_status, :success, stage: 'deploy', name: 'pages:deploy') + generic_commit_status.update!(project: project) + project.pages_metadatum.destroy! + project.reload + end - subject(:pages_domain) { build(:pages_domain, project: project) } + it 'returns the virual domain' do + expect(Pages::VirtualDomain).to receive(:new).with([project], domain: pages_domain).and_call_original - it 'returns instance of Pages::VirtualDomain' do - expect(Pages::VirtualDomain).to receive(:new).with([project], domain: pages_domain).and_call_original + expect(pages_domain.pages_virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + end - expect(pages_domain.pages_virtual_domain).to be_a(Pages::VirtualDomain) + it 'migrates project pages metadata' do + expect { pages_domain.pages_virtual_domain }.to change { + project.reload.pages_metadatum&.deployed + }.from(nil).to(true) + end end end end diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb index 66481a461ca..d0ab5afc765 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/project_services/bugzilla_service_spec.rb @@ -41,7 +41,7 @@ describe BugzillaService do { project_url: url, issues_url: url, new_issue_url: url } end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { access_params.merge(title: title, description: description) } let(:service) do diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb index 50bf15cfc8c..e749ea6eacc 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -55,7 +55,7 @@ describe CustomIssueTrackerService do { project_url: url, issues_url: url, new_issue_url: url } end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { access_params.merge(title: title, description: description) } let(:service) do diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 2dc0b67239c..defebcee9c6 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -58,7 +58,7 @@ describe GitlabIssueTrackerService do { project_url: url, issues_url: url, new_issue_url: url } end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { access_params.merge(title: title, description: description) } let(:service) do diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index b86ab8959a2..5feb8ca7839 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -15,26 +15,53 @@ describe JiraService do let(:transition_id) { 'test27' } describe '#options' do - let(:service) do - described_class.create( + let(:options) do + { project: create(:project), active: true, username: 'username', password: 'test', jira_issue_transition_id: 24, url: 'http://jira.test.com/path/' - ) + } end + let(:service) { described_class.create(options) } + it 'sets the URL properly' do - # jira-ruby gem parses the URI and handles trailing slashes - # fine: https://github.com/sumoheavy/jira-ruby/blob/v1.4.1/lib/jira/http_client.rb#L59 + # jira-ruby gem parses the URI and handles trailing slashes fine: + # https://github.com/sumoheavy/jira-ruby/blob/v1.7.0/lib/jira/http_client.rb#L62 expect(service.options[:site]).to eq('http://jira.test.com/') end it 'leaves out trailing slashes in context' do expect(service.options[:context_path]).to eq('/path') end + + context 'username with trailing whitespaces' do + before do + options.merge!(username: 'username ') + end + + it 'leaves out trailing whitespaces in username' do + expect(service.options[:username]).to eq('username') + end + end + + it 'provides additional cookies to allow basic auth with oracle webgate' do + expect(service.options[:use_cookies]).to eq(true) + expect(service.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog']) + end + + context 'using api URL' do + before do + options.merge!(api_url: 'http://jira.test.com/api_path/') + end + + it 'leaves out trailing slashes in context' do + expect(service.options[:context_path]).to eq('/api_path') + end + end end describe 'Associations' do @@ -68,7 +95,7 @@ describe JiraService do expect(subject.properties).to be_nil end - it 'sets title correctly' do + it 'sets title correctly' do service = subject expect(service.title).to eq('custom title') @@ -93,7 +120,7 @@ describe JiraService do end # we need to make sure we are able to read both from properties and jira_tracker_data table - # TODO: change this as part of #63084 + # TODO: change this as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'overriding properties' do let(:access_params) do { url: url, api_url: api_url, username: username, password: password, @@ -278,11 +305,11 @@ describe JiraService do end end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { data_params.merge(title: title, description: description) } let!(:service) do - create(:jira_service, :without_properties_callback, properties: properties) + create(:jira_service, :without_properties_callback, properties: properties.merge(additional: 'something')) end it_behaves_like 'issue tracker fields' @@ -604,26 +631,6 @@ describe JiraService do end end - describe 'additional cookies' do - let(:project) { create(:project) } - - context 'provides additional cookies to allow basic auth with oracle webgate' do - before do - @service = project.create_jira_service( - active: true, properties: { url: 'http://jira.com' }) - end - - after do - @service.destroy! - end - - it 'is initialized' do - expect(@service.options[:use_cookies]).to eq(true) - expect(@service.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog']) - end - end - end - describe 'project and issue urls' do context 'when gitlab.yml was initialized' do it 'is prepopulated with the settings' do @@ -650,7 +657,7 @@ describe JiraService do end end - describe 'favicon urls', :request_store do + describe 'favicon urls' do it 'includes the standard favicon' do props = described_class.new.send(:build_remote_link_props, url: 'http://example.com', title: 'title') expect(props[:object][:icon][:url16x16]).to match %r{^http://localhost/assets/favicon(?:-\h+).png$} diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index 2339c5a8421..6220d7b1fac 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -57,7 +57,7 @@ describe RedmineService do { project_url: url, issues_url: url, new_issue_url: url } end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { access_params.merge(title: title, description: description) } let(:service) do diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index fe608baf16b..19d4cb95315 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -45,7 +45,7 @@ describe YoutrackService do { project_url: url, issues_url: url, new_issue_url: url } end - # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084 + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { access_params.merge(title: title, description: description) } let(:service) do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 7fe48e66def..9f3313e67b5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -92,6 +92,7 @@ describe Project do it { is_expected.to have_many(:pipeline_schedules) } it { is_expected.to have_many(:members_and_requesters) } it { is_expected.to have_many(:clusters) } + it { is_expected.to have_many(:management_clusters).class_name('Clusters::Cluster') } it { is_expected.to have_many(:kubernetes_namespaces) } it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') } it { is_expected.to have_many(:project_badges).class_name('ProjectBadge') } @@ -100,6 +101,8 @@ describe Project do it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } it { is_expected.to have_many(:cycle_analytics_stages) } it { is_expected.to have_many(:external_pull_requests) } + it { is_expected.to have_many(:sourced_pipelines) } + it { is_expected.to have_many(:source_pipelines) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) @@ -132,6 +135,13 @@ describe Project do expect(project.ci_cd_settings).to be_an_instance_of(ProjectCiCdSetting) expect(project.ci_cd_settings).to be_persisted end + + it 'automatically creates a Pages metadata row' do + project = create(:project) + + expect(project.pages_metadatum).to be_an_instance_of(ProjectPagesMetadatum) + expect(project.pages_metadatum).to be_persisted + end end context 'updating cd_cd_settings' do @@ -143,7 +153,7 @@ describe Project do end describe '#members & #requesters' do - let(:project) { create(:project, :public, :access_requestable) } + let(:project) { create(:project, :public) } let(:requester) { create(:user) } let(:developer) { create(:user) } before do @@ -621,8 +631,38 @@ describe Project do describe "#web_url" do let(:project) { create(:project, path: "somewhere") } - it 'returns the full web URL for this repo' do - expect(project.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere") + context 'when given the only_path option' do + subject { project.web_url(only_path: only_path) } + + context 'when only_path is false' do + let(:only_path) { false } + + it 'returns the full web URL for this repo' do + expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere") + end + end + + context 'when only_path is true' do + let(:only_path) { true } + + it 'returns the relative web URL for this repo' do + expect(subject).to eq("/#{project.namespace.full_path}/somewhere") + end + end + + context 'when only_path is nil' do + let(:only_path) { nil } + + it 'returns the full web URL for this repo' do + expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere") + end + end + end + + context 'when not given the only_path option' do + it 'returns the full web URL for this repo' do + expect(project.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere") + end end end @@ -2380,29 +2420,6 @@ describe Project do expect(project.emails_disabled?).to be_truthy end end - - context 'when :emails_disabled feature flag is off' do - before do - stub_feature_flags(emails_disabled: false) - end - - context 'emails disabled in group' do - it 'returns false' do - allow(project.namespace).to receive(:emails_disabled?) { true } - - expect(project.emails_disabled?).to be_falsey - end - end - - context 'emails enabled in group' do - it 'returns false' do - allow(project.namespace).to receive(:emails_disabled?) { false } - project.update_attribute(:emails_disabled, true) - - expect(project.emails_disabled?).to be_falsey - end - end - end end describe '#lfs_enabled?' do @@ -3239,20 +3256,78 @@ describe Project do describe '#http_url_to_repo' do let(:project) { create(:project) } - it 'returns the url to the repo without a username' do - expect(project.http_url_to_repo).to eq("#{project.web_url}.git") - expect(project.http_url_to_repo).not_to include('@') + context 'when a custom HTTP clone URL root is not set' do + it 'returns the url to the repo without a username' do + expect(project.http_url_to_repo).to eq("#{project.web_url}.git") + expect(project.http_url_to_repo).not_to include('@') + end + end + + context 'when a custom HTTP clone URL root is set' do + before do + stub_application_setting(custom_http_clone_url_root: custom_http_clone_url_root) + end + + context 'when custom HTTP clone URL root has a relative URL root' do + context 'when custom HTTP clone URL root ends with a slash' do + let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab/' } + + it 'returns the url to the repo, with the root replaced with the custom one' do + expect(project.http_url_to_repo).to eq("https://git.example.com:51234/mygitlab/#{project.full_path}.git") + end + end + + context 'when custom HTTP clone URL root does not end with a slash' do + let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab' } + + it 'returns the url to the repo, with the root replaced with the custom one' do + expect(project.http_url_to_repo).to eq("https://git.example.com:51234/mygitlab/#{project.full_path}.git") + end + end + end + + context 'when custom HTTP clone URL root does not have a relative URL root' do + context 'when custom HTTP clone URL root ends with a slash' do + let(:custom_http_clone_url_root) { 'https://git.example.com:51234/' } + + it 'returns the url to the repo, with the root replaced with the custom one' do + expect(project.http_url_to_repo).to eq("https://git.example.com:51234/#{project.full_path}.git") + end + end + + context 'when custom HTTP clone URL root does not end with a slash' do + let(:custom_http_clone_url_root) { 'https://git.example.com:51234' } + + it 'returns the url to the repo, with the root replaced with the custom one' do + expect(project.http_url_to_repo).to eq("https://git.example.com:51234/#{project.full_path}.git") + end + end + end end end describe '#lfs_http_url_to_repo' do let(:project) { create(:project) } - it 'returns the url to the repo without a username' do - lfs_http_url_to_repo = project.lfs_http_url_to_repo('operation_that_doesnt_matter') + context 'when a custom HTTP clone URL root is not set' do + it 'returns the url to the repo without a username' do + lfs_http_url_to_repo = project.lfs_http_url_to_repo('operation_that_doesnt_matter') + + expect(lfs_http_url_to_repo).to eq("#{project.web_url}.git") + expect(lfs_http_url_to_repo).not_to include('@') + end + end + + context 'when a custom HTTP clone URL root is set' do + before do + stub_application_setting(custom_http_clone_url_root: 'https://git.example.com:51234') + end + + it 'returns the url to the repo, with the root replaced with the custom one' do + lfs_http_url_to_repo = project.lfs_http_url_to_repo('operation_that_doesnt_matter') - expect(lfs_http_url_to_repo).to eq("#{project.web_url}.git") - expect(lfs_http_url_to_repo).not_to include('@') + expect(lfs_http_url_to_repo).to eq("https://git.example.com:51234/#{project.full_path}.git") + end end end @@ -3556,7 +3631,8 @@ describe Project do end describe '#remove_pages' do - let(:project) { create(:project) } + let(:project) { create(:project).tap { |project| project.mark_pages_as_deployed } } + let(:pages_metadatum) { project.pages_metadatum } let(:namespace) { project.namespace } let(:pages_path) { project.pages_path } @@ -3569,12 +3645,12 @@ describe Project do end end - it 'removes the pages directory' do + it 'removes the pages directory and marks the project as not having pages deployed' do expect_any_instance_of(Projects::UpdatePagesConfigurationService).to receive(:execute) expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true) expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything) - project.remove_pages + expect { project.remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false) end it 'is a no-op when there is no namespace' do @@ -3584,13 +3660,13 @@ describe Project do expect_any_instance_of(Projects::UpdatePagesConfigurationService).not_to receive(:execute) expect_any_instance_of(Gitlab::PagesTransfer).not_to receive(:rename_project) - project.remove_pages + expect { project.remove_pages }.not_to change { pages_metadatum.reload.deployed } end it 'is run when the project is destroyed' do expect(project).to receive(:remove_pages).and_call_original - project.destroy + expect { project.destroy }.not_to raise_error end end @@ -3663,14 +3739,6 @@ describe Project do end end - describe '#ensure_storage_path_exists' do - it 'delegates to gitlab_shell to ensure namespace is created' do - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, project.base_dir) - - project.ensure_storage_path_exists - end - end - describe '#legacy_storage?' do it 'returns true when storage_version is nil' do project = build(:project, storage_version: nil) @@ -3785,16 +3853,6 @@ describe Project do end end - describe '#ensure_storage_path_exists' do - it 'delegates to gitlab_shell to ensure namespace is created' do - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, hashed_prefix) - - project.ensure_storage_path_exists - end - end - describe '#pages_path' do it 'returns a path where pages are stored' do expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) @@ -4225,13 +4283,24 @@ describe Project do end describe '#check_repository_path_availability' do - let(:project) { build(:project) } + let(:project) { build(:project, :repository, :legacy_storage) } + subject { project.check_repository_path_availability } - it 'skips gitlab-shell exists?' do - project.skip_disk_validation = true + context 'when the repository already exists' do + let(:project) { create(:project, :repository, :legacy_storage) } - expect(project.gitlab_shell).not_to receive(:exists?) - expect(project.check_repository_path_availability).to be_truthy + it { is_expected.to be_falsey } + end + + context 'when the repository does not exist' do + it { is_expected.to be_truthy } + + it 'skips gitlab-shell exists?' do + project.skip_disk_validation = true + + expect(project.gitlab_shell).not_to receive(:repository_exists?) + is_expected.to be_truthy + end end end @@ -4945,6 +5014,7 @@ describe Project do describe '#git_objects_poolable?' do subject { project } + context 'when not using hashed storage' do let(:project) { create(:project, :legacy_storage, :public, :repository) } @@ -5044,6 +5114,35 @@ describe Project do end end + context 'pages deployed' do + let(:project) { create(:project) } + + { + mark_pages_as_deployed: true, + mark_pages_as_not_deployed: false + }.each do |method_name, flag| + describe method_name do + it "creates new record and sets deployed to #{flag} if none exists yet" do + project.pages_metadatum.destroy! + project.reload + + project.send(method_name) + + expect(project.pages_metadatum.reload.deployed).to eq(flag) + end + + it "updates the existing record and sets deployed to #{flag}" do + pages_metadatum = project.pages_metadatum + pages_metadatum.update!(deployed: !flag) + + expect { project.send(method_name) }.to change { + pages_metadatum.reload.deployed + }.from(!flag).to(flag) + end + end + end + end + describe '#has_pool_repsitory?' do it 'returns false when it does not have a pool repository' do subject = create(:project, :repository) @@ -5084,9 +5183,146 @@ describe Project do let(:project) { build(:project) } it 'returns instance of Pages::LookupPath' do - expect(Pages::LookupPath).to receive(:new).with(project, domain: pages_domain).and_call_original + expect(Pages::LookupPath).to receive(:new).with(project, domain: pages_domain, trim_prefix: 'mygroup').and_call_original + + expect(project.pages_lookup_path(domain: pages_domain, trim_prefix: 'mygroup')).to be_a(Pages::LookupPath) + end + end + + describe '.with_pages_deployed' do + it 'returns only projects that have pages deployed' do + _project_without_pages = create(:project) + project_with_pages = create(:project) + project_with_pages.mark_pages_as_deployed + + expect(described_class.with_pages_deployed).to contain_exactly(project_with_pages) + end + end + + describe '.pages_metadata_not_migrated' do + it 'returns only projects that have pages deployed' do + _project_with_pages_metadata_migrated = create(:project) + project_with_pages_metadata_not_migrated = create(:project) + project_with_pages_metadata_not_migrated.pages_metadatum.destroy! + + expect(described_class.pages_metadata_not_migrated).to contain_exactly(project_with_pages_metadata_not_migrated) + end + end + + describe '#pages_group_root?' do + it 'returns returns true if pages_url is same as pages_group_url' do + project = build(:project) + expect(project).to receive(:pages_url).and_return(project.pages_group_url) + + expect(project.pages_group_root?).to eq(true) + end + + it 'returns returns false if pages_url is different than pages_group_url' do + project = build(:project) + + expect(project.pages_group_root?).to eq(false) + end + end + + describe '#closest_setting' do + using RSpec::Parameterized::TableSyntax + + shared_examples_for 'fetching closest setting' do + let!(:namespace) { create(:namespace) } + let!(:project) { create(:project, namespace: namespace) } + + let(:setting_name) { :some_setting } + let(:setting) { project.closest_setting(setting_name) } - expect(project.pages_lookup_path(domain: pages_domain)).to be_a(Pages::LookupPath) + before do + allow(project).to receive(:read_attribute).with(setting_name).and_return(project_setting) + allow(namespace).to receive(:closest_setting).with(setting_name).and_return(group_setting) + allow(Gitlab::CurrentSettings).to receive(setting_name).and_return(global_setting) + end + + it 'returns closest non-nil value' do + expect(setting).to eq(result) + end + end + + context 'when setting is of non-boolean type' do + where(:global_setting, :group_setting, :project_setting, :result) do + 100 | 200 | 300 | 300 + 100 | 200 | nil | 200 + 100 | nil | nil | 100 + nil | nil | nil | nil + end + + with_them do + it_behaves_like 'fetching closest setting' + end + end + + context 'when setting is of boolean type' do + where(:global_setting, :group_setting, :project_setting, :result) do + true | true | false | false + true | false | nil | false + true | nil | nil | true + end + + with_them do + it_behaves_like 'fetching closest setting' + end + end + end + + describe '#drop_visibility_level!' do + context 'when has a group' do + let(:group) { create(:group, visibility_level: group_visibility_level) } + let(:project) { build(:project, namespace: group, visibility_level: project_visibility_level) } + + context 'when the group `visibility_level` is more strict' do + let(:group_visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + let(:project_visibility_level) { Gitlab::VisibilityLevel::INTERNAL } + + it 'sets `visibility_level` value from the group' do + expect { project.drop_visibility_level! } + .to change { project.visibility_level } + .to(Gitlab::VisibilityLevel::PRIVATE) + end + end + + context 'when the group `visibility_level` is less strict' do + let(:group_visibility_level) { Gitlab::VisibilityLevel::INTERNAL } + let(:project_visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + + it 'does not change the value of the `visibility_level` field' do + expect { project.drop_visibility_level! } + .not_to change { project.visibility_level } + end + end + end + + context 'when `restricted_visibility_levels` of the GitLab instance exist' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + end + + let(:project) { build(:project, visibility_level: project_visibility_level) } + + context 'when `visibility_level` is included into `restricted_visibility_levels`' do + let(:project_visibility_level) { Gitlab::VisibilityLevel::INTERNAL } + + it 'sets `visibility_level` value to `PRIVATE`' do + expect { project.drop_visibility_level! } + .to change { project.visibility_level } + .to(Gitlab::VisibilityLevel::PRIVATE) + end + end + + context 'when `restricted_visibility_levels` does not include `visibility_level`' do + let(:project_visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + + it 'does not change the value of the `visibility_level` field' do + expect { project.drop_visibility_level! } + .to not_change { project.visibility_level } + end + end end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 77c88a04cde..d62fa58739a 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -141,7 +141,7 @@ describe ProjectTeam do describe '#find_member' do context 'personal project' do let(:project) do - create(:project, :public, :access_requestable) + create(:project, :public) end let(:requester) { create(:user) } @@ -161,7 +161,7 @@ describe ProjectTeam do end context 'group project' do - let(:group) { create(:group, :access_requestable) } + let(:group) { create(:group) } let(:project) { create(:project, group: group) } let(:requester) { create(:user) } @@ -246,7 +246,7 @@ describe ProjectTeam do context 'personal project' do let(:project) do - create(:project, :public, :access_requestable) + create(:project, :public) end context 'when project is not shared with group' do @@ -292,7 +292,7 @@ describe ProjectTeam do end context 'group project' do - let(:group) { create(:group, :access_requestable) } + let(:group) { create(:group) } let!(:project) do create(:project, group: group) end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index d12dd97bb9e..31d1d1fd7d1 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -47,11 +47,25 @@ describe ProjectWiki do describe "#http_url_to_repo" do let(:project) { create :project } - it 'returns the full http url to the repo' do - expected_url = "#{Gitlab.config.gitlab.url}/#{subject.full_path}.git" + context 'when a custom HTTP clone URL root is not set' do + it 'returns the full http url to the repo' do + expected_url = "#{Gitlab.config.gitlab.url}/#{subject.full_path}.git" - expect(project_wiki.http_url_to_repo).to eq(expected_url) - expect(project_wiki.http_url_to_repo).not_to include('@') + expect(project_wiki.http_url_to_repo).to eq(expected_url) + expect(project_wiki.http_url_to_repo).not_to include('@') + end + end + + context 'when a custom HTTP clone URL root is set' do + before do + stub_application_setting(custom_http_clone_url_root: 'https://git.example.com:51234') + end + + it 'returns the full http url to the repo, with the root replaced with the custom one' do + expected_url = "https://git.example.com:51234/#{subject.full_path}.git" + + expect(project_wiki.http_url_to_repo).to eq(expected_url) + end end end @@ -95,6 +109,7 @@ describe ProjectWiki do context "when the wiki repository is empty" do describe '#empty?' do subject { super().empty? } + it { is_expected.to be_truthy } end end @@ -107,6 +122,7 @@ describe ProjectWiki do describe '#empty?' do subject { super().empty? } + it { is_expected.to be_falsey } it 'only instantiates a Wiki page once' do diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 8714c67f29d..0aac325c2b2 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -15,12 +15,13 @@ RSpec.describe Release do it { is_expected.to have_many(:links).class_name('Releases::Link') } it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:milestone_releases) } + it { is_expected.to have_one(:evidence) } end describe 'validation' do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:description) } - it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:tag) } context 'when a release exists in the database without a name' do it 'does not require name' do @@ -90,4 +91,42 @@ RSpec.describe Release do end end end + + describe 'evidence' do + describe '#create_evidence!' do + context 'when a release is created' do + it 'creates one Evidence object too' do + expect { release }.to change(Evidence, :count).by(1) + end + end + end + + context 'when a release is deleted' do + it 'also deletes the associated evidence' do + release = create(:release) + + expect { release.destroy }.to change(Evidence, :count).by(-1) + end + end + end + + describe '#notify_new_release' do + context 'when a release is created' do + it 'instantiates NewReleaseWorker to send notifications' do + expect(NewReleaseWorker).to receive(:perform_async) + + create(:release) + end + end + + context 'when a release is updated' do + let!(:release) { create(:release) } + + it 'does not send any new notification' do + expect(NewReleaseWorker).not_to receive(:perform_async) + + release.update!(description: 'new description') + end + end + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 6dc47e0e501..cf9100eb6cf 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -279,7 +279,7 @@ describe Repository do describe '#commits' do context 'when neither the all flag nor a ref are specified' do it 'returns every commit from default branch' do - expect(repository.commits(limit: 60).size).to eq(37) + expect(repository.commits(nil, limit: 60).size).to eq(37) end end @@ -320,7 +320,7 @@ describe Repository do context "when 'all' flag is set" do it 'returns every commit from the repository' do - expect(repository.commits(all: true, limit: 60).size).to eq(60) + expect(repository.commits(nil, all: true, limit: 60).size).to eq(60) end end end @@ -1075,7 +1075,7 @@ describe Repository do let(:ref) { 'refs/heads/master' } it 'returns nil' do - is_expected.to eq(nil) + is_expected.to be_nil end end @@ -1193,66 +1193,84 @@ describe Repository do end describe '#has_visible_content?' do - before do - # If raw_repository.has_visible_content? gets called more than once then - # caching is broken. We don't want that. + it 'delegates to raw_repository when true' do expect(repository.raw_repository).to receive(:has_visible_content?) - .once - .and_return(result) - end - - context 'when true' do - let(:result) { true } + .and_return(true) - it 'returns true and caches it' do - expect(repository.has_visible_content?).to eq(true) - # Second call hits the cache - expect(repository.has_visible_content?).to eq(true) - end + expect(repository.has_visible_content?).to eq(true) end - context 'when false' do - let(:result) { false } + it 'delegates to raw_repository when false' do + expect(repository.raw_repository).to receive(:has_visible_content?) + .and_return(false) - it 'returns false and caches it' do - expect(repository.has_visible_content?).to eq(false) - # Second call hits the cache - expect(repository.has_visible_content?).to eq(false) - end + expect(repository.has_visible_content?).to eq(false) end + + it_behaves_like 'asymmetric cached method', :has_visible_content? end describe '#branch_exists?' do - it 'uses branch_names' do - allow(repository).to receive(:branch_names).and_return(['foobar']) + let(:branch) { repository.root_ref } + + subject { repository.branch_exists?(branch) } + + it 'delegates to branch_names when the cache is empty' do + repository.expire_branches_cache + + expect(repository).to receive(:branch_names).and_call_original + is_expected.to eq(true) + end + + it 'uses redis set caching when the cache is filled' do + repository.branch_names # ensure the branch name cache is filled + + expect(repository) + .to receive(:branch_names_include?) + .with(branch) + .and_call_original - expect(repository.branch_exists?('foobar')).to eq(true) - expect(repository.branch_exists?('master')).to eq(false) + is_expected.to eq(true) end end describe '#tag_exists?' do - it 'uses tag_names' do - allow(repository).to receive(:tag_names).and_return(['foobar']) + let(:tag) { repository.tags.first.name } - expect(repository.tag_exists?('foobar')).to eq(true) - expect(repository.tag_exists?('master')).to eq(false) + subject { repository.tag_exists?(tag) } + + it 'delegates to tag_names when the cache is empty' do + repository.expire_tags_cache + + expect(repository).to receive(:tag_names).and_call_original + is_expected.to eq(true) + end + + it 'uses redis set caching when the cache is filled' do + repository.tag_names # ensure the tag name cache is filled + + expect(repository) + .to receive(:tag_names_include?) + .with(tag) + .and_call_original + + is_expected.to eq(true) end end - describe '#branch_names', :use_clean_rails_memory_store_caching do + describe '#branch_names', :clean_gitlab_redis_cache do let(:fake_branch_names) { ['foobar'] } it 'gets cached across Repository instances' do allow(repository.raw_repository).to receive(:branch_names).once.and_return(fake_branch_names) - expect(repository.branch_names).to eq(fake_branch_names) + expect(repository.branch_names).to match_array(fake_branch_names) fresh_repository = Project.find(project.id).repository expect(fresh_repository.object_id).not_to eq(repository.object_id) expect(fresh_repository.raw_repository).not_to receive(:branch_names) - expect(fresh_repository.branch_names).to eq(fake_branch_names) + expect(fresh_repository.branch_names).to match_array(fake_branch_names) end end @@ -1972,7 +1990,7 @@ describe Repository do it 'returns nil if repo does not exist' do allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) - expect(repository.avatar).to eq(nil) + expect(repository.avatar).to be_nil end it 'returns the first avatar file found in the repository' do @@ -2574,6 +2592,10 @@ describe Repository do expect { repository.create_if_not_exists }.to change { repository.exists? }.from(false).to(true) end + it 'returns true' do + expect(repository.create_if_not_exists).to eq(true) + end + it 'calls out to the repository client to create a repo' do expect(repository.raw.gitaly_repository_client).to receive(:create_repository) @@ -2588,6 +2610,10 @@ describe Repository do repository.create_if_not_exists end + + it 'returns nil' do + expect(repository.create_if_not_exists).to be_nil + end end context 'when the repository exists but the cache is not up to date' do @@ -2599,6 +2625,10 @@ describe Repository do expect { repository.create_if_not_exists }.not_to raise_error end + + it 'returns nil' do + expect(repository.create_if_not_exists).to be_nil + end end end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index f4023dcb95a..f51041c9ddc 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe ResourceLabelEvent, type: :model do subject { build(:resource_label_event, issue: issue) } + let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index d96e1398677..64077b76f01 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -78,10 +78,11 @@ describe Service do end describe "Template" do + let(:project) { create(:project) } + describe '.build_from_template' do context 'when template is invalid' do it 'sets service template to inactive when template is invalid' do - project = create(:project) template = build(:prometheus_service, template: true, active: true, properties: {}) template.save(validate: false) @@ -91,6 +92,64 @@ describe Service do expect(service.active).to be false end end + + describe 'build issue tracker from a template' do + let(:title) { 'custom title' } + let(:description) { 'custom description' } + let(:url) { 'http://jira.example.com' } + let(:api_url) { 'http://api-jira.example.com' } + let(:username) { 'jira-username' } + let(:password) { 'jira-password' } + let(:data_params) do + { + url: url, api_url: api_url, + username: username, password: password + } + end + + shared_examples 'service creation from a template' do + it 'creates a correct service' do + service = described_class.build_from_template(project.id, template) + + expect(service).to be_active + expect(service.title).to eq(title) + expect(service.description).to eq(description) + expect(service.url).to eq(url) + expect(service.api_url).to eq(api_url) + expect(service.username).to eq(username) + expect(service.password).to eq(password) + end + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 + context 'when data are stored in properties' do + let(:properties) { data_params.merge(title: title, description: description) } + let!(:template) do + create(:jira_service, :without_properties_callback, template: true, properties: properties.merge(additional: 'something')) + end + + it_behaves_like 'service creation from a template' + end + + context 'when data are stored in separated fields' do + let(:template) do + create(:jira_service, data_params.merge(properties: {}, title: title, description: description, template: true)) + end + + it_behaves_like 'service creation from a template' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { data_params.merge(title: title, description: description) } + let(:template) do + create(:jira_service, :without_properties_callback, active: true, template: true, properties: properties).tap do |service| + create(:jira_tracker_data, data_params.merge(service: service)) + end + end + + it_behaves_like 'service creation from a template' + end + end end describe "for pushover service" do @@ -104,7 +163,6 @@ describe Service do api_key: '123456789' }) end - let(:project) { create(:project) } describe 'is prefilled for projects pushover service' do it "has all fields prefilled" do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 3524cdae3b8..f4dcbfbc190 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -133,6 +133,32 @@ describe Snippet do end end + describe 'when default snippet visibility set to internal' do + using RSpec::Parameterized::TableSyntax + + before do + stub_application_setting(default_snippet_visibility: Gitlab::VisibilityLevel::INTERNAL) + end + + where(:attribute_name, :value) do + :visibility | 'private' + :visibility_level | Gitlab::VisibilityLevel::PRIVATE + 'visibility' | 'private' + 'visibility_level' | Gitlab::VisibilityLevel::PRIVATE + end + + with_them do + it 'sets the visibility level' do + snippet = described_class.new(attribute_name => value, title: 'test', file_name: 'test.rb', content: 'test data') + + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(snippet.title).to eq('test') + expect(snippet.file_name).to eq('test.rb') + expect(snippet.content).to eq('test data') + end + end + end + describe '.with_optional_visibility' do context 'when a visibility level is provided' do it 'returns snippets with the given visibility' do @@ -157,12 +183,12 @@ describe Snippet do end end - describe '.only_global_snippets' do + describe '.only_personal_snippets' do it 'returns snippets not associated with any projects' do create(:project_snippet) snippet = create(:snippet) - snippets = described_class.only_global_snippets + snippets = described_class.only_personal_snippets expect(snippets).to eq([snippet]) end diff --git a/spec/models/suggestion_spec.rb b/spec/models/suggestion_spec.rb index 8d4e9070b19..2ac3ae0a5ad 100644 --- a/spec/models/suggestion_spec.rb +++ b/spec/models/suggestion_spec.rb @@ -38,16 +38,6 @@ describe Suggestion do end describe '#appliable?' do - context 'when note does not support suggestions' do - it 'returns false' do - expect_next_instance_of(DiffNote) do |note| - allow(note).to receive(:supports_suggestion?) { false } - end - - expect(suggestion).not_to be_appliable - end - end - context 'when patch is already applied' do let(:suggestion) { create(:suggestion, :applied) } diff --git a/spec/models/system_note_metadata_spec.rb b/spec/models/system_note_metadata_spec.rb index bcd3c03f947..801f139355b 100644 --- a/spec/models/system_note_metadata_spec.rb +++ b/spec/models/system_note_metadata_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe SystemNoteMetadata do describe 'associations' do it { is_expected.to belong_to(:note) } + it { is_expected.to belong_to(:description_version) } end describe 'validation' do diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 28fc82f2a32..7321a458817 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Timelog do subject { build(:timelog) } + let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index c2566ccd047..487a1c619c6 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -253,14 +253,14 @@ describe Todo do end end - describe '.for_group_and_descendants' do + describe '.for_group_ids_and_descendants' do it 'returns the todos for a group and its descendants' do parent_group = create(:group) child_group = create(:group, parent: parent_group) todo1 = create(:todo, group: parent_group) todo2 = create(:todo, group: child_group) - todos = described_class.for_group_and_descendants(parent_group) + todos = described_class.for_group_ids_and_descendants([parent_group.id]) expect(todos).to contain_exactly(todo1, todo2) end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index d97bb8cfb90..03434c95218 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Upload do - describe 'assocations' do + describe 'associations' do it { is_expected.to belong_to(:model) } end @@ -107,6 +107,52 @@ describe Upload do end end + describe '#build_uploader' do + it 'returns a uploader object with current upload associated with it' do + subject = build(:upload) + uploader = subject.build_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.mounted_as).to eq(subject.send(:mount_point)) + expect(uploader.file).to be_nil + end + end + + describe '#retrieve_uploader' do + it 'returns a uploader object with current uploader associated with and cache retrieved' do + subject = build(:upload) + uploader = subject.retrieve_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.mounted_as).to eq(subject.send(:mount_point)) + expect(uploader.file).not_to be_nil + end + end + + describe '#needs_checksum?' do + context 'with local storage' do + it 'returns true when no checksum exists' do + subject = create(:upload, :with_file, checksum: nil) + + expect(subject.needs_checksum?).to be_truthy + end + + it 'returns false when checksum is already present' do + subject = create(:upload, :with_file, checksum: 'something') + + expect(subject.needs_checksum?).to be_falsey + end + end + + context 'with remote storage' do + subject { build(:upload, :object_storage) } + + it 'returns false' do + expect(subject.needs_checksum?).to be_falsey + end + end + end + describe '#exist?' do it 'returns true when the file exists' do upload = described_class.new(path: __FILE__, store: ObjectStorage::Store::LOCAL) diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb index 4a44cf5ab0f..b93d9449da9 100644 --- a/spec/models/uploads/fog_spec.rb +++ b/spec/models/uploads/fog_spec.rb @@ -44,7 +44,7 @@ describe Uploads::Fog do subject { data_store.delete_keys(keys) } before do - uploads.each { |upload| upload.build_uploader.migrate!(2) } + uploads.each { |upload| upload.retrieve_uploader.migrate!(2) } end it 'deletes multiple data' do diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb index 47d919c1d12..b96ff08e22d 100644 --- a/spec/models/user_interacted_project_spec.rb +++ b/spec/models/user_interacted_project_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe UserInteractedProject do describe '.track' do subject { described_class.track(event) } + let(:event) { build(:event) } Event::ACTIONS.each do |action| diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a701b858783..8eb2f9b5bc0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -79,7 +79,7 @@ describe User do describe '#group_members' do it 'does not include group memberships for which user is a requester' do user = create(:user) - group = create(:group, :public, :access_requestable) + group = create(:group, :public) group.request_access(user) expect(user.group_members).to be_empty @@ -89,7 +89,7 @@ describe User do describe '#project_members' do it 'does not include project memberships for which user is a requester' do user = create(:user) - project = create(:project, :public, :access_requestable) + project = create(:project, :public) project.request_access(user) expect(user.project_members).to be_empty @@ -1120,6 +1120,30 @@ describe User do end end + describe 'deactivating a user' do + let(:user) { create(:user, name: 'John Smith') } + + context "an active user" do + it "can be deactivated" do + user.deactivate + + expect(user.deactivated?).to be_truthy + end + end + + context "a user who is blocked" do + before do + user.block + end + + it "cannot be deactivated" do + user.deactivate + + expect(user.reload.deactivated?).to be_falsy + end + end + end + describe '.filter_items' do let(:user) { double } @@ -1141,6 +1165,12 @@ describe User do expect(described_class.filter_items('blocked')).to include user end + it 'filters by deactivated' do + expect(described_class).to receive(:deactivated).and_return([user]) + + expect(described_class.filter_items('deactivated')).to include user + end + it 'filters by two_factor_disabled' do expect(described_class).to receive(:without_two_factor).and_return([user]) @@ -1161,7 +1191,7 @@ describe User do end describe '.without_projects' do - let!(:project) { create(:project, :public, :access_requestable) } + let!(:project) { create(:project, :public) } let!(:user) { create(:user) } let!(:user_without_project) { create(:user) } let!(:user_without_project2) { create(:user) } @@ -1427,10 +1457,18 @@ describe User do expect(described_class.search(user.username)).to eq([user, user2]) end + it 'returns users with a matching username starting with a @' do + expect(described_class.search("@#{user.username}")).to eq([user, user2]) + end + it 'returns users with a partially matching username' do expect(described_class.search(user.username[0..2])).to eq([user, user2]) end + it 'returns users with a partially matching username starting with @' do + expect(described_class.search("@#{user.username[0..2]}")).to eq([user, user2]) + end + it 'returns users with a matching username regardless of the casing' do expect(described_class.search(user2.username.upcase)).to eq([user2]) end @@ -1516,15 +1554,22 @@ describe User do end describe '.find_by_ssh_key_id' do - context 'using an existing SSH key ID' do - let(:user) { create(:user) } - let(:key) { create(:key, user: user) } + let_it_be(:user) { create(:user) } + let_it_be(:key) { create(:key, user: user) } + context 'using an existing SSH key ID' do it 'returns the corresponding User' do expect(described_class.find_by_ssh_key_id(key.id)).to eq(user) end end + it 'only performs a single query' do + key # Don't count the queries for creating the key and user + + expect { described_class.find_by_ssh_key_id(key.id) } + .not_to exceed_query_limit(1) + end + context 'using an invalid SSH key ID' do it 'returns nil' do expect(described_class.find_by_ssh_key_id(-1)).to be_nil @@ -2034,8 +2079,98 @@ describe User do end end + describe "#last_active_at" do + let(:last_activity_on) { 5.days.ago.to_date } + let(:current_sign_in_at) { 8.days.ago } + + context 'for a user that has `last_activity_on` set' do + let(:user) { create(:user, last_activity_on: last_activity_on) } + + it 'returns `last_activity_on` with current time zone' do + expect(user.last_active_at).to eq(last_activity_on.to_time.in_time_zone) + end + end + + context 'for a user that has `current_sign_in_at` set' do + let(:user) { create(:user, current_sign_in_at: current_sign_in_at) } + + it 'returns `current_sign_in_at`' do + expect(user.last_active_at).to eq(current_sign_in_at) + end + end + + context 'for a user that has both `current_sign_in_at` & ``last_activity_on`` set' do + let(:user) { create(:user, current_sign_in_at: current_sign_in_at, last_activity_on: last_activity_on) } + + it 'returns the latest among `current_sign_in_at` & `last_activity_on`' do + latest_event = [current_sign_in_at, last_activity_on.to_time.in_time_zone].max + expect(user.last_active_at).to eq(latest_event) + end + end + + context 'for a user that does not have both `current_sign_in_at` & `last_activity_on` set' do + let(:user) { create(:user, current_sign_in_at: nil, last_activity_on: nil) } + + it 'returns nil' do + expect(user.last_active_at).to eq(nil) + end + end + end + + 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 } + + shared_examples 'not eligible for deactivation' do + it 'returns false' do + expect(user.can_be_deactivated?).to be_falsey + end + end + + shared_examples 'eligible for deactivation' do + it 'returns true' do + expect(user.can_be_deactivated?).to be_truthy + end + end + + context "a user who is not active" do + before do + user.block + end + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has activity within the specified minimum inactive days' do + let(:activity) { { last_activity_on: day_within_minium_inactive_days_threshold } } + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has signed in within the specified minimum inactive days' do + let(:activity) { { current_sign_in_at: day_within_minium_inactive_days_threshold } } + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has no activity within the specified minimum inactive days' do + let(:activity) { { last_activity_on: day_outside_minium_inactive_days_threshold } } + + it_behaves_like 'eligible for deactivation' + end + + context 'a user who has not signed in within the specified minimum inactive days' do + let(:activity) { { current_sign_in_at: day_outside_minium_inactive_days_threshold } } + + it_behaves_like 'eligible for deactivation' + end + end + describe "#contributed_projects" do subject { create(:user) } + let!(:project1) { create(:project) } let!(:project2) { fork_project(project3) } let!(:project3) { create(:project) } @@ -3600,6 +3735,80 @@ describe User do end end + describe '#notification_settings_for' do + let(:user) { create(:user) } + let(:source) { nil } + + subject { user.notification_settings_for(source) } + + context 'when source is nil' do + it 'returns a blank global notification settings object' do + expect(subject.source).to eq(nil) + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + + context 'when source is a Group' do + let(:group) { create(:group) } + + subject { user.notification_settings_for(group, inherit: true) } + + context 'when group has no existing notification settings' do + context 'when group has no ancestors' do + it 'will be a default Global notification setting' do + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + + context 'when group has ancestors' do + context 'when an ancestor has a level other than Global' do + let(:ancestor) { create(:group) } + let(:group) { create(:group, parent: ancestor) } + + before do + create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com') + end + + it 'has the same level set' do + expect(subject.level).to eq('participating') + end + + it 'has the same email set' do + expect(subject.notification_email).to eq('ancestor@example.com') + end + + context 'when inherit is false' do + subject { user.notification_settings_for(group) } + + it 'does not inherit settings' do + expect(subject.notification_email).to eq(nil) + expect(subject.level).to eq('global') + end + end + end + + context 'when an ancestor has a Global level but has an email set' do + let(:grand_ancestor) { create(:group) } + let(:ancestor) { create(:group, parent: grand_ancestor) } + let(:group) { create(:group, parent: ancestor) } + + before do + create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com') + create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com') + end + + it 'has the same email set' do + expect(subject.level).to eq('global') + expect(subject.notification_email).to eq('ancestor@example.com') + end + end + end + end + end + end + describe '#notification_email_for' do let(:user) { create(:user) } let(:group) { create(:group) } @@ -3632,4 +3841,34 @@ describe User do end end end + + describe '#password_expired?' do + let(:user) { build(:user, password_expires_at: password_expires_at) } + + subject { user.password_expired? } + + context 'when password_expires_at is not set' do + let(:password_expires_at) {} + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when password_expires_at is in the past' do + let(:password_expires_at) { 1.minute.ago } + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when password_expires_at is in the future' do + let(:password_expires_at) { 1.minute.from_now } + + it 'returns false' do + is_expected.to be_falsey + end + end + end end |