fix(test): widen tool-control E2E timeout for CI docker sandbox - #7614
Closed
qwen-code-dev-bot wants to merge 3 commits into
Closed
fix(test): widen tool-control E2E timeout for CI docker sandbox#7614qwen-code-dev-bot wants to merge 3 commits into
qwen-code-dev-bot wants to merge 3 commits into
Conversation
The coreTools test in sdk-typescript/tool-control.test.ts timed out at 60s in the docker sandbox E2E job (run 30010867761). The model response latency in docker can exceed 60s, causing the test to fail after 3 retries (69s per attempt). Widen TEST_TIMEOUT from a flat 60s to CI-aware 120s (CI) / 60s (local), matching the pattern used in permission-control.test.ts. Fixes #7605
Collaborator
|
Thanks for the fix! Template: mostly there — the "Reviewer Test Plan" and "Linked Issues" headings from the template are missing, and there's no Chinese translation section. For a 1-line test timeout fix the substance is all present, so not blocking on this, but worth noting for future bot PRs. Problem: observed CI failure with evidence — issue #7605 links to run 30010867761 where Direction: test infrastructure stability fix, squarely within scope. No concerns. Size: not applicable — the changed file is in Approach: minimal and correct. The Moving on to code review. 🔍 中文说明感谢修复！ 模板：基本完整——缺少模板中的 "Reviewer Test Plan" 和 "Linked Issues" 标题，也没有中文翻译部分。对于 1 行测试超时修复，实质内容都在，不因此阻塞，但后续 bot PR 值得注意。 问题：已观测到的 CI 失败，有证据——issue #7605 关联了 run 30010867761，其中 方向：测试基础设施稳定性修复，完全在范围内。无顾虑。 规模：不适用——改动文件在 方案：最小且正确。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Code ReviewIndependent proposal: given a 69s timeout against a 60s limit in docker sandbox CI, I'd make vs. the diff: the PR does exactly this. The approach matches perfectly — 1 line, same pattern, sensible values. No correctness bugs, no security issues, no regressions. No blockers. The only nit is the numeric separator style ( Real-Scenario TestingRan the previously-failing test ( Test run (this environment, The test passed in 32.29s here. The CI failure occurred in docker sandbox where model response latency pushed it to 69s — the 120s CI timeout provides adequate headroom for that environment. Note: this run uses the main-branch code (flat 60s timeout); the test passed because local latency is lower than docker sandbox. The PR's value is in the CI environment where the 60s limit was breached. 中文说明代码审查独立方案： 鉴于 docker sandbox CI 中 69s 超时超过 60s 限制，我会通过 与 diff 对比： PR 完全这样做了。方案完全匹配——1 行，相同模式，合理的值。无正确性问题、安全问题或回归。 无阻塞项。唯一的 nit 是数字分隔符风格（ 真实场景测试使用 测试在本环境中 32.29s 通过。CI 失败发生在 docker sandbox 中，模型响应延迟将其推到 69s——120s 的 CI 超时为该环境提供了充足余量。注意：本次运行使用 main 分支代码（固定 60s 超时）；测试通过是因为本地延迟低于 docker sandbox。PR 的价值在于 CI 环境，那里 60s 限制被突破。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is exactly what a CI flake fix should look like: one line, follows the established pattern, backed by a real observed failure (69s against a 60s limit in docker sandbox, run 30010867761). My independent proposal was identical to what the PR does. The test passes when run locally, and the 120s CI timeout gives comfortable headroom for docker sandbox latency without slowing down local development. Nothing to second-guess here. Ship it. 中文说明置信度：5/5 —— 每个阶段都干净；毫不犹豫可以合并。 这正是 CI flake 修复应有的样子：一行改动，遵循已有模式，有真实观测到的失败支撑（docker sandbox 中 69s 超过 60s 限制，run 30010867761）。我的独立方案与 PR 完全一致。测试在本地运行通过，120s 的 CI 超时为 docker sandbox 延迟提供了舒适余量，同时不影响本地开发速度。 无需犹豫。发布吧。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 23, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
…nt file content (#7614)
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[rc:3639421195] [Suggestion] — Assert on tool-call results, not model-dependent file contentDecision: Implemented. Feedback: The assertion Changes: Replaced all 4 occurrences of the model-dependent file-content assertion pattern in
Each was changed from: const content = await helper.readFile('test.txt');
expect(content).not.toBe('<original>');To: const writeResults = findToolResults(messages, 'write_file');
expect(writeResults.length).toBeGreaterThan(0);
for (const result of writeResults) {
expect(result.isError).toBe(false);
}This verifies that Conflict notes: None ( Verification
中文说明已处理的审查反馈[rc:3639421195] [建议] — 断言工具调用结果，而非依赖模型的文件内容决定： 已实施。 反馈内容： 断言 修改内容： 替换了
每处都从： const content = await helper.readFile('test.txt');
expect(content).not.toBe('<original>');改为： const writeResults = findToolResults(messages, 'write_file');
expect(writeResults.length).toBeGreaterThan(0);
for (const result of writeResults) {
expect(result.isError).toBe(false);
}这验证了 冲突说明： 无（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (integration-tests/ is outside npm workspaces).
— qwen3.7-max via Qwen Code /review
…l-success assertions (#7614)
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Review Feedback SummaryFeedback: Extract duplicated tool-success assertion into a helper [rc:3639772964]Classification: Optional (Suggestion from automated reviewer) Decision: Implemented — valuable, codebase-consistent, and in scope. Changes:
Conflict notes: No conflicts ( Verification
中文说明审查反馈总结反馈：将重复的工具成功断言提取为辅助函数 [rc:3639772964]分类： 可选（来自自动审查机器人的建议） 决定： 已实施 — 有价值、符合代码库惯例、且在范围内。 变更内容：
冲突说明： 无冲突（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Widens
TEST_TIMEOUTinintegration-tests/sdk-typescript/tool-control.test.tsfrom a flat 60s to CI-awareprocess.env['CI'] ? 120_000 : 60_000.Why it's needed
The
coreTools parameter > should only allow specified tools when coreTools is settest timed out at 60s in the E2E docker sandbox job (run 30010867761, 69s per attempt × 3 retries). Model response latency in docker can exceed 60s.This matches the pattern already used in
permission-control.test.ts(line 37).Changes
1 file, 1 line:
const TEST_TIMEOUT = 60000→const TEST_TIMEOUT = process.env['CI'] ? 120_000 : 60_000Risk & Scope
Fixes #7605