diff options
Diffstat (limited to 'spec/models')
51 files changed, 1924 insertions, 305 deletions
diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 77b07cf1ac9..35415030154 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -20,7 +20,7 @@ describe Appearance do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', false do + it_behaves_like 'model with uploads', false do let(:model_object) { create(:appearance, :with_logo) } let(:upload_attribute) { :logo } let(:uploader_class) { AttachmentUploader } diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index ed93f94d893..e8c03b587e2 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -18,6 +18,7 @@ describe Blob do describe '.lazy' do let(:project) { create(:project, :repository) } + let(:same_project) { Project.find(project.id) } let(:other_project) { create(:project, :repository) } let(:commit_id) { 'e63f41fe459e62e1228fcef60d7189127aeba95a' } @@ -32,7 +33,7 @@ describe Blob do expect(other_project.repository).not_to receive(:blobs_at) changelog = described_class.lazy(project, commit_id, 'CHANGELOG') - contributing = described_class.lazy(project, commit_id, 'CONTRIBUTING.md') + contributing = described_class.lazy(same_project, commit_id, 'CONTRIBUTING.md') described_class.lazy(other_project, commit_id, 'CHANGELOG') diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 6dba132184c..519968b9e48 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -15,6 +15,8 @@ describe Ci::BuildMetadata do let(:build) { create(:ci_build, pipeline: pipeline) } let(:build_metadata) { build.metadata } + it_behaves_like 'having unique enum values' + describe '#update_timeout_state' do subject { build_metadata } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d02c3a5765f..89f78f629d4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -769,33 +769,15 @@ describe Ci::Build do let(:subject) { build.hide_secrets(data) } context 'hide runners token' do - let(:data) { 'new token data'} + let(:data) { "new #{project.runners_token} data"} - before do - build.project.update(runners_token: 'token') - end - - it { is_expected.to eq('new xxxxx data') } + it { is_expected.to match(/^new x+ data$/) } end context 'hide build token' do - let(:data) { 'new token data'} - - before do - build.update(token: 'token') - end - - it { is_expected.to eq('new xxxxx data') } - end - - context 'hide build token' do - let(:data) { 'new token data'} - - before do - build.update(token: 'token') - end + let(:data) { "new #{build.token} data"} - it { is_expected.to eq('new xxxxx data') } + it { is_expected.to match(/^new x+ data$/) } end end @@ -1943,7 +1925,7 @@ describe Ci::Build do context 'when token is empty' do before do - build.token = nil + build.update_columns(token: nil, token_encrypted: nil) end it { is_expected.to be_nil} @@ -2159,7 +2141,7 @@ describe Ci::Build do end before do - build.token = 'my-token' + build.set_token('my-token') build.yaml_variables = [] end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 59c861a74db..d214fdf369a 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -12,6 +12,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do described_class.new(build: build, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end + it_behaves_like 'having unique enum values' + before do stub_feature_flags(ci_enable_live_trace: true) stub_artifacts_object_storage @@ -436,32 +438,47 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:data_store) { :redis } context 'when data exists' do - let(:data) { 'Sample data in redis' } - before do build_trace_chunk.send(:unsafe_set_data!, data) end - it 'persists the data' do - expect(build_trace_chunk.redis?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil - expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + context 'when data size reached CHUNK_SIZE' do + let(:data) { 'a' * described_class::CHUNK_SIZE } - subject + it 'persists the data' do + expect(build_trace_chunk.redis?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + + subject + + expect(build_trace_chunk.fog?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + end - expect(build_trace_chunk.fog?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + it_behaves_like 'Atomic operation' end - it_behaves_like 'Atomic operation' + context 'when data size has not reached CHUNK_SIZE' do + let(:data) { 'Sample data in redis' } + + it 'does not persist the data and the orignal data is intact' do + expect { subject }.to raise_error(described_class::FailedToPersistDataError) + + expect(build_trace_chunk.redis?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + end + end end context 'when data does not exist' do it 'does not persist' do - expect { subject }.to raise_error('Can not persist empty data') + expect { subject }.to raise_error(described_class::FailedToPersistDataError) end end end @@ -470,32 +487,47 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:data_store) { :database } context 'when data exists' do - let(:data) { 'Sample data in database' } - before do build_trace_chunk.send(:unsafe_set_data!, data) end - it 'persists the data' do - expect(build_trace_chunk.database?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to eq(data) - expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + context 'when data size reached CHUNK_SIZE' do + let(:data) { 'a' * described_class::CHUNK_SIZE } - subject + it 'persists the data' do + expect(build_trace_chunk.database?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to eq(data) + expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + + subject + + expect(build_trace_chunk.fog?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + end - expect(build_trace_chunk.fog?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + it_behaves_like 'Atomic operation' end - it_behaves_like 'Atomic operation' + context 'when data size has not reached CHUNK_SIZE' do + let(:data) { 'Sample data in database' } + + it 'does not persist the data and the orignal data is intact' do + expect { subject }.to raise_error(described_class::FailedToPersistDataError) + + expect(build_trace_chunk.database?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to eq(data) + expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound) + end + end end context 'when data does not exist' do it 'does not persist' do - expect { subject }.to raise_error('Can not persist empty data') + expect { subject }.to raise_error(described_class::FailedToPersistDataError) end end end @@ -504,27 +536,37 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:data_store) { :fog } context 'when data exists' do - let(:data) { 'Sample data in fog' } - before do build_trace_chunk.send(:unsafe_set_data!, data) end - it 'does not change data store' do - expect(build_trace_chunk.fog?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + context 'when data size reached CHUNK_SIZE' do + let(:data) { 'a' * described_class::CHUNK_SIZE } - subject + it 'does not change data store' do + expect(build_trace_chunk.fog?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + + subject - expect(build_trace_chunk.fog?).to be_truthy - expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil - expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + expect(build_trace_chunk.fog?).to be_truthy + expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil + expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) + end + + it_behaves_like 'Atomic operation' end - it_behaves_like 'Atomic operation' + context 'when data size has not reached CHUNK_SIZE' do + let(:data) { 'Sample data in fog' } + + it 'does not raise error' do + expect { subject }.not_to raise_error + end + end end end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index fb5bec4108a..c68ba02b8de 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -15,6 +15,8 @@ describe Ci::JobArtifact do it { is_expected.to delegate_method(:open).to(:file) } it { is_expected.to delegate_method(:exists?).to(:file) } + it_behaves_like 'having unique enum values' + describe '.test_reports' do subject { described_class.test_reports } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 9e6146b8a44..b67c6a4cffa 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -8,10 +8,13 @@ describe Ci::Pipeline, :mailer do create(:ci_empty_pipeline, status: :created, project: project) end + it_behaves_like 'having unique enum values' + it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to belong_to(:pipeline_schedule) } + it { is_expected.to belong_to(:merge_request) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } @@ -30,8 +33,131 @@ describe Ci::Pipeline, :mailer do describe 'associations' do it 'has a bidirectional relationship with projects' do - expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:pipelines) - expect(Project.reflect_on_association(:pipelines).has_inverse?).to eq(:project) + expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:all_pipelines) + expect(Project.reflect_on_association(:all_pipelines).has_inverse?).to eq(:project) + expect(Project.reflect_on_association(:ci_pipelines).has_inverse?).to eq(:project) + end + end + + describe '.sort_by_merge_request_pipelines' do + subject { described_class.sort_by_merge_request_pipelines } + + context 'when branch pipelines exist' do + let!(:branch_pipeline_1) { create(:ci_pipeline, source: :push) } + let!(:branch_pipeline_2) { create(:ci_pipeline, source: :push) } + + it 'returns pipelines order by id' do + expect(subject).to eq([branch_pipeline_2, + branch_pipeline_1]) + end + end + + context 'when merge request pipelines exist' do + let!(:merge_request_pipeline_1) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let!(:merge_request_pipeline_2) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'returns pipelines order by id' do + expect(subject).to eq([merge_request_pipeline_2, + merge_request_pipeline_1]) + end + end + + context 'when both branch pipeline and merge request pipeline exist' do + let!(:branch_pipeline_1) { create(:ci_pipeline, source: :push) } + let!(:branch_pipeline_2) { create(:ci_pipeline, source: :push) } + + let!(:merge_request_pipeline_1) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let!(:merge_request_pipeline_2) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'returns merge request pipeline first' do + expect(subject).to eq([merge_request_pipeline_2, + merge_request_pipeline_1, + branch_pipeline_2, + branch_pipeline_1]) + end + end + end + + describe '.merge_request' do + subject { described_class.merge_request } + + context 'when there is a merge request pipeline' do + let!(:pipeline) { create(:ci_pipeline, source: :merge_request, merge_request: merge_request) } + let(:merge_request) { create(:merge_request) } + + it 'returns merge request pipeline first' do + expect(subject).to eq([pipeline]) + end + end + + context 'when there are no merge request pipelines' do + let!(:pipeline) { create(:ci_pipeline, source: :push) } + + it 'returns empty array' do + expect(subject).to be_empty + end + end + end + + describe 'Validations for merge request pipelines' do + let(:pipeline) { build(:ci_pipeline, source: source, merge_request: merge_request) } + + context 'when source is merge request' do + let(:source) { :merge_request } + + context 'when merge request is specified' do + let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_project: project, target_branch: 'master') } + + it { expect(pipeline).to be_valid } + end + + context 'when merge request is empty' do + let(:merge_request) { nil } + + it { expect(pipeline).not_to be_valid } + end + end + + context 'when source is web' do + let(:source) { :web } + + context 'when merge request is specified' do + let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_project: project, target_branch: 'master') } + + it { expect(pipeline).not_to be_valid } + end + + context 'when merge request is empty' do + let(:merge_request) { nil } + + it { expect(pipeline).to be_valid } + end end end @@ -224,6 +350,50 @@ describe Ci::Pipeline, :mailer do CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION] end + + context 'when source is merge request' do + let(:pipeline) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'exposes merge request pipeline variables' do + expect(subject.to_hash) + .to include( + 'CI_MERGE_REQUEST_ID' => merge_request.id.to_s, + 'CI_MERGE_REQUEST_IID' => merge_request.iid.to_s, + 'CI_MERGE_REQUEST_REF_PATH' => merge_request.ref_path.to_s, + 'CI_MERGE_REQUEST_PROJECT_ID' => merge_request.project.id.to_s, + 'CI_MERGE_REQUEST_PROJECT_PATH' => merge_request.project.full_path, + 'CI_MERGE_REQUEST_PROJECT_URL' => merge_request.project.web_url, + 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME' => merge_request.target_branch.to_s, + 'CI_MERGE_REQUEST_SOURCE_PROJECT_ID' => merge_request.source_project.id.to_s, + 'CI_MERGE_REQUEST_SOURCE_PROJECT_PATH' => merge_request.source_project.full_path, + 'CI_MERGE_REQUEST_SOURCE_PROJECT_URL' => merge_request.source_project.web_url, + 'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME' => merge_request.source_branch.to_s) + end + + context 'when source project does not exist' do + before do + merge_request.update_column(:source_project_id, nil) + end + + it 'does not expose source project related variables' do + expect(subject.to_hash.keys).not_to include( + %w[CI_MERGE_REQUEST_SOURCE_PROJECT_ID + CI_MERGE_REQUEST_SOURCE_PROJECT_PATH + CI_MERGE_REQUEST_SOURCE_PROJECT_URL + CI_MERGE_REQUEST_SOURCE_BRANCH_NAME]) + end + end + end end describe '#protected_ref?' do @@ -758,27 +928,85 @@ describe Ci::Pipeline, :mailer do describe '#branch?' do subject { pipeline.branch? } - context 'is not a tag' do + context 'when ref is not a tag' do before do pipeline.tag = false end - it 'return true when tag is set to false' do + it 'return true' do is_expected.to be_truthy end + + context 'when source is merge request' do + let(:pipeline) do + create(:ci_pipeline, source: :merge_request, merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'returns false' do + is_expected.to be_falsey + end + end end - context 'is not a tag' do + context 'when ref is a tag' do before do pipeline.tag = true end - it 'return false when tag is set to true' do + it 'return false' do is_expected.to be_falsey end end end + describe '#git_ref' do + subject { pipeline.send(:git_ref) } + + context 'when ref is branch' do + let(:pipeline) { create(:ci_pipeline, tag: false) } + + it 'returns branch ref' do + is_expected.to eq(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref.to_s) + end + end + + context 'when ref is tag' do + let(:pipeline) { create(:ci_pipeline, tag: true) } + + it 'returns branch ref' do + is_expected.to eq(Gitlab::Git::TAG_REF_PREFIX + pipeline.ref.to_s) + end + end + + context 'when ref is merge request' do + let(:pipeline) do + create(:ci_pipeline, + source: :merge_request, + merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master') + end + + it 'returns branch ref' do + is_expected.to eq(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref.to_s) + end + end + end + describe 'ref_exists?' do context 'when repository exists' do using RSpec::Parameterized::TableSyntax @@ -1003,7 +1231,7 @@ describe Ci::Pipeline, :mailer do create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop') create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop') - pipelines = project.pipelines.to_a + pipelines = project.ci_pipelines.to_a pipelines.each(&:number_of_warnings) @@ -1247,22 +1475,40 @@ describe Ci::Pipeline, :mailer do describe '#ci_yaml_file_path' do subject { pipeline.ci_yaml_file_path } - it 'returns the path from project' do - allow(pipeline.project).to receive(:ci_config_path) { 'custom/path' } + %i[unknown_source repository_source].each do |source| + context source.to_s do + before do + pipeline.config_source = described_class.config_sources.fetch(source) + end - is_expected.to eq('custom/path') - end + it 'returns the path from project' do + allow(pipeline.project).to receive(:ci_config_path) { 'custom/path' } + + is_expected.to eq('custom/path') + end + + it 'returns default when custom path is nil' do + allow(pipeline.project).to receive(:ci_config_path) { nil } + + is_expected.to eq('.gitlab-ci.yml') + end - it 'returns default when custom path is nil' do - allow(pipeline.project).to receive(:ci_config_path) { nil } + it 'returns default when custom path is empty' do + allow(pipeline.project).to receive(:ci_config_path) { '' } - is_expected.to eq('.gitlab-ci.yml') + is_expected.to eq('.gitlab-ci.yml') + end + end end - it 'returns default when custom path is empty' do - allow(pipeline.project).to receive(:ci_config_path) { '' } + context 'when pipeline is for auto-devops' do + before do + pipeline.config_source = 'auto_devops_source' + end - is_expected.to eq('.gitlab-ci.yml') + it 'does not return config file' do + is_expected.to be_nil + end end end @@ -1835,6 +2081,55 @@ describe Ci::Pipeline, :mailer do expect(pipeline.all_merge_requests).to be_empty end + + context 'when there is a merge request pipeline' do + let(:source_branch) { 'feature' } + let(:target_branch) { 'master' } + + let!(:pipeline) do + create(:ci_pipeline, + source: :merge_request, + project: project, + ref: source_branch, + merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: source_branch, + target_project: project, + target_branch: target_branch) + end + + it 'returns an associated merge request' do + expect(pipeline.all_merge_requests).to eq([merge_request]) + end + + context 'when there is another merge request pipeline that targets a different branch' do + let(:target_branch_2) { 'merge-test' } + + let!(:pipeline_2) do + create(:ci_pipeline, + source: :merge_request, + project: project, + ref: source_branch, + merge_request: merge_request_2) + end + + let(:merge_request_2) do + create(:merge_request, + source_project: project, + source_branch: source_branch, + target_project: project, + target_branch: target_branch_2) + end + + it 'does not return an associated merge request' do + expect(pipeline.all_merge_requests).not_to include(merge_request_2) + end + end + end end describe '#stuck?' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index b545e036aa1..ad79f8d4ce0 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Ci::Runner do + it_behaves_like 'having unique enum values' + describe 'validation' do it { is_expected.to validate_presence_of(:access_level) } it { is_expected.to validate_presence_of(:runner_type) } diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 5076f7faeac..3228c400155 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Ci::Stage, :models do let(:stage) { create(:ci_stage_entity) } + it_behaves_like 'having unique enum values' + describe 'associations' do before do create(:ci_build, stage_id: stage.id) diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index cfe0e216c78..cd28f1fe9c6 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe Clusters::Applications::Ingress do let(:ingress) { create(:clusters_applications_ingress) } + it_behaves_like 'having unique enum values' + include_examples 'cluster application core specs', :clusters_applications_ingress include_examples 'cluster application status specs', :clusters_applications_ingress include_examples 'cluster application helm specs', :clusters_applications_ingress diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index d43d88c2924..a1579b90436 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' describe Clusters::Applications::Knative do + include KubernetesHelpers + include ReactiveCachingHelpers + let(:knative) { create(:clusters_applications_knative) } include_examples 'cluster application core specs', :clusters_applications_knative @@ -121,4 +124,43 @@ describe Clusters::Applications::Knative do describe 'validations' do it { is_expected.to validate_presence_of(:hostname) } end + + describe '#services' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:service) { cluster.platform_kubernetes } + let(:knative) { create(:clusters_applications_knative, cluster: cluster) } + + let(:namespace) do + create(:cluster_kubernetes_namespace, + cluster: cluster, + cluster_project: cluster.cluster_project, + project: cluster.cluster_project.project) + end + + subject { knative.services } + + before do + stub_kubeclient_discover(service.api_url) + stub_kubeclient_knative_services + end + + it 'should have an unintialized cache' do + is_expected.to be_nil + end + + context 'when using synchronous reactive cache' do + before do + stub_reactive_cache(knative, services: kube_response(kube_knative_services_body)) + synchronous_reactive_cache(knative) + end + + it 'should have cached services' do + is_expected.not_to be_nil + end + + it 'should match our namespace' do + expect(knative.services_for(ns: namespace)).not_to be_nil + end + end + end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 97e50809647..47daa79873e 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -18,7 +18,7 @@ describe Clusters::Applications::Runner do let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.38') + expect(application.reload.version).to eq('0.1.39') end end end @@ -46,7 +46,7 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to eq('0.1.38') + expect(subject.version).to eq('0.1.39') expect(subject).not_to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) @@ -64,7 +64,7 @@ describe Clusters::Applications::Runner do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } it 'should be initialized with the locked version' do - expect(subject.version).to eq('0.1.38') + expect(subject.version).to eq('0.1.39') end end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index eb68ebccdcb..840f74c9890 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Clusters::Cluster do + it_behaves_like 'having unique enum values' + it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:cluster_projects) } it { is_expected.to have_many(:projects) } @@ -90,6 +92,26 @@ describe Clusters::Cluster do it { is_expected.to contain_exactly(cluster) } end + describe '.missing_kubernetes_namespace' do + let!(:cluster) { create(:cluster, :provided_by_gcp, :project) } + let(:project) { cluster.project } + let(:kubernetes_namespaces) { project.kubernetes_namespaces } + + subject do + described_class.joins(:projects).where(projects: { id: project.id }).missing_kubernetes_namespace(kubernetes_namespaces) + end + + it { is_expected.to contain_exactly(cluster) } + + context 'kubernetes namespace exists' do + before do + create(:cluster_kubernetes_namespace, project: project, cluster: cluster) + end + + it { is_expected.to be_empty } + end + end + describe 'validation' do subject { cluster.valid? } @@ -231,6 +253,81 @@ describe Clusters::Cluster do end end + describe '.ancestor_clusters_for_clusterable' do + let(:group_cluster) { create(:cluster, :provided_by_gcp, :group) } + let(:group) { group_cluster.group } + let(:hierarchy_order) { :desc } + let(:clusterable) { project } + + subject do + described_class.ancestor_clusters_for_clusterable(clusterable, hierarchy_order: hierarchy_order) + end + + context 'when project does not belong to this group' do + let(:project) { create(:project, group: create(:group)) } + + it 'returns nothing' do + is_expected.to be_empty + end + end + + context 'when group has a configured kubernetes cluster' do + let(:project) { create(:project, group: group) } + + it 'returns the group cluster' do + is_expected.to eq([group_cluster]) + end + end + + context 'when sub-group has configured kubernetes cluster', :nested_groups do + let(:sub_group_cluster) { create(:cluster, :provided_by_gcp, :group) } + let(:sub_group) { sub_group_cluster.group } + let(:project) { create(:project, group: sub_group) } + + before do + sub_group.update!(parent: group) + end + + it 'returns clusters in order, descending the hierachy' do + is_expected.to eq([group_cluster, sub_group_cluster]) + end + + it 'avoids N+1 queries' do + another_project = create(:project) + control_count = ActiveRecord::QueryRecorder.new do + described_class.ancestor_clusters_for_clusterable(another_project, hierarchy_order: hierarchy_order) + end.count + + cluster2 = create(:cluster, :provided_by_gcp, :group) + child2 = cluster2.group + child2.update!(parent: sub_group) + project = create(:project, group: child2) + + expect do + described_class.ancestor_clusters_for_clusterable(project, hierarchy_order: hierarchy_order) + end.not_to exceed_query_limit(control_count) + end + + context 'for a group' do + let(:clusterable) { sub_group } + + it 'returns clusters in order for a group' do + is_expected.to eq([group_cluster]) + end + end + end + + context 'scope chaining' do + let(:project) { create(:project, group: group) } + + subject { described_class.none.ancestor_clusters_for_clusterable(project) } + + it 'returns nothing' do + is_expected.to be_empty + end + end + end + describe '#provider' do subject { cluster.provider } @@ -263,6 +360,31 @@ describe Clusters::Cluster do end end + describe '#all_projects' do + let(:project) { create(:project) } + let(:cluster) { create(:cluster, projects: [project]) } + + subject { cluster.all_projects } + + context 'project cluster' do + it 'returns project' do + is_expected.to eq([project]) + end + end + + context 'group cluster' do + let(:cluster) { create(:cluster, :group) } + let(:group) { cluster.group } + let(:project) { create(:project, group: group) } + let(:subgroup) { create(:group, parent: group) } + let(:subproject) { create(:project, group: subgroup) } + + it 'returns all projects for group' do + is_expected.to contain_exactly(project, subproject) + end + end + end + describe '#first_project' do subject { cluster.first_project } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 99fd6ccc4d8..062d2fd0768 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -18,6 +18,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { is_expected.to delegate_method(:managed?).to(:cluster) } it { is_expected.to delegate_method(:kubernetes_namespace).to(:cluster) } + it_behaves_like 'having unique enum values' + describe 'before_validation' do context 'when namespace includes upper case' do let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } @@ -273,6 +275,36 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching ) end end + + context 'group level cluster' do + let!(:cluster) { create(:cluster, :group, platform_kubernetes: kubernetes) } + + let(:project) { create(:project, group: cluster.group) } + + subject { kubernetes.predefined_variables(project: project) } + + context 'no kubernetes namespace for the project' do + it_behaves_like 'setting variables' + + it 'does not return KUBE_TOKEN' do + expect(subject).not_to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end + end + + context 'kubernetes namespace exists for the project' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster, project: project) } + + it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + ) + end + end + end end describe '#terminals' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 2a0039a0635..a2d2d77746d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -204,7 +204,7 @@ describe Commit do message = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit. Vivamus egestas lacinia lacus, sed rutrum mauris.' allow(commit).to receive(:safe_message).and_return(message) - expect(commit.title).to eq('Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis…') + expect(commit.title).to eq('Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id...') end it "truncates a message with a newline before 80 characters at the newline" do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 917685399d4..8b7c88805c1 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -13,6 +13,8 @@ describe CommitStatus do create(:commit_status, pipeline: pipeline, **opts) end + it_behaves_like 'having unique enum values' + it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:project) } diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index 8847623f705..b14b773b653 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -54,7 +54,8 @@ shared_examples 'ChronicDurationAttribute writer' do subject.send("#{virtual_field}=", '-10m') expect(subject.valid?).to be_falsey - expect(subject.errors&.messages).to include(virtual_field => ['is not a correct duration']) + expect(subject.errors&.messages) + .to include(base: ['Maximum job timeout has a value which could not be accepted']) end end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 7bb89fe41dc..19ab4382b53 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -43,13 +43,86 @@ describe DeploymentPlatform do it { is_expected.to be_nil } end - context 'when user configured kubernetes from CI/CD > Clusters' do + 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 } it 'returns the Kubernetes platform' do expect(subject).to eq(platform_kubernetes) end + + context 'with a group level kubernetes cluster' do + let(:group_cluster) { create(:cluster, :provided_by_gcp, :group) } + + before do + project.update!(group: group_cluster.group) + end + + it 'returns the Kubernetes platform from the project cluster' do + expect(subject).to eq(platform_kubernetes) + end + end + end + + context 'when group has configured kubernetes cluster' do + let!(:group_cluster) { create(:cluster, :provided_by_gcp, :group) } + let(:group) { group_cluster.group } + + before do + project.update!(group: group) + end + + it 'returns the Kubernetes platform' do + is_expected.to eq(group_cluster.platform_kubernetes) + end + + context 'when child group has configured kubernetes cluster', :nested_groups do + let!(:child_group1_cluster) { create(:cluster, :provided_by_gcp, :group) } + let(:child_group1) { child_group1_cluster.group } + + before do + project.update!(group: child_group1) + child_group1.update!(parent: group) + end + + it 'returns the Kubernetes platform for the child group' do + is_expected.to eq(child_group1_cluster.platform_kubernetes) + end + + context 'deeply nested group' do + let!(:child_group2_cluster) { create(:cluster, :provided_by_gcp, :group) } + let(:child_group2) { child_group2_cluster.group } + + before do + child_group2.update!(parent: child_group1) + project.update!(group: child_group2) + end + + it 'returns most nested group cluster Kubernetes platform' do + is_expected.to eq(child_group2_cluster.platform_kubernetes) + end + + context 'cluster in the middle of hierarchy is disabled' do + before do + child_group2_cluster.update!(enabled: false) + end + + it 'returns closest enabled Kubenetes platform' do + is_expected.to eq(child_group1_cluster.platform_kubernetes) + end + end + end + end + + context 'feature flag disabled' do + before do + stub_feature_flags(group_clusters: false) + end + + it 'returns nil' do + is_expected.to be_nil + end + end end context 'when user configured kubernetes integration from project services' do diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb index 8cd129dc851..73eb7a1160d 100644 --- a/spec/models/concerns/discussion_on_diff_spec.rb +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -12,6 +12,34 @@ describe DiscussionOnDiff do expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES end + + context 'with truncated diff lines diff limit set' do + let(:truncated_lines) do + subject.truncated_diff_lines( + diff_limit: diff_limit + ) + end + + context 'when diff limit is higher than default' do + let(:diff_limit) { DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + 1 } + + it 'returns fewer lines than the default' do + expect(subject.diff_lines.count).to be > diff_limit + + expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + context 'when diff_limit is lower than default' do + let(:diff_limit) { 3 } + + it 'returns fewer lines than the default' do + expect(subject.diff_lines.count).to be > DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + + expect(truncated_lines.count).to be <= diff_limit + end + end + end end context "when some diff lines are meta" do diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 782687516ae..55d83bc3a6b 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -21,44 +21,59 @@ end describe ApplicationSetting, 'TokenAuthenticatable' do let(:token_field) { :runners_registration_token } + let(:settings) { described_class.new } + it_behaves_like 'TokenAuthenticatable' describe 'generating new token' do context 'token is not generated yet' do describe 'token field accessor' do - subject { described_class.new.send(token_field) } + subject { settings.send(token_field) } + it { is_expected.not_to be_blank } end - describe 'ensured token' do - subject { described_class.new.send("ensure_#{token_field}") } + describe "ensure_runners_registration_token" do + subject { settings.send("ensure_#{token_field}") } it { is_expected.to be_a String } it { is_expected.not_to be_blank } + + it 'does not persist token' do + expect(settings).not_to be_persisted + end end - describe 'ensured! token' do - subject { described_class.new.send("ensure_#{token_field}!") } + describe 'ensure_runners_registration_token!' do + subject { settings.send("ensure_#{token_field}!") } - it 'persists new token' do - expect(subject).to eq described_class.current[token_field] + it 'persists new token as an encrypted string' do + expect(subject).to eq settings.reload.runners_registration_token + expect(settings.read_attribute('runners_registration_token_encrypted')) + .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject) + expect(settings).to be_persisted + end + + it 'does not persist token in a clear text' do + expect(subject).not_to eq settings.reload + .read_attribute('runners_registration_token_encrypted') end end end context 'token is generated' do before do - subject.send("reset_#{token_field}!") + settings.send("reset_#{token_field}!") end it 'persists a new token' do - expect(subject.send(:read_attribute, token_field)).to be_a String + expect(settings.runners_registration_token).to be_a String end end end describe 'setting new token' do - subject { described_class.new.send("set_#{token_field}", '0123456789') } + subject { settings.send("set_#{token_field}", '0123456789') } it { is_expected.to eq '0123456789' } end @@ -336,3 +351,89 @@ describe PersonalAccessToken, 'TokenAuthenticatable' do end end end + +describe Ci::Build, 'TokenAuthenticatable' do + let(:token_field) { :token } + let(:build) { FactoryBot.build(:ci_build) } + + it_behaves_like 'TokenAuthenticatable' + + describe 'generating new token' do + context 'token is not generated yet' do + describe 'token field accessor' do + it 'makes it possible to access token' do + expect(build.token).to be_nil + + build.save! + + expect(build.token).to be_present + end + end + + describe "ensure_token" do + subject { build.ensure_token } + + it { is_expected.to be_a String } + it { is_expected.not_to be_blank } + + it 'does not persist token' do + expect(build).not_to be_persisted + end + end + + describe 'ensure_token!' do + it 'persists a new token' do + expect(build.ensure_token!).to eq build.reload.token + expect(build).to be_persisted + end + + it 'persists new token as an encrypted string' do + build.ensure_token! + + encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token) + + expect(build.read_attribute('token_encrypted')).to eq encrypted + end + + it 'does not persist a token in a clear text' do + build.ensure_token! + + expect(build.read_attribute('token')).to be_nil + end + end + end + + describe '#reset_token!' do + it 'persists a new token' do + build.save! + + build.token.yield_self do |previous_token| + build.reset_token! + + expect(build.token).not_to eq previous_token + expect(build.token).to be_a String + end + end + end + end + + describe 'setting a new token' do + subject { build.set_token('0123456789') } + + it 'returns the token' do + expect(subject).to eq '0123456789' + end + + it 'writes a new encrypted token' do + expect(build.read_attribute('token_encrypted')).to be_nil + expect(subject).to eq '0123456789' + expect(build.read_attribute('token_encrypted')).to be_present + end + + it 'does not write a new cleartext token' do + expect(build.read_attribute('token')).to be_nil + expect(subject).to eq '0123456789' + expect(build.read_attribute('token')).to be_nil + end + end +end diff --git a/spec/models/concerns/token_authenticatable_strategies/base_spec.rb b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb new file mode 100644 index 00000000000..6605f1f5a5f --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe TokenAuthenticatableStrategies::Base do + let(:instance) { double(:instance) } + let(:field) { double(:field) } + + describe '.fabricate' do + context 'when digest stragegy is specified' do + it 'fabricates digest strategy object' do + strategy = described_class.fabricate(instance, field, digest: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Digest + end + end + + context 'when encrypted strategy is specified' do + it 'fabricates encrypted strategy object' do + strategy = described_class.fabricate(instance, field, encrypted: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Encrypted + end + end + + context 'when no strategy is specified' do + it 'fabricates insecure strategy object' do + strategy = described_class.fabricate(instance, field, something: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Insecure + end + end + + context 'when incompatible options are provided' do + it 'raises an error' do + expect { described_class.fabricate(instance, field, digest: true, encrypted: true) } + .to raise_error ArgumentError + end + end + end + + describe '#fallback?' do + context 'when fallback is set' do + it 'recognizes fallback setting' do + strategy = described_class.new(instance, field, fallback: true) + + expect(strategy.fallback?).to be true + end + end + + context 'when fallback is not a valid value' do + it 'raises an error' do + strategy = described_class.new(instance, field, fallback: 'something') + + expect { strategy.fallback? }.to raise_error ArgumentError + end + end + + context 'when fallback is not set' do + it 'raises an error' do + strategy = described_class.new(instance, field, {}) + + expect(strategy.fallback?).to eq false + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb new file mode 100644 index 00000000000..93cab80cb1f --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -0,0 +1,156 @@ +require 'spec_helper' + +describe TokenAuthenticatableStrategies::Encrypted do + let(:model) { double(:model) } + let(:instance) { double(:instance) } + + let(:encrypted) do + Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') + end + + subject do + described_class.new(model, 'some_field', options) + end + + describe '.new' do + context 'when fallback and migration strategies are set' do + let(:options) { { fallback: true, migrating: true } } + + it 'raises an error' do + expect { subject }.to raise_error ArgumentError, /not compatible/ + end + end + end + + describe '#find_token_authenticatable' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field_encrypted' => encrypted) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'encrypted resource' + end + + it 'uses insecure strategy when encrypted token cannot be found' do + allow(subject.send(:insecure_strategy)) + .to receive(:find_token_authenticatable) + .and_return('plaintext resource') + + allow(model).to receive(:find_by) + .with('some_field_encrypted' => encrypted) + .and_return(nil) + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'plaintext resource' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'finds the cleartext resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field' => 'my-value') + .and_return('cleartext resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'cleartext resource' + end + + it 'returns nil if resource cannot be found' do + allow(model).to receive(:find_by) + .with('some_field' => 'my-value') + .and_return(nil) + + expect(subject.find_token_authenticatable('my-value')) + .to be_nil + end + end + end + + describe '#get_token' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'returns decrypted token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) + + expect(subject.get_token(instance)).to eq 'my-value' + end + + it 'returns the plaintext token when encrypted token is not present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(nil) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('cleartext value') + + expect(subject.get_token(instance)).to eq 'cleartext value' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'returns cleartext token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('my-cleartext-value') + + expect(subject.get_token(instance)).to eq 'my-cleartext-value' + end + + it 'returns the cleartext token when encrypted token is not present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(nil) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('cleartext value') + + expect(subject.get_token(instance)).to eq 'cleartext value' + end + end + end + + describe '#set_token' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'writes encrypted token and removes plaintext token and returns it' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + expect(instance).to receive(:[]=) + .with('some_field', nil) + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'writes encrypted token and writes plaintext token' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + expect(instance).to receive(:[]=) + .with('some_field', 'my-value') + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 270b2767c68..a8d53cfcd7d 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,6 +16,8 @@ describe Deployment do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + it_behaves_like 'having unique enum values' + describe '#scheduled_actions' do subject { deployment.scheduled_actions } diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 90f7e4a4590..9da16dea929 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -92,16 +92,12 @@ describe EnvironmentStatus do end describe '.build_environments_status' do - subject { described_class.send(:build_environments_status, merge_request, user, sha) } + subject { described_class.send(:build_environments_status, merge_request, user, pipeline) } let!(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) } let(:environment) { build.deployment.environment } let(:user) { project.owner } - before do - build.deployment&.update!(sha: sha) - end - context 'when environment is created on a forked project' do let(:project) { create(:project, :repository) } let(:forked) { fork_project(project, user, repository: true) } @@ -160,6 +156,39 @@ describe EnvironmentStatus do expect(subject.count).to eq(0) end end + + context 'when multiple deployments with the same SHA in different environments' do + let(:pipeline2) { create(:ci_pipeline, sha: sha, project: project) } + let!(:build2) { create(:ci_build, :start_review_app, pipeline: pipeline2) } + + it 'returns deployments related to the head pipeline' do + expect(subject.count).to eq(1) + expect(subject[0].environment).to eq(environment) + expect(subject[0].merge_request).to eq(merge_request) + expect(subject[0].sha).to eq(sha) + end + end + + context 'when multiple deployments in the same pipeline for the same environments' do + let!(:build2) { create(:ci_build, :deploy_to_production, pipeline: pipeline) } + + it 'returns unique entries' do + expect(subject.count).to eq(1) + expect(subject[0].environment).to eq(environment) + expect(subject[0].merge_request).to eq(merge_request) + expect(subject[0].sha).to eq(sha) + end + end + + context 'when environment is stopped' do + before do + environment.stop! + end + + it 'does not return environment status' do + expect(subject.count).to eq(0) + end + end end end end diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index 0136bb61c07..cdd7dea2064 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -8,6 +8,8 @@ RSpec.describe GpgSignature do let(:gpg_key) { create(:gpg_key) } let(:gpg_key_subkey) { create(:gpg_key_subkey) } + it_behaves_like 'having unique enum values' + describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:gpg_key) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ada00f03928..e63881242f6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -76,7 +76,7 @@ describe Group do before do group.add_developer(user) - sub_group.add_developer(user) + sub_group.add_maintainer(user) end it 'also gets notification settings from parent groups' do @@ -498,7 +498,7 @@ describe Group do it 'returns member users on every nest level without duplication' do group.add_developer(user_a) nested_group.add_developer(user_b) - deep_nested_group.add_developer(user_a) + deep_nested_group.add_maintainer(user_a) expect(group.users_with_descendants).to contain_exactly(user_a, user_b) expect(nested_group.users_with_descendants).to contain_exactly(user_a, user_b) @@ -739,10 +739,39 @@ describe Group do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', true do + it_behaves_like 'model with uploads', true do let(:model_object) { create(:group, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } end end + + describe '#group_clusters_enabled?' do + before do + # Override global stub in spec/spec_helper.rb + expect(Feature).to receive(:enabled?).and_call_original + end + + subject { group.group_clusters_enabled? } + + it { is_expected.to be_truthy } + + context 'explicitly disabled for root ancestor' do + before do + feature = Feature.get(:group_clusters) + feature.disable(group.root_ancestor) + end + + it { is_expected.to be_falsey } + end + + context 'explicitly disabled for root ancestor' do + before do + feature = Feature.get(:group_clusters) + feature.enable(group.root_ancestor) + end + + it { is_expected.to be_truthy } + end + end end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 52c00a74b4b..4696341c05f 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -7,6 +7,8 @@ describe InternalId do let(:scope) { { project: project } } let(:init) { ->(s) { s.project.issues.size } } + it_behaves_like 'having unique enum values' + context 'validations' do it { is_expected.to validate_presence_of(:usage) } end diff --git a/spec/models/list_spec.rb b/spec/models/list_spec.rb index 17dc27bd132..a51580f8292 100644 --- a/spec/models/list_spec.rb +++ b/spec/models/list_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe List do + it_behaves_like 'having unique enum values' + describe 'relationships' do it { is_expected.to belong_to(:board) } it { is_expected.to belong_to(:label) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index fca1b1f90d9..188beac1582 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -53,6 +53,29 @@ describe Member do expect(member).to be_valid end end + + context "when a child member inherits its access level" do + let(:user) { create(:user) } + let(:member) { create(:group_member, :developer, user: user) } + let(:child_group) { create(:group, parent: member.group) } + let(:child_member) { build(:group_member, group: child_group, user: user) } + + it "requires a higher level" do + child_member.access_level = GroupMember::REPORTER + + child_member.validate + + expect(child_member).not_to be_valid + end + + it "is valid with a higher level" do + child_member.access_level = GroupMember::MAINTAINER + + child_member.validate + + expect(child_member).to be_valid + end + end end describe 'Scopes & finders' do diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 97959ed4304..a3451c67bd8 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -50,4 +50,26 @@ describe GroupMember do group_member.destroy end end + + context 'access levels', :nested_groups do + context 'with parent group' do + it_behaves_like 'inherited access level as a member of entity' do + let(:entity) { create(:group, parent: parent_entity) } + end + end + + context 'with parent group and a sub subgroup' do + it_behaves_like 'inherited access level as a member of entity' do + let(:subgroup) { create(:group, parent: parent_entity) } + let(:entity) { create(:group, parent: subgroup) } + end + + context 'when only the subgroup has the member' do + it_behaves_like 'inherited access level as a member of entity' do + let(:parent_entity) { create(:group, parent: create(:group)) } + let(:entity) { create(:group, parent: parent_entity) } + end + end + end + end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 334d4f95f53..097b1bb30dc 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -124,4 +124,19 @@ describe ProjectMember do end it_behaves_like 'members notifications', :project + + context 'access levels' do + context 'with parent group' do + it_behaves_like 'inherited access level as a member of entity' do + let(:entity) { create(:project, group: parent_entity) } + end + end + + context 'with parent group and a subgroup', :nested_groups do + it_behaves_like 'inherited access level as a member of entity' do + let(:subgroup) { create(:group, parent: parent_entity) } + let(:entity) { create(:project, group: subgroup) } + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ad55c280399..9b60054e14a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1206,6 +1206,119 @@ describe MergeRequest do expect(subject.all_pipelines).to contain_exactly(pipeline) end end + + context 'when pipelines exist for the branch and merge request' do + let(:source_ref) { 'feature' } + let(:target_ref) { 'master' } + + let!(:branch_pipeline) do + create(:ci_pipeline, + source: :push, + project: project, + ref: source_ref, + sha: shas.second) + end + + let!(:merge_request_pipeline) do + create(:ci_pipeline, + source: :merge_request, + project: project, + ref: source_ref, + sha: shas.second, + merge_request: merge_request) + end + + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: source_ref, + target_project: project, + target_branch: target_ref) + end + + let(:project) { create(:project, :repository) } + let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) } + + before do + allow(merge_request).to receive(:all_commit_shas) { shas } + end + + it 'returns merge request pipeline first' do + expect(merge_request.all_pipelines) + .to eq([merge_request_pipeline, + branch_pipeline]) + end + + context 'when there are a branch pipeline and a merge request pipeline' do + let!(:branch_pipeline_2) do + create(:ci_pipeline, + source: :push, + project: project, + ref: source_ref, + sha: shas.first) + end + + let!(:merge_request_pipeline_2) do + create(:ci_pipeline, + source: :merge_request, + project: project, + ref: source_ref, + sha: shas.first, + merge_request: merge_request) + end + + it 'returns merge request pipelines first' do + expect(merge_request.all_pipelines) + .to eq([merge_request_pipeline_2, + merge_request_pipeline, + branch_pipeline_2, + branch_pipeline]) + end + end + + context 'when there are multiple merge request pipelines from the same branch' do + let!(:branch_pipeline_2) do + create(:ci_pipeline, + source: :push, + project: project, + ref: source_ref, + sha: shas.first) + end + + let!(:merge_request_pipeline_2) do + create(:ci_pipeline, + source: :merge_request, + project: project, + ref: source_ref, + sha: shas.first, + merge_request: merge_request_2) + end + + let(:merge_request_2) do + create(:merge_request, + source_project: project, + source_branch: source_ref, + target_project: project, + target_branch: 'stable') + end + + before do + allow(merge_request_2).to receive(:all_commit_shas) { shas } + end + + it 'returns only related merge request pipelines' do + expect(merge_request.all_pipelines) + .to eq([merge_request_pipeline, + branch_pipeline_2, + branch_pipeline]) + + expect(merge_request_2.all_pipelines) + .to eq([merge_request_pipeline_2, + branch_pipeline_2, + branch_pipeline]) + end + end + end end describe '#has_test_reports?' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 2db42fe802a..18b54cce834 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -249,7 +249,7 @@ describe Namespace do move_dir_result end - expect(Gitlab::Sentry).to receive(:should_raise?).and_return(false) # like prod + expect(Gitlab::Sentry).to receive(:should_raise_for_dev?).and_return(false) # like prod namespace.update(path: namespace.full_path + '_new') end @@ -538,7 +538,7 @@ describe Namespace do it 'returns member users on every nest level without duplication' do group.add_developer(user_a) nested_group.add_developer(user_b) - deep_nested_group.add_developer(user_a) + deep_nested_group.add_maintainer(user_a) expect(group.users_with_descendants).to contain_exactly(user_a, user_b) expect(nested_group.users_with_descendants).to contain_exactly(user_a, user_b) @@ -560,6 +560,7 @@ describe Namespace do let!(:project2) { create(:project_empty_repo, namespace: child) } it { expect(group.all_projects.to_a).to match_array([project2, project1]) } + it { expect(child.all_projects.to_a).to match_array([project2]) } end describe '#all_pipelines' do @@ -720,6 +721,7 @@ describe Namespace do deep_nested_group = create(:group, parent: nested_group) very_deep_nested_group = create(:group, parent: deep_nested_group) + expect(root_group.root_ancestor).to eq(root_group) expect(nested_group.root_ancestor).to eq(root_group) expect(deep_nested_group.root_ancestor).to eq(root_group) expect(very_deep_nested_group.root_ancestor).to eq(root_group) diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index f9be61e4768..bcdfe3cf1eb 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -517,7 +517,7 @@ describe Note do describe '#to_ability_name' do it 'returns snippet for a project snippet note' do - expect(build(:note_on_project_snippet).to_ability_name).to eq('snippet') + expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet') end it 'returns personal_snippet for a personal snippet note' do diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index e545b674b4f..771d834c4bc 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' RSpec.describe NotificationSetting do + it_behaves_like 'having unique enum values' + describe "Associations" do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:source) } diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb new file mode 100644 index 00000000000..541e78507e5 --- /dev/null +++ b/spec/models/pool_repository_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PoolRepository do + describe 'associations' do + it { is_expected.to belong_to(:shard) } + it { is_expected.to have_many(:member_projects) } + end + + describe 'validations' do + let!(:pool_repository) { create(:pool_repository) } + + it { is_expected.to validate_presence_of(:shard) } + end + + describe '#disk_path' do + it 'sets the hashed disk_path' do + pool = create(:pool_repository) + + elements = File.split(pool.disk_path) + + expect(elements).to all( match(/\d{2,}/) ) + end + end +end diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index 342798f730b..7ff64c76e37 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe ProjectAutoDevops do set(:project) { build(:project) } + it_behaves_like 'having unique enum values' + it { is_expected.to belong_to(:project) } it { is_expected.to define_enum_for(:deploy_strategy) } diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index f7033b28c76..e3b2d971419 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -10,4 +10,116 @@ describe ProjectImportState, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:project) } end + + describe 'Project import job' do + let(:import_state) { create(:import_state, import_url: generate(:url)) } + let(:project) { import_state.project } + + before do + allow_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository) + .with(project.import_url).and_return(true) + + # Works around https://github.com/rspec/rspec-mocks/issues/910 + allow(Project).to receive(:find).with(project.id).and_return(project) + expect(project.repository).to receive(:after_import).and_call_original + expect(project.wiki.repository).to receive(:after_import).and_call_original + end + + it 'imports a project' do + expect(RepositoryImportWorker).to receive(:perform_async).and_call_original + + expect { import_state.schedule }.to change { import_state.jid } + expect(import_state.status).to eq('finished') + end + end + + describe '#human_status_name' do + context 'when import_state exists' do + it 'returns the humanized status name' do + import_state = build(:import_state, :started) + + expect(import_state.human_status_name).to eq("started") + end + end + end + + describe 'import state transitions' do + context 'state transition: [:started] => [:finished]' do + let(:after_import_service) { spy(:after_import_service) } + let(:housekeeping_service) { spy(:housekeeping_service) } + + before do + allow(Projects::AfterImportService) + .to receive(:new) { after_import_service } + + allow(after_import_service) + .to receive(:execute) { housekeeping_service.execute } + + allow(Projects::HousekeepingService) + .to receive(:new) { housekeeping_service } + end + + it 'resets last_error' do + error_message = 'Some error' + import_state = create(:import_state, :started, last_error: error_message) + + expect { import_state.finish }.to change { import_state.last_error }.from(error_message).to(nil) + end + + it 'performs housekeeping when an import of a fresh project is completed' do + project = create(:project_empty_repo, :import_started, import_type: :github) + + project.import_state.finish + + expect(after_import_service).to have_received(:execute) + expect(housekeeping_service).to have_received(:execute) + end + + it 'does not perform housekeeping when project repository does not exist' do + project = create(:project, :import_started, import_type: :github) + + project.import_state.finish + + expect(housekeeping_service).not_to have_received(:execute) + end + + it 'does not perform housekeeping when project does not have a valid import type' do + project = create(:project, :import_started, import_type: nil) + + project.import_state.finish + + expect(housekeeping_service).not_to have_received(:execute) + end + end + end + + describe '#remove_jid', :clean_gitlab_redis_cache do + let(:project) { } + + context 'without an JID' do + it 'does nothing' do + import_state = create(:import_state) + + expect(Gitlab::SidekiqStatus) + .not_to receive(:unset) + + import_state.remove_jid + end + end + + context 'with an JID' do + it 'unsets the JID' do + import_state = create(:import_state, jid: '123') + + expect(Gitlab::SidekiqStatus) + .to receive(:unset) + .with('123') + .and_call_original + + import_state.remove_jid + + expect(import_state.jid).to be_nil + end + end + end end diff --git a/spec/models/project_repository_spec.rb b/spec/models/project_repository_spec.rb new file mode 100644 index 00000000000..c966447fedc --- /dev/null +++ b/spec/models/project_repository_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectRepository do + describe 'associations' do + it { is_expected.to belong_to(:shard) } + it { is_expected.to belong_to(:project) } + end + + describe '.find_project' do + it 'finds project by disk path' do + project = create(:project) + project.track_project_repository + + expect(described_class.find_project(project.disk_path)).to eq(project) + end + + it 'returns nil when it does not find the project' do + expect(described_class.find_project('@@unexisting/path/to/project')).to be_nil + end + end +end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index f2cb927df37..b6cf4c72450 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -13,6 +13,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do it { is_expected.to belong_to :project } end + context 'redirects' do + it 'does not follow redirects' do + redirect_to = 'https://redirected.example.com' + redirect_req_stub = stub_prometheus_request(prometheus_query_url('1'), status: 302, headers: { location: redirect_to }) + redirected_req_stub = stub_prometheus_request(redirect_to, body: { 'status': 'success' }) + + result = service.test + + # result = { success: false, result: error } + expect(result[:success]).to be_falsy + expect(result[:result]).to be_instance_of(Gitlab::PrometheusClient::Error) + + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + describe 'Validations' do context 'when manual_configuration is enabled' do before do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 47c331fbc7a..93c83fd21fd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4,6 +4,8 @@ describe Project do include ProjectForksHelper include GitHelpers + it_behaves_like 'having unique enum values' + describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } @@ -54,13 +56,14 @@ describe Project do it { is_expected.to have_one(:gitlab_issue_tracker_service) } it { is_expected.to have_one(:external_wiki_service) } it { is_expected.to have_one(:project_feature) } + it { is_expected.to have_one(:project_repository) } it { is_expected.to have_one(:statistics).class_name('ProjectStatistics') } it { is_expected.to have_one(:import_data).class_name('ProjectImportData') } it { is_expected.to have_one(:last_event).class_name('Event') } it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) } it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') } it { is_expected.to have_many(:commit_statuses) } - it { is_expected.to have_many(:pipelines) } + it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:build_trace_section_names)} it { is_expected.to have_many(:runner_projects) } @@ -84,6 +87,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(:kubernetes_namespaces) } it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') } it { is_expected.to have_many(:project_badges).class_name('ProjectBadge') } it { is_expected.to have_many(:lfs_file_locks) } @@ -139,6 +143,29 @@ describe Project do expect(subject.boards.size).to eq 1 end end + + describe 'ci_pipelines association' do + context 'when feature flag pipeline_ci_sources_only is enabled' do + it 'returns only pipelines from ci_sources' do + stub_feature_flags(pipeline_ci_sources_only: true) + + expect(Ci::Pipeline).to receive(:ci_sources).and_call_original + + subject.ci_pipelines + end + end + + context 'when feature flag pipeline_ci_sources_only is disabled' do + it 'returns all pipelines' do + stub_feature_flags(pipeline_ci_sources_only: false) + + expect(Ci::Pipeline).not_to receive(:ci_sources).and_call_original + expect(Ci::Pipeline).to receive(:all).and_call_original.at_least(:once) + + subject.ci_pipelines + end + end + end end describe 'modules' do @@ -151,6 +178,24 @@ describe Project do it { is_expected.to include_module(Sortable) } end + describe '.missing_kubernetes_namespace' do + let!(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_user, :group) } + let(:kubernetes_namespaces) { project.kubernetes_namespaces } + + subject { described_class.missing_kubernetes_namespace(kubernetes_namespaces) } + + it { is_expected.to contain_exactly(project) } + + context 'kubernetes namespace exists' do + before do + create(:cluster_kubernetes_namespace, project: project, cluster: cluster) + end + + it { is_expected.to be_empty } + end + end + describe 'validation' do let!(:project) { create(:project) } @@ -218,76 +263,93 @@ describe Project do end end - it 'does not allow an invalid URI as import_url' do - project = build(:project, import_url: 'invalid://') + describe 'import_url' do + it 'does not allow an invalid URI as import_url' do + project = build(:project, import_url: 'invalid://') - expect(project).not_to be_valid - end + expect(project).not_to be_valid + end - it 'does allow a SSH URI as import_url for persisted projects' do - project = create(:project) - project.import_url = 'ssh://test@gitlab.com/project.git' + it 'does allow a SSH URI as import_url for persisted projects' do + project = create(:project) + project.import_url = 'ssh://test@gitlab.com/project.git' - expect(project).to be_valid - end + expect(project).to be_valid + end - it 'does not allow a SSH URI as import_url for new projects' do - project = build(:project, import_url: 'ssh://test@gitlab.com/project.git') + it 'does not allow a SSH URI as import_url for new projects' do + project = build(:project, import_url: 'ssh://test@gitlab.com/project.git') - expect(project).not_to be_valid - end + expect(project).not_to be_valid + end - it 'does allow a valid URI as import_url' do - project = build(:project, import_url: 'http://gitlab.com/project.git') + it 'does allow a valid URI as import_url' do + project = build(:project, import_url: 'http://gitlab.com/project.git') - expect(project).to be_valid - end + expect(project).to be_valid + end - it 'allows an empty URI' do - project = build(:project, import_url: '') + it 'allows an empty URI' do + project = build(:project, import_url: '') - expect(project).to be_valid - end + expect(project).to be_valid + end - it 'does not produce import data on an empty URI' do - project = build(:project, import_url: '') + it 'does not produce import data on an empty URI' do + project = build(:project, import_url: '') - expect(project.import_data).to be_nil - end + expect(project.import_data).to be_nil + end - it 'does not produce import data on an invalid URI' do - project = build(:project, import_url: 'test://') + it 'does not produce import data on an invalid URI' do + project = build(:project, import_url: 'test://') - expect(project.import_data).to be_nil - end + expect(project.import_data).to be_nil + end - it "does not allow import_url pointing to localhost" do - project = build(:project, import_url: 'http://localhost:9000/t.git') + it "does not allow import_url pointing to localhost" do + project = build(:project, import_url: 'http://localhost:9000/t.git') - expect(project).to be_invalid - expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') - end + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') + end - it "does not allow import_url with invalid ports for new projects" do - project = build(:project, import_url: 'http://github.com:25/t.git') + it "does not allow import_url with invalid ports for new projects" do + project = build(:project, import_url: 'http://github.com:25/t.git') - expect(project).to be_invalid - expect(project.errors[:import_url].first).to include('Only allowed ports are 80, 443') - end + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Only allowed ports are 80, 443') + end - it "does not allow import_url with invalid ports for persisted projects" do - project = create(:project) - project.import_url = 'http://github.com:25/t.git' + it "does not allow import_url with invalid ports for persisted projects" do + project = create(:project) + project.import_url = 'http://github.com:25/t.git' - expect(project).to be_invalid - expect(project.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') - end + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') + end - it "does not allow import_url with invalid user" do - project = build(:project, import_url: 'http://$user:password@github.com/t.git') + it "does not allow import_url with invalid user" do + project = build(:project, import_url: 'http://$user:password@github.com/t.git') - expect(project).to be_invalid - expect(project.errors[:import_url].first).to include('Username needs to start with an alphanumeric character') + expect(project).to be_invalid + expect(project.errors[:import_url].first).to include('Username needs to start with an alphanumeric character') + end + + include_context 'invalid urls' + + it 'does not allow urls with CR or LF characters' do + project = build(:project) + + aggregate_failures do + urls_with_CRLF.each do |url| + project.import_url = url + + expect(project).not_to be_valid + expect(project.errors.full_messages.first).to match(/is blocked: URI is invalid/) + end + end + end end describe 'project pending deletion' do @@ -373,6 +435,8 @@ describe Project do it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } + it { is_expected.to delegate_method(:group_clusters_enabled?).to(:group).with_arguments(allow_nil: true) } + it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } end describe '#to_reference_with_postfix' do @@ -1601,6 +1665,30 @@ describe Project do end end + describe '#track_project_repository' do + let(:project) { create(:project, :repository) } + + it 'creates a project_repository' do + project.track_project_repository + + expect(project.reload.project_repository).to be_present + expect(project.project_repository.disk_path).to eq(project.disk_path) + expect(project.project_repository.shard_name).to eq(project.repository_storage) + end + + it 'updates the project_repository' do + project.track_project_repository + + allow(project).to receive(:disk_path).and_return('@fancy/new/path') + + expect do + project.track_project_repository + end.not_to change(ProjectRepository, :count) + + expect(project.reload.project_repository.disk_path).to eq(project.disk_path) + end + end + describe '#create_repository' do let(:project) { create(:project, :repository) } let(:shell) { Gitlab::Shell.new } @@ -1686,6 +1774,16 @@ describe Project do end end + describe 'handling import URL' do + it 'returns the sanitized URL' do + project = create(:project, :import_started, import_url: 'http://user:pass@test.com') + + project.import_state.finish + + expect(project.reload.import_url).to eq('http://test.com') + end + end + describe '#container_registry_url' do let(:project) { create(:project) } @@ -1799,106 +1897,6 @@ describe Project do end end - describe '#human_import_status_name' do - context 'when import_state exists' do - it 'returns the humanized status name' do - project = create(:project) - create(:import_state, :started, project: project) - - expect(project.human_import_status_name).to eq("started") - end - end - - context 'when import_state was not created yet' do - let(:project) { create(:project, :import_started) } - - it 'ensures import_state is created and returns humanized status name' do - expect do - project.human_import_status_name - end.to change { ProjectImportState.count }.from(0).to(1) - end - - it 'returns humanized status name' do - expect(project.human_import_status_name).to eq("started") - end - end - end - - describe 'Project import job' do - let(:project) { create(:project, import_url: generate(:url)) } - - before do - allow_any_instance_of(Gitlab::Shell).to receive(:import_repository) - .with(project.repository_storage, project.disk_path, project.import_url) - .and_return(true) - - # Works around https://github.com/rspec/rspec-mocks/issues/910 - allow(described_class).to receive(:find).with(project.id).and_return(project) - expect(project.repository).to receive(:after_import) - .and_call_original - expect(project.wiki.repository).to receive(:after_import) - .and_call_original - end - - it 'imports a project' do - expect_any_instance_of(RepositoryImportWorker).to receive(:perform).and_call_original - - expect { project.import_schedule }.to change { project.import_jid } - expect(project.reload.import_status).to eq('finished') - end - end - - describe 'project import state transitions' do - context 'state transition: [:started] => [:finished]' do - let(:after_import_service) { spy(:after_import_service) } - let(:housekeeping_service) { spy(:housekeeping_service) } - - before do - allow(Projects::AfterImportService) - .to receive(:new) { after_import_service } - - allow(after_import_service) - .to receive(:execute) { housekeeping_service.execute } - - allow(Projects::HousekeepingService) - .to receive(:new) { housekeeping_service } - end - - it 'resets project import_error' do - error_message = 'Some error' - mirror = create(:project_empty_repo, :import_started) - mirror.import_state.update(last_error: error_message) - - expect { mirror.import_finish }.to change { mirror.import_error }.from(error_message).to(nil) - end - - it 'performs housekeeping when an import of a fresh project is completed' do - project = create(:project_empty_repo, :import_started, import_type: :github) - - project.import_finish - - expect(after_import_service).to have_received(:execute) - expect(housekeeping_service).to have_received(:execute) - end - - it 'does not perform housekeeping when project repository does not exist' do - project = create(:project, :import_started, import_type: :github) - - project.import_finish - - expect(housekeeping_service).not_to have_received(:execute) - end - - it 'does not perform housekeeping when project does not have a valid import type' do - project = create(:project, :import_started, import_type: nil) - - project.import_finish - - expect(housekeeping_service).not_to have_received(:execute) - end - end - end - describe '#latest_successful_builds_for' do def create_pipeline(status = 'success') create(:ci_pipeline, project: project, @@ -1978,6 +1976,42 @@ describe Project do end end + describe '#import_status' do + context 'with import_state' do + it 'returns the right status' do + project = create(:project, :import_started) + + expect(project.import_status).to eq("started") + end + end + + context 'without import_state' do + it 'returns none' do + project = create(:project) + + expect(project.import_status).to eq('none') + end + end + end + + describe '#human_import_status_name' do + context 'with import_state' do + it 'returns the right human import status' do + project = create(:project, :import_started) + + expect(project.human_import_status_name).to eq('started') + end + end + + context 'without import_state' do + it 'returns none' do + project = create(:project) + + expect(project.human_import_status_name).to eq('none') + end + end + end + describe '#add_import_job' do let(:import_jid) { '123' } @@ -2108,6 +2142,39 @@ describe Project do it 'includes ancestors upto but excluding the given ancestor' do expect(project.ancestors_upto(parent)).to contain_exactly(child2, child) end + + describe 'with hierarchy_order' do + it 'returns ancestors ordered by descending hierarchy' do + expect(project.ancestors_upto(hierarchy_order: :desc)).to eq([parent, child, child2]) + end + + it 'can be used with upto option' do + expect(project.ancestors_upto(parent, hierarchy_order: :desc)).to eq([child, child2]) + end + end + end + + describe '#root_ancestor' do + let(:project) { create(:project) } + + subject { project.root_ancestor } + + it { is_expected.to eq(project.namespace) } + + context 'in a group' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + it { is_expected.to eq(group) } + end + + context 'in a nested group', :nested_groups do + let(:root) { create(:group) } + let(:child) { create(:group, parent: root) } + let(:project) { create(:project, group: child) } + + it { is_expected.to eq(root) } + end end describe '#lfs_enabled?' do @@ -2720,6 +2787,17 @@ describe Project do 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') + + expect(lfs_http_url_to_repo).to eq("#{project.web_url}.git") + expect(lfs_http_url_to_repo).not_to include('@') + end + end + describe '#pipeline_status' do let(:project) { create(:project, :repository) } it 'builds a pipeline status' do @@ -3372,7 +3450,7 @@ describe Project do context 'with a ref that is not the default branch' do it 'returns the latest successful pipeline for the given ref' do - expect(project.pipelines).to receive(:latest_successful_for).with('foo') + expect(project.ci_pipelines).to receive(:latest_successful_for).with('foo') project.latest_successful_pipeline_for('foo') end @@ -3400,7 +3478,7 @@ describe Project do it 'memoizes and returns the latest successful pipeline for the default branch' do pipeline = double(:pipeline) - expect(project.pipelines).to receive(:latest_successful_for) + expect(project.ci_pipelines).to receive(:latest_successful_for) .with(project.default_branch) .and_return(pipeline) .once @@ -3414,13 +3492,14 @@ describe Project do describe '#after_import' do let(:project) { create(:project) } + let(:import_state) { create(:import_state, project: project) } it 'runs the correct hooks' do expect(project.repository).to receive(:after_import) expect(project.wiki.repository).to receive(:after_import) - expect(project).to receive(:import_finish) + expect(import_state).to receive(:finish) expect(project).to receive(:update_project_counter_caches) - expect(project).to receive(:remove_import_jid) + expect(import_state).to receive(:remove_jid) expect(project).to receive(:after_create_default_branch) expect(project).to receive(:refresh_markdown_cache!) @@ -3430,6 +3509,10 @@ describe Project do context 'branch protection' do let(:project) { create(:project, :repository) } + before do + create(:import_state, :started, project: project) + end + it 'does not protect when branch protection is disabled' do stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) @@ -3485,37 +3568,6 @@ describe Project do end end - describe '#remove_import_jid', :clean_gitlab_redis_cache do - let(:project) { } - - context 'without an import JID' do - it 'does nothing' do - project = create(:project) - - expect(Gitlab::SidekiqStatus) - .not_to receive(:unset) - - project.remove_import_jid - end - end - - context 'with an import JID' do - it 'unsets the import JID' do - project = create(:project) - create(:import_state, project: project, jid: '123') - - expect(Gitlab::SidekiqStatus) - .to receive(:unset) - .with('123') - .and_call_original - - project.remove_import_jid - - expect(project.import_jid).to be_nil - end - end - end - describe '#wiki_repository_exists?' do it 'returns true when the wiki repository exists' do project = create(:project, :wiki_repo) @@ -3846,7 +3898,7 @@ describe Project do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', true do + it_behaves_like 'model with uploads', true do let(:model_object) { create(:project, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } @@ -4019,6 +4071,27 @@ describe Project do end end + describe '#all_clusters' do + let(:project) { create(:project) } + let(:cluster) { create(:cluster, cluster_type: :project_type, projects: [project]) } + + subject { project.all_clusters } + + it 'returns project level cluster' do + expect(subject).to eq([cluster]) + end + + context 'project belongs to a group' do + let(:group_cluster) { create(:cluster, :group) } + let(:group) { group_cluster.group } + let(:project) { create(:project, group: group) } + + it 'returns clusters for groups of this project' do + expect(subject).to contain_exactly(cluster, group_cluster) + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/models/prometheus_metric_spec.rb b/spec/models/prometheus_metric_spec.rb index a83a31ae88c..3692fe9a559 100644 --- a/spec/models/prometheus_metric_spec.rb +++ b/spec/models/prometheus_metric_spec.rb @@ -6,6 +6,8 @@ describe PrometheusMetric do subject { build(:prometheus_metric) } let(:other_project) { build(:project) } + it_behaves_like 'having unique enum values' + it { is_expected.to belong_to(:project) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:query) } diff --git a/spec/models/push_event_payload_spec.rb b/spec/models/push_event_payload_spec.rb index a049ad35584..69a4922b6fd 100644 --- a/spec/models/push_event_payload_spec.rb +++ b/spec/models/push_event_payload_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe PushEventPayload do + it_behaves_like 'having unique enum values' + describe 'saving payloads' do it 'does not allow commit messages longer than 70 characters' do event = create(:push_event) diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index da61a5f2771..b12ca79847c 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -174,7 +174,15 @@ describe RemoteMirror do end context 'with remote mirroring enabled' do + it 'defaults to disabling only protected branches' do + expect(remote_mirror.only_protected_branches?).to be_falsey + end + context 'with only protected branches enabled' do + before do + remote_mirror.only_protected_branches = true + end + context 'when it did not update in the last minute' do it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 187283b284b..f09b4b67061 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1488,6 +1488,7 @@ describe Repository do :size, :commit_count, :rendered_readme, + :readme_path, :contribution_guide, :changelog, :license_blob, @@ -1874,6 +1875,42 @@ describe Repository do end end + describe '#readme_path', :use_clean_rails_memory_store_caching do + context 'with a non-existing repository' do + let(:project) { create(:project) } + + it 'returns nil' do + expect(repository.readme_path).to be_nil + end + end + + context 'with an existing repository' do + context 'when no README exists' do + let(:project) { create(:project, :empty_repo) } + + it 'returns nil' do + expect(repository.readme_path).to be_nil + end + end + + context 'when a README exists' do + let(:project) { create(:project, :repository) } + + it 'returns the README' do + expect(repository.readme_path).to eq("README.md") + end + + it 'caches the response' do + expect(repository).to receive(:readme).and_call_original.once + + 2.times do + expect(repository.readme_path).to eq("README.md") + end + end + end + end + end + describe '#expire_statistics_caches' do it 'expires the caches' do expect(repository).to receive(:expire_method_caches) @@ -2042,9 +2079,10 @@ describe Repository do describe '#refresh_method_caches' do it 'refreshes the caches of the given types' do expect(repository).to receive(:expire_method_caches) - .with(%i(rendered_readme license_blob license_key license)) + .with(%i(rendered_readme readme_path license_blob license_key license)) expect(repository).to receive(:rendered_readme) + expect(repository).to receive(:readme_path) expect(repository).to receive(:license_blob) expect(repository).to receive(:license_key) expect(repository).to receive(:license) diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index da6e1b5610d..e7e3f7376e6 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -7,6 +7,8 @@ RSpec.describe ResourceLabelEvent, type: :model do let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } + it_behaves_like 'having unique enum values' + describe 'associations' do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:issue) } diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 82ff2a002e0..3682e21ca40 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -236,7 +236,8 @@ describe Todo do create(:todo, target: create(:merge_request)) - expect(described_class.for_target(todo.target)).to eq([todo]) + expect(described_class.for_type(Issue.name).for_target(todo.target)) + .to contain_exactly(todo) end end diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb new file mode 100644 index 00000000000..4a44cf5ab0f --- /dev/null +++ b/spec/models/uploads/fog_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Uploads::Fog do + let(:data_store) { described_class.new } + + before do + stub_uploads_object_storage(FileUploader) + end + + describe '#available?' do + subject { data_store.available? } + + context 'when object storage is enabled' do + it { is_expected.to be_truthy } + end + + context 'when object storage is disabled' do + before do + stub_uploads_object_storage(FileUploader, enabled: false) + end + + it { is_expected.to be_falsy } + end + end + + context 'model with uploads' do + let(:project) { create(:project) } + let(:relation) { project.uploads } + + describe '#keys' do + let!(:uploads) { create_list(:upload, 2, :object_storage, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } + + it 'returns keys' do + is_expected.to match_array(relation.pluck(:path)) + end + end + + describe '#delete_keys' do + let(:keys) { data_store.keys(relation) } + let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } + + before do + uploads.each { |upload| upload.build_uploader.migrate!(2) } + end + + it 'deletes multiple data' do + paths = relation.pluck(:path) + + ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + paths.each do |path| + expect(connection.get_object('uploads', path)[:body]).not_to be_nil + end + end + + subject + + ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + paths.each do |path| + expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) + end + end + end + end + end +end diff --git a/spec/models/uploads/local_spec.rb b/spec/models/uploads/local_spec.rb new file mode 100644 index 00000000000..3468399f370 --- /dev/null +++ b/spec/models/uploads/local_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Uploads::Local do + let(:data_store) { described_class.new } + + before do + stub_uploads_object_storage(FileUploader) + end + + context 'model with uploads' do + let(:project) { create(:project) } + let(:relation) { project.uploads } + + describe '#keys' do + let!(:uploads) { create_list(:upload, 2, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } + + it 'returns keys' do + is_expected.to match_array(relation.map(&:absolute_path)) + end + end + + describe '#delete_keys' do + let(:keys) { data_store.keys(relation) } + let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } + + it 'deletes multiple data' do + paths = relation.map(&:absolute_path) + + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + + subject + + paths.each do |path| + expect(File.exist?(path)).to be_falsey + end + end + end + end +end diff --git a/spec/models/user_callout_spec.rb b/spec/models/user_callout_spec.rb index 64ba17c81fe..d54355afe12 100644 --- a/spec/models/user_callout_spec.rb +++ b/spec/models/user_callout_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe UserCallout do let!(:callout) { create(:user_callout) } + it_behaves_like 'having unique enum values' + describe 'relationships' do it { is_expected.to belong_to(:user) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7bd6dccd0ad..ff075e65c76 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4,6 +4,8 @@ describe User do include ProjectForksHelper include TermsHelper + it_behaves_like 'having unique enum values' + describe 'modules' do subject { described_class } @@ -2323,11 +2325,11 @@ describe User do context 'user is member of all groups' do before do - group.add_owner(user) - nested_group_1.add_owner(user) - nested_group_1_1.add_owner(user) - nested_group_2.add_owner(user) - nested_group_2_1.add_owner(user) + group.add_reporter(user) + nested_group_1.add_developer(user) + nested_group_1_1.add_maintainer(user) + nested_group_2.add_developer(user) + nested_group_2_1.add_maintainer(user) end it 'returns all groups' do @@ -3229,7 +3231,7 @@ describe User do end context 'with uploads' do - it_behaves_like 'model with mounted uploader', false do + it_behaves_like 'model with uploads', false do let(:model_object) { create(:user, :with_avatar) } let(:upload_attribute) { :avatar } let(:uploader_class) { AttachmentUploader } |