Commit 9da3ae70 authored by Hiroyuki Sato's avatar Hiroyuki Sato

Don't create merge request pipeline without commits

parent 49efb579
...@@ -54,6 +54,7 @@ module Ci ...@@ -54,6 +54,7 @@ module Ci
validates :ref, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? }
validates :merge_request, presence: { if: :merge_request? } validates :merge_request, presence: { if: :merge_request? }
validates :merge_request, absence: { unless: :merge_request? } validates :merge_request, absence: { unless: :merge_request? }
validate :presence_of_commits_in_merge_request, if: :merge_request?
validates :tag, inclusion: { in: [false], if: :merge_request? } validates :tag, inclusion: { in: [false], if: :merge_request? }
validates :status, presence: { unless: :importing? } validates :status, presence: { unless: :importing? }
validate :valid_commit_sha, unless: :importing? validate :valid_commit_sha, unless: :importing?
...@@ -345,6 +346,12 @@ module Ci ...@@ -345,6 +346,12 @@ module Ci
end end
end end
def presence_of_commits_in_merge_request
if merge_request&.has_no_commits?
errors.add(:merge_request, "must have commits")
end
end
def git_author_name def git_author_name
strong_memoize(:git_author_name) do strong_memoize(:git_author_name) do
commit.try(:author_name) commit.try(:author_name)
......
---
title: Don't create new merge request pipeline without commits
merge_request: 24503
author: Hiroyuki Sato
type: added
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Ci::Pipeline, :mailer do describe Ci::Pipeline, :mailer do
let(:user) { create(:user) } let(:user) { create(:user) }
set(:project) { create(:project) } set(:project) { create(:project, :repository) }
let(:pipeline) do let(:pipeline) do
create(:ci_empty_pipeline, status: :created, project: project) create(:ci_empty_pipeline, status: :created, project: project)
...@@ -131,12 +131,18 @@ describe Ci::Pipeline, :mailer do ...@@ -131,12 +131,18 @@ describe Ci::Pipeline, :mailer do
context 'when source is merge request' do context 'when source is merge request' do
let(:source) { :merge_request } let(:source) { :merge_request }
context 'when merge request is specified' do context 'when merge request with commits is specified' do
let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_project: project, target_branch: 'master') } let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_project: project, target_branch: 'master') }
it { expect(pipeline).to be_valid } it { expect(pipeline).to be_valid }
end end
context 'when merge request without commits is specified' do
let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'not-merged-branch', target_project: project, target_branch: 'master') }
it { expect(pipeline).not_to be_valid }
end
context 'when merge request is empty' do context 'when merge request is empty' do
let(:merge_request) { nil } let(:merge_request) { nil }
...@@ -1036,6 +1042,8 @@ describe Ci::Pipeline, :mailer do ...@@ -1036,6 +1042,8 @@ describe Ci::Pipeline, :mailer do
end end
context 'when repository does not exist' do context 'when repository does not exist' do
let(:project) { create(:project) }
let(:pipeline) do let(:pipeline) do
create(:ci_empty_pipeline, project: project, ref: 'master') create(:ci_empty_pipeline, project: project, ref: 'master')
end end
...@@ -2071,7 +2079,6 @@ describe Ci::Pipeline, :mailer do ...@@ -2071,7 +2079,6 @@ describe Ci::Pipeline, :mailer do
end end
describe "#all_merge_requests" do describe "#all_merge_requests" do
let(:project) { create(:project) }
let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master') } let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master') }
it "returns all merge requests having the same source branch" do it "returns all merge requests having the same source branch" do
......
...@@ -1378,7 +1378,7 @@ describe MergeRequest do ...@@ -1378,7 +1378,7 @@ describe MergeRequest do
source_project: project, source_project: project,
source_branch: source_ref, source_branch: source_ref,
target_project: project, target_project: project,
target_branch: 'stable') target_branch: 'merge-test')
end end
before do before do
......
...@@ -736,6 +736,15 @@ describe Ci::CreatePipelineService do ...@@ -736,6 +736,15 @@ describe Ci::CreatePipelineService do
end end
end end
context 'when there are no commits between source branch and target branch' do
let(:ref_name) { 'refs/heads/not-merged-branch' }
it 'does not create a merge request pipeline' do
expect(pipeline).not_to be_persisted
expect(pipeline.errors[:merge_request]).to eq(["must have commits"])
end
end
context 'when merge request is created from a forked project' do context 'when merge request is created from a forked project' do
let(:merge_request) do let(:merge_request) do
create(:merge_request, create(:merge_request,
......
...@@ -150,11 +150,15 @@ describe MergeRequests::RefreshService do ...@@ -150,11 +150,15 @@ describe MergeRequests::RefreshService do
} }
end end
it 'create merge request pipeline' do it 'create merge request pipeline with commits' do
expect { subject } expect { subject }
.to change { @merge_request.merge_request_pipelines.count }.by(1) .to change { @merge_request.merge_request_pipelines.count }.by(1)
.and change { @fork_merge_request.merge_request_pipelines.count }.by(1) .and change { @fork_merge_request.merge_request_pipelines.count }.by(1)
.and change { @another_merge_request.merge_request_pipelines.count }.by(1) .and change { @another_merge_request.merge_request_pipelines.count }.by(0)
expect(@merge_request.has_commits?).to be true
expect(@fork_merge_request.has_commits?).to be true
expect(@another_merge_request.has_commits?).to be false
end end
context "when branch pipeline was created before a merge request pipline has been created" do context "when branch pipeline was created before a merge request pipline has been created" do
......
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