Skip to content

fix(tests): test_auth.py rate-limit test ineffective — Settings LRU cache not cleared after monkeypatch.setenv #271

Closed

Description

@piyushmakhija28

Summary

tests/test_auth.py patches RATE_LIMIT_DEFAULT_PER_MINUTE with monkeypatch.setenv() but never calls get_settings.cache_clear(). The get_settings() LRU cache returns the old Settings object with the original (high) rate limit. All 5 test requests return 200, making the 429 in statuses assertion unreliable — it passes only via accidental prior-test pollution.

Root Cause

tests/test_auth.py around line 154:

def test_authenticated_requests_are_rate_limited(client, monkeypatch):
    monkeypatch.setenv('RATE_LIMIT_DEFAULT_PER_MINUTE', '3')
    # get_settings.cache_clear() is MISSING here
    
    for _ in range(5):
        r = client.get('/v1/health', headers=auth_headers)
        statuses.append(r.status_code)
    
    assert 429 in statuses[3:]   # ← unreliable, usually fails

Failure Scenario

  1. monkeypatch.setenv sets the env var, but the Settings LRU cache still returns the cached Settings(rate_limit=60).
  2. The rate limiter uses limit=60. All 5 requests return 200.
  3. 429 in [200, 200, 200, 200, 200] → False. Test fails.
  4. OR: A prior test accidentally left rate_limit=3 in a cached Settings object → test passes non-deterministically.

Proposed Fix

def test_authenticated_requests_are_rate_limited(monkeypatch):
    monkeypatch.setenv('RATE_LIMIT_DEFAULT_PER_MINUTE', '3')
    get_settings.cache_clear()         # ← add this

    # Build the client AFTER clearing the cache:
    app = create_app(get_settings())
    with TestClient(app) as client:
        statuses = []
        for _ in range(5):
            r = client.get('/v1/health', headers=make_auth_headers())
            statuses.append(r.status_code)
        
        assert statuses[:3] == [200, 200, 200], "First 3 requests should succeed"
        assert 429 in statuses[3:], "Requests 4-5 should be rate-limited"

Activity

  1. piyushmakhija28 commented on Jun 21, 2026

    @piyushmakhija28
    ContributorAuthor

    Fixed in PR #280 (Sprint R3/R4: fix(sprint-r3): root-cause fixes for all 39 bugs from code review round 3 + Sprint R4 round 4)

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

    bugSomething isn't workingcode-reviewFound via automated code reviewpriority: mediumFunctional error with workaround

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions