diff options
author | Markus Doits <markus.doits@stellenticket.de> | 2018-09-19 19:38:19 +0200 |
---|---|---|
committer | Markus Doits <markus.doits@stellenticket.de> | 2018-11-07 13:02:56 +0100 |
commit | fdb5a51352fdb4796d26ab2c9a1daaa5a89a0283 (patch) | |
tree | 40c9521566f52b9616eee225bab5b2a7259da61c | |
parent | ffcc200a68fd6d08707a8fd70b128a79206fd69e (diff) | |
download | gitlab-ce-fdb5a51352fdb4796d26ab2c9a1daaa5a89a0283.tar.gz |
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.
-rw-r--r-- | lib/gitlab/ci/config/entry/job.rb | 79 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/config/entry/job_spec.rb | 2 |
2 files changed, 45 insertions, 36 deletions
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 765053d8c33..76b417c4277 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -26,23 +26,36 @@ module Gitlab validates :name, type: Symbol validates :retry, hash_or_integer: true, allowed_keys: ALLOWED_KEYS_RETRY, allow_nil: true - validate :validate_retry + validate :validate_retry, if: :validate_retry? + + with_options allow_nil: true do + validates :tags, array_of_strings: true + validates :allow_failure, boolean: true + 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 + 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 - return if !config || - !config.is_a?(Hash) || - config[:retry].nil? || - !config[:retry].is_a?(Integer) && !config[:retry].is_a?(Hash) - - check = - if config[:retry].is_a?(Integer) - { max: config[:retry] } - else - config[:retry] - end - - validate_retry_max(check[:max]) - validate_retry_when(check[:when]) + if config[:retry].is_a?(Integer) + validate_retry_max(config[:retry]) + else + validate_retry_max(config[:retry][:max]) + validate_retry_when(config[:retry][:when]) + end end def validate_retry_max(retry_max) @@ -57,34 +70,30 @@ module Gitlab def validate_retry_when(retry_when) return if retry_when.blank? - possible_failures = Gitlab::Ci::Status::Build::Failed.reasons.keys.map(&:to_s) + ['always'] - if retry_when.is_a?(String) - unless possible_failures.include?(retry_when) - errors[:base] << 'retry when is unknown' - end + validate_retry_when_string(retry_when) elsif retry_when.is_a?(Array) - unknown_whens = retry_when - possible_failures - unless unknown_whens.empty? - errors[:base] << "retry when cannot have unknown failures #{unknown_whens.join(', ')}" - end + validate_retry_when_array(retry_when) else errors[:base] << 'retry when should be an array of strings or a string' end end - with_options allow_nil: true do - validates :tags, array_of_strings: true - validates :allow_failure, boolean: true - 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' } + def possible_retry_when_options + @possible_retry_when_options ||= Gitlab::Ci::Status::Build::Failed.reasons.keys.map(&:to_s) + ['always'] + end - validates :dependencies, array_of_strings: true - validates :extends, type: String + def validate_retry_when_string(retry_when) + 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 validates :start_in, duration: { limit: '1 day' }, if: :delayed? diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 9054e909a44..7d0a5b81084 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -242,7 +242,7 @@ describe Gitlab::Ci::Config::Entry::Job do it 'returns error about the wrong format' do 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 |