diff options
Diffstat (limited to 'spec')
| -rw-r--r-- | spec/factories/merge_requests.rb | 5 | ||||
| -rw-r--r-- | spec/features/merge_requests/merge_when_build_succeeds_spec.rb | 21 | ||||
| -rw-r--r-- | spec/models/merge_request_spec.rb | 44 | ||||
| -rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb | 30 | ||||
| -rw-r--r-- | spec/services/merge_requests/refresh_service_spec.rb | 3 | ||||
| -rw-r--r-- | spec/services/system_note_service_spec.rb | 10 |
7 files changed, 84 insertions, 31 deletions
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 729a49c9f72..5b4d7f41bc4 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -65,6 +65,11 @@ FactoryGirl.define do target_branch "master" end + trait :merge_when_build_succeeds do + merge_when_build_succeeds true + merge_user author + end + factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:reopened] factory :merge_request_with_diffs, traits: [:with_diffs] diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index b25a3f05e29..2e64e903d1e 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -1,4 +1,6 @@ require 'spec_helper' +# rubocop:disable Lint/UselessAssignment +# As rubocop doesn't see a need for both `ci_commit` and `ci_build` feature 'Merge When Build Succeeds', feature: true, js: true do let(:user) { create(:user) } @@ -32,16 +34,20 @@ feature 'Merge When Build Succeeds', feature: true, js: true do it 'activates Merge When Build Succeeds feature' do expect(page).to have_link "Cancel Automatic Merge" - expect(page).to have_content "Approved by #{user.name} to be merged automatically when the build succeeds." + expect(page).to have_content "Set by #{user.name} to be merged automatically when the build succeeds." expect(page).to have_content "The source branch will not be removed." + + visit_merge_request(merge_request) # Needed to refresh the page + expect(page).to have_content /Enabled an automatic merge when the build for [0-9a-f]{8} succeeds/i end end end context 'When it is enabled' do - # No clue how, but push a new commit to the branch - let(:merge_request) { create(:merge_request_with_diffs, source_project: project, # source_branch: "mepmep", - author: user, title: "Bug NS-04", merge_when_build_succeeds: true) } + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, author: user, + merge_user: user, title: "MepMep", merge_when_build_succeeds: true) + end before do merge_request.source_project.team << [user, :master] @@ -60,10 +66,16 @@ feature 'Merge When Build Succeeds', feature: true, js: true do click_link "Cancel Automatic Merge" expect(page).to have_button "Merge When Build Succeeds" + + visit_merge_request(merge_request) # Needed to refresh the page + expect(page).to have_content "Cancelled the automatic merge" end it "allows the user to remove the source branch" do expect(page).to have_link "Remove Source Branch When Merged" + + click_link "Remove Source Branch When Merged" + expect(page).to have_content "The source branch will be removed" end end @@ -78,3 +90,4 @@ feature 'Merge When Build Succeeds', feature: true, js: true do visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) end end +# rubocop:enable Lint/UselessAssignment diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c7a9765825e..63e1cd1fb92 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -48,6 +48,24 @@ describe MergeRequest do describe 'validation' do it { is_expected.to validate_presence_of(:target_branch) } it { is_expected.to validate_presence_of(:source_branch) } + + context "Validation of merge user with Merge When Build succeeds" do + it "allows user to be nil when the feature is disabled" do + expect(subject).to be_valid + end + + it "is invalid without merge user" do + subject.merge_when_build_succeeds = true + expect(subject).not_to be_valid + end + + it "is valid with merge user" do + subject.merge_when_build_succeeds = true + subject.merge_user = build(:user) + + expect(subject).to be_valid + end + end end describe 'respond to' do @@ -175,31 +193,41 @@ describe MergeRequest do end describe '#can_remove_source_branch' do - let(:user) { build(:user)} + let(:user) { create(:user) } + let(:user2) { create(:user) } before do subject.source_project.team << [user, :master] - end - it "cant be merged when its a a protected branch" do - subject.source_project.protected_branches = []; + subject.source_branch = "feature" + subject.target_branch = "master" + subject.save! + end + it "can't be removed when its a protected branch" do + allow(subject.source_project).to receive(:protected_branch?).and_return(true) expect(subject.can_remove_source_branch?(user)).to be_falsey end it "cant remove a root ref" do - subject.source_branch = "master"; + subject.source_branch = "master" + subject.target_branch = "feature" expect(subject.can_remove_source_branch?(user)).to be_falsey end - it "is truthy in all other cases" do - expect(subject.can_remove_source_branch?(user)) + it "is unable to remove the source branch for a project the user cannot push to" do + expect(subject.can_remove_source_branch?(user2)).to be_falsey + end + + it "is can be removed in all other cases" do + expect(subject.can_remove_source_branch?(user)).to be_truthy end end describe "#reset_merge_when_build_succeeds" do - let(:merge_if_green) { create :merge_request, merge_when_build_succeeds: true } + let(:merge_if_green) { create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user) } + it "sets the item to false" do merge_if_green.reset_merge_when_build_succeeds merge_if_green.reload diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 91ae2059e95..6b7a066ac40 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -303,7 +303,7 @@ describe API::API, api: true do end describe "PUT /projects/:id/merge_request/:merge_request_id/merge" do - let (:ci_commit) { create(:ci_commit_without_jobs) } + let(:ci_commit) { create(:ci_commit_without_jobs) } it "should return merge_request in case of success" do put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 8638539173b..a62d88cde86 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -3,37 +3,38 @@ require 'spec_helper' describe MergeRequests::MergeWhenBuildSucceedsService do let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } - let(:mr_merge_if_green_enabled) { create(:merge_request, merge_when_build_succeeds: true, - source_branch: "source_branch", target_branch: project.default_branch, - source_project: project, target_project: project, state: "opened") } - let(:ci_commit) { create(:ci_commit_with_one_job, ref: mr_merge_if_green_enabled.source_branch) } + + let(:mr_merge_if_green_enabled) do + create(:merge_request, merge_when_build_succeeds: true, merge_user: user, + source_branch: "source_branch", target_branch: project.default_branch, + source_project: project, target_project: project, state: "opened") + end + let(:project) { create(:project) } + let(:ci_commit) { create(:ci_commit_with_one_job, ref: mr_merge_if_green_enabled.source_branch, gl_project: project) } let(:service) { MergeRequests::MergeWhenBuildSucceedsService.new(project, user, commit_message: 'Awesome message') } - before do - project.ci_commits = [ci_commit] - project.save! - end describe "#execute" do context 'first time enabling' do before do allow(merge_request).to receive(:ci_commit).and_return(ci_commit) + service.execute(merge_request) end it 'sets the params, merge_user, and flag' do - service.execute(merge_request) - expect(merge_request).to be_valid expect(merge_request.merge_when_build_succeeds).to be_truthy expect(merge_request.merge_params).to eq commit_message: 'Awesome message' expect(merge_request.merge_user).to be user + end + it 'creates a system note' do note = merge_request.notes.last - expect(note.note).to include 'Enabled an automatic merge when the build for' + expect(note.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-z]{8}/ end end - context 'allready approved' do + context 'already approved' do let(:service) { MergeRequests::MergeWhenBuildSucceedsService.new(project, user, new_key: true) } let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) } @@ -74,5 +75,10 @@ describe MergeRequests::MergeWhenBuildSucceedsService do expect(mr_merge_if_green_enabled.merge_params).to eq({}) expect(mr_merge_if_green_enabled.merge_user).to be nil end + + it 'Posts a system note' do + note = mr_merge_if_green_enabled.notes.last + expect(note.note).to include 'Cancelled the automatic merge' + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 18b2659c1f6..9a8174f95fd 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -18,7 +18,8 @@ describe MergeRequests::RefreshService do source_branch: 'master', target_branch: 'feature', target_project: @project, - merge_when_build_succeeds: true) + merge_when_build_succeeds: true, + merge_user: @user) @fork_merge_request = create(:merge_request, source_project: @fork_project, diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 5d41a5cdc69..333035f2d2c 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -208,20 +208,20 @@ describe SystemNoteService do end describe '.merge_when_build_succeeds' do - let(:ci_commit) { create :ci_commit_without_jobs } + let(:ci_commit) { build :ci_commit_without_jobs } let(:noteable) { create :merge_request } - subject { described_class.merge_when_build_succeeds(noteable, project, author, ci_commit) } + subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) } it_behaves_like 'a system note' it "posts the Merge When Build Succeeds system note" do - expect(subject.note).to eq "Enabled an automatic merge when the build for 97de212e80737a608d939f648d959671fb0a0142 succeeds" + expect(subject.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-f]{40} succeeds/ end end describe '.cancel_merge_when_build_succeeds' do - let(:ci_commit) { create :ci_commit_without_jobs } + let(:ci_commit) { build :ci_commit_without_jobs } let(:noteable) { create :merge_request } subject { described_class.cancel_merge_when_build_succeeds(noteable, project, author) } @@ -229,7 +229,7 @@ describe SystemNoteService do it_behaves_like 'a system note' it "posts the Merge When Build Succeeds system note" do - expect(subject.note).to eq "Canceled the automatic merge" + expect(subject.note).to eq "Cancelled the automatic merge" end end |