From 26080b4b5c9d10887f8c52fbd76cba5b22838632 Mon Sep 17 00:00:00 2001
From: Ilja <ilja@ilja.space>
Date: Fri, 8 Jul 2022 07:32:47 +0200
Subject: [PATCH] Fix rate_limiter_test.exs test "it restricts based on config
 values"

It used a timer to sleep.
But time also goes on when doing other things, so depending on hardware, the timings could be off.
I slightly changed the tests so we still test what we functionally want.
Instead of waiting until the cache expires I now have a function to expire the test and use that.

That means we're not testing any more if the cache really expires after a certain amount of time,
but that's the responsability of the dependency imo, so shouldn't be a problem.

I also changed `Pleroma.Web.Endpoint, :http, :ip` to `127.0.0.1` because that's the setting people typically have,
and I see no reason to do it differently.
Especially since it's an exernal ip, which may come over as weird or suspicious to people.
---
 test/pleroma/web/plugs/rate_limiter_test.exs | 44 +++++++++++++-------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/test/pleroma/web/plugs/rate_limiter_test.exs b/test/pleroma/web/plugs/rate_limiter_test.exs
index b1ac76120..19cee8aee 100644
--- a/test/pleroma/web/plugs/rate_limiter_test.exs
+++ b/test/pleroma/web/plugs/rate_limiter_test.exs
@@ -48,38 +48,42 @@ test "it is enabled if remote_ip_found flag doesn't exist" do
     refute RateLimiter.disabled?(build_conn())
   end
 
-  @tag :erratic
   test "it restricts based on config values" do
     limiter_name = :test_plug_opts
     scale = 80
     limit = 5
 
-    clear_config([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
+    clear_config([Pleroma.Web.Endpoint, :http, :ip], {127, 0, 0, 1})
     clear_config([:rate_limit, limiter_name], {scale, limit})
 
     plug_opts = RateLimiter.init(name: limiter_name)
     conn = build_conn(:get, "/")
 
-    for i <- 1..5 do
-      conn = RateLimiter.call(conn, plug_opts)
-      assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
-      Process.sleep(10)
+    for _ <- 1..5 do
+      conn_limited = RateLimiter.call(conn, plug_opts)
+
+      refute conn_limited.status == Conn.Status.code(:too_many_requests)
+      refute conn_limited.resp_body
+      refute conn_limited.halted
     end
 
-    conn = RateLimiter.call(conn, plug_opts)
-    assert %{"error" => "Throttled"} = ConnTest.json_response(conn, :too_many_requests)
-    assert conn.halted
+    conn_limited = RateLimiter.call(conn, plug_opts)
+    assert %{"error" => "Throttled"} = ConnTest.json_response(conn_limited, :too_many_requests)
+    assert conn_limited.halted
 
-    Process.sleep(50)
+    expire_ttl(conn, limiter_name)
 
-    conn = build_conn(:get, "/")
+    for _ <- 1..5 do
+      conn_limited = RateLimiter.call(conn, plug_opts)
 
-    conn = RateLimiter.call(conn, plug_opts)
-    assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+      refute conn_limited.status == Conn.Status.code(:too_many_requests)
+      refute conn_limited.resp_body
+      refute conn_limited.halted
+    end
 
-    refute conn.status == Conn.Status.code(:too_many_requests)
-    refute conn.resp_body
-    refute conn.halted
+    conn_limited = RateLimiter.call(conn, plug_opts)
+    assert %{"error" => "Throttled"} = ConnTest.json_response(conn_limited, :too_many_requests)
+    assert conn_limited.halted
   end
 
   describe "options" do
@@ -263,4 +267,12 @@ test "doesn't crash due to a race condition when multiple requests are made at t
 
     refute {:err, :not_found} == RateLimiter.inspect_bucket(conn, limiter_name, opts)
   end
+
+  def expire_ttl(%{remote_ip: remote_ip} = _conn, bucket_name_root) do
+    bucket_name = "anon:#{bucket_name_root}" |> String.to_atom()
+    key_name = "ip::#{remote_ip |> Tuple.to_list() |> Enum.join(".")}"
+
+    {:ok, bucket_value} = Cachex.get(bucket_name, key_name)
+    Cachex.put(bucket_name, key_name, bucket_value, ttl: -1)
+  end
 end