Stabilize rate limiter tests by using dependency injection

lib/req/rate_limiter.ex

@@ -12,15 +12,22 @@ defmodule Req.RateLimiter do
       |> Req.RateLimiter.attach(name: :music_brainz, cooldown: 500)
 
   When `cooldown` is 0, the step is a no-op.
+
+  ## Clock
+
+  Time operations are delegated to a clock module implementing
+  `Req.RateLimiter.Clock`. Defaults to `Req.RateLimiter.SystemClock`.
+  Pass `:clock` in `attach/2` opts to override (useful in tests).
   """
 
   @table __MODULE__
+  @default_clock Req.RateLimiter.SystemClock
 
   @doc """
   Creates the ETS table used to track request timestamps.
   Call once at application startup.
   """
-  @type attach_opts :: [name: atom(), cooldown: non_neg_integer()]
+  @type attach_opts :: [name: atom(), cooldown: non_neg_integer(), clock: module()]
 
   @spec new() :: :ets.table()
   def new do
@@ -34,16 +41,19 @@ defmodule Req.RateLimiter do
 
     * `:name` - atom identifying the API (e.g. `:music_brainz`)
     * `:cooldown` - minimum milliseconds between requests
+    * `:clock` - module implementing `Req.RateLimiter.Clock` (default: `Req.RateLimiter.SystemClock`)
 
   """
   @spec attach(Req.Request.t(), attach_opts()) :: Req.Request.t()
   def attach(request, opts) do
     name = Keyword.fetch!(opts, :name)
     cooldown = Keyword.fetch!(opts, :cooldown)
+    clock = Keyword.get(opts, :clock, @default_clock)
 
     request
     |> Req.Request.put_private(:rate_limiter_name, name)
     |> Req.Request.put_private(:rate_limiter_cooldown, cooldown)
+    |> Req.Request.put_private(:rate_limiter_clock, clock)
     |> Req.Request.prepend_request_steps(rate_limiter: &throttle/1)
   end
 
@@ -52,7 +62,8 @@ defmodule Req.RateLimiter do
 
     if cooldown > 0 do
       name = Req.Request.get_private(request, :rate_limiter_name)
-      now = System.monotonic_time(:millisecond)
+      clock = Req.Request.get_private(request, :rate_limiter_clock)
+      now = clock.now()
 
       case :ets.lookup(@table, name) do
         [{^name, last_at}] ->
@@ -66,14 +77,14 @@ defmodule Req.RateLimiter do
               %{name: name}
             )
 
-            Process.sleep(remaining)
+            clock.sleep(remaining)
           end
 
         [] ->
           :ok
       end
 
-      :ets.insert(@table, {name, System.monotonic_time(:millisecond)})
+      :ets.insert(@table, {name, clock.now()})
     end
 
     request

lib/req/rate_limiter/clock.ex

@@ -0,0 +1,10 @@
+defmodule Req.RateLimiter.Clock do
+  @moduledoc """
+  Behaviour for time operations used by `Req.RateLimiter`.
+
+  Allows injecting a fake clock in tests for deterministic throttle assertions.
+  """
+
+  @callback now() :: integer()
+  @callback sleep(non_neg_integer()) :: :ok
+end

lib/req/rate_limiter/system_clock.ex

@@ -0,0 +1,13 @@
+defmodule Req.RateLimiter.SystemClock do
+  @moduledoc """
+  Real clock implementation using `System.monotonic_time/1` and `Process.sleep/1`.
+  """
+
+  @behaviour Req.RateLimiter.Clock
+
+  @impl true
+  def now, do: System.monotonic_time(:millisecond)
+
+  @impl true
+  def sleep(ms), do: Process.sleep(ms)
+end

test/req/rate_limiter_test.exs

@@ -2,15 +2,28 @@ defmodule Req.RateLimiterTest do
   use ExUnit.Case, async: true
 
   alias Req.RateLimiter
+  alias Req.RateLimiter.FakeClock
+
+  setup do
+    FakeClock.set(1000)
+    :ok
+  end
+
+  defp adapter do
+    fn request ->
+      {request, Req.Response.new(status: 200, body: "ok")}
+    end
+  end
 
   describe "attach/2" do
     test "attaches rate_limiter step and sets private fields" do
       request =
         Req.new(url: "https://example.com")
-        |> RateLimiter.attach(name: :test_attach, cooldown: 100)
+        |> RateLimiter.attach(name: :test_attach, cooldown: 100, clock: FakeClock)
 
       assert Req.Request.get_private(request, :rate_limiter_name) == :test_attach
       assert Req.Request.get_private(request, :rate_limiter_cooldown) == 100
+      assert Req.Request.get_private(request, :rate_limiter_clock) == FakeClock
 
       step_names = Enum.map(request.request_steps, &elem(&1, 0))
       assert :rate_limiter in step_names
@@ -20,7 +33,7 @@ defmodule Req.RateLimiterTest do
       request =
         Req.new(url: "https://example.com")
         |> Req.Request.append_request_steps(log_attempt: fn req -> req end)
-        |> RateLimiter.attach(name: :test_order, cooldown: 100)
+        |> RateLimiter.attach(name: :test_order, cooldown: 100, clock: FakeClock)
 
       step_names = Enum.map(request.request_steps, &elem(&1, 0))
       rate_limiter_index = Enum.find_index(step_names, &(&1 == :rate_limiter))
@@ -34,76 +47,80 @@ defmodule Req.RateLimiterTest do
     test "does not sleep when cooldown is 0" do
       name = :"test_no_sleep_#{System.unique_integer([:positive])}"
 
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
-
       request =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name, cooldown: 0)
+        |> RateLimiter.attach(name: name, cooldown: 0, clock: FakeClock)
 
-      start = System.monotonic_time(:millisecond)
       {:ok, _} = Req.get(request)
       {:ok, _} = Req.get(request)
-      elapsed = System.monotonic_time(:millisecond) - start
 
-      assert elapsed < 200
+      # Clock should not have advanced since cooldown is 0
+      assert FakeClock.now() == 1000
     end
 
     test "enforces cooldown between rapid consecutive requests" do
       name = :"test_cooldown_#{System.unique_integer([:positive])}"
-      cooldown = 100
+      cooldown = 500
-
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
 
       request =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name, cooldown: cooldown)
+        |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock)
 
       {:ok, _} = Req.get(request)
 
-      start = System.monotonic_time(:millisecond)
+      # No time has passed, so second request must sleep the full cooldown
       {:ok, _} = Req.get(request)
-      elapsed = System.monotonic_time(:millisecond) - start
 
-      assert elapsed >= cooldown - 50
+      # Clock advanced exactly by the cooldown amount
+      assert FakeClock.now() == 1000 + cooldown
+    end
+
+    test "sleeps only the remaining cooldown time" do
+      name = :"test_partial_#{System.unique_integer([:positive])}"
+      cooldown = 500
+
+      request =
+        Req.new(url: "https://example.com", adapter: adapter())
+        |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock)
+
+      {:ok, _} = Req.get(request)
+
+      # Simulate 200ms passing
+      FakeClock.advance(200)
+
+      {:ok, _} = Req.get(request)
+
+      # Should have slept the remaining 300ms (500 - 200)
+      assert FakeClock.now() == 1000 + 200 + 300
     end
 
     test "does not sleep when enough time has elapsed" do
       name = :"test_elapsed_#{System.unique_integer([:positive])}"
-      cooldown = 50
+      cooldown = 500
-
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
 
       request =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name, cooldown: cooldown)
+        |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock)
 
       {:ok, _} = Req.get(request)
-      Process.sleep(cooldown + 10)
 
-      start = System.monotonic_time(:millisecond)
+      # Simulate more than cooldown passing
+      FakeClock.advance(cooldown + 100)
+
+      before_second = FakeClock.now()
       {:ok, _} = Req.get(request)
-      elapsed = System.monotonic_time(:millisecond) - start
 
-      assert elapsed < 200
+      # Clock should not have advanced (no sleep needed)
+      assert FakeClock.now() == before_second
     end
 
     test "emits telemetry event when throttling" do
       name = :"test_telemetry_#{System.unique_integer([:positive])}"
-      cooldown = 100
+      cooldown = 500
-
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
 
       request =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name, cooldown: cooldown)
+        |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock)
 
       ref =
         :telemetry_test.attach_event_handlers(self(), [
@@ -113,23 +130,16 @@ defmodule Req.RateLimiterTest do
       {:ok, _} = Req.get(request)
       {:ok, _} = Req.get(request)
 
-      assert_received {[:req, :rate_limiter, :throttle], ^ref, %{sleep_ms: sleep_ms},
+      assert_received {[:req, :rate_limiter, :throttle], ^ref, %{sleep_ms: 500}, %{name: ^name}}
-                       %{name: ^name}}
-
-      assert sleep_ms > 0
     end
 
     test "does not emit telemetry event when no throttling needed" do
       name = :"test_no_telemetry_#{System.unique_integer([:positive])}"
-      cooldown = 50
+      cooldown = 500
-
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
 
       request =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name, cooldown: cooldown)
+        |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock)
 
       ref =
         :telemetry_test.attach_event_handlers(self(), [
@@ -137,7 +147,7 @@ defmodule Req.RateLimiterTest do
         ])
 
       {:ok, _} = Req.get(request)
-      Process.sleep(cooldown + 10)
+      FakeClock.advance(cooldown + 100)
       {:ok, _} = Req.get(request)
 
       refute_received {[:req, :rate_limiter, :throttle], ^ref, _, _}
@@ -146,27 +156,23 @@ defmodule Req.RateLimiterTest do
     test "different API names are tracked independently" do
       name_a = :"test_api_a_#{System.unique_integer([:positive])}"
       name_b = :"test_api_b_#{System.unique_integer([:positive])}"
-      cooldown = 100
+      cooldown = 500
-
-      adapter = fn request ->
-        {request, Req.Response.new(status: 200, body: "ok")}
-      end
 
       request_a =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name_a, cooldown: cooldown)
+        |> RateLimiter.attach(name: name_a, cooldown: cooldown, clock: FakeClock)
 
       request_b =
-        Req.new(url: "https://example.com", adapter: adapter)
+        Req.new(url: "https://example.com", adapter: adapter())
-        |> RateLimiter.attach(name: name_b, cooldown: cooldown)
+        |> RateLimiter.attach(name: name_b, cooldown: cooldown, clock: FakeClock)
 
       {:ok, _} = Req.get(request_a)
 
-      start = System.monotonic_time(:millisecond)
+      before_b = FakeClock.now()
       {:ok, _} = Req.get(request_b)
-      elapsed = System.monotonic_time(:millisecond) - start
 
-      assert elapsed < 200
+      # API B has no prior request, so no sleep
+      assert FakeClock.now() == before_b
     end
   end
 end

test/support/fake_clock.ex

@@ -0,0 +1,34 @@
+defmodule Req.RateLimiter.FakeClock do
+  @moduledoc """
+  Fake clock for deterministic rate limiter tests.
+
+  Stores the current time in the process dictionary so each test process
+  has its own isolated clock state. Works with `async: true`.
+  """
+
+  @behaviour Req.RateLimiter.Clock
+
+  @key :fake_clock_now
+
+  @spec set(integer()) :: :ok
+  def set(now) do
+    Process.put(@key, now)
+    :ok
+  end
+
+  @spec advance(non_neg_integer()) :: :ok
+  def advance(ms) do
+    Process.put(@key, now() + ms)
+    :ok
+  end
+
+  @impl true
+  def now do
+    Process.get(@key, 0)
+  end
+
+  @impl true
+  def sleep(ms) do
+    advance(ms)
+  end
+end