Skip to content

test(telegram): prefer the real PTB library over process-wide mocks - #75615

Closed
ai-ag2026 wants to merge 1 commit into
NousResearch:mainfrom
ai-ag2026:pr/prefer-real-ptb-in-tests
Closed

test(telegram): prefer the real PTB library over process-wide mocks#75615
ai-ag2026 wants to merge 1 commit into
NousResearch:mainfrom
ai-ag2026:pr/prefer-real-ptb-in-tests

Conversation

@ai-ag2026

Copy link
Copy Markdown
Contributor

Summary

tests/gateway/conftest.py (and the file-local copy in test_telegram_approval_buttons.py) install a permanent, process-wide MagicMock for the telegram module family whenever the real library hasn't been imported yet. Collection order decides the outcome: when any gateway file collects first, every later-collected test that needs genuine PTB classes is poisoned. Concretely, tests/test_telegram_polling_progress_ptb.py subclasses the real BaseRequest and fails 6/6 with object MagicMock can't be used in 'await' expression:

pytest tests/gateway/test_telegram_approval_buttons.py tests/test_telegram_polling_progress_ptb.py
# 6 failed, 10 passed  (either order — collection imports both modules first)

Both _ensure_telegram_mock variants now try to import the real library first and install fakes only when PTB is genuinely absent. Since python-telegram-bot is a production dependency, a normal dev/CI venv now always tests against real PTB semantics; the mock remains as a fallback for PTB-less environments.

Flip side (included)

Real TelegramError capitalize()s its message where the mock did not. The bot-token-redaction assertions in test_telegram_rich_messages.py now compare case-insensitively — the property under test is the token redaction (bot123456789:***), not the URL casing.

Validation

  • Two-file reproduction above: 6 failed → 16 passed (both orders).
  • Full telegram test set (56 files, one run): 524 passed, 0 failed.

Notes

Found by bisecting a 6-test order-coupling; the mock's own guard ("skips when the real library is already imported") shows real-PTB runs were always intended to be valid — this just makes them the default instead of an accident of import order.

The gateway conftest (and test_telegram_approval_buttons' file-local copy)
install a permanent MagicMock for the telegram module family whenever the real
library has not been imported yet. Collection order decides: when a gateway
file collects first, every later test needing genuine PTB classes is poisoned
— tests/test_telegram_polling_progress_ptb.py subclasses the real BaseRequest
and fails 6/6 with 'object MagicMock can't be used in await expression'.
Reproducible with just two files:

    pytest tests/gateway/test_telegram_approval_buttons.py \
           tests/test_telegram_polling_progress_ptb.py

Both _ensure_telegram_mock variants now import the real library first and
install fakes only when PTB is genuinely absent. python-telegram-bot is a
production dependency, so in a normal dev/CI venv tests now always run
against real PTB semantics.

Flip side: the real TelegramError capitalize()s its message where the mock
did not — the token-redaction assertions in test_telegram_rich_messages now
compare case-insensitively (the property under test is the redaction, not
the URL casing).

Full telegram test set (56 files): 524 passed, 0 failed; the two-file
reproduction above goes 6-failed -> all-passed.
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Jul 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #33079: this PR implements the same current test-collection poisoning report by preferring the installed PTB library. #33875 is a closed earlier fixture-cluster attempt, not a duplicate anchor.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the PTB collection-order failure.

Current main still has the underlying cross-file state hazard: tests/gateway/conftest.py:97-146 constructs a MagicMock and registers it under telegram, telegram.ext, telegram.constants, and telegram.request, then invokes that setup at collection time (tests/gateway/conftest.py:303). The affected integration module imports BaseRequest from telegram.request and subclasses it (tests/test_telegram_polling_progress_ptb.py:6-15). The PR's import-before-mock approach directly addresses that current state dependency while retaining an ImportError fallback for environments without PTB.

The changed rich-message assertions continue to assert both absence of the raw token and presence of the redacted token (tests/gateway/test_telegram_rich_messages.py:312-318, 454-459); making only the URL-method casing insensitive is consistent with that contract.

No blocking defect found in the reviewed diff. This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 31, 2026
@ai-ag2026 ai-ag2026 closed this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants