Skip to content

fix(tests): use str-enum for mock ParseMode/ChatType to fix order-dependent failures - #33875

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/telegram-test-parse-mode-enum
Closed

fix(tests): use str-enum for mock ParseMode/ChatType to fix order-dependent failures#33875
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/telegram-test-parse-mode-enum

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 28, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

The _ensure_telegram_mock() helper in tests/gateway/conftest.py and 15 per-file copies set ParseMode.MARKDOWN_V2 = "MarkdownV2" as a plain string. When installed before the real python-telegram-bot package is imported, this poisons sys.modules for the entire pytest session. Tests asserting repr(parse_mode) fail because the mock produces 'MarkdownV2' (plain string repr) instead of ParseMode.MARKDOWN_V2 (enum repr containing "MARKDOWN_V2").

6 tests fail in combined pytest tests/gateway/ -k telegram but pass individually:

  • test_telegram_approval_buttons.py::test_send_update_prompt_escapes_dynamic_prompt
  • test_telegram_approval_buttons.py::test_approval_callback_escapes_dynamic_user_name
  • test_telegram_model_picker.py::test_send_model_picker_escapes_dynamic_provider_label
  • test_telegram_model_picker.py::test_back_button_escapes_dynamic_provider_label
  • test_telegram_model_picker.py::test_model_selected_edits_message_on_success
  • test_telegram_slash_confirm.py::test_uses_markdown_v2_and_escapes_special_chars

Related Issue

Fixes #33079

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

Code Intelligence

  • Analyzed: tests/gateway/conftest.py:_ensure_telegram_mock and 15 per-file copies
  • Blast radius: LOW — test-only change, no production code modified
  • Related patterns: sys.modules mock poisoning is a common pytest anti-pattern; str-enum ensures type-compatible comparisons

Checklist

  • Tests pass (638/638 telegram tests in combined run)
  • One root cause, focused diff
  • No unrelated changes
  • Regression test: the 6 previously-failing tests now pass in combined runs

…endent failures

The _ensure_telegram_mock() helper in tests/gateway/conftest.py and 15
per-file copies set ParseMode.MARKDOWN_V2 = "MarkdownV2" as a plain
string. When installed before the real python-telegram-bot package is
imported, this poisons sys.modules for the entire pytest session.
Tests asserting repr(parse_mode) or isinstance checks fail because
the mock produces a plain string instead of the str-enum member.

Fix: replace plain string assignments with proper str-enum classes
(_ParseMode, _ChatType) whose repr() matches the real package.
Also set enum references directly on the module object (mod.ParseMode,
mod.ChatType) so that 'from telegram.constants import ChatType'
resolves correctly.

Fixes NousResearch#33079
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels May 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Fixes #33079. Note: competing PR #28134 (open) also centralizes gateway mock installers with a broader scope (discord + telegram mock isolation + cache clearing). This PR is narrower and more targeted at the specific ParseMode/ChatType str-enum repr issue.

@Morad37

Morad37 commented May 28, 2026

Copy link
Copy Markdown
Contributor

The str-enum mock approach is the right call here. One thing to watch for: some test assertions in the codebase directly compare against the string value ("Markdown") rather than using the enum member. If a downstream assertion does , the str-enum will still match because resolves True thanks to the str mixin. But if anything does a type check (), it'll pass now since str-enum is both a str and an Enum. Should be compatible, just noting it.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Good point about downstream assertions comparing against string values. I checked the test files that import ParseMode and ChatType — they all use == comparisons against enum members (e.g., ParseMode.MARKDOWN), not raw strings. The str-enum approach preserves backward compatibility because str_enum_instance == "Markdown" resolves to True via __eq__.

If you spot any specific assertion that breaks, let me know and I'll add a targeted test.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the Telegram mock-shape issue. The central change targets a real current-main mismatch: tests/gateway/conftest.py:64-70 configures nested constants, while tests/gateway/conftest.py:91-98 registers the root mock as telegram.constants; the adapter imports directly from that module at plugins/platforms/telegram/adapter.py:169.

Problems

  • tests/e2e/conftest.py:14 adds import enum without using it or changing the E2E Telegram mock. That hunk does not affect ParseMode/ChatType behavior.

Suggested changes

  • Drop the unused E2E import, or fully convert that fixture if E2E coverage needs the same import contract.
  • During salvage, add a focused regression that imports ParseMode and ChatType from telegram.constants; this directly guards the root-versus-nested mock path that the adapter uses.

Automated hermes-sweeper review.

Comment thread tests/e2e/conftest.py

import asyncio
import sys
import enum

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is unused: the diff does not change this fixture's Telegram mock. Please remove this hunk, or convert ParseMode/ChatType and expose them on telegram_mod if the E2E fixture needs the same direct-import contract.

@teknium1 teknium1 added sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @liuhao1024 — closing in favor of #68873, which fixes the same telegram-mock str-enum class at the poison source (the dm_topics unconditional divergent mock) with a single PTB-faithful _FakeEnumMember instead of 15 per-file patches. You diagnosed this class TWO MONTHS earlier — you are credited as the earliest diagnoser in the landing commit. The salvage of #68873 is in progress.

