diff options
Diffstat (limited to 'spec/models')
67 files changed, 3219 insertions, 1187 deletions
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 02d6263094a..219db365a91 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe BroadcastMessage, models: true do - subject { create(:broadcast_message) } + subject { build(:broadcast_message) } it { is_expected.to be_valid } diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ae185de9ca3..6f1c2ae0fd8 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -87,6 +87,26 @@ describe Ci::Build, models: true do end end + describe '#persisted_environment' do + before do + @environment = create(:environment, project: project, name: "foo-#{project.default_branch}") + end + + subject { build.persisted_environment } + + context 'referenced literally' do + let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-#{project.default_branch}") } + + it { is_expected.to eq(@environment) } + end + + context 'referenced with a variable' do + let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-$CI_BUILD_REF_NAME") } + + it { is_expected.to eq(@environment) } + end + end + describe '#trace' do it { expect(build.trace).to be_nil } @@ -254,6 +274,24 @@ describe Ci::Build, models: true do end end + describe '#ref_slug' do + { + 'master' => 'master', + '1-foo' => '1-foo', + 'fix/1-foo' => 'fix-1-foo', + 'fix-1-foo' => 'fix-1-foo', + 'a' * 63 => 'a' * 63, + 'a' * 64 => 'a' * 63, + 'FOO' => 'foo', + }.each do |ref, slug| + it "transforms #{ref} to #{slug}" do + build.ref = ref + + expect(build.ref_slug).to eq(slug) + end + end + end + describe '#variables' do let(:container_registry_enabled) { false } let(:predefined_variables) do @@ -265,6 +303,7 @@ describe Ci::Build, models: true do { key: 'CI_BUILD_REF', value: build.sha, public: true }, { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, { key: 'CI_BUILD_REF_NAME', value: 'master', public: true }, + { key: 'CI_BUILD_REF_SLUG', value: 'master', public: true }, { key: 'CI_BUILD_NAME', value: 'test', public: true }, { key: 'CI_BUILD_STAGE', value: 'test', public: true }, { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, @@ -309,6 +348,22 @@ describe Ci::Build, models: true do it { user_variables.each { |v| is_expected.to include(v) } } end + context 'when build has an environment' do + before do + build.update(environment: 'production') + create(:environment, project: build.project, name: 'production', slug: 'prod-slug') + end + + let(:environment_variables) do + [ + { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true }, + { key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true } + ] + end + + it { environment_variables.each { |v| is_expected.to include(v) } } + end + context 'when build started manually' do before do build.update_attributes(when: :manual) @@ -451,6 +506,17 @@ describe Ci::Build, models: true do it { is_expected.to include({ key: 'CI_RUNNER_TAGS', value: 'docker, linux', public: true }) } end + context 'when build is for a deployment' do + let(:deployment_variable) { { key: 'KUBERNETES_TOKEN', value: 'TOKEN', public: false } } + + before do + build.environment = 'production' + allow(project).to receive(:deployment_variables).and_return([deployment_variable]) + end + + it { is_expected.to include(deployment_variable) } + end + context 'returns variables in valid order' do before do allow(build).to receive(:predefined_variables) { ['predefined'] } @@ -730,8 +796,8 @@ describe Ci::Build, models: true do pipeline2 = create(:ci_pipeline, project: project) @build2 = create(:ci_build, pipeline: pipeline2) - commits = [double(id: pipeline.sha), double(id: pipeline2.sha)] - allow(@merge_request).to receive(:commits).and_return(commits) + allow(@merge_request).to receive(:commits_sha). + and_return([pipeline.sha, pipeline2.sha]) allow(MergeRequest).to receive_message_chain(:includes, :where, :reorder).and_return([@merge_request]) end @@ -899,21 +965,87 @@ describe Ci::Build, models: true do end end + describe '#cancelable?' do + subject { build } + + context 'when build is cancelable' do + context 'when build is pending' do + it { is_expected.to be_cancelable } + end + + context 'when build is running' do + before do + build.run! + end + + it { is_expected.to be_cancelable } + end + end + + context 'when build is not cancelable' do + context 'when build is successful' do + before do + build.success! + end + + it { is_expected.not_to be_cancelable } + end + + context 'when build is failed' do + before do + build.drop! + end + + it { is_expected.not_to be_cancelable } + end + end + end + describe '#retryable?' do - context 'when build is running' do - before do - build.run! + subject { build } + + context 'when build is retryable' do + context 'when build is successful' do + before do + build.success! + end + + it { is_expected.to be_retryable } end - it { expect(build).not_to be_retryable } + context 'when build is failed' do + before do + build.drop! + end + + it { is_expected.to be_retryable } + end + + context 'when build is canceled' do + before do + build.cancel! + end + + it { is_expected.to be_retryable } + end end - context 'when build is finished' do - before do - build.success! + context 'when build is not retryable' do + context 'when build is running' do + before do + build.run! + end + + it { is_expected.not_to be_retryable } end - it { expect(build).to be_retryable } + context 'when build is skipped' do + before do + build.skip! + end + + it { is_expected.not_to be_retryable } + end end end @@ -1052,4 +1184,141 @@ describe Ci::Build, models: true do end end end + + describe '#has_environment?' do + subject { build.has_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + it { is_expected.to be_truthy } + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + + describe '#starts_environment?' do + subject { build.starts_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + context 'no action is defined' do + it { is_expected.to be_truthy } + end + + context 'and start action is defined' do + before do + build.update(options: { environment: { action: 'start' } } ) + end + + it { is_expected.to be_truthy } + end + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + + describe '#stops_environment?' do + subject { build.stops_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + context 'no action is defined' do + it { is_expected.to be_falsey } + end + + context 'and stop action is defined' do + before do + build.update(options: { environment: { action: 'stop' } } ) + end + + it { is_expected.to be_truthy } + end + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + + describe '#last_deployment' do + subject { build.last_deployment } + + context 'when multiple deployments are created' do + let!(:deployment1) { create(:deployment, deployable: build) } + let!(:deployment2) { create(:deployment, deployable: build) } + + it 'returns the latest one' do + is_expected.to eq(deployment2) + end + end + end + + describe '#outdated_deployment?' do + subject { build.outdated_deployment? } + + context 'when build succeeded' do + let(:build) { create(:ci_build, :success) } + let!(:deployment) { create(:deployment, deployable: build) } + + context 'current deployment is latest' do + it { is_expected.to be_falsey } + end + + context 'current deployment is not latest on environment' do + let!(:deployment2) { create(:deployment, environment: deployment.environment) } + + it { is_expected.to be_truthy } + end + end + + context 'when build failed' do + let(:build) { create(:ci_build, :failed) } + + it { is_expected.to be_falsey } + end + end + + describe '#expanded_environment_name' do + subject { build.expanded_environment_name } + + context 'when environment uses variables' do + let(:build) { create(:ci_build, ref: 'master', environment: 'review/$CI_BUILD_REF_NAME') } + + it { is_expected.to eq('review/master') } + end + end + + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns a detailed status' do + expect(build.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Build::Cancelable + end + end end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb new file mode 100644 index 00000000000..b02971cab82 --- /dev/null +++ b/spec/models/chat_name_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe ChatName, models: true do + subject { create(:chat_name) } + + it { is_expected.to belong_to(:service) } + it { is_expected.to belong_to(:user) } + + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:service) } + it { is_expected.to validate_presence_of(:team_id) } + it { is_expected.to validate_presence_of(:chat_id) } + + it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:service_id) } + it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:service_id, :team_id) } +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a37a00f461a..a7e90c8a381 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4,6 +4,12 @@ describe Ci::Build, models: true do let(:build) { create(:ci_build) } let(:test_trace) { 'This is a test' } + it { is_expected.to belong_to(:runner) } + it { is_expected.to belong_to(:trigger_request) } + it { is_expected.to belong_to(:erased_by) } + + it { is_expected.to have_many(:deployments) } + describe '#trace' do it 'obfuscates project runners token' do allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}") diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 5eb14dc6bd2..52dd41065e9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Ci::Pipeline, models: true do + include EmailHelpers + let(:project) { FactoryGirl.create :empty_project } let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, status: 'created', project: project } @@ -18,8 +20,6 @@ describe Ci::Pipeline, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } - it { is_expected.to delegate_method(:stages).to(:statuses) } - describe '#valid_commit_sha' do context 'commit.sha can not start with 00000000' do before do @@ -123,16 +123,55 @@ describe Ci::Pipeline, models: true do end describe '#stages' do - let(:pipeline2) { FactoryGirl.create :ci_pipeline, project: project } - subject { CommitStatus.where(pipeline: [pipeline, pipeline2]).stages } - before do - FactoryGirl.create :ci_build, pipeline: pipeline2, stage: 'test', stage_idx: 1 - FactoryGirl.create :ci_build, pipeline: pipeline, stage: 'build', stage_idx: 0 + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'linux', stage_idx: 0, status: 'success') + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'failed') + create(:commit_status, pipeline: pipeline, stage: 'deploy', name: 'staging', stage_idx: 2, status: 'running') + create(:commit_status, pipeline: pipeline, stage: 'test', name: 'rspec', stage_idx: 1, status: 'success') + end + + subject { pipeline.stages } + + context 'stages list' do + it 'returns ordered list of stages' do + expect(subject.map(&:name)).to eq(%w[build test deploy]) + end end - it 'return all stages' do - is_expected.to eq(%w(build test)) + it 'returns a valid number of stages' do + expect(pipeline.stages_count).to eq(3) + end + + it 'returns a valid names of stages' do + expect(pipeline.stages_name).to eq(['build', 'test', 'deploy']) + end + + context 'stages with statuses' do + let(:statuses) do + subject.map do |stage| + [stage.name, stage.status] + end + end + + it 'returns list of stages with statuses' do + expect(statuses).to eq([['build', 'failed'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + + context 'when build is retried' do + before do + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'success') + end + + it 'ignores the previous state' do + expect(statuses).to eq([['build', 'success'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + end end end @@ -342,6 +381,65 @@ describe Ci::Pipeline, models: true do end end + shared_context 'with some outdated pipelines' do + before do + create_pipeline(:canceled, 'ref', 'A') + create_pipeline(:success, 'ref', 'A') + create_pipeline(:failed, 'ref', 'B') + create_pipeline(:skipped, 'feature', 'C') + end + + def create_pipeline(status, ref, sha) + create(:ci_empty_pipeline, status: status, ref: ref, sha: sha) + end + end + + describe '.latest' do + include_context 'with some outdated pipelines' + + context 'when no ref is specified' do + let(:pipelines) { described_class.latest.all } + + it 'returns the latest pipeline for the same ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed', 'skipped') + end + end + + context 'when ref is specified' do + let(:pipelines) { described_class.latest('ref').all } + + it 'returns the latest pipeline for ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed') + end + end + end + + describe '.latest_status' do + include_context 'with some outdated pipelines' + + context 'when no ref is specified' do + let(:latest_status) { described_class.latest_status } + + it 'returns the latest status for the same ref and different sha' do + expect(latest_status).to eq(described_class.latest.status) + expect(latest_status).to eq('failed') + end + end + + context 'when ref is specified' do + let(:latest_status) { described_class.latest_status('ref') } + + it 'returns the latest status for ref and different sha' do + expect(latest_status).to eq(described_class.latest_status('ref')) + expect(latest_status).to eq('failed') + end + end + end + describe '#status' do let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } @@ -402,6 +500,234 @@ describe Ci::Pipeline, models: true do end end + describe '#detailed_status' do + let(:user) { create(:user) } + + subject { pipeline.detailed_status(user) } + + context 'when pipeline is created' do + let(:pipeline) { create(:ci_pipeline, status: :created) } + + it 'returns detailed status for created pipeline' do + expect(subject.text).to eq 'created' + end + end + + context 'when pipeline is pending' do + let(:pipeline) { create(:ci_pipeline, status: :pending) } + + it 'returns detailed status for pending pipeline' do + expect(subject.text).to eq 'pending' + end + end + + context 'when pipeline is running' do + let(:pipeline) { create(:ci_pipeline, status: :running) } + + it 'returns detailed status for running pipeline' do + expect(subject.text).to eq 'running' + end + end + + context 'when pipeline is successful' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + it 'returns detailed status for successful pipeline' do + expect(subject.text).to eq 'passed' + end + end + + context 'when pipeline is failed' do + let(:pipeline) { create(:ci_pipeline, status: :failed) } + + it 'returns detailed status for failed pipeline' do + expect(subject.text).to eq 'failed' + end + end + + context 'when pipeline is canceled' do + let(:pipeline) { create(:ci_pipeline, status: :canceled) } + + it 'returns detailed status for canceled pipeline' do + expect(subject.text).to eq 'canceled' + end + end + + context 'when pipeline is skipped' do + let(:pipeline) { create(:ci_pipeline, status: :skipped) } + + it 'returns detailed status for skipped pipeline' do + expect(subject.text).to eq 'skipped' + end + end + + context 'when pipeline is successful but with warnings' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + before do + create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline) + end + + it 'retruns detailed status for successful pipeline with warnings' do + expect(subject.label).to eq 'passed with warnings' + end + end + end + + describe '#cancelable?' do + %i[created running pending].each do |status0| + context "when there is a build #{status0}" do + before do + create(:ci_build, status0, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + context "when there is an external job #{status0}" do + before do + create(:generic_commit_status, status0, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + %i[success failed canceled].each do |status1| + context "when there are generic_commit_status jobs for #{status0} and #{status1}" do + before do + create(:generic_commit_status, status0, pipeline: pipeline) + create(:generic_commit_status, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + context "when there are generic_commit_status and ci_build jobs for #{status0} and #{status1}" do + before do + create(:generic_commit_status, status0, pipeline: pipeline) + create(:ci_build, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + context "when there are ci_build jobs for #{status0} and #{status1}" do + before do + create(:ci_build, status0, pipeline: pipeline) + create(:ci_build, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + end + end + + %i[success failed canceled].each do |status| + context "when there is a build #{status}" do + before do + create(:ci_build, status, pipeline: pipeline) + end + + it 'is not cancelable' do + expect(pipeline.cancelable?).to be_falsey + end + end + + context "when there is an external job #{status}" do + before do + create(:generic_commit_status, status, pipeline: pipeline) + end + + it 'is not cancelable' do + expect(pipeline.cancelable?).to be_falsey + end + end + end + end + + describe '#cancel_running' do + let(:latest_status) { pipeline.statuses.pluck(:status) } + + context 'when there is a running external job and created build' do + before do + create(:ci_build, :running, pipeline: pipeline) + create(:generic_commit_status, :running, pipeline: pipeline) + + pipeline.cancel_running + end + + it 'cancels both jobs' do + expect(latest_status).to contain_exactly('canceled', 'canceled') + end + end + + context 'when builds are in different stages' do + before do + create(:ci_build, :running, stage_idx: 0, pipeline: pipeline) + create(:ci_build, :running, stage_idx: 1, pipeline: pipeline) + + pipeline.cancel_running + end + + it 'cancels both jobs' do + expect(latest_status).to contain_exactly('canceled', 'canceled') + end + end + end + + describe '#retry_failed' do + let(:latest_status) { pipeline.statuses.latest.pluck(:status) } + + context 'when there is a failed build and failed external status' do + before do + create(:ci_build, :failed, name: 'build', pipeline: pipeline) + create(:generic_commit_status, :failed, name: 'jenkins', pipeline: pipeline) + + pipeline.retry_failed(create(:user)) + end + + it 'retries only build' do + expect(latest_status).to contain_exactly('pending', 'failed') + end + end + + context 'when builds are in different stages' do + before do + create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) + create(:ci_build, :failed, name: 'jenkins', stage_idx: 1, pipeline: pipeline) + + pipeline.retry_failed(create(:user)) + end + + it 'retries both builds' do + expect(latest_status).to contain_exactly('pending', 'pending') + end + end + + context 'when there are canceled and failed' do + before do + create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) + create(:ci_build, :canceled, name: 'jenkins', stage_idx: 1, pipeline: pipeline) + + pipeline.retry_failed(create(:user)) + end + + it 'retries both builds' do + expect(latest_status).to contain_exactly('pending', 'pending') + end + end + end + describe '#execute_hooks' do let!(:build_a) { create_build('a', 0) } let!(:build_b) { create_build('b', 1) } @@ -524,4 +850,81 @@ describe Ci::Pipeline, models: true do expect(pipeline.merge_requests).to be_empty end end + + describe 'notifications when pipeline success or failed' do + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit('master').sha, + user: create(:user)) + end + + before do + reset_delivered_emails! + + project.team << [pipeline.user, Gitlab::Access::DEVELOPER] + + perform_enqueued_jobs do + pipeline.enqueue + pipeline.run + end + end + + shared_examples 'sending a notification' do + it 'sends an email' do + should_only_email(pipeline.user, kind: :bcc) + end + end + + shared_examples 'not sending any notification' do + it 'does not send any email' do + should_not_email_anyone + end + end + + context 'with success pipeline' do + before do + perform_enqueued_jobs do + pipeline.succeed + end + end + + it_behaves_like 'sending a notification' + end + + context 'with failed pipeline' do + before do + perform_enqueued_jobs do + create(:ci_build, :failed, pipeline: pipeline) + create(:generic_commit_status, :failed, pipeline: pipeline) + + pipeline.drop + end + end + + it_behaves_like 'sending a notification' + end + + context 'with skipped pipeline' do + before do + perform_enqueued_jobs do + pipeline.skip + end + end + + it_behaves_like 'not sending any notification' + end + + context 'with cancelled pipeline' do + before do + perform_enqueued_jobs do + pipeline.cancel + end + end + + it_behaves_like 'not sending any notification' + end + end end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb new file mode 100644 index 00000000000..8fff38f7cda --- /dev/null +++ b/spec/models/ci/stage_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' + +describe Ci::Stage, models: true do + let(:stage) { build(:ci_stage) } + let(:pipeline) { stage.pipeline } + let(:stage_name) { stage.name } + + describe '#expectations' do + subject { stage } + + it { is_expected.to include_module(StaticModel) } + + it { is_expected.to respond_to(:pipeline) } + it { is_expected.to respond_to(:name) } + + it { is_expected.to delegate_method(:project).to(:pipeline) } + end + + describe '#statuses' do + let!(:stage_build) { create_job(:ci_build) } + let!(:commit_status) { create_job(:commit_status) } + let!(:other_build) { create_job(:ci_build, stage: 'other stage') } + + subject { stage.statuses } + + it "returns only matching statuses" do + is_expected.to contain_exactly(stage_build, commit_status) + end + end + + describe '#builds' do + let!(:stage_build) { create_job(:ci_build) } + let!(:commit_status) { create_job(:commit_status) } + + subject { stage.builds } + + it "returns only builds" do + is_expected.to contain_exactly(stage_build) + end + end + + describe '#status' do + subject { stage.status } + + context 'if status is already defined' do + let(:stage) { build(:ci_stage, status: 'success') } + + it "returns defined status" do + is_expected.to eq('success') + end + end + + context 'if status has to be calculated' do + let!(:stage_build) { create_job(:ci_build, status: :failed) } + + it "returns status of a build" do + is_expected.to eq('failed') + end + + context 'and builds are retried' do + let!(:new_build) { create_job(:ci_build, status: :success) } + + it "returns status of latest build" do + is_expected.to eq('success') + end + end + end + end + + describe '#detailed_status' do + let(:user) { create(:user) } + + subject { stage.detailed_status(user) } + + context 'when build is created' do + let!(:stage_build) { create_job(:ci_build, status: :created) } + + it 'returns detailed status for created stage' do + expect(subject.text).to eq 'created' + end + end + + context 'when build is pending' do + let!(:stage_build) { create_job(:ci_build, status: :pending) } + + it 'returns detailed status for pending stage' do + expect(subject.text).to eq 'pending' + end + end + + context 'when build is running' do + let!(:stage_build) { create_job(:ci_build, status: :running) } + + it 'returns detailed status for running stage' do + expect(subject.text).to eq 'running' + end + end + + context 'when build is successful' do + let!(:stage_build) { create_job(:ci_build, status: :success) } + + it 'returns detailed status for successful stage' do + expect(subject.text).to eq 'passed' + end + end + + context 'when build is failed' do + let!(:stage_build) { create_job(:ci_build, status: :failed) } + + it 'returns detailed status for failed stage' do + expect(subject.text).to eq 'failed' + end + end + + context 'when build is canceled' do + let!(:stage_build) { create_job(:ci_build, status: :canceled) } + + it 'returns detailed status for canceled stage' do + expect(subject.text).to eq 'canceled' + end + end + + context 'when build is skipped' do + let!(:stage_build) { create_job(:ci_build, status: :skipped) } + + it 'returns detailed status for skipped stage' do + expect(subject.text).to eq 'skipped' + end + end + end + + def create_job(type, status: 'success', stage: stage_name) + create(type, pipeline: pipeline, stage: stage, status: status) + end +end diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 4e7833c3162..bee9f714849 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -5,6 +5,13 @@ describe Ci::Variable, models: true do let(:secret_value) { 'secret' } + it { is_expected.to validate_presence_of(:key) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to(:gl_project_id) } + it { is_expected.to validate_length_of(:key).is_at_most(255) } + it { is_expected.to allow_value('foo').for(:key) } + it { is_expected.not_to allow_value('foo bar').for(:key) } + it { is_expected.not_to allow_value('foo/bar').for(:key) } + before :each do subject.value = secret_value end diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index c41359b55a3..30782ca75a0 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -45,7 +45,7 @@ describe CommitRange, models: true do end describe '#to_reference' do - let(:cross) { create(:project) } + let(:cross) { create(:empty_project, namespace: project.namespace) } it 'returns a String reference to the object' do expect(range.to_reference).to eq "#{full_sha_from}...#{full_sha_to}" @@ -56,12 +56,12 @@ describe CommitRange, models: true do end it 'supports a cross-project reference' do - expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{full_sha_from}...#{full_sha_to}" + expect(range.to_reference(cross)).to eq "#{project.path}@#{full_sha_from}...#{full_sha_to}" end end describe '#reference_link_text' do - let(:cross) { create(:project) } + let(:cross) { create(:empty_project, namespace: project.namespace) } it 'returns a String reference to the object' do expect(range.reference_link_text).to eq "#{sha_from}...#{sha_to}" @@ -72,7 +72,7 @@ describe CommitRange, models: true do end it 'supports a cross-project reference' do - expect(range.reference_link_text(cross)).to eq "#{project.to_reference}@#{sha_from}...#{sha_to}" + expect(range.reference_link_text(cross)).to eq "#{project.path}@#{sha_from}...#{sha_to}" end end @@ -137,26 +137,25 @@ describe CommitRange, models: true do end describe '#has_been_reverted?' do - it 'returns true if the commit has been reverted' do - issue = create(:issue) + let(:issue) { create(:issue) } + let(:user) { issue.author } + it 'returns true if the commit has been reverted' do create(:note_on_issue, noteable: issue, system: true, - note: commit1.revert_description, + note: commit1.revert_description(user), project: issue.project) expect_any_instance_of(Commit).to receive(:reverts_commit?). - with(commit1). + with(commit1, user). and_return(true) - expect(commit1.has_been_reverted?(nil, issue)).to eq(true) + expect(commit1.has_been_reverted?(user, issue)).to eq(true) end it 'returns false a commit has not been reverted' do - issue = create(:issue) - - expect(commit1.has_been_reverted?(nil, issue)).to eq(false) + expect(commit1.has_been_reverted?(user, issue)).to eq(false) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e3bb3482d67..74b50d2908d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -34,24 +34,30 @@ describe Commit, models: true do end describe '#to_reference' do + let(:project) { create(:project, path: 'sample-project') } + let(:commit) { project.commit } + it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id end it 'supports a cross-project reference' do - cross = double('project') - expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.id}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(commit.to_reference(another_project)).to eq "sample-project@#{commit.id}" end end describe '#reference_link_text' do + let(:project) { create(:project, path: 'sample-project') } + let(:commit) { project.commit } + it 'returns a String reference to the object' do expect(commit.reference_link_text).to eq commit.short_id end it 'supports a cross-project reference' do - cross = double('project') - expect(commit.reference_link_text(cross)).to eq "#{project.to_reference}@#{commit.short_id}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(commit.reference_link_text(another_project)).to eq "sample-project@#{commit.short_id}" end end @@ -173,25 +179,26 @@ eos describe '#reverts_commit?' do let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") } + let(:user) { commit.author } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } context 'commit has no description' do before { allow(commit).to receive(:description?).and_return(false) } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description does not revert commit" do before { allow(commit).to receive(:description).and_return("Foo Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_falsy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy } end context "another_commit's description reverts commit" do before { allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") } - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end context "another_commit's description reverts merged merge request" do @@ -201,28 +208,24 @@ eos allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") end - it { expect(commit.reverts_commit?(another_commit)).to be_truthy } + it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy } end end describe '#status' do - context 'without arguments for compound status' do - shared_examples 'giving the status from pipeline' do - it do - expect(commit.status).to eq(Ci::Pipeline.status) - end - end - - context 'with pipelines' do - let!(:pipeline) do - create(:ci_empty_pipeline, project: project, sha: commit.sha) + context 'without ref argument' do + before do + %w[success failed created pending].each do |status| + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + status: status) end - - it_behaves_like 'giving the status from pipeline' end - context 'without pipelines' do - it_behaves_like 'giving the status from pipeline' + it 'gives compound status from latest pipelines' do + expect(commit.status).to eq(Ci::Pipeline.latest_status) + expect(commit.status).to eq('pending') end end @@ -248,8 +251,9 @@ eos expect(commit.status('fix')).to eq(pipeline_from_fix.status) end - it 'gives compound status if ref is nil' do - expect(commit.status(nil)).to eq(commit.status) + it 'gives compound status from latest pipelines if ref is nil' do + expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) + expect(commit.status(nil)).to eq('failed') end end end @@ -302,4 +306,21 @@ eos expect(commit.uri_type('this/path/doesnt/exist')).to be_nil end end + + describe '.from_hash' do + let(:new_commit) { described_class.from_hash(commit.to_hash, project) } + + it 'returns a Commit' do + expect(new_commit).to be_an_instance_of(described_class) + end + + it 'wraps a Gitlab::Git::Commit' do + expect(new_commit.raw).to be_an_instance_of(Gitlab::Git::Commit) + end + + it 'stores the correct commit fields' do + expect(new_commit.id).to eq(commit.id) + expect(new_commit.message).to eq(commit.message) + end + end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 80c2a1bc7a9..701f3323c0f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -175,7 +175,7 @@ describe CommitStatus, models: true do end it 'returns statuses without what we want to ignore' do - is_expected.to eq(statuses.values_at(1, 2, 4, 5, 6, 8, 9)) + is_expected.to eq(statuses.values_at(0, 1, 2, 3, 4, 5, 6, 8, 9)) end end @@ -200,49 +200,6 @@ describe CommitStatus, models: true do end end - describe '#stages' do - before do - create :commit_status, pipeline: pipeline, stage: 'build', name: 'linux', stage_idx: 0, status: 'success' - create :commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'failed' - create :commit_status, pipeline: pipeline, stage: 'deploy', name: 'staging', stage_idx: 2, status: 'running' - create :commit_status, pipeline: pipeline, stage: 'test', name: 'rspec', stage_idx: 1, status: 'success' - end - - context 'stages list' do - subject { CommitStatus.where(pipeline: pipeline).stages } - - it 'returns ordered list of stages' do - is_expected.to eq(%w[build test deploy]) - end - end - - context 'stages with statuses' do - subject { CommitStatus.where(pipeline: pipeline).latest.stages_status } - - it 'returns list of stages with statuses' do - is_expected.to eq({ - 'build' => 'failed', - 'test' => 'success', - 'deploy' => 'running' - }) - end - - context 'when build is retried' do - before do - create :commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'success' - end - - it 'ignores a previous state' do - is_expected.to eq({ - 'build' => 'success', - 'test' => 'success', - 'deploy' => 'running' - }) - end - end - end - end - describe '#commit' do it 'returns commit pipeline has been created for' do expect(commit_status.commit).to eq project.commit @@ -277,4 +234,13 @@ describe CommitStatus, models: true do end end end + + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns a detailed status' do + expect(commit_status.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Success + end + end end diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb index 96eee0e8bdd..4829ef17a20 100644 --- a/spec/models/concerns/access_requestable_spec.rb +++ b/spec/models/concerns/access_requestable_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe AccessRequestable do describe 'Group' do describe '#request_access' do - let(:group) { create(:group, :public) } + let(:group) { create(:group, :public, :access_requestable) } let(:user) { create(:user) } it { expect(group.request_access(user)).to be_a(GroupMember) } @@ -11,7 +11,7 @@ describe AccessRequestable do end describe '#access_requested?' do - let(:group) { create(:group, :public) } + let(:group) { create(:group, :public, :access_requestable) } let(:user) { create(:user) } before { group.request_access(user) } @@ -22,14 +22,14 @@ describe AccessRequestable do describe 'Project' do describe '#request_access' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:empty_project, :public, :access_requestable) } let(:user) { create(:user) } it { expect(project.request_access(user)).to be_a(ProjectMember) } end describe '#access_requested?' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:empty_project, :public, :access_requestable) } let(:user) { create(:user) } before { project.request_access(user) } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 87bffbdc54e..4d0f51fe82a 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -48,7 +48,7 @@ describe HasStatus do [create(type, status: :failed, allow_failure: true)] end - it { is_expected.to eq 'success' } + it { is_expected.to eq 'skipped' } end context 'success and canceled' do @@ -123,4 +123,100 @@ describe HasStatus do it_behaves_like 'build status summary' end end + + context 'for scope with one status' do + shared_examples 'having a job' do |status| + %i[ci_build generic_commit_status].each do |type| + context "when it's #{status} #{type} job" do + let!(:job) { create(type, status) } + + describe ".#{status}" do + it 'contains the job' do + expect(CommitStatus.public_send(status).all). + to contain_exactly(job) + end + end + + describe '.relevant' do + if status == :created + it 'contains nothing' do + expect(CommitStatus.relevant.all).to be_empty + end + else + it 'contains the job' do + expect(CommitStatus.relevant.all).to contain_exactly(job) + end + end + end + end + end + end + + %i[created running pending success + failed canceled skipped].each do |status| + it_behaves_like 'having a job', status + end + end + + context 'for scope with more statuses' do + shared_examples 'containing the job' do |status| + %i[ci_build generic_commit_status].each do |type| + context "when it's #{status} #{type} job" do + let!(:job) { create(type, status) } + + it 'contains the job' do + is_expected.to contain_exactly(job) + end + end + end + end + + shared_examples 'not containing the job' do |status| + %i[ci_build generic_commit_status].each do |type| + context "when it's #{status} #{type} job" do + let!(:job) { create(type, status) } + + it 'contains nothing' do + is_expected.to be_empty + end + end + end + end + + describe '.running_or_pending' do + subject { CommitStatus.running_or_pending } + + %i[running pending].each do |status| + it_behaves_like 'containing the job', status + end + + %i[created failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + + describe '.finished' do + subject { CommitStatus.finished } + + %i[success failed canceled].each do |status| + it_behaves_like 'containing the job', status + end + + %i[created running pending].each do |status| + it_behaves_like 'not containing the job', status + end + end + + describe '.cancelable' do + subject { CommitStatus.cancelable } + + %i[running pending created].each do |status| + it_behaves_like 'containing the job', status + end + + %i[failed success skipped canceled].each do |status| + it_behaves_like 'not containing the job', status + end + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index a9603074c32..4fa06a8c60a 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -35,7 +35,7 @@ describe Issue, "Issuable" do it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_at_least(0).is_at_most(255) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } end describe "Scope" do @@ -97,6 +97,11 @@ describe Issue, "Issuable" do end end + describe '.to_ability_name' do + it { expect(Issue.to_ability_name).to eq("issue") } + it { expect(MergeRequest.to_ability_name).to eq("merge_request") } + end + describe "#today?" do it "returns true when created today" do # Avoid timezone differences and just return exactly what we want @@ -171,23 +176,25 @@ describe Issue, "Issuable" do end describe '#subscribed?' do + let(:project) { issue.project } + context 'user is not a participant in the issue' do before { allow(issue).to receive(:participants).with(user).and_return([]) } it 'returns false when no subcription exists' do - expect(issue.subscribed?(user)).to be_falsey + expect(issue.subscribed?(user, project)).to be_falsey end it 'returns true when a subcription exists and subscribed is true' do - issue.subscriptions.create(user: user, subscribed: true) + issue.subscriptions.create(user: user, project: project, subscribed: true) - expect(issue.subscribed?(user)).to be_truthy + expect(issue.subscribed?(user, project)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - issue.subscriptions.create(user: user, subscribed: false) + issue.subscriptions.create(user: user, project: project, subscribed: false) - expect(issue.subscribed?(user)).to be_falsey + expect(issue.subscribed?(user, project)).to be_falsey end end @@ -195,19 +202,19 @@ describe Issue, "Issuable" do before { allow(issue).to receive(:participants).with(user).and_return([user]) } it 'returns false when no subcription exists' do - expect(issue.subscribed?(user)).to be_truthy + expect(issue.subscribed?(user, project)).to be_truthy end it 'returns true when a subcription exists and subscribed is true' do - issue.subscriptions.create(user: user, subscribed: true) + issue.subscriptions.create(user: user, project: project, subscribed: true) - expect(issue.subscribed?(user)).to be_truthy + expect(issue.subscribed?(user, project)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - issue.subscriptions.create(user: user, subscribed: false) + issue.subscriptions.create(user: user, project: project, subscribed: false) - expect(issue.subscribed?(user)).to be_falsey + expect(issue.subscribed?(user, project)).to be_falsey end end end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index b7e973798a3..0e097559b59 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -115,4 +115,24 @@ describe Milestone, 'Milestoneish' do expect(milestone.percent_complete(admin)).to eq 60 end end + + describe '#elapsed_days' do + it 'shows 0 if no start_date set' do + milestone = build(:milestone) + + expect(milestone.elapsed_days).to eq(0) + end + + it 'shows 0 if start_date is a future' do + milestone = build(:milestone, start_date: Time.now + 2.days) + + expect(milestone.elapsed_days).to eq(0) + end + + it 'shows correct amount of days' do + milestone = build(:milestone, start_date: Time.now - 2.days) + + expect(milestone.elapsed_days).to eq(2) + end + end end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb new file mode 100644 index 00000000000..b556135532f --- /dev/null +++ b/spec/models/concerns/routable_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe Group, 'Routable' do + let!(:group) { create(:group) } + + describe 'Validations' do + it { is_expected.to validate_presence_of(:route) } + end + + describe 'Associations' do + it { is_expected.to have_one(:route).dependent(:destroy) } + end + + describe 'Callbacks' do + it 'creates route record on create' do + expect(group.route.path).to eq(group.path) + end + + it 'updates route record on path change' do + group.update_attributes(path: 'wow') + + expect(group.route.path).to eq('wow') + end + + it 'ensure route path uniqueness across different objects' do + create(:group, parent: group, path: 'xyz') + duplicate = build(:project, namespace: group, path: 'xyz') + + expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Route path has already been taken, Route is invalid') + end + end + + describe '.find_by_full_path' do + let!(:nested_group) { create(:group, parent: group) } + + it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) } + it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) } + it { expect(described_class.find_by_full_path('unknown')).to eq(nil) } + end + + describe '.where_full_path_in' do + context 'without any paths' do + it 'returns an empty relation' do + expect(described_class.where_full_path_in([])).to eq([]) + end + end + + context 'without any valid paths' do + it 'returns an empty relation' do + expect(described_class.where_full_path_in(%w[unknown])).to eq([]) + end + end + + context 'with valid paths' do + let!(:nested_group) { create(:group, parent: group) } + + it 'returns the projects matching the paths' do + result = described_class.where_full_path_in([group.to_param, nested_group.to_param]) + + expect(result).to contain_exactly(group, nested_group) + end + + it 'returns projects regardless of the casing of paths' do + result = described_class.where_full_path_in([group.to_param.upcase, nested_group.to_param.upcase]) + + expect(result).to contain_exactly(group, nested_group) + end + end + end +end diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index b7fc5a92497..58f5c164116 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -1,67 +1,128 @@ require 'spec_helper' describe Subscribable, 'Subscribable' do - let(:resource) { create(:issue) } - let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:resource) { create(:issue, project: project) } + let(:user_1) { create(:user) } describe '#subscribed?' do - it 'returns false when no subcription exists' do - expect(resource.subscribed?(user)).to be_falsey - end + context 'without project' do + it 'returns false when no subscription exists' do + expect(resource.subscribed?(user_1)).to be_falsey + end + + it 'returns true when a subcription exists and subscribed is true' do + resource.subscriptions.create(user: user_1, subscribed: true) + + expect(resource.subscribed?(user_1)).to be_truthy + end - it 'returns true when a subcription exists and subscribed is true' do - resource.subscriptions.create(user: user, subscribed: true) + it 'returns false when a subcription exists and subscribed is false' do + resource.subscriptions.create(user: user_1, subscribed: false) - expect(resource.subscribed?(user)).to be_truthy + expect(resource.subscribed?(user_1)).to be_falsey + end end - it 'returns false when a subcription exists and subscribed is false' do - resource.subscriptions.create(user: user, subscribed: false) + context 'with project' do + it 'returns false when no subscription exists' do + expect(resource.subscribed?(user_1, project)).to be_falsey + end + + it 'returns true when a subcription exists and subscribed is true' do + resource.subscriptions.create(user: user_1, project: project, subscribed: true) + + expect(resource.subscribed?(user_1, project)).to be_truthy + end - expect(resource.subscribed?(user)).to be_falsey + it 'returns false when a subcription exists and subscribed is false' do + resource.subscriptions.create(user: user_1, project: project, subscribed: false) + + expect(resource.subscribed?(user_1, project)).to be_falsey + end end end + describe '#subscribers' do it 'returns [] when no subcribers exists' do - expect(resource.subscribers).to be_empty + expect(resource.subscribers(project)).to be_empty end it 'returns the subscribed users' do - resource.subscriptions.create(user: user, subscribed: true) - resource.subscriptions.create(user: create(:user), subscribed: false) + user_2 = create(:user) + resource.subscriptions.create(user: user_1, subscribed: true) + resource.subscriptions.create(user: user_2, project: project, subscribed: true) + resource.subscriptions.create(user: create(:user), project: project, subscribed: false) - expect(resource.subscribers).to eq [user] + expect(resource.subscribers(project)).to contain_exactly(user_1, user_2) end end describe '#toggle_subscription' do - it 'toggles the current subscription state for the given user' do - expect(resource.subscribed?(user)).to be_falsey + context 'without project' do + it 'toggles the current subscription state for the given user' do + expect(resource.subscribed?(user_1)).to be_falsey - resource.toggle_subscription(user) + resource.toggle_subscription(user_1) - expect(resource.subscribed?(user)).to be_truthy + expect(resource.subscribed?(user_1)).to be_truthy + end + end + + context 'with project' do + it 'toggles the current subscription state for the given user' do + expect(resource.subscribed?(user_1, project)).to be_falsey + + resource.toggle_subscription(user_1, project) + + expect(resource.subscribed?(user_1, project)).to be_truthy + end end end describe '#subscribe' do - it 'subscribes the given user' do - expect(resource.subscribed?(user)).to be_falsey + context 'without project' do + it 'subscribes the given user' do + expect(resource.subscribed?(user_1)).to be_falsey + + resource.subscribe(user_1) + + expect(resource.subscribed?(user_1)).to be_truthy + end + end + + context 'with project' do + it 'subscribes the given user' do + expect(resource.subscribed?(user_1, project)).to be_falsey - resource.subscribe(user) + resource.subscribe(user_1, project) - expect(resource.subscribed?(user)).to be_truthy + expect(resource.subscribed?(user_1, project)).to be_truthy + end end end describe '#unsubscribe' do - it 'unsubscribes the given current user' do - resource.subscriptions.create(user: user, subscribed: true) - expect(resource.subscribed?(user)).to be_truthy + context 'without project' do + it 'unsubscribes the given current user' do + resource.subscriptions.create(user: user_1, subscribed: true) + expect(resource.subscribed?(user_1)).to be_truthy + + resource.unsubscribe(user_1) + + expect(resource.subscribed?(user_1)).to be_falsey + end + end + + context 'with project' do + it 'unsubscribes the given current user' do + resource.subscriptions.create(user: user_1, project: project, subscribed: true) + expect(resource.subscribed?(user_1, project)).to be_truthy - resource.unsubscribe(user) + resource.unsubscribe(user_1, project) - expect(resource.subscribed?(user)).to be_falsey + expect(resource.subscribed?(user_1, project)).to be_falsey + end end end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index eb64f3d0c83..4b0bfa43abf 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -6,6 +6,7 @@ shared_examples 'TokenAuthenticatable' do it { expect(described_class).to be_private_method_defined(:write_new_token) } it { expect(described_class).to respond_to("find_by_#{token_field}") } it { is_expected.to respond_to("ensure_#{token_field}") } + it { is_expected.to respond_to("set_#{token_field}") } it { is_expected.to respond_to("reset_#{token_field}!") } end end @@ -55,6 +56,12 @@ describe ApplicationSetting, 'TokenAuthenticatable' do end end + describe 'setting new token' do + subject { described_class.new.send("set_#{token_field}", '0123456789') } + + it { is_expected.to eq '0123456789' } + end + describe 'multiple token fields' do before do described_class.send(:add_authentication_token_field, :yet_another_token) diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index 7691d690db0..7771785ead3 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#code', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } context 'with deployment' do generate_cycle_analytics_spec( diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index f649b44d367..5ed3d37f2fb 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#issue', models: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :issue, diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 2cdefbeef21..baf3e3241a1 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#plan', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :plan, diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 1f5e5cab92d..21b9c6e7150 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#production', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :production, diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 0ed080a42b1..158621d59a4 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#review', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :review, diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index af1c4477ddb..dad653964b7 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#staging', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :staging, diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb index 9d67bc82cba..725bc68b25f 100644 --- a/spec/models/cycle_analytics/summary_spec.rb +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -4,7 +4,7 @@ describe CycleAnalytics::Summary, models: true do let(:project) { create(:project) } let(:from) { Time.now } let(:user) { create(:user, :admin) } - subject { described_class.new(project, from: from) } + subject { described_class.new(project, user, from: from) } describe "#new_issues" do it "finds the number of issues created after the 'from date'" do diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index 02ddfeed9c1..2313724e8f3 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#test', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalytics.new(project, from: from_date) } + subject { CycleAnalytics.new(project, user, from: from_date) } generate_cycle_analytics_spec( phase: :test, diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 0142706d140..bc32fadd391 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -521,6 +521,15 @@ describe Discussion, model: true do end end + describe "#first_note_to_resolve" do + it "returns the first not that still needs to be resolved" do + allow(first_note).to receive(:to_be_resolved?).and_return(false) + allow(second_note).to receive(:to_be_resolved?).and_return(true) + + expect(subject.first_note_to_resolve).to eq(second_note) + end + end + describe "#collapsed?" do context "when a diff discussion" do before do @@ -590,4 +599,23 @@ describe Discussion, model: true do end end end + + describe "#truncated_diff_lines" do + let(:truncated_lines) { subject.truncated_diff_lines } + + context "when diff is greater than allowed number of truncated diff lines " do + it "returns fewer lines" do + expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + + expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + context "when some diff lines are meta" do + it "returns no meta lines" do + expect(subject.diff_lines).to include(be_meta) + expect(truncated_lines).not_to include(be_meta) + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index a94e6d0165f..97cbb093ed2 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment, models: true do - let(:environment) { create(:environment) } + subject(:environment) { create(:environment) } it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:deployments) } @@ -9,20 +9,17 @@ describe Environment, models: true do it { is_expected.to delegate_method(:last_deployment).to(:deployments).as(:last) } it { is_expected.to delegate_method(:stop_action).to(:last_deployment) } + it { is_expected.to delegate_method(:manual_actions).to(:last_deployment) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } - it { is_expected.to validate_length_of(:name).is_within(0..255) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } - it { is_expected.to validate_length_of(:external_url).is_within(0..255) } + it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:project_id) } + it { is_expected.to validate_length_of(:slug).is_at_most(24) } - # To circumvent a not null violation of the name column: - # https://github.com/thoughtbot/shoulda-matchers/issues/336 - it 'validates uniqueness of :external_url' do - create(:environment) - - is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) - end + it { is_expected.to validate_length_of(:external_url).is_at_most(255) } + it { is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) } describe '#nullify_external_url' do it 'replaces a blank url with nil' do @@ -166,4 +163,70 @@ describe Environment, models: true do end end end + + describe 'recently_updated_on_branch?' do + subject { environment.recently_updated_on_branch?('feature') } + + context 'when last deployment to environment is the most recent one' do + before do + create(:deployment, environment: environment, ref: 'feature') + end + + it { is_expected.to be true } + end + + context 'when last deployment to environment is not the most recent' do + before do + create(:deployment, environment: environment, ref: 'feature') + create(:deployment, environment: environment, ref: 'master') + end + + it { is_expected.to be false } + end + end + + describe '#actions_for' do + let(:deployment) { create(:deployment, environment: environment) } + let(:pipeline) { deployment.deployable.pipeline } + let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_BUILD_REF_NAME' )} + let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production' )} + + it 'returns a list of actions with matching environment' do + expect(environment.actions_for('review/master')).to contain_exactly(review_action) + end + end + + describe '#slug' do + it "is automatically generated" do + expect(environment.slug).not_to be_nil + end + + it "is not regenerated if name changes" do + original_slug = environment.slug + environment.update_attributes!(name: environment.name.reverse) + + expect(environment.slug).to eq(original_slug) + end + end + + describe '#generate_slug' do + SUFFIX = "-[a-z0-9]{6}" + { + "staging-12345678901234567" => "staging-123456789" + SUFFIX, + "9-staging-123456789012345" => "env-9-staging-123" + SUFFIX, + "staging-1234567890123456" => "staging-1234567890123456", + "production" => "production", + "PRODUCTION" => "production" + SUFFIX, + "review/1-foo" => "review-1-foo" + SUFFIX, + "1-foo" => "env-1-foo" + SUFFIX, + "1/foo" => "env-1-foo" + SUFFIX, + "foo-" => "foo" + SUFFIX, + }.each do |name, matcher| + it "returns a slug matching #{matcher}, given #{name}" do + slug = described_class.new(name: name).generate_slug + + expect(slug).to match(/\A#{matcher}\z/) + end + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index aca49be2942..f8660da031d 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -27,13 +27,14 @@ describe Event, models: true do end describe "Push event" do - let(:project) { create(:project) } + let(:project) { create(:project, :private) } let(:user) { project.owner } let(:event) { create_event(project, user) } it do expect(event.push?).to be_truthy - expect(event.visible_to_user?).to be_truthy + expect(event.visible_to_user?(user)).to be_truthy + expect(event.visible_to_user?(nil)).to be_falsey expect(event.tag?).to be_falsey expect(event.branch_name).to eq("master") expect(event.author).to eq(user) @@ -93,6 +94,7 @@ describe Event, models: true do let(:admin) { create(:admin) } let(:issue) { create(:issue, project: project, author: author, assignee: assignee) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:note_on_commit) { create(:note_on_commit, project: project) } let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) } let(:event) { Event.new(project: project, target: target, author_id: author.id) } @@ -102,6 +104,32 @@ describe Event, models: true do project.team << [guest, :guest] end + context 'commit note event' do + let(:target) { note_on_commit } + + it do + aggregate_failures do + expect(event.visible_to_user?(non_member)).to eq true + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq true + expect(event.visible_to_user?(admin)).to eq true + end + end + + context 'private project' do + let(:project) { create(:empty_project, :private) } + + it do + aggregate_failures do + expect(event.visible_to_user?(non_member)).to eq false + expect(event.visible_to_user?(member)).to eq true + expect(event.visible_to_user?(guest)).to eq false + expect(event.visible_to_user?(admin)).to eq true + end + end + end + end + context 'issue event' do context 'for non confidential issues' do let(:target) { issue } @@ -232,6 +260,24 @@ describe Event, models: true do end end + describe '#authored_by?' do + let(:event) { build(:event) } + + it 'returns true when the event author and user are the same' do + expect(event.authored_by?(event.author)).to eq(true) + end + + it 'returns false when passing nil as an argument' do + expect(event.authored_by?(nil)).to eq(false) + end + + it 'returns false when the given user is not the author of the event' do + user = double(:user, id: -1) + + expect(event.authored_by?(user)).to eq(false) + end + end + def create_event(project, user, attrs = {}) data = { before: Gitlab::Git::BLANK_SHA, diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 615cfe3142b..6004bfdb7b7 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -1,8 +1,11 @@ require 'spec_helper' describe GenericCommitStatus, models: true do - let(:pipeline) { FactoryGirl.create :ci_pipeline } - let(:generic_commit_status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline } + let(:pipeline) { create(:ci_pipeline) } + + let(:generic_commit_status) do + create(:generic_commit_status, pipeline: pipeline) + end describe '#context' do subject { generic_commit_status.context } @@ -17,6 +20,15 @@ describe GenericCommitStatus, models: true do it { is_expected.to eq([:external]) } end + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns detailed status object' do + expect(generic_commit_status.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Success + end + end + describe 'set_default_values' do before do generic_commit_status.context = nil diff --git a/spec/models/group_label_spec.rb b/spec/models/group_label_spec.rb index 85eb889225b..668aa6fb357 100644 --- a/spec/models/group_label_spec.rb +++ b/spec/models/group_label_spec.rb @@ -18,7 +18,7 @@ describe GroupLabel, models: true do end describe '#to_reference' do - let(:label) { create(:group_label) } + let(:label) { create(:group_label, title: 'feature') } context 'using id' do it 'returns a String reference to the object' do @@ -37,6 +37,16 @@ describe GroupLabel, models: true do end end + context 'cross-project' do + let(:namespace) { build_stubbed(:namespace) } + let(:source_project) { build_stubbed(:empty_project, name: 'project-1', namespace: namespace) } + let(:target_project) { build_stubbed(:empty_project, name: 'project-2', namespace: namespace) } + + it 'returns a String reference to the object' do + expect(label.to_reference(source_project, target_project)).to eq %(project-1~#{label.id}) + end + end + context 'using invalid format' do it 'raises error' do expect { label.to_reference(format: :invalid) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 47f89f744cb..7d5ecfbaa64 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Group, models: true do - let!(:group) { create(:group) } + let!(:group) { create(:group, :access_requestable) } describe 'associations' do it { is_expected.to have_many :projects } @@ -50,9 +50,8 @@ describe Group, models: true do describe 'validations' do it { is_expected.to validate_presence_of :name } - it { is_expected.to validate_uniqueness_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } it { is_expected.to validate_presence_of :path } - it { is_expected.to validate_uniqueness_of(:path) } it { is_expected.not_to validate_presence_of :owner } end @@ -271,4 +270,11 @@ describe Group, models: true do expect(group.web_url).to include("groups/#{group.name}") end end + + describe 'nested group' do + subject { build(:group, :nested) } + + it { is_expected.to be_valid } + it { expect(subject.parent).to be_kind_of(Group) } + end end diff --git a/spec/models/guest_spec.rb b/spec/models/guest_spec.rb new file mode 100644 index 00000000000..d79f929f7a1 --- /dev/null +++ b/spec/models/guest_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Guest, lib: true do + let(:public_project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:internal_project) { create(:project, :internal) } + + describe '.can_pull?' do + context 'when project is private' do + it 'does not allow to pull the repo' do + expect(Guest.can?(:download_code, private_project)).to eq(false) + end + end + + context 'when project is internal' do + it 'does not allow to pull the repo' do + expect(Guest.can?(:download_code, internal_project)).to eq(false) + end + end + + context 'when project is public' do + context 'when repository is disabled' do + it 'does not allow to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) + + expect(Guest.can?(:download_code, public_project)).to eq(false) + end + end + + context 'when repository is accessible only by team members' do + it 'does not allow to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::PRIVATE) + + expect(Guest.can?(:download_code, public_project)).to eq(false) + end + end + + context 'when repository is enabled' do + it 'allows to pull the repo' do + public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::ENABLED) + + expect(Guest.can?(:download_code, public_project)).to eq(true) + end + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 60d30eb7418..bb56e44db29 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -22,35 +22,17 @@ describe Issue, models: true do it { is_expected.to have_db_index(:deleted_at) } end - describe 'visible_to_user' do - let(:user) { create(:user) } - let(:authorized_user) { create(:user) } - let(:project) { create(:project, namespace: authorized_user.namespace) } - let!(:public_issue) { create(:issue, project: project) } - let!(:confidential_issue) { create(:issue, project: project, confidential: true) } - - it 'returns non confidential issues for nil user' do - expect(Issue.visible_to_user(nil).count).to be(1) - end - - it 'returns non confidential issues for user not authorized for the issues projects' do - expect(Issue.visible_to_user(user).count).to be(1) - end - - it 'returns all issues for user authorized for the issues projects' do - expect(Issue.visible_to_user(authorized_user).count).to be(2) - end - end - describe '#to_reference' do + let(:project) { build(:empty_project, name: 'sample-project') } + let(:issue) { build(:issue, iid: 1, project: project) } + it 'returns a String reference to the object' do - expect(subject.to_reference).to eq "##{subject.iid}" + expect(issue.to_reference).to eq "#1" end it 'supports a cross-project reference' do - cross = double('project') - expect(subject.to_reference(cross)). - to eq "#{subject.project.to_reference}##{subject.iid}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(issue.to_reference(another_project)).to eq "sample-project#1" end end @@ -102,17 +84,17 @@ describe Issue, models: true do it 'returns the merge request to close this issue' do mr - expect(issue.closed_by_merge_requests).to eq([mr]) + expect(issue.closed_by_merge_requests(mr.author)).to eq([mr]) end it "returns an empty array when the merge request is closed already" do closed_mr - expect(issue.closed_by_merge_requests).to eq([]) + expect(issue.closed_by_merge_requests(closed_mr.author)).to eq([]) end it "returns an empty array when the current issue is closed already" do - expect(closed_issue.closed_by_merge_requests).to eq([]) + expect(closed_issue.closed_by_merge_requests(closed_issue.author)).to eq([]) end end @@ -218,7 +200,7 @@ describe Issue, models: true do source_project: subject.project, source_branch: "#{subject.iid}-branch" }) merge_request.create_cross_references!(user) - expect(subject.referenced_merge_requests).not_to be_empty + expect(subject.referenced_merge_requests(user)).not_to be_empty expect(subject.related_branches(user)).to eq([subject.to_branch_name]) end @@ -314,8 +296,24 @@ describe Issue, models: true do end describe '#visible_to_user?' do + context 'without a user' do + let(:issue) { build(:issue) } + + it 'returns true when the issue is publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(true) + + expect(issue.visible_to_user?).to eq(true) + end + + it 'returns false when the issue is not publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(false) + + expect(issue.visible_to_user?).to eq(false) + end + end + context 'with a user' do - let(:user) { build(:user) } + let(:user) { create(:user) } let(:issue) { build(:issue) } it 'returns true when the issue is readable' do @@ -329,26 +327,24 @@ describe Issue, models: true do expect(issue.visible_to_user?(user)).to eq(false) end - end - context 'without a user' do - let(:issue) { build(:issue) } + it 'returns false when feature is disabled' do + expect(issue).not_to receive(:readable_by?) - it 'returns true when the issue is publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(true) + issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) - expect(issue.visible_to_user?).to eq(true) + expect(issue.visible_to_user?(user)).to eq(false) end - it 'returns false when the issue is not publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(false) + it 'returns false when restricted for members' do + expect(issue).not_to receive(:readable_by?) - expect(issue.visible_to_user?).to eq(false) + issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + + expect(issue.visible_to_user?(user)).to eq(false) end end - end - describe '#readable_by?' do describe 'with a regular user that is not a team member' do let(:user) { create(:user) } @@ -358,13 +354,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns false for a confidential issue' do issue = build(:issue, project: project, confidential: true) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end @@ -375,13 +371,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end @@ -393,13 +389,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end end end @@ -410,26 +406,28 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(false) end context 'when the user is the project owner' do + before { project.team << [user, :master] } + it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -447,13 +445,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end @@ -467,13 +465,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end @@ -487,13 +485,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -505,13 +503,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end it 'returns true for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).to be_readable_by(user) + expect(issue.visible_to_user?(user)).to eq(true) end end end @@ -523,13 +521,13 @@ describe Issue, models: true do it 'returns true for a regular issue' do issue = build(:issue, project: project) - expect(issue).to be_publicly_visible + expect(issue).to be_truthy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end @@ -539,13 +537,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end @@ -555,13 +553,13 @@ describe Issue, models: true do it 'returns false for a regular issue' do issue = build(:issue, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end it 'returns false for a confidential issue' do issue = build(:issue, :confidential, project: project) - expect(issue).not_to be_publicly_visible + expect(issue).not_to be_falsy end end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 7fc6ed1dd54..2a33d819138 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -7,9 +7,13 @@ describe Key, models: true do describe "Validation" do it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } + it { is_expected.to validate_presence_of(:key) } - it { is_expected.to validate_length_of(:title).is_within(0..255) } - it { is_expected.to validate_length_of(:key).is_within(0..5000) } + it { is_expected.to validate_length_of(:key).is_at_most(5000) } + it { is_expected.to allow_value('ssh-foo').for(:key) } + it { is_expected.to allow_value('ecdsa-foo').for(:key) } + it { is_expected.not_to allow_value('foo-bar').for(:key) } end describe "Methods" do @@ -19,7 +23,7 @@ describe Key, models: true do describe "#publishable_keys" do it 'replaces SSH key comment with simple identifier of username + hostname' do - expect(build(:key, user: user).publishable_key).to include("#{user.name} (localhost)") + expect(build(:key, user: user).publishable_key).to include("#{user.name} (#{Gitlab.config.gitlab.host})") end end end @@ -71,15 +75,25 @@ describe Key, models: true do context 'callbacks' do it 'adds new key to authorized_file' do - @key = build(:personal_key, id: 7) - expect(GitlabShellWorker).to receive(:perform_async).with(:add_key, @key.shell_id, @key.key) - @key.save + key = build(:personal_key, id: 7) + expect(GitlabShellWorker).to receive(:perform_async).with(:add_key, key.shell_id, key.key) + key.save! end it 'removes key from authorized_file' do - @key = create(:personal_key) - expect(GitlabShellWorker).to receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) - @key.destroy + key = create(:personal_key) + expect(GitlabShellWorker).to receive(:perform_async).with(:remove_key, key.shell_id, key.key) + key.destroy + end + end + + describe '#key=' do + let(:valid_key) do + "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com" + end + + it 'strips white spaces' do + expect(described_class.new(key: " #{valid_key} ").key).to eq(valid_key) end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 485121701af..4f7c8a36cb5 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -57,7 +57,7 @@ describe Member, models: true do describe 'Scopes & finders' do before do - project = create(:empty_project, :public) + project = create(:empty_project, :public, :access_requestable) group = create(:group) @owner_user = create(:user).tap { |u| group.add_owner(u) } @owner = group.members.find_by(user_id: @owner_user.id) @@ -174,7 +174,7 @@ describe Member, models: true do describe '.add_user' do %w[project group].each do |source_type| context "when source is a #{source_type}" do - let!(:source) { create(source_type, :public) } + let!(:source) { create(source_type, :public, :access_requestable) } let!(:user) { create(:user) } let!(:admin) { create(:admin) } @@ -443,6 +443,16 @@ describe Member, models: true do member.accept_invite!(user) end + + it "refreshes user's authorized projects", truncate: true do + project = member.source + + expect(user.authorized_projects).not_to include(project) + + member.accept_invite!(user) + + expect(user.authorized_projects.reload).to include(project) + end end describe "#decline_invite!" do @@ -468,4 +478,16 @@ describe Member, models: true do expect { member.generate_invite_token }.to change { member.invite_token} end end + + describe "destroying a record", truncate: true do + it "refreshes user's authorized projects" do + project = create(:project, :private) + user = create(:user) + member = project.team << [user, :reporter] + + member.destroy + + expect(user.authorized_projects).not_to include(project) + end + end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e5007424041..eb876d105da 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -77,24 +77,13 @@ describe MergeRequestDiff, models: true do end describe '#commits_sha' do - shared_examples 'returning all commits SHA' do - it 'returns all commits SHA' do - commits_sha = subject.commits_sha + it 'returns all commits SHA using serialized commits' do + subject.st_commits = [ + { id: 'sha1' }, + { id: 'sha2' } + ] - expect(commits_sha).to eq(subject.commits.map(&:sha)) - end - end - - context 'when commits were loaded' do - before do - subject.commits - end - - it_behaves_like 'returning all commits SHA' - end - - context 'when commits were not loaded' do - it_behaves_like 'returning all commits SHA' + expect(subject.commits_sha).to eq(['sha1', 'sha2']) end end @@ -113,4 +102,15 @@ describe MergeRequestDiff, models: true do expect(diffs.size).to eq(3) end end + + describe '#commits_count' do + it 'returns number of commits using serialized commits' do + subject.st_commits = [ + { id: 'sha1' }, + { id: 'sha2' } + ] + + expect(subject.commits_count).to eq 2 + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fb032a89d50..5da00a8636a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -31,7 +31,7 @@ describe MergeRequest, models: true do it { is_expected.to validate_presence_of(:target_branch) } it { is_expected.to validate_presence_of(:source_branch) } - context "Validation of merge user with Merge When Build succeeds" do + context "Validation of merge user with Merge When Pipeline Succeeds" do it "allows user to be nil when the feature is disabled" do expect(subject).to be_valid end @@ -142,13 +142,16 @@ describe MergeRequest, models: true do end describe '#to_reference' do + let(:project) { build(:empty_project, name: 'sample-project') } + let(:merge_request) { build(:merge_request, target_project: project, iid: 1) } + it 'returns a String reference to the object' do - expect(subject.to_reference).to eq "!#{subject.iid}" + expect(merge_request.to_reference).to eq "!1" end it 'supports a cross-project reference' do - cross = double('project') - expect(subject.to_reference(cross)).to eq "#{subject.source_project.to_reference}!#{subject.iid}" + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(merge_request.to_reference(another_project)).to eq "sample-project!1" end end @@ -202,7 +205,7 @@ describe MergeRequest, models: true do end end - describe "#mr_and_commit_notes" do + describe "#related_notes" do let!(:merge_request) { create(:merge_request) } before do @@ -214,7 +217,7 @@ describe MergeRequest, models: true do it "includes notes for commits" do expect(merge_request.commits).not_to be_empty - expect(merge_request.mr_and_commit_notes.count).to eq(2) + expect(merge_request.related_notes.count).to eq(2) end it "includes notes for commits from target project as well" do @@ -222,7 +225,7 @@ describe MergeRequest, models: true do project: merge_request.target_project) expect(merge_request.commits).not_to be_empty - expect(merge_request.mr_and_commit_notes.count).to eq(3) + expect(merge_request.related_notes.count).to eq(3) end end @@ -249,7 +252,7 @@ describe MergeRequest, models: true do end end - describe 'detection of issues to be closed' do + describe '#closes_issues' do let(:issue0) { create :issue, project: subject.project } let(:issue1) { create :issue, project: subject.project } @@ -277,14 +280,19 @@ describe MergeRequest, models: true do expect(subject.closes_issues).to be_empty end + end + + describe '#issues_mentioned_but_not_closing' do + it 'detects issues mentioned in description but not closed' do + mentioned_issue = create(:issue, project: subject.project) + + subject.project.team << [subject.author, :developer] + subject.description = "Is related to #{mentioned_issue.to_reference}" - it 'detects issues mentioned in the description' do - issue2 = create(:issue, project: subject.project) - subject.description = "Closes #{issue2.to_reference}" allow(subject.project).to receive(:default_branch). and_return(subject.target_branch) - expect(subject.closes_issues).to include(issue2) + expect(subject.issues_mentioned_but_not_closing).to match_array([mentioned_issue]) end end @@ -407,11 +415,17 @@ describe MergeRequest, models: true do .to match("Remove all technical debt\n\n") end - it 'includes its description in the body' do - request = build(:merge_request, description: 'By removing all code') + it 'includes its closed issues in the body' do + issue = create(:issue, project: subject.project) - expect(request.merge_commit_message) - .to match("By removing all code\n\n") + subject.project.team << [subject.author, :developer] + subject.description = "This issue Closes #{issue.to_reference}" + + allow(subject.project).to receive(:default_branch). + and_return(subject.target_branch) + + expect(subject.merge_commit_message) + .to match("Closes #{issue.to_reference}") end it 'includes its reference in the body' do @@ -426,6 +440,20 @@ describe MergeRequest, models: true do expect(request.merge_commit_message).not_to match("Title\n\n\n\n") end + + it 'includes its description in the body' do + request = build(:merge_request, description: 'By removing all code') + + expect(request.merge_commit_message(include_description: true)) + .to match("By removing all code\n\n") + end + + it 'does not includes its description in the body' do + request = build(:merge_request, description: 'By removing all code') + + expect(request.merge_commit_message) + .not_to match("By removing all code\n\n") + end end describe "#reset_merge_when_build_succeeds" do @@ -557,20 +585,17 @@ describe MergeRequest, models: true do end describe '#commits_sha' do - let(:commit0) { double('commit0', sha: 'sha1') } - let(:commit1) { double('commit1', sha: 'sha2') } - let(:commit2) { double('commit2', sha: 'sha3') } - before do - allow(subject.merge_request_diff).to receive(:commits).and_return([commit0, commit1, commit2]) + allow(subject.merge_request_diff).to receive(:commits_sha). + and_return(['sha1']) end - it 'returns sha of commits' do - expect(subject.commits_sha).to contain_exactly('sha1', 'sha2', 'sha3') + it 'delegates to merge request diff' do + expect(subject.commits_sha).to eq ['sha1'] end end - describe '#pipeline' do + describe '#head_pipeline' do describe 'when the source project exists' do it 'returns the latest pipeline' do pipeline = double(:ci_pipeline, ref: 'master') @@ -581,7 +606,7 @@ describe MergeRequest, models: true do with('master', '123abc'). and_return(pipeline) - expect(subject.pipeline).to eq(pipeline) + expect(subject.head_pipeline).to eq(pipeline) end end @@ -589,7 +614,7 @@ describe MergeRequest, models: true do it 'returns nil' do allow(subject).to receive(:source_project).and_return(nil) - expect(subject.pipeline).to be_nil + expect(subject.head_pipeline).to be_nil end end end @@ -856,16 +881,34 @@ describe MergeRequest, models: true do context 'when it is only allowed to merge when build is green' do context 'and a failed pipeline is associated' do before do - pipeline.statuses << create(:commit_status, status: 'failed', project: project) - allow(subject).to receive(:pipeline) { pipeline } + pipeline.update(status: 'failed') + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_falsey } end + context 'and a successful pipeline is associated' do + before do + pipeline.update(status: 'success') + allow(subject).to receive(:head_pipeline) { pipeline } + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'and a skipped pipeline is associated' do + before do + pipeline.update(status: 'skipped') + allow(subject).to receive(:head_pipeline) { pipeline } + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + context 'when no pipeline is associated' do before do - allow(subject).to receive(:pipeline) { nil } + allow(subject).to receive(:head_pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -878,7 +921,7 @@ describe MergeRequest, models: true do context 'and a failed pipeline is associated' do before do pipeline.statuses << create(:commit_status, status: 'failed', project: project) - allow(subject).to receive(:pipeline) { pipeline } + allow(subject).to receive(:head_pipeline) { pipeline } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -886,7 +929,7 @@ describe MergeRequest, models: true do context 'when no pipeline is associated' do before do - allow(subject).to receive(:pipeline) { nil } + allow(subject).to receive(:head_pipeline) { nil } end it { expect(subject.mergeable_ci_state?).to be_truthy } @@ -919,6 +962,16 @@ describe MergeRequest, models: true do expect(merge_request.mergeable_discussions_state?).to be_falsey end end + + context 'with no discussions' do + before do + merge_request.notes.destroy_all + end + + it 'returns true' do + expect(merge_request.mergeable_discussions_state?).to be_truthy + end + end end context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do @@ -1099,6 +1152,46 @@ describe MergeRequest, models: true do allow(subject).to receive(:diff_discussions).and_return([first_discussion, second_discussion, third_discussion]) end + describe '#resolvable_discussions' do + before do + allow(first_discussion).to receive(:to_be_resolved?).and_return(true) + allow(second_discussion).to receive(:to_be_resolved?).and_return(false) + allow(third_discussion).to receive(:to_be_resolved?).and_return(false) + end + + it 'includes only discussions that need to be resolved' do + expect(subject.resolvable_discussions).to eq([first_discussion]) + end + end + + describe '#discussions_can_be_resolved_by? user' do + let(:user) { build(:user) } + + context 'all discussions can be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(true) + end + end + + context 'one discussion cannot be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(false) + end + end + end + describe "#discussions_resolvable?" do context "when all discussions are unresolvable" do before do @@ -1180,6 +1273,50 @@ describe MergeRequest, models: true do end end end + + describe "#discussions_to_be_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.discussions_to_be_resolved?).to be true + end + end + end + end end describe '#conflicts_can_be_resolved_in_ui?' do @@ -1368,4 +1505,26 @@ describe MergeRequest, models: true do end end end + + describe '#has_commits?' do + before do + allow(subject.merge_request_diff).to receive(:commits_count). + and_return(2) + end + + it 'returns true when merge request diff has commits' do + expect(subject.has_commits?).to be_truthy + end + end + + describe '#has_no_commits?' do + before do + allow(subject.merge_request_diff).to receive(:commits_count). + and_return(0) + end + + it 'returns true when merge request diff has 0 commits' do + expect(subject.has_no_commits?).to be_truthy + end + end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 33fe22dd98c..064f29d2d66 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -1,11 +1,6 @@ require 'spec_helper' describe Milestone, models: true do - describe "Associations" do - it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:issues) } - end - describe "Validation" do before do allow(subject).to receive(:set_iid).and_return(false) @@ -13,10 +8,25 @@ describe Milestone, models: true do it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:project) } + + describe 'start_date' do + it 'adds an error when start_date is greated then due_date' do + milestone = build(:milestone, start_date: Date.tomorrow, due_date: Date.yesterday) + + expect(milestone).not_to be_valid + expect(milestone.errors[:start_date]).to include("Can't be greater than due date") + end + end + end + + describe "Associations" do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:issues) } end - let(:milestone) { create(:milestone) } - let(:issue) { create(:issue) } + let(:project) { create(:project, :public) } + let(:milestone) { create(:milestone, project: project) } + let(:issue) { create(:issue, project: project) } let(:user) { create(:user) } describe "#title" do @@ -58,18 +68,6 @@ describe Milestone, models: true do end end - describe "#expires_at" do - it "is nil when due_date is unset" do - milestone.update_attributes(due_date: nil) - expect(milestone.expires_at).to be_nil - end - - it "is not nil when due_date is set" do - milestone.update_attributes(due_date: Date.tomorrow) - expect(milestone.expires_at).to be_present - end - end - describe '#expired?' do context "expired" do before do @@ -88,6 +86,18 @@ describe Milestone, models: true do end end + describe '#upcoming?' do + it 'returns true' do + milestone = build(:milestone, start_date: Time.now + 1.month) + expect(milestone.upcoming?).to be_truthy + end + + it 'returns false' do + milestone = build(:milestone, start_date: Date.today.prev_year) + expect(milestone.upcoming?).to be_falsey + end + end + describe '#percent_complete' do before do allow(milestone).to receive_messages( @@ -101,8 +111,8 @@ describe Milestone, models: true do describe :items_count do before do - milestone.issues << create(:issue) - milestone.issues << create(:closed_issue) + milestone.issues << create(:issue, project: project) + milestone.issues << create(:closed_issue, project: project) milestone.merge_requests << create(:merge_request) end @@ -117,7 +127,7 @@ describe Milestone, models: true do describe '#total_items_count' do before do - create :closed_issue, milestone: milestone + create :closed_issue, milestone: milestone, project: project create :merge_request, milestone: milestone end @@ -237,4 +247,18 @@ describe Milestone, models: true do end end end + + describe '#to_reference' do + let(:project) { build(:empty_project, name: 'sample-project') } + let(:milestone) { build(:milestone, iid: 1, project: project) } + + it 'returns a String reference to the object' do + expect(milestone.to_reference).to eq "%1" + end + + it 'supports a cross-project reference' do + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(milestone.to_reference(another_project)).to eq "sample-project%1" + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 431b3e4435f..9fd06bb6b23 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -4,14 +4,17 @@ describe Namespace, models: true do let!(:namespace) { create(:namespace) } it { is_expected.to have_many :projects } - it { is_expected.to validate_presence_of :name } - it { is_expected.to validate_uniqueness_of(:name) } - it { is_expected.to validate_presence_of :path } - it { is_expected.to validate_uniqueness_of(:path) } - it { is_expected.to validate_presence_of :owner } - describe "Mass assignment" do - end + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + + it { is_expected.to validate_length_of(:description).is_at_most(255) } + + it { is_expected.to validate_presence_of(:path) } + it { is_expected.to validate_length_of(:path).is_at_most(255) } + + it { is_expected.to validate_presence_of(:owner) } describe "Respond to" do it { is_expected.to respond_to(:human_name) } @@ -117,4 +120,34 @@ describe Namespace, models: true do expect(Namespace.clean_path("--%+--valid_*&%name=.git.%.atom.atom.@email.com")).to eq("valid_name") end end + + describe '#full_path' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + + it { expect(group.full_path).to eq(group.path) } + it { expect(nested_group.full_path).to eq("#{group.path}/#{nested_group.path}") } + end + + describe '#full_name' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + + it { expect(group.full_name).to eq(group.name) } + it { expect(nested_group.full_name).to eq("#{group.name} / #{nested_group.name}") } + end + + describe '#parents' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } + + it 'returns the correct parents' do + expect(very_deep_nested_group.parents).to eq([group, nested_group, deep_nested_group]) + expect(deep_nested_group.parents).to eq([group, nested_group]) + expect(nested_group.parents).to eq([group]) + expect(group.parents).to eq([]) + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index e6b6e7c0634..310fecd8a5c 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -162,44 +162,6 @@ describe Note, models: true do end end - describe '.search' do - let(:note) { create(:note_on_issue, note: 'WoW') } - - it 'returns notes with matching content' do - expect(described_class.search(note.note)).to eq([note]) - end - - it 'returns notes with matching content regardless of the casing' do - expect(described_class.search('WOW')).to eq([note]) - end - - context "confidential issues" do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } - let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) } - - it "returns notes with matching content if user can see the issue" do - expect(described_class.search(confidential_note.note, as_user: user)).to eq([confidential_note]) - end - - it "does not return notes with matching content if user can not see the issue" do - user = create(:user) - expect(described_class.search(confidential_note.note, as_user: user)).to be_empty - end - - it "does not return notes with matching content for project members with guest role" do - user = create(:user) - project.team << [user, :guest] - expect(described_class.search(confidential_note.note, as_user: user)).to be_empty - end - - it "does not return notes with matching content for unauthenticated users" do - expect(described_class.search(confidential_note.note)).to be_empty - end - end - end - describe "editable?" do it "returns true" do note = build(:note) @@ -223,7 +185,7 @@ describe Note, models: true do let(:note) do create :note, noteable: ext_issue, project: ext_proj, - note: "Mentioned in issue #{private_issue.to_reference(ext_proj)}", + note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", system: true end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index c5ff1941378..47397a822c1 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -14,4 +14,20 @@ describe ProjectGroupLink do it { should validate_presence_of(:group) } it { should validate_presence_of(:group_access) } end + + describe "destroying a record", truncate: true do + it "refreshes group users' authorized projects" do + project = create(:project, :private) + group = create(:group) + reporter = create(:user) + group_users = group.users + + group.add_reporter(reporter) + project.project_group_links.create(group: group) + group_users.each { |user| expect(user.authorized_projects).to include(project) } + + project.project_group_links.destroy_all + group_users.each { |user| expect(user.authorized_projects).not_to include(project) } + end + end end diff --git a/spec/models/project_label_spec.rb b/spec/models/project_label_spec.rb index 18c9d449ee5..4d538cac007 100644 --- a/spec/models/project_label_spec.rb +++ b/spec/models/project_label_spec.rb @@ -105,14 +105,14 @@ describe ProjectLabel, models: true do context 'using name' do it 'returns cross reference with label name' do expect(label.to_reference(project, format: :name)) - .to eq %Q(#{label.project.to_reference}~"#{label.name}") + .to eq %Q(#{label.project.path_with_namespace}~"#{label.name}") end end context 'using id' do it 'returns cross reference with label id' do expect(label.to_reference(project, format: :id)) - .to eq %Q(#{label.project.to_reference}~#{label.id}) + .to eq %Q(#{label.project.path_with_namespace}~#{label.id}) end end end diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/chat_message/build_message_spec.rb index 452f4e2782c..b71d153f814 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/chat_message/build_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackService::BuildMessage do - subject { SlackService::BuildMessage.new(args) } +describe ChatMessage::BuildMessage do + subject { described_class.new(args) } let(:args) do { diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index 98c36ec088d..ebe0ead4408 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackService::IssueMessage, models: true do - subject { SlackService::IssueMessage.new(args) } +describe ChatMessage::IssueMessage, models: true do + subject { described_class.new(args) } let(:args) do { diff --git a/spec/models/project_services/slack_service/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index c5c052d9af1..07c414c6ca4 100644 --- a/spec/models/project_services/slack_service/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackService::MergeMessage, models: true do - subject { SlackService::MergeMessage.new(args) } +describe ChatMessage::MergeMessage, models: true do + subject { described_class.new(args) } let(:args) do { diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index 38cfe4ad3e3..31936da40a2 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe SlackService::NoteMessage, models: true do +describe ChatMessage::NoteMessage, models: true do let(:color) { '#345' } before do @@ -36,9 +36,9 @@ describe SlackService::NoteMessage, models: true do end it 'returns a message regarding notes on commits' do - message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("test.user commented on " \ - "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \ + message = described_class.new(@args) + expect(message.pretext).to eq("test.user <url|commented on " \ + "commit 5f163b2b> in <somewhere.com|project_name>: " \ "*Added a commit message*") expected_attachments = [ { @@ -62,9 +62,9 @@ describe SlackService::NoteMessage, models: true do end it 'returns a message regarding notes on a merge request' do - message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("test.user commented on " \ - "<url|merge request !30> in <somewhere.com|project_name>: " \ + message = described_class.new(@args) + expect(message.pretext).to eq("test.user <url|commented on " \ + "merge request !30> in <somewhere.com|project_name>: " \ "*merge request title*") expected_attachments = [ { @@ -88,10 +88,10 @@ describe SlackService::NoteMessage, models: true do end it 'returns a message regarding notes on an issue' do - message = SlackService::NoteMessage.new(@args) + message = described_class.new(@args) expect(message.pretext).to eq( - "test.user commented on " \ - "<url|issue #20> in <somewhere.com|project_name>: " \ + "test.user <url|commented on " \ + "issue #20> in <somewhere.com|project_name>: " \ "*issue title*") expected_attachments = [ { @@ -114,9 +114,9 @@ describe SlackService::NoteMessage, models: true do end it 'returns a message regarding notes on a project snippet' do - message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("test.user commented on " \ - "<url|snippet #5> in <somewhere.com|project_name>: " \ + message = described_class.new(@args) + expect(message.pretext).to eq("test.user <url|commented on " \ + "snippet #5> in <somewhere.com|project_name>: " \ "*snippet title*") expected_attachments = [ { diff --git a/spec/models/project_services/slack_service/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index babb3909f56..eca71db07b6 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' -describe SlackService::PipelineMessage do - subject { SlackService::PipelineMessage.new(args) } +describe ChatMessage::PipelineMessage do + subject { described_class.new(args) } + let(:user) { { name: 'hacker' } } let(:args) do { @@ -15,7 +16,7 @@ describe SlackService::PipelineMessage do }, project: { path_with_namespace: 'project_name', web_url: 'example.gitlab.com' }, - commit: { author_name: 'hacker' } + user: user } end @@ -28,9 +29,7 @@ describe SlackService::PipelineMessage do let(:message) { build_message('passed') } it 'returns a message with information about succeeded build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + verify_message end end @@ -40,16 +39,29 @@ describe SlackService::PipelineMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + verify_message end + + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_message(status, 'API') } + + it 'returns a message stating it is by API' do + verify_message + end + end + end + + def verify_message + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) end - def build_message(status_text = status) + def build_message(status_text = status, name = user[:name]) "<example.gitlab.com|project_name>:" \ - " Pipeline <example.gitlab.com/pipelines/123|97de212e>" \ + " Pipeline <example.gitlab.com/pipelines/123|#123>" \ " of <example.gitlab.com/commits/develop|develop> branch" \ - " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" + " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index 17cd05e24f1..b781c4505db 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackService::PushMessage, models: true do - subject { SlackService::PushMessage.new(args) } +describe ChatMessage::PushMessage, models: true do + subject { described_class.new(args) } let(:args) do { diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index 093911598b0..94c04dc0865 100644 --- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe SlackService::WikiPageMessage, models: true do +describe ChatMessage::WikiPageMessage, models: true do subject { described_class.new(args) } let(:args) do diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb new file mode 100644 index 00000000000..c98e7ee14fd --- /dev/null +++ b/spec/models/project_services/chat_notification_service_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe ChatNotificationService, models: true do + describe "Associations" do + before do + allow(subject).to receive(:activated?).and_return(true) + end + + it { is_expected.to validate_presence_of :webhook } + end +end diff --git a/spec/models/project_services/chat_service_spec.rb b/spec/models/project_services/chat_service_spec.rb new file mode 100644 index 00000000000..c6a45a3e1be --- /dev/null +++ b/spec/models/project_services/chat_service_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe ChatService, models: true do + describe "Associations" do + it { is_expected.to have_many :chat_names } + end + + describe '#valid_token?' do + subject { described_class.new } + + it 'is false as it has no token' do + expect(subject.valid_token?('wer')).to be_falsey + end + end +end diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 652804fb444..9b80f0e7296 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -35,9 +35,9 @@ describe GitlabIssueTrackerService, models: true do end it 'gives the correct path' do - expect(@service.project_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues") - expect(@service.new_issue_url).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/new") - expect(@service.issue_url(432)).to eq("http://localhost/gitlab/root/#{project.path_with_namespace}/issues/432") + expect(@service.project_url).to eq("http://#{Gitlab.config.gitlab.host}/gitlab/root/#{project.path_with_namespace}/issues") + expect(@service.new_issue_url).to eq("http://#{Gitlab.config.gitlab.host}/gitlab/root/#{project.path_with_namespace}/issues/new") + expect(@service.issue_url(432)).to eq("http://#{Gitlab.config.gitlab.host}/gitlab/root/#{project.path_with_namespace}/issues/432") end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 05ee4a08391..862e3a72a73 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -68,7 +68,8 @@ describe JiraService, models: true do end end - describe "Execute" do + describe '#close_issue' do + let(:custom_base_url) { 'http://custom_url' } let(:user) { create(:user) } let(:project) { create(:project) } let(:merge_request) { create(:merge_request) } @@ -82,35 +83,80 @@ describe JiraService, models: true do url: 'http://jira.example.com', username: 'gitlab_jira_username', password: 'gitlab_jira_password', - project_key: 'GitLabProject' + project_key: 'GitLabProject', + jira_issue_transition_id: "custom-id" ) + # These stubs are needed to test JiraService#close_issue. + # We close the issue then do another request to API to check if it got closed. + # Here is stubbed the API return with a closed and an opened issues. + open_issue = JIRA::Resource::Issue.new(@jira_service.client, attrs: { "id" => "JIRA-123" }) + closed_issue = open_issue.dup + allow(open_issue).to receive(:resolution).and_return(false) + allow(closed_issue).to receive(:resolution).and_return(true) + allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) + + allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-123") + @jira_service.save project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' - @project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' + @remote_link_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/remotelink' - WebMock.stub_request(:get, @project_url) WebMock.stub_request(:get, project_issues_url) WebMock.stub_request(:post, @transitions_url) WebMock.stub_request(:post, @comment_url) + WebMock.stub_request(:post, @remote_link_url) end it "calls JIRA API" do - @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @comment_url).with( body: /Issue solved with/ ).once end + # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links + # for more information + it "creates Remote Link reference in JIRA for comment" do + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) + + # Creates comment + expect(WebMock).to have_requested(:post, @comment_url) + + # Creates Remote Link in JIRA issue fields + expect(WebMock).to have_requested(:post, @remote_link_url).with( + body: hash_including( + GlobalID: "GitLab", + object: { + url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{merge_request.diff_head_sha}", + title: "GitLab: Solved by commit #{merge_request.diff_head_sha}.", + icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" }, + status: { resolved: true, icon: { url16x16: "http://www.openwebgraphics.com/resources/data/1768/16x16_apply.png", title: "Closed" } } + } + ) + ).once + end + + it "does not send comment or remote links to issues already closed" do + allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true) + + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) + + expect(WebMock).not_to have_requested(:post, @comment_url) + expect(WebMock).not_to have_requested(:post, @remote_link_url) + end + it "references the GitLab commit/merge request" do - @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) + stub_config_setting(base_url: custom_base_url) + + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @comment_url).with( - body: /#{Gitlab.config.gitlab.url}\/#{project.path_with_namespace}\/commit\/#{merge_request.diff_head_sha}/ + body: /#{custom_base_url}\/#{project.path_with_namespace}\/commit\/#{merge_request.diff_head_sha}/ ).once end @@ -122,7 +168,7 @@ describe JiraService, models: true do { script_name: '/gitlab' } end - @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @comment_url).with( body: /#{Gitlab.config.gitlab.url}\/#{project.path_with_namespace}\/commit\/#{merge_request.diff_head_sha}/ @@ -130,20 +176,33 @@ describe JiraService, models: true do end it "calls the api with jira_issue_transition_id" do - @jira_service.jira_issue_transition_id = 'this-is-a-custom-id' - @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @transitions_url).with( - body: /this-is-a-custom-id/ + body: /custom-id/ ).once end + end - context "when testing" do - it "tries to get jira project" do - @jira_service.execute(nil) + describe '#test_settings' do + let(:jira_service) do + described_class.new( + url: 'http://jira.example.com', + username: 'gitlab_jira_username', + password: 'gitlab_jira_password', + project_key: 'GitLabProject' + ) + end + let(:project_url) { 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' } - expect(WebMock).to have_requested(:get, @project_url) - end + before do + WebMock.stub_request(:get, project_url) + end + + it 'tries to get JIRA project' do + jira_service.test_settings + + expect(WebMock).to have_requested(:get, project_url) end end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb new file mode 100644 index 00000000000..3603602e41d --- /dev/null +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -0,0 +1,159 @@ +require 'spec_helper' + +describe KubernetesService, models: true do + let(:project) { create(:empty_project) } + + describe "Associations" do + it { is_expected.to belong_to :project } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_presence_of(:api_url) } + it { is_expected.to validate_presence_of(:token) } + + context 'namespace format' do + before do + subject.project = project + subject.api_url = "http://example.com" + subject.token = "test" + end + + { + 'foo' => true, + '1foo' => true, + 'foo1' => true, + 'foo-bar' => true, + '-foo' => false, + 'foo-' => false, + 'a' * 63 => true, + 'a' * 64 => false, + 'a.b' => false, + 'a*b' => false, + }.each do |namespace, validity| + it "should validate #{namespace} as #{validity ? 'valid' : 'invalid'}" do + subject.namespace = namespace + + expect(subject.valid?).to eq(validity) + end + end + end + end + + context 'when service is inactive' do + before { subject.active = false } + it { is_expected.not_to validate_presence_of(:namespace) } + it { is_expected.not_to validate_presence_of(:api_url) } + it { is_expected.not_to validate_presence_of(:token) } + end + end + + describe '#initialize_properties' do + context 'with a project' do + it 'defaults to the project name' do + expect(described_class.new(project: project).namespace).to eq(project.name) + end + end + + context 'without a project' do + it 'leaves the namespace unset' do + expect(described_class.new.namespace).to be_nil + end + end + end + + describe '#test' do + let(:project) { create(:kubernetes_project) } + let(:service) { project.kubernetes_service } + let(:discovery_url) { service.api_url + '/api/v1' } + + # JSON response body from Kubernetes GET /api/v1 request + let(:discovery_response) { { "kind" => "APIResourceList", "groupVersion" => "v1", "resources" => [] }.to_json } + + context 'with path prefix in api_url' do + let(:discovery_url) { 'https://kubernetes.example.com/prefix/api/v1' } + + before do + service.api_url = 'https://kubernetes.example.com/prefix/' + end + + it 'tests with the prefix' do + WebMock.stub_request(:get, discovery_url).to_return(body: discovery_response) + + expect(service.test[:success]).to be_truthy + expect(WebMock).to have_requested(:get, discovery_url).once + end + end + + context 'with custom CA certificate' do + let(:certificate) { "CA PEM DATA" } + before do + service.update_attributes!(ca_pem: certificate) + end + + it 'is added to the certificate store' do + cert = double("certificate") + + expect(OpenSSL::X509::Certificate).to receive(:new).with(certificate).and_return(cert) + expect_any_instance_of(OpenSSL::X509::Store).to receive(:add_cert).with(cert) + WebMock.stub_request(:get, discovery_url).to_return(body: discovery_response) + + expect(service.test[:success]).to be_truthy + expect(WebMock).to have_requested(:get, discovery_url).once + end + end + + context 'success' do + it 'reads the discovery endpoint' do + WebMock.stub_request(:get, discovery_url).to_return(body: discovery_response) + + expect(service.test[:success]).to be_truthy + expect(WebMock).to have_requested(:get, discovery_url).once + end + end + + context 'failure' do + it 'fails to read the discovery endpoint' do + WebMock.stub_request(:get, discovery_url).to_return(status: 404) + + expect(service.test[:success]).to be_falsy + expect(WebMock).to have_requested(:get, discovery_url).once + end + end + end + + describe '#predefined_variables' do + before do + subject.api_url = 'https://kube.domain.com' + subject.token = 'token' + subject.namespace = 'my-project' + subject.ca_pem = 'CA PEM DATA' + end + + it 'sets KUBE_URL' do + expect(subject.predefined_variables).to include( + { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true } + ) + end + + it 'sets KUBE_TOKEN' do + expect(subject.predefined_variables).to include( + { key: 'KUBE_TOKEN', value: 'token', public: false } + ) + end + + it 'sets KUBE_NAMESPACE' do + expect(subject.predefined_variables).to include( + { key: 'KUBE_NAMESPACE', value: 'my-project', public: true } + ) + end + + it 'sets KUBE_CA_PEM' do + expect(subject.predefined_variables).to include( + { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true } + ) + end + end +end diff --git a/spec/models/project_services/mattermost_notification_service_spec.rb b/spec/models/project_services/mattermost_notification_service_spec.rb new file mode 100644 index 00000000000..c01e64b4c8e --- /dev/null +++ b/spec/models/project_services/mattermost_notification_service_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe MattermostNotificationService, models: true do + it_behaves_like "slack or mattermost" +end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb new file mode 100644 index 00000000000..4a1037e950b --- /dev/null +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +describe MattermostSlashCommandsService, models: true do + describe "Associations" do + it { is_expected.to respond_to :token } + end + + describe '#valid_token?' do + subject { described_class.new } + + context 'when the token is empty' do + it 'is false' do + expect(subject.valid_token?('wer')).to be_falsey + end + end + + context 'when there is a token' do + before do + subject.token = '123' + end + + it 'accepts equal tokens' do + expect(subject.valid_token?('123')).to be_truthy + end + end + end + + describe '#trigger' do + subject { described_class.new } + + context 'no token is passed' do + let(:params) { Hash.new } + + it 'returns nil' do + expect(subject.trigger(params)).to be_nil + end + end + + context 'with a token passed' do + let(:project) { create(:empty_project) } + let(:params) { { token: 'token' } } + + before do + allow(subject).to receive(:token).and_return('token') + end + + context 'no user can be found' do + context 'when no url can be generated' do + it 'responds with the authorize url' do + response = subject.trigger(params) + + expect(response[:response_type]).to eq :ephemeral + expect(response[:text]).to start_with ":sweat_smile: Couldn't identify you" + end + end + + context 'when an auth url can be generated' do + let(:params) do + { + team_domain: 'http://domain.tld', + team_id: 'T3423423', + user_id: 'U234234', + user_name: 'mepmep', + token: 'token' + } + end + + let(:service) do + project.create_mattermost_slash_commands_service( + properties: { token: 'token' } + ) + end + + it 'generates the url' do + response = service.trigger(params) + + expect(response[:text]).to start_with(':wave: Hi there!') + end + end + end + + context 'when the user is authenticated' do + let!(:chat_name) { create(:chat_name, service: service) } + let(:service) do + project.create_mattermost_slash_commands_service( + properties: { token: 'token' } + ) + end + let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } } + + it 'triggers the command' do + expect_any_instance_of(Gitlab::ChatCommands::Command).to receive(:execute) + + service.trigger(params) + end + end + end + end +end diff --git a/spec/models/project_services/pipeline_email_service_spec.rb b/spec/models/project_services/pipeline_email_service_spec.rb index 1368a2925e8..7c8824485f5 100644 --- a/spec/models/project_services/pipeline_email_service_spec.rb +++ b/spec/models/project_services/pipeline_email_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe PipelinesEmailService do + include EmailHelpers + let(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit('master').sha) end @@ -13,7 +15,7 @@ describe PipelinesEmailService do end before do - ActionMailer::Base.deliveries.clear + reset_delivered_emails! end describe 'Validations' do @@ -23,14 +25,6 @@ describe PipelinesEmailService do end it { is_expected.to validate_presence_of(:recipients) } - - context 'when pusher is added' do - before do - subject.add_pusher = true - end - - it { is_expected.not_to validate_presence_of(:recipients) } - end end context 'when service is inactive' do @@ -66,8 +60,7 @@ describe PipelinesEmailService do end it 'sends email' do - sent_to = ActionMailer::Base.deliveries.flat_map(&:to) - expect(sent_to).to contain_exactly(recipient) + should_only_email(double(notification_email: recipient), kind: :bcc) end end @@ -79,7 +72,7 @@ describe PipelinesEmailService do end it 'does not send email' do - expect(ActionMailer::Base.deliveries).to be_empty + should_not_email_anyone end end diff --git a/spec/models/project_services/slack_notification_service_spec.rb b/spec/models/project_services/slack_notification_service_spec.rb new file mode 100644 index 00000000000..59ddddf7454 --- /dev/null +++ b/spec/models/project_services/slack_notification_service_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe SlackNotificationService, models: true do + it_behaves_like "slack or mattermost" +end diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb deleted file mode 100644 index c07a70a8069..00000000000 --- a/spec/models/project_services/slack_service_spec.rb +++ /dev/null @@ -1,327 +0,0 @@ -require 'spec_helper' - -describe SlackService, models: true do - let(:slack) { SlackService.new } - let(:webhook_url) { 'https://example.gitlab.com/' } - - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - - describe 'Validations' do - context 'when service is active' do - before { subject.active = true } - - it { is_expected.to validate_presence_of(:webhook) } - it_behaves_like 'issue tracker service URL attribute', :webhook - end - - context 'when service is inactive' do - before { subject.active = false } - - it { is_expected.not_to validate_presence_of(:webhook) } - end - end - - describe "Execute" do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:username) { 'slack_username' } - let(:channel) { 'slack_channel' } - - let(:push_sample_data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end - - before do - allow(slack).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) - - WebMock.stub_request(:post, webhook_url) - - opts = { - title: 'Awesome issue', - description: 'please fix' - } - - issue_service = Issues::CreateService.new(project, user, opts) - @issue = issue_service.execute - @issues_sample_data = issue_service.hook_data(@issue, 'open') - - opts = { - title: 'Awesome merge_request', - description: 'please fix', - source_branch: 'feature', - target_branch: 'master' - } - merge_service = MergeRequests::CreateService.new(project, - user, opts) - @merge_request = merge_service.execute - @merge_sample_data = merge_service.hook_data(@merge_request, - 'open') - - opts = { - title: "Awesome wiki_page", - content: "Some text describing some thing or another", - format: "md", - message: "user created page: Awesome wiki_page" - } - - wiki_page_service = WikiPages::CreateService.new(project, user, opts) - @wiki_page = wiki_page_service.execute - @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') - end - - it "calls Slack API for push events" do - slack.execute(push_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack API for issue events" do - slack.execute(@issues_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack API for merge requests events" do - slack.execute(@merge_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack API for wiki page events" do - slack.execute(@wiki_page_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it 'uses the username as an option for slack when configured' do - allow(slack).to receive(:username).and_return(username) - expect(Slack::Notifier).to receive(:new). - with(webhook_url, username: username). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(push_sample_data) - end - - it 'uses the channel as an option when it is configured' do - allow(slack).to receive(:channel).and_return(channel) - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: channel). - and_return( - double(:slack_service).as_null_object - ) - slack.execute(push_sample_data) - end - - context "event channels" do - it "uses the right channel for push event" do - slack.update_attributes(push_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(push_sample_data) - end - - it "uses the right channel for merge request event" do - slack.update_attributes(merge_request_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(@merge_sample_data) - end - - it "uses the right channel for issue event" do - slack.update_attributes(issue_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(@issues_sample_data) - end - - it "uses the right channel for wiki event" do - slack.update_attributes(wiki_page_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(@wiki_page_sample_data) - end - - context "note event" do - let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") - end - - it "uses the right channel" do - slack.update_attributes(note_channel: "random") - - note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - slack.execute(note_data) - end - end - end - end - - describe "Note events" do - let(:user) { create(:user) } - let(:project) { create(:project, creator_id: user.id) } - - before do - allow(slack).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) - - WebMock.stub_request(:post, webhook_url) - end - - context 'when commit comment event executed' do - let(:commit_note) do - create(:note_on_commit, author: user, - project: project, - commit_id: project.repository.commit.id, - note: 'a comment on a commit') - end - - it "calls Slack API for commit comment events" do - data = Gitlab::DataBuilder::Note.build(commit_note, user) - slack.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when merge request comment event executed' do - let(:merge_request_note) do - create(:note_on_merge_request, project: project, - note: "merge request note") - end - - it "calls Slack API for merge request comment events" do - data = Gitlab::DataBuilder::Note.build(merge_request_note, user) - slack.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when issue comment event executed' do - let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") - end - - it "calls Slack API for issue comment events" do - data = Gitlab::DataBuilder::Note.build(issue_note, user) - slack.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when snippet comment event executed' do - let(:snippet_note) do - create(:note_on_project_snippet, project: project, - note: "snippet note") - end - - it "calls Slack API for snippet comment events" do - data = Gitlab::DataBuilder::Note.build(snippet_note, user) - slack.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - end - - describe 'Pipeline events' do - let(:user) { create(:user) } - let(:project) { create(:project) } - - let(:pipeline) do - create(:ci_pipeline, - project: project, status: status, - sha: project.commit.sha, ref: project.default_branch) - end - - before do - allow(slack).to receive_messages( - project: project, - service_hook: true, - webhook: webhook_url - ) - end - - shared_examples 'call Slack API' do - before do - WebMock.stub_request(:post, webhook_url) - end - - it 'calls Slack API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - slack.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'with failed pipeline' do - let(:status) { 'failed' } - - it_behaves_like 'call Slack API' - end - - context 'with succeeded pipeline' do - let(:status) { 'success' } - - context 'with default to notify_only_broken_pipelines' do - it 'does not call Slack API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - result = slack.execute(data) - - expect(result).to be_falsy - end - end - - context 'with setting notify_only_broken_pipelines to false' do - before do - slack.notify_only_broken_pipelines = false - end - - it_behaves_like 'call Slack API' - end - end - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 0810d06b50f..ed6b2c6a22b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -20,8 +20,10 @@ describe Project, models: true do it { is_expected.to have_many(:deploy_keys) } it { is_expected.to have_many(:hooks).dependent(:destroy) } it { is_expected.to have_many(:protected_branches).dependent(:destroy) } + it { is_expected.to have_many(:chat_services) } it { is_expected.to have_one(:forked_project_link).dependent(:destroy) } - it { is_expected.to have_one(:slack_service).dependent(:destroy) } + it { is_expected.to have_one(:slack_notification_service).dependent(:destroy) } + it { is_expected.to have_one(:mattermost_notification_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } it { is_expected.to have_many(:boards).dependent(:destroy) } @@ -35,6 +37,7 @@ describe Project, models: true do it { is_expected.to have_one(:hipchat_service).dependent(:destroy) } it { is_expected.to have_one(:flowdock_service).dependent(:destroy) } it { is_expected.to have_one(:assembla_service).dependent(:destroy) } + it { is_expected.to have_one(:mattermost_slash_commands_service).dependent(:destroy) } it { is_expected.to have_one(:gemnasium_service).dependent(:destroy) } it { is_expected.to have_one(:buildkite_service).dependent(:destroy) } it { is_expected.to have_one(:bamboo_service).dependent(:destroy) } @@ -76,7 +79,7 @@ describe Project, models: true do end describe '#members & #requesters' do - let(:project) { create(:project, :public) } + let(:project) { create(:empty_project, :public, :access_requestable) } let(:requester) { create(:user) } let(:developer) { create(:user) } before do @@ -129,14 +132,18 @@ describe Project, models: true do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) } - it { is_expected.to validate_length_of(:name).is_within(0..255) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_uniqueness_of(:path).scoped_to(:namespace_id) } - it { is_expected.to validate_length_of(:path).is_within(0..255) } - it { is_expected.to validate_length_of(:description).is_within(0..2000) } + it { is_expected.to validate_length_of(:path).is_at_most(255) } + + it { is_expected.to validate_length_of(:description).is_at_most(2000) } + it { is_expected.to validate_presence_of(:creator) } + it { is_expected.to validate_presence_of(:namespace) } + it { is_expected.to validate_presence_of(:repository_storage) } it 'does not allow new projects beyond user limits' do @@ -241,6 +248,13 @@ describe Project, models: true do it { is_expected.to respond_to(:path_with_namespace) } end + describe 'delegation' do + it { is_expected.to delegate_method(:add_guest).to(:team) } + it { is_expected.to delegate_method(:add_reporter).to(:team) } + it { is_expected.to delegate_method(:add_developer).to(:team) } + it { is_expected.to delegate_method(:add_master).to(:team) } + end + describe '#name_with_namespace' do let(:project) { build_stubbed(:empty_project) } @@ -249,10 +263,70 @@ describe Project, models: true do end describe '#to_reference' do - let(:project) { create(:empty_project) } + let(:owner) { create(:user, name: 'Gitlab') } + let(:namespace) { create(:namespace, path: 'sample-namespace', owner: owner) } + let(:project) { create(:empty_project, path: 'sample-project', namespace: namespace) } + + context 'when nil argument' do + it 'returns nil' do + expect(project.to_reference).to be_nil + end + end + + context 'when same project argument' do + it 'returns nil' do + expect(project.to_reference(project)).to be_nil + end + end + + context 'when cross namespace project argument' do + let(:another_namespace_project) { create(:empty_project, name: 'another-project') } + + it 'returns complete path to the project' do + expect(project.to_reference(another_namespace_project)).to eq 'sample-namespace/sample-project' + end + end + + context 'when same namespace / cross-project argument' do + let(:another_project) { create(:empty_project, namespace: namespace) } + + it 'returns complete path to the project' do + expect(project.to_reference(another_project)).to eq 'sample-project' + end + end + end + + describe '#to_human_reference' do + let(:owner) { create(:user, name: 'Gitlab') } + let(:namespace) { create(:namespace, name: 'Sample namespace', owner: owner) } + let(:project) { create(:empty_project, name: 'Sample project', namespace: namespace) } + + context 'when nil argument' do + it 'returns nil' do + expect(project.to_human_reference).to be_nil + end + end + + context 'when same project argument' do + it 'returns nil' do + expect(project.to_human_reference(project)).to be_nil + end + end + + context 'when cross namespace project argument' do + let(:another_namespace_project) { create(:empty_project, name: 'another-project') } - it 'returns a String reference to the object' do - expect(project.to_reference).to eq project.path_with_namespace + it 'returns complete name with namespace of the project' do + expect(project.to_human_reference(another_namespace_project)).to eq 'Gitlab / Sample project' + end + end + + context 'when same namespace / cross-project argument' do + let(:another_project) { create(:empty_project, namespace: namespace) } + + it 'returns name of the project' do + expect(project.to_human_reference(another_project)).to eq 'Sample project' + end end end @@ -352,10 +426,15 @@ describe Project, models: true do describe '#get_issue' do let(:project) { create(:empty_project) } let!(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + end context 'with default issues tracker' do it 'returns an issue' do - expect(project.get_issue(issue.iid)).to eq issue + expect(project.get_issue(issue.iid, user)).to eq issue end it 'returns count of open issues' do @@ -363,7 +442,12 @@ describe Project, models: true do end it 'returns nil when no issue found' do - expect(project.get_issue(999)).to be_nil + expect(project.get_issue(999, user)).to be_nil + end + + it "returns nil when user doesn't have access" do + user = create(:user) + expect(project.get_issue(issue.iid, user)).to eq nil end end @@ -373,7 +457,7 @@ describe Project, models: true do end it 'returns an ExternalIssue' do - issue = project.get_issue('FOO-1234') + issue = project.get_issue('FOO-1234', user) expect(issue).to be_kind_of(ExternalIssue) expect(issue.iid).to eq 'FOO-1234' expect(issue.project).to eq project @@ -395,35 +479,6 @@ describe Project, models: true do end end - describe '.find_with_namespace' do - context 'with namespace' do - before do - @group = create :group, name: 'gitlab' - @project = create(:project, name: 'gitlabhq', namespace: @group) - end - - it { expect(Project.find_with_namespace('gitlab/gitlabhq')).to eq(@project) } - it { expect(Project.find_with_namespace('GitLab/GitlabHQ')).to eq(@project) } - it { expect(Project.find_with_namespace('gitlab-ci')).to be_nil } - end - - context 'when multiple projects using a similar name exist' do - let(:group) { create(:group, name: 'gitlab') } - - let!(:project1) do - create(:empty_project, name: 'gitlab1', path: 'gitlab', namespace: group) - end - - let!(:project2) do - create(:empty_project, name: 'gitlab2', path: 'GITLAB', namespace: group) - end - - it 'returns the row where the path matches literally' do - expect(Project.find_with_namespace('gitlab/GITLAB')).to eq(project2) - end - end - end - describe '#to_param' do context 'with namespace' do before do @@ -496,9 +551,6 @@ describe Project, models: true do end it 'returns nil and does not query services when there is no external issue tracker' do - project.build_missing_services - project.reload - expect(project).not_to receive(:services) expect(project.external_issue_tracker).to eq(nil) @@ -506,9 +558,6 @@ describe Project, models: true do it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do ext_project.reload # Factory returns a project with changed attributes - ext_project.build_missing_services - ext_project.reload - expect(ext_project).to receive(:services).once.and_call_original 2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) } @@ -706,7 +755,7 @@ describe Project, models: true do "/uploads/project/avatar/#{project.id}/uploads/avatar.png" end - it { should eq "http://localhost#{avatar_path}" } + it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } end context 'When avatar file in git' do @@ -718,7 +767,7 @@ describe Project, models: true do "/#{project.namespace.name}/#{project.path}/avatar" end - it { should eq "http://localhost#{avatar_path}" } + it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } end context 'when git repo is empty' do @@ -1471,88 +1520,6 @@ describe Project, models: true do end end - describe '.where_paths_in' do - context 'without any paths' do - it 'returns an empty relation' do - expect(Project.where_paths_in([])).to eq([]) - end - end - - context 'without any valid paths' do - it 'returns an empty relation' do - expect(Project.where_paths_in(%w[foo])).to eq([]) - end - end - - context 'with valid paths' do - let!(:project1) { create(:project) } - let!(:project2) { create(:project) } - - it 'returns the projects matching the paths' do - projects = Project.where_paths_in([project1.path_with_namespace, - project2.path_with_namespace]) - - expect(projects).to contain_exactly(project1, project2) - end - - it 'returns projects regardless of the casing of paths' do - projects = Project.where_paths_in([project1.path_with_namespace.upcase, - project2.path_with_namespace.upcase]) - - expect(projects).to contain_exactly(project1, project2) - end - end - end - - describe 'authorized_for_user' do - let(:group) { create(:group) } - let(:developer) { create(:user) } - let(:master) { create(:user) } - let(:personal_project) { create(:project, namespace: developer.namespace) } - let(:group_project) { create(:project, namespace: group) } - let(:members_project) { create(:project) } - let(:shared_project) { create(:project) } - - before do - group.add_master(master) - group.add_developer(developer) - - members_project.team << [developer, :developer] - members_project.team << [master, :master] - - create(:project_group_link, project: shared_project, group: group) - end - - it 'returns false for no user' do - expect(personal_project.authorized_for_user?(nil)).to be(false) - end - - it 'returns true for personal projects of the user' do - expect(personal_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects of groups the user is a member of' do - expect(group_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects for which the user is a member of' do - expect(members_project.authorized_for_user?(developer)).to be(true) - end - - it 'returns true for projects shared on a group the user is a member of' do - expect(shared_project.authorized_for_user?(developer)).to be(true) - end - - it 'checks for the correct minimum level access' do - expect(group_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(group_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true) - expect(members_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(members_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true) - expect(shared_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false) - expect(shared_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true) - end - end - describe 'change_head' do let(:project) { create(:project) } @@ -1574,7 +1541,7 @@ describe Project, models: true do end it 'expires the avatar cache' do - expect(project.repository).to receive(:expire_avatar_cache).with(project.default_branch) + expect(project.repository).to receive(:expire_avatar_cache) project.change_head(project.default_branch) end @@ -1646,15 +1613,18 @@ describe Project, models: true do end it 'returns environment when with_tags is set' do - expect(project.environments_for('master', project.commit, with_tags: true)).to contain_exactly(environment) + expect(project.environments_for('master', commit: project.commit, with_tags: true)) + .to contain_exactly(environment) end it 'does not return environment when no with_tags is set' do - expect(project.environments_for('master', project.commit)).to be_empty + expect(project.environments_for('master', commit: project.commit)) + .to be_empty end it 'does not return environment when commit is not part of deployment' do - expect(project.environments_for('master', project.commit('feature'))).to be_empty + expect(project.environments_for('master', commit: project.commit('feature'))) + .to be_empty end end @@ -1664,15 +1634,85 @@ describe Project, models: true do end it 'returns environment when ref is set' do - expect(project.environments_for('master', project.commit)).to contain_exactly(environment) + expect(project.environments_for('master', commit: project.commit)) + .to contain_exactly(environment) end it 'does not environment when ref is different' do - expect(project.environments_for('feature', project.commit)).to be_empty + expect(project.environments_for('feature', commit: project.commit)) + .to be_empty end it 'does not return environment when commit is not part of deployment' do - expect(project.environments_for('master', project.commit('feature'))).to be_empty + expect(project.environments_for('master', commit: project.commit('feature'))) + .to be_empty + end + + it 'returns environment when commit constraint is not set' do + expect(project.environments_for('master')) + .to contain_exactly(environment) + end + end + end + + describe '#environments_recently_updated_on_branch' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + context 'when last deployment to environment is the most recent one' do + before do + create(:deployment, environment: environment, ref: 'feature') + end + + it 'finds recently updated environment' do + expect(project.environments_recently_updated_on_branch('feature')) + .to contain_exactly(environment) + end + end + + context 'when last deployment to environment is not the most recent' do + before do + create(:deployment, environment: environment, ref: 'feature') + create(:deployment, environment: environment, ref: 'master') + end + + it 'does not find environment' do + expect(project.environments_recently_updated_on_branch('feature')) + .to be_empty + end + end + + context 'when there are two environments that deploy to the same branch' do + let(:second_environment) { create(:environment, project: project) } + + before do + create(:deployment, environment: environment, ref: 'feature') + create(:deployment, environment: second_environment, ref: 'feature') + end + + it 'finds both environments' do + expect(project.environments_recently_updated_on_branch('feature')) + .to contain_exactly(environment, second_environment) + end + end + end + + describe '#deployment_variables' do + context 'when project has no deployment service' do + let(:project) { create(:empty_project) } + + it 'returns an empty array' do + expect(project.deployment_variables).to eq [] + end + end + + context 'when project has a deployment service' do + let(:project) { create(:kubernetes_project) } + + it 'returns variables from this service' do + expect(project.deployment_variables).to include( + { key: 'KUBE_TOKEN', value: project.kubernetes_service.token, public: false } + ) end end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index e0f2dadf189..0475cecaa2d 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -10,9 +10,9 @@ describe ProjectTeam, models: true do let(:project) { create(:empty_project) } before do - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [guest, :guest] + project.add_master(master) + project.add_reporter(reporter) + project.add_guest(guest) end describe 'members collection' do @@ -37,7 +37,7 @@ describe ProjectTeam, models: true do context 'group project' do let(:group) { create(:group) } - let(:project) { create(:empty_project, group: group) } + let!(:project) { create(:empty_project, group: group) } before do group.add_master(master) @@ -47,8 +47,8 @@ describe ProjectTeam, models: true do # If user is a group and a project member - GitLab uses highest permission # So we add group guest as master and add group master as guest # to this project to test highest access - project.team << [guest, :master] - project.team << [master, :guest] + project.add_master(guest) + project.add_guest(master) end describe 'members collection' do @@ -79,14 +79,14 @@ describe ProjectTeam, models: true do it 'returns project members' do user = create(:user) - project.team << [user, :guest] + project.add_guest(user) expect(project.team.members).to contain_exactly(user) end it 'returns project members of a specified level' do user = create(:user) - project.team << [user, :reporter] + project.add_reporter(user) expect(project.team.guests).to be_empty expect(project.team.reporters).to contain_exactly(user) @@ -118,7 +118,7 @@ describe ProjectTeam, models: true do context 'group project' do let(:group) { create(:group) } - let(:project) { create(:empty_project, group: group) } + let!(:project) { create(:empty_project, group: group) } it 'returns project members' do group_member = create(:group_member, group: group) @@ -137,13 +137,13 @@ describe ProjectTeam, models: true do describe '#find_member' do context 'personal project' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:empty_project, :public, :access_requestable) } let(:requester) { create(:user) } before do - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [guest, :guest] + project.add_master(master) + project.add_reporter(reporter) + project.add_guest(guest) project.request_access(requester) end @@ -155,7 +155,7 @@ describe ProjectTeam, models: true do end context 'group project' do - let(:group) { create(:group) } + let(:group) { create(:group, :access_requestable) } let(:project) { create(:empty_project, group: group) } let(:requester) { create(:user) } @@ -178,9 +178,9 @@ describe ProjectTeam, models: true do it 'returns Master role' do user = create(:user) group = create(:group) - group.add_master(user) + project = create(:empty_project, namespace: group) - project = build_stubbed(:empty_project, namespace: group) + group.add_master(user) expect(project.team.human_max_access(user.id)).to eq 'Master' end @@ -188,9 +188,9 @@ describe ProjectTeam, models: true do it 'returns Owner role' do user = create(:user) group = create(:group) - group.add_owner(user) + project = create(:empty_project, namespace: group) - project = build_stubbed(:empty_project, namespace: group) + group.add_owner(user) expect(project.team.human_max_access(user.id)).to eq 'Owner' end @@ -200,13 +200,13 @@ describe ProjectTeam, models: true do let(:requester) { create(:user) } context 'personal project' do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:empty_project, :public, :access_requestable) } context 'when project is not shared with group' do before do - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [guest, :guest] + project.add_master(master) + project.add_reporter(reporter) + project.add_guest(guest) project.request_access(requester) end @@ -243,8 +243,8 @@ describe ProjectTeam, models: true do end context 'group project' do - let(:group) { create(:group) } - let(:project) { create(:empty_project, group: group) } + let(:group) { create(:group, :access_requestable) } + let!(:project) { create(:empty_project, group: group) } before do group.add_master(master) @@ -261,6 +261,57 @@ describe ProjectTeam, models: true do end end + describe '#member?' do + let(:group) { create(:group) } + let(:developer) { create(:user) } + let(:master) { create(:user) } + let(:personal_project) { create(:project, namespace: developer.namespace) } + let(:group_project) { create(:project, namespace: group) } + let(:members_project) { create(:project) } + let(:shared_project) { create(:project) } + + before do + group.add_master(master) + group.add_developer(developer) + + members_project.team << [developer, :developer] + members_project.team << [master, :master] + + create(:project_group_link, project: shared_project, group: group) + end + + it 'returns false for no user' do + expect(personal_project.team.member?(nil)).to be(false) + end + + it 'returns true for personal projects of the user' do + expect(personal_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects of groups the user is a member of' do + expect(group_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects for which the user is a member of' do + expect(members_project.team.member?(developer)).to be(true) + end + + it 'returns true for projects shared on a group the user is a member of' do + expect(shared_project.team.member?(developer)).to be(true) + end + + it 'checks for the correct minimum level access' do + expect(group_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(group_project.team.member?(master, Gitlab::Access::MASTER)).to be(true) + expect(members_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(members_project.team.member?(master, Gitlab::Access::MASTER)).to be(true) + expect(shared_project.team.member?(developer, Gitlab::Access::MASTER)).to be(false) + expect(shared_project.team.member?(master, Gitlab::Access::MASTER)).to be(false) + expect(shared_project.team.member?(developer, Gitlab::Access::DEVELOPER)).to be(true) + expect(shared_project.team.member?(master, Gitlab::Access::DEVELOPER)).to be(true) + end + end + shared_examples_for "#max_member_access_for_users" do |enable_request_store| describe "#max_member_access_for_users" do before do @@ -281,10 +332,10 @@ describe ProjectTeam, models: true do guest = create(:user) project = create(:project) - project.team << [master, :master] - project.team << [reporter, :reporter] - project.team << [promoted_guest, :guest] - project.team << [guest, :guest] + project.add_master(master) + project.add_reporter(reporter) + project.add_guest(promoted_guest) + project.add_guest(guest) group = create(:group) group_developer = create(:user) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 04b7d19d414..b5a42edd192 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -113,6 +113,26 @@ describe Repository, models: true do end end + describe '#ref_exists?' do + context 'when ref exists' do + it 'returns true' do + expect(repository.ref_exists?('refs/heads/master')).to be true + end + end + + context 'when ref does not exist' do + it 'returns false' do + expect(repository.ref_exists?('refs/heads/non-existent')).to be false + end + end + + context 'when ref format is incorrect' do + it 'returns false' do + expect(repository.ref_exists?('refs/heads/invalid:master')).to be false + end + end + end + describe '#last_commit_for_path' do subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } @@ -197,6 +217,35 @@ describe Repository, models: true do end end + describe '#commit' do + context 'when ref exists' do + it 'returns commit object' do + expect(repository.commit('master')) + .to be_an_instance_of Commit + end + end + + context 'when ref does not exist' do + it 'returns nil' do + expect(repository.commit('non-existent-ref')).to be_nil + end + end + + context 'when ref is not valid' do + context 'when preceding tree element exists' do + it 'returns nil' do + expect(repository.commit('master:ref')).to be_nil + end + end + + context 'when preceding tree element does not exist' do + it 'returns nil' do + expect(repository.commit('non-existent:ref')).to be_nil + end + end + end + end + describe "#commit_dir" do it "commits a change that creates a new directory" do expect do @@ -344,24 +393,37 @@ describe Repository, models: true do end end - describe "search_files" do - let(:results) { repository.search_files('feature', 'master') } + describe "search_files_by_content" do + let(:results) { repository.search_files_by_content('feature', 'master') } subject { results } it { is_expected.to be_an Array } it 'regex-escapes the query string' do - results = repository.search_files("test\\", 'master') + results = repository.search_files_by_content("test\\", 'master') expect(results.first).not_to start_with('fatal:') end it 'properly handles an unmatched parenthesis' do - results = repository.search_files("test(", 'master') + results = repository.search_files_by_content("test(", 'master') expect(results.first).not_to start_with('fatal:') end + it 'properly handles when query is not present' do + results = repository.search_files_by_content('', 'master') + + expect(results).to match_array([]) + end + + it 'properly handles query when repo is empty' do + repository = create(:empty_project).repository + results = repository.search_files_by_content('test', 'master') + + expect(results).to match_array([]) + end + describe 'result' do subject { results.first } @@ -370,6 +432,28 @@ describe Repository, models: true do end end + describe "search_files_by_name" do + let(:results) { repository.search_files_by_name('files', 'master') } + + it 'returns result' do + expect(results.first).to eq('files/html/500.html') + end + + it 'properly handles when query is not present' do + results = repository.search_files_by_name('', 'master') + + expect(results).to match_array([]) + end + + it 'properly handles query when repo is empty' do + repository = create(:empty_project).repository + + results = repository.search_files_by_name('test', 'master') + + expect(results).to match_array([]) + end + end + describe '#create_ref' do it 'redirects the call to fetch_ref' do ref, ref_path = '1', '2' @@ -380,11 +464,7 @@ describe Repository, models: true do end end - describe "#changelog" do - before do - repository.send(:cache).expire(:changelog) - end - + describe "#changelog", caching: true do it 'accepts changelog' do expect(repository.tree).to receive(:blobs).and_return([TestBlob.new('changelog')]) @@ -416,17 +496,16 @@ describe Repository, models: true do end end - describe "#license_blob" do + describe "#license_blob", caching: true do before do - repository.send(:cache).expire(:license_blob) repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master') end it 'handles when HEAD points to non-existent ref' do repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false) - rugged = double('rugged') - expect(rugged).to receive(:head_unborn?).and_return(true) - expect(repository).to receive(:rugged).and_return(rugged) + + allow(repository).to receive(:file_on_head). + and_raise(Rugged::ReferenceError) expect(repository.license_blob).to be_nil end @@ -453,22 +532,18 @@ describe Repository, models: true do end end - describe '#license_key' do + describe '#license_key', caching: true do before do - repository.send(:cache).expire(:license_key) repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master') end - it 'handles when HEAD points to non-existent ref' do - repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false) - rugged = double('rugged') - expect(rugged).to receive(:head_unborn?).and_return(true) - expect(repository).to receive(:rugged).and_return(rugged) - + it 'returns nil when no license is detected' do expect(repository.license_key).to be_nil end - it 'returns nil when no license is detected' do + it 'returns nil when the repository does not exist' do + expect(repository).to receive(:exists?).and_return(false) + expect(repository.license_key).to be_nil end @@ -485,7 +560,7 @@ describe Repository, models: true do end end - describe "#gitlab_ci_yml" do + describe "#gitlab_ci_yml", caching: true do it 'returns valid file' do files = [TestBlob.new('file'), TestBlob.new('.gitlab-ci.yml'), TestBlob.new('copying')] expect(repository.tree).to receive(:blobs).and_return(files) @@ -499,7 +574,7 @@ describe Repository, models: true do end it 'returns nil for empty repository' do - expect(repository).to receive(:empty?).and_return(true) + allow(repository).to receive(:file_on_head).and_raise(Rugged::ReferenceError) expect(repository.gitlab_ci_yml).to be_nil end end @@ -693,8 +768,6 @@ describe Repository, models: true do expect(repository).not_to receive(:expire_root_ref_cache) expect(repository).not_to receive(:expire_emptiness_caches) expect(repository).to receive(:expire_branches_cache) - expect(repository).to receive(:expire_has_visible_content_cache) - expect(repository).to receive(:expire_branch_count_cache) repository.update_branch_with_hooks(user, 'new-feature') { new_rev } end @@ -712,8 +785,6 @@ describe Repository, models: true do expect(empty_repository).to receive(:expire_root_ref_cache) expect(empty_repository).to receive(:expire_emptiness_caches) expect(empty_repository).to receive(:expire_branches_cache) - expect(empty_repository).to receive(:expire_has_visible_content_cache) - expect(empty_repository).to receive(:expire_branch_count_cache) empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', 'Updates file content', 'master', false) @@ -727,8 +798,7 @@ describe Repository, models: true do end it 'returns false when a repository does not exist' do - expect(repository.raw_repository).to receive(:rugged). - and_raise(Gitlab::Git::Repository::NoRepository) + allow(repository).to receive(:refs_directory_exists?).and_return(false) expect(repository.exists?).to eq(false) end @@ -757,15 +827,6 @@ describe Repository, models: true do expect(subject).to eq(true) end - - it 'caches the output' do - expect(repository).to receive(:branch_count). - once. - and_return(3) - - repository.has_visible_content? - repository.has_visible_content? - end end end @@ -832,34 +893,6 @@ describe Repository, models: true do end end - describe '#expire_cache' do - it 'expires all caches' do - expect(repository).to receive(:expire_branch_cache) - - repository.expire_cache - end - - it 'expires the caches for a specific branch' do - expect(repository).to receive(:expire_branch_cache).with('master') - - repository.expire_cache('master') - end - - it 'expires the emptiness caches for an empty repository' do - expect(repository).to receive(:empty?).and_return(true) - expect(repository).to receive(:expire_emptiness_caches) - - repository.expire_cache - end - - it 'does not expire the emptiness caches for a non-empty repository' do - expect(repository).to receive(:empty?).and_return(false) - expect(repository).not_to receive(:expire_emptiness_caches) - - repository.expire_cache - end - end - describe '#expire_root_ref_cache' do it 'expires the root reference cache' do repository.root_ref @@ -874,20 +907,6 @@ describe Repository, models: true do end end - describe '#expire_has_visible_content_cache' do - it 'expires the visible content cache' do - repository.has_visible_content? - - expect(repository).to receive(:branch_count). - once. - and_return(0) - - repository.expire_has_visible_content_cache - - expect(repository.has_visible_content?).to eq(false) - end - end - describe '#expire_branch_cache' do # This method is private but we need it for testing purposes. Sadly there's # no other proper way of testing caching operations. @@ -919,9 +938,18 @@ describe Repository, models: true do describe '#expire_emptiness_caches' do let(:cache) { repository.send(:cache) } - it 'expires the caches' do + it 'expires the caches for an empty repository' do + allow(repository).to receive(:empty?).and_return(true) + expect(cache).to receive(:expire).with(:empty?) - expect(repository).to receive(:expire_has_visible_content_cache) + + repository.expire_emptiness_caches + end + + it 'does not expire the cache for a non-empty repository' do + allow(repository).to receive(:empty?).and_return(false) + + expect(cache).not_to receive(:expire).with(:empty?) repository.expire_emptiness_caches end @@ -1036,24 +1064,12 @@ describe Repository, models: true do repository.before_delete end - it 'flushes the tag count cache' do - expect(repository).to receive(:expire_tag_count_cache) - - repository.before_delete - end - it 'flushes the branches cache' do expect(repository).to receive(:expire_branches_cache) repository.before_delete end - it 'flushes the branch count cache' do - expect(repository).to receive(:expire_branch_count_cache) - - repository.before_delete - end - it 'flushes the root ref cache' do expect(repository).to receive(:expire_root_ref_cache) @@ -1078,36 +1094,18 @@ describe Repository, models: true do allow(repository).to receive(:exists?).and_return(true) end - it 'flushes the caches that depend on repository data' do - expect(repository).to receive(:expire_cache) - - repository.before_delete - end - it 'flushes the tags cache' do expect(repository).to receive(:expire_tags_cache) repository.before_delete end - it 'flushes the tag count cache' do - expect(repository).to receive(:expire_tag_count_cache) - - repository.before_delete - end - it 'flushes the branches cache' do expect(repository).to receive(:expire_branches_cache) repository.before_delete end - it 'flushes the branch count cache' do - expect(repository).to receive(:expire_branch_count_cache) - - repository.before_delete - end - it 'flushes the root ref cache' do expect(repository).to receive(:expire_root_ref_cache) @@ -1138,8 +1136,9 @@ describe Repository, models: true do describe '#before_push_tag' do it 'flushes the cache' do - expect(repository).to receive(:expire_cache) - expect(repository).to receive(:expire_tag_count_cache) + expect(repository).to receive(:expire_statistics_caches) + expect(repository).to receive(:expire_emptiness_caches) + expect(repository).to receive(:expire_tags_cache) repository.before_push_tag end @@ -1156,31 +1155,37 @@ describe Repository, models: true do describe '#after_import' do it 'flushes and builds the cache' do expect(repository).to receive(:expire_content_cache) - expect(repository).to receive(:build_cache) + expect(repository).to receive(:expire_tags_cache) + expect(repository).to receive(:expire_branches_cache) repository.after_import end end describe '#after_push_commit' do - it 'flushes the cache' do - expect(repository).to receive(:expire_cache).with('master', '123') + it 'expires statistics caches' do + expect(repository).to receive(:expire_statistics_caches). + and_call_original - repository.after_push_commit('master', '123') + expect(repository).to receive(:expire_branch_cache). + with('master'). + and_call_original + + repository.after_push_commit('master') end end describe '#after_create_branch' do - it 'flushes the visible content cache' do - expect(repository).to receive(:expire_has_visible_content_cache) + it 'expires the branch caches' do + expect(repository).to receive(:expire_branches_cache) repository.after_create_branch end end describe '#after_remove_branch' do - it 'flushes the visible content cache' do - expect(repository).to receive(:expire_has_visible_content_cache) + it 'expires the branch caches' do + expect(repository).to receive(:expire_branches_cache) repository.after_remove_branch end @@ -1218,7 +1223,8 @@ describe Repository, models: true do describe '#before_remove_tag' do it 'flushes the tag cache' do - expect(repository).to receive(:expire_tag_count_cache) + expect(repository).to receive(:expire_tags_cache).and_call_original + expect(repository).to receive(:expire_statistics_caches).and_call_original repository.before_remove_tag end @@ -1236,23 +1242,23 @@ describe Repository, models: true do end end - describe '#expire_branch_count_cache' do - let(:cache) { repository.send(:cache) } - + describe '#expire_branches_cache' do it 'expires the cache' do - expect(cache).to receive(:expire).with(:branch_count) + expect(repository).to receive(:expire_method_caches). + with(%i(branch_names branch_count)). + and_call_original - repository.expire_branch_count_cache + repository.expire_branches_cache end end - describe '#expire_tag_count_cache' do - let(:cache) { repository.send(:cache) } - + describe '#expire_tags_cache' do it 'expires the cache' do - expect(cache).to receive(:expire).with(:tag_count) + expect(repository).to receive(:expire_method_caches). + with(%i(tag_names tag_count)). + and_call_original - repository.expire_tag_count_cache + repository.expire_tags_cache end end @@ -1275,6 +1281,32 @@ describe Repository, models: true do expect(tag).to be_a(Gitlab::Git::Tag) end + + it 'passes commit SHA to pre-receive and update hooks,\ + and tag SHA to post-receive hook' do + pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', repository.path_to_repo) + update_hook = Gitlab::Git::Hook.new('update', repository.path_to_repo) + post_receive_hook = Gitlab::Git::Hook.new('post-receive', repository.path_to_repo) + + allow(Gitlab::Git::Hook).to receive(:new). + and_return(pre_receive_hook, update_hook, post_receive_hook) + + allow(pre_receive_hook).to receive(:trigger).and_call_original + allow(update_hook).to receive(:trigger).and_call_original + allow(post_receive_hook).to receive(:trigger).and_call_original + + tag = repository.add_tag(user, '8.5', 'master', 'foo') + + commit_sha = repository.commit('master').id + tag_sha = tag.target + + expect(pre_receive_hook).to have_received(:trigger). + with(anything, anything, commit_sha, anything) + expect(update_hook).to have_received(:trigger). + with(anything, anything, commit_sha, anything) + expect(post_receive_hook).to have_received(:trigger). + with(anything, anything, tag_sha, anything) + end end context 'with an invalid target' do @@ -1306,180 +1338,316 @@ describe Repository, models: true do describe '#avatar' do it 'returns nil if repo does not exist' do - expect(repository).to receive(:exists?).and_return(false) + expect(repository).to receive(:file_on_head). + and_raise(Rugged::ReferenceError) expect(repository.avatar).to eq(nil) end it 'returns the first avatar file found in the repository' do - expect(repository).to receive(:blob_at_branch). - with('master', 'logo.png'). - and_return(true) + expect(repository).to receive(:file_on_head). + with(:avatar). + and_return(double(:tree, path: 'logo.png')) expect(repository.avatar).to eq('logo.png') end it 'caches the output' do - allow(repository).to receive(:blob_at_branch). - with('master', 'logo.png'). - and_return(true) - - expect(repository.avatar).to eq('logo.png') + expect(repository).to receive(:file_on_head). + with(:avatar). + once. + and_return(double(:tree, path: 'logo.png')) - expect(repository).not_to receive(:blob_at_branch) - expect(repository.avatar).to eq('logo.png') + 2.times { expect(repository.avatar).to eq('logo.png') } end end - describe '#expire_avatar_cache' do + describe '#expire_exists_cache' do let(:cache) { repository.send(:cache) } - before do - allow(repository).to receive(:cache).and_return(cache) + it 'expires the cache' do + expect(cache).to receive(:expire).with(:exists?) + + repository.expire_exists_cache end + end - context 'without a branch or revision' do - it 'flushes the cache' do - expect(cache).to receive(:expire).with(:avatar) + describe "#keep_around" do + it "does not fail if we attempt to reference bad commit" do + expect(repository.kept_around?('abc1234')).to be_falsey + end - repository.expire_avatar_cache - end + it "stores a reference to the specified commit sha so it isn't garbage collected" do + repository.keep_around(sample_commit.id) + + expect(repository.kept_around?(sample_commit.id)).to be_truthy end - context 'with a branch' do - it 'does not flush the cache if the branch is not the default branch' do - expect(cache).not_to receive(:expire) + it "attempting to call keep_around on truncated ref does not fail" do + repository.keep_around(sample_commit.id) + ref = repository.send(:keep_around_ref_name, sample_commit.id) + path = File.join(repository.path, ref) + # Corrupt the reference + File.truncate(path, 0) + + expect(repository.kept_around?(sample_commit.id)).to be_falsey + + repository.keep_around(sample_commit.id) - repository.expire_avatar_cache('cats') + expect(repository.kept_around?(sample_commit.id)).to be_falsey + + File.delete(path) + end + end + + describe '#update_ref!' do + it 'can create a ref' do + repository.update_ref!('refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) + + expect(repository.find_branch('foobar')).not_to be_nil + end + + it 'raises CommitError when the ref update fails' do + expect do + repository.update_ref!('refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA) + end.to raise_error(Repository::CommitError) + end + end + + describe '#contribution_guide', caching: true do + it 'returns and caches the output' do + expect(repository).to receive(:file_on_head). + with(:contributing). + and_return(Gitlab::Git::Tree.new(path: 'CONTRIBUTING.md')). + once + + 2.times do + expect(repository.contribution_guide). + to be_an_instance_of(Gitlab::Git::Tree) end + end + end - it 'flushes the cache if the branch equals the default branch' do - expect(cache).to receive(:expire).with(:avatar) + describe '#gitignore', caching: true do + it 'returns and caches the output' do + expect(repository).to receive(:file_on_head). + with(:gitignore). + and_return(Gitlab::Git::Tree.new(path: '.gitignore')). + once - repository.expire_avatar_cache(repository.root_ref) + 2.times do + expect(repository.gitignore).to be_an_instance_of(Gitlab::Git::Tree) end end + end - context 'with a branch and revision' do - let(:commit) { double(:commit) } + describe '#koding_yml', caching: true do + it 'returns and caches the output' do + expect(repository).to receive(:file_on_head). + with(:koding). + and_return(Gitlab::Git::Tree.new(path: '.koding.yml')). + once - before do - allow(repository).to receive(:commit).and_return(commit) + 2.times do + expect(repository.koding_yml).to be_an_instance_of(Gitlab::Git::Tree) end + end + end - it 'does not flush the cache if the commit does not change any logos' do - diff = double(:diff, new_path: 'test.txt') + describe '#readme', caching: true do + context 'with a non-existing repository' do + it 'returns nil' do + expect(repository).to receive(:tree).with(:head).and_return(nil) - expect(commit).to receive(:raw_diffs).and_return([diff]) - expect(cache).not_to receive(:expire) + expect(repository.readme).to be_nil + end + end - repository.expire_avatar_cache(repository.root_ref, '123') + context 'with an existing repository' do + it 'returns the README' do + expect(repository.readme).to be_an_instance_of(Gitlab::Git::Blob) end + end + end - it 'flushes the cache if the commit changes any of the logos' do - diff = double(:diff, new_path: Repository::AVATAR_FILES[0]) + describe '#expire_statistics_caches' do + it 'expires the caches' do + expect(repository).to receive(:expire_method_caches). + with(%i(size commit_count)) - expect(commit).to receive(:raw_diffs).and_return([diff]) - expect(cache).to receive(:expire).with(:avatar) + repository.expire_statistics_caches + end + end - repository.expire_avatar_cache(repository.root_ref, '123') - end + describe '#expire_method_caches' do + it 'expires the caches of the given methods' do + expect_any_instance_of(RepositoryCache).to receive(:expire).with(:readme) + expect_any_instance_of(RepositoryCache).to receive(:expire).with(:gitignore) + + repository.expire_method_caches(%i(readme gitignore)) end end - describe '#expire_exists_cache' do - let(:cache) { repository.send(:cache) } + describe '#expire_all_method_caches' do + it 'expires the caches of all methods' do + expect(repository).to receive(:expire_method_caches). + with(Repository::CACHED_METHODS) + repository.expire_all_method_caches + end + end + + describe '#expire_avatar_cache' do it 'expires the cache' do - expect(cache).to receive(:expire).with(:exists?) + expect(repository).to receive(:expire_method_caches).with(%i(avatar)) - repository.expire_exists_cache + repository.expire_avatar_cache end end - describe '#build_cache' do - let(:cache) { repository.send(:cache) } + describe '#file_on_head' do + context 'with a non-existing repository' do + it 'returns nil' do + expect(repository).to receive(:tree).with(:head).and_return(nil) - it 'builds the caches if they do not already exist' do - cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags + expect(repository.file_on_head(:readme)).to be_nil + end + end - expect(cache).to receive(:exist?). - exactly(cache_keys.length). - times. - and_return(false) + context 'with a repository that has no blobs' do + it 'returns nil' do + expect_any_instance_of(Tree).to receive(:blobs).and_return([]) - cache_keys.each do |key| - expect(repository).to receive(key) + expect(repository.file_on_head(:readme)).to be_nil end + end - repository.build_cache + context 'with an existing repository' do + it 'returns a Gitlab::Git::Tree' do + expect(repository.file_on_head(:readme)). + to be_an_instance_of(Gitlab::Git::Tree) + end end + end - it 'does not build any caches that already exist' do - cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags + describe '#head_tree' do + context 'with an existing repository' do + it 'returns a Tree' do + expect(repository.head_tree).to be_an_instance_of(Tree) + end + end - expect(cache).to receive(:exist?). - exactly(cache_keys.length). - times. - and_return(true) + context 'with a non-existing repository' do + it 'returns nil' do + expect(repository).to receive(:head_commit).and_return(nil) - cache_keys.each do |key| - expect(repository).not_to receive(key) + expect(repository.head_tree).to be_nil end - - repository.build_cache end end - describe "#keep_around" do - it "does not fail if we attempt to reference bad commit" do - expect(repository.kept_around?('abc1234')).to be_falsey - end + describe '#tree' do + context 'using a non-existing repository' do + before do + allow(repository).to receive(:head_commit).and_return(nil) + end - it "stores a reference to the specified commit sha so it isn't garbage collected" do - repository.keep_around(sample_commit.id) + it 'returns nil' do + expect(repository.tree(:head)).to be_nil + end - expect(repository.kept_around?(sample_commit.id)).to be_truthy + it 'returns nil when using a path' do + expect(repository.tree(:head, 'README.md')).to be_nil + end end - it "attempting to call keep_around on truncated ref does not fail" do - repository.keep_around(sample_commit.id) - ref = repository.send(:keep_around_ref_name, sample_commit.id) - path = File.join(repository.path, ref) - # Corrupt the reference - File.truncate(path, 0) + context 'using an existing repository' do + it 'returns a Tree' do + expect(repository.tree(:head)).to be_an_instance_of(Tree) + end + end + end - expect(repository.kept_around?(sample_commit.id)).to be_falsey + describe '#size' do + context 'with a non-existing repository' do + it 'returns 0' do + expect(repository).to receive(:exists?).and_return(false) - repository.keep_around(sample_commit.id) + expect(repository.size).to eq(0.0) + end + end - expect(repository.kept_around?(sample_commit.id)).to be_falsey + context 'with an existing repository' do + it 'returns the repository size as a Float' do + expect(repository.size).to be_an_instance_of(Float) + end + end + end - File.delete(path) + describe '#commit_count' do + context 'with a non-existing repository' do + it 'returns 0' do + expect(repository).to receive(:root_ref).and_return(nil) + + expect(repository.commit_count).to eq(0) + end + end + + context 'with an existing repository' do + it 'returns the commit count' do + expect(repository.commit_count).to be_an_instance_of(Fixnum) + end end end - describe '#update_ref!' do - it 'can create a ref' do - repository.update_ref!('refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) + describe '#cache_method_output', caching: true do + context 'with a non-existing repository' do + let(:value) do + repository.cache_method_output(:cats, fallback: 10) do + raise Rugged::ReferenceError + end + end - expect(repository.find_branch('foobar')).not_to be_nil + it 'returns a fallback value' do + expect(value).to eq(10) + end + + it 'does not cache the data' do + value + + expect(repository.instance_variable_defined?(:@cats)).to eq(false) + expect(repository.send(:cache).exist?(:cats)).to eq(false) + end end - it 'raises CommitError when the ref update fails' do - expect do - repository.update_ref!('refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA) - end.to raise_error(Repository::CommitError) + context 'with an existing repository' do + it 'caches the output' do + object = double + + expect(object).to receive(:number).once.and_return(10) + + 2.times do + val = repository.cache_method_output(:cats) { object.number } + + expect(val).to eq(10) + end + + expect(repository.send(:cache).exist?(:cats)).to eq(true) + expect(repository.instance_variable_get(:@cats)).to eq(10) + end end end - describe '#remove_storage_from_path' do - let(:storage_path) { project.repository_storage_path } - let(:project_path) { project.path_with_namespace } - let(:full_path) { File.join(storage_path, project_path) } + describe '#refresh_method_caches' do + it 'refreshes the caches of the given types' do + expect(repository).to receive(:expire_method_caches). + with(%i(readme license_blob license_key)) - it { expect(Repository.remove_storage_from_path(full_path)).to eq(project_path) } - it { expect(Repository.remove_storage_from_path(project_path)).to eq(project_path) } - it { expect(Repository.remove_storage_from_path(storage_path)).to eq('') } + expect(repository).to receive(:readme) + expect(repository).to receive(:license_blob) + expect(repository).to receive(:license_key) + + repository.refresh_method_caches(%i(readme license)) + end end end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb new file mode 100644 index 00000000000..6f491fdf9a0 --- /dev/null +++ b/spec/models/route_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Route, models: true do + let!(:group) { create(:group) } + let!(:route) { group.route } + + describe 'relationships' do + it { is_expected.to belong_to(:source) } + end + + describe 'validations' 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) } + end + + describe '#rename_children' do + let!(:nested_group) { create(:group, path: "test", parent: group) } + let!(:deep_nested_group) { create(:group, path: "foo", parent: nested_group) } + + it "updates children routes with new path" do + route.update_attributes(path: 'bar') + + expect(described_class.exists?(path: 'bar')).to be_truthy + expect(described_class.exists?(path: 'bar/test')).to be_truthy + expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy + end + end +end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 43937a54b2c..691511cd93f 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -6,9 +6,6 @@ describe Service, models: true do it { is_expected.to have_one :service_hook } end - describe "Mass assignment" do - end - describe "Test Button" do before do @service = Service.new @@ -53,7 +50,7 @@ describe Service, models: true do describe "Template" do describe "for pushover service" do - let(:service_template) do + let!(:service_template) do PushoverService.create( template: true, properties: { @@ -66,13 +63,9 @@ describe Service, models: true do let(:project) { create(:project) } describe 'is prefilled for projects pushover service' do - before do - service_template - project.build_missing_services - end - it "has all fields prefilled" do - service = project.pushover_service + service = project.find_or_initialize_service('pushover') + expect(service.template).to eq(false) expect(service.device).to eq('MyDevice') expect(service.sound).to eq('mic') diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index f62f6bacbaa..7425a897769 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -23,9 +23,9 @@ describe Snippet, models: true do it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_length_of(:title).is_within(0..255) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } - it { is_expected.to validate_length_of(:file_name).is_within(0..255) } + it { is_expected.to validate_length_of(:file_name).is_at_most(255) } it { is_expected.to validate_presence_of(:content) } @@ -33,16 +33,51 @@ describe Snippet, models: true do end describe '#to_reference' do + context 'when snippet belongs to a project' do + let(:project) { build(:empty_project, name: 'sample-project') } + let(:snippet) { build(:snippet, id: 1, project: project) } + + it 'returns a String reference to the object' do + expect(snippet.to_reference).to eq "$1" + end + + it 'supports a cross-project reference' do + another_project = build(:project, name: 'another-project', namespace: project.namespace) + expect(snippet.to_reference(another_project)).to eq "sample-project$1" + end + end + + context 'when snippet does not belong to a project' do + let(:snippet) { build(:snippet, id: 1, project: nil) } + + it 'returns a String reference to the object' do + expect(snippet.to_reference).to eq "$1" + end + + it 'still returns shortest reference when project arg present' do + another_project = build(:project, name: 'another-project') + expect(snippet.to_reference(another_project)).to eq "$1" + end + end + end + + describe '#file_name' do let(:project) { create(:empty_project) } - let(:snippet) { create(:snippet, project: project) } - it 'returns a String reference to the object' do - expect(snippet.to_reference).to eq "$#{snippet.id}" + context 'file_name is nil' do + let(:snippet) { create(:snippet, project: project, file_name: nil) } + + it 'returns an empty string' do + expect(snippet.file_name).to eq '' + end end - it 'supports a cross-project reference' do - cross = double('project') - expect(snippet.to_reference(cross)).to eq "#{project.to_reference}$#{snippet.id}" + context 'file_name is not nil' do + let(:snippet) { create(:snippet, project: project, file_name: 'foo.txt') } + + it 'returns the file_name' do + expect(snippet.file_name).to eq 'foo.txt' + end end end diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb new file mode 100644 index 00000000000..9ab112bb2ee --- /dev/null +++ b/spec/models/subscription_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Subscription, models: true do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:subscribable) } + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:subscribable) } + it { is_expected.to validate_presence_of(:user) } + + it 'validates uniqueness of project_id scoped to subscribable_id, subscribable_type, and user_id' do + create(:subscription) + + expect(subject).to validate_uniqueness_of(:project_id).scoped_to([:subscribable_id, :subscribable_type, :user_id]) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3b152e15b61..8b20ee81614 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -33,11 +33,12 @@ describe User, models: true do it { is_expected.to have_many(:award_emoji).dependent(:destroy) } it { is_expected.to have_many(:builds).dependent(:nullify) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } + it { is_expected.to have_many(:chat_names).dependent(:destroy) } describe '#group_members' do it 'does not include group memberships for which user is a requester' do user = create(:user) - group = create(:group, :public) + group = create(:group, :public, :access_requestable) group.request_access(user) expect(user.group_members).to be_empty @@ -47,7 +48,7 @@ describe User, models: true do describe '#project_members' do it 'does not include project memberships for which user is a requester' do user = create(:user) - project = create(:project, :public) + project = create(:project, :public, :access_requestable) project.request_access(user) expect(user.project_members).to be_empty @@ -78,7 +79,7 @@ describe User, models: true do it { is_expected.to allow_value(0).for(:projects_limit) } it { is_expected.not_to allow_value(-1).for(:projects_limit) } - it { is_expected.to validate_length_of(:bio).is_within(0..255) } + it { is_expected.to validate_length_of(:bio).is_at_most(255) } it_behaves_like 'an object with email-formated attributes', :email do subject { build(:user) } @@ -490,6 +491,28 @@ describe User, models: true do end end + describe '.without_projects' do + let!(:project) { create(:empty_project, :public, :access_requestable) } + let!(:user) { create(:user) } + let!(:user_without_project) { create(:user) } + let!(:user_without_project2) { create(:user) } + + before do + # add user to project + project.team << [user, :master] + + # create invite to projet + create(:project_member, :developer, project: project, invite_token: '1234', invite_email: 'inviteduser1@example.com') + + # create request to join project + project.request_access(user_without_project2) + end + + it { expect(User.without_projects).not_to include user } + it { expect(User.without_projects).to include user_without_project } + it { expect(User.without_projects).to include user_without_project2 } + end + describe '.not_in_project' do before do User.delete_all @@ -552,6 +575,23 @@ describe User, models: true do end end end + + describe '#require_ssh_key?' do + protocol_and_expectation = { + 'http' => false, + 'ssh' => true, + '' => true, + } + + protocol_and_expectation.each do |protocol, expected| + it "has correct require_ssh_key?" do + stub_application_setting(enabled_git_access_protocol: protocol) + user = build(:user) + + expect(user.require_ssh_key?).to eq(expected) + end + end + end end describe '.find_by_any_email' do @@ -687,17 +727,6 @@ describe User, models: true do end end - describe 'by_username_or_id' do - let(:user1) { create(:user, username: 'foo') } - - it "gets the correct user" do - expect(User.by_username_or_id(user1.id)).to eq(user1) - expect(User.by_username_or_id('foo')).to eq(user1) - expect(User.by_username_or_id(-1)).to be_nil - expect(User.by_username_or_id('bar')).to be_nil - end - end - describe '.find_by_ssh_key_id' do context 'using an existing SSH key ID' do let(:user) { create(:user) } @@ -729,6 +758,17 @@ describe User, models: true do end end + describe '.find_by_username' do + it 'returns nil if not found' do + expect(described_class.find_by_username('JohnDoe')).to be_nil + end + + it 'is case-insensitive' do + user = create(:user, username: 'JohnDoe') + expect(described_class.find_by_username('JOHNDOE')).to eq user + end + end + describe '.find_by_username!' do it 'raises RecordNotFound' do expect { described_class.find_by_username!('JohnDoe') }. @@ -1050,7 +1090,7 @@ describe User, models: true do it { is_expected.to eq([private_group]) } end - describe '#authorized_projects' do + describe '#authorized_projects', truncate: true do context 'with a minimum access level' do it 'includes projects for which the user is an owner' do user = create(:user) @@ -1070,6 +1110,80 @@ describe User, models: true do .to contain_exactly(project) end end + + it "includes user's personal projects" do + user = create(:user) + project = create(:project, :private, namespace: user.namespace) + + expect(user.authorized_projects).to include(project) + end + + it "includes personal projects user has been given access to" do + user1 = create(:user) + user2 = create(:user) + project = create(:project, :private, namespace: user1.namespace) + + project.team << [user2, Gitlab::Access::DEVELOPER] + + expect(user2.authorized_projects).to include(project) + end + + it "includes projects of groups user has been added to" do + group = create(:group) + project = create(:project, group: group) + user = create(:user) + + group.add_developer(user) + + expect(user.authorized_projects).to include(project) + end + + it "does not include projects of groups user has been removed from" do + group = create(:group) + project = create(:project, group: group) + user = create(:user) + + member = group.add_developer(user) + expect(user.authorized_projects).to include(project) + + member.destroy + expect(user.authorized_projects).not_to include(project) + end + + it "includes projects shared with user's group" do + user = create(:user) + project = create(:project, :private) + group = create(:group) + + group.add_reporter(user) + project.project_group_links.create(group: group) + + expect(user.authorized_projects).to include(project) + end + + it "does not include destroyed projects user had access to" do + user1 = create(:user) + user2 = create(:user) + project = create(:project, :private, namespace: user1.namespace) + + project.team << [user2, Gitlab::Access::DEVELOPER] + expect(user2.authorized_projects).to include(project) + + project.destroy + expect(user2.authorized_projects).not_to include(project) + end + + it "does not include projects of destroyed groups user had access to" do + group = create(:group) + project = create(:project, namespace: group) + user = create(:user) + + group.add_developer(user) + expect(user.authorized_projects).to include(project) + + group.destroy + expect(user.authorized_projects).not_to include(project) + end end describe '#projects_where_can_admin_issues' do @@ -1241,4 +1355,31 @@ describe User, models: true do expect(projects).to be_empty end end + + describe '#refresh_authorized_projects', redis: true do + let(:project1) { create(:empty_project) } + let(:project2) { create(:empty_project) } + let(:user) { create(:user) } + + before do + project1.team << [user, :reporter] + project2.team << [user, :guest] + + user.project_authorizations.delete_all + user.refresh_authorized_projects + end + + it 'refreshes the list of authorized projects' do + expect(user.project_authorizations.count).to eq(2) + end + + it 'sets the authorized_projects_populated column' do + expect(user.authorized_projects_populated).to eq(true) + end + + it 'stores the correct access levels' do + expect(user.project_authorizations.where(access_level: Gitlab::Access::GUEST).exists?).to eq(true) + expect(user.project_authorizations.where(access_level: Gitlab::Access::REPORTER).exists?).to eq(true) + end + end end |