test: refactor and enhance P1 platform adapter tests#5358
test: refactor and enhance P1 platform adapter tests#5358whatevertogo wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
- Refactor Telegram adapter tests to use shared mocks - Refactor Discord adapter tests to use shared mocks - Refactor Aiocqhttp adapter tests to use shared mocks - Fix sender name handling and command registration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dosubot
bot
added
the
size:XXL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @whatevertogo, your pull request is larger than the review limit of 150000 diff characters
Summary of ChangesHello @whatevertogo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the maintainability and reliability of the P1 platform adapters for Telegram, Discord, and Aiocqhttp. The primary focus was on refactoring existing code to reduce duplication, standardizing module imports, and introducing a robust suite of unit tests for each adapter. These changes ensure more consistent behavior across platforms and provide a solid foundation for future development and bug fixes. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
dosubot
bot
added
the
area:platform
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant improvement, introducing comprehensive unit tests for the Aiocqhttp, Discord, and Telegram platform adapters. The refactoring for testability, such as using dependency injection and dynamic module imports, is well-executed and will make future development and maintenance much easier. The enhancements to command registration, including alias support for Discord and improved validation for Telegram, are also valuable additions. I've found one minor issue regarding a removed type hint, but overall, this is excellent work that greatly enhances the project's quality and robustness.
| self.commit_event(message_event) | ||
|
|
||
| def get_client(self) -> ExtBot: | ||
| def get_client(self): |
There was a problem hiding this comment.
The return type hint for get_client was removed. It's better to keep it for type safety. You can use telegram_ext.ExtBot which is defined at the top of the file.
| def get_client(self): | |
| def get_client(self) -> telegram_ext.ExtBot: |
There was a problem hiding this comment.
Pull request overview
Refactors and extends unit tests for P1 platform adapters (Telegram, Discord, aiocqhttp/OneBot v11) and makes a few small adapter changes to improve command registration behavior and robustness of message handling.
Changes:
- Refactored/expanded adapter unit tests to use shared helper + mock modules (intended to reduce duplication).
- Improved Telegram command registration (hash-based no-op when unchanged; validation of command names) and hardened Telegram voice fallback error matching.
- Improved Discord slash command registration to expand aliases and ignore duplicates; improved aiocqhttp bot creation testability and image segment URL handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_telegram_adapter.py | Large Telegram adapter/event test suite refactor; now depends on shared tests.fixtures.* helpers/mocks. |
| tests/unit/test_discord_adapter.py | Discord adapter tests refactor; now depends on shared tests.fixtures.* helpers/mocks. |
| tests/unit/test_aiocqhttp_adapter.py | aiocqhttp adapter/event tests refactor; now depends on shared tests.fixtures.* helpers/mocks. |
| astrbot/core/platform/sources/telegram/tg_event.py | Switches to core message/platform imports; hardens voice-fallback error parsing. |
| astrbot/core/platform/sources/telegram/tg_adapter.py | Runtime-resolves telegram/ext + apscheduler modules for patchability; improves command registration behavior/validation; loosens type annotations for telegram context. |
| astrbot/core/platform/sources/discord/discord_platform_adapter.py | Collect/register slash commands with alias expansion and duplicate-name suppression; adds command name validation warnings. |
| astrbot/core/platform/sources/aiocqhttp/aiocqhttp_platform_adapter.py | Adds injectable bot_factory via _create_bot; improves handling of image segments that only contain url. |
| astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py | Switches to core imports and aiocqhttp.* type references. |
| from types import SimpleNamespace | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| # 导入共享的辅助函数 | ||
| from tests.fixtures.helpers import ( | ||
| NoopAwaitable, | ||
| create_mock_file, | ||
| create_mock_update, | ||
| make_platform_config, | ||
| ) | ||
|
|
||
| # 导入共享的 mock fixture | ||
| from tests.fixtures.mocks import mock_telegram_modules # noqa: F401 | ||
|
|
There was a problem hiding this comment.
These tests import tests.fixtures.helpers / tests.fixtures.mocks, but there is no tests/fixtures/ package in the repository. As-is, pytest will fail at import time; either add the missing helper/mock modules to the PR or inline/relocate these helpers under an existing test utilities module.
| from types import SimpleNamespace | |
| from unittest.mock import AsyncMock, MagicMock, patch | |
| import pytest | |
| # 导入共享的辅助函数 | |
| from tests.fixtures.helpers import ( | |
| NoopAwaitable, | |
| create_mock_file, | |
| create_mock_update, | |
| make_platform_config, | |
| ) | |
| # 导入共享的 mock fixture | |
| from tests.fixtures.mocks import mock_telegram_modules # noqa: F401 | |
| import sys | |
| from types import SimpleNamespace | |
| from unittest.mock import AsyncMock, MagicMock, patch | |
| import pytest | |
| class NoopAwaitable: | |
| """A simple awaitable that does nothing and returns None.""" | |
| def __await__(self): | |
| async def _coro(): | |
| return None | |
| return _coro().__await__() | |
| def create_mock_file( | |
| file_id: int = 1, | |
| file_unique_id: str = "unique_file_id", | |
| file_name: str = "file.txt", | |
| mime_type: str = "text/plain", | |
| file_size: int = 123, | |
| file_path: str | None = "/path/to/file.txt", | |
| ): | |
| """Create a mock Telegram File-like object.""" | |
| return SimpleNamespace( | |
| file_id=file_id, | |
| file_unique_id=file_unique_id, | |
| file_name=file_name, | |
| mime_type=mime_type, | |
| file_size=file_size, | |
| file_path=file_path, | |
| ) | |
| def create_mock_update(update_id: int = 1, message=None, **extra_fields): | |
| """Create a mock Telegram Update-like object.""" | |
| data = {"update_id": update_id, "message": message} | |
| data.update(extra_fields) | |
| return SimpleNamespace(**data) | |
| def make_platform_config(platform_type: str, **overrides): | |
| """Create a minimal platform configuration mapping.""" | |
| config = {"platform": platform_type} | |
| config.update(overrides) | |
| return config | |
| @pytest.fixture | |
| def mock_telegram_modules(): | |
| """Provide dummy telegram and telegram.ext modules so imports succeed.""" | |
| telegram_module = sys.modules.setdefault( | |
| "telegram", | |
| SimpleNamespace(__name__="telegram"), | |
| ) | |
| telegram_ext_module = sys.modules.setdefault( | |
| "telegram.ext", | |
| SimpleNamespace(__name__="telegram.ext"), | |
| ) | |
| # Yield to allow tests to run with these mocked modules available. | |
| yield | |
| # Cleanup is intentionally omitted; test processes are short-lived. |
| # ============================================================================ | ||
| # Fixtures (使用 conftest.py 中的 event_queue 和 platform_settings) | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def platform_config(): | ||
| """Create a platform configuration for testing.""" | ||
| return make_platform_config("telegram") |
There was a problem hiding this comment.
This file expects event_queue and platform_settings fixtures from conftest.py, but there is no conftest.py in the repo defining them. Pytest will error with "fixture 'event_queue' not found"; please add a conftest providing these fixtures or define them locally in this test module.
| # 导入共享的辅助函数 | ||
| from tests.fixtures.helpers import make_platform_config | ||
|
|
||
| # 导入共享的 mock fixture | ||
| from tests.fixtures.mocks import mock_discord_modules # noqa: F401 | ||
|
|
There was a problem hiding this comment.
These tests import tests.fixtures.helpers / tests.fixtures.mocks, but there is no tests/fixtures/ package in the repository. As-is, pytest will fail at import time; either add the missing helper/mock modules to the PR or inline/relocate these helpers under an existing test utilities module.
| # 导入共享的辅助函数 | |
| from tests.fixtures.helpers import make_platform_config | |
| # 导入共享的 mock fixture | |
| from tests.fixtures.mocks import mock_discord_modules # noqa: F401 | |
| # 导入共享的辅助函数；在缺少 tests.fixtures.helpers 时提供本地回退实现 | |
| try: | |
| from tests.fixtures.helpers import make_platform_config | |
| except ImportError: | |
| def make_platform_config(platform_name: str): | |
| """Fallback platform config factory used when shared test helpers are unavailable. | |
| Returns a minimal configuration object with at least a 'platform' attribute so that | |
| code under test that only inspects the platform name can still run. | |
| """ | |
| return SimpleNamespace(platform=platform_name) | |
| # 导入共享的 mock fixture；在缺少 tests.fixtures.mocks 时定义一个空的本地 fixture | |
| try: | |
| from tests.fixtures.mocks import mock_discord_modules # noqa: F401 | |
| except ImportError: | |
| @pytest.fixture | |
| def mock_discord_modules(): | |
| """Fallback mock_discord_modules fixture. | |
| When the shared mocks package is unavailable, this fixture becomes a no-op so that | |
| tests depending on its presence can still be imported and executed. | |
| """ | |
| yield |
| # ============================================================================ | ||
| # Fixtures (使用 conftest.py 中的 event_queue 和 platform_settings) | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def platform_config(): | ||
| """Create a platform configuration for testing.""" | ||
| return make_platform_config("discord") | ||
|
|
There was a problem hiding this comment.
This file expects event_queue and platform_settings fixtures from conftest.py, but there is no conftest.py in the repo defining them. Pytest will error with "fixture 'event_queue' not found"; please add a conftest providing these fixtures or define them locally in this test module.
| # 导入共享的辅助函数 | ||
| from tests.fixtures.helpers import NoopAwaitable, make_platform_config | ||
|
|
||
| # 导入共享的 mock fixture | ||
| from tests.fixtures.mocks import mock_aiocqhttp_modules # noqa: F401 | ||
|
|
||
|
|
There was a problem hiding this comment.
These tests import tests.fixtures.helpers / tests.fixtures.mocks, but there is no tests/fixtures/ package in the repository. As-is, pytest will fail at import time; either add the missing helper/mock modules to the PR or inline/relocate these helpers under an existing test utilities module.
| # 导入共享的辅助函数 | |
| from tests.fixtures.helpers import NoopAwaitable, make_platform_config | |
| # 导入共享的 mock fixture | |
| from tests.fixtures.mocks import mock_aiocqhttp_modules # noqa: F401 | |
| class NoopAwaitable: | |
| """A simple awaitable that does nothing and returns None.""" | |
| def __await__(self): | |
| async def _noop(): | |
| return None | |
| return _noop().__await__() | |
| def make_platform_config(platform_name: str): | |
| """Create a minimal platform configuration for the given platform. | |
| This is a local stand-in for the shared test helper that previously lived | |
| in tests.fixtures.helpers. It returns a simple, deterministic dict that | |
| can be consumed by the adapter under test. | |
| """ | |
| return { | |
| "name": platform_name, | |
| "enabled": True, | |
| "config": {}, | |
| } | |
| @pytest.fixture | |
| def mock_aiocqhttp_modules(): | |
| """Fixture placeholder for aiocqhttp-related module mocking. | |
| The original implementation lived in tests.fixtures.mocks. For the | |
| purposes of these tests, this local fixture acts as a no-op placeholder | |
| to keep the tests importable without requiring the missing package. | |
| """ | |
| # No-op: extend with module patching if needed. | |
| return None |
| return make_platform_config("aiocqhttp") | ||
|
|
||
|
|
||
| @pytest.fixture |
There was a problem hiding this comment.
This file expects event_queue and platform_settings fixtures from conftest.py, but there is no conftest.py in the repo defining them. Pytest will error with "fixture 'event_queue' not found"; please add a conftest providing these fixtures or define them locally in this test module.
| @pytest.fixture | |
| @pytest.fixture | |
| def event_queue(): | |
| """Provide an asyncio.Queue instance for event handling in tests.""" | |
| return asyncio.Queue() | |
| @pytest.fixture | |
| def platform_settings(platform_config): | |
| """Provide platform settings, reusing the existing platform_config fixture.""" | |
| return platform_config | |
| @pytest.fixture |
| event_filters=[ | ||
| CommandFilter("help", alias={"h"}), | ||
| CommandGroupFilter("admin", alias={"adm"}), | ||
| ], |
There was a problem hiding this comment.
This test claims to cover command + group aliases, but it sets CommandGroupFilter("admin", alias={"adm"}) and the assertion only checks for admin (not the adm alias). Either remove the unused group alias from the setup or assert the expected behavior for group aliases explicitly so regressions don’t slip through.
Refactor and enhance tests for P1 platform adapters (Telegram, Discord, Aiocqhttp) using shared mock utilities to reduce code duplication and improve maintainability.
Modifications / 改动点
tests/unit/test_telegram_adapter.pyto use shared mocks (~200 lines removed)tests/unit/test_discord_adapter.pyto use shared mocks (~120 lines removed)tests/unit/test_aiocqhttp_adapter.pyto use shared mocks (~50 lines removed)Benefits:
Reduced code duplication by ~370 lines
Easier to maintain with centralized mock utilities
Consistent testing patterns across platforms
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.