Skip to content

Conversation

@RC-CHN
Copy link
Member

@RC-CHN RC-CHN commented Sep 15, 2025

Motivation / 动机

似乎单元测试一直是坏的,至少需要开个头把他修复起来

Modifications / 改动点

修改了tests文件夹下的pytest脚本,未修改主程序

Verification Steps / 验证步骤

使用pytest框架运行测试即可

Screenshots or Test Results / 运行截图或测试结果

除pipeline部分外已修复image

Compatibility & Breaking Changes / 兼容性与破坏性变更

重构并拆分了部分测试方法,但原来测试本就是不可用状态

  • 这是一个破坏性变更 (Breaking Change)。/ This is a breaking change.
  • 这不是一个破坏性变更。/ This is NOT a breaking change.

Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Sourcery 总结

全面改进测试套件,通过重构和隔离 PluginManager、dashboard 和主测试来修复 pytest 故障,采用异步 fixture 并模拟外部依赖项,以实现可靠执行。

增强功能:

  • 重构 PluginManager 测试以使用临时环境、隔离 CRUD 操作并添加异常场景
  • 更新 dashboard 测试以利用 pytest_asyncio fixture,引入 authenticated_header fixture 并替换 header 用法
  • 在更新端点测试中模拟核心更新、下载和安装步骤,以防止副作用
  • 将 check_dashboard_files 测试扩展为针对缺失文件、版本匹配、版本不匹配和自定义 webui_dir 的不同用例
Original summary in English

Summary by Sourcery

Overhaul the test suite by refactoring and isolating PluginManager, dashboard, and main tests to fix pytest failures, adopt async fixtures, and mock external dependencies for reliable execution.

Enhancements:

  • Refactor PluginManager tests to use temporary environments, isolate CRUD operations, and add exception scenarios
  • Update dashboard tests to leverage pytest_asyncio fixtures, introduce an authenticated_header fixture, and replace header usage
  • Mock core update, download, and installation steps in the update endpoint test to prevent side effects
  • Expand check_dashboard_files tests into distinct cases for missing files, matching versions, version mismatches, and custom webui_dir

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

你好 - 我已审阅了你的更改 - 以下是一些反馈意见:

  • 插件管理器测试仍然执行真实的 GitHub 安装——考虑模拟克隆/下载步骤或使用本地测试夹具以避免网络不稳定。
  • 在你的仪表板测试中,你混合使用了 pytest.fixture 和 pytest_asyncio.fixture 来处理异步夹具——请统一它们(及其作用域),使所有异步夹具都使用 pytest_asyncio 以提高清晰度。
  • 在 test_main 中,你调用了 mock.patch 而没有导入或别名 mock;请添加正确的导入(例如,from unittest import mock)以防止 NameError。
供 AI 代理的提示
请处理此代码审查中的评论:
## 总体评论
- 插件管理器测试仍然执行真实的 GitHub 安装——考虑模拟克隆/下载步骤或使用本地测试夹具以避免网络不稳定。
- 在你的仪表板测试中,你混合使用了 pytest.fixture 和 pytest_asyncio.fixture 来处理异步夹具——请统一它们(及其作用域),使所有异步夹具都使用 pytest_asyncio 以提高清晰度。
- 在 test_main 中,你调用了 mock.patch 而没有导入或别名 mock;请添加正确的导入(例如,from unittest import mock)以防止 NameError。

## 单独评论

### 评论 1
<location> `tests/test_plugin_manager.py:100` </location>
<code_context>
+
+
+@pytest.mark.asyncio
+async def test_update_plugin(plugin_manager_pm: PluginManager):
+    """Tests updating an existing plugin in an isolated environment."""
+    # First, install the plugin
+    test_repo = "https://github.com/Soulter/astrbot_plugin_essential"
+    await plugin_manager_pm.install_plugin(test_repo)

-    # update
+    # Then, update it
     await plugin_manager_pm.update_plugin("astrbot_plugin_essential")

