Commit 9818bb56 authored by Markus Doits's avatar Markus Doits

refactoring after latest feedback

parent ff3484a0
...@@ -318,15 +318,11 @@ module Ci ...@@ -318,15 +318,11 @@ module Ci
end end
def retries_max def retries_max
retries = self.options[:retry] options&.dig(:retry, :max) || 0
retries.is_a?(Hash) && retries.fetch(:max, 0) || retries || 0
end end
def retry_when def retry_when
retries = self.options[:retry] options&.dig(:retry, :when) || ['always']
Array.wrap(
retries.is_a?(Hash) && retries.fetch(:when, 'always') || 'always'
)
end end
def retry_failure? def retry_failure?
......
...@@ -18,6 +18,12 @@ module Gitlab ...@@ -18,6 +18,12 @@ module Gitlab
less_than_or_equal_to: 2 } less_than_or_equal_to: 2 }
end end
def value
{
max: config
}
end
def location def location
'retry' 'retry'
end end
...@@ -49,7 +55,15 @@ module Gitlab ...@@ -49,7 +55,15 @@ module Gitlab
end end
def self.possible_retry_when_values def self.possible_retry_when_values
@possible_retry_when_values ||= Gitlab::Ci::Status::Build::Failed.reasons.keys.map(&:to_s) + ['always'] @possible_retry_when_values ||= CommitStatus.failure_reasons.keys.map(&:to_s) + ['always']
end
def value
super.tap do |config|
# make sure that `when` is an array, because we allow it to
# be passed as a String in config for simplicity
config[:when] = Array.wrap(config[:when]) if config[:when]
end
end end
def location def location
......
...@@ -3,72 +3,125 @@ require 'spec_helper' ...@@ -3,72 +3,125 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Retry do describe Gitlab::Ci::Config::Entry::Retry do
let(:entry) { described_class.new(config) } let(:entry) { described_class.new(config) }
describe 'validation' do shared_context 'when retry value is a numeric', :numeric do
context 'when retry value is correct' do let(:config) { max }
context 'when it is a numeric' do let(:max) {}
let(:config) { 2 } end
it 'is valid' do shared_context 'when retry value is a hash', :hash do
expect(entry).to be_valid let(:config) { { max: max, when: public_send(:when) }.compact }
end let(:when) {}
let(:max) {}
end
describe '#value' do
subject(:value) { entry.value }
context 'when retry value is a numeric', :numeric do
let(:max) { 2 }
it 'is returned as a hash with max key' do
expect(value).to eq(max: 2)
end end
end
context 'when it is a hash without when' do context 'when retry value is a hash', :hash do
let(:config) { { max: 2 } } context 'and `when` is a string' do
let(:when) { 'unknown_failure' }
it 'is valid' do it 'returns when wrapped in an array' do
expect(entry).to be_valid expect(value).to eq(when: ['unknown_failure'])
end end
end end
context 'when it is a hash with string when' do context 'and `when` is an array' do
let(:config) { { max: 2, when: 'unknown_failure' } } let(:when) { %w[unknown_failure runner_system_failure] }
it 'is valid' do it 'returns when as it was passed' do
expect(entry).to be_valid expect(value).to eq(when: %w[unknown_failure runner_system_failure])
end end
end end
end
end
context 'when it is a hash with string when always' do describe 'validation' do
let(:config) { { max: 2, when: 'always' } } context 'when retry value is correct' do
context 'when it is a numeric', :numeric do
let(:max) { 2 }
it 'is valid' do it 'is valid' do
expect(entry).to be_valid expect(entry).to be_valid
end end
end end
context 'when it is a hash with array when' do context 'when it is a hash', :hash do
let(:config) { { max: 2, when: %w[unknown_failure runner_system_failure] } } context 'with max' do
let(:max) { 2 }
it 'is valid' do it 'is valid' do
expect(entry).to be_valid expect(entry).to be_valid
end
end
context 'with string when' do
let(:when) { 'unknown_failure' }
it 'is valid' do
expect(entry).to be_valid
end
end
context 'with string when always' do
let(:when) { 'always' }
it 'is valid' do
expect(entry).to be_valid
end
end end
end
# Those values are documented at `doc/ci/yaml/README.md`. If any of context 'with array when' do
# those values gets invalid, documentation must be updated. To make let(:when) { %w[unknown_failure runner_system_failure] }
# sure this is catched, check explicitly that all of the documented
# values are valid. If they are not it means the documentation and this
# array must be updated.
RETRY_WHEN_IN_DOCUMENTATION = %w[
always
unknown_failure
script_failure
api_failure
stuck_or_timeout_failure
runner_system_failure
missing_dependency_failure
runner_unsupported
].freeze
RETRY_WHEN_IN_DOCUMENTATION.each do |reason|
context "when it is a hash with value from documentation `#{reason}`" do
let(:config) { { max: 2, when: reason } }
it 'is valid' do it 'is valid' do
expect(entry).to be_valid expect(entry).to be_valid
end end
end end
# Those values are documented at `doc/ci/yaml/README.md`. If any of
# those values gets invalid, documentation must be updated. To make
# sure this is catched, check explicitly that all of the documented
# values are valid. If they are not it means the documentation and this
# array must be updated.
RETRY_WHEN_IN_DOCUMENTATION = %w[
always
unknown_failure
script_failure
api_failure
stuck_or_timeout_failure
runner_system_failure
missing_dependency_failure
runner_unsupported
].freeze
RETRY_WHEN_IN_DOCUMENTATION.each do |reason|
context "with when from documentation `#{reason}`" do
let(:when) { reason }
it 'is valid' do
expect(entry).to be_valid
end
end
end
CommitStatus.failure_reasons.each_key do |reason|
context "with when from CommitStatus.failure_reasons `#{reason}`" do
let(:when) { reason }
it 'is valid' do
expect(entry).to be_valid
end
end
end
end end
end end
...@@ -82,9 +135,9 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -82,9 +135,9 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'not defined as a hash' do context 'when it is a numeric', :numeric do
context 'when it is lower than zero' do context 'when it is lower than zero' do
let(:config) { -1 } let(:max) { -1 }
it 'returns error about value too low' do it 'returns error about value too low' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -94,7 +147,7 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -94,7 +147,7 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
context 'when it is not an integer' do context 'when it is not an integer' do
let(:config) { 1.5 } let(:max) { 1.5 }
it 'returns error about wrong value' do it 'returns error about wrong value' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -103,7 +156,7 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -103,7 +156,7 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
context 'when the value is too high' do context 'when the value is too high' do
let(:config) { 10 } let(:max) { 10 }
it 'returns error about value too high' do it 'returns error about value too high' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -112,7 +165,7 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -112,7 +165,7 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'defined as a hash' do context 'when it is a hash', :hash do
context 'with unknown keys' do context 'with unknown keys' do
let(:config) { { max: 2, unknown_key: :something, one_more: :key } } let(:config) { { max: 2, unknown_key: :something, one_more: :key } }
...@@ -123,8 +176,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -123,8 +176,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when max is lower than zero' do context 'with max lower than zero' do
let(:config) { { max: -1 } } let(:max) { -1 }
it 'returns error about value too low' do it 'returns error about value too low' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -133,8 +186,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -133,8 +186,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when max is not an integer' do context 'with max not an integer' do
let(:config) { { max: 1.5 } } let(:max) { 1.5 }
it 'returns error about wrong value' do it 'returns error about wrong value' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -142,8 +195,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -142,8 +195,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when max is too high' do context 'iwth max too high' do
let(:config) { { max: 10 } } let(:max) { 10 }
it 'returns error about value too high' do it 'returns error about value too high' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -151,8 +204,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -151,8 +204,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when when has the wrong format' do context 'with when in wrong format' do
let(:config) { { when: true } } let(:when) { true }
it 'returns error about the wrong format' do it 'returns error about the wrong format' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -160,8 +213,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -160,8 +213,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when when is a string and unknown' do context 'with an unknown when string' do
let(:config) { { when: 'unknown_reason' } } let(:when) { 'unknown_reason' }
it 'returns error about the wrong format' do it 'returns error about the wrong format' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
...@@ -169,8 +222,8 @@ describe Gitlab::Ci::Config::Entry::Retry do ...@@ -169,8 +222,8 @@ describe Gitlab::Ci::Config::Entry::Retry do
end end
end end
context 'when when is an array and includes unknown failures' do context 'with an unknown failure reason in a when array' do
let(:config) { { when: %w[unknown_reason runner_system_failure] } } let(:when) { %w[unknown_reason runner_system_failure] }
it 'returns error about the wrong format' do it 'returns error about the wrong format' do
expect(entry).not_to be_valid expect(entry).not_to be_valid
......
...@@ -1472,15 +1472,7 @@ describe Ci::Build do ...@@ -1472,15 +1472,7 @@ describe Ci::Build do
end end
describe '#retries_max' do describe '#retries_max' do
context 'when max retries value is defined as an integer' do context 'with retries max config option' do
subject { create(:ci_build, options: { retry: 1 }) }
it 'returns the number of configured max retries' do
expect(subject.retries_max).to eq 1
end
end
context 'when retries value is defined as a hash' do
subject { create(:ci_build, options: { retry: { max: 1 } }) } subject { create(:ci_build, options: { retry: { max: 1 } }) }
it 'returns the number of configured max retries' do it 'returns the number of configured max retries' do
...@@ -1488,15 +1480,7 @@ describe Ci::Build do ...@@ -1488,15 +1480,7 @@ describe Ci::Build do
end end
end end
context 'when retries value is defined as a hash without max key' do context 'without retries max config option' do
subject { create(:ci_build, options: { retry: { something: :else } }) }
it 'returns zero' do
expect(subject.retries_max).to eq 0
end
end
context 'when max retries value is not defined' do
subject { create(:ci_build) } subject { create(:ci_build) }
it 'returns zero' do it 'returns zero' do
...@@ -1514,34 +1498,18 @@ describe Ci::Build do ...@@ -1514,34 +1498,18 @@ describe Ci::Build do
end end
describe '#retry_when' do describe '#retry_when' do
context 'when value is defined without an array' do context 'with retries when config option' do
subject { create(:ci_build, options: { retry: { when: 'something' } }) } subject { create(:ci_build, options: { retry: { when: ['some_reason'] } }) }
it 'returns the configured value inside an array' do
expect(subject.retry_when).to eq ['something']
end
end
context 'when value is defined as an array' do
subject { create(:ci_build, options: { retry: { when: %w[something more] } }) }
it 'returns the configured value' do it 'returns the configured when' do
expect(subject.retry_when).to eq %w[something more] expect(subject.retry_when).to eq ['some_reason']
end end
end end
context 'when value is not defined' do context 'without retries when config option' do
subject { create(:ci_build) } subject { create(:ci_build) }
it 'returns `always`' do it 'returns always array' do
expect(subject.retry_when).to eq ['always']
end
end
context 'when retry is only defined as an integer' do
subject { create(:ci_build, options: { retry: 1 }) }
it 'returns `always`' do
expect(subject.retry_when).to eq ['always'] expect(subject.retry_when).to eq ['always']
end end
end end
...@@ -3001,7 +2969,7 @@ describe Ci::Build do ...@@ -3001,7 +2969,7 @@ describe Ci::Build do
end end
context 'when build is configured to be retried' do context 'when build is configured to be retried' do
subject { create(:ci_build, :running, options: { retry: 3 }, project: project, user: user) } subject { create(:ci_build, :running, options: { retry: { max: 3 } }, project: project, user: user) }
it 'retries build and assigns the same user to it' do it 'retries build and assigns the same user to it' do
expect(described_class).to receive(:retry) expect(described_class).to receive(:retry)
......
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