diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5644aee348e9cd59530cbf84aab2e79ef828f9b7..b3804907b33fdcbba467f2f8c3c402356703dfb2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1414,11 +1414,7 @@ def auto_merge_strategy end def default_auto_merge_strategy - if Feature.enabled?(:merge_when_checks_pass, project) - AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS - else - AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS - end + AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS end def auto_merge_strategy=(strategy) diff --git a/app/services/auto_merge/merge_when_checks_pass_service.rb b/app/services/auto_merge/merge_when_checks_pass_service.rb index ab1b1d51409f59ddebe2027e4d266cc8f980e48c..3cdaf3a89377650b5a32fc2d4d0d9c78d8639318 100644 --- a/app/services/auto_merge/merge_when_checks_pass_service.rb +++ b/app/services/auto_merge/merge_when_checks_pass_service.rb @@ -44,7 +44,6 @@ def abort(merge_request, reason) override :available_for def available_for?(merge_request) super do - next false if Feature.disabled?(:merge_when_checks_pass, merge_request.project) next false if merge_request.project.merge_trains_enabled? next false if merge_request.mergeable? && !merge_request.diff_head_pipeline_considered_in_progress? diff --git a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb index 143dfa3f8b866cf819127111d1c869f61646cc62..2e12f93eca5e46403ebb45bbf0ad9e5688aa2f69 100644 --- a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb +++ b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb @@ -32,12 +32,8 @@ def abort(merge_request, reason) end end - def available_for?(merge_request) - super do - next false if Feature.enabled?(:merge_when_checks_pass, merge_request.project) - - merge_request.diff_head_pipeline_considered_in_progress? - end + def available_for?(_merge_request) + false end private diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 03a0ad902d11c56c1ca1e77500f1c140b9e7afcb..fd11518578d56637c29e1b4d4a4b8317bf8966a0 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -83,15 +83,11 @@ def send_graphql_triggers def process_auto_merge return unless discussions_ready_to_merge? - if Feature.enabled?(:merge_when_checks_pass, merge_request.project) - Gitlab::EventStore.publish( - MergeRequests::DiscussionsResolvedEvent.new( - data: { current_user_id: current_user.id, merge_request_id: merge_request.id } - ) + Gitlab::EventStore.publish( + MergeRequests::DiscussionsResolvedEvent.new( + data: { current_user_id: current_user.id, merge_request_id: merge_request.id } ) - else - AutoMergeProcessWorker.perform_async({ 'merge_request_id' => merge_request.id }) - end + ) end def discussions_ready_to_merge? diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index f63bd704e8082e34cd2d3d7fe458ca7b67b5b3d4..19398c4c4eca4c3f0ade13ad3bbfdd91b4623ca3 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -114,7 +114,6 @@ def create_event(merge_request) end def cancel_auto_merges_targeting_source_branch(merge_request) - return unless Feature.enabled?(:merge_when_checks_pass, merge_request.project) return unless params[:delete_source_branch] merge_request.source_project diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 0c70468b9079acbd765cd97758016c130d7e81fc..a43174d1d8bccd68ce3ec87645cd60888df3d4c5 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -233,7 +233,7 @@ def handle_draft_status_change(merge_request, changed_fields) # email template itself, see `change_in_merge_request_draft_status_email` template. notify_draft_status_changed(merge_request) trigger_merge_request_status_updated(merge_request) - publish_draft_change_event(merge_request) if Feature.enabled?(:merge_when_checks_pass, project) + publish_draft_change_event(merge_request) end if !old_title_draft && new_title_draft diff --git a/app/workers/merge_requests/process_auto_merge_from_event_worker.rb b/app/workers/merge_requests/process_auto_merge_from_event_worker.rb index 6d6f651fe7535614d6f8a61afdf6242c43a26063..5589dff7d0aae600a1f2bbe19eca4546e0503b31 100644 --- a/app/workers/merge_requests/process_auto_merge_from_event_worker.rb +++ b/app/workers/merge_requests/process_auto_merge_from_event_worker.rb @@ -19,8 +19,6 @@ def handle_event(event) return end - return unless Feature.enabled?(:merge_when_checks_pass, merge_request.project) - AutoMergeService.new(merge_request.project, merge_request.merge_user) .process(merge_request) end diff --git a/config/feature_flags/beta/merge_when_checks_pass.yml b/config/feature_flags/beta/merge_when_checks_pass.yml deleted file mode 100644 index 9df51f55a6657060dbb26f18f9362437484f0ca9..0000000000000000000000000000000000000000 --- a/config/feature_flags/beta/merge_when_checks_pass.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: merge_when_checks_pass -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121828 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/412995 -milestone: '16.2' -type: beta -group: group::code review -default_enabled: true diff --git a/doc/user/project/merge_requests/auto_merge.md b/doc/user/project/merge_requests/auto_merge.md index f114a7afe3657c736cddc057987b4fb764e82c21..b4d2fce9c4a6c43b2c047a49053576ce30e69e55 100644 --- a/doc/user/project/merge_requests/auto_merge.md +++ b/doc/user/project/merge_requests/auto_merge.md @@ -16,8 +16,9 @@ DETAILS: > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/10874) in GitLab 16.5 [with two flags](../../../administration/feature_flags.md) named `merge_when_checks_pass` and `additional_merge_when_checks_ready`. Disabled by default. > - [Enabled](https://gitlab.com/gitlab-org/gitlab/-/issues/412995) the flags `merge_when_checks_pass` and `additional_merge_when_checks_ready` on GitLab.com in GitLab 17.0. > - [Enabled](https://gitlab.com/gitlab-org/gitlab/-/issues/412995) the flags `merge_when_checks_pass` by default in GitLab 17.4. +> - Merge when checks pass [generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/412995) in GitLab 17.7. Feature flag `merge_when_checks_pass` removed. -When you enable the `merge_when_checks_pass` feature flag, if the content of a merge request is ready to merge, +If the content of a merge request is ready to merge, you can select **Set to auto-merge**. The merge request auto-merges when all required checks complete successfully, and you don't need to remember to manually merge the merge request. After you set auto-merge, these checks must all pass before the merge request merges: diff --git a/ee/app/models/merge_requests/status_check_response.rb b/ee/app/models/merge_requests/status_check_response.rb index d82869e3d2e59218a5edc6d4275e46068e57eeec..0dce9db37d23764243c4e31e7ce1589300e5a916 100644 --- a/ee/app/models/merge_requests/status_check_response.rb +++ b/ee/app/models/merge_requests/status_check_response.rb @@ -32,8 +32,6 @@ def self.timeout_eligible private def publish_new_passing_event - return unless ::Feature.enabled?(:merge_when_checks_pass, merge_request.project) - ::Gitlab::EventStore.publish( ::MergeRequests::ExternalStatusCheckPassedEvent.new( data: { merge_request_id: merge_request.id } diff --git a/ee/app/services/auto_merge/add_to_merge_train_when_checks_pass_service.rb b/ee/app/services/auto_merge/add_to_merge_train_when_checks_pass_service.rb index f6507d54e16dbaa08cd2b72023a016fd555e3844..ff77d29526ba020005c5c17f7697b2e5eaef6f25 100644 --- a/ee/app/services/auto_merge/add_to_merge_train_when_checks_pass_service.rb +++ b/ee/app/services/auto_merge/add_to_merge_train_when_checks_pass_service.rb @@ -55,7 +55,6 @@ def abort(merge_request, reason) def available_for?(merge_request) super do next false unless ::Feature.enabled?(:merge_when_checks_pass_merge_train, merge_request.project) - next false unless ::Feature.enabled?(:merge_when_checks_pass, merge_request.project) next false unless merge_request.has_ci_enabled? next false if merge_request.mergeable? && !merge_request.diff_head_pipeline_considered_in_progress? diff --git a/ee/app/services/ee/merge_requests/post_merge_service.rb b/ee/app/services/ee/merge_requests/post_merge_service.rb index 17e8743aa017acc54e62c2b946db93ca3a6665ff..5e2dfbf424813f9dd1327ec49d99987fc479509f 100644 --- a/ee/app/services/ee/merge_requests/post_merge_service.rb +++ b/ee/app/services/ee/merge_requests/post_merge_service.rb @@ -94,13 +94,11 @@ def trigger_blocked_merge_requests_merge_status_updated(merge_request) merge_request.blocked_merge_requests.find_each do |blocked_mr| ::GraphqlTriggers.merge_request_merge_status_updated(blocked_mr) - if ::Feature.enabled?(:merge_when_checks_pass, merge_request.project) - ::Gitlab::EventStore.publish( - ::MergeRequests::UnblockedStateEvent.new( - data: { current_user_id: current_user.id, merge_request_id: blocked_mr.id } - ) + ::Gitlab::EventStore.publish( + ::MergeRequests::UnblockedStateEvent.new( + data: { current_user_id: current_user.id, merge_request_id: blocked_mr.id } ) - end + ) end end end diff --git a/ee/app/services/ee/merge_requests/update_service.rb b/ee/app/services/ee/merge_requests/update_service.rb index 3628932055f70778caad2602f1336db44bfffa6e..3862c87adca0056ef1b0a0769c59962beab86d56 100644 --- a/ee/app/services/ee/merge_requests/update_service.rb +++ b/ee/app/services/ee/merge_requests/update_service.rb @@ -77,8 +77,6 @@ def handle_override_requested_changes(merge_request, changed_fields) ::SystemNoteService.override_requested_changes(merge_request, current_user, override_requested_changes.last) trigger_merge_request_status_updated(merge_request) - return unless ::Feature.enabled?(:merge_when_checks_pass, merge_request.project) - ::Gitlab::EventStore.publish( ::MergeRequests::OverrideRequestedChangesStateEvent.new( data: { current_user_id: current_user.id, merge_request_id: merge_request.id } @@ -87,8 +85,6 @@ def handle_override_requested_changes(merge_request, changed_fields) end def handle_title_and_desc_edits(merge_request, changed_fields) - return unless ::Feature.enabled?(:merge_when_checks_pass, merge_request.project) - fields = %w[title description] return unless changed_fields.any? { |field| fields.include?(field) } diff --git a/ee/app/services/merge_requests/update_blocks_service.rb b/ee/app/services/merge_requests/update_blocks_service.rb index 280806fba0d6b70d39f9e2f51d7d1737a6e03f35..191d7c6f45969648c3229b45d8775452abb6bb0c 100644 --- a/ee/app/services/merge_requests/update_blocks_service.rb +++ b/ee/app/services/merge_requests/update_blocks_service.rb @@ -71,13 +71,11 @@ def execute if merge_request.blocking_merge_requests.reset != previous_blocking_merge_requests GraphqlTriggers.merge_request_merge_status_updated(merge_request) - if Feature.enabled?(:merge_when_checks_pass, merge_request.project) - Gitlab::EventStore.publish( - ::MergeRequests::UnblockedStateEvent.new( - data: { current_user_id: current_user.id, merge_request_id: merge_request.id } - ) + Gitlab::EventStore.publish( + ::MergeRequests::UnblockedStateEvent.new( + data: { current_user_id: current_user.id, merge_request_id: merge_request.id } ) - end + ) end true diff --git a/ee/spec/controllers/projects/merge_requests_controller_spec.rb b/ee/spec/controllers/projects/merge_requests_controller_spec.rb index 38f56412eba3ba51d335f151958102c3eadf1a6d..115e5cce39faa055f2c0d02f58c15fde09ba6358 100644 --- a/ee/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/ee/spec/controllers/projects/merge_requests_controller_spec.rb @@ -434,18 +434,6 @@ def merge_when_pipeline_succeeds expect(json_response).to eq('status' => 'merge_when_checks_pass') end - - context 'when feature flag "merge_when_checks_pass" is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'fails to merge' do - merge_when_pipeline_succeeds - - expect(json_response).to eq('status' => 'failed') - end - end end describe 'mergeable conditions' do diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 1d139186b07686d00508a5655091919406d9414b..cf99fb84ab8f905cf840006ed2612d3853b426fc 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -1632,7 +1632,6 @@ where(:auto_merge_strategy, :skip_approved_check, :skip_draft_check, :skip_blocked_check, :skip_discussions_check, :skip_external_status_check, :skip_locked_paths_check) do '' | false | false | false | false | false | false - AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS | false | false | false | false | false | false AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true | true | true | true | true | true end diff --git a/ee/spec/models/merge_requests/status_check_response_spec.rb b/ee/spec/models/merge_requests/status_check_response_spec.rb index 76c7fbfcd09a287fb5da3ad143720e7ceea8442a..45016315c9f7b4502a61a9b498a1ff093828e544 100644 --- a/ee/spec/models/merge_requests/status_check_response_spec.rb +++ b/ee/spec/models/merge_requests/status_check_response_spec.rb @@ -83,18 +83,6 @@ response end end - - context 'when feature flag merge_when_checks_pass is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not send a status passed event' do - expect(::Gitlab::EventStore).not_to receive(:publish) - - response - end - end end end end diff --git a/ee/spec/serializers/merge_request_poll_widget_entity_spec.rb b/ee/spec/serializers/merge_request_poll_widget_entity_spec.rb index 018592334aa1ba08ba141e74c5fece6a3f11a29d..fbc141c7a6021b482c9ac9ccf0e2b059cadab981 100644 --- a/ee/spec/serializers/merge_request_poll_widget_entity_spec.rb +++ b/ee/spec/serializers/merge_request_poll_widget_entity_spec.rb @@ -41,18 +41,6 @@ eq(%w[merge_when_checks_pass]) ) end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'returns available auto merge strategies' do - expect(entity[:available_auto_merge_strategies]).to( - eq(%w[merge_when_pipeline_succeeds]) - ) - end - end end context 'when head pipeline is finished and approvals are pending' do diff --git a/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb b/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb index 6420a2ac323dd5f7eaeeb53a91799924e3f50e33..d72ef515905649a6c2d075db9802426ad7a2cd2f 100644 --- a/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb +++ b/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb @@ -15,12 +15,6 @@ describe '#available_for?' do subject { service.available_for?(mr_merge_if_green_enabled) } - let(:feature_flag) { true } - - before do - stub_feature_flags(merge_when_checks_pass: feature_flag) - end - context 'when missing approvals' do let(:approval_rule) do create(:approval_merge_request_rule, merge_request: mr_merge_if_green_enabled, @@ -35,12 +29,6 @@ end it { is_expected.to eq true } - - context 'when merge_when_checks_pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end end context 'when blocked status' do @@ -51,12 +39,6 @@ end it { is_expected.to eq true } - - context 'when merge_when_checks_pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end end context 'when merge trains are enabled' do diff --git a/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb b/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb index 359aa524d4ff9b47d6c00ad5f2c40e450d2882fe..14f2ac8a64675ec5c806b078932b82f3e5587be3 100644 --- a/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb @@ -251,23 +251,6 @@ end end - context 'when merge_when_checks_pass is disabled' do - let(:merge_request) { create :merge_request } - - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not send any unblocked events' do - expect(::Gitlab::EventStore).not_to receive(:publish) - .with(::MergeRequests::UnblockedStateEvent.new(data: { - current_user_id: current_user.id, merge_request_id: merge_request.id - })) - - subject - end - end - context 'when a temporary unapproval is needed for the MR' do it 'removes the unmergeable flag after the service is run' do merge_request.approval_state.temporarily_unapprove! diff --git a/ee/spec/services/ee/merge_requests/update_service_spec.rb b/ee/spec/services/ee/merge_requests/update_service_spec.rb index 7cc568cd8a8259affa37ebfb5d5e3ed702485da7..618b921ccb8574fe5811c481d3bc22ed6a7a25bd 100644 --- a/ee/spec/services/ee/merge_requests/update_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/update_service_spec.rb @@ -140,18 +140,6 @@ def update_merge_request(opts) }) end - context 'when merge_when_checks_pass ff is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not publish a OverrideRequestedChanges state event' do - expect do - update_merge_request(override_requested_changes: true) - end.not_to publish_event(MergeRequests::OverrideRequestedChangesStateEvent) - end - end - it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do let(:action) { update_merge_request(override_requested_changes: true) } end @@ -596,33 +584,8 @@ def update_merge_request(opts) allow(merge_request).to receive(:has_jira_issue_keys?).and_return(has_jira_key) end - context 'when the merge_when_checks_pass feature flag is on' do - context 'when the description changes' do - context 'when the description or title has a jira key' do - it 'publishes a JiraTitleDescriptionUpdateEvent' do - expected_data = { - current_user_id: user.id, - merge_request_id: merge_request.id - } - - expect do - update_merge_request(description: 'New description') - end.to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent).with(expected_data) - end - end - - context 'when the description or title does not have a jira key' do - let(:has_jira_key) { false } - - it 'does not publish a JiraTitleDescriptionUpdateEvent' do - expect do - update_merge_request(description: 'New description') - end.not_to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent) - end - end - end - - context 'when the title changes' do + context 'when the description changes' do + context 'when the description or title has a jira key' do it 'publishes a JiraTitleDescriptionUpdateEvent' do expected_data = { current_user_id: user.id, @@ -630,39 +593,40 @@ def update_merge_request(opts) } expect do - update_merge_request(title: 'New title') + update_merge_request(description: 'New description') end.to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent).with(expected_data) end - - context 'when the description or title does not have a jira key' do - let(:has_jira_key) { false } - - it 'does not publish a JiraTitleDescriptionUpdateEvent' do - expect do - update_merge_request(description: 'New description') - end.not_to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent) - end - end end - end - context 'when the merge_when_checks_pass feature flag is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end + context 'when the description or title does not have a jira key' do + let(:has_jira_key) { false } - context 'when the description changes' do it 'does not publish a JiraTitleDescriptionUpdateEvent' do expect do update_merge_request(description: 'New description') end.not_to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent) end end + end - context 'when the title changes' do - it 'publishes a JiraTitleDescriptionUpdateEvent' do + context 'when the title changes' do + it 'publishes a JiraTitleDescriptionUpdateEvent' do + expected_data = { + current_user_id: user.id, + merge_request_id: merge_request.id + } + + expect do + update_merge_request(title: 'New title') + end.to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent).with(expected_data) + end + + context 'when the description or title does not have a jira key' do + let(:has_jira_key) { false } + + it 'does not publish a JiraTitleDescriptionUpdateEvent' do expect do - update_merge_request(title: 'New title') + update_merge_request(description: 'New description') end.not_to publish_event(MergeRequests::JiraTitleDescriptionUpdateEvent) end end diff --git a/ee/spec/services/quick_actions/interpret_service_spec.rb b/ee/spec/services/quick_actions/interpret_service_spec.rb index f0e82e4fa77e425fb1075ca3faf3fc867783e31c..c40b1462059d7fa0beb723897b7b9ae3eeb92093 100644 --- a/ee/spec/services/quick_actions/interpret_service_spec.rb +++ b/ee/spec/services/quick_actions/interpret_service_spec.rb @@ -1231,50 +1231,30 @@ expect(message).to eq('Scheduled to merge this merge request (Merge when checks pass).') end end - - context 'when "merge_when_checks_pass" is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it_behaves_like 'failed command', 'Could not apply merge command.' do - let(:issuable) { merge_request } - end - end end context 'when the merge request is blocked' do let(:content) { '/merge' } + let(:service) do + described_class.new( + container: project, + current_user: current_user, + params: { merge_request_diff_head_sha: issuable.diff_head_sha } + ) + end + let(:issuable) { create(:merge_request, :blocked, source_project: project) } before do stub_licensed_features(blocking_merge_requests: true) end - context 'when merge_when_checks_pass is enabled' do - let(:service) do - described_class.new( - container: project, - current_user: current_user, - params: { merge_request_diff_head_sha: issuable.diff_head_sha } - ) - end - - it 'runs merge command and returns merge message' do - _, updates, message = service.execute(content, issuable) - - expect(updates).to eq(merge: issuable.diff_head_sha) + it 'runs merge command and returns merge message' do + _, updates, message = service.execute(content, issuable) - expect(message).to eq('Scheduled to merge this merge request (Merge when checks pass).') - end - end - - context 'when merge_when_checks_pass id disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end + expect(updates).to eq(merge: issuable.diff_head_sha) - it_behaves_like 'failed command', 'Could not apply merge command.' + expect(message).to eq('Scheduled to merge this merge request (Merge when checks pass).') end end diff --git a/ee/spec/support/helpers/features/security_policy_helpers.rb b/ee/spec/support/helpers/features/security_policy_helpers.rb index 528a53c69c0a2acfdb0cdb7fffebf95487f1dae1..56a86380a4f4caa6f202b6db497d93a139290076 100644 --- a/ee/spec/support/helpers/features/security_policy_helpers.rb +++ b/ee/spec/support/helpers/features/security_policy_helpers.rb @@ -11,7 +11,6 @@ def create_security_policy end def create_policy_setup - stub_feature_flags(merge_when_checks_pass: false) stub_feature_flags(bulk_create_scan_result_policies: false) stub_feature_flags(custom_software_license: false) stub_licensed_features(security_dashboard: true, diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 53ca36ed642f26cdb7daa698a7c37f8f5b87793d..3a24dda6daaf6ecbce5e1bd29c2bf43898dbe38e 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -662,18 +662,6 @@ def set_auto_merge let(:status) { 'merge_when_checks_pass' } let(:not_current_pipeline_status) { 'merge_when_checks_pass' } end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it_behaves_like 'api merge with auto merge' do - let(:service_class) { AutoMerge::MergeWhenPipelineSucceedsService } - let(:status) { 'merge_when_pipeline_succeeds' } - let(:not_current_pipeline_status) { 'failed' } - end - end end describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 62ec00f49530a323b8419a491a5b582af064acdb..3da3b4070f3ae03a3c972cfb48b4ceb664dd4d89 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -137,7 +137,9 @@ auto_merge_enabled { true } auto_merge_strategy { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS } merge_user { author } - merge_params { { sha: diff_head_sha } } + merge_params do + { 'auto_merge_strategy' => AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, sha: diff_head_sha } + end end trait :merge_when_checks_pass do diff --git a/spec/features/merge_request/user_resolves_wip_mr_spec.rb b/spec/features/merge_request/user_resolves_wip_mr_spec.rb index ac61e6993754b1c2f1436c3ad8e96a5e52a7a297..e79796ad7177a80e49e42b44c574857b6de0b4af 100644 --- a/spec/features/merge_request/user_resolves_wip_mr_spec.rb +++ b/spec/features/merge_request/user_resolves_wip_mr_spec.rb @@ -30,11 +30,7 @@ end context 'when there is active pipeline for merge request' do - let(:feature_flags_state) { true } - before do - stub_feature_flags(merge_when_checks_pass: feature_flags_state) - create(:ci_build, pipeline: pipeline) sign_in(user) @@ -58,27 +54,5 @@ expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}") expect(page).to have_content("Set to auto-merge") end - - context 'when the new merge_when_checks_pass and merge blocked components are disabled' do - let(:feature_flags_state) { false } - - it 'retains merge request data after clicking Resolve WIP status' do - expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}") - expect(page).to have_content "Merge blocked: 1 check failed" - expect(page).to have_content "Merge request must not be draft" - - page.within('.mr-state-widget') do - click_button('Mark as ready') - end - - wait_for_requests - - # If we don't disable the wait here, the test will wait until the - # merge request widget refreshes, which masks missing elements - # that should already be present. - expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}") - expect(page).not_to have_content("Merge blocked") - end - end end end diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index e4c937686f6b45c34c4c5afc9ad4725a089d0e52..66f6f43e54e2485caacfc8dc5749b008c9bcd5a3 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -331,34 +331,19 @@ def click_expand_button visit project_merge_request_path(project_only_mwps, merge_request_in_only_mwps_project) end - context 'when using merge when pipeline succeeds' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'is not allowed to merge' do - # Wait for the `ci_status` and `merge_check` requests - wait_for_requests - - expect(page).not_to have_selector('.accept-merge-request') - end - end - - context 'when using merge when checks pass' do - it 'is not allowed to set auto merge' do - # Wait for the `ci_status` and `merge_check` requests - wait_for_requests + it 'is not allowed to set auto merge' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests - expect(page).to have_selector('.accept-merge-request') - end + expect(page).to have_selector('.accept-merge-request') end end - context 'view merge request with MWPS enabled but automatically merge fails' do + context 'view merge request with auto merge enabled but automatically merge fails' do before do merge_request.update!( auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS, merge_user: merge_request.author, merge_error: 'Something went wrong' ) @@ -376,7 +361,7 @@ def click_expand_button end end - context 'view merge request with MWPS enabled but automatically merge fails' do + context 'view merge request with auto merge enabled but automatically merge fails' do before do merge_request.update!( merge_when_pipeline_succeeds: true, diff --git a/spec/graphql/mutations/merge_requests/accept_spec.rb b/spec/graphql/mutations/merge_requests/accept_spec.rb index d0820fbb412575f8e32d6498936af19572245d25..529c8fe651e4fc5af4e3ca883cf9d6a5e56c59d4 100644 --- a/spec/graphql/mutations/merge_requests/accept_spec.rb +++ b/spec/graphql/mutations/merge_requests/accept_spec.rb @@ -94,23 +94,6 @@ end expect(result).to include(errors: be_empty, merge_request: be_auto_merge_enabled) end - - context 'when merge_when_checks_pass is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - let(:merge_request) { create(:merge_request, :with_head_pipeline, source_project: project) } - let(:strategy) { ::Types::MergeStrategyEnum.values['MERGE_WHEN_PIPELINE_SUCCEEDS'].value } - let(:additional_args) { { auto_merge_strategy: strategy } } - - it "can use the MERGE_WHEN_PIPELINE_SUCCEEDS strategy" do - expect_next_found_instance_of(MergeRequest) do |instance| - expect(instance).not_to receive(:merge_async) - end - expect(result).to include(errors: be_empty, merge_request: be_auto_merge_enabled) - end - end end end end diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index 5723d1a4b7aaa6223313bb0f6f100c74f80fa409..eaee2cb905e89605bab486a939a41b83f7c1fa49 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -164,8 +164,8 @@ end end - context 'when MR is set to merge when pipeline succeeds' do - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, target_project: project, source_project: project) } + context 'when MR is set to auto merge' do + let(:merge_request) { create(:merge_request, :merge_when_checks_pass, target_project: project, source_project: project) } it 'is not nil' do value = resolve_field(:merge_user, merge_request) diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index c3fa9e1ea909a7e2465ac6f1460f4ccae35b5fa5..54bf741f0a215af70e7e143f00d4e0d81eb467f8 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -173,7 +173,7 @@ def values expect(created_object.target_project).to equal(project) end - it 'has MWPS set to false' do + it 'has auto merge set to false' do expect(created_object.merge_when_pipeline_succeeds).to eq(false) end diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index df2861f67d259c7300ee361987abe24e4c7e6540..b6590701544604e3272aea543a29201b989ee7d4 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -313,7 +313,7 @@ def match_mr1_note(content_regex) end end - it 'sets MWPS to false for all merge requests' do + it 'sets auto merge to false for all merge requests' do MergeRequest.find_each do |merge_request| expect(merge_request.merge_when_pipeline_succeeds).to eq(false) end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8649cfb4f3defe3d3f17e722d71bac4dc05c477c..07872926906b96dffeff209cfd4102cf6ef5851f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1822,7 +1822,7 @@ def create_build(name, status) describe 'auto merge' do context 'when auto merge is enabled' do - let_it_be_with_reload(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let_it_be_with_reload(:merge_request) { create(:merge_request, :merge_when_checks_pass) } let_it_be_with_reload(:pipeline) do create(:ci_pipeline, :running, project: merge_request.source_project, ref: merge_request.source_branch, sha: merge_request.diff_head_sha) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 48d1a2332cc270dabf9b902ef5d7ae958f29b44f..438d8c90283031156d6df4376b72d1adab7c769d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2312,18 +2312,10 @@ def set_compare(merge_request) describe "#auto_merge_strategy" do subject { merge_request.auto_merge_strategy } - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } it { is_expected.to eq('merge_when_checks_pass') } - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it { is_expected.to eq('merge_when_pipeline_succeeds') } - end - context 'when auto merge is disabled' do let(:merge_request) { create(:merge_request) } @@ -2334,17 +2326,9 @@ def set_compare(merge_request) describe '#default_auto_merge_strategy' do subject { merge_request.default_auto_merge_strategy } - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } it { is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS) } - - context 'when merge_when_checks_pass feature flag is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it { is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) } - end end describe '#committers' do @@ -3936,7 +3920,6 @@ def set_compare(merge_request) where(:auto_merge_strategy, :skip_checks) do '' | false - AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS | false AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true end @@ -6034,7 +6017,7 @@ def transition! let!(:merge_request1) do create( :merge_request, - :merge_when_pipeline_succeeds, + :merge_when_checks_pass, target_project: project, target_branch: 'master', source_project: project, diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 66fb86660e1d82f7cadac3b4d35a000608f90243..3366fdf713d05b87461e0ddc36f1e0504b2e1664 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1583,10 +1583,10 @@ end context 'merge_user' do - context 'when MR is set to MWPS' do - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project, target_project: project) } + context 'when MR is set to auto merge' do + let(:merge_request) { create(:merge_request, :merge_when_checks_pass, source_project: project, target_project: project) } - it 'returns user who set MWPS' do + it 'returns user who set to auto merge' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(response).to have_gitlab_http_status(:ok) @@ -3179,45 +3179,6 @@ expect(response).to have_gitlab_http_status(:ok) end - context 'when merge_when_checks_pass is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it_behaves_like 'merging with auto merge strategies' - - it 'does not enable auto merge if MR is not mergeable and only_allow_merge_if_pipeline_succeeds is true' do - allow_any_instance_of(MergeRequest) - .to receive_messages( - head_pipeline: pipeline, - diff_head_pipeline: pipeline - ) - - merge_request.update!(title: 'Draft: 1234') - - project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) - - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - - expect(response).to have_gitlab_http_status(:method_not_allowed) - expect(merge_request.reload.state).to eq('opened') - end - - context 'when the pipeline failed' do - let(:pipeline) { create(:ci_pipeline, :failed, project: project) } - - it 'does not enable auto merge if the pipeline failed and only_allow_merge_if_pipeline_succeeds is true' do - allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) - project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) - - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - - expect(response).to have_gitlab_http_status(:method_not_allowed) - expect(merge_request.reload.state).to eq('opened') - end - end - end - it_behaves_like 'merging with auto merge strategies' it 'enables auto merge if the MR is not mergeable and only_allow_merge_if_pipeline_succeeds is true' do @@ -3990,7 +3951,7 @@ describe 'POST :id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do before do - ::AutoMergeService.new(merge_request.target_project, user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + ::AutoMergeService.new(merge_request.target_project, user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS) end it 'removes the merge_when_pipeline_succeeds status' do diff --git a/spec/serializers/merge_request_poll_widget_entity_spec.rb b/spec/serializers/merge_request_poll_widget_entity_spec.rb index 878bb8b0086baf3d7df9a4930805b04caa34b0f4..2d2b9282173aa39be952c669ee39b151d45743ca 100644 --- a/spec/serializers/merge_request_poll_widget_entity_spec.rb +++ b/spec/serializers/merge_request_poll_widget_entity_spec.rb @@ -49,21 +49,11 @@ end context 'when auto merge is enabled' do - let(:resource) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:resource) { create(:merge_request, :merge_when_checks_pass) } it 'returns auto merge related information' do expect(subject[:auto_merge_strategy]).to eq('merge_when_checks_pass') end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'returns auto merge related information' do - expect(subject[:auto_merge_strategy]).to eq('merge_when_pipeline_succeeds') - end - end end context 'when auto merge is not enabled' do @@ -83,16 +73,6 @@ it 'returns available auto merge strategies' do expect(subject[:available_auto_merge_strategies]).to eq(%w[merge_when_checks_pass]) end - - context 'when the merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'returns available auto merge strategies' do - expect(subject[:available_auto_merge_strategies]).to eq(%w[merge_when_pipeline_succeeds]) - end - end end describe 'squash defaults for projects' do diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb index 8022f81f2905fb38b30b881a7c77867191e18d64..3323fac76ddba476b3e75d6fe5b8f6ee6d3070f0 100644 --- a/spec/services/auto_merge/base_service_spec.rb +++ b/spec/services/auto_merge/base_service_spec.rb @@ -56,32 +56,6 @@ end end - context 'when strategy is merge when pipeline succeeds' do - let(:service) { AutoMerge::MergeWhenPipelineSucceedsService.new(project, user) } - - before do - pipeline = build(:ci_pipeline) - allow(merge_request).to receive(:diff_head_pipeline) { pipeline } - end - - it 'sets the auto merge strategy' do - subject - - merge_request.reload - expect(merge_request.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) - end - - it 'returns activated strategy name' do - is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS.to_sym) - end - - it 'calls AutoMergeProcessWorker' do - expect(AutoMergeProcessWorker).to receive(:perform_async).with({ 'merge_request_id' => merge_request.id }).once - - subject - end - end - context 'when failed to save merge request' do before do allow(merge_request).to receive(:save!) { raise ActiveRecord::RecordInvalid } @@ -133,7 +107,7 @@ def execute_with_error_in_yield describe '#update' do subject { service.update(merge_request) } # rubocop:disable Rails/SaveBang - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } context 'when merge params are specified' do let(:params) do @@ -178,7 +152,7 @@ def execute_with_error_in_yield 'should_remove_source_branch' => false, 'commit_message' => "Merge branch 'patch-12' into 'master'", 'squash_commit_message' => "Update README.md", - 'auto_merge_strategy' => 'merge_when_pipeline_succeeds' + 'auto_merge_strategy' => 'merge_when_checks_pass' }) end @@ -207,7 +181,7 @@ def execute_with_error_in_yield describe '#cancel' do subject { service.cancel(merge_request) } - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } it_behaves_like 'Canceled or Dropped' @@ -253,7 +227,7 @@ def cancel_with_error_in_yield describe '#abort' do subject { service.abort(merge_request, reason) } - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } let(:reason) { 'an error' } it_behaves_like 'Canceled or Dropped' diff --git a/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb b/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb index 66ef2104680e14c63b48f198e05531b009b36340..7803aa2aa3f994bcea032c46124ee7bf0e74ae32 100644 --- a/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb +++ b/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb @@ -15,12 +15,6 @@ describe '#available_for?' do subject { service.available_for?(mr_merge_if_green_enabled) } - let(:feature_flag) { true } - - before do - stub_feature_flags(merge_when_checks_pass: feature_flag) - end - context 'when immediately mergeable' do context 'when a non active pipeline' do before do @@ -47,24 +41,12 @@ end end - context 'when merge when checks pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end - context 'when draft status' do before do mr_merge_if_green_enabled.update!(title: 'Draft: check') end it { is_expected.to eq true } - - context 'when merge_when_checks_pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end end context 'when discussions open' do @@ -75,12 +57,6 @@ end it { is_expected.to eq true } - - context 'when merge_when_checks_pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end end context 'when pipline is active' do @@ -94,12 +70,6 @@ end it { is_expected.to eq true } - - context 'when merge_when_checks_pass flag is off' do - let(:feature_flag) { false } - - it { is_expected.to eq false } - end end context 'when the user does not have permission to merge' do diff --git a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb index d9b8a10e33866ae227159f724079dfda06cffbe2..ea89bb8cd629144c9b7a4eebdae4a0e92870dc8e 100644 --- a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb @@ -5,12 +5,6 @@ RSpec.describe AutoMerge::MergeWhenPipelineSucceedsService, feature_category: :code_review_workflow do include_context 'for auto_merge strategy context' - let(:merge_when_checks_pass_ff) { false } - - before do - stub_feature_flags(merge_when_checks_pass: merge_when_checks_pass_ff) - end - describe "#available_for?" do subject { service.available_for?(mr_merge_if_green_enabled) } @@ -27,19 +21,7 @@ mr_merge_if_green_enabled.update_head_pipeline end - it { is_expected.to be_truthy } - - context 'when merge when checks ff is true' do - let(:merge_when_checks_pass_ff) { true } - - it { is_expected.to be_falsey } - end - - it 'memoizes the result' do - expect(mr_merge_if_green_enabled).to receive(:can_be_merged_by?).once.and_call_original - - 2.times { is_expected.to be_truthy } - end + it { is_expected.to be_falsey } context 'when the head pipeline succeeded' do let(:pipeline_status) { :success } diff --git a/spec/services/auto_merge_service_spec.rb b/spec/services/auto_merge_service_spec.rb index 50dec47163250e99822dfaf9a2fb31b192e8af65..070e237f8e71f4fbfe3e7c00f0e323c31dc1ab53 100644 --- a/spec/services/auto_merge_service_spec.rb +++ b/spec/services/auto_merge_service_spec.rb @@ -52,17 +52,7 @@ is_expected.to include('merge_when_checks_pass') end - context 'when merge_when_checks_pass is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'returns available strategies' do - is_expected.to include('merge_when_pipeline_succeeds') - end - end - - context 'when the head piipeline succeeded' do + context 'when the head pipeline succeeded' do let(:pipeline_status) { :success } it 'returns available strategies' do @@ -157,19 +147,15 @@ end end - context 'when the strategy is MWPS and merge_when_checks_pass is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - + context 'when the strategy is MWPS' do let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS } - it 'delegates to a relevant service instance' do + it 'does not call execute and returns failed' do expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| - expect(service).to receive(:execute).with(merge_request) + expect(service).not_to receive(:execute).with(merge_request) end - subject + expect(subject).to eq(:failed) end end @@ -213,10 +199,6 @@ context 'when the merge request is MWPS' do let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - before do - stub_feature_flags(merge_when_checks_pass: false) - end - it 'delegates to a relevant service instance' do expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| expect(service).to receive(:update).with(merge_request) @@ -254,10 +236,6 @@ context 'when the merge request is MWPS' do let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - before do - stub_feature_flags(merge_when_checks_pass: false) - end - it 'delegates to a relevant service instance' do expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| expect(service).to receive(:process).with(merge_request) @@ -294,10 +272,6 @@ context 'when the merge request is MWPS' do let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - before do - stub_feature_flags(merge_when_checks_pass: false) - end - it 'delegates to a relevant service instance' do expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| expect(service).to receive(:cancel).with(merge_request) @@ -338,10 +312,6 @@ context 'when the merge request is MWPS' do let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } - before do - stub_feature_flags(merge_when_checks_pass: false) - end - it 'delegates to a relevant service instance' do expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| expect(service).to receive(:abort).with(merge_request, error) diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb index 79b0a4bbb7e4313eade42f60f5e878c3e48291c7..15fcaedd2bcbad5406f2f4b561d88b2796ec5e5b 100644 --- a/spec/services/discussions/resolve_service_spec.rb +++ b/spec/services/discussions/resolve_service_spec.rb @@ -6,7 +6,7 @@ describe '#execute' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user, developer_of: project) } - let_it_be(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project) } + let_it_be(:merge_request) { create(:merge_request, :merge_when_checks_pass, source_project: project) } let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } let(:service) { described_class.new(project, user, one_or_more_discussions: discussion) } @@ -46,18 +46,6 @@ .to publish_event(MergeRequests::DiscussionsResolvedEvent) .with(current_user_id: user.id, merge_request_id: merge_request.id) end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'schedules an auto-merge' do - expect(AutoMergeProcessWorker).to receive(:perform_async) - - service.execute - end - end end context 'when not all discussions are resolved' do @@ -66,18 +54,6 @@ it 'does not publish the discussions resolved event' do expect { service.execute }.not_to publish_event(MergeRequests::DiscussionsResolvedEvent) end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'schedules an auto-merge' do - expect(AutoMergeProcessWorker).to receive(:perform_async) - - described_class.new(project, user, one_or_more_discussions: [discussion, other_discussion]).execute - end - end end it 'sends GraphQL triggers' do diff --git a/spec/services/discussions/unresolve_service_spec.rb b/spec/services/discussions/unresolve_service_spec.rb index a9eb4d8a992eac2c59383d99d0cf424da7fb4e60..8b2843bb6770195b47f6c233387c396de2ef9e44 100644 --- a/spec/services/discussions/unresolve_service_spec.rb +++ b/spec/services/discussions/unresolve_service_spec.rb @@ -6,7 +6,7 @@ describe "#execute" do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user, developer_of: project) } - let_it_be(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project) } + let_it_be(:merge_request) { create(:merge_request, :merge_when_checks_pass, source_project: project) } let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index f140d3bfe63e51bc97a1c7ef0d4e613ee3e2871a..c2c27ff206ed2478e699f24adaa7d3238e2bc598 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -58,7 +58,7 @@ def execute end context 'when auto merge is enabled' do - let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + let(:merge_request) { create(:merge_request, :merge_when_checks_pass) } it 'cancels the auto merge' do expect(@merge_request).not_to be_auto_merge_enabled diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb index 020d433352cadb050a41023a287d7f451e0bda3b..fa95c72b826f6a70f1492737a898dc546acb74a5 100644 --- a/spec/services/merge_requests/merge_orchestration_service_spec.rb +++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb @@ -119,16 +119,6 @@ it 'fetches preferred auto merge strategy' do is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS) end - - context 'when merge_when_checks_pass feature is off' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'fetches preferred auto merge strategy' do - is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) - end - end end context 'when merge request cannot be merged automatically' do diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 9d5b53be55e782a97c441cb4d6b72ac90be3d57b..7da872e1a25c28f603ce10e17874cc61b82e21d7 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -198,11 +198,11 @@ let(:params) { { delete_source_branch: true } } it 'aborts auto merges' do - mr_1 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: merge_request.source_branch, + mr_1 = create(:merge_request, :merge_when_checks_pass, target_branch: merge_request.source_branch, source_branch: "test", source_project: merge_request.project) mr_2 = create(:merge_request, :merge_when_checks_pass, target_branch: merge_request.source_branch, source_branch: "feature", source_project: merge_request.project) - mr_3 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: 'feature', + mr_3 = create(:merge_request, :merge_when_checks_pass, target_branch: 'feature', source_branch: 'second', source_project: merge_request.project) expect(merge_request.source_project.merge_requests.with_auto_merge_enabled).to contain_exactly(mr_1, mr_2, mr_3) @@ -213,30 +213,11 @@ context 'when source branch is not be deleted' do it 'does not abort any auto merges' do - mr_1 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: merge_request.source_branch, + mr_1 = create(:merge_request, :merge_when_checks_pass, target_branch: merge_request.source_branch, source_branch: "test", source_project: merge_request.project) mr_2 = create(:merge_request, :merge_when_checks_pass, target_branch: merge_request.source_branch, source_branch: "feature", source_project: merge_request.project) - mr_3 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: 'feature', - source_branch: 'second', source_project: merge_request.project) - - expect(merge_request.source_project.merge_requests.with_auto_merge_enabled).to contain_exactly(mr_1, mr_2, mr_3) - subject - expect(merge_request.source_project.merge_requests.with_auto_merge_enabled).to contain_exactly(mr_1, mr_2, mr_3) - end - end - - context 'when merge_when_checks_pass is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not aborts any auto merges' do - mr_1 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: merge_request.source_branch, - source_branch: "test", source_project: merge_request.project) - mr_2 = create(:merge_request, :merge_when_checks_pass, target_branch: merge_request.source_branch, - source_branch: "feature", source_project: merge_request.project) - mr_3 = create(:merge_request, :merge_when_pipeline_succeeds, target_branch: 'feature', + mr_3 = create(:merge_request, :merge_when_checks_pass, target_branch: 'feature', source_branch: 'second', source_project: merge_request.project) expect(merge_request.source_project.merge_requests.with_auto_merge_enabled).to contain_exactly(mr_1, mr_2, mr_3) diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb index 2690714e7fa94c4f081074234f05940b46edf012..0b6c9bcb5378ed614122e4ada08b1206a9566a1d 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -139,21 +139,6 @@ expect(last_mr.merge_user).to eq(user1) expect(last_mr.merge_params['sha']).to eq(change[:newrev]) end - - context 'when merge_when_checks_pass is false' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'sets auto_merge_enabled' do - service.execute - - expect(last_mr.auto_merge_enabled).to eq(true) - expect(last_mr.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) - expect(last_mr.merge_user).to eq(user1) - expect(last_mr.merge_params['sha']).to eq(change[:newrev]) - end - end end shared_examples_for 'a service that can remove the source branch when it is merged' do diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index b0a449e5674c47a68c207c633bff64a02af0431c..307d738f1205494ec0b1b5d508856565d4b3cb3d 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -26,7 +26,7 @@ target_branch: 'feature', target_project: @project, auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS, merge_user: @user ) @@ -37,7 +37,7 @@ target_branch: 'test', target_project: @project, auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS, + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS, merge_user: @user ) @@ -543,6 +543,15 @@ context 'With merged MR that contains the same SHA' do before do + @merge_request.head_pipeline = create( + :ci_pipeline, + :success, + project: @merge_request.source_project, + ref: @merge_request.source_branch, + sha: @merge_request.diff_head_sha) + + @merge_request.update_head_pipeline + # Merged via UI MergeRequests::MergeService .new(project: @merge_request.target_project, current_user: @user, params: { sha: @merge_request.diff_head_sha }) @@ -1012,7 +1021,7 @@ def reload_mrs target_project: project, merge_user: user, auto_merge_enabled: true, - auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS + auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS ) end @@ -1030,7 +1039,7 @@ def reload_mrs merge_request.reload end - it 'aborts MWPS for merge requests' do + it 'aborts auto merge for merge requests' do expect(merge_request.auto_merge_enabled?).to be_falsey expect(merge_request.merge_user).to be_nil end @@ -1038,7 +1047,7 @@ def reload_mrs context 'when merge params contains up-to-date sha' do let(:merge_sha) { newrev } - it 'maintains MWPS for merge requests' do + it 'maintains auto merge for merge requests' do expect(merge_request.auto_merge_enabled?).to be_truthy expect(merge_request.merge_user).to eq(user) end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 55435bb3fcdb0f9140ac376126500bd4e8ed445b..6a3b195f00d96ffa803aed6c738f53ec0a0d2ac9 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -857,7 +857,7 @@ def update_merge_request(opts) context 'when auto merge is enabled and target branch changed' do before do - AutoMergeService.new(project, user, { sha: merge_request.diff_head_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + AutoMergeService.new(project, user, { sha: merge_request.diff_head_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS) end it 'calls MergeRequests::ResolveTodosService#async_execute' do @@ -920,16 +920,6 @@ def update_merge_request(opts) end end - context 'when merge_when_checks_pass is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not publish a DraftStateChangeEvent' do - expect { update_merge_request(title: 'New title') }.not_to publish_event(MergeRequests::DraftStateChangeEvent) - end - end - context 'when removing through wip_event param' do it 'removes Draft from the title' do expect { update_merge_request({ wip_event: "ready" }) } @@ -967,16 +957,6 @@ def update_merge_request(opts) end end - context 'when merge_when_checks_pass is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it 'does not publish a DraftStateChangeEvent' do - expect { update_merge_request(title: 'Draft: New title') }.not_to publish_event(MergeRequests::DraftStateChangeEvent) - end - end - it 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do expect(GraphqlTriggers).to receive(:merge_request_merge_status_updated).with(merge_request) diff --git a/spec/support/shared_examples/workers/auto_merge_from_event_shared_examples.rb b/spec/support/shared_examples/workers/auto_merge_from_event_shared_examples.rb index 9db3ef43a85cd8bc898ed399fc2ccb3438471730..9e76fc6971eb65ad6ec3c4f850feff9193d026dc 100644 --- a/spec/support/shared_examples/workers/auto_merge_from_event_shared_examples.rb +++ b/spec/support/shared_examples/workers/auto_merge_from_event_shared_examples.rb @@ -33,17 +33,5 @@ .not_to raise_exception end end - - context 'when feature flag "merge_when_checks_pass" is disabled' do - before do - stub_feature_flags(merge_when_checks_pass: false) - end - - it "doesn't call AutoMergeService" do - expect(AutoMergeService).not_to receive(:new) - - consume_event(subscriber: described_class, event: event) - end - end end end