Commit fdb5a513 authored by Markus Doits's avatar Markus Doits

refactor for hopefully lower cognitive complexity

before:

- Method `validate_retry` has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
- Method `validate_retry_max` has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring.
- Method `validate_retry_when` has a Cognitive Complexity of 14 (exceeds 5 allowed). Consider refactoring.
parent ffcc200a
...@@ -26,23 +26,36 @@ module Gitlab ...@@ -26,23 +26,36 @@ module Gitlab
validates :name, type: Symbol validates :name, type: Symbol
validates :retry, hash_or_integer: true, allowed_keys: ALLOWED_KEYS_RETRY, allow_nil: true validates :retry, hash_or_integer: true, allowed_keys: ALLOWED_KEYS_RETRY, allow_nil: true
validate :validate_retry validate :validate_retry, if: :validate_retry?
def validate_retry with_options allow_nil: true do
return if !config || validates :tags, array_of_strings: true
!config.is_a?(Hash) || validates :allow_failure, boolean: true
config[:retry].nil? || validates :parallel, numericality: { only_integer: true,
!config[:retry].is_a?(Integer) && !config[:retry].is_a?(Hash) greater_than_or_equal_to: 2 }
validates :when,
inclusion: { in: %w[on_success on_failure always manual delayed],
message: 'should be on_success, on_failure, ' \
'always, manual or delayed' }
check = validates :dependencies, array_of_strings: true
validates :extends, type: String
end
def validate_retry?
config&.is_a?(Hash) &&
config[:retry].present? &&
(config[:retry].is_a?(Integer) || config[:retry].is_a?(Hash))
end
def validate_retry
if config[:retry].is_a?(Integer) if config[:retry].is_a?(Integer)
{ max: config[:retry] } validate_retry_max(config[:retry])
else else
config[:retry] validate_retry_max(config[:retry][:max])
validate_retry_when(config[:retry][:when])
end end
validate_retry_max(check[:max])
validate_retry_when(check[:when])
end end
def validate_retry_max(retry_max) def validate_retry_max(retry_max)
...@@ -57,34 +70,30 @@ module Gitlab ...@@ -57,34 +70,30 @@ module Gitlab
def validate_retry_when(retry_when) def validate_retry_when(retry_when)
return if retry_when.blank? return if retry_when.blank?
possible_failures = Gitlab::Ci::Status::Build::Failed.reasons.keys.map(&:to_s) + ['always']
if retry_when.is_a?(String) if retry_when.is_a?(String)
unless possible_failures.include?(retry_when) validate_retry_when_string(retry_when)
errors[:base] << 'retry when is unknown'
end
elsif retry_when.is_a?(Array) elsif retry_when.is_a?(Array)
unknown_whens = retry_when - possible_failures validate_retry_when_array(retry_when)
unless unknown_whens.empty?
errors[:base] << "retry when cannot have unknown failures #{unknown_whens.join(', ')}"
end
else else
errors[:base] << 'retry when should be an array of strings or a string' errors[:base] << 'retry when should be an array of strings or a string'
end end
end end
with_options allow_nil: true do def possible_retry_when_options
validates :tags, array_of_strings: true @possible_retry_when_options ||= Gitlab::Ci::Status::Build::Failed.reasons.keys.map(&:to_s) + ['always']
validates :allow_failure, boolean: true end
validates :parallel, numericality: { only_integer: true,
greater_than_or_equal_to: 2 }
validates :when,
inclusion: { in: %w[on_success on_failure always manual delayed],
message: 'should be on_success, on_failure, ' \
'always, manual or delayed' }
validates :dependencies, array_of_strings: true def validate_retry_when_string(retry_when)
validates :extends, type: String unless possible_retry_when_options.include?(retry_when)
errors[:base] << 'retry when is unknown'
end
end
def validate_retry_when_array(retry_when)
unknown_whens = retry_when - possible_retry_when_options
unless unknown_whens.empty?
errors[:base] << "retry when contains unknown values: #{unknown_whens.join(', ')}"
end
end end
validates :start_in, duration: { limit: '1 day' }, if: :delayed? validates :start_in, duration: { limit: '1 day' }, if: :delayed?
......
...@@ -242,7 +242,7 @@ describe Gitlab::Ci::Config::Entry::Job do ...@@ -242,7 +242,7 @@ describe Gitlab::Ci::Config::Entry::Job do
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
expect(entry.errors).to include 'job retry when cannot have unknown failures unknown_reason' expect(entry.errors).to include 'job retry when contains unknown values: unknown_reason'
end 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