+
</code_context>

<issue_to_address>
测试未对插件更新的任何结果进行断言。

添加断言以确认插件已更新,例如验证文件更改、版本或注册表状态。
</issue_to_address>

### 评论 2
<location> `tests/test_dashboard.py:84` </location>
<code_context>

 @pytest.mark.asyncio
-async def test_plugins(app: Quart, header: dict):
+async def test_plugins(app: Quart, authenticated_header: dict):
     test_client = app.test_client()
     # 已经安装的插件
-    response = await test_client.get("/api/plugin/get", headers=header)
+    response = await test_client.get("/api/plugin/get", headers=authenticated_header)
     assert response.status_code == 200
     data = await response.get_json()
     assert data["status"] == "ok"

     # 插件市场
</code_context>

<issue_to_address>
缺少使用无效 URL 安装插件的负面测试。

添加一个测试,尝试使用无效或格式错误的 URL 安装插件,并验证 API 返回错误响应。
</issue_to_address>

Sourcery 对开源项目免费 - 如果你喜欢我们的评论,请考虑分享它们 ✨
帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English

Hey there - I've reviewed your changes - here's some feedback:

  • The plugin manager tests still perform real GitHub installs—consider mocking the clone/download steps or using a local test fixture to avoid network flakiness.
  • You’ve mixed pytest.fixture and pytest_asyncio.fixture for async fixtures in your dashboard tests—unify them (and their scopes) so all async fixtures use pytest_asyncio for clarity.
  • In test_main you call mock.patch without importing or aliasing mock; add the proper import (e.g., from unittest import mock) to prevent NameError.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The plugin manager tests still perform real GitHub installs—consider mocking the clone/download steps or using a local test fixture to avoid network flakiness.
- You’ve mixed pytest.fixture and pytest_asyncio.fixture for async fixtures in your dashboard tests—unify them (and their scopes) so all async fixtures use pytest_asyncio for clarity.
- In test_main you call mock.patch without importing or aliasing mock; add the proper import (e.g., from unittest import mock) to prevent NameError.

## Individual Comments

### Comment 1
<location> `tests/test_plugin_manager.py:100` </location>
<code_context>
+
+
+@pytest.mark.asyncio
+async def test_update_plugin(plugin_manager_pm: PluginManager):
+    """Tests updating an existing plugin in an isolated environment."""
+    # First, install the plugin
+    test_repo = "https://github.com/Soulter/astrbot_plugin_essential"
+    await plugin_manager_pm.install_plugin(test_repo)

-    # update
+    # Then, update it
     await plugin_manager_pm.update_plugin("astrbot_plugin_essential")

+
</code_context>

<issue_to_address>
Test does not assert any outcome for plugin update.

Add assertions to confirm the plugin was updated, such as verifying changes to files, version, or registry state.
</issue_to_address>

### Comment 2
<location> `tests/test_dashboard.py:84` </location>
<code_context>

 @pytest.mark.asyncio
-async def test_plugins(app: Quart, header: dict):
+async def test_plugins(app: Quart, authenticated_header: dict):
     test_client = app.test_client()
     # 已经安装的插件
-    response = await test_client.get("/api/plugin/get", headers=header)
+    response = await test_client.get("/api/plugin/get", headers=authenticated_header)
     assert response.status_code == 200
     data = await response.get_json()
     assert data["status"] == "ok"

     # 插件市场
</code_context>

<issue_to_address>
Missing negative test for plugin installation with invalid URL.

Add a test that attempts to install a plugin using an invalid or malformed URL, and verify that the API returns an error response.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Soulter Soulter changed the title Fix pytest fix: unit tests Sep 16, 2025
@Soulter Soulter merged commit 19d7438 into AstrBotDevs:master Sep 27, 2025
4 checks passed
@RC-CHN RC-CHN deleted the fix-pytest branch September 27, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants