diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2016-11-17 00:57:50 +0800 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2016-11-17 20:22:57 +0800 |
commit | 6d1c5761cd520f3cb7fa4dbb1a1937c29f468884 (patch) | |
tree | d5edc1d78a744d62bbe6832b10a022ac12fc63a6 | |
parent | 891465ba8cd57bb928e82ba070f2d7efb63f6282 (diff) | |
download | gitlab-ce-6d1c5761cd520f3cb7fa4dbb1a1937c29f468884.tar.gz |
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
-rw-r--r-- | app/models/ci/pipeline.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/has_status.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/fix-cancelling-pipelines.yml | 4 | ||||
-rw-r--r-- | spec/features/projects/pipelines_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 40 |
5 files changed, 52 insertions, 4 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3fee6c18770..4eb85f62ee4 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -167,11 +167,11 @@ module Ci end def cancelable? - builds.running_or_pending.any? + statuses.cancelable.any? end def cancel_running - builds.running_or_pending.each(&:cancel) + statuses.cancelable.each(&:cancel) end def retry_failed(user) diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index ef3e73a4072..1332743429d 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -73,6 +73,10 @@ module HasStatus scope :skipped, -> { where(status: 'skipped') } scope :running_or_pending, -> { where(status: [:running, :pending]) } scope :finished, -> { where(status: [:success, :failed, :canceled]) } + + scope :cancelable, -> do + where(status: [:running, :pending, :created]) + end end def started? diff --git a/changelogs/unreleased/fix-cancelling-pipelines.yml b/changelogs/unreleased/fix-cancelling-pipelines.yml new file mode 100644 index 00000000000..c21e663093a --- /dev/null +++ b/changelogs/unreleased/fix-cancelling-pipelines.yml @@ -0,0 +1,4 @@ +--- +title: Fix cancelling created or external pipelines +merge_request: 7508 +author: diff --git a/spec/features/projects/pipelines_spec.rb b/spec/features/projects/pipelines_spec.rb index db56a50e058..b3ef9a11d56 100644 --- a/spec/features/projects/pipelines_spec.rb +++ b/spec/features/projects/pipelines_spec.rb @@ -90,8 +90,8 @@ describe "Pipelines" do visit namespace_project_pipelines_path(project.namespace, project) end - it 'is not cancelable' do - expect(page).not_to have_link('Cancel') + it 'is cancelable' do + expect(page).to have_link('Cancel') end it 'has pipeline running' do 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) } |