fix(test): 修复 batch_download session 测试全量跑时 flaky 失败 #1409

Merged
xiaoxia merged 2 commits from fix/batch-download-session-flaky-v2 into develop 2026-08-17 21:08:52 +08:00
Owner

问题

PR #1406 修复了 test_batch_download_session_closedtest_batch_download_closes_session_on_error 的 flaky 行为,使用 patch.object(_db_mod, "SessionLocal", ...) 替代字符串路径 patch。

但在 develop push 的全量测试中(Run #32255),这两个测试仍然失败:

FAILED test_batch_download_session_closed - AssertionError: Expected close to have been called once. Called 0 times.
FAILED test_batch_download_closes_session_on_error - AssertionError: Expected close to have been called once. Called 0 times.

增量模式(只跑改动的测试文件)通过,全量模式失败 — 说明前序测试影响了 SessionLocal 的 mock 效果。

根因分析

batch_download_videos 函数内部做 from worker_app.db import SessionLocalpatch.object 替换了模块属性,但在全量跑时,前序测试可能通过不同路径导入模块(如 apps.worker.worker_app.db vs worker_app.db),导致 sys.modules 中的模块引用不一致。

修复

  1. 在所有可能的模块路径上同时 patch SessionLocal_db_modsys.modules["worker_app.db"]、函数 __globals__(如果存在)
  2. 直接调用 raw_fn 而非通过 _call_task 间接调用
  3. 增加诊断断言 mock_session_factory.called,帮助定位是 mock 未生效还是 session 未关闭

验证

  • 增量模式 pytest 通过
  • 等待全量 CI 验证
## 问题 PR #1406 修复了 `test_batch_download_session_closed` 和 `test_batch_download_closes_session_on_error` 的 flaky 行为,使用 `patch.object(_db_mod, "SessionLocal", ...)` 替代字符串路径 patch。 但在 develop push 的全量测试中(Run #32255),这两个测试仍然失败: ``` FAILED test_batch_download_session_closed - AssertionError: Expected close to have been called once. Called 0 times. FAILED test_batch_download_closes_session_on_error - AssertionError: Expected close to have been called once. Called 0 times. ``` 增量模式(只跑改动的测试文件)通过,全量模式失败 — 说明前序测试影响了 `SessionLocal` 的 mock 效果。 ## 根因分析 `batch_download_videos` 函数内部做 `from worker_app.db import SessionLocal`。`patch.object` 替换了模块属性,但在全量跑时,前序测试可能通过不同路径导入模块(如 `apps.worker.worker_app.db` vs `worker_app.db`),导致 `sys.modules` 中的模块引用不一致。 ## 修复 1. 在所有可能的模块路径上同时 patch `SessionLocal`:`_db_mod`、`sys.modules["worker_app.db"]`、函数 `__globals__`(如果存在) 2. 直接调用 `raw_fn` 而非通过 `_call_task` 间接调用 3. 增加诊断断言 `mock_session_factory.called`,帮助定位是 mock 未生效还是 session 未关闭 ## 验证 - 增量模式 pytest 通过 - 等待全量 CI 验证
xiaoxia added 1 commit 2026-08-17 20:53:31 +08:00
fix(test): harden batch_download session tests with multi-path patching + diagnostic
CI/CD Pipeline / Build Staging API Image (pull_request) Has been skipped
CI/CD Pipeline / Build Staging Worker Image (pull_request) Has been skipped
CI/CD Pipeline / Build Staging Web Image (pull_request) Has been skipped
CI/CD Pipeline / Deploy Staging (Watchtower auto-deploy) (pull_request) Has been skipped
CI/CD Pipeline / Staging E2E Tests (pull_request) Has been skipped
CI/CD Pipeline / Staging API Integration Tests (pull_request) Has been skipped
CI/CD Pipeline / ACR Image Cleanup (pull_request) Has been skipped
CI/CD Pipeline / Check if frontend-only change (pull_request) Successful in 21s
CI/CD Pipeline / Frontend Lint (pull_request) Has been skipped
CI/CD Pipeline / Frontend Unit Tests (pull_request) Has been skipped
CI/CD Pipeline / PR Build Web Image (pull_request) Has been skipped
CI/CD Pipeline / Validate - Migration (alembic) (pull_request) Successful in 1m49s
AI Code Review / AI Code Review (pull_request) Failing after 1m53s
Preview Deploy / Deploy Preview Environment (pull_request) Successful in 1m53s
PR Automation / Auto Merge on CI Green + Approved (pull_request) Successful in 2m6s
CI/CD Pipeline / Validate - Type Check (mypy) (pull_request) Successful in 3m11s
PR Automation / Auto Approve on CI Green (pull_request) Successful in 3m59s
CI/CD Pipeline / Validate - Code Quality (pull_request) Has been cancelled
CI/CD Pipeline / Unit Tests (pull_request) Has been cancelled
CI/CD Pipeline / Integration Tests (pull_request) Has been cancelled
CI/CD Pipeline / PR Build API Image (pull_request) Has been cancelled
CI/CD Pipeline / PR Build Worker Image (pull_request) Has been cancelled
CI/CD Pipeline / Build Production API Image (pull_request) Has been cancelled
CI/CD Pipeline / Build Production Web Image (pull_request) Has been cancelled
CI/CD Pipeline / Build Production Worker Image (pull_request) Has been cancelled
CI/CD Pipeline / Deploy Production (pull_request) Has been cancelled
CI/CD Pipeline / Production Browser E2E (pull_request) Has been cancelled
CI/CD Pipeline / Canary Release to Production (pull_request) Has been cancelled
CI/CD Pipeline / CI Gate (pull_request) Has been cancelled
a6baaf9829
Full-suite runs showed test_batch_download_session_closed and
test_batch_download_closes_session_on_error failing with
"Expected close to have been called once. Called 0 times."

Changes:
- Patch SessionLocal in all possible locations: _db_mod, sys.modules,
  and function globals (if present)
- Call raw_fn directly instead of going through _call_task
- Add diagnostic assertion to verify mock_session_factory was called
- Remove try/finally restore (not needed for isolated tests)

Also includes fix from PR #1407: add missing str fields to mock clip.

🚀 预览环境已部署

项目 详情
PR号 #1409
预览链接 https://pr-1409.preview.xiaoxiajianji.com
API环境 staging

💡 预览环境使用 staging API 数据,请勿在预览环境中操作重要数据。

🔄 每次提交新代码后预览环境会自动更新。

🗑️ PR 关闭或合并后,预览环境会自动清理。

🚀 **预览环境已部署** | 项目 | 详情 | |------|------| | PR号 | #1409 | | 预览链接 | [https://pr-1409.preview.xiaoxiajianji.com](https://pr-1409.preview.xiaoxiajianji.com) | | API环境 | staging | > 💡 预览环境使用 staging API 数据,请勿在预览环境中操作重要数据。 > > 🔄 每次提交新代码后预览环境会自动更新。 > > 🗑️ PR 关闭或合并后,预览环境会自动清理。
auto-approve-bot added 1 commit 2026-08-17 20:57:47 +08:00
style: auto-format with black + isort + prettier [skip ci-format-check]
CI/CD Pipeline / Build Staging API Image (pull_request) Has been skipped
CI/CD Pipeline / Build Staging Web Image (pull_request) Has been skipped
CI/CD Pipeline / Build Staging Worker Image (pull_request) Has been skipped
PR Automation / Auto Merge on CI Green + Approved (pull_request) Failing after 1s
CI/CD Pipeline / Deploy Staging (Watchtower auto-deploy) (pull_request) Has been skipped
CI/CD Pipeline / Staging E2E Tests (pull_request) Has been skipped
CI/CD Pipeline / Staging API Integration Tests (pull_request) Has been skipped
CI/CD Pipeline / ACR Image Cleanup (pull_request) Has been skipped
CI/CD Pipeline / Check if frontend-only change (pull_request) Successful in 44s
CI/CD Pipeline / Frontend Unit Tests (pull_request) Has been skipped
CI/CD Pipeline / Frontend Lint (pull_request) Has been skipped
CI/CD Pipeline / PR Build Web Image (pull_request) Has been skipped
CI/CD Pipeline / PR Build Worker Image (pull_request) Failing after 0s
AI Code Review / AI Code Review (pull_request) Failing after 2m4s
CI/CD Pipeline / Validate - Migration (alembic) (pull_request) Successful in 2m12s
CI/CD Pipeline / Validate - Type Check (mypy) (pull_request) Successful in 2m23s
Preview Deploy / Deploy Preview Environment (pull_request) Successful in 2m24s
PR Automation / Auto Approve on CI Green (pull_request) Successful in 3m30s
CI/CD Pipeline / PR Build API Image (pull_request) Successful in 3m41s
CI/CD Pipeline / Unit Tests (pull_request) Successful in 4m1s
CI/CD Pipeline / Validate - Code Quality (pull_request) Successful in 6m59s
CI/CD Pipeline / Build Production Web Image (pull_request) Has been skipped
CI/CD Pipeline / Build Production API Image (pull_request) Has been skipped
CI/CD Pipeline / Build Production Worker Image (pull_request) Has been skipped
CI/CD Pipeline / Deploy Production (pull_request) Has been skipped
CI/CD Pipeline / Canary Release to Production (pull_request) Has been skipped
CI/CD Pipeline / Production Browser E2E (pull_request) Has been skipped
CI/CD Pipeline / Integration Tests (pull_request) Successful in 2m32s
CI/CD Pipeline / CI Gate (pull_request) Failing after 5s
ACR Cleanup / ACR Image Cleanup (pull_request_target) Successful in 29s
Preview Cleanup / Cleanup Preview Environment (pull_request) Successful in 1m2s
44648320df
Collaborator

【阻塞级判定】

  • 是否存在阻塞级问题:是
  • 阻塞级问题数量:2 个

📊 审查概览

  • 整体评价:需修改
  • 建议级问题数量:1 个

🔴 阻塞级问题(必须修复)

  1. [tests/unit/test_batch_download.py: 282-284] 严重破坏测试隔离性,永久污染全局状态

    • 问题类型:逻辑bug
    • 问题描述:代码直接修改了 sys.modules 和模块全局变量中的 SessionLocal,但在 finally 块中显式不进行还原(pass)。这导致测试运行后,Mock 对象永久残留在 Python 解释器的全局模块缓存中。后续运行的测试将直接使用这个 Mock 对象,而不是它们预期的配置,导致测试结果不可信,极易引发“测试在单独运行时通过,但在全量运行时失败”的问题,严重破坏 CI/CD 的稳定性。
    • 修改建议:必须在 finally 块中恢复原始的 SessionLocal 对象。或者,更推荐的做法是使用 unittest.mock.patch 上下文管理器或 pytestmonkeypatch fixture,它们会自动处理清理工作,避免手动管理全局状态带来的风险。
  2. [tests/unit/test_batch_download.py: 299] 绕过任务包装器调用,可能导致测试场景失真

    • 问题类型:逻辑bug
    • 问题描述:代码将原有的 _call_task(batch_download_videos, ...) 修改为直接调用 raw_fn(...)。通常 _call_task 封装了 Celery 任务的上下文初始化(如 current_task)、请求 ID 绑定或特定的异常处理逻辑。直接调用原始函数破坏了这一层封装,可能导致测试无法正确模拟真实的异步任务执行环境,从而遗漏上下文相关的 Bug(例如依赖 current_task 的逻辑将无法被测试覆盖)。
    • 修改建议:应继续使用 _call_task 调用方式,或者确保 raw_fn 的调用方式能完整复现任务执行时的上下文环境。如果为了解决导入缓存问题必须绕过,应证明 _call_task 在此场景下是多余的且不影响测试有效性。

💡 改进建议(不阻塞合并)

  1. [tests/unit/test_batch_download.py: 273-278] 避免手动修改 sys.modules,建议使用标准 Mock 工具
    • 具体内容:手动修改 sys.modules__globals__ 是一种极其脆弱的做法(Fragile Test),容易随着代码重构或导入顺序的变化而失效。建议使用 pytestmonkeypatchunittest.mock.patch,或者通过 importlib.reload 在测试 setup 阶段解决导入缓存问题,而不是暴力修改全局状态。

良好实践

  • 针对测试中遇到的 PromiseProxy 缓存难点进行了深入的尝试和修复,表明开发者关注到了测试隔离中的复杂问题。

格式检查通过 | 逻辑审查需修改 | 性能无明显问题


🤖 由 AI 代码审查机器人自动生成 | 2026-08-17 12:59:52 | 模型:

### 【阻塞级判定】 - 是否存在阻塞级问题:是 - 阻塞级问题数量:2 个 ### 📊 审查概览 - 整体评价:需修改 - 建议级问题数量:1 个 ### 🔴 阻塞级问题(必须修复) 1. **[tests/unit/test_batch_download.py: 282-284] 严重破坏测试隔离性,永久污染全局状态** - 问题类型:逻辑bug - 问题描述:代码直接修改了 `sys.modules` 和模块全局变量中的 `SessionLocal`,但在 `finally` 块中显式不进行还原(`pass`)。这导致测试运行后,Mock 对象永久残留在 Python 解释器的全局模块缓存中。后续运行的测试将直接使用这个 Mock 对象,而不是它们预期的配置,导致测试结果不可信,极易引发“测试在单独运行时通过,但在全量运行时失败”的问题,严重破坏 CI/CD 的稳定性。 - 修改建议:必须在 `finally` 块中恢复原始的 `SessionLocal` 对象。或者,更推荐的做法是使用 `unittest.mock.patch` 上下文管理器或 `pytest` 的 `monkeypatch` fixture,它们会自动处理清理工作,避免手动管理全局状态带来的风险。 2. **[tests/unit/test_batch_download.py: 299] 绕过任务包装器调用,可能导致测试场景失真** - 问题类型:逻辑bug - 问题描述:代码将原有的 `_call_task(batch_download_videos, ...)` 修改为直接调用 `raw_fn(...)`。通常 `_call_task` 封装了 Celery 任务的上下文初始化(如 `current_task`)、请求 ID 绑定或特定的异常处理逻辑。直接调用原始函数破坏了这一层封装,可能导致测试无法正确模拟真实的异步任务执行环境,从而遗漏上下文相关的 Bug(例如依赖 `current_task` 的逻辑将无法被测试覆盖)。 - 修改建议:应继续使用 `_call_task` 调用方式,或者确保 `raw_fn` 的调用方式能完整复现任务执行时的上下文环境。如果为了解决导入缓存问题必须绕过,应证明 `_call_task` 在此场景下是多余的且不影响测试有效性。 ### 💡 改进建议(不阻塞合并) 1. **[tests/unit/test_batch_download.py: 273-278] 避免手动修改 sys.modules,建议使用标准 Mock 工具** - 具体内容:手动修改 `sys.modules` 和 `__globals__` 是一种极其脆弱的做法(Fragile Test),容易随着代码重构或导入顺序的变化而失效。建议使用 `pytest` 的 `monkeypatch` 或 `unittest.mock.patch`,或者通过 `importlib.reload` 在测试 setup 阶段解决导入缓存问题,而不是暴力修改全局状态。 ### ✅ 良好实践 - 针对测试中遇到的 `PromiseProxy` 缓存难点进行了深入的尝试和修复,表明开发者关注到了测试隔离中的复杂问题。 --- ✅ 格式检查通过 | ❌ 逻辑审查需修改 | ✅ 性能无明显问题 --- <sub>🤖 由 AI 代码审查机器人自动生成 | 2026-08-17 12:59:52 | 模型: </sub> <!-- AI_CODE_REVIEW_AUTO_COMMENT -->
auto-approve-bot approved these changes 2026-08-17 21:05:30 +08:00
auto-approve-bot left a comment
Collaborator

CI全绿,自动审批通过。

CI全绿,自动审批通过。
auto-approve-bot approved these changes 2026-08-17 21:05:30 +08:00
auto-approve-bot left a comment
Collaborator

CI全绿,自动审批通过。

CI全绿,自动审批通过。
xiaoxia merged commit cf796c2e3c into develop 2026-08-17 21:08:52 +08:00

🗑️ 预览环境已清理

PR #1409 已关闭或合并,对应的预览环境已被清理。

如有需要,可以重新打开 PR 来重新生成预览环境。

🗑️ **预览环境已清理** PR #1409 已关闭或合并,对应的预览环境已被清理。 > 如有需要,可以重新打开 PR 来重新生成预览环境。
Sign in to join this conversation.