diff options
Diffstat (limited to 'spec/models')
27 files changed, 828 insertions, 152 deletions
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 871e8b47650..45a606c1ea8 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -25,6 +25,13 @@ describe Ci::Build do it { is_expected.to be_a(ArtifactMigratable) } + describe 'associations' do + it 'has a bidirectional relationship with projects' do + expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:builds) + expect(Project.reflect_on_association(:builds).has_inverse?).to eq(:project) + end + end + describe 'callbacks' do context 'when running after_create callback' do it 'triggers asynchronous build hooks worker' do @@ -255,6 +262,42 @@ describe Ci::Build do end end + describe '#cache' do + let(:options) { { cache: { key: "key", paths: ["public"], policy: "pull-push" } } } + + subject { build.cache } + + context 'when build has cache' do + before do + allow(build).to receive(:options).and_return(options) + end + + context 'when project has jobs_cache_index' do + before do + allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) + end + + it { is_expected.to be_an(Array).and all(include(key: "key:1")) } + end + + context 'when project does not have jobs_cache_index' do + before do + allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil) + end + + it { is_expected.to eq([options[:cache]]) } + end + end + + context 'when build does not have cache' do + before do + allow(build).to receive(:options).and_return({}) + end + + it { is_expected.to eq([nil]) } + end + end + describe '#depends_on_builds' do let!(:build) { create(:ci_build, pipeline: pipeline, name: 'build', stage_idx: 0, stage: 'build') } let!(:rspec_test) { create(:ci_build, pipeline: pipeline, name: 'rspec', stage_idx: 1, stage: 'test') } diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 9a278212efc..8ee15f0e734 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -12,7 +12,6 @@ describe Ci::PipelineSchedule do it { is_expected.to respond_to(:cron_timezone) } it { is_expected.to respond_to(:description) } it { is_expected.to respond_to(:next_run_at) } - it { is_expected.to respond_to(:deleted_at) } describe 'validations' do it 'does not allow invalid cron patters' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 7bef798a782..14d234f6aab 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -28,6 +28,13 @@ describe Ci::Pipeline, :mailer do it { is_expected.to respond_to :short_sha } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } + 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) + end + end + describe '#source' do context 'when creating new pipeline' do let(:pipeline) do diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 38829773599..f2efcd9d0e9 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -151,11 +151,11 @@ describe CommitRange do .with(commit1, user) .and_return(true) - expect(commit1.has_been_reverted?(user, issue)).to eq(true) + expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(true) end - it 'returns false a commit has not been reverted' do - expect(commit1.has_been_reverted?(user, issue)).to eq(false) + it 'returns false if the commit has not been reverted' do + expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(false) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 4f02dc33cd8..f8a98b43e46 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -181,7 +181,6 @@ eos it { is_expected.to respond_to(:parents) } it { is_expected.to respond_to(:date) } it { is_expected.to respond_to(:diffs) } - it { is_expected.to respond_to(:tree) } it { is_expected.to respond_to(:id) } it { is_expected.to respond_to(:to_patch) } end @@ -440,15 +439,25 @@ eos end describe '#uri_type' do - it 'returns the URI type at the given path' do - expect(commit.uri_type('files/html')).to be(:tree) - expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) - expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) - expect(commit.uri_type('files/js/application.js')).to be(:blob) + shared_examples 'URI type' do + it 'returns the URI type at the given path' do + expect(commit.uri_type('files/html')).to be(:tree) + expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) + expect(commit.uri_type('files/js/application.js')).to be(:blob) + end + + it "returns nil if the path doesn't exists" do + expect(commit.uri_type('this/path/doesnt/exist')).to be_nil + end + end + + context 'when Gitaly commit_tree_entry feature is enabled' do + it_behaves_like 'URI type' end - it "returns nil if the path doesn't exists" do - expect(commit.uri_type('this/path/doesnt/exist')).to be_nil + context 'when Gitaly commit_tree_entry feature is disabled', :disable_gitaly do + it_behaves_like 'URI type' end end @@ -514,4 +523,17 @@ eos expect(described_class.valid_hash?('a' * 41)).to be false end end + + describe '#merge_requests' do + let!(:project) { create(:project, :repository) } + let!(:merge_request1) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } + let!(:merge_request2) { create(:merge_request, source_project: project, source_branch: 'merged-target', target_branch: 'feature') } + let(:commit1) { merge_request1.merge_request_diff.commits.last } + let(:commit2) { merge_request1.merge_request_diff.commits.first } + + it 'returns merge_requests that introduced that commit' do + expect(commit1.merge_requests).to eq([merge_request1, merge_request2]) + expect(commit2.merge_requests).to eq([merge_request1]) + end + end end diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb index cbdc438be0b..3696e6f62fd 100644 --- a/spec/models/concerns/avatarable_spec.rb +++ b/spec/models/concerns/avatarable_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper' describe Avatarable do - subject { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } + set(:project) { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } let(:gitlab_host) { "https://gitlab.example.com" } let(:relative_url_root) { "/gitlab" } - let(:asset_host) { "https://gitlab-assets.example.com" } + let(:asset_host) { 'https://gitlab-assets.example.com' } before do stub_config_setting(base_url: gitlab_host) @@ -15,29 +15,32 @@ describe Avatarable do describe '#avatar_path' do using RSpec::Parameterized::TableSyntax - where(:has_asset_host, :visibility_level, :only_path, :avatar_path) do - true | Project::PRIVATE | true | [gitlab_host, relative_url_root, subject.avatar.url] - true | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url] - true | Project::INTERNAL | true | [gitlab_host, relative_url_root, subject.avatar.url] - true | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url] - true | Project::PUBLIC | true | [subject.avatar.url] - true | Project::PUBLIC | false | [asset_host, subject.avatar.url] - false | Project::PRIVATE | true | [relative_url_root, subject.avatar.url] - false | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url] - false | Project::INTERNAL | true | [relative_url_root, subject.avatar.url] - false | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url] - false | Project::PUBLIC | true | [relative_url_root, subject.avatar.url] - false | Project::PUBLIC | false | [gitlab_host, relative_url_root, subject.avatar.url] + where(:has_asset_host, :visibility_level, :only_path, :avatar_path_prefix) do + true | Project::PRIVATE | true | [gitlab_host, relative_url_root] + true | Project::PRIVATE | false | [gitlab_host, relative_url_root] + true | Project::INTERNAL | true | [gitlab_host, relative_url_root] + true | Project::INTERNAL | false | [gitlab_host, relative_url_root] + true | Project::PUBLIC | true | [] + true | Project::PUBLIC | false | [asset_host] + false | Project::PRIVATE | true | [relative_url_root] + false | Project::PRIVATE | false | [gitlab_host, relative_url_root] + false | Project::INTERNAL | true | [relative_url_root] + false | Project::INTERNAL | false | [gitlab_host, relative_url_root] + false | Project::PUBLIC | true | [relative_url_root] + false | Project::PUBLIC | false | [gitlab_host, relative_url_root] end with_them do before do - allow(ActionController::Base).to receive(:asset_host).and_return(has_asset_host ? asset_host : nil) - subject.visibility_level = visibility_level + allow(ActionController::Base).to receive(:asset_host) { has_asset_host && asset_host } + + project.visibility_level = visibility_level end + let(:avatar_path) { (avatar_path_prefix + [project.avatar.url]).join } + it 'returns the expected avatar path' do - expect(subject.avatar_path(only_path: only_path)).to eq(avatar_path.join) + expect(project.avatar_path(only_path: only_path)).to eq(avatar_path) end end end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb new file mode 100644 index 00000000000..7bb89fe41dc --- /dev/null +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -0,0 +1,73 @@ +require 'rails_helper' + +describe DeploymentPlatform do + let(:project) { create(:project) } + + describe '#deployment_platform' do + subject { project.deployment_platform } + + context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service and a Kubernetes template configured' do + let!(:kubernetes_service) { create(:kubernetes_service, template: true) } + + it 'returns a platform kubernetes' do + expect(subject).to be_a_kind_of(Clusters::Platforms::Kubernetes) + end + + it 'creates a cluster and a platform kubernetes' do + expect { subject } + .to change { Clusters::Cluster.count }.by(1) + .and change { Clusters::Platforms::Kubernetes.count }.by(1) + end + + it 'includes appropriate attributes for Cluster' do + cluster = subject.cluster + expect(cluster.name).to eq('kubernetes-template') + expect(cluster.project).to eq(project) + expect(cluster.provider_type).to eq('user') + expect(cluster.platform_type).to eq('kubernetes') + end + + it 'creates a platform kubernetes' do + expect { subject }.to change { Clusters::Platforms::Kubernetes.count }.by(1) + end + + it 'copies attributes from Clusters::Platform::Kubernetes template into the new Cluster::Platforms::Kubernetes' do + expect(subject.api_url).to eq(kubernetes_service.api_url) + expect(subject.ca_pem).to eq(kubernetes_service.ca_pem) + expect(subject.token).to eq(kubernetes_service.token) + expect(subject.namespace).to eq(kubernetes_service.namespace) + end + end + + context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service and no Kubernetes template configured' do + it { is_expected.to be_nil } + end + + context 'when user 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 + end + + context 'when user configured kubernetes integration from project services' do + let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + it 'returns the Kubernetes service' do + expect(subject).to eq(kubernetes_service) + end + end + + context 'when the cluster creation fails' do + let!(:kubernetes_service) { create(:kubernetes_service, template: true) } + + before do + allow_any_instance_of(Clusters::Cluster).to receive(:persisted?).and_return(false) + end + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb new file mode 100644 index 00000000000..621d2d38eae --- /dev/null +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -0,0 +1,43 @@ +require 'rails_helper' + +RSpec.describe TriggerableHooks do + before do + class TestableHook < WebHook + include TriggerableHooks + triggerable_hooks [:push_hooks] + end + end + + describe 'scopes' do + it 'defines a scope for each of the requested triggers' do + expect(TestableHook).to respond_to :push_hooks + expect(TestableHook).not_to respond_to :tag_push_hooks + end + end + + describe '.hooks_for' do + context 'the model has the required trigger scope' do + it 'returns the record' do + hook = TestableHook.create!(url: 'http://example.com', push_events: true) + + expect(TestableHook.hooks_for(:push_hooks)).to eq [hook] + end + end + + context 'the model does not have the required trigger scope' do + it 'returns an empty relation' do + TestableHook.create!(url: 'http://example.com') + + expect(TestableHook.hooks_for(:tag_push_hooks)).to eq [] + end + end + + context 'the stock scope ".all" is accepted' do + it 'returns the record' do + hook = TestableHook.create!(url: 'http://example.com') + + expect(TestableHook.hooks_for(:all)).to eq [hook] + end + end + end +end diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb index 0345fefb254..fca3090ff4a 100644 --- a/spec/models/deploy_keys_project_spec.rb +++ b/spec/models/deploy_keys_project_spec.rb @@ -8,7 +8,7 @@ describe DeployKeysProject do describe "Validation" do it { is_expected.to validate_presence_of(:project_id) } - it { is_expected.to validate_presence_of(:deploy_key_id) } + it { is_expected.to validate_presence_of(:deploy_key) } end describe "Destroying" do diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 0e965f541d8..8bc45715dcd 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -7,7 +7,8 @@ describe SystemHook do it 'sets defined default parameters' do attrs = { push_events: false, - repository_update_events: true + repository_update_events: true, + merge_requests_events: false } expect(system_hook).to have_attributes(attrs) end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 388120160ab..ea6d6e53ef5 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -29,6 +29,12 @@ describe WebHook do expect(hook.url).to eq('https://example.com') end end + + describe 'token' do + it { is_expected.to allow_value("foobar").for(:token) } + + it { is_expected.not_to allow_values("foo\nbar", "foo\r\nbar").for(:token) } + end end describe 'execute' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 5ced000cdb6..f5c9f551e65 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -18,11 +18,6 @@ describe Issue do subject { create(:issue) } - describe "act_as_paranoid" do - it { is_expected.to have_db_column(:deleted_at) } - it { is_expected.to have_db_index(:deleted_at) } - end - describe 'callbacks' do describe '#ensure_metrics' do it 'creates metrics after saving' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 6aa0e7f49c3..c64cdf8f812 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -488,7 +488,7 @@ describe Member do member.accept_invite!(user) end - it "refreshes user's authorized projects", :truncate do + it "refreshes user's authorized projects", :delete do project = member.source expect(user.authorized_projects).not_to include(project) @@ -523,7 +523,7 @@ describe Member do end end - describe "destroying a record", :truncate do + describe "destroying a record", :delete do it "refreshes user's authorized projects" do project = create(:project, :private) user = create(:user) diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index d556004eccf..b4249d72fc8 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -15,6 +15,28 @@ describe MergeRequestDiff do it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge request diffs' do + expect(by_commit_sha).to eq([merge_request.merge_request_diff]) + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '#latest' do let!(:mr) { create(:merge_request, :with_diffs) } let!(:first_diff) { mr.merge_request_diff } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d8ebd46faab..c76f32b3989 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -24,11 +24,6 @@ describe MergeRequest do it { is_expected.to include_module(Taskable) } end - describe "act_as_paranoid" do - it { is_expected.to have_db_column(:deleted_at) } - it { is_expected.to have_db_index(:deleted_at) } - end - describe 'validation' do it { is_expected.to validate_presence_of(:target_branch) } it { is_expected.to validate_presence_of(:source_branch) } @@ -92,6 +87,39 @@ describe MergeRequest do it { is_expected.to respond_to(:merge_when_pipeline_succeeds) } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge requests' do + expect(by_commit_sha).to eq([merge_request]) + end + end + + context 'with sha contained not in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns empty requests' do + latest_merge_request_diff = merge_request.merge_request_diffs.create + latest_merge_request_diff.merge_request_diff_commits.where(sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0').delete_all + + expect(by_commit_sha).to be_empty + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '.in_projects' do it 'returns the merge requests for a set of projects' do expect(described_class.in_projects(Project.all)).to eq([subject]) @@ -1035,6 +1063,83 @@ describe MergeRequest do end end + describe '#can_be_reverted?' do + context 'when there is no merged_at for the MR' do + before do + subject.metrics.update!(merged_at: nil) + end + + it 'returns false' do + expect(subject.can_be_reverted?(nil)).to be_falsey + end + end + + context 'when there is no merge_commit for the MR' do + before do + subject.metrics.update!(merged_at: Time.now.utc) + end + + it 'returns false' do + expect(subject.can_be_reverted?(nil)).to be_falsey + end + end + + context 'when the MR has been merged' do + before do + MergeRequests::MergeService + .new(subject.target_project, subject.author) + .execute(subject) + end + + context 'when there is no revert commit' do + it 'returns true' do + expect(subject.can_be_reverted?(nil)).to be_truthy + end + end + + context 'when there is a revert commit' do + let(:current_user) { subject.author } + let(:branch) { subject.target_branch } + let(:project) { subject.target_project } + + let(:revert_commit_id) do + params = { + commit: subject.merge_commit, + branch_name: branch, + start_branch: branch + } + + Commits::RevertService.new(project, current_user, params).execute[:result] + end + + before do + project.add_master(current_user) + + ProcessCommitWorker.new.perform(project.id, + current_user.id, + project.commit(revert_commit_id).to_hash, + project.default_branch == branch) + end + + context 'when the revert commit is mentioned in a note after the MR was merged' do + it 'returns false' do + expect(subject.can_be_reverted?(current_user)).to be_falsey + end + end + + context 'when the revert commit is mentioned in a note before the MR was merged' do + before do + subject.notes.last.update!(created_at: subject.metrics.merged_at - 1.second) + end + + it 'returns true' do + expect(subject.can_be_reverted?(current_user)).to be_truthy + end + end + end + end + end + describe '#participants' do let(:project) { create(:project, :public) } @@ -1903,4 +2008,56 @@ describe MergeRequest do end end end + + describe '#should_be_rebased?' do + let(:project) { create(:project, :repository) } + + it 'returns false for the same source and target branches' do + merge_request = create(:merge_request, source_project: project, target_project: project) + + expect(merge_request.should_be_rebased?).to be_falsey + end + end + + describe '#rebase_in_progress?' do + shared_examples 'checking whether a rebase is in progress' do + let(:repo_path) { subject.source_project.repository.path } + let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } + + before do + system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) + end + + it 'returns true when there is a current rebase directory' do + expect(subject.rebase_in_progress?).to be_truthy + end + + it 'returns false when there is no rebase directory' do + FileUtils.rm_rf(rebase_path) + + expect(subject.rebase_in_progress?).to be_falsey + end + + it 'returns false when the rebase directory has expired' do + time = 20.minutes.ago.to_time + File.utime(time, time, rebase_path) + + expect(subject.rebase_in_progress?).to be_falsey + end + + it 'returns false when the source project has been removed' do + allow(subject).to receive(:source_project).and_return(nil) + + expect(subject.rebase_in_progress?).to be_falsey + end + end + + context 'when Gitaly rebase_in_progress is enabled' do + it_behaves_like 'checking whether a rebase is in progress' + end + + context 'when Gitaly rebase_in_progress is enabled', :disable_gitaly do + it_behaves_like 'checking whether a rebase is in progress' + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 86048fad938..d7cff7284fc 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -251,9 +251,13 @@ describe Namespace do parent.update(path: 'mygroup_new') - expect(project_in_parent_group.repo.config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" - expect(hashed_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" - expect(legacy_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" + expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" + expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" + expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" + end + + def project_rugged(project) + project.repository.rugged end end @@ -407,17 +411,6 @@ describe Namespace do end end - describe '#soft_delete_without_removing_associations' do - let(:project1) { create(:project_empty_repo, namespace: namespace) } - - it 'updates the deleted_at timestamp but preserves projects' do - namespace.soft_delete_without_removing_associations - - expect(Project.all).to include(project1) - expect(namespace.deleted_at).not_to be_nil - end - end - describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do expect(namespace.user_ids_for_project_authorizations) diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 7d835511dfb..9d12f96c642 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -68,7 +68,7 @@ describe PagesDomain do subject { domain.url } context 'without the certificate' do - let(:domain) { build(:pages_domain) } + let(:domain) { build(:pages_domain, certificate: '') } it { is_expected.to eq('http://my.domain.com') } end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index 41e2ab20d69..1fccf92627a 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -30,7 +30,7 @@ describe ProjectGroupLink do end end - describe "destroying a record", :truncate do + describe "destroying a record", :delete do it "refreshes group users' authorized projects" do project = create(:project, :private) group = create(:group) diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index 6a5d0decfec..733086e258f 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -92,6 +92,10 @@ describe MicrosoftTeamsService do service.hook_data(merge_request, 'open') end + before do + project.add_developer(user) + end + it "calls Microsoft Teams API" do chat_service.execute(merge_sample_data) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cea22bbd184..31dcb543cbd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -418,14 +418,21 @@ describe Project do end describe '#merge_method' do - it 'returns "ff" merge_method when ff is enabled' do - project = build(:project, merge_requests_ff_only_enabled: true) - expect(project.merge_method).to be :ff + using RSpec::Parameterized::TableSyntax + + where(:ff, :rebase, :method) do + true | true | :ff + true | false | :ff + false | true | :rebase_merge + false | false | :merge end - it 'returns "merge" merge_method when ff is disabled' do - project = build(:project, merge_requests_ff_only_enabled: false) - expect(project.merge_method).to be :merge + with_them do + let(:project) { build(:project, merge_requests_rebase_enabled: rebase, merge_requests_ff_only_enabled: ff) } + + subject { project.merge_method } + + it { is_expected.to eq(method) } end end @@ -1864,9 +1871,8 @@ describe Project do end it 'creates the new reference with rugged' do - expect(project.repository.rugged.references).to receive(:create).with('HEAD', - "refs/heads/#{project.default_branch}", - force: true) + expect(project.repository.raw_repository).to receive(:write_ref).with('HEAD', "refs/heads/#{project.default_branch}", shell: false) + project.change_head(project.default_branch) end @@ -1945,6 +1951,10 @@ describe Project do expect(second_fork.fork_source).to eq(project) end + + it 'returns nil if it is the root of the fork network' do + expect(project.fork_source).to be_nil + end end describe '#lfs_storage_project' do @@ -2632,7 +2642,7 @@ describe Project do project.rename_repo - expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path) end end @@ -2793,7 +2803,7 @@ describe Project do it 'updates project full path in .git/config' do project.rename_repo - expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path) end end @@ -3072,9 +3082,51 @@ describe Project do expect(project).to receive(:import_finish) expect(project).to receive(:update_project_counter_caches) expect(project).to receive(:remove_import_jid) + expect(project).to receive(:after_create_default_branch) project.after_import end + + context 'branch protection' do + let(:project) { create(:project, :repository) } + + it 'does not protect when branch protection is disabled' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + project.after_import + + expect(project.protected_branches).to be_empty + end + + it "gives developer access to push when branch protection is set to 'developers can push'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it "gives developer access to merge when branch protection is set to 'developers can merge'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it 'protects default branch' do + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + end + end end describe '#update_project_counter_caches' do @@ -3137,38 +3189,19 @@ describe Project do end end - describe '#deployment_platform' do - subject { project.deployment_platform } - - let(:project) { create(:project) } - - context 'when user configured kubernetes from Integration > Kubernetes' do - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - it { is_expected.to eq(kubernetes_service) } - end - - context 'when user configured kubernetes from CI/CD > Clusters' do - let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - let(:platform_kubernetes) { cluster.platform_kubernetes } - - it { is_expected.to eq(platform_kubernetes) } - end - end - describe '#write_repository_config' do set(:project) { create(:project, :repository) } it 'writes full path in .git/config when key is missing' do project.write_repository_config - expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path end it 'updates full path in .git/config when key is present' do project.write_repository_config(gl_full_path: 'old/path') - expect { project.write_repository_config }.to change { project.repo.config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + expect { project.write_repository_config }.to change { project.repository.rugged.config['gitlab.fullpath'] }.from('old/path').to(project.full_path) end it 'does not raise an error with an empty repository' do @@ -3177,4 +3210,40 @@ describe Project do expect { project.write_repository_config }.not_to raise_error end end + + describe '#execute_hooks' do + it 'executes the projects hooks with the specified scope' do + hook1 = create(:project_hook, merge_requests_events: true, tag_push_events: false) + hook2 = create(:project_hook, merge_requests_events: false, tag_push_events: true) + project = create(:project, hooks: [hook1, hook2]) + + expect_any_instance_of(ProjectHook).to receive(:async_execute).once + + project.execute_hooks({}, :tag_push_hooks) + end + + it 'executes the system hooks with the specified scope' do + expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with({ data: 'data' }, :merge_request_hooks) + + project = build(:project) + project.execute_hooks({ data: 'data' }, :merge_request_hooks) + end + + it 'executes the system hooks when inside a transaction' do + allow_any_instance_of(WebHookService).to receive(:execute) + + create(:system_hook, merge_requests_events: true) + + project = build(:project) + + # Ideally, we'd test that `WebHookWorker.jobs.size` increased by 1, + # but since the entire spec run takes place in a transaction, we never + # actually get to the `after_commit` hook that queues these jobs. + expect do + project.transaction do + project.execute_hooks({ data: 'data' }, :merge_request_hooks) + end + end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError + end + end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index e78ed1df821..5cff2af4aca 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -146,6 +146,12 @@ describe ProjectStatistics do expect(statistics.build_artifacts_size).to be(106365) end + + it 'calculates related build artifacts by project' do + expect(Ci::JobArtifact).to receive(:artifacts_size_for).with(project) { 0 } + + statistics.update_build_artifacts_size + end end context 'when legacy artifacts are used' do diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb index ad3c3a406d9..bfe7a30b96a 100644 --- a/spec/models/push_event_spec.rb +++ b/spec/models/push_event_spec.rb @@ -63,12 +63,14 @@ describe PushEvent do let(:event2) { create(:push_event, project: project) } let(:event3) { create(:push_event, project: project) } let(:event4) { create(:push_event, project: project) } + let(:event5) { create(:push_event, project: project) } before do create(:push_event_payload, event: event1, ref: 'foo', action: :created) create(:push_event_payload, event: event2, ref: 'bar', action: :created) - create(:push_event_payload, event: event3, ref: 'baz', action: :removed) - create(:push_event_payload, event: event4, ref: 'baz', ref_type: :tag) + create(:push_event_payload, event: event3, ref: 'qux', action: :created) + create(:push_event_payload, event: event4, ref: 'baz', action: :removed) + create(:push_event_payload, event: event5, ref: 'baz', ref_type: :tag) project.repository.create_branch('bar', 'master') @@ -78,6 +80,16 @@ describe PushEvent do target_project: project, source_branch: 'bar' ) + + project.repository.create_branch('qux', 'master') + + create( + :merge_request, + :closed, + source_project: project, + target_project: project, + source_branch: 'qux' + ) end let(:relation) { described_class.without_existing_merge_requests } @@ -86,16 +98,20 @@ describe PushEvent do expect(relation).to include(event1) end - it 'does not include events that have a corresponding merge request' do + it 'does not include events that have a corresponding open merge request' do expect(relation).not_to include(event2) end + it 'includes events that has corresponding closed/merged merge requests' do + expect(relation).to include(event3) + end + it 'does not include events for removed refs' do - expect(relation).not_to include(event3) + expect(relation).not_to include(event4) end it 'does not include events for pushing to tags' do - expect(relation).not_to include(event4) + expect(relation).not_to include(event5) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9a68ae086ea..8f406253f39 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -358,28 +358,44 @@ describe Repository do end describe '#can_be_merged?' do - context 'mergeable branches' do - subject { repository.can_be_merged?('0b4bc9a49b562e85de7cc9e834518ea6828729b9', 'master') } + shared_examples 'can be merged' do + context 'mergeable branches' do + subject { repository.can_be_merged?('0b4bc9a49b562e85de7cc9e834518ea6828729b9', 'master') } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } + end - context 'non-mergeable branches' do - subject { repository.can_be_merged?('bb5206fee213d983da88c47f9cf4cc6caf9c66dc', 'feature') } + context 'non-mergeable branches without conflict sides missing' do + subject { repository.can_be_merged?('bb5206fee213d983da88c47f9cf4cc6caf9c66dc', 'feature') } - it { is_expected.to be_falsey } - end + it { is_expected.to be_falsey } + end - context 'non merged branch' do - subject { repository.merged_to_root_ref?('fix') } + context 'non-mergeable branches with conflict sides missing' do + subject { repository.can_be_merged?('conflict-missing-side', 'conflict-start') } - it { is_expected.to be_falsey } + it { is_expected.to be_falsey } + end + + context 'non merged branch' do + subject { repository.merged_to_root_ref?('fix') } + + it { is_expected.to be_falsey } + end + + context 'non existent branch' do + subject { repository.merged_to_root_ref?('non_existent_branch') } + + it { is_expected.to be_nil } + end end - context 'non existent branch' do - subject { repository.merged_to_root_ref?('non_existent_branch') } + context 'when Gitaly can_be_merged feature is enabled' do + it_behaves_like 'can be merged' + end - it { is_expected.to be_nil } + context 'when Gitaly can_be_merged feature is disabled', :disable_gitaly do + it_behaves_like 'can be merged' end end @@ -412,6 +428,28 @@ describe Repository do end end + describe '#create_hooks' do + let(:hook_path) { File.join(repository.path_to_repo, 'hooks') } + + it 'symlinks the global hooks directory' do + repository.create_hooks + + expect(File.symlink?(hook_path)).to be true + expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) + end + + it 'replaces existing symlink with the right directory' do + FileUtils.mkdir_p(hook_path) + + expect(File.symlink?(hook_path)).to be false + + repository.create_hooks + + expect(File.symlink?(hook_path)).to be true + expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) + end + end + describe "#create_dir" do it "commits a change that creates a new directory" do expect do @@ -582,38 +620,6 @@ describe Repository do end end - describe '#get_committer_and_author' do - it 'returns the committer and author data' do - options = repository.get_committer_and_author(user) - expect(options[:committer][:email]).to eq(user.email) - expect(options[:author][:email]).to eq(user.email) - end - - context 'when the email/name are given' do - it 'returns an object containing the email/name' do - options = repository.get_committer_and_author(user, email: author_email, name: author_name) - expect(options[:author][:email]).to eq(author_email) - expect(options[:author][:name]).to eq(author_name) - end - end - - context 'when the email is given but the name is not' do - it 'returns the committer as the author' do - options = repository.get_committer_and_author(user, email: author_email) - expect(options[:author][:email]).to eq(user.email) - expect(options[:author][:name]).to eq(user.name) - end - end - - context 'when the name is given but the email is not' do - it 'returns nil' do - options = repository.get_committer_and_author(user, name: author_name) - expect(options[:author][:email]).to eq(user.email) - expect(options[:author][:name]).to eq(user.name) - end - end - end - describe "search_files_by_content" do let(:results) { repository.search_files_by_content('feature', 'master') } subject { results } @@ -657,7 +663,7 @@ describe Repository do subject { results.first } it { is_expected.to be_an String } - it { expect(subject.lines[2]).to eq("master:CHANGELOG:190: - Feature: Replace teams with group membership\n") } + it { expect(subject.lines[2]).to eq("master:CHANGELOG\x00190\x00 - Feature: Replace teams with group membership\n") } end end @@ -668,6 +674,18 @@ describe Repository do expect(results.first).to eq('files/html/500.html') end + it 'ignores leading slashes' do + results = repository.search_files_by_name('/files', 'master') + + expect(results.first).to eq('files/html/500.html') + end + + it 'properly handles when query is only slashes' do + results = repository.search_files_by_name('//', 'master') + + expect(results).to match_array([]) + end + it 'properly handles when query is not present' do results = repository.search_files_by_name('', 'master') @@ -1112,16 +1130,16 @@ describe Repository do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) end - it 'expires branch cache' do - expect(repository).not_to receive(:expire_exists_cache) - expect(repository).not_to receive(:expire_root_ref_cache) - expect(repository).not_to receive(:expire_emptiness_caches) - expect(repository).to receive(:expire_branches_cache) - - repository.with_branch(user, 'new-feature') do + subject do + Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('new-feature') do new_rev end end + + it 'returns branch_created as true' do + expect(subject).not_to be_repo_created + expect(subject).to be_branch_created + end end context 'when repository is empty' do @@ -2215,6 +2233,15 @@ describe Repository do end end + describe '#diverging_commit_counts' do + it 'returns the commit counts behind and ahead of default branch' do + result = repository.diverging_commit_counts( + repository.find_branch('fix')) + + expect(result).to eq(behind: 29, ahead: 2) + end + end + describe '#cache_method_output', :use_clean_rails_memory_store_caching do let(:fallback) { 10 } diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index ddad6862a63..88f54fd10e5 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -16,9 +16,76 @@ describe Route do it { is_expected.to validate_presence_of(:source) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_uniqueness_of(:path).case_insensitive } + + describe '#ensure_permanent_paths' do + context 'when the route is not yet persisted' do + let(:new_route) { described_class.new(path: 'foo', source: build(:group)) } + + context 'when permanent conflicting redirects exist' do + it 'is invalid' do + redirect = build(:redirect_route, :permanent, path: 'foo/bar/baz') + redirect.save!(validate: false) + + expect(new_route.valid?).to be_falsey + expect(new_route.errors.first[1]).to eq('foo has been taken before. Please use another one') + end + end + + context 'when no permanent conflicting redirects exist' do + it 'is valid' do + expect(new_route.valid?).to be_truthy + end + end + end + + context 'when path has changed' do + before do + route.path = 'foo' + end + + context 'when permanent conflicting redirects exist' do + it 'is invalid' do + redirect = build(:redirect_route, :permanent, path: 'foo/bar/baz') + redirect.save!(validate: false) + + expect(route.valid?).to be_falsey + expect(route.errors.first[1]).to eq('foo has been taken before. Please use another one') + end + end + + context 'when no permanent conflicting redirects exist' do + it 'is valid' do + expect(route.valid?).to be_truthy + end + end + end + + context 'when path has not changed' do + context 'when permanent conflicting redirects exist' do + it 'is valid' do + redirect = build(:redirect_route, :permanent, path: 'git_lab/foo/bar') + redirect.save!(validate: false) + + expect(route.valid?).to be_truthy + end + end + context 'when no permanent conflicting redirects exist' do + it 'is valid' do + expect(route.valid?).to be_truthy + end + end + end + end end describe 'callbacks' do + context 'before validation' do + it 'calls #delete_conflicting_orphaned_routes' do + expect(route).to receive(:delete_conflicting_orphaned_routes) + route.valid? + end + end + context 'after update' do it 'calls #create_redirect_for_old_path' do expect(route).to receive(:create_redirect_for_old_path) @@ -318,4 +385,58 @@ describe Route do end end end + + describe '#delete_conflicting_orphaned_routes' do + context 'when there is a conflicting route' do + let!(:conflicting_group) { create(:group, path: 'foo') } + + before do + route.path = conflicting_group.route.path + end + + context 'when the route is orphaned' do + let!(:offending_route) { conflicting_group.route } + + before do + Group.delete(conflicting_group) # Orphan the route + end + + it 'deletes the orphaned route' do + expect do + route.valid? + end.to change { described_class.count }.from(2).to(1) + end + + it 'passes validation, as usual' do + expect(route.valid?).to be_truthy + end + end + + context 'when the route is not orphaned' do + it 'does not delete the conflicting route' do + expect do + route.valid? + end.not_to change { described_class.count } + end + + it 'fails validation, as usual' do + expect(route.valid?).to be_falsey + end + end + end + + context 'when there are no conflicting routes' do + it 'does not delete any routes' do + route + + expect do + route.valid? + end.not_to change { described_class.count } + end + + it 'passes validation, as usual' do + expect(route.valid?).to be_truthy + end + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 540615de117..79f25dc4360 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -272,4 +272,46 @@ describe Service do expect(service.deprecation_message).to be_nil end end + + describe '.find_by_template' do + let!(:kubernetes_service) { create(:kubernetes_service, template: true) } + + it 'returns service template' do + expect(KubernetesService.find_by_template).to eq(kubernetes_service) + end + end + + describe '#api_field_names' do + let(:fake_service) do + Class.new(Service) do + def fields + [ + { name: 'token' }, + { name: 'api_token' }, + { name: 'key' }, + { name: 'api_key' }, + { name: 'password' }, + { name: 'password_field' }, + { name: 'safe_field' } + ] + end + end + end + + let(:service) do + fake_service.new(properties: [ + { token: 'token-value' }, + { api_token: 'api_token-value' }, + { key: 'key-value' }, + { api_key: 'api_key-value' }, + { password: 'password-value' }, + { password_field: 'password_field-value' }, + { safe_field: 'safe_field-value' } + ]) + end + + it 'filters out sensitive fields' do + expect(service.api_field_names).to eq(['safe_field']) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8d0eaf565a7..594f23718da 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -966,6 +966,14 @@ describe User do expect(described_class.search(user3.username.upcase)).to eq([user3]) end end + + it 'returns no matches for an empty string' do + expect(described_class.search('')).to be_empty + end + + it 'returns no matches for nil' do + expect(described_class.search(nil)).to be_empty + end end describe '.search_with_secondary_emails' do @@ -1020,6 +1028,14 @@ describe User do it 'does not return users with a matching part of secondary email' do expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user]) end + + it 'returns no matches for an empty string' do + expect(search_with_secondary_emails('')).to be_empty + end + + it 'returns no matches for nil' do + expect(search_with_secondary_emails(nil)).to be_empty + end end describe '.find_by_ssh_key_id' do @@ -1553,7 +1569,7 @@ describe User do it { is_expected.to eq([private_group]) } end - describe '#authorized_projects', :truncate do + describe '#authorized_projects', :delete do context 'with a minimum access level' do it 'includes projects for which the user is an owner' do user = create(:user) diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index ea75434e399..cc9d79da708 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -386,6 +386,17 @@ describe WikiPage do end end + describe '#formatted_content' do + it 'returns processed content of the page', :disable_gitaly do + subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" }) + page = wiki.find_page('RDoc') + + expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n") + + destroy_page('RDoc') + end + end + private def remove_temp_repo(path) |