refactor retry logic to define any reason and more than one reason to retry

2 files changed, 27 insertions, 110 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 592b784807c..08d6e669cf9 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -324,20 +324,15 @@ module Ci
 
     def retry_when
       retries = self.options[:retry]
-      retries.is_a?(Hash) && retries.fetch(:when, 'always').to_s || 'always'
+      Array.wrap(
+        retries.is_a?(Hash) && retries.fetch(:when, 'always') || 'always'
+      )
     end
 
     def retry_failure?
       return false if retries_max.zero? || retries_count >= retries_max
 
-      case failure_reason.to_s
-      when 'runner_system_failure'
-        %['always', 'system failure'].include?(retry_when)
-      when 'script_failure'
-        %['always', 'script failure'].include?(retry_when)
-      else
-        retry_when == 'always'
-      end
+      retry_when.include?('always') || retry_when.include?(failure_reason.to_s)
     end
 
     def latest?
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 3085cf7b271..4f113d2e653 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1514,11 +1514,19 @@ describe Ci::Build do
     end
 
     describe '#retry_when' do
-      context 'when value is defined' do
-        subject { create(:ci_build, options: { retry: { when: :something } }) }
+      context 'when value is defined without an array' do
+        subject { create(:ci_build, options: { retry: { when: 'something' } }) }
+
+        it 'returns the configured value inside an array' do
+          expect(subject.retry_when).to eq ['something']
+        end
+      end
+
+      context 'when value is defined without as an array' do
+        subject { create(:ci_build, options: { retry: { when: %w[something more] } }) }
 
         it 'returns the configured value' do
-          expect(subject.retry_when).to eq :something
+          expect(subject.retry_when).to eq %w[something more]
         end
       end
 
@@ -1526,7 +1534,7 @@ describe Ci::Build do
         subject { create(:ci_build) }
 
         it 'returns `always`' do
-          expect(subject.retry_when).to eq :always
+          expect(subject.retry_when).to eq ['always']
         end
       end
 
@@ -1534,7 +1542,7 @@ describe Ci::Build 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
@@ -1579,111 +1587,25 @@ describe Ci::Build do
           end
         end
 
-        context 'and failure was a system runner failure' do
-          before do
-            expect(subject).to receive(:failure_reason).at_least(:once).and_return('runner_system_failure')
-          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 is system failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
-            end
-
-            it 'returns true' do
-              expect(subject.retry_failure?).to eq true
-            end
-          end
-
-          context 'and retry when is script failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
-            end
-
-            it 'returns false' do
-              expect(subject.retry_failure?).to eq false
-            end
-          end
-        end
-
-        context 'and failure was a script failure' do
+        context 'and retry when includes the failure_reason' do
           before do
-            expect(subject).to receive(:failure_reason).at_least(:once).and_return('script_failure')
-          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 is system failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
-            end
-
-            it 'returns false' do
-              expect(subject.retry_failure?).to eq false
-            end
+            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
 
-          context 'and retry when is script failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
-            end
-
-            it 'returns true' do
-              expect(subject.retry_failure?).to eq true
-            end
+          it 'returns true' do
+            expect(subject.retry_failure?).to eq true
           end
         end
 
-        context 'and failure was some other failure' do
+        context 'and retry when does not include failure_reason' do
           before do
-            expect(subject).to receive(:failure_reason).at_least(:once).and_return('some other reason')
-          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
+            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
 
-          context 'and retry when is system failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('system failure')
-            end
-
-            it 'returns false' do
-              expect(subject.retry_failure?).to eq false
-            end
-          end
-
-          context 'and retry when is script failure' do
-            before do
-              expect(subject).to receive(:retry_when).at_least(:once).and_return('script failure')
-            end
-
-            it 'returns false' do
-              expect(subject.retry_failure?).to eq false
-            end
+          it 'returns false' do
+            expect(subject.retry_failure?).to eq false
           end
         end
       end