Commit 5ec10aa4 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'id-speed-up-merge-request-spec' into 'master'

Use FactoryDefault and let_it_be to speed up merge_request_spec

See merge request gitlab-org/gitlab!39996
parents db5f07b0 3b30daca
...@@ -2,13 +2,16 @@ ...@@ -2,13 +2,16 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe MergeRequest do RSpec.describe MergeRequest, factory_default: :keep do
include RepoHelpers include RepoHelpers
include ProjectForksHelper include ProjectForksHelper
include ReactiveCachingHelpers include ReactiveCachingHelpers
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let_it_be(:namespace) { create_default(:namespace) }
let_it_be(:project, refind: true) { create_default(:project, :repository) }
subject { create(:merge_request) } subject { create(:merge_request) }
describe 'associations' do describe 'associations' do
...@@ -360,7 +363,7 @@ RSpec.describe MergeRequest do ...@@ -360,7 +363,7 @@ RSpec.describe MergeRequest do
it 'returns merge requests that match the given merge commit' do it 'returns merge requests that match the given merge commit' do
note = create(:track_mr_picking_note, commit_id: '456abc') note = create(:track_mr_picking_note, commit_id: '456abc')
create(:track_mr_picking_note, commit_id: '456def') create(:track_mr_picking_note, project: create(:project), commit_id: '456def')
expect(described_class.by_cherry_pick_sha('456abc')).to eq([note.noteable]) expect(described_class.by_cherry_pick_sha('456abc')).to eq([note.noteable])
end end
...@@ -832,7 +835,7 @@ RSpec.describe MergeRequest do ...@@ -832,7 +835,7 @@ RSpec.describe MergeRequest do
end end
context 'with commit diff note' do context 'with commit diff note' do
let(:other_merge_request) { create(:merge_request) } let(:other_merge_request) { create(:merge_request, source_project: create(:project, :repository)) }
let!(:diff_note) do let!(:diff_note) do
create(:diff_note_on_commit, project: merge_request.project) create(:diff_note_on_commit, project: merge_request.project)
...@@ -1031,6 +1034,8 @@ RSpec.describe MergeRequest do ...@@ -1031,6 +1034,8 @@ RSpec.describe MergeRequest do
end end
describe '#closes_issues' do describe '#closes_issues' do
let(:project) { create(:project) }
let(:issue0) { create :issue, project: subject.project } let(:issue0) { create :issue, project: subject.project }
let(:issue1) { create :issue, project: subject.project } let(:issue1) { create :issue, project: subject.project }
...@@ -1038,6 +1043,8 @@ RSpec.describe MergeRequest do ...@@ -1038,6 +1043,8 @@ RSpec.describe MergeRequest do
let(:commit1) { double('commit1', safe_message: "Fixes #{issue0.to_reference}") } let(:commit1) { double('commit1', safe_message: "Fixes #{issue0.to_reference}") }
let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") } let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") }
subject { create(:merge_request, source_project: project) }
before do before do
subject.project.add_developer(subject.author) subject.project.add_developer(subject.author)
allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) allow(subject).to receive(:commits).and_return([commit0, commit1, commit2])
...@@ -1088,6 +1095,8 @@ RSpec.describe MergeRequest do ...@@ -1088,6 +1095,8 @@ RSpec.describe MergeRequest do
end end
context 'when the project has an external issue tracker' do context 'when the project has an external issue tracker' do
subject { create(:merge_request, source_project: create(:project, :repository)) }
before do before do
subject.project.add_developer(subject.author) subject.project.add_developer(subject.author)
commit = double(:commit, safe_message: 'Fixes TEST-3') commit = double(:commit, safe_message: 'Fixes TEST-3')
...@@ -1254,7 +1263,8 @@ RSpec.describe MergeRequest do ...@@ -1254,7 +1263,8 @@ RSpec.describe MergeRequest do
end end
describe "#source_branch_exists?" do describe "#source_branch_exists?" do
let(:merge_request) { subject } let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) }
let(:repository) { merge_request.source_project.repository } let(:repository) { merge_request.source_project.repository }
context 'when the source project is set' do context 'when the source project is set' do
...@@ -1730,16 +1740,14 @@ RSpec.describe MergeRequest do ...@@ -1730,16 +1740,14 @@ RSpec.describe MergeRequest do
describe '#has_test_reports?' do describe '#has_test_reports?' do
subject { merge_request.has_test_reports? } subject { merge_request.has_test_reports? }
let(:project) { create(:project, :repository) }
context 'when head pipeline has test reports' do context 'when head pipeline has test reports' do
let(:merge_request) { create(:merge_request, :with_test_reports, source_project: project) } let(:merge_request) { create(:merge_request, :with_test_reports) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'when head pipeline does not have test reports' do context 'when head pipeline does not have test reports' do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request) }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
...@@ -1748,16 +1756,14 @@ RSpec.describe MergeRequest do ...@@ -1748,16 +1756,14 @@ RSpec.describe MergeRequest do
describe '#has_accessibility_reports?' do describe '#has_accessibility_reports?' do
subject { merge_request.has_accessibility_reports? } subject { merge_request.has_accessibility_reports? }
let(:project) { create(:project, :repository) }
context 'when head pipeline has an accessibility reports' do context 'when head pipeline has an accessibility reports' do
let(:merge_request) { create(:merge_request, :with_accessibility_reports, source_project: project) } let(:merge_request) { create(:merge_request, :with_accessibility_reports) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'when head pipeline does not have accessibility reports' do context 'when head pipeline does not have accessibility reports' do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request) }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
...@@ -1766,27 +1772,23 @@ RSpec.describe MergeRequest do ...@@ -1766,27 +1772,23 @@ RSpec.describe MergeRequest do
describe '#has_coverage_reports?' do describe '#has_coverage_reports?' do
subject { merge_request.has_coverage_reports? } subject { merge_request.has_coverage_reports? }
let(:project) { create(:project, :repository) }
context 'when head pipeline has coverage reports' do context 'when head pipeline has coverage reports' do
let(:merge_request) { create(:merge_request, :with_coverage_reports, source_project: project) } let(:merge_request) { create(:merge_request, :with_coverage_reports) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'when head pipeline does not have coverage reports' do context 'when head pipeline does not have coverage reports' do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request) }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end end
end end
describe '#has_terraform_reports?' do describe '#has_terraform_reports?' do
let_it_be(:project) { create(:project, :repository) }
context 'when head pipeline has terraform reports' do context 'when head pipeline has terraform reports' do
it 'returns true' do it 'returns true' do
merge_request = create(:merge_request, :with_terraform_reports, source_project: project) merge_request = create(:merge_request, :with_terraform_reports)
expect(merge_request.has_terraform_reports?).to be_truthy expect(merge_request.has_terraform_reports?).to be_truthy
end end
...@@ -1794,7 +1796,7 @@ RSpec.describe MergeRequest do ...@@ -1794,7 +1796,7 @@ RSpec.describe MergeRequest do
context 'when head pipeline does not have terraform reports' do context 'when head pipeline does not have terraform reports' do
it 'returns false' do it 'returns false' do
merge_request = create(:merge_request, source_project: project) merge_request = create(:merge_request)
expect(merge_request.has_terraform_reports?).to be_falsey expect(merge_request.has_terraform_reports?).to be_falsey
end end
...@@ -1802,8 +1804,7 @@ RSpec.describe MergeRequest do ...@@ -1802,8 +1804,7 @@ RSpec.describe MergeRequest do
end end
describe '#calculate_reactive_cache' do describe '#calculate_reactive_cache' do
let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request) }
let(:merge_request) { create(:merge_request, source_project: project) }
subject { merge_request.calculate_reactive_cache(service_class_name) } subject { merge_request.calculate_reactive_cache(service_class_name) }
...@@ -2131,6 +2132,8 @@ RSpec.describe MergeRequest do ...@@ -2131,6 +2132,8 @@ RSpec.describe MergeRequest do
end end
describe '#can_be_reverted?' do describe '#can_be_reverted?' do
subject { create(:merge_request, source_project: create(:project, :repository)) }
context 'when there is no merge_commit for the MR' do context 'when there is no merge_commit for the MR' do
before do before do
subject.metrics.update!(merged_at: Time.current.utc) subject.metrics.update!(merged_at: Time.current.utc)
...@@ -2319,8 +2322,6 @@ RSpec.describe MergeRequest do ...@@ -2319,8 +2322,6 @@ RSpec.describe MergeRequest do
end end
describe '#participants' do describe '#participants' do
let(:project) { create(:project, :public) }
let(:mr) do let(:mr) do
create(:merge_request, source_project: project, target_project: project) create(:merge_request, source_project: project, target_project: project)
end end
...@@ -2428,9 +2429,7 @@ RSpec.describe MergeRequest do ...@@ -2428,9 +2429,7 @@ RSpec.describe MergeRequest do
end end
describe '#mergeable?' do describe '#mergeable?' do
let(:project) { create(:project) } subject { build_stubbed(:merge_request) }
subject { create(:merge_request, source_project: project) }
it 'returns false if #mergeable_state? is false' do it 'returns false if #mergeable_state? is false' do
expect(subject).to receive(:mergeable_state?) { false } expect(subject).to receive(:mergeable_state?) { false }
...@@ -2500,9 +2499,7 @@ RSpec.describe MergeRequest do ...@@ -2500,9 +2499,7 @@ RSpec.describe MergeRequest do
end end
describe '#mergeable_state?' do describe '#mergeable_state?' do
let(:project) { create(:project, :repository) } subject { create(:merge_request) }
subject { create(:merge_request, source_project: project) }
it 'checks if merge request can be merged' do it 'checks if merge request can be merged' do
allow(subject).to receive(:mergeable_ci_state?) { true } allow(subject).to receive(:mergeable_ci_state?) { true }
...@@ -2617,7 +2614,7 @@ RSpec.describe MergeRequest do ...@@ -2617,7 +2614,7 @@ RSpec.describe MergeRequest do
let(:pipeline) { create(:ci_empty_pipeline) } let(:pipeline) { create(:ci_empty_pipeline) }
context 'when it is only allowed to merge when build is green' do context 'when it is only allowed to merge when build is green' do
let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) }
subject { build(:merge_request, target_project: project) } subject { build(:merge_request, target_project: project) }
...@@ -2658,7 +2655,7 @@ RSpec.describe MergeRequest do ...@@ -2658,7 +2655,7 @@ RSpec.describe MergeRequest do
end end
context 'when it is only allowed to merge when build is green or skipped' do context 'when it is only allowed to merge when build is green or skipped' do
let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) } let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) }
subject { build(:merge_request, target_project: project) } subject { build(:merge_request, target_project: project) }
...@@ -2699,7 +2696,7 @@ RSpec.describe MergeRequest do ...@@ -2699,7 +2696,7 @@ RSpec.describe MergeRequest do
end end
context 'when merges are not restricted to green builds' do context 'when merges are not restricted to green builds' do
let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: false) } let_it_be(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: false) }
subject { build(:merge_request, target_project: project) } subject { build(:merge_request, target_project: project) }
...@@ -2743,7 +2740,7 @@ RSpec.describe MergeRequest do ...@@ -2743,7 +2740,7 @@ RSpec.describe MergeRequest do
let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) } let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do
let(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) } let_it_be(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) }
context 'with all discussions resolved' do context 'with all discussions resolved' do
before do before do
...@@ -2992,6 +2989,10 @@ RSpec.describe MergeRequest do ...@@ -2992,6 +2989,10 @@ RSpec.describe MergeRequest do
end end
describe '#branch_merge_base_commit' do describe '#branch_merge_base_commit' do
let(:project) { create(:project, :repository) }
subject { create(:merge_request, :with_diffs, source_project: project) }
context 'source and target branch exist' do context 'source and target branch exist' do
it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
it { expect(subject.branch_merge_base_commit).to be_a(Commit) } it { expect(subject.branch_merge_base_commit).to be_a(Commit) }
...@@ -3011,7 +3012,9 @@ RSpec.describe MergeRequest do ...@@ -3011,7 +3012,9 @@ RSpec.describe MergeRequest do
describe "#diff_refs" do describe "#diff_refs" do
context "with diffs" do context "with diffs" do
subject { create(:merge_request, :with_diffs) } let(:project) { create(:project, :repository) }
subject { create(:merge_request, :with_diffs, source_project: project) }
let(:expected_diff_refs) do let(:expected_diff_refs) do
Gitlab::Diff::DiffRefs.new( Gitlab::Diff::DiffRefs.new(
...@@ -3213,7 +3216,8 @@ RSpec.describe MergeRequest do ...@@ -3213,7 +3216,8 @@ RSpec.describe MergeRequest do
pipeline pipeline
end end
let(:project) { create(:project, :public, :repository, only_allow_merge_if_pipeline_succeeds: true) } let_it_be(:project) { create(:project, :public, :repository, only_allow_merge_if_pipeline_succeeds: true) }
let(:developer) { create(:user) } let(:developer) { create(:user) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
...@@ -3307,8 +3311,7 @@ RSpec.describe MergeRequest do ...@@ -3307,8 +3311,7 @@ RSpec.describe MergeRequest do
end end
describe '#pipeline_coverage_delta' do describe '#pipeline_coverage_delta' do
let!(:project) { create(:project, :repository) } let!(:merge_request) { create(:merge_request) }
let!(:merge_request) { create(:merge_request, source_project: project) }
let!(:source_pipeline) do let!(:source_pipeline) do
create(:ci_pipeline, create(:ci_pipeline,
...@@ -3414,7 +3417,9 @@ RSpec.describe MergeRequest do ...@@ -3414,7 +3417,9 @@ RSpec.describe MergeRequest do
end end
describe '#merge_request_diff_for' do describe '#merge_request_diff_for' do
subject { create(:merge_request, importing: true) } let(:project) { create(:project, :repository) }
subject { create(:merge_request, importing: true, source_project: project) }
let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) }
...@@ -3445,9 +3450,10 @@ RSpec.describe MergeRequest do ...@@ -3445,9 +3450,10 @@ RSpec.describe MergeRequest do
end end
describe '#version_params_for' do describe '#version_params_for' do
subject { create(:merge_request, importing: true) } let(:project) { create(:project, :repository) }
subject { create(:merge_request, importing: true, source_project: project) }
let(:project) { subject.project }
let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } let!(:merge_request_diff1) { subject.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) } let!(:merge_request_diff2) { subject.merge_request_diffs.create(head_commit_sha: nil) }
let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } let!(:merge_request_diff3) { subject.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
...@@ -3478,6 +3484,10 @@ RSpec.describe MergeRequest do ...@@ -3478,6 +3484,10 @@ RSpec.describe MergeRequest do
end end
describe '#fetch_ref!' do describe '#fetch_ref!' do
let(:project) { create(:project, :repository) }
subject { create(:merge_request, :with_diffs, source_project: project) }
it 'fetches the ref correctly' do it 'fetches the ref correctly' do
expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
...@@ -3500,8 +3510,10 @@ RSpec.describe MergeRequest do ...@@ -3500,8 +3510,10 @@ RSpec.describe MergeRequest do
end end
context 'state machine transitions' do context 'state machine transitions' do
let(:project) { create(:project, :repository) }
describe '#unlock_mr' do describe '#unlock_mr' do
subject { create(:merge_request, state: 'locked', merge_jid: 123) } subject { create(:merge_request, state: 'locked', source_project: project, merge_jid: 123) }
it 'updates merge request head pipeline and sets merge_jid to nil', :sidekiq_might_not_need_inline do it 'updates merge request head pipeline and sets merge_jid to nil', :sidekiq_might_not_need_inline do
pipeline = create(:ci_empty_pipeline, project: subject.project, ref: subject.source_branch, sha: subject.source_branch_sha) pipeline = create(:ci_empty_pipeline, project: subject.project, ref: subject.source_branch, sha: subject.source_branch_sha)
...@@ -3518,7 +3530,7 @@ RSpec.describe MergeRequest do ...@@ -3518,7 +3530,7 @@ RSpec.describe MergeRequest do
let(:notification_service) { double(:notification_service) } let(:notification_service) { double(:notification_service) }
let(:todo_service) { double(:todo_service) } let(:todo_service) { double(:todo_service) }
subject { create(:merge_request, state, merge_status: :unchecked) } subject { create(:merge_request, state, source_project: project, merge_status: :unchecked) }
before do before do
allow(NotificationService).to receive(:new).and_return(notification_service) allow(NotificationService).to receive(:new).and_return(notification_service)
...@@ -3607,7 +3619,7 @@ RSpec.describe MergeRequest do ...@@ -3607,7 +3619,7 @@ RSpec.describe MergeRequest do
end end
context 'source branch is missing' do context 'source branch is missing' do
subject { create(:merge_request, :invalid, :opened, merge_status: :unchecked, target_branch: 'master') } subject { create(:merge_request, :invalid, :opened, source_project: project, merge_status: :unchecked, target_branch: 'master') }
before do before do
allow(subject.project.repository).to receive(:can_be_merged?).and_call_original allow(subject.project.repository).to receive(:can_be_merged?).and_call_original
...@@ -3640,10 +3652,8 @@ RSpec.describe MergeRequest do ...@@ -3640,10 +3652,8 @@ RSpec.describe MergeRequest do
end end
describe '#should_be_rebased?' do describe '#should_be_rebased?' do
let(:project) { create(:project, :repository) }
it 'returns false for the same source and target branches' do it 'returns false for the same source and target branches' do
merge_request = create(:merge_request, source_project: project, target_project: project) merge_request = build_stubbed(:merge_request, source_project: project, target_project: project)
expect(merge_request.should_be_rebased?).to be_falsey expect(merge_request.should_be_rebased?).to be_falsey
end end
...@@ -3658,7 +3668,7 @@ RSpec.describe MergeRequest do ...@@ -3658,7 +3668,7 @@ RSpec.describe MergeRequest do
end end
with_them do with_them do
let(:merge_request) { create(:merge_request) } let(:merge_request) { build_stubbed(:merge_request) }
subject { merge_request.rebase_in_progress? } subject { merge_request.rebase_in_progress? }
...@@ -3881,7 +3891,7 @@ RSpec.describe MergeRequest do ...@@ -3881,7 +3891,7 @@ RSpec.describe MergeRequest do
describe '#cleanup_refs' do describe '#cleanup_refs' do
subject { merge_request.cleanup_refs(only: only) } subject { merge_request.cleanup_refs(only: only) }
let(:merge_request) { build(:merge_request) } let(:merge_request) { build(:merge_request, source_project: create(:project, :repository)) }
context 'when removing all refs' do context 'when removing all refs' do
let(:only) { :all } let(:only) { :all }
......
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