diff options
Diffstat (limited to 'spec/models')
58 files changed, 1604 insertions, 646 deletions
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 1acb5846fcf..853f6943cef 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -1,6 +1,62 @@ require 'spec_helper' describe Ability, lib: true do + describe '.can_edit_note?' do + let(:project) { create(:empty_project) } + let!(:note) { create(:note_on_issue, project: project) } + + context 'using an anonymous user' do + it 'returns false' do + expect(described_class.can_edit_note?(nil, note)).to be_falsy + end + end + + context 'using a system note' do + it 'returns false' do + system_note = create(:note, system: true) + user = create(:user) + + expect(described_class.can_edit_note?(user, system_note)).to be_falsy + end + end + + context 'using users with different access levels' do + let(:user) { create(:user) } + + it 'returns true for the author' do + expect(described_class.can_edit_note?(note.author, note)).to be_truthy + end + + it 'returns false for a guest user' do + project.team << [user, :guest] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns false for a developer' do + project.team << [user, :developer] + + expect(described_class.can_edit_note?(user, note)).to be_falsy + end + + it 'returns true for a master' do + project.team << [user, :master] + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + + it 'returns true for a group owner' do + group = create(:group) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::MASTER) + group.add_owner(user) + + expect(described_class.can_edit_note?(user, note)).to be_truthy + end + end + end + describe '.users_that_can_read_project' do context 'using a public project' do it 'returns all the users' do @@ -114,4 +170,52 @@ describe Ability, lib: true do end end end + + describe '.issues_readable_by_user' do + context 'with an admin user' do + it 'returns all given issues' do + user = build(:user, admin: true) + issue = build(:issue) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + end + + context 'with a regular user' do + it 'returns the issues readable by the user' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + + it 'returns an empty Array when no issues are readable' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(described_class.issues_readable_by_user([issue], user)).to eq([]) + end + end + + context 'without a regular user' do + it 'returns issues that are publicly visible' do + hidden_issue = build(:issue) + visible_issue = build(:issue) + + expect(hidden_issue).to receive(:publicly_visible?).and_return(false) + expect(visible_issue).to receive(:publicly_visible?).and_return(true) + + issues = described_class. + issues_readable_by_user([hidden_issue, visible_issue]) + + expect(issues).to eq([visible_issue]) + end + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index fb040ba82bc..cc215d252f9 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -53,59 +53,59 @@ describe ApplicationSetting, models: true do end context 'restricted signup domains' do - it 'set single domain' do + it 'sets single domain' do setting.domain_whitelist_raw = 'example.com' expect(setting.domain_whitelist).to eq(['example.com']) end - it 'set multiple domains with spaces' do + it 'sets multiple domains with spaces' do setting.domain_whitelist_raw = 'example.com *.example.com' expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end - it 'set multiple domains with newlines and a space' do + it 'sets multiple domains with newlines and a space' do setting.domain_whitelist_raw = "example.com\n *.example.com" expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end - it 'set multiple domains with commas' do + it 'sets multiple domains with commas' do setting.domain_whitelist_raw = "example.com, *.example.com" expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end end context 'blacklisted signup domains' do - it 'set single domain' do + it 'sets single domain' do setting.domain_blacklist_raw = 'example.com' expect(setting.domain_blacklist).to contain_exactly('example.com') end - it 'set multiple domains with spaces' do + it 'sets multiple domains with spaces' do setting.domain_blacklist_raw = 'example.com *.example.com' expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with newlines and a space' do + it 'sets multiple domains with newlines and a space' do setting.domain_blacklist_raw = "example.com\n *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with commas' do + it 'sets multiple domains with commas' do setting.domain_blacklist_raw = "example.com, *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with semicolon' do + it 'sets multiple domains with semicolon' do setting.domain_blacklist_raw = "example.com; *.example.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') end - it 'set multiple domains with mixture of everything' do + it 'sets multiple domains with mixture of everything' do setting.domain_blacklist_raw = "example.com; *.example.com\n test.com\sblock.com yes.com" expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com', 'test.com', 'block.com', 'yes.com') end - it 'set multiple domain with file' do + it 'sets multiple domain with file' do setting.domain_blacklist_file = File.open(Rails.root.join('spec/fixtures/', 'domain_blacklist.txt')) expect(setting.domain_blacklist).to contain_exactly('example.com', 'test.com', 'foo.bar') end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 78e95c8fac5..cee20234e1f 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -33,6 +33,22 @@ describe Blob do end end + describe '#video?' do + it 'is falsey with image extension' do + git_blob = Gitlab::Git::Blob.new(name: 'image.png') + + expect(described_class.decorate(git_blob)).not_to be_video + end + + UploaderHelper::VIDEO_EXT.each do |ext| + it "is truthy when extension is .#{ext}" do + git_blob = Gitlab::Git::Blob.new(name: "video.#{ext}") + + expect(described_class.decorate(git_blob)).to be_video + end + end + end + describe '#to_partial_path' do def stubbed_blob(overrides = {}) overrides.reverse_merge!( @@ -78,4 +94,26 @@ describe Blob do expect(blob.to_partial_path).to eq 'download' end end + + describe '#size_within_svg_limits?' do + let(:blob) { described_class.decorate(double(:blob)) } + + it 'returns true when the blob size is smaller than the SVG limit' do + expect(blob).to receive(:size).and_return(42) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns true when the blob size is equal to the SVG limit' do + expect(blob).to receive(:size).and_return(Blob::MAXIMUM_SVG_SIZE) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns false when the blob size is larger than the SVG limit' do + expect(blob).to receive(:size).and_return(1.terabyte) + + expect(blob.size_within_svg_limits?).to eq(false) + end + end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 6ad8bfef4f2..72688137f08 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -23,19 +23,19 @@ describe BroadcastMessage, models: true do end describe '.current' do - it "should return last message if time match" do + it "returns last message if time match" do message = create(:broadcast_message) expect(BroadcastMessage.current).to eq message end - it "should return nil if time not come" do + it "returns nil if time not come" do create(:broadcast_message, :future) expect(BroadcastMessage.current).to be_nil end - it "should return nil if time has passed" do + it "returns nil if time has passed" do create(:broadcast_message, :expired) expect(BroadcastMessage.current).to be_nil diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 978ad9c52d5..5980f6ddc32 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -32,7 +32,7 @@ describe Ci::Build, models: true do end let(:create_from_build) { Ci::Build.create_from build } - it 'there should be a pending task' do + it 'exists a pending task' do expect(Ci::Build.pending.count(:all)).to eq 0 create_from_build expect(Ci::Build.pending.count(:all)).to be > 0 @@ -259,7 +259,7 @@ describe Ci::Build, models: true do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } let(:user_trigger_variable) do - { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } + { key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1', public: false } end let(:predefined_trigger_variable) do { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } @@ -573,19 +573,19 @@ describe Ci::Build, models: true do let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, stage: 'test') } let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, stage: 'deploy') } - it 'to have no dependents if this is first build' do + it 'expects to have no dependents if this is first build' do expect(build.depends_on_builds).to be_empty end - it 'to have one dependent if this is test' do + it 'expects to have one dependent if this is test' do expect(rspec_test.depends_on_builds.map(&:id)).to contain_exactly(build.id) end - it 'to have all builds from build and test stage if this is last' do + it 'expects to have all builds from build and test stage if this is last' do expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, rspec_test.id, rubocop_test.id) end - it 'to have retried builds instead the original ones' do + it 'expects to have retried builds instead the original ones' do retried_rspec = Ci::Build.retry(rspec_test) expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, retried_rspec.id, rubocop_test.id) end @@ -655,23 +655,23 @@ describe Ci::Build, models: true do describe 'build erasable' do shared_examples 'erasable' do - it 'should remove artifact file' do + it 'removes artifact file' do expect(build.artifacts_file.exists?).to be_falsy end - it 'should remove artifact metadata file' do + it 'removes artifact metadata file' do expect(build.artifacts_metadata.exists?).to be_falsy end - it 'should erase build trace in trace file' do + it 'erases build trace in trace file' do expect(build.trace).to be_empty end - it 'should set erased to true' do + it 'sets erased to true' do expect(build.erased?).to be true end - it 'should set erase date' do + it 'sets erase date' do expect(build.erased_at).not_to be_falsy end end @@ -704,7 +704,7 @@ describe Ci::Build, models: true do include_examples 'erasable' - it 'should record user who erased a build' do + it 'records user who erased a build' do expect(build.erased_by).to eq user end end @@ -714,7 +714,7 @@ describe Ci::Build, models: true do include_examples 'erasable' - it 'should not set user who erased a build' do + it 'does not set user who erased a build' do expect(build.erased_by).to be_nil end end @@ -750,7 +750,7 @@ describe Ci::Build, models: true do end describe '#erase' do - it 'should not raise error' do + it 'does not raise error' do expect { build.erase }.not_to raise_error end end @@ -764,6 +764,53 @@ describe Ci::Build, models: true do end end + describe '#when' do + subject { build.when } + + context 'if is undefined' do + before do + build.when = nil + end + + context 'use from gitlab-ci.yml' do + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'if config is not found' do + let(:config) { nil } + + it { is_expected.to eq('on_success') } + end + + context 'if config does not have a questioned job' do + let(:config) do + YAML.dump({ + test_other: { + script: 'Hello World' + } + }) + end + + it { is_expected.to eq('on_success') } + end + + context 'if config has when' do + let(:config) do + YAML.dump({ + test: { + script: 'Hello World', + when: 'always' + } + }) + end + + it { is_expected.to eq('always') } + end + end + end + end + describe '#retryable?' do context 'when build is running' do before do @@ -839,8 +886,10 @@ describe Ci::Build, models: true do is_expected.to eq(build) end - context 'for success build' do - before { build.queue } + context 'for successful build' do + before do + build.update(status: 'success') + end it 'creates a new build' do is_expected.to be_pending @@ -900,7 +949,7 @@ describe Ci::Build, models: true do context 'when build is running' do before { build.run! } - it 'should return false' do + it 'returns false' do expect(build.retryable?).to be false end end @@ -908,7 +957,7 @@ describe Ci::Build, models: true do context 'when build is finished' do before { build.success! } - it 'should return true' do + it 'returns true' do expect(build.retryable?).to be true end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0d4c86955ce..950833cb219 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ci::Pipeline, models: true do let(:project) { FactoryGirl.create :empty_project } - let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } + let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } @@ -38,9 +38,6 @@ describe Ci::Pipeline, models: true do it { expect(pipeline.sha).to start_with(subject) } end - describe '#create_next_builds' do - end - describe '#retried' do subject { pipeline.retried } @@ -54,312 +51,9 @@ describe Ci::Pipeline, models: true do end end - describe '#create_builds' do - let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project, ref: 'master', tag: false } - - def create_builds(trigger_request = nil) - pipeline.create_builds(nil, trigger_request) - end - - def create_next_builds - pipeline.create_next_builds(pipeline.builds.order(:id).last) - end - - it 'creates builds' do - expect(create_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(2) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(4) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(5) - - expect(create_next_builds).to be_falsey - end - - context 'custom stage with first job allowed to fail' do - let(:yaml) do - { - stages: ['clean', 'test'], - clean_job: { - stage: 'clean', - allow_failure: true, - script: 'BUILD', - }, - test_job: { - stage: 'test', - script: 'TEST', - }, - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - create_builds - end - - it 'properly schedules builds' do - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:drop) - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending', 'failed') - end - end - - context 'properly creates builds when "when" is defined' do - let(:yaml) do - { - stages: ["build", "test", "test_failure", "deploy", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - test_failure: { - stage: "test_failure", - script: "ON test failure", - when: "on_failure", - }, - deploy: { - stage: "deploy", - script: "PUBLISH", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - } - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - end - - context 'when builds are successful' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('success') - end - end - - context 'when test job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when test and test_failure jobs fail' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when deploy job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when build is canceled in the second stage' do - it 'does not schedule builds after build has been canceled' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.running_or_pending).not_to be_empty - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:cancel) - - expect(pipeline.builds.running_or_pending).to be_empty - expect(pipeline.reload.status).to eq('canceled') - end - end - - context 'when listing manual actions' do - let(:yaml) do - { - stages: ["build", "test", "staging", "production", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - staging: { - stage: "staging", - script: "PUBLISH", - }, - production: { - stage: "production", - script: "PUBLISH", - when: "manual", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - }, - clear_cache: { - stage: "cleanup", - script: "CLEAR CACHE", - when: "manual", - } - } - end - - it 'returns only for skipped builds' do - # currently all builds are created - expect(create_builds).to be_truthy - expect(manual_actions).to be_empty - - # succeed stage build - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage test - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage staging and skip stage production - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_many # production and clear cache - - # succeed stage cleanup - pipeline.builds.running_or_pending.each(&:success) - - # after processing a pipeline we should have 6 builds, 5 succeeded - expect(pipeline.builds.count).to eq(6) - expect(pipeline.builds.success.count).to eq(4) - end - - def manual_actions - pipeline.manual_actions - end - end - end - - context 'when no builds created' do - let(:pipeline) { build(:ci_pipeline) } - - before do - stub_ci_pipeline_yaml_file(YAML.dump(before_script: ['ls'])) - end - - it 'returns false' do - expect(pipeline.create_builds(nil)).to be_falsey - expect(pipeline).not_to be_persisted - end - end - end - - describe "#finished_at" do - let(:pipeline) { FactoryGirl.create :ci_pipeline } - - it "returns finished_at of latest build" do - build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60 - FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120 - - expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i) - end - - it "returns nil if there is no finished build" do - FactoryGirl.create :ci_not_started_build, pipeline: pipeline - - expect(pipeline.finished_at).to be_nil - end - end - describe "coverage" do let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" } - let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } + let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } it "calculates average when there are two builds with coverage" do FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline @@ -426,33 +120,47 @@ describe Ci::Pipeline, models: true do end end - describe '#update_state' do - it 'execute update_state after touching object' do - expect(pipeline).to receive(:update_state).and_return(true) - pipeline.touch + describe 'state machine' do + let(:current) { Time.now.change(usec: 0) } + let(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current } + let(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current } + + describe '#duration' do + before do + build.skip + build2.skip + end + + it 'matches sum of builds duration' do + expect(pipeline.reload.duration).to eq(build.duration + build2.duration) + end end - context 'dependent objects' do - let(:commit_status) { build :commit_status, pipeline: pipeline } + describe '#started_at' do + it 'updates on transitioning to running' do + build.run + + expect(pipeline.reload.started_at).not_to be_nil + end + + it 'does not update on transitioning to success' do + build.success - it 'execute update_state after saving dependent object' do - expect(pipeline).to receive(:update_state).and_return(true) - commit_status.save + expect(pipeline.reload.started_at).to be_nil end end - context 'update state' do - let(:current) { Time.now.change(usec: 0) } - let(:build) { FactoryGirl.create :ci_build, :success, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } + describe '#finished_at' do + it 'updates on transitioning to success' do + build.success - before do - build + expect(pipeline.reload.finished_at).not_to be_nil end - [:status, :started_at, :finished_at, :duration].each do |param| - it "update #{param}" do - expect(pipeline.send(param)).to eq(build.send(param)) - end + it 'does not update on transitioning to running' do + build.run + + expect(pipeline.reload.finished_at).to be_nil end end end @@ -513,7 +221,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop' end - + it 'returns true' do is_expected.to be_truthy end @@ -524,7 +232,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop' end - + it 'returns false' do is_expected.to be_falsey end @@ -542,4 +250,64 @@ describe Ci::Pipeline, models: true do end end end + + describe '#status' do + let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } + + subject { pipeline.reload.status } + + context 'on queuing' do + before do + build.enqueue + end + + it { is_expected.to eq('pending') } + end + + context 'on run' do + before do + build.enqueue + build.run + end + + it { is_expected.to eq('running') } + end + + context 'on drop' do + before do + build.drop + end + + it { is_expected.to eq('failed') } + end + + context 'on success' do + before do + build.success + end + + it { is_expected.to eq('success') } + end + + context 'on cancel' do + before do + build.cancel + end + + it { is_expected.to eq('canceled') } + end + + context 'on failure and build retry' do + before do + build.drop + Ci::Build.retry(build) + end + + # We are changing a state: created > failed > running + # Instead of: created > failed > pending + # Since the pipeline already run, so it should not be pending anymore + + it { is_expected.to eq('running') } + end + end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 474b0b1621d..3ca9231f58e 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -4,12 +4,12 @@ describe Ci::Trigger, models: true do let(:project) { FactoryGirl.create :empty_project } describe 'before_validation' do - it 'should set an random token if none provided' do + it 'sets an random token if none provided' do trigger = FactoryGirl.create :ci_trigger_without_token, project: project expect(trigger.token).not_to be_nil end - it 'should not set an random token if one provided' do + it 'does not set an random token if one provided' do trigger = FactoryGirl.create :ci_trigger, project: project expect(trigger.token).to eq('token') end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ba02d5fe977..d3e6a6648cc 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -13,6 +13,26 @@ describe Commit, models: true do it { is_expected.to include_module(StaticModel) } end + describe '#author' do + it 'looks up the author in a case-insensitive way' do + user = create(:user, email: commit.author_email.upcase) + expect(commit.author).to eq(user) + end + + it 'caches the author' do + user = create(:user, email: commit.author_email) + expect(RequestStore).to receive(:active?).twice.and_return(true) + expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original + + expect(commit.author).to eq(user) + key = "commit_author:#{commit.author_email}" + expect(RequestStore.store[key]).to eq(user) + + expect(commit.author).to eq(user) + RequestStore.store.clear + end + end + describe '#to_reference' do it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id @@ -66,6 +86,27 @@ eos end end + describe '#full_title' do + it "returns no_commit_message when safe_message is blank" do + allow(commit).to receive(:safe_message).and_return('') + expect(commit.full_title).to eq("--no commit message") + end + + it "returns entire message if there is no newline" do + message = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit. Vivamus egestas lacinia lacus, sed rutrum mauris.' + + allow(commit).to receive(:safe_message).and_return(message) + expect(commit.full_title).to eq(message) + end + + it "returns first line of message if there is a newLine" do + message = commit.safe_message.split(" ").first + + allow(commit).to receive(:safe_message).and_return(message + "\n" + message) + expect(commit.full_title).to eq(message) + end + end + describe "delegation" do subject { commit } @@ -212,6 +253,7 @@ eos it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) expect(commit.uri_type('files/js/application.js')).to be(:blob) end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index ff6371ad685..fcfa3138ce5 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -133,7 +133,7 @@ describe CommitStatus, models: true do @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'success' end - it 'return unique statuses' do + it 'returns unique statuses' do is_expected.to eq([@commit4, @commit5]) end end @@ -149,7 +149,7 @@ describe CommitStatus, models: true do @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'ee', ref: nil, status: 'canceled' end - it 'return statuses that are running or pending' do + it 'returns statuses that are running or pending' do is_expected.to eq([@commit1, @commit2]) end end @@ -160,7 +160,7 @@ describe CommitStatus, models: true do context 'when no before_sha is set for pipeline' do before { pipeline.before_sha = nil } - it 'return blank sha' do + it 'returns blank sha' do is_expected.to eq(Gitlab::Git::BLANK_SHA) end end @@ -169,7 +169,7 @@ describe CommitStatus, models: true do let(:value) { '1234' } before { pipeline.before_sha = value } - it 'return the set value' do + it 'returns the set value' do is_expected.to eq(value) end end @@ -186,7 +186,7 @@ describe CommitStatus, models: true do context 'stages list' do subject { CommitStatus.where(pipeline: pipeline).stages } - it 'return ordered list of stages' do + it 'returns ordered list of stages' do is_expected.to eq(%w(build test deploy)) end end @@ -194,7 +194,7 @@ describe CommitStatus, models: true do context 'stages with statuses' do subject { CommitStatus.where(pipeline: pipeline).latest.stages_status } - it 'return list of stages with statuses' do + it 'returns list of stages with statuses' do is_expected.to eq({ 'build' => 'failed', 'test' => 'success', diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb new file mode 100644 index 00000000000..49ab3c4b6e9 --- /dev/null +++ b/spec/models/compare_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Compare, models: true do + include RepoHelpers + + let(:project) { create(:project, :public) } + let(:commit) { project.commit } + + let(:start_commit) { sample_image_commit } + let(:head_commit) { sample_commit } + + let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, start_commit.id, head_commit.id) } + + subject { described_class.new(raw_compare, project) } + + describe '#start_commit' do + it 'returns raw compare base commit' do + expect(subject.start_commit.id).to eq(start_commit.id) + end + + it 'returns nil if compare base commit is nil' do + expect(raw_compare).to receive(:base).and_return(nil) + + expect(subject.start_commit).to eq(nil) + end + end + + describe '#commit' do + it 'returns raw compare head commit' do + expect(subject.commit.id).to eq(head_commit.id) + end + + it 'returns nil if compare head commit is nil' do + expect(raw_compare).to receive(:head).and_return(nil) + + expect(subject.commit).to eq(nil) + end + end + + describe '#base_commit' do + let(:base_commit) { Commit.new(another_sample_commit, project) } + + it 'returns project merge base commit' do + expect(project).to receive(:merge_base_commit).with(start_commit.id, head_commit.id).and_return(base_commit) + + expect(subject.base_commit).to eq(base_commit) + end + + it 'returns nil if there is no start_commit' do + expect(subject).to receive(:start_commit).and_return(nil) + + expect(subject.base_commit).to eq(nil) + end + + it 'returns nil if there is no head commit' do + expect(subject).to receive(:head_commit).and_return(nil) + + expect(subject.base_commit).to eq(nil) + end + end + + describe '#diff_refs' do + it 'uses base_commit sha as base_sha' do + expect(subject).to receive(:base_commit).at_least(:once).and_call_original + + expect(subject.diff_refs.base_sha).to eq(subject.base_commit.id) + end + + it 'uses start_commit sha as start_sha' do + expect(subject.diff_refs.start_sha).to eq(start_commit.id) + end + + it 'uses commit sha as head sha' do + expect(subject.diff_refs.head_sha).to eq(head_commit.id) + end + end +end diff --git a/spec/models/concerns/faster_cache_keys_spec.rb b/spec/models/concerns/faster_cache_keys_spec.rb new file mode 100644 index 00000000000..8d3f94267fa --- /dev/null +++ b/spec/models/concerns/faster_cache_keys_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe FasterCacheKeys do + describe '#cache_key' do + it 'returns a String' do + # We're using a fixed string here so it's easier to set an expectation for + # the resulting cache key. + time = '2016-08-08 16:39:00+02' + issue = build(:issue, updated_at: time) + issue.extend(described_class) + + expect(issue).to receive(:id).and_return(1) + + expect(issue.cache_key).to eq("issues/1-#{time}") + end + end +end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 5e652660e2c..549b0042038 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -68,7 +68,7 @@ describe Issue, "Mentionable" do describe '#create_cross_references!' do let(:project) { create(:project) } - let(:author) { double('author') } + let(:author) { build(:user) } let(:commit) { project.commit } let(:commit2) { project.commit } @@ -88,6 +88,10 @@ describe Issue, "Mentionable" do let(:author) { create(:author) } let(:issues) { create_list(:issue, 2, project: project, author: author) } + before do + project.team << [author, Gitlab::Access::DEVELOPER] + end + context 'before changes are persisted' do it 'ignores pre-existing references' do issue = create_issue(description: issues[0].to_reference) diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 7e9ab8940cf..b7e973798a3 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -26,53 +26,53 @@ describe Milestone, 'Milestoneish' do end describe '#closed_items_count' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.closed_items_count(non_member)).to eq 2 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.closed_items_count(guest)).to eq 2 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.closed_items_count(author)).to eq 4 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.closed_items_count(assignee)).to eq 4 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.closed_items_count(member)).to eq 6 end - it 'should count all issues for admin' do + it 'counts all issues for admin' do expect(milestone.closed_items_count(admin)).to eq 6 end end describe '#total_items_count' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.total_items_count(non_member)).to eq 4 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.total_items_count(guest)).to eq 4 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.total_items_count(author)).to eq 7 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.total_items_count(assignee)).to eq 7 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.total_items_count(member)).to eq 10 end - it 'should count all issues for admin' do + it 'counts all issues for admin' do expect(milestone.total_items_count(admin)).to eq 10 end end @@ -91,27 +91,27 @@ describe Milestone, 'Milestoneish' do end describe '#percent_complete' do - it 'should not count confidential issues for non project members' do + it 'does not count confidential issues for non project members' do expect(milestone.percent_complete(non_member)).to eq 50 end - it 'should not count confidential issues for project members with guest role' do + it 'does not count confidential issues for project members with guest role' do expect(milestone.percent_complete(guest)).to eq 50 end - it 'should count confidential issues for author' do + it 'counts confidential issues for author' do expect(milestone.percent_complete(author)).to eq 57 end - it 'should count confidential issues for assignee' do + it 'counts confidential issues for assignee' do expect(milestone.percent_complete(assignee)).to eq 57 end - it 'should count confidential issues for project members' do + it 'counts confidential issues for project members' do expect(milestone.percent_complete(member)).to eq 60 end - it 'should count confidential issues for admin' do + it 'counts confidential issues for admin' do expect(milestone.percent_complete(admin)).to eq 60 end end diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb new file mode 100644 index 00000000000..32935bc0b09 --- /dev/null +++ b/spec/models/concerns/spammable_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Issue, 'Spammable' do + let(:issue) { create(:issue, description: 'Test Desc.') } + + describe 'Associations' do + it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) } + end + + describe 'ClassMethods' do + it 'should return correct attr_spammable' do + expect(issue.spammable_text).to eq("#{issue.title}\n#{issue.description}") + end + end + + describe 'InstanceMethods' do + it 'should be invalid if spam' do + issue = build(:issue, spam: true) + expect(issue.valid?).to be_falsey + end + + describe '#check_for_spam?' do + it 'returns true for public project' do + issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + expect(issue.check_for_spam?).to eq(true) + end + + it 'returns false for other visibility levels' do + expect(issue.check_for_spam?).to eq(false) + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 9e8ebc56a31..eb64f3d0c83 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -41,7 +41,7 @@ describe ApplicationSetting, 'TokenAuthenticatable' do describe 'ensured! token' do subject { described_class.new.send("ensure_#{token_field}!") } - it 'should persist new token' do + it 'persists new token' do expect(subject).to eq described_class.current[token_field] end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 7df3df4bb9e..bfff639ad78 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -15,4 +15,28 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + + describe '#includes_commit?' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + let(:deployment) do + create(:deployment, environment: environment, sha: project.commit.id) + end + + context 'when there is no project commit' do + it 'returns false' do + commit = project.commit('feature') + + expect(deployment.includes_commit?(commit)).to be false + end + end + + context 'when they share the same tree branch' do + it 'returns true' do + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be true + end + end + end end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index af8e890ca95..1fa96eb1f15 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -119,7 +119,7 @@ describe DiffNote, models: true do context "when the merge request's diff refs don't match that of the diff note" do before do - allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs) + allow(subject.noteable).to receive(:diff_sha_refs).and_return(commit.diff_refs) end it "returns false" do @@ -168,7 +168,7 @@ describe DiffNote, models: true do context "when the note is outdated" do before do - allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs) + allow(merge_request).to receive(:diff_sha_refs).and_return(commit.diff_refs) end it "uses the DiffPositionUpdateService" do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 7629af6a570..c881897926e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -11,4 +11,56 @@ describe Environment, models: true do 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(:external_url).is_within(0..255) } + + # 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 + + describe '#nullify_external_url' do + it 'replaces a blank url with nil' do + env = build(:environment, external_url: "") + + expect(env.save).to be true + expect(env.external_url).to be_nil + end + end + + describe '#includes_commit?' do + context 'without a last deployment' do + it "returns false" do + expect(environment.includes_commit?('HEAD')).to be false + end + end + + context 'with a last deployment' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + context 'in the same branch' do + it 'returns true' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be true + end + end + + context 'not in the same branch' do + before do + deployment.update(sha: project.commit('feature').id) + end + + it 'returns false' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be false + end + end + end + end end diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index f94987dcaff..9c81d159cdf 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -9,11 +9,11 @@ describe ForkedProjectLink, "add link on fork" do @project_to = fork_project(project_from, user) end - it "project_to should know it is forked" do + it "project_to knows it is forked" do expect(@project_to.forked?).to be_truthy end - it "project should know who it is forked from" do + it "project knows who it is forked from" do expect(@project_to.forked_from_project).to eq(project_from) end end @@ -29,15 +29,15 @@ describe '#forked?' do forked_project_link.save! end - it "project_to should know it is forked" do + it "project_to knows it is forked" do expect(project_to.forked?).to be_truthy end - it "project_from should not be forked" do + it "project_from is not forked" do expect(project_from.forked?).to be_falsey end - it "project_to.destroy should destroy fork_link" do + it "project_to.destroy destroys fork_link" do expect(forked_project_link).to receive(:destroy) project_to.destroy end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index ae77ec5b348..92e0f7f27ce 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -29,15 +29,15 @@ describe GlobalMilestone, models: true do @global_milestones = GlobalMilestone.build_collection(milestones) end - it 'should have all project milestones' do + it 'has all project milestones' do expect(@global_milestones.count).to eq(2) end - it 'should have all project milestones titles' do + it 'has all project milestones titles' do expect(@global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'VD-123']) end - it 'should have all project milestones' do + it 'has all project milestones' do expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6) end end @@ -54,11 +54,11 @@ describe GlobalMilestone, models: true do @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones) end - it 'should have exactly one group milestone' do + it 'has exactly one group milestone' do expect(@global_milestone.title).to eq('Milestone v1.2') end - it 'should have all project milestones with the same title' do + it 'has all project milestones with the same title' do expect(@global_milestone.milestones.count).to eq(3) end end @@ -66,7 +66,7 @@ describe GlobalMilestone, models: true do describe '#safe_title' do let(:milestone) { create(:milestone, title: "git / test", project: project1) } - it 'should strip out slashes and spaces' do + it 'strips out slashes and spaces' do global_milestone = GlobalMilestone.new(milestone.title, [milestone]) expect(global_milestone.safe_title).to eq('git-test') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 266c46213a6..ea4b59c26b1 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -116,7 +116,7 @@ describe Group, models: true do let(:user) { create(:user) } before { group.add_users([user.id], GroupMember::GUEST) } - it "should update the group permission" do + it "updates the group permission" do expect(group.group_members.guests.map(&:user)).to include(user) group.add_users([user.id], GroupMember::DEVELOPER) expect(group.group_members.developers.map(&:user)).to include(user) @@ -128,12 +128,12 @@ describe Group, models: true do let(:user) { create(:user) } before { group.add_user(user, GroupMember::MASTER) } - it "should be true if avatar is image" do + it "is true if avatar is image" do group.update_attribute(:avatar, 'uploads/avatar.png') expect(group.avatar_type).to be_truthy end - it "should be false if avatar is html page" do + it "is false if avatar is html page" do group.update_attribute(:avatar, 'uploads/avatar.html') expect(group.avatar_type).to eq(["only images allowed"]) end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 983848392b7..4a457997a4f 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -24,7 +24,7 @@ describe ProjectHook, models: true do end describe '.push_hooks' do - it 'should return hooks for push events only' do + it 'returns hooks for push events only' do hook = create(:project_hook, push_events: true) create(:project_hook, push_events: false) expect(ProjectHook.push_hooks).to eq([hook]) @@ -32,7 +32,7 @@ describe ProjectHook, models: true do end describe '.tag_push_hooks' do - it 'should return hooks for tag push events only' do + it 'returns hooks for tag push events only' do hook = create(:project_hook, tag_push_events: true) create(:project_hook, tag_push_events: false) expect(ProjectHook.tag_push_hooks).to eq([hook]) diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 4078b9e4ff5..cbdf7eec082 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -38,7 +38,7 @@ describe SystemHook, models: true do end it "project_destroy hook" do - Projects::DestroyService.new(project, user, {}).pending_delete! + Projects::DestroyService.new(project, user, {}).async_execute expect(WebMock).to have_requested(:post, system_hook.url).with( body: /project_destroy/, diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6a897c96690..3259f795296 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -306,4 +306,257 @@ describe Issue, models: true do expect(user2.assigned_open_issues_count).to eq(1) end end + + describe '#visible_to_user?' do + context 'with a user' do + let(:user) { build(:user) } + let(:issue) { build(:issue) } + + it 'returns true when the issue is readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(issue.visible_to_user?(user)).to eq(true) + end + + it 'returns false when the issue is not readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(issue.visible_to_user?(user)).to eq(false) + end + end + + 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 + end + + describe '#readable_by?' do + describe 'with a regular user that is not a team member' do + let(:user) { create(:user) } + + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, project: project, confidential: true) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + context 'using an internal user' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) + end + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + + context 'when the user is the project owner' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + end + + context 'with a regular user that is a team member' do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + context 'using a public project' do + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + context 'with an admin user' do + let(:project) { create(:empty_project) } + let(:user) { create(:user, admin: true) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + describe '#publicly_visible?' do + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 49cf3d8633a..fd4a2beff58 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -16,12 +16,13 @@ describe Key, models: true do end describe "Methods" do + let(:user) { create(:user) } it { is_expected.to respond_to :projects } it { is_expected.to respond_to :publishable_key } describe "#publishable_keys" do - it 'strips all personal information' do - expect(build(:key).publishable_key).not_to match(/dummy@gitlab/) + it 'replaces SSH key comment with simple identifier of username + hostname' do + expect(build(:key, user: user).publishable_key).to include("#{user.name} (localhost)") end end end @@ -72,13 +73,13 @@ describe Key, models: true do end context 'callbacks' do - it 'should add new key to authorized_file' 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 end - it 'should remove key from authorized_file' do + 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 diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index f37f44a608e..2a09063f857 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -18,7 +18,7 @@ describe Label, models: true do describe 'validation' do it { is_expected.to validate_presence_of(:project) } - it 'should validate color code' do + it 'validates color code' do expect(label).not_to allow_value('G-ITLAB').for(:color) expect(label).not_to allow_value('AABBCC').for(:color) expect(label).not_to allow_value('#AABBCCEE').for(:color) @@ -30,7 +30,7 @@ describe Label, models: true do expect(label).to allow_value('#abcdef').for(:color) end - it 'should validate title' do + it 'validates title' do expect(label).not_to allow_value('G,ITLAB').for(:title) expect(label).not_to allow_value('').for(:title) diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index d23fc06c3ad..2cfd26419ca 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -5,12 +5,12 @@ describe LegacyDiffNote, models: true do let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } - it "should save a valid note" do + it "saves a valid note" do expect(note.commit_id).to eq(commit.id) expect(note.noteable.id).to eq(commit.id) end - it "should be recognized by #legacy_diff_note?" do + it "is recognized by #legacy_diff_note?" do expect(note).to be_legacy_diff_note end end @@ -58,7 +58,7 @@ describe LegacyDiffNote, models: true do # Generate a real line_code value so we know it will match. We use a # random line from a random diff just for funsies. - diff = merge.diffs.to_a.sample + diff = merge.raw_diffs.to_a.sample line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 40181a8b906..2277f4e13bf 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -10,7 +10,7 @@ describe Member, models: true do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:source) } - it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) } it_behaves_like 'an object with email-formated attributes', :invite_email do subject { build(:project_member) } @@ -79,6 +79,18 @@ describe Member, models: true do @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } end + describe '.access_for_user_ids' do + it 'returns the right access levels' do + users = [@owner_user.id, @master_user.id] + expected = { + @owner_user.id => Gitlab::Access::OWNER, + @master_user.id => Gitlab::Access::MASTER + } + + expect(described_class.access_for_user_ids(users)).to eq(expected) + end + end + describe '.invite' do it { expect(described_class.invite).not_to include @master } it { expect(described_class.invite).to include @invited_member } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 18439cac2a4..4f875fd257a 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -22,7 +22,7 @@ require 'spec_helper' describe GroupMember, models: true do describe 'notifications' do describe "#after_create" do - it "should send email to user" do + it "sends email to user" do membership = build(:group_member) allow(membership).to receive(:notification_service). @@ -40,7 +40,7 @@ describe GroupMember, models: true do and_return(double('NotificationService').as_null_object) end - it "should send email to user" do + it "sends email to user" do expect(@group_member).to receive(:notification_service) @group_member.update_attribute(:access_level, GroupMember::MASTER) end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index ba622dfb9be..913d74645a7 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -27,6 +27,7 @@ describe ProjectMember, models: true do describe 'validations' do it { is_expected.to allow_value('Project').for(:source_type) } it { is_expected.not_to allow_value('project').for(:source_type) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end describe 'modules' do @@ -40,7 +41,7 @@ describe ProjectMember, models: true do end describe "#destroy" do - let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) } + let(:owner) { create(:project_member, access_level: ProjectMember::MASTER) } let(:project) { owner.project } let(:master) { create(:project_member, project: project) } @@ -52,7 +53,7 @@ describe ProjectMember, models: true do master_todos end - it "destroy itself and delete associated todos" do + it "destroys itself and delete associated todos" do expect(owner.user.todos.size).to eq(2) expect(master.user.todos.size).to eq(3) expect(Todo.count).to eq(5) @@ -101,7 +102,7 @@ describe ProjectMember, models: true do end end - describe '.add_users_into_projects' do + describe '.add_users_to_projects' do before do @project_1 = create :project @project_2 = create :project @@ -109,7 +110,7 @@ describe ProjectMember, models: true do @user_1 = create :user @user_2 = create :user - ProjectMember.add_users_into_projects( + ProjectMember.add_users_to_projects( [@project_1.id, @project_2.id], [@user_1.id, @user_2.id], ProjectMember::MASTER diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 9a637c94fbe..29f7396f862 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -10,7 +10,7 @@ describe MergeRequestDiff, models: true do expect(mr_diff).not_to receive(:load_diffs) expect(Gitlab::Git::Compare).to receive(:new).and_call_original - mr_diff.diffs(ignore_whitespace_change: true) + mr_diff.raw_diffs(ignore_whitespace_change: true) end end @@ -18,19 +18,19 @@ describe MergeRequestDiff, models: true do before { mr_diff.update_attributes(st_diffs: '') } it 'returns an empty DiffCollection' do - expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) - expect(mr_diff.diffs).to be_empty + expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.raw_diffs).to be_empty end end context 'when the raw diffs exist' do it 'returns the diffs' do - expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection) - expect(mr_diff.diffs).not_to be_empty + expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(mr_diff.raw_diffs).not_to be_empty end context 'when the :paths option is set' do - let(:diffs) { mr_diff.diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } + let(:diffs) { mr_diff.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } it 'only returns diffs that match the (old path, new path) given' do expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 43a87e07435..acb75ec21a9 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -65,11 +65,11 @@ describe MergeRequest, models: true do end describe '#target_branch_sha' do - context 'when the target branch does not exist anymore' do - let(:project) { create(:project) } + let(:project) { create(:project) } - subject { create(:merge_request, source_project: project, target_project: project) } + subject { create(:merge_request, source_project: project, target_project: project) } + context 'when the target branch does not exist' do before do project.repository.raw_repository.delete_branch(subject.target_branch) end @@ -78,6 +78,12 @@ describe MergeRequest, models: true do expect(subject.target_branch_sha).to be_nil end end + + it 'returns memoized value' do + subject.target_branch_sha = '8ffb3c15a5475e59ae909384297fede4badcb4c7' + + expect(subject.target_branch_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7' + end end describe '#source_branch_sha' do @@ -103,6 +109,12 @@ describe MergeRequest, models: true do expect(subject.source_branch_sha).to be_nil end end + + it 'returns memoized value' do + subject.source_branch_sha = '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + + expect(subject.source_branch_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + end end describe '#to_reference' do @@ -116,6 +128,31 @@ describe MergeRequest, models: true do end end + describe '#raw_diffs' do + let(:merge_request) { build(:merge_request) } + let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } + + context 'when there are MR diffs' do + it 'delegates to the MR diffs' do + merge_request.merge_request_diff = MergeRequestDiff.new + + expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(options) + + merge_request.raw_diffs(options) + end + end + + context 'when there are no MR diffs' do + it 'delegates to the compare object' do + merge_request.compare = double(:compare) + + expect(merge_request.compare).to receive(:raw_diffs).with(options) + + merge_request.raw_diffs(options) + end + end + end + describe '#diffs' do let(:merge_request) { build(:merge_request) } let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } } @@ -124,7 +161,7 @@ describe MergeRequest, models: true do it 'delegates to the MR diffs' do merge_request.merge_request_diff = MergeRequestDiff.new - expect(merge_request.merge_request_diff).to receive(:diffs).with(options) + expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)) merge_request.diffs(options) end @@ -151,12 +188,12 @@ describe MergeRequest, models: true do create(:note, noteable: merge_request, project: merge_request.project) end - it "should include notes for commits" 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) end - it "should include notes for commits from target project as well" do + it "includes notes for commits from target project as well" do create(:note_on_commit, commit_id: merge_request.commits.first.id, project: merge_request.target_project) @@ -267,7 +304,7 @@ describe MergeRequest, models: true do expect(subject.can_remove_source_branch?(user)).to be_falsey end - it "cant remove a root ref" do + it "can't remove a root ref" do subject.source_branch = "master" subject.target_branch = "feature" @@ -664,6 +701,21 @@ describe MergeRequest, models: true do end end + describe "#environments" do + let(:project) { create(:project) } + let!(:environment) { create(:environment, project: project) } + let!(:environment1) { create(:environment, project: project) } + let!(:environment2) { create(:environment, project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'selects deployed environments' do + create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, environment: environment1, sha: project.commit('feature').id) + + expect(merge_request.environments).to eq [environment] + end + end + describe "#reload_diff" do let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) } @@ -675,6 +727,12 @@ describe MergeRequest, models: true do subject.reload_diff end + it "executs diff cache service" do + expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject) + + subject.reload_diff + end + it "updates diff note positions" do old_diff_refs = subject.diff_refs @@ -701,4 +759,28 @@ describe MergeRequest, models: true do subject.reload_diff end end + + describe "#diff_sha_refs" do + context "with diffs" do + subject { create(:merge_request, :with_diffs) } + + it "does not touch the repository" do + subject # Instantiate the object + + expect_any_instance_of(Repository).not_to receive(:commit) + + subject.diff_sha_refs + end + + it "returns expected diff_refs" do + expected_diff_refs = Gitlab::Diff::DiffRefs.new( + base_sha: subject.merge_request_diff.base_commit_sha, + start_sha: subject.merge_request_diff.start_commit_sha, + head_sha: subject.merge_request_diff.head_commit_sha + ) + + expect(subject.diff_sha_refs).to eq(expected_diff_refs) + end + end + end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index d661dc0e59a..d64d6cde2b5 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -28,12 +28,12 @@ describe Milestone, models: true do end describe "unique milestone title per project" do - it "shouldn't accept the same title in a project twice" do + it "does not accept the same title in a project twice" do new_milestone = Milestone.new(project: milestone.project, title: milestone.title) expect(new_milestone).not_to be_valid end - it "should accept the same title in another project" do + it "accepts the same title in another project" do project = build(:project) new_milestone = Milestone.new(project: project, title: milestone.title) @@ -42,29 +42,29 @@ describe Milestone, models: true do end describe "#percent_complete" do - it "should not count open issues" do + it "does not count open issues" do milestone.issues << issue expect(milestone.percent_complete(user)).to eq(0) end - it "should count closed issues" do + it "counts closed issues" do issue.close milestone.issues << issue expect(milestone.percent_complete(user)).to eq(100) end - it "should recover from dividing by zero" do + it "recovers from dividing by zero" do expect(milestone.percent_complete(user)).to eq(0) end end describe "#expires_at" do - it "should be nil when due_date is unset" 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 "should not be nil when due_date is set" do + 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 @@ -121,7 +121,7 @@ describe Milestone, models: true do create :merge_request, milestone: milestone end - it 'Should return total count of issues and merge requests assigned to milestone' do + it 'returns total count of issues and merge requests assigned to milestone' do expect(milestone.total_items_count(user)).to eq 2 end end @@ -134,11 +134,11 @@ describe Milestone, models: true do create :issue end - it 'should be true if milestone active and all nested issues closed' do + it 'returns true if milestone active and all nested issues closed' do expect(milestone.can_be_closed?).to be_truthy end - it 'should be false if milestone active and not all nested issues closed' do + it 'returns false if milestone active and not all nested issues closed' do issue.milestone = milestone issue.save diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index a162da0208e..544920d1824 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -61,11 +61,11 @@ describe Namespace, models: true do allow(@namespace).to receive(:path_changed?).and_return(true) end - it "should raise error when directory exists" do + it "raises error when directory exists" do expect { @namespace.move_dir }.to raise_error("namespace directory cannot be moved") end - it "should move dir if path changed" do + it "moves dir if path changed" do new_path = @namespace.path + "_new" allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return(new_path) @@ -93,7 +93,7 @@ describe Namespace, models: true do before { namespace.destroy } - it "should remove its dirs when deleted" do + it "removes its dirs when deleted" do expect(File.exist?(path)).to be(false) end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1243f5420a7..53733d253f7 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -56,18 +56,18 @@ describe Note, models: true do let!(:note) { create(:note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } - it "should be accessible through #noteable" do + it "is accessible through #noteable" do expect(note.commit_id).to eq(commit.id) expect(note.noteable).to be_a(Commit) expect(note.noteable).to eq(commit) end - it "should save a valid note" do + it "saves a valid note" do expect(note.commit_id).to eq(commit.id) note.noteable == commit end - it "should be recognized by #for_commit?" do + it "is recognized by #for_commit?" do expect(note).to be_for_commit end diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index 2142c7c13ef..36379074ea0 100644 --- a/spec/models/project_security_spec.rb +++ b/spec/models/project_security_spec.rb @@ -21,7 +21,7 @@ describe Project, models: true do let(:owner_actions) { Ability.project_owner_rules } describe "Non member rules" do - it "should deny for non-project users any actions" do + it "denies for non-project users any actions" do owner_actions.each do |action| expect(@abilities.allowed?(@u1, action, @p1)).to be_falsey end @@ -33,7 +33,7 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::GUEST) end - it "should allow for project user any guest actions" do + it "allows for project user any guest actions" do guest_actions.each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy end @@ -45,7 +45,7 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) end - it "should allow for project user any report actions" do + it "allows for project user any report actions" do report_actions.each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy end @@ -58,13 +58,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::DEVELOPER) end - it "should deny for developer master-specific actions" do + it "denies for developer master-specific actions" do [dev_actions - report_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project user any dev actions" do + it "allows for project user any dev actions" do dev_actions.each do |action| expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy end @@ -77,13 +77,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end - it "should deny for developer master-specific actions" do + it "denies for developer master-specific actions" do [master_actions - dev_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project user any master actions" do + it "allows for project user any master actions" do master_actions.each do |action| expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy end @@ -96,13 +96,13 @@ describe Project, models: true do @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) end - it "should deny for masters admin-specific actions" do + it "denies for masters admin-specific actions" do [owner_actions - master_actions].each do |action| expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey end end - it "should allow for project owner any admin actions" do + it "allows for project owner any admin actions" do owner_actions.each do |action| expect(@abilities.allowed?(@u4, action, @p1)).to be_truthy end diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index f3d15f3c1ea..dc702cfc42c 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -65,7 +65,7 @@ describe AsanaService, models: true do ) end - it 'should call Asana service to create a story' do + it 'calls Asana service to create a story' do data = create_data_for_commits('Message from commit. related to #123456') expected_message = "#{data[:user_name]} pushed to branch #{data[:ref]} of #{project.name_with_namespace} ( #{data[:commits][0][:url]} ): #{data[:commits][0][:message]}" @@ -76,7 +76,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should call Asana service to create a story and close a task' do + it 'calls Asana service to create a story and close a task' do data = create_data_for_commits('fix #456789') d1 = double('Asana::Task') expect(d1).to receive(:add_comment) @@ -86,7 +86,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should be able to close via url' do + it 'is able to close via url' do data = create_data_for_commits('closes https://app.asana.com/19292/956299/42') d1 = double('Asana::Task') expect(d1).to receive(:add_comment) @@ -96,7 +96,7 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should allow multiple matches per line' do + it 'allows multiple matches per line' do message = <<-EOF minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index 17e9361dd5c..00c4e0fb64c 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -44,7 +44,7 @@ describe AssemblaService, models: true do WebMock.stub_request(:post, @api_url) end - it "should call Assembla API" do + it "calls Assembla API" do @assembla_service.execute(@sample_data) expect(WebMock).to have_requested(:post, @api_url).with( body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/ diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index 3e6da42803b..1adf93258f3 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -39,4 +39,62 @@ describe CampfireService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + @campfire_service = CampfireService.new + allow(@campfire_service).to receive_messages( + project_id: project.id, + project: project, + service_hook: true, + token: 'verySecret', + subdomain: 'project-name', + room: 'test-room' + ) + @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json' + @headers = { 'Content-Type' => 'application/json; charset=utf-8' } + end + + it "calls Campfire API to get a list of rooms and speak in a room" do + # make sure a valid list of rooms is returned + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # stub the speak request with the room id found in the previous request's response + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/123/speak.json' + WebMock.stub_request(:post, speak_url) + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).to have_requested(:post, speak_url).with( + body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/ + ).once + end + + it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do + # return a list of rooms that do not contain a room named 'test-room' + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # we want to make sure no request is sent to the /speak endpoint, here is a basic + # regexp that matches this endpoint + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json' + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).not_to have_requested(:post, /#{speak_url}/) + end + end end diff --git a/spec/models/project_services/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb index 5fe5ea7d2df..d7c5ea95d71 100644 --- a/spec/models/project_services/external_wiki_service_spec.rb +++ b/spec/models/project_services/external_wiki_service_spec.rb @@ -56,7 +56,7 @@ describe ExternalWikiService, models: true do @service.destroy! end - it 'should replace the wiki url' do + it 'replaces the wiki url' do wiki_path = get_project_wiki_path(project) expect(wiki_path).to match('https://gitlab.com') end diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index b7e627e6518..6518098ceea 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -57,7 +57,7 @@ describe FlowdockService, models: true do WebMock.stub_request(:post, @api_url) end - it "should call FlowDock API" do + it "calls FlowDock API" do @flowdock_service.execute(@sample_data) @sample_data[:commits].each do |commit| # One request to Flowdock per new commit diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index a08f1ac229f..2c5583bdaa2 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -57,7 +57,7 @@ describe GemnasiumService, models: true do ) @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) end - it "should call Gemnasium service" do + it "calls Gemnasium service" do expect(Gemnasium::GitlabService).to receive(:execute).with(an_instance_of(Hash)).once @gemnasium_service.execute(@sample_data) 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 7a1f106d6e3..8ef79a17d50 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -54,7 +54,7 @@ describe GitlabIssueTrackerService, models: true do @service.destroy! end - it 'should give the correct path' do + 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") @@ -71,7 +71,7 @@ describe GitlabIssueTrackerService, models: true do @service.destroy! end - it 'should give the correct path' do + it 'gives the correct path' do expect(@service.project_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues") expect(@service.new_issue_path).to eq("/gitlab/root/#{project.path_with_namespace}/issues/new") expect(@service.issue_path(432)).to eq("/gitlab/root/#{project.path_with_namespace}/issues/432") diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 5f618322aab..1b383219eb9 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -61,7 +61,7 @@ describe HipchatService, models: true do WebMock.stub_request(:post, api_url) end - it 'should test and return errors' do + it 'tests and return errors' do allow(hipchat).to receive(:execute).and_raise(StandardError, 'no such room') result = hipchat.test(push_sample_data) @@ -69,7 +69,7 @@ describe HipchatService, models: true do expect(result[:result].to_s).to eq('no such room') end - it 'should use v1 if version is provided' do + it 'uses v1 if version is provided' do allow(hipchat).to receive(:api_version).and_return('v1') expect(HipChat::Client).to receive(:new).with( token, @@ -79,7 +79,7 @@ describe HipchatService, models: true do hipchat.execute(push_sample_data) end - it 'should use v2 as the version when nothing is provided' do + it 'uses v2 as the version when nothing is provided' do allow(hipchat).to receive(:api_version).and_return('') expect(HipChat::Client).to receive(:new).with( token, @@ -90,13 +90,13 @@ describe HipchatService, models: true do end context 'push events' do - it "should call Hipchat API for push events" do + it "calls Hipchat API for push events" do hipchat.execute(push_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a push message" do + it "creates a push message" do message = hipchat.send(:create_push_message, push_sample_data) push_sample_data[:object_attributes] @@ -110,13 +110,13 @@ describe HipchatService, models: true do context 'tag_push events' do let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, Gitlab::Git::BLANK_SHA, '1' * 40, 'refs/tags/test', []) } - it "should call Hipchat API for tag push events" do + it "calls Hipchat API for tag push events" do hipchat.execute(push_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a tag push message" do + it "creates a tag push message" do message = hipchat.send(:create_push_message, push_sample_data) push_sample_data[:object_attributes] @@ -131,13 +131,13 @@ describe HipchatService, models: true do let(:issue_service) { Issues::CreateService.new(project, user) } let(:issues_sample_data) { issue_service.hook_data(issue, 'open') } - it "should call Hipchat API for issue events" do + it "calls Hipchat API for issue events" do hipchat.execute(issues_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create an issue message" do + it "creates an issue message" do message = hipchat.send(:create_issue_message, issues_sample_data) obj_attr = issues_sample_data[:object_attributes] @@ -154,13 +154,13 @@ describe HipchatService, models: true do let(:merge_service) { MergeRequests::CreateService.new(project, user) } let(:merge_sample_data) { merge_service.hook_data(merge_request, 'open') } - it "should call Hipchat API for merge requests events" do + it "calls Hipchat API for merge requests events" do hipchat.execute(merge_sample_data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a merge request message" do + it "creates a merge request message" do message = hipchat.send(:create_merge_request_message, merge_sample_data) @@ -184,7 +184,7 @@ describe HipchatService, models: true do note: 'a comment on a commit') end - it "should call Hipchat API for commit comment events" do + it "calls Hipchat API for commit comment events" do data = Gitlab::NoteDataBuilder.build(commit_note, user) hipchat.execute(data) @@ -216,7 +216,7 @@ describe HipchatService, models: true do note: "merge request note") end - it "should call Hipchat API for merge request comment events" do + it "calls Hipchat API for merge request comment events" do data = Gitlab::NoteDataBuilder.build(merge_request_note, user) hipchat.execute(data) @@ -243,7 +243,7 @@ describe HipchatService, models: true do note: "issue note") end - it "should call Hipchat API for issue comment events" do + it "calls Hipchat API for issue comment events" do data = Gitlab::NoteDataBuilder.build(issue_note, user) hipchat.execute(data) @@ -269,7 +269,7 @@ describe HipchatService, models: true do note: "snippet note") end - it "should call Hipchat API for snippet comment events" do + it "calls Hipchat API for snippet comment events" do data = Gitlab::NoteDataBuilder.build(snippet_note, user) hipchat.execute(data) @@ -291,19 +291,20 @@ describe HipchatService, models: true do end context 'build events' do - let(:build) { create(:ci_build) } + let(:pipeline) { create(:ci_empty_pipeline) } + let(:build) { create(:ci_build, pipeline: pipeline) } let(:data) { Gitlab::BuildDataBuilder.build(build) } context 'for failed' do before { build.drop } - it "should call Hipchat API" do + it "calls Hipchat API" do hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once end - it "should create a build message" do + it "creates a build message" do message = hipchat.send(:create_build_message, data) project_url = project.web_url @@ -325,13 +326,13 @@ describe HipchatService, models: true do build.success end - it "should call Hipchat API" do + it "calls Hipchat API" do hipchat.notify_only_broken_builds = false hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once end - it "should notify only broken" do + it "notifies only broken" do hipchat.notify_only_broken_builds = true hipchat.execute(data) expect(WebMock).not_to have_requested(:post, api_url).once @@ -340,18 +341,36 @@ describe HipchatService, models: true do end context "#message_options" do - it "should be set to the defaults" do - expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'yellow' }) + it "is set to the defaults" do + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'yellow' }) end - it "should set notfiy to true" do + it "sets notify to true" do allow(hipchat).to receive(:notify).and_return('1') - expect(hipchat.send(:message_options)).to eq({ notify: true, color: 'yellow' }) + + expect(hipchat.__send__(:message_options)).to eq({ notify: true, color: 'yellow' }) end - it "should set the color" do + it "sets the color" do allow(hipchat).to receive(:color).and_return('red') - expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'red' }) + + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'red' }) + end + + context 'with a successful build' do + it 'uses the green color' do + build_data = { object_kind: 'build', commit: { status: 'success' } } + + expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'green' }) + end + end + + context 'with a failed build' do + it 'uses the red color' do + build_data = { object_kind: 'build', commit: { status: 'failed' } } + + expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'red' }) + end end end end diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 4ee022a5171..ea718232255 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -52,26 +52,27 @@ describe IrkerService, models: true do let(:colorize_messages) { '1' } before do + @irker_server = TCPServer.new 'localhost', 0 + allow(irker).to receive_messages( active: true, project: project, project_id: project.id, service_hook: true, - server_host: 'localhost', - server_port: 6659, + server_host: @irker_server.addr[2], + server_port: @irker_server.addr[1], default_irc_uri: 'irc://chat.freenode.net/', recipients: recipients, colorize_messages: colorize_messages) irker.valid? - @irker_server = TCPServer.new 'localhost', 6659 end after do @irker_server.close end - it 'should send valid JSON messages to an Irker listener' do + it 'sends valid JSON messages to an Irker listener' do irker.execute(sample_data) conn = @irker_server.accept diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 5a97cf370da..342403f6354 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -75,7 +75,7 @@ describe JiraService, models: true do WebMock.stub_request(:post, @comment_url) end - it "should call JIRA API" do + it "calls JIRA API" do @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) expect(WebMock).to have_requested(:post, @comment_url).with( @@ -128,7 +128,7 @@ describe JiraService, models: true do expect(@jira_service.api_url).to eq("http://jira_edited.example.com/rest/api/2") end - it "should reset password if url changed, even if setter called multiple times" do + it "resets password if url changed, even if setter called multiple times" do @jira_service.api_url = 'http://jira1.example.com/rest/api/2' @jira_service.api_url = 'http://jira1.example.com/rest/api/2' @jira_service.save @@ -181,7 +181,7 @@ describe JiraService, models: true do @service.destroy! end - it 'should be initialized' do + it 'is initialized' do expect(@service.title).to eq('JIRA') expect(@service.description).to eq("Jira issue tracker") end @@ -197,7 +197,7 @@ describe JiraService, models: true do @service.destroy! end - it "should be correct" do + it "is correct" do expect(@service.title).to eq('Jira One') expect(@service.description).to eq('Jira One issue tracker') end @@ -225,7 +225,7 @@ describe JiraService, models: true do @service.destroy! end - it 'should be prepopulated with the settings' do + it 'is prepopulated with the settings' do expect(@service.properties["project_url"]).to eq('http://jira.sample/projects/project_a') expect(@service.properties["issues_url"]).to eq("http://jira.sample/issues/:id") expect(@service.properties["new_issue_url"]).to eq("http://jira.sample/projects/project_a/issues/new") diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb index f37edd4d970..d098d988521 100644 --- a/spec/models/project_services/pivotaltracker_service_spec.rb +++ b/spec/models/project_services/pivotaltracker_service_spec.rb @@ -39,4 +39,75 @@ describe PivotaltrackerService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe 'Execute' do + let(:service) do + PivotaltrackerService.new.tap do |service| + service.token = 'secret_api_token' + end + end + + let(:url) { PivotaltrackerService::API_ENDPOINT } + + def push_data(branch: 'master') + { + object_kind: 'push', + ref: "refs/heads/#{branch}", + commits: [ + { + id: '21c12ea', + author: { + name: 'Some User' + }, + url: 'https://example.com/commit', + message: 'commit message', + } + ] + } + end + + before do + WebMock.stub_request(:post, url) + end + + it 'should post correct message' do + service.execute(push_data) + expect(WebMock).to have_requested(:post, url).with( + body: { + 'source_commit' => { + 'commit_id' => '21c12ea', + 'author' => 'Some User', + 'url' => 'https://example.com/commit', + 'message' => 'commit message' + } + }, + headers: { + 'Content-Type' => 'application/json', + 'X-TrackerToken' => 'secret_api_token' + } + ).once + end + + context 'when allowed branches is specified' do + let(:service) do + super().tap do |service| + service.restrict_to_branch = 'master,v10' + end + end + + it 'should post message if branch is in the list' do + service.execute(push_data(branch: 'master')) + service.execute(push_data(branch: 'v10')) + + expect(WebMock).to have_requested(:post, url).twice + end + + it 'should not post message if branch is not in the list' do + service.execute(push_data(branch: 'mas')) + service.execute(push_data(branch: 'v11')) + + expect(WebMock).not_to have_requested(:post, url) + end + end + end end diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 555d9757b47..19c0270a493 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -72,7 +72,7 @@ describe PushoverService, models: true do WebMock.stub_request(:post, api_url) end - it 'should call Pushover API' do + it 'calls Pushover API' do pushover.execute(sample_data) expect(WebMock).to have_requested(:post, api_url).once diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index 379c3e1219c..41b93f08050 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -60,6 +60,7 @@ describe SlackService::NoteMessage, models: true do title: "merge request title\ndetails\n" } 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 " \ diff --git a/spec/models/project_services/slack_service/wiki_page_message_spec.rb b/spec/models/project_services/slack_service/wiki_page_message_spec.rb index 46dedb66c7c..13aea0b0600 100644 --- a/spec/models/project_services/slack_service/wiki_page_message_spec.rb +++ b/spec/models/project_services/slack_service/wiki_page_message_spec.rb @@ -47,7 +47,7 @@ describe SlackService::WikiPageMessage, models: true do context 'when :action == "create"' do before { args[:object_attributes][:action] = 'create' } - it 'it returns the attachment for a new wiki page' do + it 'returns the attachment for a new wiki page' do expect(subject.attachments).to eq([ { text: "Wiki page description", @@ -60,7 +60,7 @@ describe SlackService::WikiPageMessage, models: true do context 'when :action == "update"' do before { args[:object_attributes][:action] = 'update' } - it 'it returns the attachment for an updated wiki page' do + it 'returns the attachment for an updated wiki page' do expect(subject.attachments).to eq([ { text: "Wiki page description", diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index df511b1bc4c..45a5f4ef12a 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -93,31 +93,31 @@ describe SlackService, models: true do @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') end - it "should call Slack API for push events" do + it "calls Slack API for push events" do slack.execute(push_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "should call Slack API for issue events" do + it "calls Slack API for issue events" do slack.execute(@issues_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "should call Slack API for merge requests events" do + 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 "should call Slack API for wiki page events" do + 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 'should use the username as an option for slack when configured' do + 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). @@ -128,7 +128,7 @@ describe SlackService, models: true do slack.execute(push_sample_data) end - it 'should use the channel as an option when it is configured' do + 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). @@ -234,7 +234,7 @@ describe SlackService, models: true do note: 'a comment on a commit') end - it "should call Slack API for commit comment events" do + it "calls Slack API for commit comment events" do data = Gitlab::NoteDataBuilder.build(commit_note, user) slack.execute(data) @@ -248,7 +248,7 @@ describe SlackService, models: true do note: "merge request note") end - it "should call Slack API for merge request comment events" do + it "calls Slack API for merge request comment events" do data = Gitlab::NoteDataBuilder.build(merge_request_note, user) slack.execute(data) @@ -261,7 +261,7 @@ describe SlackService, models: true do create(:note_on_issue, project: project, note: "issue note") end - it "should call Slack API for issue comment events" do + it "calls Slack API for issue comment events" do data = Gitlab::NoteDataBuilder.build(issue_note, user) slack.execute(data) @@ -275,7 +275,7 @@ describe SlackService, models: true do note: "snippet note") end - it "should call Slack API for snippet comment events" do + it "calls Slack API for snippet comment events" do data = Gitlab::NoteDataBuilder.build(snippet_note, user) slack.execute(data) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9b017288488..9c3b4712cab 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -69,6 +69,7 @@ describe Project, models: true do it { is_expected.to include_module(Gitlab::ConfigHelper) } it { is_expected.to include_module(Gitlab::ShellAdapter) } it { is_expected.to include_module(Gitlab::VisibilityLevel) } + it { is_expected.to include_module(Gitlab::CurrentSettings) } it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } end @@ -88,7 +89,7 @@ describe Project, models: true do it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:repository_storage) } - it 'should not allow new projects beyond user limits' do + it 'does not allow new projects beyond user limits' do project2 = build(:project) allow(project2).to receive(:creator).and_return(double(can_create_project?: false, projects_limit: 0).as_null_object) expect(project2).not_to be_valid @@ -97,7 +98,7 @@ describe Project, models: true do describe 'wiki path conflict' do context "when the new path has been used by the wiki of other Project" do - it 'should have an error on the name attribute' do + it 'has an error on the name attribute' do new_project = build_stubbed(:project, namespace_id: project.namespace_id, path: "#{project.path}.wiki") expect(new_project).not_to be_valid @@ -106,7 +107,7 @@ describe Project, models: true do end context "when the new wiki path has been used by the path of other Project" do - it 'should have an error on the name attribute' do + it 'has an error on the name attribute' do project_with_wiki_suffix = create(:project, path: 'foo.wiki') new_project = build_stubbed(:project, namespace_id: project_with_wiki_suffix.namespace_id, path: 'foo') @@ -124,7 +125,7 @@ describe Project, models: true do allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end - it "should not allow repository storages that don't match a label in the configuration" do + it "does not allow repository storages that don't match a label in the configuration" do expect(project2).not_to be_valid expect(project2.errors[:repository_storage].first).to match(/is not included in the list/) end @@ -171,12 +172,12 @@ describe Project, models: true do end describe 'project token' do - it 'should set an random token if none provided' do + it 'sets an random token if none provided' do project = FactoryGirl.create :empty_project, runners_token: '' expect(project.runners_token).not_to eq('') end - it 'should not set an random toke if one provided' do + it 'does not set an random toke if one provided' do project = FactoryGirl.create :empty_project, runners_token: 'my-token' expect(project.runners_token).to eq('my-token') end @@ -224,7 +225,7 @@ describe Project, models: true do end end - it 'should return valid url to repo' do + it 'returns valid url to repo' do project = Project.new(path: 'somewhere') expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git') end @@ -245,12 +246,40 @@ describe Project, models: true do end end + describe "#new_issue_address" do + let(:project) { create(:empty_project, path: "somewhere") } + let(:user) { create(:user) } + + context 'incoming email enabled' do + before do + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + end + + it 'returns the address to create a new issue' do + token = user.authentication_token + address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab" + + expect(project.new_issue_address(user)).to eq(address) + end + end + + context 'incoming email disabled' do + before do + stub_incoming_email_setting(enabled: false) + end + + it 'returns nil' do + expect(project.new_issue_address(user)).to be_nil + end + end + end + describe 'last_activity methods' do let(:project) { create(:project) } let(:last_event) { double(created_at: Time.now) } describe 'last_activity' do - it 'should alias last_activity to last_event' do + it 'alias last_activity to last_event' do allow(project).to receive(:last_event).and_return(last_event) expect(project.last_activity).to eq(last_event) end @@ -321,13 +350,13 @@ describe Project, models: true do let(:prev_commit_id) { merge_request.commits.last.id } let(:commit_id) { merge_request.commits.first.id } - it 'should close merge request if last commit from source branch was pushed to target branch' do + it 'closes merge request if last commit from source branch was pushed to target branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.target_branch}", key.user) merge_request.reload expect(merge_request.merged?).to be_truthy end - it 'should update merge request commits with new one if pushed to source branch' do + it 'updates merge request commits with new one if pushed to source branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) merge_request.reload expect(merge_request.diff_head_sha).to eq(commit_id) @@ -372,6 +401,24 @@ describe Project, models: true do it { expect(@project.to_param).to eq('gitlabhq') } end + + context 'with invalid path' do + it 'returns previous path to keep project suitable for use in URLs when persisted' do + project = create(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + expect(project).not_to be_valid + expect(project.to_param).to eq 'gitlab' + end + + it 'returns current path when new record' do + project = build(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + expect(project).not_to be_valid + expect(project.to_param).to eq 'foo&bar' + end + end end describe '#repository' do @@ -386,11 +433,11 @@ describe Project, models: true do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } - it "should be true if used internal tracker" do + it "is true if used internal tracker" do expect(project.default_issues_tracker?).to be_truthy end - it "should be false if used other tracker" do + it "is false if used other tracker" do expect(ext_project.default_issues_tracker?).to be_falsey end end @@ -589,12 +636,12 @@ describe Project, models: true do describe '#avatar_type' do let(:project) { create(:project) } - it 'should be true if avatar is image' do + it 'is true if avatar is image' do project.update_attribute(:avatar, 'uploads/avatar.png') expect(project.avatar_type).to be_truthy end - it 'should be false if avatar is html page' do + it 'is false if avatar is html page' do project.update_attribute(:avatar, 'uploads/avatar.html') expect(project.avatar_type).to eq(['only images allowed']) end @@ -667,6 +714,20 @@ describe Project, models: true do it { expect(project.builds_enabled?).to be_truthy } end + describe '.cached_count', caching: true do + let(:group) { create(:group, :public) } + let!(:project1) { create(:empty_project, :public, group: group) } + let!(:project2) { create(:empty_project, :public, group: group) } + + it 'returns total project count' do + expect(Project).to receive(:count).once.and_call_original + + 3.times do + expect(Project.cached_count).to eq(2) + end + end + end + describe '.trending' do let(:group) { create(:group, :public) } let(:project1) { create(:empty_project, :public, group: group) } @@ -767,16 +828,16 @@ describe Project, models: true do context 'for shared runners disabled' do let(:shared_runners_enabled) { false } - it 'there are no runners available' do + it 'has no runners available' do expect(project.any_runners?).to be_falsey end - it 'there is a specific runner' do + it 'has a specific runner' do project.runners << specific_runner expect(project.any_runners?).to be_truthy end - it 'there is a shared runner, but they are prohibited to use' do + it 'has a shared runner, but they are prohibited to use' do shared_runner expect(project.any_runners?).to be_falsey end @@ -790,7 +851,7 @@ describe Project, models: true do context 'for shared runners enabled' do let(:shared_runners_enabled) { true } - it 'there is a shared runner' do + it 'has a shared runner' do shared_runner expect(project.any_runners?).to be_truthy end @@ -1024,68 +1085,97 @@ describe Project, models: true do end describe '#protected_branch?' do - let(:project) { create(:empty_project) } + context 'existing project' do + let(:project) { create(:project) } - it 'returns true when the branch matches a protected branch via direct match' do - project.protected_branches.create!(name: 'foo') + it 'returns true when the branch matches a protected branch via direct match' do + project.protected_branches.create!(name: 'foo') - expect(project.protected_branch?('foo')).to eq(true) - end + expect(project.protected_branch?('foo')).to eq(true) + end - it 'returns true when the branch matches a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + it 'returns true when the branch matches a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') - expect(project.protected_branch?('production/some-branch')).to eq(true) - end + expect(project.protected_branch?('production/some-branch')).to eq(true) + end - it 'returns false when the branch does not match a protected branch via direct match' do - expect(project.protected_branch?('foo')).to eq(false) - end + it 'returns false when the branch does not match a protected branch via direct match' do + expect(project.protected_branch?('foo')).to eq(false) + end - it 'returns false when the branch does not match a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + it 'returns false when the branch does not match a protected branch via wildcard match' do + project.protected_branches.create!(name: 'production/*') - expect(project.protected_branch?('staging/some-branch')).to eq(false) + expect(project.protected_branch?('staging/some-branch')).to eq(false) + end end - end - describe "#developers_can_push_to_protected_branch?" do - let(:project) { create(:empty_project) } + context "new project" do + let(:project) { create(:empty_project) } - context "when the branch matches a protected branch via direct match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production", project: project, developers_can_push: true) + it 'returns false when default_protected_branch is unprotected' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - expect(project.developers_can_push_to_protected_branch?('production')).to be true + expect(project.protected_branch?('master')).to be false end - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production", project: project, developers_can_push: false) + it 'returns false when default_protected_branch lets developers push' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - expect(project.developers_can_push_to_protected_branch?('production')).to be false + expect(project.protected_branch?('master')).to be false end - end - context "when the branch matches a protected branch via wilcard match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true + expect(project.protected_branch?('master')).to be true end - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: false) + it 'returns true when default_branch_protection is in full protection' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false + expect(project.protected_branch?('master')).to be true end end + end + + describe '#user_can_push_to_empty_repo?' do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + it 'returns false when default_branch_protection is in full protection and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey + end - context "when the branch does not match a protected branch" do - it "returns false" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) + it 'returns false when default_branch_protection only lets devs merge and user is dev' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false - end + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey + end + + it 'returns true when default_branch_protection lets devs push and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy + end + + it 'returns true when default_branch_protection is unprotected and user is developer' do + project.team << [user, :developer] + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy + end + + it 'returns true when user is master' do + project.team << [user, :master] + + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end end @@ -1244,6 +1334,32 @@ describe Project, models: true do end end + describe '#add_import_job' do + context 'forked' do + let(:forked_project_link) { create(:forked_project_link) } + let(:forked_from_project) { forked_project_link.forked_from_project } + let(:project) { forked_project_link.forked_to_project } + + it 'schedules a RepositoryForkWorker job' do + expect(RepositoryForkWorker).to receive(:perform_async). + with(project.id, forked_from_project.repository_storage_path, + forked_from_project.path_with_namespace, project.namespace.path) + + project.add_import_job + end + end + + context 'not forked' do + let(:project) { create(:project) } + + it 'schedules a RepositoryImportWorker job' do + expect(RepositoryImportWorker).to receive(:perform_async).with(project.id) + + project.add_import_job + end + end + end + describe '.where_paths_in' do context 'without any paths' do it 'returns an empty relation' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 9262aeb6ed8..5eaf0d3b7a6 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -151,8 +151,8 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } end context 'when project is shared with group' do @@ -168,14 +168,14 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } context 'but share_with_group_lock is true' do before { project.namespace.update(share_with_group_lock: true) } - it { expect(project.team.max_member_access(master.id)).to be_nil } - it { expect(project.team.max_member_access(reporter.id)).to be_nil } + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::NO_ACCESS) } end end end @@ -194,8 +194,74 @@ describe ProjectTeam, models: true do it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::MASTER) } it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_member_access(guest.id)).to eq(Gitlab::Access::GUEST) } - it { expect(project.team.max_member_access(nonmember.id)).to be_nil } - it { expect(project.team.max_member_access(requester.id)).to be_nil } + it { expect(project.team.max_member_access(nonmember.id)).to eq(Gitlab::Access::NO_ACCESS) } + it { expect(project.team.max_member_access(requester.id)).to eq(Gitlab::Access::NO_ACCESS) } end end + + shared_examples_for "#max_member_access_for_users" do |enable_request_store| + describe "#max_member_access_for_users" do + before do + RequestStore.begin! if enable_request_store + end + + after do + if enable_request_store + RequestStore.end! + RequestStore.clear! + end + end + + it 'returns correct roles for different users' do + master = create(:user) + reporter = create(:user) + promoted_guest = create(:user) + guest = create(:user) + project = create(:project) + + project.team << [master, :master] + project.team << [reporter, :reporter] + project.team << [promoted_guest, :guest] + project.team << [guest, :guest] + + group = create(:group) + group_developer = create(:user) + second_developer = create(:user) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::DEVELOPER) + + group.add_master(promoted_guest) + group.add_developer(group_developer) + group.add_developer(second_developer) + + second_group = create(:group) + project.project_group_links.create( + group: second_group, + group_access: Gitlab::Access::MASTER) + second_group.add_master(second_developer) + + users = [master, reporter, promoted_guest, guest, group_developer, second_developer].map(&:id) + + expected = { + master.id => Gitlab::Access::MASTER, + reporter.id => Gitlab::Access::REPORTER, + promoted_guest.id => Gitlab::Access::DEVELOPER, + guest.id => Gitlab::Access::GUEST, + group_developer.id => Gitlab::Access::DEVELOPER, + second_developer.id => Gitlab::Access::MASTER + } + + expect(project.team.max_member_access_for_user_ids(users)).to eq(expected) + end + end + end + + describe '#max_member_access_for_users with RequestStore' do + it_behaves_like "#max_member_access_for_users", true + end + + describe '#max_member_access_for_users without RequestStore' do + it_behaves_like "#max_member_access_for_users", false + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 881ab5ff8dc..f7dbfd712cc 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -50,8 +50,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now) double_last = double(committed_date: Time.now - 1.second) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_first) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_last) + allow(tag_a).to receive(:target).and_return(double_first) + allow(tag_b).to receive(:target).and_return(double_last) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } @@ -64,8 +65,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_last) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_first) + allow(tag_a).to receive(:target).and_return(double_last) + allow(tag_b).to receive(:target).and_return(double_first) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } @@ -338,14 +340,14 @@ describe Repository, models: true do describe '#add_branch' do context 'when pre hooks were successful' do - it 'should run without errors' do + it 'runs without errors' do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error end - it 'should create the branch' do + it 'creates the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) branch = repository.add_branch(user, 'new_feature', 'master') @@ -361,7 +363,7 @@ describe Repository, models: true do end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -369,7 +371,7 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end - it 'should not create the branch' do + it 'does not create the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -381,14 +383,18 @@ describe Repository, models: true do end describe '#rm_branch' do + let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature + let(:blank_sha) { '0000000000000000000000000000000000000000' } + context 'when pre hooks were successful' do - it 'should run without errors' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) + it 'runs without errors' do + expect_any_instance_of(GitHooksService).to receive(:execute). + with(user, project.repository.path_to_repo, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end - it 'should delete the branch' do + it 'deletes the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) expect { repository.rm_branch(user, 'feature') }.not_to raise_error @@ -398,7 +404,7 @@ describe Repository, models: true do end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -406,7 +412,7 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end - it 'should not delete the branch' do + it 'does not delete the branch' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -418,27 +424,38 @@ describe Repository, models: true do end describe '#commit_with_hooks' do + let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature + context 'when pre hooks were successful' do before do expect_any_instance_of(GitHooksService).to receive(:execute). - and_return(true) + with(user, repository.path_to_repo, old_rev, sample_commit.id, 'refs/heads/feature'). + and_yield.and_return(true) end - it 'should run without errors' do + it 'runs without errors' do expect do repository.commit_with_hooks(user, 'feature') { sample_commit.id } end.not_to raise_error end - it 'should ensure the autocrlf Git option is set to :input' do + it 'ensures the autocrlf Git option is set to :input' do expect(repository).to receive(:update_autocrlf_option) repository.commit_with_hooks(user, 'feature') { sample_commit.id } end + + context "when the branch wasn't empty" do + it 'updates the head' do + expect(repository.find_branch('feature').target.id).to eq(old_rev) + repository.commit_with_hooks(user, 'feature') { sample_commit.id } + expect(repository.find_branch('feature').target.id).to eq(sample_commit.id) + end + end end context 'when pre hooks failed' do - it 'should get an error' do + it 'gets an error' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do @@ -446,6 +463,43 @@ describe Repository, models: true do end.to raise_error(GitHooksService::PreReceiveError) end end + + context 'when target branch is different from source branch' do + before do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) + end + + it 'expires branch cache' do + expect(repository).not_to receive(:expire_exists_cache) + expect(repository).not_to receive(:expire_root_ref_cache) + expect(repository).not_to receive(:expire_emptiness_caches) + expect(repository).to receive(:expire_branches_cache) + expect(repository).to receive(:expire_has_visible_content_cache) + expect(repository).to receive(:expire_branch_count_cache) + + repository.commit_with_hooks(user, 'new-feature') { sample_commit.id } + end + end + + context 'when repository is empty' do + before do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) + end + + it 'expires creation and branch cache' do + empty_repository = create(:empty_project, :empty_repo).repository + + expect(empty_repository).to receive(:expire_exists_cache) + 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) + end + end end describe '#exists?' do @@ -661,7 +715,7 @@ describe Repository, models: true do end describe '#merge' do - it 'should merge the code and return the commit id' do + it 'merges the code and return the commit id' do expect(merge_commit).to be_present expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end @@ -672,13 +726,13 @@ describe Repository, models: true do let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } context 'when there is a conflict' do - it 'should abort the operation' do + it 'aborts the operation' do expect(repository.revert(user, new_image_commit, 'master')).to eq(false) end end context 'when commit was already reverted' do - it 'should abort the operation' do + it 'aborts the operation' do repository.revert(user, update_image_commit, 'master') expect(repository.revert(user, update_image_commit, 'master')).to eq(false) @@ -686,13 +740,13 @@ describe Repository, models: true do end context 'when commit can be reverted' do - it 'should revert the changes' do + it 'reverts the changes' do expect(repository.revert(user, update_image_commit, 'master')).to be_truthy end end context 'reverting a merge commit' do - it 'should revert the changes' do + it 'reverts the changes' do merge_commit expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present @@ -708,13 +762,13 @@ describe Repository, models: true do let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } context 'when there is a conflict' do - it 'should abort the operation' do + it 'aborts the operation' do expect(repository.cherry_pick(user, conflict_commit, 'master')).to eq(false) end end context 'when commit was already cherry-picked' do - it 'should abort the operation' do + it 'aborts the operation' do repository.cherry_pick(user, pickable_commit, 'master') expect(repository.cherry_pick(user, pickable_commit, 'master')).to eq(false) @@ -722,13 +776,13 @@ describe Repository, models: true do end context 'when commit can be cherry-picked' do - it 'should cherry-pick the changes' do + it 'cherry-picks the changes' do expect(repository.cherry_pick(user, pickable_commit, 'master')).to be_truthy end end context 'cherry-picking a merge commit' do - it 'should cherry-pick the changes' do + it 'cherry-picks the changes' do expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil repository.cherry_pick(user, pickable_merge, 'master') @@ -1100,7 +1154,7 @@ describe Repository, models: true do it 'does not flush the cache if the commit does not change any logos' do diff = double(:diff, new_path: 'test.txt') - expect(commit).to receive(:diffs).and_return([diff]) + expect(commit).to receive(:raw_diffs).and_return([diff]) expect(cache).not_to receive(:expire) repository.expire_avatar_cache(repository.root_ref, '123') @@ -1109,7 +1163,7 @@ describe Repository, models: true do it 'flushes the cache if the commit changes any of the logos' do diff = double(:diff, new_path: Repository::AVATAR_FILES[0]) - expect(commit).to receive(:diffs).and_return([diff]) + expect(commit).to receive(:raw_diffs).and_return([diff]) expect(cache).to receive(:expire).with(:avatar) repository.expire_avatar_cache(repository.root_ref, '123') @@ -1161,17 +1215,6 @@ describe Repository, models: true do end end - describe '#local_branches' do - it 'returns the local branches' do - masterrev = repository.find_branch('master').target - create_remote_branch('joe', 'remote_branch', masterrev) - repository.add_branch(user, 'local_branch', masterrev) - - expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) - expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) - 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 @@ -1199,9 +1242,4 @@ describe Repository, models: true do File.delete(path) end end - - def create_remote_branch(remote_name, branch_name, target) - rugged = repository.rugged - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target) - end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 67b3783d514..05056a4bb47 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -65,13 +65,13 @@ describe Service, models: true do end let(:project) { create(:project) } - describe 'should be prefilled for projects pushover service' do + describe 'is prefilled for projects pushover service' do before do service_template project.build_missing_services end - it "should have all fields prefilled" do + it "has all fields prefilled" do service = project.pushover_service expect(service.template).to eq(false) expect(service.device).to eq('MyDevice') diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb new file mode 100644 index 00000000000..a8c25766e73 --- /dev/null +++ b/spec/models/user_agent_detail_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe UserAgentDetail, type: :model do + describe '.submittable?' do + it 'is submittable when not already submitted' do + detail = build(:user_agent_detail) + + expect(detail.submittable?).to be_truthy + end + + it 'is not submittable when already submitted' do + detail = build(:user_agent_detail, submitted: true) + + expect(detail.submittable?).to be_falsey + end + end + + describe '.valid?' do + it 'is valid with a subject' do + detail = build(:user_agent_detail) + + expect(detail).to be_valid + end + + it 'is invalid without a subject' do + detail = build(:user_agent_detail, subject: nil) + + expect(detail).not_to be_valid + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2a5a7fb2fc6..f67acbbef37 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -166,7 +166,7 @@ describe User, models: true do allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['*.example.com']) end - it 'should give priority to whitelist and allow info@test.example.com' do + it 'gives priority to whitelist and allow info@test.example.com' do user = build(:user, email: 'info@test.example.com') expect(user).to be_valid end @@ -304,18 +304,18 @@ describe User, models: true do end describe '#generate_password' do - it "should execute callback when force_random_password specified" do + it "executes callback when force_random_password specified" do user = build(:user, force_random_password: true) expect(user).to receive(:generate_password) user.save end - it "should not generate password by default" do + it "does not generate password by default" do user = create(:user, password: 'abcdefghe') expect(user.password).to eq('abcdefghe') end - it "should generate password when forcing random password" do + it "generates password when forcing random password" do allow(Devise).to receive(:friendly_token).and_return('123456789') user = create(:user, password: 'abcdefg', force_random_password: true) expect(user.password).to eq('12345678') @@ -323,7 +323,7 @@ describe User, models: true do end describe 'authentication token' do - it "should have authentication token" do + it "has authentication token" do user = create(:user) expect(user.authentication_token).not_to be_blank end @@ -430,7 +430,7 @@ describe User, models: true do describe 'blocking user' do let(:user) { create(:user, name: 'John Smith') } - it "should block user" do + it "blocks user" do user.block expect(user.blocked?).to be_truthy end @@ -501,7 +501,7 @@ describe User, models: true do describe 'with defaults' do let(:user) { User.new } - it "should apply defaults to user" do + it "applies defaults to user" do expect(user.projects_limit).to eq(Gitlab.config.gitlab.default_projects_limit) expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) @@ -512,7 +512,7 @@ describe User, models: true do describe 'with default overrides' do let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: 1) } - it "should apply defaults to user" do + it "applies defaults to user" do expect(user.projects_limit).to eq(123) expect(user.can_create_group).to be_falsey expect(user.theme_id).to eq(1) @@ -602,7 +602,7 @@ describe User, models: true do describe 'by_username_or_id' do let(:user1) { create(:user, username: 'foo') } - it "should get the correct user" do + 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 @@ -614,7 +614,7 @@ describe User, models: true do let(:username) { 'John' } let!(:user) { create(:user, username: username) } - it 'should get the correct user' do + it 'gets the correct user' do expect(User.by_login(user.email.upcase)).to eq user expect(User.by_login(user.email)).to eq user expect(User.by_login(username.downcase)).to eq user @@ -639,23 +639,23 @@ describe User, models: true do describe 'all_ssh_keys' do it { is_expected.to have_many(:keys).dependent(:destroy) } - it "should have all ssh keys" do + it "has all ssh keys" do user = create :user key = create :key, key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD33bWLBxu48Sev9Fert1yzEO4WGcWglWF7K/AwblIUFselOt/QdOL9DSjpQGxLagO1s9wl53STIO8qGS4Ms0EJZyIXOEFMjFJ5xmjSy+S37By4sG7SsltQEHMxtbtFOaW5LV2wCrX+rUsRNqLMamZjgjcPO0/EgGCXIGMAYW4O7cwGZdXWYIhQ1Vwy+CsVMDdPkPgBXqK7nR/ey8KMs8ho5fMNgB5hBw/AL9fNGhRw3QTD6Q12Nkhl4VZES2EsZqlpNnJttnPdp847DUsT6yuLRlfiQfz5Cn9ysHFdXObMN5VYIiPFwHeYCZp1X2S4fDZooRE8uOLTfxWHPXwrhqSH", user_id: user.id - expect(user.all_ssh_keys).to include(key.key) + expect(user.all_ssh_keys).to include(a_string_starting_with(key.key)) end end describe '#avatar_type' do let(:user) { create(:user) } - it "should be true if avatar is image" do + it "is true if avatar is image" do user.update_attribute(:avatar, 'uploads/avatar.png') expect(user.avatar_type).to be_truthy end - it "should be false if avatar is html page" do + it "is false if avatar is html page" do user.update_attribute(:avatar, 'uploads/avatar.html') expect(user.avatar_type).to eq(["only images allowed"]) end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index ddc49495eda..5c34b1b0a30 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -147,12 +147,12 @@ describe WikiPage, models: true do @page = wiki.find_page("Delete Page") end - it "should delete the page" do + it "deletes the page" do @page.delete expect(wiki.pages).to be_empty end - it "should return true" do + it "returns true" do expect(@page.delete).to eq(true) end end @@ -183,7 +183,7 @@ describe WikiPage, models: true do destroy_page("Title") end - it "should be replace a hyphen to a space" do + it "replaces a hyphen to a space" do @page.title = "Import-existing-repositories-into-GitLab" expect(@page.title).to eq("Import existing repositories into GitLab") end |