Skip to content

test: RateLimiterTest 'tracks active bots' flakes — the suite shares one app-supervised GenServer whose state a crash silently resets #857

Description

@hyperpolymath

Symptom

Integration Tests failed on PR #856 with a single failure out of 1593:

1) test stats/0 tracks active bots after dispatch (Hypatia.Safety.RateLimiterTest)
   test/safety_test.exs:122
   Assertion with >= failed
   code:  assert stats.active_bots >= 2
   left:  0
   right: 2
   stacktrace:
     test/safety_test.exs:130: (test)

1593 tests, 1 failure, 242 excluded

It is a flake, and that is proven rather than assumed

PR #856's head d84e18a and the merged main commit ecc56fa2 have the
byte-identical tree 8cdf99ae049fc5d5d12ea8e07def4d0da24def18 (the squash
changed only the commit object). Both runs logged
Running ExUnit with seed: 0, max_cases: 8seed 0 disables shuffling, so
execution order was identical too.

Same tree, same seed, opposite verdicts: red on the PR run, green on main.
The nondeterminism is therefore timing/state-based, not ordering-based, and it
is independent of the change under test (a comment in an unrelated module).

⚠ This rules out the obvious first hypothesis. Do not "fix" it by reordering
tests or pinning a seed — the seed is already pinned and it still flakes.

Mechanism

Hypatia.Safety.RateLimiter is started by the application supervisor
(lib/application.ex:60). Three consequences:

  1. setup's recovery branch is dead code in the normal case.
    test/safety_test.exs:10-14 and test/concurrency_test.exs:165 both do:

    case GenServer.whereis(RateLimiter) do
      nil -> start_supervised!(RateLimiter)
      _pid -> :ok
    end

    whereis almost always returns the application's pid, so start_supervised!
    never runs and the test controls neither the process's lifetime nor its state.

  2. State is global and never reset across all 1593 tests. bot_windows is a
    plain map; handle_cast({:record, _}) only ever Map.puts into it and
    nothing prunes whole entries (handle_call({:check, _}) filters timestamps
    within a bot's window, never removes the key). active_bots is
    map_size(state.bot_windows) (lib/safety/rate_limiter.ex:124), so it grows
    monotonically for the life of the VM.

  3. 0 is only reachable via a restart. record_dispatch/1 is a cast and
    stats/0 is a call; issued from the same test process to the same
    GenServer they are strictly ordered in the mailbox, so two casts followed by a
    call can never observe an empty map — unless the GenServer died and was
    restarted by its supervisor with a fresh %__MODULE__{} between them
    .

So the failure is evidence that something elsewhere in the suite crashes the
shared RateLimiter at a nondeterministic moment, silently resetting global
state underneath an unrelated test. The assertion is the detector, not the bug.

Why it matters beyond one red square

Any test asserting on RateLimiter state is asserting on a process that 1592
other tests share and that a crash can reset without warning. Today that costs
an occasional red PR; it also means a genuine rate-limiter regression could be
masked by a restart that clears the evidence.

Acceptance criteria

  1. Identify what crashes the shared RateLimiter (prime suspect:
    test/concurrency_test.exs, which stresses it and carries the same
    whereis/start_supervised! pattern). Name it explicitly; do not guess.
  2. Give RateLimiterTest a process whose lifetime and state it owns — e.g.
    start a named-per-test instance rather than reaching for the global one,
    so setup genuinely establishes the precondition it appears to establish.
    Remove the whereis/start_supervised! branch, which today asserts nothing.
  3. Add a reset path (or per-test instance) so bot_windows cannot accumulate
    across the suite.
  4. Kill a mutant: make the shared GenServer restart deliberately mid-test
    (e.g. Process.exit(GenServer.whereis(RateLimiter), :kill)) and show the
    old test goes red while the fixed test stays green. A fix that merely
    stops reproducing is not evidence — the flake did not reproduce on main
    either.
  5. Do not paper over it with :timer.sleep, a retry, or @tag :flaky.

Evidence

  • Failing run: PR fix(governance): green the Validate Hypatia Baseline gate on main #856, Integration Tests, 21:54:39Z–21:54:52Z, tree 8cdf99ae.
  • Passing run: main @ ecc56fa2, Integration Tests success, same tree.
  • lib/safety/rate_limiter.ex:124active_bots: map_size(state.bot_windows).
  • lib/application.ex:60Hypatia.Safety.RateLimiter in the supervision tree.

Surfaced while landing #855; filed as an issue rather than folded into that PR,
per the standing ruling that a new finding is an issue and not a merge blocker.
It is unreachable from #856's change (a comment in scanner_suppression.ex).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    automationBots, schedulers, dispatch, self-healing, fan-outtestingTests, benchmarks, fuzzing, property checks, coverage

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions