ci: freeze time in more specs (#643)

* ci: freeze time in more specs

* Introduce within_same_period helper method

---------

Co-authored-by: Gonzalo <456459+grzuy@users.noreply.github.com>

spec/acceptance/stores/active_support_dalli_store_spec.rb

@@ -9,7 +9,6 @@ should_run =
 if should_run
   require_relative "../../support/cache_store_helper"
   require "active_support/cache/dalli_store"
-  require "timecop"
 
   describe "ActiveSupport::Cache::DalliStore as a cache backend" do
     before do

spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb

@@ -4,7 +4,6 @@ require_relative "../../spec_helper"
 
 if defined?(::ConnectionPool) && defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::MemCacheStore (pooled) as a cache backend" do
     before do

spec/acceptance/stores/active_support_mem_cache_store_spec.rb

@@ -4,7 +4,6 @@ require_relative "../../spec_helper"
 
 if defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do
     before do

spec/acceptance/stores/active_support_memory_store_spec.rb

@@ -3,8 +3,6 @@
 require_relative "../../spec_helper"
 require_relative "../../support/cache_store_helper"
 
-require "timecop"
-
 describe "ActiveSupport::Cache::MemoryStore as a cache backend" do
   before do
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new

spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb

@@ -10,7 +10,6 @@ should_run =
 
 if should_run
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do
     before do

spec/acceptance/stores/active_support_redis_cache_store_spec.rb

@@ -9,7 +9,6 @@ should_run =
 
 if should_run
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::RedisCacheStore as a cache backend" do
     before do

spec/acceptance/stores/connection_pool_dalli_client_spec.rb

@@ -6,7 +6,6 @@ if defined?(::Dalli) && defined?(::ConnectionPool)
   require_relative "../../support/cache_store_helper"
   require "connection_pool"
   require "dalli"
-  require "timecop"
 
   describe "ConnectionPool with Dalli::Client as a cache backend" do
     before do

spec/acceptance/stores/dalli_client_spec.rb

@@ -5,7 +5,6 @@ require_relative "../../spec_helper"
 if defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
   require "dalli"
-  require "timecop"
 
   describe "Dalli::Client as a cache backend" do
     before do

spec/acceptance/stores/redis_spec.rb

@@ -4,7 +4,6 @@ require_relative "../../spec_helper"
 
 if defined?(::Redis)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "Plain redis as a cache backend" do
     before do

spec/acceptance/stores/redis_store_spec.rb

@@ -4,8 +4,6 @@ require_relative "../../spec_helper"
 require_relative "../../support/cache_store_helper"
 
 if defined?(::Redis::Store)
-  require "timecop"
-
   describe "Redis::Store as a cache backend" do
     before do
       Rack::Attack.cache.store = ::Redis::Store.new

spec/rack_attack_throttle_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require_relative 'spec_helper'
-require 'timecop'
+require_relative 'support/freeze_time_helper'
 
 describe 'Rack::Attack.throttle' do
   before do
@@ -16,7 +16,7 @@ describe 'Rack::Attack.throttle' do
 
   describe 'a single request' do
     it 'should set the counter for one request' do
-      Timecop.freeze do
+      within_same_period do
         get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
 
         key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
@@ -41,7 +41,7 @@ describe 'Rack::Attack.throttle' do
 
   describe "with 2 requests" do
     before do
-      Timecop.freeze do
+      within_same_period do
         2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
       end
     end
@@ -78,7 +78,7 @@ describe 'Rack::Attack.throttle with limit as proc' do
 
   describe 'a single request' do
     it 'should set the counter for one request' do
-      Timecop.freeze do
+      within_same_period do
         get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
 
         key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
@@ -112,7 +112,7 @@ describe 'Rack::Attack.throttle with period as proc' do
 
   describe 'a single request' do
     it 'should set the counter for one request' do
-      Timecop.freeze do
+      within_same_period do
         get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
 
         key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
@@ -147,7 +147,7 @@ describe 'Rack::Attack.throttle with block returning nil' do
 
   describe 'a single request' do
     it 'should not set the counter' do
-      Timecop.freeze do
+      within_same_period do
         get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
 
         key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
@@ -179,7 +179,7 @@ describe 'Rack::Attack.throttle with throttle_discriminator_normalizer' do
   end
 
   it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
-    Timecop.freeze do
+    within_same_period 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
@@ -191,7 +191,7 @@ describe 'Rack::Attack.throttle with throttle_discriminator_normalizer' do
       prev = Rack::Attack.throttle_discriminator_normalizer
       Rack::Attack.throttle_discriminator_normalizer = nil
 
-      Timecop.freeze do
+      within_same_period do
         post_logins
         @emails.each do |email|
           key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"

spec/support/cache_store_helper.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'freeze_time_helper'
+
 class Minitest::Spec
   def self.it_works_for_cache_backed_features(options)
     fetch_from_store = options.fetch(:fetch_from_store)
@@ -9,11 +11,13 @@ class Minitest::Spec
         request.ip
       end
 
-      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+      within_same_period do
-      assert_equal 200, last_response.status
+        get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+        assert_equal 200, last_response.status
 
-      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+        get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
-      assert_equal 429, last_response.status
+        assert_equal 429, last_response.status
+      end
     end
 
     it "works for fail2ban" do
@@ -23,17 +27,19 @@ class Minitest::Spec
         end
       end
 
-      get "/"
+      within_same_period do
-      assert_equal 200, last_response.status
+        get "/"
+        assert_equal 200, last_response.status
 
-      get "/private-place"
+        get "/private-place"
-      assert_equal 403, last_response.status
+        assert_equal 403, last_response.status
 
-      get "/private-place"
+        get "/private-place"
-      assert_equal 403, last_response.status
+        assert_equal 403, last_response.status
 
-      get "/"
+        get "/"
-      assert_equal 403, last_response.status
+        assert_equal 403, last_response.status
+      end
     end
 
     it "works for allow2ban" do
@@ -43,20 +49,22 @@ class Minitest::Spec
         end
       end
 
-      get "/"
+      within_same_period do
-      assert_equal 200, last_response.status
+        get "/"
+        assert_equal 200, last_response.status
 
-      get "/scarce-resource"
+        get "/scarce-resource"
-      assert_equal 200, last_response.status
+        assert_equal 200, last_response.status
 
-      get "/scarce-resource"
+        get "/scarce-resource"
-      assert_equal 200, last_response.status
+        assert_equal 200, last_response.status
 
-      get "/scarce-resource"
+        get "/scarce-resource"
-      assert_equal 403, last_response.status
+        assert_equal 403, last_response.status
 
-      get "/"
+        get "/"
-      assert_equal 403, last_response.status
+        assert_equal 403, last_response.status
+      end
     end
 
     it "doesn't leak keys" do
@@ -66,9 +74,7 @@ class Minitest::Spec
 
       key = nil
 
-      # Freeze time during these statement to be sure that the key used by rack attack is the same
+      within_same_period do
-      # we pre-calculate in local variable `key`
-      Timecop.freeze do
         key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4"
 
         get "/", {}, "REMOTE_ADDR" => "1.2.3.4"

spec/support/freeze_time_helper.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require "timecop"
+
+class Minitest::Spec
+  def within_same_period(&block)
+    Timecop.freeze(&block)
+  end
+end