@teknium1 teknium1 closed this Jul 30, 2026
teknium1 pushed a commit that referenced this pull request Jul 30, 2026
The file-local telegram mock in test_dm_topics.py installed unconditionally
(no __file__ guard), registered a separate string-valued telegram.constants
module, and force-popped the adapter — poisoning the session for any later
telegram test in the same process (assert 'MARKDOWN_V2' in "'MarkdownV2'").

Fix at the source:
- conftest: _FakeEnumMember(str) with PTB-faithful str()==value and
  repr()==<ChatType.X: 'x'>, satisfying both repr assertions and the
  adapter's str(chat.type) normalization; the same object is bound to
  mod.ParseMode and mod.constants.ParseMode.
- test_dm_topics.py: delete the divergent local mock installer; import the
  shared conftest one.
- release.py: mailmap entry for the author.

Verified: the 5-failure cluster repro (dm_topics + slash_confirm +
approval_buttons + model_picker + network_reconnect + telegram_format in one
process) goes 83/83 green (3x); full tests/gateway single-process run drops
10 -> 5 failed, the remainder being pre-existing discord order-dep failures
out of scope here.

Salvaged from #68873. Credit to @liuhao1024 for the earliest root-cause
diagnosis of this str-enum mock class in PR #33875, two months earlier.

Fixes the telegram-mock order-dependent flake cluster.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
The file-local telegram mock in test_dm_topics.py installed unconditionally
(no __file__ guard), registered a separate string-valued telegram.constants
module, and force-popped the adapter — poisoning the session for any later
telegram test in the same process (assert 'MARKDOWN_V2' in "'MarkdownV2'").

Fix at the source:
- conftest: _FakeEnumMember(str) with PTB-faithful str()==value and
  repr()==<ChatType.X: 'x'>, satisfying both repr assertions and the
  adapter's str(chat.type) normalization; the same object is bound to
  mod.ParseMode and mod.constants.ParseMode.
- test_dm_topics.py: delete the divergent local mock installer; import the
  shared conftest one.
- release.py: mailmap entry for the author.

Verified: the 5-failure cluster repro (dm_topics + slash_confirm +
approval_buttons + model_picker + network_reconnect + telegram_format in one
process) goes 83/83 green (3x); full tests/gateway single-process run drops
10 -> 5 failed, the remainder being pre-existing discord order-dep failures
out of scope here.

Salvaged from NousResearch#68873. Credit to @liuhao1024 for the earliest root-cause
diagnosis of this str-enum mock class in PR NousResearch#33875, two months earlier.

Fixes the telegram-mock order-dependent flake cluster.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
The file-local telegram mock in test_dm_topics.py installed unconditionally
(no __file__ guard), registered a separate string-valued telegram.constants
module, and force-popped the adapter — poisoning the session for any later
telegram test in the same process (assert 'MARKDOWN_V2' in "'MarkdownV2'").

Fix at the source:
- conftest: _FakeEnumMember(str) with PTB-faithful str()==value and
  repr()==<ChatType.X: 'x'>, satisfying both repr assertions and the
  adapter's str(chat.type) normalization; the same object is bound to
  mod.ParseMode and mod.constants.ParseMode.
- test_dm_topics.py: delete the divergent local mock installer; import the
  shared conftest one.
- release.py: mailmap entry for the author.

Verified: the 5-failure cluster repro (dm_topics + slash_confirm +
approval_buttons + model_picker + network_reconnect + telegram_format in one
process) goes 83/83 green (3x); full tests/gateway single-process run drops
10 -> 5 failed, the remainder being pre-existing discord order-dep failures
out of scope here.

Salvaged from NousResearch#68873. Credit to @liuhao1024 for the earliest root-cause
diagnosis of this str-enum mock class in PR NousResearch#33875, two months earlier.

Fixes the telegram-mock order-dependent flake cluster.
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
The file-local telegram mock in test_dm_topics.py installed unconditionally
(no __file__ guard), registered a separate string-valued telegram.constants
module, and force-popped the adapter — poisoning the session for any later
telegram test in the same process (assert 'MARKDOWN_V2' in "'MarkdownV2'").

Fix at the source:
- conftest: _FakeEnumMember(str) with PTB-faithful str()==value and
  repr()==<ChatType.X: 'x'>, satisfying both repr assertions and the
  adapter's str(chat.type) normalization; the same object is bound to
  mod.ParseMode and mod.constants.ParseMode.
- test_dm_topics.py: delete the divergent local mock installer; import the
  shared conftest one.
- release.py: mailmap entry for the author.

Verified: the 5-failure cluster repro (dm_topics + slash_confirm +
approval_buttons + model_picker + network_reconnect + telegram_format in one
process) goes 83/83 green (3x); full tests/gateway single-process run drops
10 -> 5 failed, the remainder being pre-existing discord order-dep failures
out of scope here.

Salvaged from NousResearch#68873. Credit to @liuhao1024 for the earliest root-cause
diagnosis of this str-enum mock class in PR NousResearch#33875, two months earlier.

Fixes the telegram-mock order-dependent flake cluster.
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.

Telegram gateway tests: sys.modules ParseMode mock poisoning causes order-dependent false failures

4 participants