1 files changed, 80 insertions, 40 deletions

diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 036d2effc0f..da8c10b67a6 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -139,6 +139,16 @@ RSpec.describe WebHook do
     it { is_expected.to contain_exactly(:token, :url, :url_variables) }
   end
 
+  describe '.web_hooks_disable_failed?' do
+    it 'returns true when feature is enabled for parent' do
+      second_hook = build(:project_hook, project: create(:project))
+      stub_feature_flags(web_hooks_disable_failed: [false, second_hook.project])
+
+      expect(described_class.web_hooks_disable_failed?(hook)).to eq(false)
+      expect(described_class.web_hooks_disable_failed?(second_hook)).to eq(true)
+    end
+  end
+
   describe 'execute' do
     let(:data) { { key: 'value' } }
     let(:hook_name) { 'project hook' }
@@ -170,7 +180,7 @@ RSpec.describe WebHook do
     end
 
     it 'does not async execute non-executable hooks' do
-      hook.update!(disabled_until: 1.day.from_now)
+      allow(hook).to receive(:executable?).and_return(false)
 
       expect(WebHookService).not_to receive(:new)
 
@@ -238,17 +248,18 @@ RSpec.describe WebHook do
       [
         [0, :not_set, true],
         [0, :past,    true],
-        [0, :future,  false],
-        [0, :now,     false],
+        [0, :future,  true],
+        [0, :now,     true],
         [1, :not_set, true],
         [1, :past,    true],
-        [1, :future,  false],
+        [1, :future,  true],
         [3, :not_set, true],
         [3, :past,    true],
-        [3, :future,  false],
+        [3, :future,  true],
         [4, :not_set, false],
-        [4, :past,    false],
-        [4, :future,  false]
+        [4, :past,    true], # expired suspension
+        [4, :now,     false], # active suspension
+        [4, :future,  false] # active suspension
       ]
     end
 
@@ -315,7 +326,7 @@ RSpec.describe WebHook do
       end
 
       it 'is twice the initial value' do
-        expect(hook.next_backoff).to eq(20.minutes)
+        expect(hook.next_backoff).to eq(2 * described_class::INITIAL_BACKOFF)
       end
     end
 
@@ -325,7 +336,7 @@ RSpec.describe WebHook do
       end
 
       it 'grows exponentially' do
-        expect(hook.next_backoff).to eq(80.minutes)
+        expect(hook.next_backoff).to eq(2 * 2 * 2 * described_class::INITIAL_BACKOFF)
       end
     end
 
@@ -357,6 +368,7 @@ RSpec.describe WebHook do
     end
 
     it 'makes a hook executable if it is currently backed off' do
+      hook.recent_failures = 1000
       hook.disabled_until = 1.hour.from_now
 
       expect { hook.enable! }.to change(hook, :executable?).from(false).to(true)
@@ -378,55 +390,71 @@ RSpec.describe WebHook do
   end
 
   describe 'backoff!' do
-    it 'sets disabled_until to the next backoff' do
-      expect { hook.backoff! }.to change(hook, :disabled_until).to(hook.next_backoff.from_now)
-    end
+    context 'when we have not backed off before' do
+      it 'does not disable the hook' do
+        expect { hook.backoff! }.not_to change(hook, :executable?).from(true)
+      end
 
-    it 'increments the backoff count' do
-      expect { hook.backoff! }.to change(hook, :backoff_count).by(1)
+      it 'increments the recent_failures count' do
+        expect { hook.backoff! }.to change(hook, :recent_failures).by(1)
+      end
     end
 
-    context 'when the hook is permanently disabled' do
+    context 'when we have exhausted the grace period' do
       before do
-        allow(hook).to receive(:permanently_disabled?).and_return(true)
-      end
-
-      it 'does not set disabled_until' do
-        expect { hook.backoff! }.not_to change(hook, :disabled_until)
+        hook.update!(recent_failures: described_class::FAILURE_THRESHOLD)
       end
 
