From 811de6ac73ea68ae3361ec49d2094d4f2006f493 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Sep 2016 21:57:04 +0800 Subject: Add test for sending pipeline notifications --- spec/models/ci/pipeline_spec.rb | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 550a890797e..f564a2d4fdd 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -523,4 +523,58 @@ describe Ci::Pipeline, models: true do expect(pipeline.merge_requests).to be_empty end end + + describe 'notifications when pipeline success or failed' do + before do + ActionMailer::Base.deliveries.clear + + pipeline.enqueue + pipeline.run + end + + shared_examples 'sending a notification' do + it 'sends an email' do + sent_to = ActionMailer::Base.deliveries.flat_map(&:to) + expect(sent_to).to contain_exactly(pipeline.user.email) + end + end + + shared_examples 'not sending any notification' do + it 'does not send any email' do + expect(ActionMailer::Base.deliveries).to be_empty + end + end + + context 'with success pipeline' do + before do + perform_enqueued_jobs do + pipeline.success + end + end + end + + context 'with failed pipeline' do + before do + perform_enqueued_jobs do + pipeline.drop + end + end + end + + context 'with skipped pipeline' do + before do + perform_enqueued_jobs do + pipeline.skip + end + end + end + + context 'with cancelled pipeline' do + before do + perform_enqueued_jobs do + pipeline.cancel + end + end + end + end end -- cgit v1.2.1 From aafd42c49d516d301d72a4f43d7e56d7bf168d80 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Sep 2016 22:12:31 +0800 Subject: How could I forget this? --- spec/models/ci/pipeline_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f564a2d4fdd..dc1d1de12a1 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -528,6 +528,7 @@ describe Ci::Pipeline, models: true do before do ActionMailer::Base.deliveries.clear + pipeline.update(user: create(:user)) pipeline.enqueue pipeline.run end @@ -548,9 +549,11 @@ describe Ci::Pipeline, models: true do context 'with success pipeline' do before do perform_enqueued_jobs do - pipeline.success + pipeline.succeed end end + + it_behaves_like 'sending a notification' end context 'with failed pipeline' do @@ -559,6 +562,8 @@ describe Ci::Pipeline, models: true do pipeline.drop end end + + it_behaves_like 'sending a notification' end context 'with skipped pipeline' do @@ -567,6 +572,8 @@ describe Ci::Pipeline, models: true do pipeline.skip end end + + it_behaves_like 'not sending any notification' end context 'with cancelled pipeline' do @@ -575,6 +582,8 @@ describe Ci::Pipeline, models: true do pipeline.cancel end end + + it_behaves_like 'not sending any notification' end end end -- cgit v1.2.1 From d39b1959d80f857e57fe67696c8d4525cb5966d9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Sep 2016 13:47:11 +0800 Subject: Prefer methods from EmailHelpers, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6342/diffs#note_15434869 --- spec/models/ci/pipeline_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index dc1d1de12a1..03dd8867b18 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -526,7 +526,7 @@ describe Ci::Pipeline, models: true do describe 'notifications when pipeline success or failed' do before do - ActionMailer::Base.deliveries.clear + reset_delivered_emails! pipeline.update(user: create(:user)) pipeline.enqueue -- cgit v1.2.1 From 572585665f838584b9547831528f26fb60df8d0f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Sep 2016 13:54:16 +0800 Subject: Use EmailHelpers where possible, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6342#note_15434860 --- spec/models/ci/pipeline_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 03dd8867b18..050ab50f84f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -535,14 +535,13 @@ describe Ci::Pipeline, models: true do shared_examples 'sending a notification' do it 'sends an email' do - sent_to = ActionMailer::Base.deliveries.flat_map(&:to) - expect(sent_to).to contain_exactly(pipeline.user.email) + should_only_email(pipeline.user) end end shared_examples 'not sending any notification' do it 'does not send any email' do - expect(ActionMailer::Base.deliveries).to be_empty + should_email_no_one end end -- cgit v1.2.1 From 612820b2964e77397b6d85f74eb96888b72c4cbd Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 26 Sep 2016 19:04:02 +0800 Subject: Fix test by having a real commit --- spec/models/ci/pipeline_spec.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 050ab50f84f..787042cf055 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -525,10 +525,18 @@ describe Ci::Pipeline, models: true do end describe 'notifications when pipeline success or failed' do + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit('master').sha, + user: create(:user)) + end + before do reset_delivered_emails! - pipeline.update(user: create(:user)) pipeline.enqueue pipeline.run end -- cgit v1.2.1 From eeeb96c9d0cab9c5da850809a9614e2a01fdb7d2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 28 Sep 2016 17:22:06 +0800 Subject: Change service to be a worker, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6342#note_16118195 --- spec/models/ci/pipeline_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 787042cf055..6ea712ae1b3 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -537,8 +537,10 @@ describe Ci::Pipeline, models: true do before do reset_delivered_emails! - pipeline.enqueue - pipeline.run + perform_enqueued_jobs do + pipeline.enqueue + pipeline.run + end end shared_examples 'sending a notification' do -- cgit v1.2.1 From 98217bc0674253f30dea3892a8e833436b89d5b9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 17 Oct 2016 18:03:53 +0800 Subject: should_email_no_one -> should_not_email_anyone, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6342/diffs#note_17039876 --- spec/models/ci/pipeline_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 6ea712ae1b3..85820a9dc57 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -551,7 +551,7 @@ describe Ci::Pipeline, models: true do shared_examples 'not sending any notification' do it 'does not send any email' do - should_email_no_one + should_not_email_anyone end end -- cgit v1.2.1 From 045c6715330d25adff95304a3de908037c63a163 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 18 Oct 2016 20:02:35 +0800 Subject: Use bcc for pipeline emails because: We use bcc here because we don't want to generate this emails for a thousand times. This could be potentially expensive in a loop, and recipients would contain all project watchers so it could be a lot. --- spec/models/ci/pipeline_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index caf191e11ae..4032029b80b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -536,7 +536,7 @@ describe Ci::Pipeline, models: true do shared_examples 'sending a notification' do it 'sends an email' do - should_only_email(pipeline.user) + should_only_email(pipeline.user, kind: :bcc) end end -- cgit v1.2.1 From f388fceb4cc64d67b95e45cdbe77c9907803d6f1 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 22 Oct 2016 01:10:47 +0800 Subject: Make sure pusher has the read_build privilege --- spec/models/ci/pipeline_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ab05150c97e..65e663be411 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -542,6 +542,8 @@ describe Ci::Pipeline, models: true do before do reset_delivered_emails! + project.team << [pipeline.user, Gitlab::Access::DEVELOPER] + perform_enqueued_jobs do pipeline.enqueue pipeline.run -- cgit v1.2.1 From 6d1c5761cd520f3cb7fa4dbb1a1937c29f468884 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 17 Nov 2016 00:57:50 +0800 Subject: Improve how we could cancel pipelines: * Introduce `HasStatus.cancelable` which we might be able to cancel * Cancel and check upon `cancelable` * Also cancel on `CommitStatus` rather than just `Ci::Build` Fixes #23635 Fixes #17845 --- spec/models/ci/pipeline_spec.rb | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 71b7628ef10..d013dc7b31a 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -402,6 +402,46 @@ describe Ci::Pipeline, models: true do end end + describe '#cancelable?' do + subject { pipeline.cancelable? } + + %i[created running pending].each do |status| + context "when there is a build #{status}" do + before do + create(:ci_build, status, pipeline: pipeline) + end + + it { is_expected.to be_truthy } + end + + context "when there is an external job #{status}" do + before do + create(:generic_commit_status, status, pipeline: pipeline) + end + + it { is_expected.to be_truthy } + end + end + + %i[success failed canceled].each do |status| + context "when there is a build #{status}" do + before do + create(:ci_build, status, pipeline: pipeline) + end + + it { is_expected.to be_falsey } + end + + context "when there is an external job #{status}" do + before do + create(:generic_commit_status, status, pipeline: pipeline) + end + + it { is_expected.to be_falsey } + end + end + end + describe '#execute_hooks' do let!(:build_a) { create_build('a', 0) } let!(:build_b) { create_build('b', 1) } -- cgit v1.2.1 From 100076ecbbdf3eae361a6356ddfb55b1694e4741 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 18 Nov 2016 23:27:06 +0800 Subject: Add tests against two jobs having different status Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18622469 --- spec/models/ci/pipeline_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d013dc7b31a..2cc6d1be606 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -421,6 +421,18 @@ describe Ci::Pipeline, models: true do it { is_expected.to be_truthy } end + + %i[success failed canceled].each do |status2| + context "when there are two builds for #{status} and #{status2}" do + before do + build = %i[ci_build generic_commit_status] + create(build.sample, status, pipeline: pipeline) + create(build.sample, status2, pipeline: pipeline) + end + + it { is_expected.to be_truthy } + end + end end %i[success failed canceled].each do |status| -- cgit v1.2.1 From 92a83aaee1c5715bf95715649aa8ced50a70dea8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 19 Nov 2016 01:17:40 +0800 Subject: Add a test for Ci::Pipeline#cancel_running: Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18622559 I didn't write an exhaustive one because we already have it on HasStatus, from: https://gitlab.com/gitlab-org/gitlab-ce/commit/b6a7a4783435a7fa34f26dbf3b16ab8e7ed21b88 --- spec/models/ci/pipeline_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2cc6d1be606..74579e0c832 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -454,6 +454,22 @@ describe Ci::Pipeline, models: true do end end + describe '#cancel_running' do + context 'when there is a running external job and created build' do + before do + create(:generic_commit_status, :running, pipeline: pipeline) + create(:ci_build, :created, pipeline: pipeline) + + pipeline.cancel_running + end + + it 'cancels both jobs' do + expect(pipeline.statuses.pluck(:status)). + to contain_exactly('canceled', 'canceled') + end + end + end + describe '#execute_hooks' do let!(:build_a) { create_build('a', 0) } let!(:build_b) { create_build('b', 1) } -- cgit v1.2.1 From ca639c9b824d6c8effb620bc71255eb0895ab2cc Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 19 Nov 2016 14:04:11 +0100 Subject: Allow to retry failed or canceled builds and fix cancel running specs failure --- spec/models/ci/pipeline_spec.rb | 63 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 74579e0c832..af619a02ed9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -455,17 +455,74 @@ describe Ci::Pipeline, models: true do end describe '#cancel_running' do + let(:latest_status) { pipeline.statuses.pluck(:status) } + context 'when there is a running external job and created build' do before do + create(:ci_build, :running, pipeline: pipeline) create(:generic_commit_status, :running, pipeline: pipeline) - create(:ci_build, :created, pipeline: pipeline) pipeline.cancel_running end it 'cancels both jobs' do - expect(pipeline.statuses.pluck(:status)). - to contain_exactly('canceled', 'canceled') + expect(latest_status).to contain_exactly('canceled', 'canceled') + end + end + + context 'when builds are in different stages' do + before do + create(:ci_build, :running, stage_idx: 0, pipeline: pipeline) + create(:ci_build, :running, stage_idx: 1, pipeline: pipeline) + + pipeline.cancel_running + end + + it 'cancels both jobs' do + expect(latest_status).to contain_exactly('canceled', 'canceled') + end + end + end + + describe '#retry_failed' do + let(:latest_status) { pipeline.statuses.latest.pluck(:status) } + + context 'when there is a failed build and failed external status' do + before do + create(:ci_build, :failed, name: 'build', pipeline: pipeline) + create(:generic_commit_status, :failed, name: 'jenkins', pipeline: pipeline) + + pipeline.retry_failed(nil) + end + + it 'retries only build' do + expect(latest_status).to contain_exactly('pending', 'failed') + end + end + + context 'when builds are in different stages' do + before do + create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) + create(:ci_build, :failed, name: 'jenkins', stage_idx: 1, pipeline: pipeline) + + pipeline.retry_failed(nil) + end + + it 'retries both builds' do + expect(latest_status).to contain_exactly('pending', 'pending') + end + end + + context 'when there are canceled and failed' do + before do + create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) + create(:ci_build, :canceled, name: 'jenkins', stage_idx: 1, pipeline: pipeline) + + pipeline.retry_failed(nil) + end + + it 'retries both builds' do + expect(latest_status).to contain_exactly('pending', 'pending') end end end -- cgit v1.2.1 From 1edb1746a51a19fae24c976c329e80a1dbd6062a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 21 Nov 2016 17:59:57 +0800 Subject: Prefer a description for it and split the case: Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18730091 --- spec/models/ci/pipeline_spec.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index af619a02ed9..29e5693d5ab 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -403,15 +403,15 @@ describe Ci::Pipeline, models: true do end describe '#cancelable?' do - subject { pipeline.cancelable? } - %i[created running pending].each do |status| context "when there is a build #{status}" do before do create(:ci_build, status, pipeline: pipeline) end - it { is_expected.to be_truthy } + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end end context "when there is an external job #{status}" do @@ -419,7 +419,9 @@ describe Ci::Pipeline, models: true do create(:generic_commit_status, status, pipeline: pipeline) end - it { is_expected.to be_truthy } + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end end %i[success failed canceled].each do |status2| @@ -430,7 +432,9 @@ describe Ci::Pipeline, models: true do create(build.sample, status2, pipeline: pipeline) end - it { is_expected.to be_truthy } + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end end end end @@ -441,7 +445,9 @@ describe Ci::Pipeline, models: true do create(:ci_build, status, pipeline: pipeline) end - it { is_expected.to be_falsey } + it 'is not cancelable' do + expect(pipeline.cancelable?).to be_falsey + end end context "when there is an external job #{status}" do @@ -449,7 +455,9 @@ describe Ci::Pipeline, models: true do create(:generic_commit_status, status, pipeline: pipeline) end - it { is_expected.to be_falsey } + it 'is not cancelable' do + expect(pipeline.cancelable?).to be_falsey + end end end end -- cgit v1.2.1 From fa936c68083810043b8c84fcbcad9fb3ce717eb6 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 21 Nov 2016 20:29:34 +0800 Subject: External jobs do not have show page nor traces Fixes #24677 --- spec/models/ci/pipeline_spec.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 71b7628ef10..ea5e0e28c5c 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -571,7 +571,13 @@ describe Ci::Pipeline, models: true do context 'with failed pipeline' do before do perform_enqueued_jobs do - pipeline.drop + create(:ci_build, :created, pipeline: pipeline) + create(:generic_commit_status, :created, pipeline: pipeline) + + pipeline.statuses.count.times do |offset| + # workaround race conditions + pipeline.statuses.offset(offset).first.drop + end end end -- cgit v1.2.1 From c7c4850d0b9d6751a5f6fdaa1f9c34aee6728676 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 22 Nov 2016 01:21:15 +0800 Subject: Test against all possible cases, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18755739 --- spec/models/ci/pipeline_spec.rb | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 29e5693d5ab..cbf25c23b05 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -403,10 +403,10 @@ describe Ci::Pipeline, models: true do end describe '#cancelable?' do - %i[created running pending].each do |status| - context "when there is a build #{status}" do + %i[created running pending].each do |status0| + context "when there is a build #{status0}" do before do - create(:ci_build, status, pipeline: pipeline) + create(:ci_build, status0, pipeline: pipeline) end it 'is cancelable' do @@ -414,9 +414,9 @@ describe Ci::Pipeline, models: true do end end - context "when there is an external job #{status}" do + context "when there is an external job #{status0}" do before do - create(:generic_commit_status, status, pipeline: pipeline) + create(:generic_commit_status, status0, pipeline: pipeline) end it 'is cancelable' do @@ -424,16 +424,19 @@ describe Ci::Pipeline, models: true do end end - %i[success failed canceled].each do |status2| - context "when there are two builds for #{status} and #{status2}" do - before do - build = %i[ci_build generic_commit_status] - create(build.sample, status, pipeline: pipeline) - create(build.sample, status2, pipeline: pipeline) - end + %i[success failed canceled].each do |status1| + %i[ci_build generic_commit_status].each do |type0| + %i[ci_build generic_commit_status].each do |type1| + context "when there are #{type0} and #{type1} for #{status0} and #{status1}" do + before do + create(type0, status0, pipeline: pipeline) + create(type1, status1, pipeline: pipeline) + end - it 'is cancelable' do - expect(pipeline.cancelable?).to be_truthy + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end end end end -- cgit v1.2.1 From d20c0682ac4a91f9cdea1d71f9eb515755955c7a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 22 Nov 2016 13:41:39 +0800 Subject: Just create failed builds rather than calling events Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7617#note_18763033 --- spec/models/ci/pipeline_spec.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ea5e0e28c5c..ea022e03608 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -571,13 +571,10 @@ describe Ci::Pipeline, models: true do context 'with failed pipeline' do before do perform_enqueued_jobs do - create(:ci_build, :created, pipeline: pipeline) - create(:generic_commit_status, :created, pipeline: pipeline) + create(:ci_build, :failed, pipeline: pipeline) + create(:generic_commit_status, :failed, pipeline: pipeline) - pipeline.statuses.count.times do |offset| - # workaround race conditions - pipeline.statuses.offset(offset).first.drop - end + pipeline.drop end end -- cgit v1.2.1 From 3566965417b7921cdb301c44cfb308551cdc1e82 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 22 Nov 2016 18:55:00 +0800 Subject: Passing a user to retry_failed in tests Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18794547 --- spec/models/ci/pipeline_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index cbf25c23b05..03924a436de 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -503,7 +503,7 @@ describe Ci::Pipeline, models: true do create(:ci_build, :failed, name: 'build', pipeline: pipeline) create(:generic_commit_status, :failed, name: 'jenkins', pipeline: pipeline) - pipeline.retry_failed(nil) + pipeline.retry_failed(create(:user)) end it 'retries only build' do @@ -516,7 +516,7 @@ describe Ci::Pipeline, models: true do create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) create(:ci_build, :failed, name: 'jenkins', stage_idx: 1, pipeline: pipeline) - pipeline.retry_failed(nil) + pipeline.retry_failed(create(:user)) end it 'retries both builds' do @@ -529,7 +529,7 @@ describe Ci::Pipeline, models: true do create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline) create(:ci_build, :canceled, name: 'jenkins', stage_idx: 1, pipeline: pipeline) - pipeline.retry_failed(nil) + pipeline.retry_failed(create(:user)) end it 'retries both builds' do -- cgit v1.2.1 From cc43c9acc29117cd9ec3c25805e6c5ea874e5969 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 22 Nov 2016 19:07:12 +0800 Subject: Expand the loop and reduce overlapped conditions Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7508#note_18794681 --- spec/models/ci/pipeline_spec.rb | 42 +++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 03924a436de..438810d8206 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -425,18 +425,36 @@ describe Ci::Pipeline, models: true do end %i[success failed canceled].each do |status1| - %i[ci_build generic_commit_status].each do |type0| - %i[ci_build generic_commit_status].each do |type1| - context "when there are #{type0} and #{type1} for #{status0} and #{status1}" do - before do - create(type0, status0, pipeline: pipeline) - create(type1, status1, pipeline: pipeline) - end - - it 'is cancelable' do - expect(pipeline.cancelable?).to be_truthy - end - end + context "when there are generic_commit_status jobs for #{status0} and #{status1}" do + before do + create(:generic_commit_status, status0, pipeline: pipeline) + create(:generic_commit_status, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + context "when there are generic_commit_status and ci_build jobs for #{status0} and #{status1}" do + before do + create(:generic_commit_status, status0, pipeline: pipeline) + create(:ci_build, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy + end + end + + context "when there are ci_build jobs for #{status0} and #{status1}" do + before do + create(:ci_build, status0, pipeline: pipeline) + create(:ci_build, status1, pipeline: pipeline) + end + + it 'is cancelable' do + expect(pipeline.cancelable?).to be_truthy end end end -- cgit v1.2.1 From 819f459b69935f75cbe423884149564cf6cea001 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 30 Nov 2016 16:29:27 +0800 Subject: Only include EmailHelpers in mailer specs and specs using them --- spec/models/ci/pipeline_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 3b12f16b4db..0d2b4920835 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Ci::Pipeline, models: true do + include EmailHelpers + let(:project) { FactoryGirl.create :empty_project } let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, status: 'created', project: project } -- cgit v1.2.1 From 5e3cfe2f091a1803aa7d08238232aea846ebfb5a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 5 Dec 2016 13:23:58 +0100 Subject: Expose pipeline detailed status using status factory --- spec/models/ci/pipeline_spec.rb | 70 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0d2b4920835..3f93d9ddf19 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -404,6 +404,76 @@ describe Ci::Pipeline, models: true do end end + describe '#detailed_status' do + context 'when pipeline is created' do + let(:pipeline) { create(:ci_pipeline, status: :created) } + + it 'returns detailed status for created pipeline' do + expect(pipeline.detailed_status.text).to eq 'created' + end + end + + context 'when pipeline is pending' do + let(:pipeline) { create(:ci_pipeline, status: :pending) } + + it 'returns detailed status for pending pipeline' do + expect(pipeline.detailed_status.text).to eq 'pending' + end + end + + context 'when pipeline is running' do + let(:pipeline) { create(:ci_pipeline, status: :running) } + + it 'returns detailed status for running pipeline' do + expect(pipeline.detailed_status.text).to eq 'running' + end + end + + context 'when pipeline is successful' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + it 'returns detailed status for successful pipeline' do + expect(pipeline.detailed_status.text).to eq 'passed' + end + end + + context 'when pipeline is failed' do + let(:pipeline) { create(:ci_pipeline, status: :failed) } + + it 'returns detailed status for failed pipeline' do + expect(pipeline.detailed_status.text).to eq 'failed' + end + end + + context 'when pipeline is canceled' do + let(:pipeline) { create(:ci_pipeline, status: :canceled) } + + it 'returns detailed status for canceled pipeline' do + expect(pipeline.detailed_status.text).to eq 'canceled' + end + end + + context 'when pipeline is skipped' do + let(:pipeline) { create(:ci_pipeline, status: :skipped) } + + it 'returns detailed status for skipped pipeline' do + expect(pipeline.detailed_status.text).to eq 'skipped' + end + end + + context 'when pipeline is successful but with warnings' do + let(:pipeline) { create(:ci_pipeline, status: :success) } + + before do + create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline) + end + + it 'retruns detailed status for successful pipeline with warnings' do + expect(pipeline.detailed_status.label).to eq 'passed with warnings' + end + end + end + describe '#cancelable?' do %i[created running pending].each do |status0| context "when there is a build #{status0}" do -- cgit v1.2.1 From 260d754ca89c14297e0e360d35d7914d57e290bf Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 5 Dec 2016 17:52:50 +0100 Subject: Fix handling of allowed to failure jobs --- spec/models/ci/pipeline_spec.rb | 51 +++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 9 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 3f93d9ddf19..cdc858c13b4 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -20,8 +20,6 @@ describe Ci::Pipeline, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } - it { is_expected.to delegate_method(:stages).to(:statuses) } - describe '#valid_commit_sha' do context 'commit.sha can not start with 00000000' do before do @@ -125,16 +123,51 @@ describe Ci::Pipeline, models: true do end describe '#stages' do - let(:pipeline2) { FactoryGirl.create :ci_pipeline, project: project } - subject { CommitStatus.where(pipeline: [pipeline, pipeline2]).stages } - before do - FactoryGirl.create :ci_build, pipeline: pipeline2, stage: 'test', stage_idx: 1 - FactoryGirl.create :ci_build, pipeline: pipeline, stage: 'build', stage_idx: 0 + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'linux', stage_idx: 0, status: 'success') + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'failed') + create(:commit_status, pipeline: pipeline, stage: 'deploy', name: 'staging', stage_idx: 2, status: 'running') + create(:commit_status, pipeline: pipeline, stage: 'test', name: 'rspec', stage_idx: 1, status: 'success') end - it 'return all stages' do - is_expected.to eq(%w(build test)) + subject { pipeline.stages } + + context 'stages list' do + it 'returns ordered list of stages' do + expect(subject.map(&:name)).to eq(%w[build test deploy]) + end + end + + it 'returns a valid number of stages' do + expect(pipeline.stages_count).to eq(3) + end + + context 'stages with statuses' do + let(:statuses) do + subject.map do |stage| + [stage.name, stage.status] + end + end + + it 'returns list of stages with statuses' do + expect(statuses).to eq([['build', 'failed'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + + context 'when build is retried' do + before do + create(:commit_status, pipeline: pipeline, stage: 'build', name: 'mac', stage_idx: 0, status: 'success') + end + + it 'ignores the previous state' do + expect(statuses).to eq([['build', 'success'], + ['test', 'success'], + ['deploy', 'running'] + ]) + end + end end end -- cgit v1.2.1 From 953a10947bea8ff75290cd50fae4ae1f7636a71d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 6 Dec 2016 14:49:37 +0100 Subject: Added Ci::Stage specs --- spec/models/ci/pipeline_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index cdc858c13b4..8158e71dd55 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -142,6 +142,10 @@ describe Ci::Pipeline, models: true do expect(pipeline.stages_count).to eq(3) end + it 'returns a valid names of stages' do + expect(pipeline.stages_name).to eq(['build', 'test', 'deploy']) + end + context 'stages with statuses' do let(:statuses) do subject.map do |stage| -- cgit v1.2.1 From 48ea142819f206bb005184a050e812966cd23157 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 15:14:31 +0100 Subject: Fix pipeline specs for detailed statuses [ci skip] --- spec/models/ci/pipeline_spec.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8158e71dd55..e78ae14b737 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -442,11 +442,15 @@ describe Ci::Pipeline, models: true do end describe '#detailed_status' do + let(:user) { create(:user) } + + subject { pipeline.detailed_status(user) } + context 'when pipeline is created' do let(:pipeline) { create(:ci_pipeline, status: :created) } it 'returns detailed status for created pipeline' do - expect(pipeline.detailed_status.text).to eq 'created' + expect(subject.text).to eq 'created' end end @@ -454,7 +458,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :pending) } it 'returns detailed status for pending pipeline' do - expect(pipeline.detailed_status.text).to eq 'pending' + expect(subject.text).to eq 'pending' end end @@ -462,7 +466,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :running) } it 'returns detailed status for running pipeline' do - expect(pipeline.detailed_status.text).to eq 'running' + expect(subject.text).to eq 'running' end end @@ -470,7 +474,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :success) } it 'returns detailed status for successful pipeline' do - expect(pipeline.detailed_status.text).to eq 'passed' + expect(subject.text).to eq 'passed' end end @@ -478,7 +482,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :failed) } it 'returns detailed status for failed pipeline' do - expect(pipeline.detailed_status.text).to eq 'failed' + expect(subject.text).to eq 'failed' end end @@ -486,7 +490,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :canceled) } it 'returns detailed status for canceled pipeline' do - expect(pipeline.detailed_status.text).to eq 'canceled' + expect(subject.text).to eq 'canceled' end end @@ -494,7 +498,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :skipped) } it 'returns detailed status for skipped pipeline' do - expect(pipeline.detailed_status.text).to eq 'skipped' + expect(subject.text).to eq 'skipped' end end @@ -506,7 +510,7 @@ describe Ci::Pipeline, models: true do end it 'retruns detailed status for successful pipeline with warnings' do - expect(pipeline.detailed_status.label).to eq 'passed with warnings' + expect(subject.label).to eq 'passed with warnings' end end end -- cgit v1.2.1 From 7cced60069c248156decf6ceabc4d1f447e47ff7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Dec 2016 21:00:06 +0800 Subject: Introduce latest_status and add a few tests Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20003268 --- spec/models/ci/pipeline_spec.rb | 57 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e78ae14b737..21df5df1b76 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -381,6 +381,63 @@ describe Ci::Pipeline, models: true do end end + shared_context 'with some empty pipelines' do + before do + create_pipeline(:canceled, 'ref', 'A') + create_pipeline(:success, 'ref', 'A') + create_pipeline(:failed, 'ref', 'B') + create_pipeline(:skipped, 'feature', 'C') + end + + def create_pipeline(status, ref, sha) + create(:ci_empty_pipeline, status: status, ref: ref, sha: sha) + end + end + + describe '.latest' do + include_context 'with some empty pipelines' + + context 'when no ref is specified' do + let(:pipelines) { Ci::Pipeline.latest.all } + + it 'returns the latest pipeline for the same ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed', 'skipped') + end + end + + context 'when ref is specified' do + let(:pipelines) { Ci::Pipeline.latest('ref').all } + + it 'returns the latest pipeline for ref and different sha' do + expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') + expect(pipelines.map(&:status)). + to contain_exactly('success', 'failed') + end + end + end + + describe '.latest_status' do + include_context 'with some empty pipelines' + + context 'when no ref is specified' do + let(:latest_status) { Ci::Pipeline.latest_status } + + it 'returns the latest status for the same ref and different sha' do + expect(latest_status).to eq(Ci::Pipeline.latest.status) + end + end + + context 'when ref is specified' do + let(:latest_status) { Ci::Pipeline.latest_status('ref') } + + it 'returns the latest status for ref and different sha' do + expect(latest_status).to eq(Ci::Pipeline.latest_status('ref')) + end + end + end + describe '#status' do let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') } -- cgit v1.2.1 From cc6f578d5fc45f9c3d4cc7df5af72b15ce47b3a8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 18:14:48 +0800 Subject: Use described_class and update description Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059124 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059187 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059322 --- spec/models/ci/pipeline_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 21df5df1b76..d06c30a891c 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -381,7 +381,7 @@ describe Ci::Pipeline, models: true do end end - shared_context 'with some empty pipelines' do + shared_context 'with some outdated pipelines' do before do create_pipeline(:canceled, 'ref', 'A') create_pipeline(:success, 'ref', 'A') @@ -395,10 +395,10 @@ describe Ci::Pipeline, models: true do end describe '.latest' do - include_context 'with some empty pipelines' + include_context 'with some outdated pipelines' context 'when no ref is specified' do - let(:pipelines) { Ci::Pipeline.latest.all } + let(:pipelines) { described_class.latest.all } it 'returns the latest pipeline for the same ref and different sha' do expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') @@ -408,7 +408,7 @@ describe Ci::Pipeline, models: true do end context 'when ref is specified' do - let(:pipelines) { Ci::Pipeline.latest('ref').all } + let(:pipelines) { described_class.latest('ref').all } it 'returns the latest pipeline for ref and different sha' do expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') @@ -419,21 +419,21 @@ describe Ci::Pipeline, models: true do end describe '.latest_status' do - include_context 'with some empty pipelines' + include_context 'with some outdated pipelines' context 'when no ref is specified' do - let(:latest_status) { Ci::Pipeline.latest_status } + let(:latest_status) { described_class.latest_status } it 'returns the latest status for the same ref and different sha' do - expect(latest_status).to eq(Ci::Pipeline.latest.status) + expect(latest_status).to eq(described_class.latest.status) end end context 'when ref is specified' do - let(:latest_status) { Ci::Pipeline.latest_status('ref') } + let(:latest_status) { described_class.latest_status('ref') } it 'returns the latest status for ref and different sha' do - expect(latest_status).to eq(Ci::Pipeline.latest_status('ref')) + expect(latest_status).to eq(described_class.latest_status('ref')) end end end -- cgit v1.2.1 From d9000184e5e8a63e0c24fe264e864fc27f6515c4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 18:52:36 +0800 Subject: Add explicit status test, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20058993 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059060 https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059357 --- spec/models/ci/pipeline_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec/models/ci/pipeline_spec.rb') diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d06c30a891c..52dd41065e9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -426,6 +426,7 @@ describe Ci::Pipeline, models: true do it 'returns the latest status for the same ref and different sha' do expect(latest_status).to eq(described_class.latest.status) + expect(latest_status).to eq('failed') end end @@ -434,6 +435,7 @@ describe Ci::Pipeline, models: true do it 'returns the latest status for ref and different sha' do expect(latest_status).to eq(described_class.latest_status('ref')) + expect(latest_status).to eq('failed') end end end -- cgit v1.2.1