.2059806565729300:323b1b60faa062879e446db1ae76ec60_69e3499ab13256d09ece8088.69e3a8ffb13256d09ece82eb.69e3a8ff7afba9383018b938:Trae CN.T(2026/4/18 23:53:35)#1273
Conversation
重构集成测试脚本，增加健康检查轮询机制替代固定等待时间 添加详细的测试帮助文档和环境变量配置说明 改进日志记录和错误检测功能
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if path and os.path.exists(path): | ||
| try: | ||
| os.remove(path) | ||
| except Exception: |
Greptile SummaryThis PR refactors
Confidence Score: 4/5Not safe to merge as-is — One P1 finding: the Makefile change omits Makefile line 193 — missing Important Files Changed
Sequence DiagramsequenceDiagram
participant Make as make test-integration
participant Pytest as pytest
participant SF as server_process fixture
participant SC as ServerConfig
participant SP as ServerProcess
participant AW as aw-server binary
Make->>Pytest: pytest scripts/tests/integration_tests.py -v
Pytest->>SF: setup fixture (scope=session)
SF->>SC: ServerConfig() (reads env vars)
SF->>SP: ServerProcess(config)
SP->>AW: subprocess.Popen([bin, --testing, --port, 5666])
loop Poll every poll_interval until timeout
SP->>AW: GET /api/0/info
AW-->>SP: 200 OK (ready) or error
end
SP-->>SF: server ready
SF-->>Pytest: yield server
Pytest->>SP: test_server_starts → is_alive()
Pytest->>SP: test_api_info_endpoint → get_api_info()
SP->>AW: GET /api/0/info
AW-->>SP: {version, hostname, ...}
Pytest->>SP: test_api_buckets_endpoint → get_api_buckets()
SP->>AW: GET /api/0/buckets
AW-->>SP: {} or []
Pytest->>SP: test_no_error_indicators_in_logs → check_for_errors()
SP->>SP: scan stdout/stderr log files
SF->>SP: cleanup() — kill process, remove temp logs
Reviews (1): Last reviewed commit: "test: 重构集成测试并添加详细文档" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @echo "" | ||
| @echo "For help: make test-integration-help" | ||
| @echo "" | ||
| @pytest scripts/tests/integration_tests.py -v |
There was a problem hiding this comment.
aw-server/tests/ silently dropped from integration run
The original command passed ./aw-server/tests/ as a second pytest argument, so aw-server's own test suite ran as part of the integration target. The new command omits it entirely, meaning those tests are no longer executed when make test-integration is called.
| @pytest scripts/tests/integration_tests.py -v | |
| @pytest scripts/tests/integration_tests.py aw-server/tests/ -v |
| if "--port" not in " ".join(self.args) and "-p" not in " ".join(self.args): | ||
| self.args.extend(["--port", str(self.port)]) |
There was a problem hiding this comment.
-p flag
"-p" not in " ".join(self.args) performs a plain substring search on the joined argument string. Any arg that happens to contain the two-character sequence -p (e.g. --split-page, --type=temp) would incorrectly suppress appending --port, silently using whatever default port the server picks rather than the configured one.
A safer approach is to check the args list directly:
if not any(a in ("--port", "-p") for a in self.args):
self.args.extend(["--port", str(self.port)])| def get_api_buckets(self) -> Dict[str, Any]: | ||
| if not HAS_URLLIB: | ||
| pytest.skip("urllib not available") | ||
|
|
||
| status, data = self._make_request("buckets") | ||
| if status != 200: | ||
| self._print_log_summary(f"API /buckets request failed (status: {status})") | ||
| pytest.fail(f"GET /api/0/buckets failed with status {status}") | ||
|
|
||
| return data |
There was a problem hiding this comment.
get_api_buckets is annotated as -> Dict[str, Any], but test_api_buckets_endpoint explicitly checks isinstance(buckets, list) — meaning the API can return a JSON array. The mismatched annotation can mislead callers and type checkers.
| def get_api_buckets(self) -> Dict[str, Any]: | |
| if not HAS_URLLIB: | |
| pytest.skip("urllib not available") | |
| status, data = self._make_request("buckets") | |
| if status != 200: | |
| self._print_log_summary(f"API /buckets request failed (status: {status})") | |
| pytest.fail(f"GET /api/0/buckets failed with status {status}") | |
| return data | |
| def get_api_buckets(self) -> Any: |
| self.process.wait(timeout=5) | ||
| self.process.communicate(timeout=5) |
There was a problem hiding this comment.
communicate() after wait()
process.wait() blocks until the process exits, so calling process.communicate() afterwards is a no-op — it will immediately return (b'', b'') because the process has already terminated and stdout/stderr are redirected to files (not PIPE). The redundant call can be removed to simplify cleanup.
| self.process.wait(timeout=5) | |
| self.process.communicate(timeout=5) | |
| self.process.wait(timeout=5) | |
| print(f"[INFO] Server stopped with code: {self.process.returncode}") |
3 participants
重构集成测试脚本，增加健康检查轮询机制替代固定等待时间
添加详细的测试帮助文档和环境变量配置说明
改进日志记录和错误检测功能