-      it 'does not increment the backoff count' do
-        expect { hook.backoff! }.not_to change(hook, :backoff_count)
+      it 'sets disabled_until to the next backoff' do
+        expect { hook.backoff! }.to change(hook, :disabled_until).to(hook.next_backoff.from_now)
       end
-    end
 
-    context 'when we have backed off MAX_FAILURES times' do
-      before do
-        stub_const("#{described_class}::MAX_FAILURES", 5)
-        5.times { hook.backoff! }
+      it 'increments the backoff count' do
+        expect { hook.backoff! }.to change(hook, :backoff_count).by(1)
       end
 
-      it 'does not let the backoff count exceed the maximum failure count' do
-        expect { hook.backoff! }.not_to change(hook, :backoff_count)
-      end
+      context 'when the hook is permanently disabled' do
+        before do
+          allow(hook).to receive(:permanently_disabled?).and_return(true)
+        end
 
-      it 'does not change disabled_until', :skip_freeze_time do
-        travel_to(hook.disabled_until - 1.minute) do
+        it 'does not set disabled_until' do
           expect { hook.backoff! }.not_to change(hook, :disabled_until)
         end
+
+        it 'does not increment the backoff count' do
+          expect { hook.backoff! }.not_to change(hook, :backoff_count)
+        end
       end
 
-      it 'changes disabled_until when it has elapsed', :skip_freeze_time do
-        travel_to(hook.disabled_until + 1.minute) do
-          expect { hook.backoff! }.to change { hook.disabled_until }
-          expect(hook.backoff_count).to eq(described_class::MAX_FAILURES)
+      context 'when we have backed off MAX_FAILURES times' do
+        before do
+          stub_const("#{described_class}::MAX_FAILURES", 5)
+          (described_class::FAILURE_THRESHOLD + 5).times { hook.backoff! }
+        end
+
+        it 'does not let the backoff count exceed the maximum failure count' do
+          expect { hook.backoff! }.not_to change(hook, :backoff_count)
+        end
+
+        it 'does not change disabled_until', :skip_freeze_time do
+          travel_to(hook.disabled_until - 1.minute) do
+            expect { hook.backoff! }.not_to change(hook, :disabled_until)
+          end
+        end
+
+        it 'changes disabled_until when it has elapsed', :skip_freeze_time do
+          travel_to(hook.disabled_until + 1.minute) do
+            expect { hook.backoff! }.to change { hook.disabled_until }
+            expect(hook.backoff_count).to eq(described_class::MAX_FAILURES)
+          end
         end
       end
-    end
 
-    include_examples 'is tolerant of invalid records' do
-      def run_expectation
-        expect { hook.backoff! }.to change(hook, :backoff_count).by(1)
+      include_examples 'is tolerant of invalid records' do
+        def run_expectation
+          expect { hook.backoff! }.to change(hook, :backoff_count).by(1)
+        end
       end
     end
   end
@@ -468,8 +496,19 @@ RSpec.describe WebHook do
       expect(hook).not_to be_temporarily_disabled
     end
 
+    it 'allows FAILURE_THRESHOLD initial failures before we back-off' do
+      described_class::FAILURE_THRESHOLD.times do
+        hook.backoff!
+        expect(hook).not_to be_temporarily_disabled
+      end
+
+      hook.backoff!
+      expect(hook).to be_temporarily_disabled
+    end
+
     context 'when hook has been told to back off' do
       before do
+        hook.update!(recent_failures: described_class::FAILURE_THRESHOLD)
         hook.backoff!
       end
 
@@ -550,6 +589,7 @@ RSpec.describe WebHook do
 
     context 'when hook has been backed off' do
       before do
+        hook.update!(recent_failures: described_class::FAILURE_THRESHOLD + 1)
         hook.disabled_until = 1.hour.from_now
       end