fix(tests): test_review_fixes.py rate-limit env patch applied after client fixture, making test a false positive #268

Summary

test_review_fixes.py patches RATE_LIMIT_DEFAULT_PER_MINUTE via monkeypatch.setenv() and calls get_settings.cache_clear() inside the test body — but the client fixture is already fully resolved before the test body runs. The rate limiter in the client always sees the original (high) limit, so the 429 assertion never actually triggers. The test is a false positive that permanently passes while verifying nothing.

Root Cause

tests/test_review_fixes.py around line 54:

def test_rate_limit_kicks_in_after_one_request(client, monkeypatch):
    monkeypatch.setenv('RATE_LIMIT_DEFAULT_PER_MINUTE', '1')   # ← too late
    get_settings.cache_clear()                                   # ← too late
    
    r1 = client.get('/v1/health')
    r2 = client.get('/v1/health')
    assert r2.status_code == 429   # ← never reached, r2 is 200

The client fixture creates the FastAPI TestClient using create_app(get_settings()) before monkeypatch.setenv runs. The rate limiter captured limit=60 at fixture construction time.

Failure Scenario

1. test_rate_limit_kicks_in_after_one_request runs.
2. client fixture builds the app with rate_limit=60.
3. Test body patches env var and clears cache — no effect on the already-built app.
4. Both requests return 200 (count 1 and 2 of 60).
5. assert r2.status_code == 429 → AssertionError: 200 != 429.
6. OR: test passes via prior-test contamination from a previous test that set a low limit — making the pass order-dependent and non-deterministic.

Proposed Fix

Move the env patch to a fixture that runs BEFORE the client fixture is resolved:

@pytest.fixture
def low_rate_limit_client(monkeypatch):
    monkeypatch.setenv('RATE_LIMIT_DEFAULT_PER_MINUTE', '1')
    get_settings.cache_clear()
    # Now create the client AFTER the env is patched:
    app = create_app(get_settings())
    with TestClient(app) as c:
        yield c

def test_rate_limit_kicks_in_after_one_request(low_rate_limit_client):
    r1 = low_rate_limit_client.get('/v1/health')
    r2 = low_rate_limit_client.get('/v1/health')
    assert r1.status_code == 200
    assert r2.status_code == 429