Commit f8de35f7 authored by Fabio Pitino's avatar Fabio Pitino

Merge branch '326476-CreatePipelineService-returns-ServiceResponse' into 'master'

Return ServiceResponse from  CreatePipelineService#execute [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!64318
parents 7bc94d1d 47b591b6
...@@ -68,20 +68,22 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -68,20 +68,22 @@ class Projects::PipelinesController < Projects::ApplicationController
end end
def create def create
@pipeline = Ci::CreatePipelineService service_response = Ci::CreatePipelineService
.new(project, current_user, create_params) .new(project, current_user, create_params)
.execute(:web, ignore_skip_ci: true, save_on_errors: false) .execute(:web, ignore_skip_ci: true, save_on_errors: false)
@pipeline = service_response.payload
respond_to do |format| respond_to do |format|
format.html do format.html do
if @pipeline.created_successfully? if service_response.success?
redirect_to project_pipeline_path(project, @pipeline) redirect_to project_pipeline_path(project, @pipeline)
else else
render 'new', status: :bad_request render 'new', status: :bad_request
end end
end end
format.json do format.json do
if @pipeline.created_successfully? if service_response.success?
render json: PipelineSerializer render json: PipelineSerializer
.new(project: project, current_user: current_user) .new(project: project, current_user: current_user)
.represent(@pipeline), .represent(@pipeline),
......
...@@ -33,8 +33,9 @@ module Ci ...@@ -33,8 +33,9 @@ module Ci
current_user, current_user,
pipeline_params.fetch(:target_revision)) pipeline_params.fetch(:target_revision))
downstream_pipeline = service.execute( downstream_pipeline = service
pipeline_params.fetch(:source), **pipeline_params[:execute_params]) .execute(pipeline_params.fetch(:source), **pipeline_params[:execute_params])
.payload
downstream_pipeline.tap do |pipeline| downstream_pipeline.tap do |pipeline|
update_bridge_status!(@bridge, pipeline) update_bridge_status!(@bridge, pipeline)
......
...@@ -87,12 +87,16 @@ module Ci ...@@ -87,12 +87,16 @@ module Ci
if pipeline.persisted? if pipeline.persisted?
schedule_head_pipeline_update schedule_head_pipeline_update
create_namespace_onboarding_action create_namespace_onboarding_action
else
# If pipeline is not persisted, try to recover IID
pipeline.reset_project_iid
end end
# If pipeline is not persisted, try to recover IID if error_message = pipeline.full_error_messages.presence || pipeline.failure_reason.presence
pipeline.reset_project_iid unless pipeline.persisted? ServiceResponse.error(message: error_message, payload: pipeline)
else
pipeline ServiceResponse.success(payload: pipeline)
end
end end
# rubocop: enable Metrics/ParameterLists # rubocop: enable Metrics/ParameterLists
...@@ -100,8 +104,8 @@ module Ci ...@@ -100,8 +104,8 @@ module Ci
source = args[0] source = args[0]
params = Hash(args[1]) params = Hash(args[1])
execute(source, **params, &block).tap do |pipeline| execute(source, **params, &block).tap do |response|
unless pipeline.persisted? unless response.payload.persisted?
raise CreateError, pipeline.full_error_messages raise CreateError, pipeline.full_error_messages
end end
end end
......
...@@ -7,7 +7,8 @@ module Ci ...@@ -7,7 +7,8 @@ module Ci
module ExternalPullRequests module ExternalPullRequests
class CreatePipelineService < BaseService class CreatePipelineService < BaseService
def execute(pull_request) def execute(pull_request)
return unless pull_request.open? && pull_request.actual_branch_head? return pull_request_not_open_error unless pull_request.open?
return pull_request_branch_error unless pull_request.actual_branch_head?
create_pipeline_for(pull_request) create_pipeline_for(pull_request)
end end
...@@ -26,6 +27,14 @@ module Ci ...@@ -26,6 +27,14 @@ module Ci
target_sha: pull_request.target_sha target_sha: pull_request.target_sha
} }
end end
def pull_request_not_open_error
ServiceResponse.error(message: 'The pull request is not opened', payload: nil)
end
def pull_request_branch_error
ServiceResponse.error(message: 'The source sha is not the head of the source branch', payload: nil)
end
end end
end end
end end
...@@ -27,13 +27,13 @@ module Ci ...@@ -27,13 +27,13 @@ module Ci
# this check is to not leak the presence of the project if user cannot read it # this check is to not leak the presence of the project if user cannot read it
return unless trigger.project == project return unless trigger.project == project
pipeline = Ci::CreatePipelineService response = Ci::CreatePipelineService
.new(project, trigger.owner, ref: params[:ref], variables_attributes: variables) .new(project, trigger.owner, ref: params[:ref], variables_attributes: variables)
.execute(:trigger, ignore_skip_ci: true) do |pipeline| .execute(:trigger, ignore_skip_ci: true) do |pipeline|
pipeline.trigger_requests.build(trigger: trigger) pipeline.trigger_requests.build(trigger: trigger)
end end
pipeline_service_response(pipeline) pipeline_service_response(response.payload)
end end
def pipeline_service_response(pipeline) def pipeline_service_response(pipeline)
...@@ -57,7 +57,7 @@ module Ci ...@@ -57,7 +57,7 @@ module Ci
# this check is to not leak the presence of the project if user cannot read it # this check is to not leak the presence of the project if user cannot read it
return unless can?(job.user, :read_project, project) return unless can?(job.user, :read_project, project)
pipeline = Ci::CreatePipelineService response = Ci::CreatePipelineService
.new(project, job.user, ref: params[:ref], variables_attributes: variables) .new(project, job.user, ref: params[:ref], variables_attributes: variables)
.execute(:pipeline, ignore_skip_ci: true) do |pipeline| .execute(:pipeline, ignore_skip_ci: true) do |pipeline|
source = job.sourced_pipelines.build( source = job.sourced_pipelines.build(
...@@ -69,7 +69,7 @@ module Ci ...@@ -69,7 +69,7 @@ module Ci
pipeline.source_pipeline = source pipeline.source_pipeline = source
end end
pipeline_service_response(pipeline) pipeline_service_response(response.payload)
end end
def job_from_token def job_from_token
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module MergeRequests module MergeRequests
class CreatePipelineService < MergeRequests::BaseService class CreatePipelineService < MergeRequests::BaseService
def execute(merge_request) def execute(merge_request)
return unless can_create_pipeline_for?(merge_request) return cannot_create_pipeline_error unless can_create_pipeline_for?(merge_request)
create_detached_merge_request_pipeline(merge_request) create_detached_merge_request_pipeline(merge_request)
end end
...@@ -60,6 +60,10 @@ module MergeRequests ...@@ -60,6 +60,10 @@ module MergeRequests
::Gitlab::UserAccess.new(current_user, container: merge_request.target_project) ::Gitlab::UserAccess.new(current_user, container: merge_request.target_project)
.can_update_branch?(merge_request.source_branch_ref) .can_update_branch?(merge_request.source_branch_ref)
end end
def cannot_create_pipeline_error
ServiceResponse.error(message: 'Cannot create a pipeline for this merge request.', payload: nil)
end
end end
end end
......
...@@ -24,7 +24,7 @@ module AppSec ...@@ -24,7 +24,7 @@ module AppSec
response = create_scan(dast_profile) response = create_scan(dast_profile)
return response if response.error? return error(response.message) if response.error?
success(dast_profile: dast_profile, pipeline_url: response.payload.fetch(:pipeline_url)) success(dast_profile: dast_profile, pipeline_url: response.payload.fetch(:pipeline_url))
end end
......
...@@ -7,7 +7,7 @@ module Ci ...@@ -7,7 +7,7 @@ module Ci
service = Ci::CreatePipelineService.new(project, current_user, ref: branch) service = Ci::CreatePipelineService.new(project, current_user, ref: branch)
pipeline = service.execute(:ondemand_dast_scan, content: ci_configuration) do |pipeline| response = service.execute(:ondemand_dast_scan, content: ci_configuration) do |pipeline|
if dast_profile if dast_profile
pipeline.dast_profile = dast_profile pipeline.dast_profile = dast_profile
else else
...@@ -16,6 +16,7 @@ module Ci ...@@ -16,6 +16,7 @@ module Ci
end end
end end
pipeline = response.payload
if pipeline.created_successfully? if pipeline.created_successfully?
ServiceResponse.success(payload: pipeline) ServiceResponse.success(payload: pipeline)
else else
......
...@@ -7,17 +7,19 @@ module EE ...@@ -7,17 +7,19 @@ module EE
override :execute override :execute
def execute(merge_request) def execute(merge_request)
create_merged_result_pipeline_for(merge_request) || super response = create_merged_result_pipeline_for(merge_request)
return response if response.success?
super
end end
def create_merged_result_pipeline_for(merge_request) def create_merged_result_pipeline_for(merge_request)
return unless can_create_merged_result_pipeline_for?(merge_request) return cannot_create_pipeline_error unless can_create_merged_result_pipeline_for?(merge_request)
result = ::MergeRequests::MergeabilityCheckService.new(merge_request).execute(recheck: true) result = ::MergeRequests::MergeabilityCheckService.new(merge_request).execute(recheck: true)
if result.success? && if result.success? && merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
ref_payload = result.payload.fetch(:merge_ref_head) ref_payload = result.payload.fetch(:merge_ref_head)
::Ci::CreatePipelineService.new(merge_request.target_project, current_user, ::Ci::CreatePipelineService.new(merge_request.target_project, current_user,
...@@ -26,6 +28,8 @@ module EE ...@@ -26,6 +28,8 @@ module EE
target_sha: ref_payload[:target_id], target_sha: ref_payload[:target_id],
source_sha: ref_payload[:source_id]) source_sha: ref_payload[:source_id])
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
else
cannot_create_pipeline_error
end end
end end
......
...@@ -39,16 +39,16 @@ module MergeTrains ...@@ -39,16 +39,16 @@ module MergeTrains
end end
def create_pipeline(merge_request, merge_status) def create_pipeline(merge_request, merge_status)
pipeline = ::Ci::CreatePipelineService.new(merge_request.target_project, merge_request.merge_user, response = ::Ci::CreatePipelineService.new(merge_request.target_project, merge_request.merge_user,
ref: merge_request.train_ref_path, ref: merge_request.train_ref_path,
checkout_sha: merge_status[:commit_id], checkout_sha: merge_status[:commit_id],
target_sha: merge_status[:target_id], target_sha: merge_status[:target_id],
source_sha: merge_status[:source_id]) source_sha: merge_status[:source_id])
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
return error(pipeline.full_error_messages) unless pipeline.persisted? return error(response.message) if response.error? && !response.payload.persisted?
success(pipeline: pipeline) success(pipeline: response.payload)
end end
def can_add_to_merge_train?(merge_request) def can_add_to_merge_train?(merge_request)
......
...@@ -29,7 +29,7 @@ RSpec.describe 'Jobs/Browser-Performance-Testing.gitlab-ci.yml' do ...@@ -29,7 +29,7 @@ RSpec.describe 'Jobs/Browser-Performance-Testing.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -77,7 +77,7 @@ RSpec.describe 'Jobs/Browser-Performance-Testing.gitlab-ci.yml' do ...@@ -77,7 +77,7 @@ RSpec.describe 'Jobs/Browser-Performance-Testing.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -32,7 +32,7 @@ RSpec.describe 'Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml' do ...@@ -32,7 +32,7 @@ RSpec.describe 'Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -29,7 +29,7 @@ RSpec.describe 'Jobs/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -29,7 +29,7 @@ RSpec.describe 'Jobs/Load-Performance-Testing.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -75,7 +75,7 @@ RSpec.describe 'Jobs/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -75,7 +75,7 @@ RSpec.describe 'Jobs/Load-Performance-Testing.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Browser-Performance.gitlab-ci.yml' do ...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Browser-Performance.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -66,7 +66,7 @@ RSpec.describe 'Verify/Browser-Performance.gitlab-ci.yml' do ...@@ -66,7 +66,7 @@ RSpec.describe 'Verify/Browser-Performance.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -65,7 +65,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -65,7 +65,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -35,7 +35,7 @@ RSpec.describe 'API-Fuzzing.gitlab-ci.yml' do ...@@ -35,7 +35,7 @@ RSpec.describe 'API-Fuzzing.gitlab-ci.yml' do
let(:pipeline_branch) { default_branch } let(:pipeline_branch) { default_branch }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
context 'when no stages' do context 'when no stages' do
......
...@@ -35,7 +35,7 @@ RSpec.describe 'API-Fuzzing.latest.gitlab-ci.yml' do ...@@ -35,7 +35,7 @@ RSpec.describe 'API-Fuzzing.latest.gitlab-ci.yml' do
let(:pipeline_branch) { default_branch } let(:pipeline_branch) { default_branch }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
context 'when no stages' do context 'when no stages' do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'Cluster-Image-Scanning.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'Cluster-Image-Scanning.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master' ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master' ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'Container-Scanning.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'Container-Scanning.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master' ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master' ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -35,7 +35,7 @@ RSpec.describe 'DAST-API.gitlab-ci.yml' do ...@@ -35,7 +35,7 @@ RSpec.describe 'DAST-API.gitlab-ci.yml' do
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'DAST.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'DAST.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: { 'README.txt' => '' }) } let(:project) { create(:project, :custom_repo, files: { 'README.txt' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
let(:ci_pipeline_yaml) { "stages: [\"dast\"]\n" } let(:ci_pipeline_yaml) { "stages: [\"dast\"]\n" }
......
...@@ -23,7 +23,7 @@ RSpec.describe 'DAST.latest.gitlab-ci.yml' do ...@@ -23,7 +23,7 @@ RSpec.describe 'DAST.latest.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: { 'README.txt' => '' }) } let(:project) { create(:project, :custom_repo, files: { 'README.txt' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
let(:ci_pipeline_yaml) { "stages: [\"dast\"]\n" } let(:ci_pipeline_yaml) { "stages: [\"dast\"]\n" }
......
...@@ -11,7 +11,7 @@ RSpec.describe 'Dependency-Scanning.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'Dependency-Scanning.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: files) } let(:project) { create(:project, :custom_repo, files: files) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') } let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'License-Scanning.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'License-Scanning.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') } let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'SAST.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'SAST.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: files) } let(:project) { create(:project, :custom_repo, files: files) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') } let(:service) { Ci::CreatePipelineService.new(project, user, ref: 'master') }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -14,7 +14,7 @@ RSpec.describe 'Secure-Binaries.gitlab-ci.yml' do ...@@ -14,7 +14,7 @@ RSpec.describe 'Secure-Binaries.gitlab-ci.yml' do
let(:pipeline_branch) { default_branch } let(:pipeline_branch) { default_branch }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -32,14 +32,14 @@ RSpec.describe 'Running a DAST Profile' do ...@@ -32,14 +32,14 @@ RSpec.describe 'Running a DAST Profile' do
end end
context 'when pipeline creation fails' do context 'when pipeline creation fails' do
let(:fake_pipeline) { instance_double('Ci::Pipeline', created_successfully?: false, full_error_messages: 'full error messages') }
let(:fake_service) { instance_double('Ci::CreatePipelineService', execute: ServiceResponse.error(message: 'error message', payload: fake_pipeline)) }
before do before do
allow_next_instance_of(Ci::Pipeline) do |instance| allow(Ci::CreatePipelineService).to receive(:new).and_return(fake_service)
allow(instance).to receive(:created_successfully?).and_return(false)
allow(instance).to receive(:full_error_messages).and_return('error message')
end
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['error message'] it_behaves_like 'a mutation that returns errors in the response', errors: ['full error messages']
end end
end end
end end
...@@ -63,12 +63,14 @@ RSpec.describe 'Running a DAST Scan' do ...@@ -63,12 +63,14 @@ RSpec.describe 'Running a DAST Scan' do
end end
context 'when pipeline creation fails' do context 'when pipeline creation fails' do
let(:fake_pipeline) { instance_double('Ci::Pipeline', created_successfully?: false, full_error_messages: 'full error messages') }
let(:fake_service) { instance_double('Ci::CreatePipelineService', execute: ServiceResponse.error(message: 'error message', payload: fake_pipeline)) }
before do before do
allow_any_instance_of(Ci::Pipeline).to receive(:created_successfully?).and_return(false) allow(Ci::CreatePipelineService).to receive(:new).and_return(fake_service)
allow_any_instance_of(Ci::Pipeline).to receive(:full_error_messages).and_return('error message')
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['error message'] it_behaves_like 'a mutation that returns errors in the response', errors: ['full error messages']
end end
end end
end end
...@@ -41,12 +41,16 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -41,12 +41,16 @@ RSpec.describe Ci::CreatePipelineService do
compliance_project.add_maintainer(project.owner) compliance_project.add_maintainer(project.owner)
end end
it 'responds with success' do
is_expected.to be_success
end
it 'persists pipeline' do it 'persists pipeline' do
is_expected.to be_persisted expect(execute.payload).to be_persisted
end end
it 'sets the correct source' do it 'sets the correct source' do
expect(execute.config_source).to eq("compliance_source") expect(execute.payload.config_source).to eq("compliance_source")
end end
it 'persists jobs' do it 'persists jobs' do
...@@ -54,13 +58,13 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -54,13 +58,13 @@ RSpec.describe Ci::CreatePipelineService do
end end
it do it do
expect(execute.processables.map(&:name)).to contain_exactly('compliance_build', 'compliance_test') expect(execute.payload.processables.map(&:name)).to contain_exactly('compliance_build', 'compliance_test')
end end
end end
context 'when user does not have access to compliance project' do context 'when user does not have access to compliance project' do
it 'includes access denied error' do it 'includes access denied error' do
expect(execute.yaml_errors).to eq "Project `compliance/hippa` not found or access denied!" expect(execute.payload.yaml_errors).to eq "Project `compliance/hippa` not found or access denied!"
end end
it 'does not persist jobs' do it 'does not persist jobs' do
......
...@@ -36,10 +36,10 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -36,10 +36,10 @@ RSpec.describe Ci::CreatePipelineService do
] ]
end end
let(:build_job) { subject.builds.find_by!(name: :build_job) } let(:build_job) { subject.payload.builds.find_by!(name: :build_job) }
it 'persists pipeline' do it 'persists pipeline' do
is_expected.to be_persisted expect(subject.payload).to be_persisted
end end
it 'persists job' do it 'persists job' do
...@@ -124,10 +124,10 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -124,10 +124,10 @@ RSpec.describe Ci::CreatePipelineService do
%w[build_job_1] %w[build_job_1]
end end
let(:test_job) { subject.builds.find_by!(name: :test_job) } let(:test_job) { subject.payload.builds.find_by!(name: :test_job) }
it 'persists pipeline' do it 'persists pipeline' do
is_expected.to be_persisted expect(subject.payload).to be_persisted
end end
it 'persists jobs' do it 'persists jobs' do
...@@ -175,11 +175,11 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -175,11 +175,11 @@ RSpec.describe Ci::CreatePipelineService do
end end
it 'persists pipeline' do it 'persists pipeline' do
is_expected.to be_persisted expect(subject.payload).to be_persisted
end end
it 'has errors' do it 'has errors' do
expect(subject.yaml_errors) expect(subject.payload.yaml_errors)
.to include('jobs:build_job:needs:need ref should be a string') .to include('jobs:build_job:needs:need ref should be a string')
end end
end end
......
...@@ -50,7 +50,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -50,7 +50,7 @@ RSpec.describe Ci::CreatePipelineService do
.to_runner_variables .to_runner_variables
end end
subject { described_class.new(project, user, ref: 'refs/heads/master').execute(:push) } subject { described_class.new(project, user, ref: 'refs/heads/master').execute(:push).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -126,6 +126,6 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -126,6 +126,6 @@ RSpec.describe Ci::CreatePipelineService do
end end
def create_pipeline! def create_pipeline!
service.execute(:push) service.execute(:push).payload
end end
end end
...@@ -33,8 +33,8 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do ...@@ -33,8 +33,8 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
end end
it 'drops builds that match shared runners' do it 'drops builds that match shared runners', :aggregate_failures do
pipeline = create_pipeline! pipeline = create_pipeline
job1 = pipeline.builds.find_by_name('job1') job1 = pipeline.builds.find_by_name('job1')
job2 = pipeline.builds.find_by_name('job2') job2 = pipeline.builds.find_by_name('job2')
...@@ -49,8 +49,8 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do ...@@ -49,8 +49,8 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do
create(:ci_runner, :project, :online, projects: [project]) create(:ci_runner, :project, :online, projects: [project])
end end
it 'does not drop the builds' do it 'does not drop the builds', :aggregate_failures do
pipeline = create_pipeline! pipeline = create_pipeline
job1 = pipeline.builds.find_by_name('job1') job1 = pipeline.builds.find_by_name('job1')
job2 = pipeline.builds.find_by_name('job2') job2 = pipeline.builds.find_by_name('job2')
...@@ -60,7 +60,7 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do ...@@ -60,7 +60,7 @@ RSpec.describe Ci::CreatePipelineService, :sidekiq_inline do
end end
end end
def create_pipeline! def create_pipeline
service.execute(:push) service.execute(:push).payload
end end
end end
...@@ -29,9 +29,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -29,9 +29,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
describe 'CI/CD Quotas / Limits' do describe 'CI/CD Quotas / Limits' do
context 'when there are not limits enabled' do context 'when there are not limits enabled' do
it 'enqueues a new pipeline' do it 'enqueues a new pipeline', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_success
expect(pipeline).to be_created_successfully expect(pipeline).to be_created_successfully
end end
end end
...@@ -44,9 +45,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -44,9 +45,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
create(:ci_pipeline, project: project, status: 'running') create(:ci_pipeline, project: project, status: 'running')
end end
it 'drops the pipeline and does not process jobs' do it 'drops the pipeline and does not process jobs', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_error
expect(pipeline).to be_persisted expect(pipeline).to be_persisted
expect(pipeline).to be_failed expect(pipeline).to be_failed
expect(pipeline.statuses).not_to be_empty expect(pipeline.statuses).not_to be_empty
...@@ -60,9 +62,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -60,9 +62,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
plan_limits.update_column(:ci_pipeline_size, 2) plan_limits.update_column(:ci_pipeline_size, 2)
end end
it 'drops pipeline without creating jobs' do it 'drops pipeline without creating jobs', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_error
expect(pipeline).to be_persisted expect(pipeline).to be_persisted
expect(pipeline).to be_failed expect(pipeline).to be_failed
expect(pipeline.statuses).to be_empty expect(pipeline.statuses).to be_empty
...@@ -85,12 +88,13 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -85,12 +88,13 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
YAML YAML
end end
it 'creates bridge jobs correctly' do it 'creates bridge jobs correctly', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
test = pipeline.statuses.find_by(name: 'test') test = pipeline.statuses.find_by(name: 'test')
bridge = pipeline.statuses.find_by(name: 'deploy') bridge = pipeline.statuses.find_by(name: 'deploy')
expect(response).to be_success
expect(pipeline).to be_persisted expect(pipeline).to be_persisted
expect(test).to be_a Ci::Build expect(test).to be_a Ci::Build
expect(bridge).to be_a Ci::Bridge expect(bridge).to be_a Ci::Bridge
...@@ -124,7 +128,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -124,7 +128,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
context 'that include the bridge job' do context 'that include the bridge job' do
it 'persists the bridge job' do it 'persists the bridge job' do
pipeline = create_pipeline! _, pipeline = create_pipeline!
expect(pipeline.processables.pluck(:name)).to contain_exactly('hello', 'bridge-job') expect(pipeline.processables.pluck(:name)).to contain_exactly('hello', 'bridge-job')
end end
...@@ -134,7 +138,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -134,7 +138,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
let(:ref_name) { 'refs/heads/wip' } let(:ref_name) { 'refs/heads/wip' }
it 'does not include the bridge job' do it 'does not include the bridge job' do
pipeline = create_pipeline! _, pipeline = create_pipeline!
expect(pipeline.processables.pluck(:name)).to eq(%w[hello]) expect(pipeline.processables.pluck(:name)).to eq(%w[hello])
end end
...@@ -154,9 +158,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -154,9 +158,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
YAML YAML
end end
it 'persists secrets as job metadata' do it 'persists secrets as job metadata', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_success
expect(pipeline).to be_persisted expect(pipeline).to be_persisted
build = Ci::Build.find(pipeline.builds.first.id) build = Ci::Build.find(pipeline.builds.first.id)
...@@ -175,9 +180,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -175,9 +180,10 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
describe 'credit card requirement' do describe 'credit card requirement' do
shared_examples 'creates a successful pipeline' do shared_examples 'creates a successful pipeline' do
it 'creates a successful pipeline' do it 'creates a successful pipeline', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_success
expect(pipeline).to be_created_successfully expect(pipeline).to be_created_successfully
end end
end end
...@@ -200,7 +206,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -200,7 +206,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
context 'when user does not have credit card' do context 'when user does not have credit card' do
it 'creates a pipeline with errors', :aggregate_failures do it 'creates a pipeline with errors', :aggregate_failures do
pipeline = create_pipeline! _, pipeline = create_pipeline!
expect(pipeline).not_to be_created_successfully expect(pipeline).not_to be_created_successfully
expect(pipeline.failure_reason).to eq('user_not_verified') expect(pipeline.failure_reason).to eq('user_not_verified')
...@@ -212,8 +218,9 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -212,8 +218,9 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
end end
it 'does not create a pipeline', :aggregate_failures do it 'does not create a pipeline', :aggregate_failures do
pipeline = create_pipeline! response, pipeline = create_pipeline!
expect(response).to be_error
expect(pipeline).not_to be_persisted expect(pipeline).not_to be_persisted
end end
end end
...@@ -235,6 +242,8 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -235,6 +242,8 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
end end
def create_pipeline! def create_pipeline!
service.execute(:push) response = service.execute(:push)
[response, response.payload]
end end
end end
...@@ -185,19 +185,20 @@ RSpec.describe Ci::RunDastScanService do ...@@ -185,19 +185,20 @@ RSpec.describe Ci::RunDastScanService do
end end
context 'when the pipeline fails to save' do context 'when the pipeline fails to save' do
let(:fake_pipeline) { instance_double 'Ci::Pipeline', created_successfully?: false, full_error_messages: 'Fake full error messages' }
let(:fake_response) { ServiceResponse.error(message: 'Fake error message', payload: fake_pipeline) }
let(:fake_service) { instance_double "Ci::CreatePipelineService", execute: fake_response }
before do before do
allow_any_instance_of(Ci::Pipeline).to receive(:created_successfully?).and_return(false) allow(Ci::CreatePipelineService).to receive(:new).and_return(fake_service)
allow_any_instance_of(Ci::Pipeline).to receive(:full_error_messages).and_return(full_error_messages)
end end
let(:full_error_messages) { SecureRandom.hex }
it 'returns an error status' do it 'returns an error status' do
expect(status).to eq(:error) expect(status).to eq(:error)
end end
it 'populates message' do it 'populates message' do
expect(message).to eq(full_error_messages) expect(message).to eq('Fake full error messages')
end end
end end
......
...@@ -52,6 +52,11 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_ ...@@ -52,6 +52,11 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_
expect(merge_request.all_pipelines.last).not_to be_merged_result_pipeline expect(merge_request.all_pipelines.last).not_to be_merged_result_pipeline
expect(merge_request.all_pipelines.last).to be_detached_merge_request_pipeline expect(merge_request.all_pipelines.last).to be_detached_merge_request_pipeline
end end
it 'responds with success', :sidekiq_might_not_need_inline, :aggregate_failures do
expect(subject).to be_success
expect(subject.payload).to eq(Ci::Pipeline.last)
end
end end
it 'creates a merge request pipeline', :sidekiq_might_not_need_inline do it 'creates a merge request pipeline', :sidekiq_might_not_need_inline do
...@@ -62,6 +67,11 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_ ...@@ -62,6 +67,11 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_
expect(merge_request.all_pipelines.last).not_to be_detached_merge_request_pipeline expect(merge_request.all_pipelines.last).not_to be_detached_merge_request_pipeline
end end
it 'responds with success', :sidekiq_might_not_need_inline, :aggregate_failures do
expect(subject).to be_success
expect(subject.payload).to eq(Ci::Pipeline.last)
end
context 'when merge request is WIP' do context 'when merge request is WIP' do
before do before do
merge_request.update!(title: merge_request.wip_title) merge_request.update!(title: merge_request.wip_title)
...@@ -128,11 +138,16 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_ ...@@ -128,11 +138,16 @@ RSpec.describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_
context 'when .gitlab-ci.yml is invalid' do context 'when .gitlab-ci.yml is invalid' do
let(:ci_yaml) { 'invalid yaml file' } let(:ci_yaml) { 'invalid yaml file' }
it 'persists a pipeline with a config error' do it 'persists a pipeline with a config error', :aggregate_failures do
expect { subject }.to change { Ci::Pipeline.count }.by(1) expect { subject }.to change { Ci::Pipeline.count }.by(1)
expect(merge_request.all_pipelines.last).to be_failed expect(merge_request.all_pipelines.last).to be_failed
expect(merge_request.all_pipelines.last).to be_config_error expect(merge_request.all_pipelines.last).to be_config_error
end end
it 'responds with error', :aggregate_failures do
expect(subject).to be_error
expect(subject.message).to eq('Cannot create a pipeline for this merge request.')
end
end end
end end
end end
...@@ -78,12 +78,11 @@ module API ...@@ -78,12 +78,11 @@ module API
.merge(variables_attributes: params[:variables]) .merge(variables_attributes: params[:variables])
.except(:variables) .except(:variables)
new_pipeline = ::Ci::CreatePipelineService.new(user_project, response = ::Ci::CreatePipelineService.new(user_project, current_user, pipeline_params)
current_user, .execute(:api, ignore_skip_ci: true, save_on_errors: false)
pipeline_params) new_pipeline = response.payload
.execute(:api, ignore_skip_ci: true, save_on_errors: false)
if new_pipeline.persisted? if response.success?
present new_pipeline, with: Entities::Ci::Pipeline present new_pipeline, with: Entities::Ci::Pipeline
else else
render_validation_error!(new_pipeline) render_validation_error!(new_pipeline)
......
...@@ -404,6 +404,7 @@ module API ...@@ -404,6 +404,7 @@ module API
pipeline = ::MergeRequests::CreatePipelineService pipeline = ::MergeRequests::CreatePipelineService
.new(project: user_project, current_user: current_user, params: { allow_duplicate: true }) .new(project: user_project, current_user: current_user, params: { allow_duplicate: true })
.execute(find_merge_request_with_access(params[:merge_request_iid])) .execute(find_merge_request_with_access(params[:merge_request_iid]))
.payload
if pipeline.nil? if pipeline.nil?
not_allowed! not_allowed!
......
...@@ -54,10 +54,12 @@ module Gitlab ...@@ -54,10 +54,12 @@ module Gitlab
} }
) )
service.execute(:chat) do |pipeline| response = service.execute(:chat) do |pipeline|
build_environment_variables(pipeline) build_environment_variables(pipeline)
build_chat_data(pipeline) build_chat_data(pipeline)
end end
response.payload
end end
# pipeline - The `Ci::Pipeline` to create the environment variables for. # pipeline - The `Ci::Pipeline` to create the environment variables for.
......
...@@ -38,6 +38,7 @@ module Gitlab ...@@ -38,6 +38,7 @@ module Gitlab
pipeline = ::Ci::CreatePipelineService pipeline = ::Ci::CreatePipelineService
.new(@project, @current_user, ref: @project.default_branch) .new(@project, @current_user, ref: @project.default_branch)
.execute(:push, dry_run: true, content: content) .execute(:push, dry_run: true, content: content)
.payload
Result.new( Result.new(
jobs: dry_run_convert_to_jobs(pipeline.stages), jobs: dry_run_convert_to_jobs(pipeline.stages),
......
...@@ -43,12 +43,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -43,12 +43,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
let!(:push_pipeline) do let!(:push_pipeline) do
Ci::CreatePipelineService.new(project, user, ref: 'feature') Ci::CreatePipelineService.new(project, user, ref: 'feature')
.execute(:push) .execute(:push)
.payload
end end
let!(:detached_merge_request_pipeline) do let!(:detached_merge_request_pipeline) do
Ci::CreatePipelineService.new(project, user, ref: 'feature') Ci::CreatePipelineService.new(project, user, ref: 'feature')
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
.payload
end end
before do before do
...@@ -77,12 +79,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -77,12 +79,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
context 'when a user updated a merge request in the parent project', :sidekiq_might_not_need_inline do context 'when a user updated a merge request in the parent project', :sidekiq_might_not_need_inline do
let!(:push_pipeline_2) do let!(:push_pipeline_2) do
Ci::CreatePipelineService.new(project, user, ref: 'feature') Ci::CreatePipelineService.new(project, user, ref: 'feature')
.execute(:push) .execute(:push)
.payload
end end
let!(:detached_merge_request_pipeline_2) do let!(:detached_merge_request_pipeline_2) do
Ci::CreatePipelineService.new(project, user, ref: 'feature') Ci::CreatePipelineService.new(project, user, ref: 'feature')
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
.payload
end end
before do before do
...@@ -222,12 +226,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -222,12 +226,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
let!(:push_pipeline) do let!(:push_pipeline) do
Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature') Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature')
.execute(:push) .execute(:push)
.payload
end end
let!(:detached_merge_request_pipeline) do let!(:detached_merge_request_pipeline) do
Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature') Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature')
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
.payload
end end
let(:forked_project) { fork_project(project, user2, repository: true) } let(:forked_project) { fork_project(project, user2, repository: true) }
...@@ -267,12 +273,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -267,12 +273,14 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
context 'when a user updated a merge request from a forked project to the parent project' do context 'when a user updated a merge request from a forked project to the parent project' do
let!(:push_pipeline_2) do let!(:push_pipeline_2) do
Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature') Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature')
.execute(:push) .execute(:push)
.payload
end end
let!(:detached_merge_request_pipeline_2) do let!(:detached_merge_request_pipeline_2) do
Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature') Ci::CreatePipelineService.new(forked_project, user2, ref: 'feature')
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
.payload
end end
before do before do
......
...@@ -245,7 +245,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do ...@@ -245,7 +245,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
threads << Thread.new do threads << Thread.new do
Sidekiq::Worker.skipping_transaction_check do Sidekiq::Worker.skipping_transaction_check do
@pipeline = Ci::CreatePipelineService.new(project, user, build_push_data).execute(:push) @pipeline = Ci::CreatePipelineService.new(project, user, build_push_data).execute(:push).payload
end end
end end
......
...@@ -12,7 +12,7 @@ RSpec.describe '5-Minute-Production-App.gitlab-ci.yml' do ...@@ -12,7 +12,7 @@ RSpec.describe '5-Minute-Production-App.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_branch) { default_branch } let(:pipeline_branch) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -11,7 +11,7 @@ RSpec.describe 'Deploy-ECS.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'Deploy-ECS.gitlab-ci.yml' do
let(:project) { create(:project, :auto_devops, :custom_repo, files: { 'README.md' => '' }) } let(:project) { create(:project, :auto_devops, :custom_repo, files: { 'README.md' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
let(:platform_target) { 'ECS' } let(:platform_target) { 'ECS' }
......
...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Build.gitlab-ci.yml' do ...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Build.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Build.gitlab-ci.yml' do ...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Build.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do ...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do ...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -33,7 +33,7 @@ RSpec.describe 'Jobs/Deploy.gitlab-ci.yml' do ...@@ -33,7 +33,7 @@ RSpec.describe 'Jobs/Deploy.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -210,7 +210,7 @@ RSpec.describe 'Jobs/Deploy.gitlab-ci.yml' do ...@@ -210,7 +210,7 @@ RSpec.describe 'Jobs/Deploy.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do ...@@ -12,7 +12,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do ...@@ -47,7 +47,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -11,7 +11,7 @@ RSpec.describe 'Terraform/Base.latest.gitlab-ci.yml' do ...@@ -11,7 +11,7 @@ RSpec.describe 'Terraform/Base.latest.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) } let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -25,7 +25,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do
let(:default_branch) { 'master' } let(:default_branch) { 'master' }
let(:pipeline_ref) { default_branch } let(:pipeline_ref) { default_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -64,7 +64,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do ...@@ -64,7 +64,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do
context 'on merge request' do context 'on merge request' do
let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project) }
let(:pipeline) { service.execute(merge_request) } let(:pipeline) { service.execute(merge_request).payload }
it 'has no jobs' do it 'has no jobs' do
expect(pipeline).to be_merge_request_event expect(pipeline).to be_merge_request_event
......
...@@ -17,7 +17,7 @@ RSpec.describe 'Auto-DevOps.gitlab-ci.yml' do ...@@ -17,7 +17,7 @@ RSpec.describe 'Auto-DevOps.gitlab-ci.yml' do
let(:project) { create(:project, :auto_devops, :custom_repo, files: { 'README.md' => '' }) } let(:project) { create(:project, :auto_devops, :custom_repo, files: { 'README.md' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
...@@ -264,7 +264,7 @@ RSpec.describe 'Auto-DevOps.gitlab-ci.yml' do ...@@ -264,7 +264,7 @@ RSpec.describe 'Auto-DevOps.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: files) } let(:project) { create(:project, :custom_repo, files: files) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: default_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: default_branch ) }
let(:pipeline) { service.execute(:push) } let(:pipeline) { service.execute(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -10,7 +10,7 @@ RSpec.describe 'Flutter.gitlab-ci.yml' do ...@@ -10,7 +10,7 @@ RSpec.describe 'Flutter.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) } let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -10,7 +10,7 @@ RSpec.describe 'Managed-Cluster-Applications.gitlab-ci.yml' do ...@@ -10,7 +10,7 @@ RSpec.describe 'Managed-Cluster-Applications.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, namespace: user.namespace, files: { 'README.md' => '' }) } let(:project) { create(:project, :custom_repo, namespace: user.namespace, files: { 'README.md' => '' }) }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
let(:default_branch) { project.default_branch_or_main } let(:default_branch) { project.default_branch_or_main }
let(:pipeline_branch) { default_branch } let(:pipeline_branch) { default_branch }
......
...@@ -14,7 +14,7 @@ RSpec.describe 'npm.gitlab-ci.yml' do ...@@ -14,7 +14,7 @@ RSpec.describe 'npm.gitlab-ci.yml' do
let(:pipeline_tag) { 'v1.2.1' } let(:pipeline_tag) { 'v1.2.1' }
let(:pipeline_ref) { pipeline_branch } let(:pipeline_ref) { pipeline_branch }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_ref ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
def create_branch(name:) def create_branch(name:)
......
...@@ -15,7 +15,7 @@ RSpec.describe 'Terraform.latest.gitlab-ci.yml' do ...@@ -15,7 +15,7 @@ RSpec.describe 'Terraform.latest.gitlab-ci.yml' do
let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) } let(:project) { create(:project, :custom_repo, files: { 'README.md' => '' }) }
let(:user) { project.owner } let(:user) { project.owner }
let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) } let(:service) { Ci::CreatePipelineService.new(project, user, ref: pipeline_branch ) }
let(:pipeline) { service.execute!(:push) } let(:pipeline) { service.execute!(:push).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
before do before do
......
...@@ -624,6 +624,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do ...@@ -624,6 +624,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
let(:primary_pipeline) do let(:primary_pipeline) do
Ci::CreatePipelineService.new(upstream_project, upstream_project.owner, { ref: 'master' }) Ci::CreatePipelineService.new(upstream_project, upstream_project.owner, { ref: 'master' })
.execute(:push, save_on_errors: false) .execute(:push, save_on_errors: false)
.payload
end end
let(:bridge) { primary_pipeline.processables.find_by(name: 'bridge-job') } let(:bridge) { primary_pipeline.processables.find_by(name: 'bridge-job') }
......
...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
let(:job) { pipeline.builds.find_by(name: 'job') } let(:job) { pipeline.builds.find_by(name: 'job') }
before do before do
......
...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -79,7 +79,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -79,7 +79,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
end end
def create_pipeline! def create_pipeline!
service.execute(:push) service.execute(:push).payload
end end
def create_gitlab_ci_yml(project, content) def create_gitlab_ci_yml(project, content)
......
...@@ -11,7 +11,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -11,7 +11,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:upstream_pipeline) { create(:ci_pipeline, project: project) } let(:upstream_pipeline) { create(:ci_pipeline, project: project) }
let(:bridge) { create(:ci_bridge, pipeline: upstream_pipeline) } let(:bridge) { create(:ci_bridge, pipeline: upstream_pipeline) }
subject { service.execute(:push, bridge: bridge) } subject { service.execute(:push, bridge: bridge).payload }
context 'custom config content' do context 'custom config content' do
let(:bridge) do let(:bridge) do
......
...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
subject { service.execute(:push, dry_run: true) } subject { service.execute(:push, dry_run: true).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -14,7 +14,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -14,7 +14,7 @@ RSpec.describe Ci::CreatePipelineService do
end end
describe '#execute' do describe '#execute' do
subject { service.execute(:push) } subject { service.execute(:push).payload }
context 'with deployment tier' do context 'with deployment tier' do
before do before do
......
...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -9,7 +9,7 @@ RSpec.describe Ci::CreatePipelineService do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:service) { described_class.new(project, user, ref: 'master') } let(:service) { described_class.new(project, user, ref: 'master') }
let(:pipeline) { service.execute(:push) } let(:pipeline) { service.execute(:push).payload }
let(:job) { pipeline.builds.find_by(name: 'job') } let(:job) { pipeline.builds.find_by(name: 'job') }
before do before do
......
...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/feature' } let(:ref) { 'refs/heads/feature' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
before do before do
stub_ci_pipeline_yaml_file <<-EOS stub_ci_pipeline_yaml_file <<-EOS
......
...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -10,7 +10,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -6,7 +6,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -6,7 +6,7 @@ RSpec.describe Ci::CreatePipelineService do
let_it_be(:user) { project.owner } let_it_be(:user) { project.owner }
let(:service) { described_class.new(project, user, { ref: 'master' }) } let(:service) { described_class.new(project, user, { ref: 'master' }) }
let(:pipeline) { service.execute(:push) } let(:pipeline) { service.execute(:push).payload }
before do before do
stub_ci_pipeline_yaml_file(config) stub_ci_pipeline_yaml_file(config)
......
...@@ -30,7 +30,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -30,7 +30,7 @@ RSpec.describe Ci::CreatePipelineService do
describe '#execute' do describe '#execute' do
context 'when source is a dangling build' do context 'when source is a dangling build' do
subject { service.execute(:ondemand_dast_scan, content: content) } subject { service.execute(:ondemand_dast_scan, content: content).payload }
context 'parameter config content' do context 'parameter config content' do
it 'creates a pipeline' do it 'creates a pipeline' do
......
...@@ -369,6 +369,6 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do ...@@ -369,6 +369,6 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do
end end
def create_pipeline! def create_pipeline!
service.execute(:push) service.execute(:push).payload
end end
end end
...@@ -8,7 +8,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -8,7 +8,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
let(:config) do let(:config) do
<<~YAML <<~YAML
......
...@@ -7,7 +7,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -7,7 +7,7 @@ RSpec.describe Ci::CreatePipelineService do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:source) { :push } let(:source) { :push }
let(:service) { described_class.new(project, user, { ref: ref }) } let(:service) { described_class.new(project, user, { ref: ref }) }
let(:pipeline) { service.execute(source) } let(:pipeline) { service.execute(source).payload }
let(:build_names) { pipeline.builds.pluck(:name) } let(:build_names) { pipeline.builds.pluck(:name) }
context 'job:rules' do context 'job:rules' do
......
...@@ -6,14 +6,13 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do ...@@ -6,14 +6,13 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do
describe '#execute' do describe '#execute' do
let_it_be(:project) { create(:project, :auto_devops, :repository) } let_it_be(:project) { create(:project, :auto_devops, :repository) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be_with_reload(:pull_request) { create(:external_pull_request, project: project) }
let(:pull_request) { create(:external_pull_request, project: project) }
before do before do
project.add_maintainer(user) project.add_maintainer(user)
end end
subject { described_class.new(project, user).execute(pull_request) } subject(:response) { described_class.new(project, user).execute(pull_request) }
context 'when pull request is open' do context 'when pull request is open' do
before do before do
...@@ -28,17 +27,20 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do ...@@ -28,17 +27,20 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do
pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target) pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target)
end end
it 'creates a pipeline for external pull request' do it 'creates a pipeline for external pull request', :aggregate_failures do
expect(subject).to be_valid pipeline = response.payload
expect(subject).to be_persisted
expect(subject).to be_external_pull_request_event expect(response).to be_success
expect(subject).to eq(project.ci_pipelines.last) expect(pipeline).to be_valid
expect(subject.external_pull_request).to eq(pull_request) expect(pipeline).to be_persisted
expect(subject.user).to eq(user) expect(pipeline).to be_external_pull_request_event
expect(subject.status).to eq('created') expect(pipeline).to eq(project.ci_pipelines.last)
expect(subject.ref).to eq(pull_request.source_branch) expect(pipeline.external_pull_request).to eq(pull_request)
expect(subject.sha).to eq(pull_request.source_sha) expect(pipeline.user).to eq(user)
expect(subject.source_sha).to eq(pull_request.source_sha) expect(pipeline.status).to eq('created')
expect(pipeline.ref).to eq(pull_request.source_branch)
expect(pipeline.sha).to eq(pull_request.source_sha)
expect(pipeline.source_sha).to eq(pull_request.source_sha)
end end
end end
...@@ -50,10 +52,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do ...@@ -50,10 +52,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do
pull_request.update!(source_branch: source_branch.name, source_sha: commit.sha) pull_request.update!(source_branch: source_branch.name, source_sha: commit.sha)
end end
it 'does nothing' do it 'does nothing', :aggregate_failures do
expect(Ci::CreatePipelineService).not_to receive(:new) expect(Ci::CreatePipelineService).not_to receive(:new)
expect(subject).to be_nil expect(response).to be_error
expect(response.message).to eq('The source sha is not the head of the source branch')
expect(response.payload).to be_nil
end end
end end
end end
...@@ -63,10 +67,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do ...@@ -63,10 +67,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do
pull_request.update!(status: :closed) pull_request.update!(status: :closed)
end end
it 'does nothing' do it 'does nothing', :aggregate_failures do
expect(Ci::CreatePipelineService).not_to receive(:new) expect(Ci::CreatePipelineService).not_to receive(:new)
expect(subject).to be_nil expect(response).to be_error
expect(response.message).to eq('The pull request is not opened')
expect(response.payload).to be_nil
end end
end end
end end
......
...@@ -871,7 +871,7 @@ RSpec.shared_examples 'Pipeline Processing Service' do ...@@ -871,7 +871,7 @@ RSpec.shared_examples 'Pipeline Processing Service' do
end end
let(:pipeline) do let(:pipeline) do
Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push) Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload
end end
before do before do
......
...@@ -10,7 +10,7 @@ RSpec.shared_context 'Pipeline Processing Service Tests With Yaml' do ...@@ -10,7 +10,7 @@ RSpec.shared_context 'Pipeline Processing Service Tests With Yaml' do
with_them do with_them do
let(:test_file) { YAML.load_file(test_file_path) } let(:test_file) { YAML.load_file(test_file_path) }
let(:pipeline) { Ci::CreatePipelineService.new(project, user, ref: 'master').execute(:pipeline) } let(:pipeline) { Ci::CreatePipelineService.new(project, user, ref: 'master').execute(:pipeline).payload }
before do before do
stub_ci_pipeline_yaml_file(YAML.dump(test_file['config'])) stub_ci_pipeline_yaml_file(YAML.dump(test_file['config']))
......
...@@ -24,9 +24,11 @@ RSpec.describe Ci::PipelineTriggerService do ...@@ -24,9 +24,11 @@ RSpec.describe Ci::PipelineTriggerService do
context 'when the pipeline was not created successfully' do context 'when the pipeline was not created successfully' do
let(:fail_pipeline) do let(:fail_pipeline) do
receive(:execute).and_wrap_original do |original, *args| receive(:execute).and_wrap_original do |original, *args|
pipeline = original.call(*args) response = original.call(*args)
pipeline = response.payload
pipeline.update!(failure_reason: 'unknown_failure') pipeline.update!(failure_reason: 'unknown_failure')
pipeline
response
end end
end end
......
...@@ -18,7 +18,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -18,7 +18,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
end end
describe '#execute' do describe '#execute' do
subject { service.execute(merge_request) } subject(:response) { service.execute(merge_request) }
before do before do
stub_ci_pipeline_yaml_file(YAML.dump(config)) stub_ci_pipeline_yaml_file(YAML.dump(config))
...@@ -39,14 +39,15 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -39,14 +39,15 @@ RSpec.describe MergeRequests::CreatePipelineService do
let(:source_project) { project } let(:source_project) { project }
it 'creates a detached merge request pipeline' do it 'creates a detached merge request pipeline' do
expect { subject }.to change { Ci::Pipeline.count }.by(1) expect { response }.to change { Ci::Pipeline.count }.by(1)
expect(subject).to be_persisted expect(response).to be_success
expect(subject).to be_detached_merge_request_pipeline expect(response.payload).to be_persisted
expect(response.payload).to be_detached_merge_request_pipeline
end end
it 'defaults to merge_request_event' do it 'defaults to merge_request_event' do
expect(subject.source).to eq('merge_request_event') expect(response.payload.source).to eq('merge_request_event')
end end
context 'with fork merge request' do context 'with fork merge request' do
...@@ -58,7 +59,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -58,7 +59,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
let(:actor) { user } let(:actor) { user }
it 'creates a pipeline in the target project' do it 'creates a pipeline in the target project' do
expect(subject.project).to eq(project) expect(response.payload.project).to eq(project)
end end
context 'when source branch is protected' do context 'when source branch is protected' do
...@@ -66,7 +67,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -66,7 +67,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
let!(:protected_branch) { create(:protected_branch, name: '*', project: project) } let!(:protected_branch) { create(:protected_branch, name: '*', project: project) }
it 'creates a pipeline in the source project' do it 'creates a pipeline in the source project' do
expect(subject.project).to eq(source_project) expect(response.payload.project).to eq(source_project)
end end
end end
...@@ -74,7 +75,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -74,7 +75,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
let!(:protected_branch) { create(:protected_branch, :developers_can_merge, name: '*', project: project) } let!(:protected_branch) { create(:protected_branch, :developers_can_merge, name: '*', project: project) }
it 'creates a pipeline in the target project' do it 'creates a pipeline in the target project' do
expect(subject.project).to eq(project) expect(response.payload.project).to eq(project)
end end
end end
end end
...@@ -85,7 +86,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -85,7 +86,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
end end
it 'creates a pipeline in the source project' do it 'creates a pipeline in the source project' do
expect(subject.project).to eq(source_project) expect(response.payload.project).to eq(source_project)
end end
end end
end end
...@@ -99,15 +100,16 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -99,15 +100,16 @@ RSpec.describe MergeRequests::CreatePipelineService do
end end
it 'creates a pipeline in the source project' do it 'creates a pipeline in the source project' do
expect(subject.project).to eq(source_project) expect(response.payload.project).to eq(source_project)
end end
end end
context 'when actor does not have permission to create pipelines' do context 'when actor does not have permission to create pipelines' do
let(:actor) { create(:user) } let(:actor) { create(:user) }
it 'returns nothing' do it 'responds with error' do
expect(subject.full_error_messages).to include('Insufficient permissions to create a new pipeline') expect(response).to be_error
expect(response.message).to include('Insufficient permissions to create a new pipeline')
end end
end end
end end
...@@ -139,7 +141,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -139,7 +141,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
end end
it 'does not create a pipeline' do it 'does not create a pipeline' do
expect { subject }.not_to change { Ci::Pipeline.count } expect { response }.not_to change { Ci::Pipeline.count }
end end
end end
...@@ -154,7 +156,7 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -154,7 +156,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
end end
it 'does not create a pipeline' do it 'does not create a pipeline' do
expect { subject }.not_to change { Ci::Pipeline.count } expect { response }.not_to change { Ci::Pipeline.count }
end end
end end
end end
...@@ -170,11 +172,12 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -170,11 +172,12 @@ RSpec.describe MergeRequests::CreatePipelineService do
} }
end end
it 'creates a detached merge request pipeline' do it 'creates a detached merge request pipeline', :aggregate_failures do
expect { subject }.to change { Ci::Pipeline.count }.by(1) expect { response }.to change { Ci::Pipeline.count }.by(1)
expect(subject).to be_persisted expect(response).to be_success
expect(subject).to be_detached_merge_request_pipeline expect(response.payload).to be_persisted
expect(response.payload).to be_detached_merge_request_pipeline
end end
end end
...@@ -188,10 +191,25 @@ RSpec.describe MergeRequests::CreatePipelineService do ...@@ -188,10 +191,25 @@ RSpec.describe MergeRequests::CreatePipelineService do
} }
end end
it 'does not create a pipeline' do it 'does not create a pipeline', :aggregate_failures do
expect { subject }.not_to change { Ci::Pipeline.count } expect { response }.not_to change { Ci::Pipeline.count }
expect(response).to be_error
end end
end end
end end
context 'when merge request has no commits' do
before do
allow(merge_request).to receive(:has_no_commits?).and_return(true)
end
it 'does not create a pipeline', :aggregate_failures do
expect { response }.not_to change { Ci::Pipeline.count }
expect(response).to be_error
expect(response.message).to eq('Cannot create a pipeline for this merge request.')
expect(response.payload).to be_nil
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment