diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2017-11-07 12:52:52 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2017-11-07 12:52:52 +0000 |
commit | ad526918d360cbf5d8ab3efeb0eddf482b9e6e77 (patch) | |
tree | 2d40de788985057716f7cd3d7f04ff7e574fb7ff | |
parent | 5c136f1d717b037ed8a5c88273ed3ed760b0f1d3 (diff) | |
parent | 0d8b695569010ef958176c8ebf635e56d27d5d41 (diff) | |
download | gitlab-ce-ad526918d360cbf5d8ab3efeb0eddf482b9e6e77.tar.gz |
Merge branch 'fix/gb/ensure-that-job-belongs-to-stage' into 'master'
Make sure that every job has a stage assigned
Closes #37979
See merge request gitlab-org/gitlab-ce!14724
-rw-r--r-- | app/models/commit_status.rb | 12 | ||||
-rw-r--r-- | app/services/ci/ensure_stage_service.rb | 39 | ||||
-rw-r--r-- | spec/factories/commit_statuses.rb | 1 | ||||
-rw-r--r-- | spec/models/commit_status_spec.rb | 73 | ||||
-rw-r--r-- | spec/serializers/pipeline_details_entity_spec.rb | 2 | ||||
-rw-r--r-- | spec/support/cycle_analytics_helpers.rb | 1 |
6 files changed, 124 insertions, 4 deletions
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index f3888528940..6b07dbdf3ea 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -14,7 +14,6 @@ class CommitStatus < ActiveRecord::Base delegate :sha, :short_sha, to: :pipeline validates :pipeline, presence: true, unless: :importing? - validates :name, presence: true, unless: :importing? alias_attribute :author, :user @@ -46,6 +45,17 @@ class CommitStatus < ActiveRecord::Base runner_system_failure: 4 } + ## + # We still create some CommitStatuses outside of CreatePipelineService. + # + # These are pages deployments and external statuses. + # + before_create unless: :importing? do + Ci::EnsureStageService.new(project, user).execute(self) do |stage| + self.run_after_commit { StageUpdateWorker.perform_async(stage.id) } + end + end + state_machine :status do event :process do transition [:skipped, :manual] => :created diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb new file mode 100644 index 00000000000..dc2f49e8db1 --- /dev/null +++ b/app/services/ci/ensure_stage_service.rb @@ -0,0 +1,39 @@ +module Ci + ## + # We call this service everytime we persist a CI/CD job. + # + # In most cases a job should already have a stage assigned, but in cases it + # doesn't have we need to either find existing one or create a brand new + # stage. + # + class EnsureStageService < BaseService + def execute(build) + @build = build + + return if build.stage_id.present? + return if build.invalid? + + ensure_stage.tap do |stage| + build.stage_id = stage.id + + yield stage if block_given? + end + end + + private + + def ensure_stage + find_stage || create_stage + end + + def find_stage + @build.pipeline.stages.find_by(name: @build.stage) + end + + def create_stage + Ci::Stage.create!(name: @build.stage, + pipeline: @build.pipeline, + project: @build.project) + end + end +end diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index 169590deb8e..abbe37df90e 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -1,6 +1,7 @@ FactoryGirl.define do factory :commit_status, class: CommitStatus do name 'default' + stage 'test' status 'success' description 'commit status' pipeline factory: :ci_pipeline_with_one_job diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 858ec831200..c536dab2681 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe CommitStatus do - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository) } - let(:pipeline) do + set(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit.id) end @@ -464,4 +464,73 @@ describe CommitStatus do it { is_expected.to be_script_failure } end end + + describe 'ensure stage assignment' do + context 'when commit status has a stage_id assigned' do + let!(:stage) do + create(:ci_stage_entity, project: project, pipeline: pipeline) + end + + let(:commit_status) do + create(:commit_status, stage_id: stage.id, name: 'rspec', stage: 'test') + end + + it 'does not create a new stage' do + expect { commit_status }.not_to change { Ci::Stage.count } + expect(commit_status.stage_id).to eq stage.id + end + end + + context 'when commit status does not have a stage_id assigned' do + let(:commit_status) do + create(:commit_status, name: 'rspec', stage: 'test', status: :success) + end + + let(:stage) { Ci::Stage.first } + + it 'creates a new stage' do + expect { commit_status }.to change { Ci::Stage.count }.by(1) + + expect(stage.name).to eq 'test' + expect(stage.project).to eq commit_status.project + expect(stage.pipeline).to eq commit_status.pipeline + expect(stage.status).to eq commit_status.status + expect(commit_status.stage_id).to eq stage.id + end + end + + context 'when commit status does not have stage but it exists' do + let!(:stage) do + create(:ci_stage_entity, project: project, + pipeline: pipeline, + name: 'test') + end + + let(:commit_status) do + create(:commit_status, project: project, + pipeline: pipeline, + name: 'rspec', + stage: 'test', + status: :success) + end + + it 'uses existing stage' do + expect { commit_status }.not_to change { Ci::Stage.count } + + expect(commit_status.stage_id).to eq stage.id + expect(stage.reload.status).to eq commit_status.status + end + end + + context 'when commit status is being imported' do + let(:commit_status) do + create(:commit_status, name: 'rspec', stage: 'test', importing: true) + end + + it 'does not create a new stage' do + expect { commit_status }.not_to change { Ci::Stage.count } + expect(commit_status.stage_id).not_to be_present + end + end + end end diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index f60d1843581..45e18086894 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -107,7 +107,7 @@ describe PipelineDetailsEntity do it 'contains stages' do expect(subject).to include(:details) expect(subject[:details]).to include(:stages) - expect(subject[:details][:stages].first).to include(name: 'external') + expect(subject[:details][:stages].first).to include(name: 'test') end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 934b4557ba2..26fd271ce31 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -94,6 +94,7 @@ module CycleAnalyticsHelpers ref: 'master', tag: false, name: 'dummy', + stage: 'dummy', pipeline: dummy_pipeline, protected: false) end |