ExHammer / hammer

An Elixir rate-limiter with pluggable backends
https://hexdocs.pm/hammer/
MIT License
738 stars 42 forks source link

Resetting Hammer.Backend.ETS in tests #35

Open lucaspolonio opened 5 years ago

lucaspolonio commented 5 years ago

Hi there,

I've integrated Hammer into my application in different places. And when I run my application tests (using Hammer.Backend.ETS), tests that run first affect the rate limits for the subsequent tests.

One way to avoid this issue would be to reset the rate limits (ETS tables) before each test, but I couldn't find anything built into Hammer to achieve that.

Another alternative would be to create a mock of Hammer for my tests with resetting capabilities, but this would be too complex as I'd basically be creating an in-memory version of Hammer.

To get my tests to work, for now I've set expiry_ms to 0 (only for tests), and I've created a little test helper that I call in a setup block so that it resets all rate limits before each test:

  def reset_rate_limits() do
    [{_, pool_pid, _, _}] = Supervisor.which_children(Hammer.Supervisor)
    {:state, _, children_pids, _, _, _, _, _, _} = :sys.get_state(pool_pid)
    Enum.each(children_pids, &Process.send(&1, :prune, []))
  end

This solution feels too hacky, though. Is there another built-in way to solve my problem that I haven't realized yet?

Thanks in advance and thanks for the great library!

JuneKelly commented 5 years ago

Hi there!

Yes, this is a bit of a problem. I'll see if I can add a special reset() function that will do this work for you.

JuneKelly commented 5 years ago

In the meantime, this (I think) will clear out all data from the backing ETS table:

:ets.delete_all_objects(:hammer_ets_buckets)

(Or supply the name of the table, if you're setting it in config and not using the default)

lucaspolonio commented 5 years ago

Thanks @ShaneKilkelly! I believe that would be a great addition to the library. For now clearing out the ETS table works though so no rush 😄

sascha-wolf commented 4 years ago

Any progress on this? The proposed workaround is functional but obviously not ideal.

JuneKelly commented 4 years ago

Hi! I haven't had much time to work on this project recently, but I may have some time coming up in the next month or so. Or, I'd be happy to merge a PR that added this feature.

egeersoz commented 9 months ago

In the meantime, this (I think) will clear out all data from the backing ETS table:

:ets.delete_all_objects(:hammer_ets_buckets)

(Or supply the name of the table, if you're setting it in config and not using the default)

I can confirm that adding this to the setup block in tests works fine.

epinault commented 9 months ago

yea not sure what the actual plan is for this one but seems like introducing a reset API for all backend would do. PR welcome or else I ll see if I can do something in the coming month