diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-11-07 15:02:18 +0000 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-11-07 15:02:18 +0000 |
commit | f323d6148b7b71609cbd32ab9db4724fc108729d (patch) | |
tree | 28762cadb685f35a7a4b70bbf243a3023e999262 /spec | |
parent | 5dc0577b84b9d41b1e2a6e781dfeaa400e8e2c10 (diff) | |
parent | 0c59fdaab69e93e2f00fdf70f2a31bb095e3d8d0 (diff) | |
download | gitlab-ce-f323d6148b7b71609cbd32ab9db4724fc108729d.tar.gz |
Merge branch 'max_retries_when' into 'master'
Allow to configure when to retry builds
Closes gitlab-runner#3515
See merge request gitlab-org/gitlab-ce!21758
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/ci/config/entry/job_spec.rb | 41 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/config/entry/retry_spec.rb | 236 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 108 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 32 | ||||
-rw-r--r-- | spec/services/ci/process_pipeline_service_spec.rb | 4 |
6 files changed, 369 insertions, 56 deletions
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index f1a2946acda..ac9b0c674a5 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::Ci::Config::Entry::Job do let(:result) do %i[before_script script stage type after_script cache image services only except variables artifacts - environment coverage] + environment coverage retry] end it { is_expected.to match_array result } @@ -98,45 +98,6 @@ describe Gitlab::Ci::Config::Entry::Job do end end - context 'when retry value is not correct' do - context 'when it is not a numeric value' do - let(:config) { { retry: true } } - - it 'returns error about invalid type' do - expect(entry).not_to be_valid - expect(entry.errors).to include 'job retry is not a number' - end - end - - context 'when it is lower than zero' do - let(:config) { { retry: -1 } } - - it 'returns error about value too low' do - expect(entry).not_to be_valid - expect(entry.errors) - .to include 'job retry must be greater than or equal to 0' - end - end - - context 'when it is not an integer' do - let(:config) { { retry: 1.5 } } - - it 'returns error about wrong value' do - expect(entry).not_to be_valid - expect(entry.errors).to include 'job retry must be an integer' - end - end - - context 'when the value is too high' do - let(:config) { { retry: 10 } } - - it 'returns error about value too high' do - expect(entry).not_to be_valid - expect(entry.errors).to include 'job retry must be less than or equal to 2' - end - end - end - context 'when parallel value is not correct' do context 'when it is not a numeric value' do let(:config) { { parallel: true } } diff --git a/spec/lib/gitlab/ci/config/entry/retry_spec.rb b/spec/lib/gitlab/ci/config/entry/retry_spec.rb new file mode 100644 index 00000000000..164a9ed4c3d --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/retry_spec.rb @@ -0,0 +1,236 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::Retry do + let(:entry) { described_class.new(config) } + + shared_context 'when retry value is a numeric', :numeric do + let(:config) { max } + let(:max) {} + end + + shared_context 'when retry value is a hash', :hash do + let(:config) { { max: max, when: public_send(:when) }.compact } + 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 + + context 'when retry value is a hash', :hash do + context 'and `when` is a string' do + let(:when) { 'unknown_failure' } + + it 'returns when wrapped in an array' do + expect(value).to eq(when: ['unknown_failure']) + end + end + + context 'and `when` is an array' do + let(:when) { %w[unknown_failure runner_system_failure] } + + it 'returns when as it was passed' do + expect(value).to eq(when: %w[unknown_failure runner_system_failure]) + end + end + end + end + + describe 'validation' do + context 'when retry value is correct' do + context 'when it is a numeric', :numeric do + let(:max) { 2 } + + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when it is a hash', :hash do + context 'with max' do + let(:max) { 2 } + + it 'is valid' do + 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 + + context 'with array when' do + let(:when) { %w[unknown_failure runner_system_failure] } + + it 'is valid' do + expect(entry).to be_valid + 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 + + ::Ci::Build.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 + + context 'when retry value is not correct' do + context 'when it is not a numeric nor an array' do + let(:config) { true } + + it 'returns error about invalid type' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry config has to be either an integer or a hash' + end + end + + context 'when it is a numeric', :numeric do + context 'when it is lower than zero' do + let(:max) { -1 } + + it 'returns error about value too low' do + expect(entry).not_to be_valid + expect(entry.errors) + .to include 'retry config must be greater than or equal to 0' + end + end + + context 'when it is not an integer' do + let(:max) { 1.5 } + + it 'returns error about wrong value' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry config has to be either an integer or a hash' + end + end + + context 'when the value is too high' do + let(:max) { 10 } + + it 'returns error about value too high' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry config must be less than or equal to 2' + end + end + end + + context 'when it is a hash', :hash do + context 'with unknown keys' do + let(:config) { { max: 2, unknown_key: :something, one_more: :key } } + + it 'returns error about the unknown key' do + expect(entry).not_to be_valid + expect(entry.errors) + .to include 'retry config contains unknown keys: unknown_key, one_more' + end + end + + context 'with max lower than zero' do + let(:max) { -1 } + + it 'returns error about value too low' do + expect(entry).not_to be_valid + expect(entry.errors) + .to include 'retry max must be greater than or equal to 0' + end + end + + context 'with max not an integer' do + let(:max) { 1.5 } + + it 'returns error about wrong value' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry max must be an integer' + end + end + + context 'iwth max too high' do + let(:max) { 10 } + + it 'returns error about value too high' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry max must be less than or equal to 2' + end + end + + context 'with when in wrong format' do + let(:when) { true } + + it 'returns error about the wrong format' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry when should be an array of strings or a string' + end + end + + context 'with an unknown when string' do + let(:when) { 'unknown_reason' } + + it 'returns error about the wrong format' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry when is not included in the list' + end + end + + context 'with an unknown failure reason in a when array' do + let(:when) { %w[unknown_reason runner_system_failure] } + + it 'returns error about the wrong format' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'retry when contains unknown values: unknown_reason' + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index dcfd54107a3..441e8214181 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -53,11 +53,11 @@ module Gitlab describe 'retry entry' do context 'when retry count is specified' do let(:config) do - YAML.dump(rspec: { script: 'rspec', retry: 1 }) + YAML.dump(rspec: { script: 'rspec', retry: { max: 1 } }) end it 'includes retry count in build options attribute' do - expect(subject[:options]).to include(retry: 1) + expect(subject[:options]).to include(retry: { max: 1 }) end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5bd2f096656..6849bc6db7a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1472,15 +1472,15 @@ describe Ci::Build do end describe '#retries_max' do - context 'when max retries value is defined' do - subject { create(:ci_build, options: { retry: 1 }) } + context 'with retries max config option' do + subject { create(:ci_build, options: { retry: { max: 1 } }) } - it 'returns a number of configured max retries' do + it 'returns the number of configured max retries' do expect(subject.retries_max).to eq 1 end end - context 'when max retries value is not defined' do + context 'without retries max config option' do subject { create(:ci_build) } it 'returns zero' do @@ -1495,6 +1495,104 @@ describe Ci::Build do expect(subject.retries_max).to eq 0 end end + + context 'with integer only 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 + end + + describe '#retry_when' do + context 'with retries when config option' do + subject { create(:ci_build, options: { retry: { when: ['some_reason'] } }) } + + it 'returns the configured when' do + expect(subject.retry_when).to eq ['some_reason'] + end + end + + context 'without retries when config option' do + subject { create(:ci_build) } + + it 'returns always array' do + expect(subject.retry_when).to eq ['always'] + end + end + + context 'with integer only config option' do + subject { create(:ci_build, options: { retry: 1 }) } + + it 'returns always array' do + expect(subject.retry_when).to eq ['always'] + end + end + end + + describe '#retry_failure?' do + subject { create(:ci_build) } + + context 'when retries max is zero' do + before do + expect(subject).to receive(:retries_max).at_least(:once).and_return(0) + end + + it 'returns false' do + expect(subject.retry_failure?).to eq false + end + end + + context 'when retries max equals retries count' do + before do + expect(subject).to receive(:retries_max).at_least(:once).and_return(1) + expect(subject).to receive(:retries_count).at_least(:once).and_return(1) + end + + it 'returns false' do + expect(subject.retry_failure?).to eq false + end + end + + context 'when retries max is higher than retries count' do + before do + expect(subject).to receive(:retries_max).at_least(:once).and_return(2) + expect(subject).to receive(:retries_count).at_least(:once).and_return(1) + end + + context 'and retry when is always' do + before do + expect(subject).to receive(:retry_when).at_least(:once).and_return(['always']) + end + + it 'returns true' do + expect(subject.retry_failure?).to eq true + end + end + + context 'and retry when includes the failure_reason' do + before do + expect(subject).to receive(:failure_reason).at_least(:once).and_return('some_reason') + expect(subject).to receive(:retry_when).at_least(:once).and_return(['some_reason']) + end + + it 'returns true' do + expect(subject.retry_failure?).to eq true + end + end + + context 'and retry when does not include failure_reason' do + before do + expect(subject).to receive(:failure_reason).at_least(:once).and_return('some_reason') + expect(subject).to receive(:retry_when).at_least(:once).and_return(['some', 'other failure']) + end + + it 'returns false' do + expect(subject.retry_failure?).to eq false + end + end + end end end @@ -2887,7 +2985,7 @@ describe Ci::Build do end 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 expect(described_class).to receive(:retry) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 054b7b1561c..5c87ed5c3c6 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -435,16 +435,34 @@ describe Ci::CreatePipelineService do end context 'when builds with auto-retries are configured' do - before do - config = YAML.dump(rspec: { script: 'rspec', retry: 2 }) - stub_ci_pipeline_yaml_file(config) + context 'as an integer' do + before do + config = YAML.dump(rspec: { script: 'rspec', retry: 2 }) + stub_ci_pipeline_yaml_file(config) + end + + it 'correctly creates builds with auto-retry value configured' do + pipeline = execute_service + + expect(pipeline).to be_persisted + expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 + expect(pipeline.builds.find_by(name: 'rspec').retry_when).to eq ['always'] + end end - it 'correctly creates builds with auto-retry value configured' do - pipeline = execute_service + context 'as hash' do + before do + config = YAML.dump(rspec: { script: 'rspec', retry: { max: 2, when: 'runner_system_failure' } }) + stub_ci_pipeline_yaml_file(config) + end - expect(pipeline).to be_persisted - expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 + it 'correctly creates builds with auto-retry value configured' do + pipeline = execute_service + + expect(pipeline).to be_persisted + expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 + expect(pipeline.builds.find_by(name: 'rspec').retry_when).to eq ['runner_system_failure'] + end end end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 8c7258c42ad..538992b621e 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -671,9 +671,9 @@ describe Ci::ProcessPipelineService, '#execute' do context 'when builds with auto-retries are configured' do before do - create_build('build:1', stage_idx: 0, user: user, options: { retry: 2 }) + create_build('build:1', stage_idx: 0, user: user, options: { retry: { max: 2 } }) create_build('test:1', stage_idx: 1, user: user, when: :on_failure) - create_build('test:2', stage_idx: 1, user: user, options: { retry: 1 }) + create_build('test:2', stage_idx: 1, user: user, options: { retry: { max: 1 } }) end it 'automatically retries builds in a valid order' do |