Skip to content

fix(oc2ov-test): add Context Engine test case and main branch add cron#1586

Merged
qin-ctx merged 1 commit intovolcengine:mainfrom
kaisongli:fix/oc2ov-p0-test-improvements
Apr 20, 2026
Merged

fix(oc2ov-test): add Context Engine test case and main branch add cron#1586
qin-ctx merged 1 commit intovolcengine:mainfrom
kaisongli:fix/oc2ov-p0-test-improvements

Conversation

@kaisongli
Copy link
Copy Markdown
Collaborator

主要变更:

  • base_cli_test: smart_wait_for_sync/wait_for_sync 新增 session_id 参数,修复独立session检查错误session的Bug
  • base_cli_test: 新增 send_and_retry_on_timeout 方法,处理LLM超时和空响应自动重试
  • assertions: extract_response_text 拼接所有payloads而非只取第一个;空payloads时返回空字符串而非str(response)
  • test_context_engine: 新增6个Context Engine核心交互测试(assemble/compact/recall/expand/isolation)
  • test_memory_crud: TestMemoryRead增加确认步骤和关键词容错;使用独立session_id
  • conftest: 更新测试报告描述,覆盖Context Engine交互场景
  • run_tests: 更新测试路径配置

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 85
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use context managers for HTTP requests

Wrap all requests calls in context managers to ensure proper HTTP connection
cleanup. This avoids leaving dangling connections open and improves resource
management, especially in test environments with many API calls.

tests/oc2ov_test/tests/p0/test_context_engine.py [50-76]

 def list_session_ids(self) -> Set[str]:
     try:
-        resp = requests.get(f"{self.server_url}/api/v1/sessions", headers=self.headers, timeout=10)
-        if resp.status_code != 200:
-            return set()
-        sessions = resp.json().get("result", [])
-        return {s["session_id"] for s in sessions}
+        with requests.get(f"{self.server_url}/api/v1/sessions", headers=self.headers, timeout=10) as resp:
+            if resp.status_code != 200:
+                return set()
+            sessions = resp.json().get("result", [])
+            return {s["session_id"] for s in sessions}
     except Exception:
         return set()
 
 def get_session_detail(self, session_id: str) -> Optional[Dict]:
     try:
-        resp = requests.get(
+        with requests.get(
             f"{self.server_url}/api/v1/sessions/{session_id}",
             headers=self.headers,
             timeout=10,
-        )
-        if resp.status_code == 200:
-            return resp.json().get("result", {})
+        ) as resp:
+            if resp.status_code == 200:
+                return resp.json().get("result", {})
     except Exception:
         pass
     return None
Suggestion importance[1-10]: 5

__

Why: Using context managers for HTTP requests is a good practice to ensure proper connection cleanup, improving resource management in tests.

Low
Wrap remaining requests in context managers

Wrap the remaining requests.post and requests.get calls in OVSessionVerifier with
context managers for consistent resource cleanup. Apply this to commit_session and
poll_task_until_done methods.

tests/oc2ov_test/tests/p0/test_context_engine.py [78-108]

 def commit_session(self, session_id: str) -> Optional[str]:
     try:
-        resp = requests.post(
+        with requests.post(
             f"{self.server_url}/api/v1/sessions/{session_id}/commit",
             headers=self.headers,
             timeout=30,
-        )
-        if resp.status_code == 200:
-            return resp.json().get("result", {}).get("task_id")
+        ) as resp:
+            if resp.status_code == 200:
+                return resp.json().get("result", {}).get("task_id")
     except Exception:
         pass
     return None
 
 def poll_task_until_done(self, task_id: str, max_wait: int = TASK_POLL_MAX_WAIT) -> Optional[Dict]:
     start = time.time()
     while time.time() - start < max_wait:
         try:
-            resp = requests.get(
+            with requests.get(
                 f"{self.server_url}/api/v1/tasks/{task_id}",
                 headers=self.headers,
                 timeout=10,
-            )
-            if resp.status_code == 200:
-                task_data = resp.json().get("result", {})
-                status = task_data.get("status", "unknown")
-                if status in ("completed", "failed"):
-                    return task_data
+            ) as resp:
+                if resp.status_code == 200:
+                    task_data = resp.json().get("result", {})
+                    status = task_data.get("status", "unknown")
+                    if status in ("completed", "failed"):
+                        return task_data
         except Exception:
             pass
         time.sleep(TASK_POLL_INTERVAL)
     return None
Suggestion importance[1-10]: 5

__

Why: Consistent use of context managers for all requests in OVSessionVerifier ensures uniform resource cleanup, aligning with best practices.

Low

@kaisongli kaisongli force-pushed the fix/oc2ov-p0-test-improvements branch from de10ff1 to c242e54 Compare April 20, 2026 02:42
主要变更:
- base_cli_test: smart_wait_for_sync/wait_for_sync 新增 session_id 参数,修复独立session检查错误session的Bug
- base_cli_test: 新增 send_and_retry_on_timeout 方法,处理LLM超时和空响应自动重试
- assertions: extract_response_text 拼接所有payloads而非只取第一个;空payloads时返回空字符串而非str(response)
- test_context_engine: 新增6个Context Engine核心交互测试(assemble/compact/recall/expand/isolation)
- test_memory_crud: TestMemoryRead增加确认步骤和关键词容错;使用独立session_id
- conftest: 更新测试报告描述,覆盖Context Engine交互场景
- run_tests: 更新测试路径配置
@kaisongli kaisongli force-pushed the fix/oc2ov-p0-test-improvements branch from c242e54 to 5112269 Compare April 20, 2026 02:53
@qin-ctx qin-ctx merged commit fe19ba5 into volcengine:main Apr 20, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants