From 41b8dca877ba790cd56677dc6405e16b631f9854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 17 Jul 2019 01:36:49 +0200 Subject: [PATCH] Add specs for specifying pipeline behavior Adds specs for testing the new behavior of specifying a pipeline when POSTing a status. --- app/models/project.rb | 12 ++++++++-- .../21671-multiple-pipeline-status-api.yml | 2 +- doc/api/commits.md | 2 +- lib/api/commit_statuses.rb | 9 +++---- spec/models/project_spec.rb | 20 ++++++++++++++++ spec/requests/api/commit_statuses_spec.rb | 24 +++++++++++++++---- 6 files changed, 55 insertions(+), 14 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 2906aca75fc..aad24a5534c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1496,12 +1496,20 @@ class Project < ApplicationRecord !namespace.share_with_group_lock end - def pipeline_for(ref, sha = nil) + def pipeline_for(ref, sha = nil, id = nil) + if id.present? + pipelines_for(ref, sha).find_by(id: id) + else + pipelines_for(ref, sha).take + end + end + + def pipelines_for(ref, sha = nil) sha ||= commit(ref).try(:sha) return unless sha - ci_pipelines.order(id: :desc).find_by(sha: sha, ref: ref) + ci_pipelines.order(id: :desc).where(sha: sha, ref: ref) end def latest_successful_pipeline_for_default_branch diff --git a/changelogs/unreleased/21671-multiple-pipeline-status-api.yml b/changelogs/unreleased/21671-multiple-pipeline-status-api.yml index f87366803e2..b7b0f5fa0c7 100644 --- a/changelogs/unreleased/21671-multiple-pipeline-status-api.yml +++ b/changelogs/unreleased/21671-multiple-pipeline-status-api.yml @@ -1,5 +1,5 @@ --- title: Multiple pipeline support for Commit status -merge_request: 21671 +merge_request: 30828 author: Gaetan Semet type: changed diff --git a/doc/api/commits.md b/doc/api/commits.md index a6264897e5e..c0bf53c1db4 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -581,7 +581,7 @@ POST /projects/:id/statuses/:sha | `target_url` | string | no | The target URL to associate with this status | `description` | string | no | The short description of the status | `coverage` | float | no | The total code coverage -| `pipeline_id` | integer | no | The id of the pipeline to set status. Use in case of several pipeline on same sha. +| `pipeline_id` | integer | no | The ID of the pipeline to set status. Use in case of several pipeline on same SHA. ```bash curl --request POST --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/17/statuses/18f3e63d05582537db6d183d9d557be09e1f90c8?state=success" diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 61cf929bcdc..d58a5e214ed 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -52,7 +52,7 @@ module API optional :name, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' optional :context, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' optional :coverage, type: Float, desc: 'The total code coverage' - optional :pipeline_id, type: Integer, desc: 'An existing pipeline id, when multiple pipelines on the same commit sha have been triggered' + optional :pipeline_id, type: Integer, desc: 'An existing pipeline ID, when multiple pipelines on the same commit SHA have been triggered' end # rubocop: disable CodeReuse/ActiveRecord post ':id/statuses/:sha' do @@ -73,11 +73,8 @@ module API not_found! 'References for commit' unless ref name = params[:name] || params[:context] || 'default' - pipeline = if params[:pipeline_id] - @project.ci_pipelines.find_by(id: params[:pipeline_id]) - else - @project.pipeline_for(ref, commit.sha) - end + + pipeline = @project.pipeline_for(ref, commit.sha, params[:pipeline_id]) unless pipeline pipeline = @project.ci_pipelines.create!( diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 927c072be10..3a8ad5a3066 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1190,6 +1190,14 @@ describe Project do subject { project.pipeline_for('master', pipeline.sha) } it_behaves_like 'giving the correct pipeline' + + context 'with supplied id' do + let!(:other_pipeline) { create_pipeline(project) } + + subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) } + + it { is_expected.to eq(other_pipeline) } + end end context 'with implicit sha' do @@ -1199,6 +1207,18 @@ describe Project do end end + describe '#pipelines_for' do + let(:project) { create(:project, :repository) } + let!(:pipeline) { create_pipeline(project) } + let!(:other_pipeline) { create_pipeline(project) } + + context 'with implicit sha' do + subject { project.pipelines_for('master') } + + it { is_expected.to contain_exactly(pipeline, other_pipeline) } + end + end + describe '#builds_enabled' do let(:project) { create(:project) } diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index b5e45f99109..1be8883bd3c 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -8,10 +8,6 @@ describe API::CommitStatuses do let(:developer) { create_user(:developer) } let(:sha) { commit.id } - let(:commit_status) do - create(:commit_status, status: :pending, pipeline: pipeline) - end - describe "GET /projects/:id/repository/commits/:sha/statuses" do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } @@ -239,6 +235,26 @@ describe API::CommitStatuses do expect(CommitStatus.count).to eq 1 end end + + context 'when a pipeline id is specified' do + let!(:first_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + let!(:other_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + + subject do + post api(post_url, developer), params: { + pipeline_id: other_pipeline.id, + state: 'success', + ref: 'master' + } + end + + it 'update the correct pipeline' do + subject + + expect(first_pipeline.reload.status).to eq('created') + expect(other_pipeline.reload.status).to eq('success') + end + end end context 'when retrying a commit status' do