Skip to content

G
gitlab-ce
Commit 15eaa062 authored by nicolasdular's avatar nicolasdular
Browse files
Options

De-dupe MR update service EE specs 

De-duplicates the MR update service specs
parent c8557573
Show whitespace changes
Inline Side-by-side
Showing with 223 additions and 248 deletions
.rubocop.yml
View file @ 15eaa062
...@@ -219,12 +219,10 @@ Gitlab/DuplicateSpecLocation: ...@@ -219,12 +219,10 @@ Gitlab/DuplicateSpecLocation:
- ee/spec/models/namespace_spec.rb - ee/spec/models/namespace_spec.rb
- ee/spec/services/merge_requests/create_service_spec.rb - ee/spec/services/merge_requests/create_service_spec.rb
- ee/spec/services/merge_requests/refresh_service_spec.rb - ee/spec/services/merge_requests/refresh_service_spec.rb
- ee/spec/services/merge_requests/update_service_spec.rb
- ee/spec/helpers/ee/auth_helper_spec.rb - ee/spec/helpers/ee/auth_helper_spec.rb
- ee/spec/models/ee/namespace_spec.rb - ee/spec/models/ee/namespace_spec.rb
- ee/spec/services/ee/merge_requests/create_service_spec.rb - ee/spec/services/ee/merge_requests/create_service_spec.rb
- ee/spec/services/ee/merge_requests/refresh_service_spec.rb - ee/spec/services/ee/merge_requests/refresh_service_spec.rb
- ee/spec/services/ee/merge_requests/update_service_spec.rb
Cop/InjectEnterpriseEditionModule: Cop/InjectEnterpriseEditionModule:
Enabled: true Enabled: true
......
ee/spec/services/ee/merge_requests/update_service_spec.rb
View file @ 15eaa062
...@@ -2,21 +2,32 @@ ...@@ -2,21 +2,32 @@
require 'spec_helper' require 'spec_helper'
describe MergeRequests::UpdateService do describe MergeRequests::UpdateService, :mailer do
include ProjectForksHelper include ProjectForksHelper
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:service) { described_class.new(project, user, {}) } let(:user2) { create(:user) }
let(:user3) { create(:user) }
let(:label) { create(:label, project: project) }
let(:label2) { create(:label) }
let(:merge_request) do let(:merge_request) do
create(:merge_request, :simple, title: 'Old title', create(
:merge_request,
:simple,
title: 'Old title',
description: "FYI #{user2.to_reference}",
assignee_id: user3.id,
source_project: project, source_project: project,
author: user) author: create(:user)
)
end end
before do before do
allow(service).to receive(:execute_hooks) project.add_maintainer(user)
project.add_developer(user2)
project.add_developer(user3)
end end
describe '#execute' do describe '#execute' do
...@@ -24,5 +35,211 @@ describe MergeRequests::UpdateService do ...@@ -24,5 +35,211 @@ describe MergeRequests::UpdateService do
let(:issuable) { merge_request } let(:issuable) { merge_request }
let(:parent) { project } let(:parent) { project }
end end
def update_merge_request(opts)
described_class.new(project, user, opts).execute(merge_request)
end
context 'when code owners changes' do
let(:code_owner) { create(:user) }
before do
project.add_maintainer(code_owner)
allow(merge_request).to receive(:code_owners).and_return([], [code_owner])
end
it 'does not create any todos' do
expect do
update_merge_request(title: 'New title')
end.not_to change { Todo.count }
end
it 'does not send any emails' do
expect do
update_merge_request(title: 'New title')
end.not_to change { ActionMailer::Base.deliveries.count }
end
end
context 'when approvals_before_merge changes' do
using RSpec::Parameterized::TableSyntax
where(:project_value, :mr_before_value, :mr_after_value, :result) do
3 | 4 | 5 | 5
3 | 4 | nil | 3
3 | nil | 5 | 5
end
with_them do
let(:project) { create(:project, :repository, approvals_before_merge: project_value) }
it "does not update" do
merge_request.update(approvals_before_merge: mr_before_value)
rule = create(:approval_merge_request_rule, merge_request: merge_request)
update_merge_request(approvals_before_merge: mr_after_value)
expect(rule.reload.approvals_required).to eq(0)
end
end
end
context 'merge' do
let(:opts) { { merge: merge_request.diff_head_sha } }
context 'when not approved' do
before do
merge_request.update(approvals_before_merge: 1)
perform_enqueued_jobs do
update_merge_request(opts)
@merge_request = MergeRequest.find(merge_request.id)
end
end
it { expect(@merge_request).to be_valid }
it { expect(@merge_request.state).to eq('opened') }
end
context 'when approved' do
before do
merge_request.update(approvals_before_merge: 1)
merge_request.approvals.create(user: user)
perform_enqueued_jobs do
update_merge_request(opts)
@merge_request = MergeRequest.find(merge_request.id)
end
end
it { expect(@merge_request).to be_valid }
it 'is in the "merge" state', :sidekiq_might_not_need_inline do
expect(@merge_request.state).to eq('merged')
end
end
end
context 'when the approvers change' do
let(:existing_approver) { create(:user) }
let(:removed_approver) { create(:user) }
let(:new_approver) { create(:user) }
before do
project.add_developer(existing_approver)
project.add_developer(removed_approver)
project.add_developer(new_approver)
perform_enqueued_jobs do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end
Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all # rubocop: disable DestroyAll
ActionMailer::Base.deliveries.clear
end
context 'when an approver is added and an approver is removed' do
before do
perform_enqueued_jobs do
update_merge_request(approver_ids: [new_approver, existing_approver].map(&:id).join(','))
end
end
it 'adds todos for and sends emails to the new approvers' do
expect(Todo.where(user: new_approver, action: Todo::APPROVAL_REQUIRED)).not_to be_empty
should_email(new_approver)
end
it 'does not add todos for or send emails to the existing approvers' do
expect(Todo.where(user: existing_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
should_not_email(existing_approver)
end
it 'does not add todos for or send emails to the removed approvers' do
expect(Todo.where(user: removed_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
should_not_email(removed_approver)
end
end
context 'when the approvers are set to the same values' do
it 'does not create any todos' do
expect do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end.not_to change { Todo.count }
end
it 'does not send any emails' do
expect do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end.not_to change { ActionMailer::Base.deliveries.count }
end
end
end
context 'updating target_branch' do
let(:existing_approver) { create(:user) }
let(:new_approver) { create(:user) }
before do
project.add_developer(existing_approver)
project.add_developer(new_approver)
perform_enqueued_jobs do
update_merge_request(approver_ids: "#{existing_approver.id},#{new_approver.id}")
end
merge_request.target_project.update(reset_approvals_on_push: true)
merge_request.approvals.create(user_id: existing_approver.id)
end
it 'resets approvals when target_branch is changed' do
update_merge_request(target_branch: 'video')
expect(merge_request.reload.approvals).to be_empty
end
it 'creates new todos for the approvers' do
expect(Todo.where(action: Todo::APPROVAL_REQUIRED).map(&:user)).to contain_exactly(new_approver, existing_approver)
end
end
context 'updating blocking merge requests' do
it 'delegates to MergeRequests::UpdateBlocksService' do
expect(MergeRequests::UpdateBlocksService)
.to receive(:extract_params!)
.and_return(:extracted_params)
expect_next_instance_of(MergeRequests::UpdateBlocksService) do |service|
expect(service.merge_request).to eq(merge_request)
expect(service.current_user).to eq(user)
expect(service.params).to eq(:extracted_params)
expect(service).to receive(:execute)
end
update_merge_request({})
end
end
context 'when reassigned' do
it 'schedules for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker)
.to receive(:perform_async).with('Analytics::RefreshReassignData', merge_request.id, {})
update_merge_request({ assignee_ids: [user2.id] })
end
context 'when code_review_analytics is not available' do
before do
stub_licensed_features(code_review_analytics: false)
end
it 'does not schedule for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker).not_to receive(:perform_async)
update_merge_request({ assignee_ids: [user2.id] })
end
end
end
end end
end end
ee/spec/services/merge_requests/update_service_spec.rb deleted 100644 → 0
View file @ c8557573
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::UpdateService, :mailer do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
let(:label) { create(:label, project: project) }
let(:label2) { create(:label) }
let(:merge_request) do
create(
:merge_request,
:simple,
title: 'Old title',
description: "FYI #{user2.to_reference}",
assignee_id: user3.id,
source_project: project,
author: create(:user)
)
end
before do
project.add_maintainer(user)
project.add_developer(user2)
project.add_developer(user3)
end
describe '#execute' do
def update_merge_request(opts)
described_class.new(project, user, opts).execute(merge_request)
end
context 'when code owners changes' do
let(:code_owner) { create(:user) }
before do
project.add_maintainer(code_owner)
allow(merge_request).to receive(:code_owners).and_return([], [code_owner])
end
it 'does not create any todos' do
expect do
update_merge_request(title: 'New title')
end.not_to change { Todo.count }
end
it 'does not send any emails' do
expect do
update_merge_request(title: 'New title')
end.not_to change { ActionMailer::Base.deliveries.count }
end
end
context 'when approvals_before_merge changes' do
using RSpec::Parameterized::TableSyntax
where(:project_value, :mr_before_value, :mr_after_value, :result) do
3 | 4 | 5 | 5
3 | 4 | nil | 3
3 | nil | 5 | 5
end
with_them do
let(:project) { create(:project, :repository, approvals_before_merge: project_value) }
it "does not update" do
merge_request.update(approvals_before_merge: mr_before_value)
rule = create(:approval_merge_request_rule, merge_request: merge_request)
update_merge_request(approvals_before_merge: mr_after_value)
expect(rule.reload.approvals_required).to eq(0)
end
end
end
context 'merge' do
let(:opts) { { merge: merge_request.diff_head_sha } }
context 'when not approved' do
before do
merge_request.update(approvals_before_merge: 1)
perform_enqueued_jobs do
update_merge_request(opts)
@merge_request = MergeRequest.find(merge_request.id)
end
end
it { expect(@merge_request).to be_valid }
it { expect(@merge_request.state).to eq('opened') }
end
context 'when approved' do
before do
merge_request.update(approvals_before_merge: 1)
merge_request.approvals.create(user: user)
perform_enqueued_jobs do
update_merge_request(opts)
@merge_request = MergeRequest.find(merge_request.id)
end
end
it { expect(@merge_request).to be_valid }
it 'is in the "merge" state', :sidekiq_might_not_need_inline do
expect(@merge_request.state).to eq('merged')
end
end
end
context 'when the approvers change' do
let(:existing_approver) { create(:user) }
let(:removed_approver) { create(:user) }
let(:new_approver) { create(:user) }
before do
project.add_developer(existing_approver)
project.add_developer(removed_approver)
project.add_developer(new_approver)
perform_enqueued_jobs do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end
Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all # rubocop: disable DestroyAll
ActionMailer::Base.deliveries.clear
end
context 'when an approver is added and an approver is removed' do
before do
perform_enqueued_jobs do
update_merge_request(approver_ids: [new_approver, existing_approver].map(&:id).join(','))
end
end
it 'adds todos for and sends emails to the new approvers' do
expect(Todo.where(user: new_approver, action: Todo::APPROVAL_REQUIRED)).not_to be_empty
should_email(new_approver)
end
it 'does not add todos for or send emails to the existing approvers' do
expect(Todo.where(user: existing_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
should_not_email(existing_approver)
end
it 'does not add todos for or send emails to the removed approvers' do
expect(Todo.where(user: removed_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
should_not_email(removed_approver)
end
end
context 'when the approvers are set to the same values' do
it 'does not create any todos' do
expect do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end.not_to change { Todo.count }
end
it 'does not send any emails' do
expect do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end.not_to change { ActionMailer::Base.deliveries.count }
end
end
end
context 'updating target_branch' do
let(:existing_approver) { create(:user) }
let(:new_approver) { create(:user) }
before do
project.add_developer(existing_approver)
project.add_developer(new_approver)
perform_enqueued_jobs do
update_merge_request(approver_ids: "#{existing_approver.id},#{new_approver.id}")
end
merge_request.target_project.update(reset_approvals_on_push: true)
merge_request.approvals.create(user_id: existing_approver.id)
end
it 'resets approvals when target_branch is changed' do
update_merge_request(target_branch: 'video')
expect(merge_request.reload.approvals).to be_empty
end
it 'creates new todos for the approvers' do
expect(Todo.where(action: Todo::APPROVAL_REQUIRED).map(&:user)).to contain_exactly(new_approver, existing_approver)
end
end
context 'updating blocking merge requests' do
it 'delegates to MergeRequests::UpdateBlocksService' do
expect(MergeRequests::UpdateBlocksService)
.to receive(:extract_params!)
.and_return(:extracted_params)
expect_next_instance_of(MergeRequests::UpdateBlocksService) do |service|
expect(service.merge_request).to eq(merge_request)
expect(service.current_user).to eq(user)
expect(service.params).to eq(:extracted_params)
expect(service).to receive(:execute)
end
update_merge_request({})
end
end
context 'when reassigned' do
it 'schedules for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker)
.to receive(:perform_async).with('Analytics::RefreshReassignData', merge_request.id, {})
update_merge_request({ assignee_ids: [user2.id] })
end
context 'when code_review_analytics is not available' do
before do
stub_licensed_features(code_review_analytics: false)
end
it 'does not schedule for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker).not_to receive(:perform_async)
update_merge_request({ assignee_ids: [user2.id] })
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7