Skip to content

Fix: OpenAI LLM test memory exhaustion during collection#3291

Merged
WMC001 merged 1 commit into
developfrom
fix/openai-llm-test-memory
Jun 24, 2026
Merged

Fix: OpenAI LLM test memory exhaustion during collection#3291
WMC001 merged 1 commit into
developfrom
fix/openai-llm-test-memory

Conversation

@JasonW404

Copy link
Copy Markdown
Member

Problem

test/sdk/core/models/test_openai_llm.py was calling spec.loader.exec_module() at module level during test collection. This triggered a second import of openai_llm before the full mock graph was installed, which initialised the real monitoring stack and exhausted local resources (memory).

Fix

Remove the premature exec_module() call and its try/except cleanup. Instead, defer the import to the test body where the mock graph is fully in place. OpenAIModel is set to None at module level and resolved lazily.

Changes

  • test/sdk/core/models/test_openai_llm.py — removed 10 lines of eager module execution, added explanatory comment (4 lines)

Copilot AI review requested due to automatic review settings June 24, 2026 02:40

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@YehongPan

Copy link
Copy Markdown
Contributor

🔍 Code Review Comments

1. [代码规范] 模块初始化逻辑缺失
删除了 spec.loader.exec_module 的 try/except 块但没有补充替代的模块初始化逻辑,后续如果 OpenAIModel 需要在此测试文件中使用,将始终为 None

2. [逻辑漏洞] 测试覆盖有效性
注释说明"importing it twice can initialise the real monitoring stack",但应确认这是否意味着该测试文件中的测试用例全部跳过了实际功能验证,降低了测试价值。

@YehongPan YehongPan 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.

Code Review

  • [代码规范] 删除了 spec.loader.exec_module 的 try/except 块但没有补充替代的模块初始化逻辑,后续如果 OpenAIModel 需要在此测试文件中使用,将始终为 None
  • [逻辑漏洞] 注释说明"importing it twice can initialise the real monitoring stack",应确认这是否意味着该测试文件中的测试用例全部跳过了实际功能验证,降低了测试价值。

Comment thread test/sdk/core/models/test_openai_llm.py
@WMC001 WMC001 merged commit f95e6d1 into develop Jun 24, 2026
6 checks passed
@WMC001

WMC001 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Bug: test passes removed parameter

In test/backend/utils/test_context_utils.py, test_kb_not_duplicated_in_tools calls build_tools_component with knowledge_base_summary='KB text', but this parameter was removed from _format_tools_description in this PR. The test will fail at runtime with TypeError: unexpected keyword argument.

Fix: remove knowledge_base_summary from the test call, or drop the test entirely.

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.

4 participants