diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2016-08-15 17:27:22 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2016-08-15 17:27:22 +0300 |
commit | 5e95c0b8d201a6bfdd020aa29a5a6e14c7085e3f (patch) | |
tree | 5e221b64c68dfbe5c6cb026b7ac92065104342f1 /spec/models | |
parent | 94a7198ade54595d72797cab09db2c2a89172535 (diff) | |
parent | 6af598fc173bd0f7cd4237fa3e60d223103301a3 (diff) | |
download | gitlab-ce-5e95c0b8d201a6bfdd020aa29a5a6e14c7085e3f.tar.gz |
Merge branch 'master' into dz-merge-request-version
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Diffstat (limited to 'spec/models')
49 files changed, 778 insertions, 575 deletions
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/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 dc88697199b..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 @@ -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 c3392ee7440..d3e6a6648cc 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -86,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 } 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/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/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/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/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/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 44cd3c08718..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) } 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 f2954a39b08..2f3bb3ed2c4 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -22,7 +22,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 @@ -30,19 +30,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 d32bec53a28..9de9f19a27c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -128,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/*'] } } @@ -136,7 +161,7 @@ describe MergeRequest, models: true do it 'delegates to the MR diffs' do merge_request.merge_request_diffs.build - 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 @@ -163,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) @@ -279,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" @@ -663,6 +688,12 @@ describe MergeRequest, models: true do expect { subject.reload_diff }.to change { subject.merge_request_diffs.count }.by(1) 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 @@ -692,6 +723,7 @@ describe MergeRequest, models: true do end end +<<<<<<< HEAD describe '#branch_merge_base_commit' do context 'source and target branch exist' do it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } @@ -705,6 +737,28 @@ describe MergeRequest, models: true do it 'returns nil' do expect(subject.branch_merge_base_commit).to be_nil +======= + 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) +>>>>>>> master 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 62ae5f6cf74..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 diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 4ee022a5171..b528baaf15c 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -71,7 +71,7 @@ describe IrkerService, models: true 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 e365e4e98b2..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 @@ -278,7 +279,7 @@ describe Project, models: true do 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 @@ -349,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) @@ -432,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 @@ -635,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 @@ -713,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) } @@ -813,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 @@ -836,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 @@ -1070,28 +1085,97 @@ describe Project, models: true do end describe '#protected_branch?' do + 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') + + 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/*') + + 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 wildcard match' do + project.protected_branches.create!(name: 'production/*') + + expect(project.protected_branch?('staging/some-branch')).to eq(false) + end + end + + context "new project" do + let(:project) { create(:empty_project) } + + it 'returns false when default_protected_branch is unprotected' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(project.protected_branch?('master')).to be false + end + + 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.protected_branch?('master')).to be false + end + + 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.protected_branch?('master')).to be true + end + + it 'returns true when default_branch_protection is in full protection' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + + 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 true when the branch matches a protected branch via direct match' do - project.protected_branches.create!(name: 'foo') + 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.protected_branch?('foo')).to eq(true) + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end - it 'returns true when the branch matches a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + 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.protected_branch?('production/some-branch')).to eq(true) + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey 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) + 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 false when the branch does not match a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + it 'returns true when user is master' do + project.team << [user, :master] - expect(project.protected_branch?('staging/some-branch')).to eq(false) + expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cce15538b93..f7dbfd712cc 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -340,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') @@ -363,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 @@ -371,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 @@ -387,14 +387,14 @@ describe Repository, models: true do let(:blank_sha) { '0000000000000000000000000000000000000000' } context 'when pre hooks were successful' do - it 'should run without errors' do + 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 @@ -404,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 @@ -412,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 @@ -433,13 +433,13 @@ describe Repository, models: true do 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 } @@ -455,7 +455,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 @@ -715,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 @@ -726,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) @@ -740,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 @@ -762,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) @@ -776,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') @@ -1154,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') @@ -1163,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') 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_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 |