From bf639fd504c84929ff8542eef81578a6745bf428 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 21 Mar 2019 20:08:32 +0700 Subject: [PATCH] Create detached merge request pipelines By using `refs/merge-requests/:iid/head` ok ok Improve naming nicely Add nice tests add nice tests fix some more revert --- app/models/ci/build.rb | 6 ++- app/models/ci/pipeline.rb | 8 ++++ app/models/merge_request.rb | 4 ++ app/presenters/ci/build_runner_presenter.rb | 9 +++- app/services/merge_requests/base_service.rb | 26 +++++++--- app/services/merge_requests/create_service.rb | 2 +- .../merge_requests/refresh_service.rb | 2 +- ...ersist-fulll-ref-path-for-mr-pipelines.yml | 5 ++ lib/gitlab/ci/pipeline/chain/command.rb | 7 +++ .../ci/pipeline/chain/validate/abilities.rb | 2 + .../ci/pipeline/chain/validate/repository.rb | 2 +- spec/factories/merge_requests.rb | 17 +++++-- .../gitlab/ci/pipeline/chain/command_spec.rb | 18 +++++++ .../pipeline/chain/validate/abilities_spec.rb | 47 +++++++++++++++++- .../chain/validate/repository_spec.rb | 19 ++++++++ spec/models/ci/build_spec.rb | 20 ++++++++ spec/models/ci/pipeline_spec.rb | 36 ++++++++++++++ spec/models/merge_request_spec.rb | 28 +++++++++++ .../ci/build_runner_presenter_spec.rb | 38 +++++++++++++++ .../merge_requests/create_service_spec.rb | 48 ++++++++++++++++--- .../merge_requests/refresh_service_spec.rb | 41 +++++++++++++--- 21 files changed, 354 insertions(+), 31 deletions(-) create mode 100644 changelogs/unreleased/persist-fulll-ref-path-for-mr-pipelines.yml diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 59f47effff7..1bd517641ac 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -26,7 +26,8 @@ module Ci belongs_to :erased_by, class_name: 'User' RUNNER_FEATURES = { - upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? } + upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? }, + refspecs: -> (build) { build.merge_request_ref? } }.freeze has_one :deployment, as: :deployable, class_name: 'Deployment' @@ -47,7 +48,8 @@ module Ci delegate :terminal_specification, to: :runner_session, allow_nil: true delegate :gitlab_deploy_token, to: :project delegate :trigger_short_token, to: :trigger_request, allow_nil: true - delegate :merge_request_event?, to: :pipeline + delegate :merge_request_event?, :merge_request_ref?, + :legacy_detached_merge_request_pipeline?, to: :pipeline ## # Since Gitlab 11.5, deployments records started being created right after diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 826b3f82bbf..e6da0a58dce 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -738,6 +738,10 @@ module Ci triggered_by_merge_request? && target_sha.nil? end + def legacy_detached_merge_request_pipeline? + detached_merge_request_pipeline? && !merge_request_ref? + end + def merge_request_pipeline? triggered_by_merge_request? && target_sha.present? end @@ -746,6 +750,10 @@ module Ci triggered_by_merge_request? && target_sha == merge_request.target_branch_sha end + def merge_request_ref? + MergeRequest.merge_request_ref?(ref) + end + def matches_sha_or_source_sha?(sha) self.sha == sha || self.source_sha == sha end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 19557fd476e..7135cf58e33 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1129,6 +1129,10 @@ class MergeRequest < ActiveRecord::Base "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/merge" end + def self.merge_request_ref?(ref) + ref.start_with?("refs/#{Repository::REF_MERGE_REQUEST}/") + end + def in_locked_state begin lock_mr diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index 29656b17183..ed3daf6585b 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -4,6 +4,7 @@ module Ci class BuildRunnerPresenter < SimpleDelegator include Gitlab::Utils::StrongMemoize + DEFAULT_GIT_DEPTH_MERGE_REQUEST = 10 RUNNER_REMOTE_TAG_PREFIX = 'refs/tags/'.freeze RUNNER_REMOTE_BRANCH_PREFIX = 'refs/remotes/origin/'.freeze @@ -27,6 +28,7 @@ module Ci def git_depth strong_memoize(:git_depth) do git_depth = variables&.find { |variable| variable[:key] == 'GIT_DEPTH' }&.dig(:value) + git_depth ||= DEFAULT_GIT_DEPTH_MERGE_REQUEST if merge_request_ref? git_depth.to_i end end @@ -35,8 +37,9 @@ module Ci specs = [] if git_depth > 0 - specs << refspec_for_branch(ref) if branch? || merge_request_event? + specs << refspec_for_branch(ref) if branch? || legacy_detached_merge_request_pipeline? specs << refspec_for_tag(ref) if tag? + specs << refspec_for_merge_request_ref if merge_request_ref? else specs << refspec_for_branch specs << refspec_for_tag @@ -83,5 +86,9 @@ module Ci def refspec_for_tag(ref = '*') "+#{Gitlab::Git::TAG_REF_PREFIX}#{ref}:#{RUNNER_REMOTE_TAG_PREFIX}#{ref}" end + + def refspec_for_merge_request_ref + "+#{ref}:#{ref}" + end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 11ede5223e5..3e208241da5 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -54,7 +54,7 @@ module MergeRequests merge_request, merge_request.project, current_user, merge_request.assignee) end - def create_merge_request_pipeline(merge_request, user) + def create_pipeline_for(merge_request, user) return unless Feature.enabled?(:ci_merge_request_pipeline, merge_request.source_project, default_enabled: true) @@ -65,12 +65,24 @@ module MergeRequests return if merge_request.merge_request_pipeline_exists? return if merge_request.has_no_commits? - Ci::CreatePipelineService - .new(merge_request.source_project, user, ref: merge_request.source_branch) - .execute(:merge_request_event, - ignore_skip_ci: true, - save_on_errors: false, - merge_request: merge_request) + create_detached_merge_request_pipeline(merge_request, user) + end + + def create_detached_merge_request_pipeline(merge_request, user) + if can_use_merge_request_ref?(merge_request) + Ci::CreatePipelineService.new(merge_request.source_project, user, + ref: merge_request.ref_path) + .execute(:merge_request_event, merge_request: merge_request) + else + Ci::CreatePipelineService.new(merge_request.source_project, user, + ref: merge_request.source_branch) + .execute(:merge_request_event, merge_request: merge_request) + end + end + + def can_use_merge_request_ref?(merge_request) + Feature.enabled?(:ci_use_merge_request_ref, project, default_enabled: true) && + !merge_request.for_fork? end # Returns all origin and fork merge requests from `@project` satisfying passed arguments. diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 02c2388c05c..06e46595b95 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -25,7 +25,7 @@ module MergeRequests def after_create(issuable) todo_service.new_merge_request(issuable, current_user) issuable.cache_merge_request_closes_issues!(current_user) - create_merge_request_pipeline(issuable, current_user) + create_pipeline_for(issuable, current_user) issuable.update_head_pipeline super diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index f5dc5a0256d..51d27673787 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -107,7 +107,7 @@ module MergeRequests end merge_request.mark_as_unchecked - create_merge_request_pipeline(merge_request, current_user) + create_pipeline_for(merge_request, current_user) UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end diff --git a/changelogs/unreleased/persist-fulll-ref-path-for-mr-pipelines.yml b/changelogs/unreleased/persist-fulll-ref-path-for-mr-pipelines.yml new file mode 100644 index 00000000000..ca42a26e8ff --- /dev/null +++ b/changelogs/unreleased/persist-fulll-ref-path-for-mr-pipelines.yml @@ -0,0 +1,5 @@ +--- +title: Create MR pipelines with `refs/merge-requests/:iid/head` +merge_request: 25504 +author: +type: changed diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index bf9f03f6134..03af99ba9a5 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -33,6 +33,13 @@ module Gitlab end end + def merge_request_ref_exists? + strong_memoize(:merge_request_ref_exists) do + MergeRequest.merge_request_ref?(origin_ref) && + project.repository.ref_exists?(origin_ref) + end + end + def ref strong_memoize(:ref) do Gitlab::Git.ref_name(origin_ref) diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index ebd7e6e8289..aaa3daddcc5 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -44,6 +44,8 @@ module Gitlab access.can_update_branch?(@command.ref) elsif @command.tag_exists? access.can_create_tag?(@command.ref) + elsif @command.merge_request_ref_exists? + access.can_update_branch?(@command.merge_request.source_branch) else true # Allow it for now and we'll reject when we check ref existence end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 9c6c2bc8e25..8f5445850d7 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -9,7 +9,7 @@ module Gitlab include Chain::Helpers def perform! - unless @command.branch_exists? || @command.tag_exists? + unless @command.branch_exists? || @command.tag_exists? || @command.merge_request_ref_exists? return error('Reference not found') end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index a1809a26265..abf0e6bccb7 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -117,9 +117,20 @@ FactoryBot.define do end end + trait :with_legacy_detached_merge_request_pipeline do + after(:create) do |merge_request| + merge_request.merge_request_pipelines << create(:ci_pipeline, + source: :merge_request_event, + merge_request: merge_request, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.source_branch_sha) + end + end + trait :with_detached_merge_request_pipeline do - after(:build) do |merge_request| - merge_request.merge_request_pipelines << build(:ci_pipeline, + after(:create) do |merge_request| + merge_request.merge_request_pipelines << create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, project: merge_request.source_project, @@ -135,7 +146,7 @@ FactoryBot.define do target_sha { target_branch_sha } end - after(:build) do |merge_request, evaluator| + after(:create) do |merge_request, evaluator| merge_request.merge_request_pipelines << create(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index dab0fb51bcc..5181e9c1583 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -48,6 +48,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do end end + describe '#merge_request_ref_exists?' do + subject { command.merge_request_ref_exists? } + + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + context 'for existing merge request ref' do + let(:origin_ref) { merge_request.ref_path } + + it { is_expected.to eq(true) } + end + + context 'for branch ref' do + let(:origin_ref) { merge_request.source_branch } + + it { is_expected.to eq(false) } + end + end + describe '#ref' do subject { command.ref } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index 8ba56d73838..7d750877d09 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -10,12 +10,33 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( - project: project, current_user: user, origin_ref: ref) + project: project, current_user: user, origin_ref: origin_ref, merge_request: merge_request) end let(:step) { described_class.new(pipeline, command) } let(:ref) { 'master' } + let(:origin_ref) { ref } + let(:merge_request) { nil } + + shared_context 'detached merge request pipeline' do + let(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: ref, + target_project: project, + target_branch: 'feature') + end + + let(:pipeline) do + build(:ci_pipeline, + source: :merge_request_event, + merge_request: merge_request, + project: project) + end + + let(:origin_ref) { merge_request.ref_path } + end context 'when users has no ability to run a pipeline' do before do @@ -58,6 +79,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do it { is_expected.to be_truthy } + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end + context 'when the branch is protected' do let!(:protected_branch) do create(:protected_branch, project: project, name: ref) @@ -65,6 +92,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do it { is_expected.to be_falsey } + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_falsey } + end + context 'when developers are allowed to merge' do let!(:protected_branch) do create(:protected_branch, @@ -74,6 +107,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do end it { is_expected.to be_truthy } + + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end end end @@ -112,6 +151,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do end it { is_expected.to be_truthy } + + context 'when pipeline is a detached merge request pipeline' do + include_context 'detached merge request pipeline' + + it { is_expected.to be_truthy } + end end context 'when the tag is protected' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index a7cad423d09..2e8c9d70098 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -42,6 +42,25 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end + context 'when origin ref is a merge request ref' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: origin_ref, checkout_sha: checkout_sha) + end + + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:origin_ref) { merge_request.ref_path } + let(:checkout_sha) { project.repository.commit(merge_request.ref_path).id } + + it 'does not break the chain' do + expect(step.break?).to be false + end + + it 'does not append pipeline errors' do + expect(pipeline.errors).to be_empty + end + end + context 'when ref is ambiguous' do let(:project) do create(:project, :repository).tap do |proj| diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 7500e6ae5b1..3ec07143e93 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -24,6 +24,8 @@ describe Ci::Build do it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } it { is_expected.to delegate_method(:merge_request_event?).to(:pipeline) } + it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } + it { is_expected.to delegate_method(:legacy_detached_merge_request_pipeline?).to(:pipeline) } it { is_expected.to be_a(ArtifactMigratable) } @@ -3626,6 +3628,24 @@ describe Ci::Build do it { is_expected.to be_falsey } end end + + context 'when refspecs feature is required by build' do + before do + allow(build).to receive(:merge_request_ref?) { true } + end + + context 'when runner provides given feature' do + let(:runner_features) { { refspecs: true } } + + it { is_expected.to be_truthy } + end + + context 'when runner does not provide given feature' do + let(:runner_features) { {} } + + it { is_expected.to be_falsey } + end + end end describe '#deployment_status' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 5b8097621e0..eb6ed3b658d 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -362,6 +362,42 @@ describe Ci::Pipeline, :mailer do end end + describe '#merge_request_ref?' do + subject { pipeline.merge_request_ref? } + + it 'calls MergeRequest#merge_request_ref?' do + expect(MergeRequest).to receive(:merge_request_ref?).with(pipeline.ref) + + subject + end + end + + describe '#legacy_detached_merge_request_pipeline?' do + subject { pipeline.legacy_detached_merge_request_pipeline? } + + set(:merge_request) { create(:merge_request) } + let(:ref) { 'feature' } + let(:target_sha) { nil } + + let(:pipeline) do + build(:ci_pipeline, source: :merge_request_event, merge_request: merge_request, ref: ref, target_sha: target_sha) + end + + it { is_expected.to be_truthy } + + context 'when pipeline ref is a merge request ref' do + let(:ref) { 'refs/merge-requests/1/head' } + + it { is_expected.to be_falsy } + end + + context 'when target sha is set' do + let(:target_sha) { 'target-sha' } + + it { is_expected.to be_falsy } + end + end + describe '#matches_sha_or_source_sha?' do subject { pipeline.matches_sha_or_source_sha?(sample_sha) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a1de0c63623..5adeb616e84 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3115,4 +3115,32 @@ describe MergeRequest do end end end + + describe '.merge_request_ref?' do + subject { described_class.merge_request_ref?(ref) } + + context 'when ref is ref name of a branch' do + let(:ref) { 'feature' } + + it { is_expected.to be_falsey } + end + + context 'when ref is HEAD ref path of a branch' do + let(:ref) { 'refs/heads/feature' } + + it { is_expected.to be_falsey } + end + + context 'when ref is HEAD ref path of a merge request' do + let(:ref) { 'refs/merge-requests/1/head' } + + it { is_expected.to be_truthy } + end + + context 'when ref is merge ref path of a merge request' do + let(:ref) { 'refs/merge-requests/1/merge' } + + it { is_expected.to be_truthy } + end + end end diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb index f50bcf54b46..ad6cb012d0b 100644 --- a/spec/presenters/ci/build_runner_presenter_spec.rb +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -136,6 +136,24 @@ describe Ci::BuildRunnerPresenter do is_expected.to eq(1) end end + + context 'when pipeline is detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.first } + let(:build) { create(:ci_build, ref: pipeline.ref, pipeline: pipeline) } + + it 'returns the default git depth for pipelines for merge requests' do + is_expected.to eq(described_class::DEFAULT_GIT_DEPTH_MERGE_REQUEST) + end + + context 'when pipeline is legacy detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) } + + it 'behaves as branch pipeline' do + is_expected.to eq(0) + end + end + end end describe '#refspecs' do @@ -165,5 +183,25 @@ describe Ci::BuildRunnerPresenter do end end end + + context 'when pipeline is detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.first } + let(:build) { create(:ci_build, ref: pipeline.ref, pipeline: pipeline) } + + it 'returns the correct refspecs' do + is_expected + .to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head') + end + + context 'when pipeline is legacy detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) } + + it 'returns the correct refspecs' do + is_expected.to contain_exactly('+refs/tags/*:refs/tags/*', + '+refs/heads/*:refs/remotes/origin/*') + end + end + end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index a04a4d5fc36..55e7b46248b 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -173,7 +173,7 @@ describe MergeRequests::CreateService do end end - describe 'Merge request pipelines' do + describe 'Pipelines for merge requests' do before do stub_ci_pipeline_yaml_file(YAML.dump(config)) end @@ -189,12 +189,46 @@ describe MergeRequests::CreateService do } end - it 'creates a merge request pipeline and sets it as a head pipeline' do + it 'creates a detached merge request pipeline and sets it as a head pipeline' do expect(merge_request).to be_persisted merge_request.reload expect(merge_request.merge_request_pipelines.count).to eq(1) - expect(merge_request.actual_head_pipeline).to be_merge_request_event + expect(merge_request.actual_head_pipeline).to be_detached_merge_request_pipeline + end + + context 'when merge request is submitted from forked project' do + let(:target_project) { fork_project(project, nil, repository: true) } + + let(:opts) do + { + title: 'Awesome merge_request', + source_branch: 'feature', + target_branch: 'master', + target_project_id: target_project.id + } + end + + before do + target_project.add_developer(assignee) + target_project.add_maintainer(user) + end + + it 'create legacy detached merge request pipeline for fork merge request' do + expect(merge_request.actual_head_pipeline) + .to be_legacy_detached_merge_request_pipeline + end + end + + context 'when ci_use_merge_request_ref feature flag is false' do + before do + stub_feature_flags(ci_use_merge_request_ref: false) + end + + it 'create legacy detached merge request pipeline for non-fork merge request' do + expect(merge_request.actual_head_pipeline) + .to be_legacy_detached_merge_request_pipeline + end end context 'when there are no commits between source branch and target branch' do @@ -207,7 +241,7 @@ describe MergeRequests::CreateService do } end - it 'does not create a merge request pipeline' do + it 'does not create a detached merge request pipeline' do expect(merge_request).to be_persisted merge_request.reload @@ -225,7 +259,7 @@ describe MergeRequests::CreateService do merge_request end - it 'sets the latest merge request pipeline as the head pipeline' do + it 'sets the latest detached merge request pipeline as the head pipeline' do expect(merge_request.actual_head_pipeline).to be_merge_request_event end end @@ -235,7 +269,7 @@ describe MergeRequests::CreateService do stub_feature_flags(ci_merge_request_pipeline: false) end - it 'does not create a merge request pipeline' do + it 'does not create a detached merge request pipeline' do expect(merge_request).to be_persisted merge_request.reload @@ -254,7 +288,7 @@ describe MergeRequests::CreateService do } end - it 'does not create a merge request pipeline' do + it 'does not create a detached merge request pipeline' do expect(merge_request).to be_persisted merge_request.reload diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 6c8ff163692..25cbac6d7ee 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -141,7 +141,7 @@ describe MergeRequests::RefreshService do end end - describe 'Merge request pipelines' do + describe 'Pipelines for merge requests' do before do stub_ci_pipeline_yaml_file(YAML.dump(config)) end @@ -159,7 +159,7 @@ describe MergeRequests::RefreshService do } end - it 'create merge request pipeline with commits' do + it 'create detached merge request pipeline with commits' do expect { subject } .to change { @merge_request.merge_request_pipelines.count }.by(1) .and change { @fork_merge_request.merge_request_pipelines.count }.by(1) @@ -170,7 +170,34 @@ describe MergeRequests::RefreshService do expect(@another_merge_request.has_commits?).to be_falsy end - context "when branch pipeline was created before a merge request pipline has been created" do + it 'create detached merge request pipeline for non-fork merge request' do + subject + + expect(@merge_request.merge_request_pipelines.first) + .to be_detached_merge_request_pipeline + end + + it 'create legacy detached merge request pipeline for fork merge request' do + subject + + expect(@fork_merge_request.merge_request_pipelines.first) + .to be_legacy_detached_merge_request_pipeline + end + + context 'when ci_use_merge_request_ref feature flag is false' do + before do + stub_feature_flags(ci_use_merge_request_ref: false) + end + + it 'create legacy detached merge request pipeline for non-fork merge request' do + subject + + expect(@merge_request.merge_request_pipelines.first) + .to be_legacy_detached_merge_request_pipeline + end + end + + context "when branch pipeline was created before a detaced merge request pipeline has been created" do before do create(:ci_pipeline, project: @merge_request.source_project, sha: @merge_request.diff_head_sha, @@ -180,7 +207,7 @@ describe MergeRequests::RefreshService do subject end - it 'sets the latest merge request pipeline as a head pipeline' do + it 'sets the latest detached merge request pipeline as a head pipeline' do @merge_request.reload expect(@merge_request.actual_head_pipeline).to be_merge_request_event end @@ -193,7 +220,7 @@ describe MergeRequests::RefreshService do end context "when MergeRequestUpdateWorker is retried by an exception" do - it 'does not re-create a duplicate merge request pipeline' do + it 'does not re-create a duplicate detached merge request pipeline' do expect do service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') end.to change { @merge_request.merge_request_pipelines.count }.by(1) @@ -209,7 +236,7 @@ describe MergeRequests::RefreshService do stub_feature_flags(ci_merge_request_pipeline: false) end - it 'does not create a merge request pipeline' do + it 'does not create a detached merge request pipeline' do expect { subject } .not_to change { @merge_request.merge_request_pipelines.count } end @@ -226,7 +253,7 @@ describe MergeRequests::RefreshService do } end - it 'does not create a merge request pipeline' do + it 'does not create a detached merge request pipeline' do expect { subject } .not_to change { @merge_request.merge_request_pipelines.count } end