Stabilize phone call router tests

[tianmind-studio]

Summary

• install the shared Twilio unit-test stub before importing the phone call router so test_phone_calls.py does not require the Twilio SDK locally
• set the same test Twilio env vars used by the Twilio service tests before utils.twilio_service is imported, avoiding cross-test import-order pollution
• isolate phone call quota/destination guards in the router tests so they do not initialize Redis-backed phone call config/cache paths
• mock phone_call_usage_db in the router tests and cover the free-tier TwiML usage-counting path without touching the real database

Testing

• python -m pytest tests\unit\test_phone_calls.py -q -> 18 passed, 2 warnings
• python -m pytest tests\unit\test_phone_calls.py tests\unit\test_twilio_service.py tests\unit\test_twilio_account_deletion.py -q -> 27 passed, 2 warnings
• python -m black --line-length 120 --skip-string-normalization tests\unit\test_phone_calls.py
• python -m py_compile tests\unit\test_phone_calls.py
• git diff --check origin/main...HEAD

[greptile-apps]

Greptile Summary

This PR stabilizes test_phone_calls.py so it runs independently of the Twilio SDK and Redis-backed services. It sets the required Twilio environment variables before any module import, calls install_twilio_stub() to register fake Twilio submodules when the real SDK is absent, and adds an autouse fixture that patches the three quota/destination helpers (check_call_access, get_quota_snapshot, check_destination_allowed) to prevent Redis and Firebase initialization during the router tests.

• Twilio env vars are now set with os.environ.setdefault at the top of the file, matching the pattern used by the Twilio service tests and eliminating cross-file import-order pollution.
• isolate_phone_call_quota autouse fixture patches the quota helpers per-test via monkeypatch, ensuring automatic teardown and preventing one test's state from leaking into another.
• _make_quota_snapshot() always sets is_paid=True, which silently skips the phone_call_usage_db.increment_current_month call; phone_call_usage_db is therefore never mocked, leaving the free-tier billing path untested.

Confidence Score: 4/5

Safe to merge — this is a test-only change that does not touch production code.

The change is confined to a test file. The autouse fixture always grants paid-tier access (is_paid=True), which means phone_call_usage_db.increment_current_month is never reached and never mocked. A follow-up test for the free-tier billing path would hit an unmocked database call and fail. No production logic is affected.

Only backend/tests/unit/test_phone_calls.py changed. The is_paid=True default in _make_quota_snapshot() and the absence of a phone_call_usage_db mock are worth a second look if free-tier call-usage tests are added later.

Important Files Changed

Filename

Overview

backend/tests/unit/test_phone_calls.py

Adds Twilio env-var defaults, installs the shared twilio_stub before the router import, and introduces an autouse fixture that patches check_call_access, get_quota_snapshot, and check_destination_allowed to eliminate Redis/external-dependency initialization in tests; is_paid=True in the snapshot silently bypasses phone_call_usage_db.increment_current_month, leaving the free-tier billing path untested and unguarded.

Sequence Diagram

sequenceDiagram
    participant TF as Test File (module load)
    participant Stub as twilio_stub.install_twilio_stub()
    participant SysM as sys.modules
    participant Router as routers.phone_calls
    participant Fixture as isolate_phone_call_quota (autouse)
    participant Test as Test Function

    TF->>SysM: setdefault ENCRYPTION_SECRET + Twilio env vars
    TF->>SysM: setdefault database._client, firebase_admin (MagicMock)
    TF->>Stub: install_twilio_stub()
    Stub->>SysM: check 'twilio' in sys.modules / find_spec
    alt Twilio not available
        Stub->>SysM: "register fake twilio.*, twilio.base.exceptions, etc."
    else Twilio SDK found
        Stub-->>TF: return (no-op)
    end
    TF->>Router: import routers.phone_calls (binds stub or real Twilio names)
    Note over TF,Router: All three quota helpers bound at module level

    Test->>Fixture: pytest autouse setup
    Fixture->>Router: monkeypatch check_call_access → MagicMock
    Fixture->>Router: "monkeypatch get_quota_snapshot → MagicMock (is_paid=True)"
    Fixture->>Router: monkeypatch check_destination_allowed → MagicMock(None)
    Fixture-->>Test: ready

    Test->>Router: HTTP request via TestClient
    Router->>Router: check_call_access(uid) → mock
    Router->>Router: "get_quota_snapshot(uid) → mock snapshot (is_paid=True)"
    Router->>Router: check_destination_allowed(...) → None (mock)
    Note over Router: is_paid=True → skip phone_call_usage_db.increment_current_month
    Router-->>Test: response

Loading

_{Reviews (1): Last reviewed commit: "Stabilize phone call router tests" | Re-trigger Greptile}

[greptile-apps]

phone_call_usage_db unguarded when is_paid is flipped

The autouse fixture always sets snapshot.is_paid = True. In twiml_voice_webhook the only guard keeping phone_call_usage_db.increment_current_month from running is if not snapshot.is_paid:. Because the fixture hard-codes True, the usage DB is never exercised and is never mocked. Any future test that sets is_paid = False (to cover the free-tier billing path) will hit the real phone_call_usage_db call and fail with an uninitialized database connection, since database.phone_call_usage is not in the sys.modules stub table.

[tianmind-studio]

Fixed in aa699f6.

I added a router-level phone_call_usage_db mock in the autouse fixture and made the fixture return the quota snapshot plus usage DB mock. There is now a free-tier TwiML regression test that flips snapshot.is_paid = False and asserts increment_current_month(TEST_UID) is called without touching the real database.

Validation:

• python -m pytest tests\unit\test_phone_calls.py -q -> 18 passed, 2 warnings
• python -m pytest tests\unit\test_phone_calls.py tests\unit\test_twilio_service.py tests\unit\test_twilio_account_deletion.py -q -> 27 passed, 2 warnings
• python -m py_compile tests\unit\test_phone_calls.py
• git diff --check origin/main...HEAD

[kodjima33]

Backend test stabilization (Windows/UTF-8/optional-dep isolation) — approve only per policy.

[tianmind-studio]

Closing this in favor of #7798.

#7798 covers the same test_phone_calls.py Windows/lightweight-environment issue with a more self-contained Twilio stub surface, the plan/quota guard isolation, and the free-tier phone_call_usage_db.increment_current_month(TEST_UID) regression coverage. Keeping the newer PR avoids duplicate review work.

[github-actions]

Hey @tianmind-studio 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