ci: freeze time to fix flakey specs (#640)

rack · Nov 21, 2023 · d3b3da4 · d3b3da4
1 parent d9fedfa
commit d3b3da4
Showing 1 changed file with 49 additions and 28 deletions.
diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb
@@ -1,10 +1,11 @@
 # frozen_string_literal: true
 
 require_relative 'spec_helper'
+require 'timecop'
 
 describe 'Rack::Attack.throttle' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip }
   end
@@ -14,14 +15,18 @@
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      Timecop.freeze do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
       data = {
         count: 1,
         limit: 1,
@@ -36,7 +41,9 @@
 
   describe "with 2 requests" do
     before do
-      2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
+      Timecop.freeze do
+        2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
+      end
     end
 
     it 'should block the last request' do
@@ -62,22 +69,25 @@
 
 describe 'Rack::Attack.throttle with limit as proc' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: @period) { |req| req.ip }
   end
 
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      Timecop.freeze do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
       data = {
         count: 1,
         limit: 1,
@@ -93,22 +103,26 @@
 
 describe 'Rack::Attack.throttle with period as proc' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: lambda { |_req| @period }) { |req| req.ip }
   end
 
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      Timecop.freeze do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
       data = {
         count: 1,
         limit: 1,
@@ -122,7 +136,7 @@
   end
 end
 
-describe 'Rack::Attack.throttle with block retuning nil' do
+describe 'Rack::Attack.throttle with block returning nil' do
   before do
     @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
@@ -132,14 +146,17 @@
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should not set the counter' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      assert_nil Rack::Attack.cache.store.read(key)
+      Timecop.freeze do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        assert_nil Rack::Attack.cache.store.read(key)
+      end
     end
 
     it 'should not populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
       assert_nil last_request.env['rack.attack.throttle_data']
     end
   end
@@ -162,20 +179,24 @@
   end
 
   it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
-    post_logins
-    key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com"
-    _(Rack::Attack.cache.store.read(key)).must_equal 3
+    Timecop.freeze do
+      post_logins
+      key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com"
+      _(Rack::Attack.cache.store.read(key)).must_equal 3
+    end
   end
 
   it 'should differentiate requests when throttle_discriminator_normalizer is disabled' do
     begin
       prev = Rack::Attack.throttle_discriminator_normalizer
       Rack::Attack.throttle_discriminator_normalizer = nil
 
-      post_logins
-      @emails.each do |email|
-        key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
-        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      Timecop.freeze do
+        post_logins
+        @emails.each do |email|
+          key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
+          _(Rack::Attack.cache.store.read(key)).must_equal 1
+        end
       end
     ensure
       Rack::Attack.throttle_discriminator_normalizer = prev