Commit 46fa3089 authored by Shinya Maeda's avatar Shinya Maeda

Rescue RecordNotUnique when pipeline is created with non-unique iid

parent 910a7d02
...@@ -23,7 +23,7 @@ module Gitlab ...@@ -23,7 +23,7 @@ module Gitlab
end end
end end
end end
rescue ActiveRecord::RecordInvalid => e rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => e
error("Failed to persist the pipeline: #{e}") error("Failed to persist the pipeline: #{e}")
end end
......
...@@ -37,6 +37,18 @@ describe Gitlab::Ci::Pipeline::Chain::Create do ...@@ -37,6 +37,18 @@ describe Gitlab::Ci::Pipeline::Chain::Create do
end end
context 'when pipeline has validation errors' do context 'when pipeline has validation errors' do
shared_examples_for 'expectations' do
it 'breaks the chain' do
expect(step.break?).to be true
end
it 'appends validation error' do
expect(pipeline.errors.to_a)
.to include /Failed to persist the pipeline/
end
end
context 'when ref is nil' do
let(:pipeline) do let(:pipeline) do
build(:ci_pipeline, project: project, ref: nil) build(:ci_pipeline, project: project, ref: nil)
end end
...@@ -45,13 +57,18 @@ describe Gitlab::Ci::Pipeline::Chain::Create do ...@@ -45,13 +57,18 @@ describe Gitlab::Ci::Pipeline::Chain::Create do
step.perform! step.perform!
end end
it 'breaks the chain' do it_behaves_like 'expectations'
expect(step.break?).to be true
end end
it 'appends validation error' do context 'when pipeline has a duplicate iid' do
expect(pipeline.errors.to_a) before do
.to include /Failed to persist the pipeline/ allow_any_instance_of(Ci::Pipeline).to receive(:ensure_project_iid!) { |p| p.send(:write_attribute, :iid, 1) }
create(:ci_pipeline, project: project)
step.perform!
end
it_behaves_like 'expectations'
end end
end end
end end
...@@ -41,7 +41,7 @@ describe Ci::Pipeline, :mailer do ...@@ -41,7 +41,7 @@ describe Ci::Pipeline, :mailer do
let(:instance) { build(:ci_pipeline) } let(:instance) { build(:ci_pipeline) }
let(:scope_attrs) { { project: instance.project } } let(:scope_attrs) { { project: instance.project } }
let(:usage) { :ci_pipelines } let(:usage) { :ci_pipelines }
let(:validate_presence) { false } let(:allow_nil) { true }
end end
end end
......
require 'spec_helper' require 'spec_helper'
shared_examples_for 'AtomicInternalId' do shared_examples_for 'AtomicInternalId' do
let(:validate_presence) { true } let(:allow_nil) { false }
describe '.has_internal_id' do describe '.has_internal_id' do
describe 'Module inclusion' do describe 'Module inclusion' do
...@@ -18,7 +18,11 @@ shared_examples_for 'AtomicInternalId' do ...@@ -18,7 +18,11 @@ shared_examples_for 'AtomicInternalId' do
it 'validates presence' do it 'validates presence' do
instance.valid? instance.valid?
expect(instance.errors[:iid]).to include("can't be blank") if validate_presence if allow_nil
expect(instance.errors[:iid]).to be_empty
else
expect(instance.errors[:iid]).to include("can't be blank")
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