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 @@
 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 @@
 
 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 @@
 
 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 @@
 
 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 @@
 
 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 @@
  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 @@
 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 @@
 
 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 "../../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 '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 "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 '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 '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 '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 @@
  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 @@
  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 @@ def self.it_works_for_cache_backed_features(options)
  request.ip
  end
 
- get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
- assert_equal 200, last_response.status
+ within_same_period do
+ get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+ assert_equal 200, last_response.status
 
- get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
- assert_equal 429, last_response.status
+ get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+ assert_equal 429, last_response.status
+ end
  end
 
  it "works for fail2ban" do
@@ -23,17 +27,19 @@ def self.it_works_for_cache_backed_features(options)
  end
  end
 
- get "/"
- assert_equal 200, last_response.status
+ within_same_period do
+ get "/"
+ assert_equal 200, last_response.status
 
- get "/private-place"
- assert_equal 403, last_response.status
+  get "/private-place"
+  assert_equal 403, last_response.status
 
- get "/private-place"
- assert_equal 403, last_response.status
+  get "/private-place"
+  assert_equal 403, last_response.status
 
- get "/"
- assert_equal 403, last_response.status
+ get "/"
+ assert_equal 403, last_response.status
+ end
  end
 
  it "works for allow2ban" do
@@ -43,20 +49,22 @@ def self.it_works_for_cache_backed_features(options)
  end
  end
 
- get "/"
- assert_equal 200, last_response.status
+ within_same_period do
+ get "/"
+ assert_equal 200, last_response.status
 
- get "/scarce-resource"
- assert_equal 200, last_response.status
+  get "/scarce-resource"
+  assert_equal 200, last_response.status
 
- get "/scarce-resource"
- assert_equal 200, last_response.status
+  get "/scarce-resource"
+  assert_equal 200, last_response.status
 
- get "/scarce-resource"
- assert_equal 403, last_response.status
+  get "/scarce-resource"
+  assert_equal 403, last_response.status
 
- get "/"
- assert_equal 403, last_response.status
+ get "/"
+ assert_equal 403, last_response.status
+ end
  end
 
  it "doesn't leak keys" do
@@ -66,9 +74,7 @@ def self.it_works_for_cache_backed_features(options)
 
  key = nil
 
- # Freeze time during these statement to be sure that the key used by rack attack is the same
- # we pre-calculate in local variable `key`
- Timecop.freeze do
+ within_same_period 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