Commit e86e1e51 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Try to report why it's failing and fix tests

parent 9984f07a
...@@ -23,7 +23,7 @@ module Ci ...@@ -23,7 +23,7 @@ module Ci
return error('Insufficient permissions to create a new pipeline') return error('Insufficient permissions to create a new pipeline')
end end
unless trigger_request && trigger_request.trigger.owner if trigger_request && !trigger_request.trigger.owner
return error('Legacy trigger without a owner is not allowed') return error('Legacy trigger without a owner is not allowed')
end end
......
...@@ -6,7 +6,8 @@ module Ci ...@@ -6,7 +6,8 @@ module Ci
pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref).
execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request)
trigger_request if pipeline.persisted? trigger_request.pipeline = pipeline
trigger_request
end end
end end
end end
...@@ -28,11 +28,12 @@ module API ...@@ -28,11 +28,12 @@ module API
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables)
if trigger_request pipeline = trigger_request.pipeline
present trigger_request.pipeline, with: Entities::Pipeline
if pipeline.persisted?
present pipeline, with: Entities::Pipeline
else else
errors = 'No pipeline created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
......
...@@ -29,11 +29,12 @@ module API ...@@ -29,11 +29,12 @@ module API
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables)
if trigger_request pipeline = trigger_request.pipeline
if pipeline.persisted?
present trigger_request, with: ::API::V3::Entities::TriggerRequest present trigger_request, with: ::API::V3::Entities::TriggerRequest
else else
errors = 'No builds created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
......
...@@ -25,11 +25,12 @@ module Ci ...@@ -25,11 +25,12 @@ module Ci
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref], variables) trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref], variables)
if trigger_request pipeline = trigger_request.pipeline
if pipeline.persisted?
present trigger_request, with: Entities::TriggerRequest present trigger_request, with: Entities::TriggerRequest
else else
errors = 'No builds created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
end end
......
...@@ -5,7 +5,14 @@ describe Ci::API::Triggers do ...@@ -5,7 +5,14 @@ describe Ci::API::Triggers do
let!(:trigger_token) { 'secure token' } let!(:trigger_token) { 'secure token' }
let!(:project) { create(:project, :repository, ci_id: 10) } let!(:project) { create(:project, :repository, ci_id: 10) }
let!(:project2) { create(:empty_project, ci_id: 11) } let!(:project2) { create(:empty_project, ci_id: 11) }
let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) }
let!(:trigger) do
create(:ci_trigger,
project: project,
token: trigger_token,
owner: create(:user))
end
let(:options) do let(:options) do
{ {
token: trigger_token token: trigger_token
...@@ -14,6 +21,8 @@ describe Ci::API::Triggers do ...@@ -14,6 +21,8 @@ describe Ci::API::Triggers do
before do before do
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
project.add_developer(trigger.owner)
end end
context 'Handles errors' do context 'Handles errors' do
...@@ -47,7 +56,8 @@ describe Ci::API::Triggers do ...@@ -47,7 +56,8 @@ describe Ci::API::Triggers do
it 'returns bad request with no builds created if there\'s no commit for that ref' do it 'returns bad request with no builds created if there\'s no commit for that ref' do
post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq('No builds created') expect(json_response['message']['base'])
.to contain_exactly('Reference not found')
end end
context 'Validates variables' do context 'Validates variables' do
......
...@@ -3,10 +3,13 @@ require 'spec_helper' ...@@ -3,10 +3,13 @@ require 'spec_helper'
describe Ci::CreateTriggerRequestService, services: true do describe Ci::CreateTriggerRequestService, services: true do
let(:service) { described_class.new } let(:service) { described_class.new }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:trigger) { create(:ci_trigger, project: project) } let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
let(:owner) { create(:user) }
before do before do
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
project.add_developer(owner)
end end
describe '#execute' do describe '#execute' do
...@@ -21,9 +24,6 @@ describe Ci::CreateTriggerRequestService, services: true do ...@@ -21,9 +24,6 @@ describe Ci::CreateTriggerRequestService, services: true do
end end
context 'with owner' do context 'with owner' do
let(:owner) { create(:user) }
let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
it { expect(subject).to be_kind_of(Ci::TriggerRequest) } it { expect(subject).to be_kind_of(Ci::TriggerRequest) }
it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) }
it { expect(subject.pipeline).to be_trigger } it { expect(subject.pipeline).to be_trigger }
...@@ -36,7 +36,7 @@ describe Ci::CreateTriggerRequestService, services: true do ...@@ -36,7 +36,7 @@ describe Ci::CreateTriggerRequestService, services: true do
context 'no commit for ref' do context 'no commit for ref' do
subject { service.execute(project, trigger, 'other-branch') } subject { service.execute(project, trigger, 'other-branch') }
it { expect(subject).to be_nil } it { expect(subject.pipeline).not_to be_persisted }
end end
context 'no builds created' do context 'no builds created' do
...@@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do ...@@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do
stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }')
end end
it { expect(subject).to be_nil } it { expect(subject.pipeline).not_to be_persisted }
end end
end end
end end
...@@ -82,6 +82,7 @@ describe PostReceive do ...@@ -82,6 +82,7 @@ describe PostReceive do
OpenStruct.new(id: '123456') OpenStruct.new(id: '123456')
end end
allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true)
allow_any_instance_of(Repository).to receive(:ref_exists?).and_return(true)
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
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