fix(cli): share hosted harness deadlines and bail after CI retries (#14752)

This commit is contained in:
烽宁 authored and GitHub committed 2026-10-30 23:54:26 +08:00
1 parent ad21b1f7a0
commit 4bd9ed0143
7 files changed
+558 -269

No files matched your search

@@ -0,0 +1,47 @@
# Hosted harness settle and CI failure budgets
[English](./hosted-harness-settle-budget.md) | [简体中文](./hosted-harness-settle-budget.zh-CN.md)
## Problem
The hosted harness tests give every settle wait a fresh timeout. On a loaded non-ECS host that timeout equals Vitest's default 15-second test ceiling, so the runner can time out before the wait reports its assertion. Sequential waits in helpers such as `parkToolTurn` can also exceed an explicit 30-second ceiling even though each individual wait appears bounded.
Real Vitest reproductions confirm the lost assertion and a separate aggregate failure problem: the CLI CI command continues through every failing test, with two retries per test. The per-test deadline alone cannot stop those repeated failures from exhausting the CI step's 110-minute cap.
The current suite collects 604 concrete tests. A source analysis identifies 265 tests with unconditional call chains to the affected settle assertion. A budget scenario that assigns only one failed wait per test costs 245 × 45 + 20 × 29 = 11,605 seconds before retries, or 34,815 seconds with two retries. These are configured-budget scenarios, not measured multi-hour runtimes or universal lower bounds; the issue's historical estimates are not reused. A short real-timer reproduction confirms that three permanent failures consume nine attempts and still allow the following file to run.
## Decision
Keep the existing per-wait slow-host allowance of 45 seconds and the ordinary allowance of 10 seconds. At the beginning of each test attempt, derive one monotonic deadline from Vitest's resolved `task.timeout`, reserving one second for reporting the assertion and finishing the test body. Every settle wait receives the smaller of its per-wait allowance and the remaining time before that deadline.
The deadline starts in the existing outer `beforeEach`, so fixture setup conservatively consumes the same budget. Nested helpers and the describe-local `waitFor` and recovery `idle` wrappers use that same deadline. Every new test and retry resets it. The deadline is local to this sequential test file.
Use `node:perf_hooks` rather than `Date.now`, because some tests change the wall clock. Once the deadline is exhausted, use a one-millisecond timeout so Vitest still evaluates the current assertion and reports its failure. When Vitest explicitly disables the test ceiling with zero, retain only the per-wait allowance.
## CI failure policy
Use Vitest's built-in `--bail=1` in the CLI's `test:ci` entry. Vitest first exhausts a test's configured retries. Only a final failure cancels pending tests, preserves the original failure in the reports, and exits nonzero. A failure recovered by retry does not stop the run.
This removes the number of queued failing tests from the aggregate settle-failure cost. In the reproduced single-worker scenario, three attempts of the first permanently failing test replace three attempts of every failing test. Concurrent work already in progress can finish; this is not a process-kill timer or a guarantee against unrelated stalls elsewhere in the workflow. The CI step cap remains a final backstop.
The tradeoff is fewer diagnostics after a permanent CI failure. The ordinary `test` entry remains exhaustive for local investigation. Both workspace CI and release commands call `test:ci`, so both use this policy for the CLI package. No other package's failure policy changes.
## Scope and implementation
A small `settle-timeout` test utility returns the remaining timeout from a captured deadline. The hosted harness suite initializes it from the actual test context and replaces fixed settle-budget reads with calls to it. Collocated tests cover sequential consumption, explicit ceilings, slow-host slack, exhaustion, wall-clock independence, and reset between attempts.
No product behavior, host-load detection, package-wide timeout, retry count, or CI step limit changes. Waits with separate existing fixed budgets remain outside this change. A subprocess regression test exercises the actual CLI script arguments and installed Vitest, checking permanent failures, recovery on the final retry, exhaustive local diagnostics, nonzero exit status, and failure reporting. Attempt markers verify that queued tests did not execute; Vitest 3's aggregate skipped counters do not include tests cancelled by bail.
## Verification
- A permanently failing settle assertion on a slow non-ECS host must be reported before the 15-second runner ceiling.
- Sequential helper waits must share the remaining budget of an explicitly 30-second test rather than each receiving 45 seconds.
- A 60-second ECS test must retain the full 45-second first-wait allowance when enough time remains; an ordinary host retains its 10-second allowance.
- All existing hosted harness tests must pass on the host and Docker Linux with unchanged assertions and outer timeouts.
- A permanent failure with two retries must run three attempts, stop queued tests, exit nonzero, and preserve its assertion in JUnit. A test recovered by its final retry must allow following tests to run.
- The ordinary local test command must continue through all failures for diagnosis.
- Build, typecheck, focused regression tests, lint, and formatting must pass.
## Open questions
None for the reported settle-budget and cumulative-failure mechanisms. Replacing host-load heuristics, diagnosing unrelated CI stalls, and changing other packages' failure policies remain outside this change.
@@ -0,0 +1,47 @@
# Hosted harness 收敛等待与 CI 失败预算
[English](./hosted-harness-settle-budget.md) | [简体中文](./hosted-harness-settle-budget.zh-CN.md)
## 问题
Hosted harness 测试目前为每次收敛等待分配一份完整的新超时预算。在满载的非 ECS 主机上,该预算等于 Vitest 默认的 15 秒测试上限,因此 runner 可能先超时,来不及呈现等待中的断言。`parkToolTurn` 等辅助函数中的连续等待,也可能超过显式的 30 秒测试上限,即使每个等待看起来都有上限。
真实 Vitest 复现确认了断言丢失,以及独立的累计失败问题:CLI 的 CI 命令会继续运行每条失败测试,每条测试还会重试两次。仅靠单条测试的截止时间,无法阻止这些重复失败耗尽 CI 步骤的 110 分钟上限。
当前测试集收集到 604 条展开后的测试。源码分析识别出 265 条通过无条件调用链到达目标收敛断言的测试。假设每条测试只消耗一次失败等待,预算场景为 245 × 45 + 20 × 29 = 11,605 秒,包含两次重试后为 34,815 秒。这是配置预算场景,不是多小时实测或普适下界;未沿用 issue 中的历史估算。短时真实计时复现确认,三个永久失败会消耗九次尝试,之后仍执行后续文件。
## 决策
保留现有慢主机每次等待最多 45 秒、普通主机最多 10 秒的余量。在每次测试尝试开始时,根据 Vitest 已解析的 `task.timeout` 创建一个单调时钟截止时间,预留一秒用于报告断言和完成测试体。每次收敛等待取其单次上限与该截止时间前剩余时间的较小值。
截止时间在现有最外层 `beforeEach` 中开始计算,因此测试准备工作也保守地消耗同一预算。嵌套辅助函数、describe 内的 `waitFor` 和恢复流程的 `idle` 包装函数共用该截止时间。每条新测试及重试都会重新初始化。截止时间仅属于这个顺序执行的测试文件。
使用 `node:perf_hooks`,不使用 `Date.now`,因为部分测试会修改墙上时钟。截止时间耗尽后使用一毫秒超时,让 Vitest 仍能执行当前断言并报告其失败。Vitest 显式以零禁用测试时间上限时,仅保留单次等待上限。
## CI 失败策略
在 CLI 的 `test:ci` 入口使用 Vitest 内建的 `--bail=1`。Vitest 会先用完该测试配置的重试机会,只有最终失败才取消排队中的测试,将原始失败保留在报告中,并以非零状态退出。重试恢复成功不会停止运行。
这消除了排队中的失败测试数量对累计收敛失败成本的乘数效应。在复现的单 worker 场景中,运行首条永久失败测试的三次尝试后即停止,无需让每条失败测试都尝试三次。已经运行的并发任务可以完成;这不是强制杀进程的计时器,也不保证工作流其他位置不会发生无关阻塞。CI 步骤上限继续作为最终兜底。
代价是在 CI 出现永久失败后收集到的诊断更少。本地普通 `test` 入口继续完整运行,方便排查。Workspace CI 和发布命令都调用 `test:ci`,因此二者都对 CLI 包使用该策略。其他包的失败策略保持不变。
## 范围与实现
小型 `settle-timeout` 测试工具根据捕获的截止时间返回剩余超时。Hosted harness 测试集通过真实测试上下文初始化它,并将固定收敛预算的读取改为调用该工具。同目录测试覆盖连续等待消耗预算、显式上限、慢主机余量、预算耗尽、不受墙上时钟影响及测试尝试之间的重置。
不改变产品行为、主机负载检测、包级测试上限、重试次数或 CI 步骤时限。原本使用独立固定预算的其他等待不在本次范围内。子进程回归测试使用实际 CLI 脚本参数和已安装的 Vitest,验证永久失败、最后一次重试恢复、本地完整诊断、非零退出状态及失败报告。通过尝试标记验证排队中的测试没有执行;Vitest 3 的汇总跳过计数不包含被 bail 取消的测试。
## 验证
- 在满载非 ECS 主机上,持续失败的收敛断言必须在 15 秒 runner 上限之前被报告。
- 对显式上限为 30 秒的测试,辅助函数的连续等待必须共享剩余预算,不能每次各获得 45 秒。
- 对上限为 60 秒的 ECS 测试,时间充足时首次等待必须保留完整的 45 秒余量;普通主机保留 10 秒余量。
- 本机和 Docker Linux 中所有现有 hosted harness 测试均通过,不改变断言或外层超时。
- 永久失败在配置两次重试时必须运行三次尝试,停止排队中的测试,以非零状态退出,并在 JUnit 中保留具体断言。最后一次重试恢复成功时,必须继续运行后续测试。
- 本地普通测试命令必须继续运行所有失败测试以供诊断。
- 构建、类型检查、针对性回归测试、lint 和格式检查均通过。
## 待决问题
对于报告中的收敛预算及累计失败机制,没有待决问题。替换主机负载启发式规则、排查无关 CI 阻塞,以及改变其他包的失败策略,均不在本次范围内。
+1 -1
View File
@@ -30,7 +30,7 @@
"lint": "eslint . --ext .ts,.tsx",
"format": "prettier --write .",
"test": "vitest run",
"test:ci": "vitest run",
"test:ci": "vitest run --bail=1",
"typecheck": "tsc --noEmit",
"check-i18n": "tsx ../../scripts/check-i18n.ts"
},
File diff suppressed because it is too large. Load diff
@@ -0,0 +1,76 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import { performance } from 'node:perf_hooks';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { createSettleTimeout } from './settle-timeout.js';
beforeEach(() => {
vi.spyOn(performance, 'now').mockReturnValue(1_000);
});
afterEach(() => {
vi.restoreAllMocks();
});
describe('createSettleTimeout', () => {
it.each([
['ordinary host', 15_000, 10_000, 10_000],
['loaded non-ECS host', 15_000, 45_000, 14_000],
['ECS host', 60_000, 45_000, 45_000],
['explicit test ceiling', 30_000, 45_000, 29_000],
])('leaves reporting headroom on an %s', (_, testMs, waitMs, expected) => {
expect(createSettleTimeout(testMs, waitMs)()).toBe(expected);
});
it('shares the deadline across setup and sequential helper waits', () => {
const remaining = createSettleTimeout(30_000, 45_000);
vi.mocked(performance.now).mockReturnValue(3_000);
expect(remaining()).toBe(27_000);
vi.mocked(performance.now).mockReturnValue(23_000);
expect(remaining()).toBe(7_000);
vi.mocked(performance.now).mockReturnValue(29_000);
expect(remaining()).toBe(1_000);
});
it('retains the per-wait cap while time remains in the test', () => {
const remaining = createSettleTimeout(60_000, 45_000);
vi.mocked(performance.now).mockReturnValue(6_000);
expect(remaining()).toBe(45_000);
vi.mocked(performance.now).mockReturnValue(21_000);
expect(remaining()).toBe(39_000);
});
it('gives the current assertion one attempt after the budget is exhausted', () => {
const remaining = createSettleTimeout(15_000, 45_000);
vi.mocked(performance.now).mockReturnValue(15_000);
expect(remaining()).toBe(1);
vi.mocked(performance.now).mockReturnValue(20_000);
expect(remaining()).toBe(1);
});
it('starts a fresh deadline for the next test or retry', () => {
const previous = createSettleTimeout(15_000, 45_000);
vi.mocked(performance.now).mockReturnValue(20_000);
const next = createSettleTimeout(15_000, 45_000);
expect(previous()).toBe(1);
expect(next()).toBe(14_000);
});
it('does not consume the deadline when a test changes the wall clock', () => {
const remaining = createSettleTimeout(15_000, 45_000);
vi.spyOn(Date, 'now').mockReturnValue(Date.now() + 3_600_000);
expect(remaining()).toBe(14_000);
});
it('retains the per-wait cap when the test timeout is disabled', () => {
const remaining = createSettleTimeout(0, 45_000);
vi.mocked(performance.now).mockReturnValue(1_000_000);
expect(remaining()).toBe(45_000);
});
});
@@ -0,0 +1,22 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import { performance } from 'node:perf_hooks';
export function createSettleTimeout(
testTimeoutMs: number,
waitTimeoutMs: number,
): () => number {
// Sequential waits share the test deadline, leaving time to report their
// assertion instead of being interrupted by Vitest's runner timeout.
const deadline =
testTimeoutMs === 0
? Number.POSITIVE_INFINITY
: performance.now() + testTimeoutMs - 1_000;
return () =>
Math.max(1, Math.min(waitTimeoutMs, deadline - performance.now()));
}
+107
View File
@@ -0,0 +1,107 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import { spawnSync } from 'node:child_process';
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { describe, expect, it } from 'vitest';
const root = fileURLToPath(new URL('../../', import.meta.url));
const cli = path.join(root, 'packages/cli');
const { scripts } = JSON.parse(
readFileSync(path.join(cli, 'package.json'), 'utf8'),
);
describe('CLI CI failure budget', () => {
it.each([
['test:ci', true, 1, 2, 3],
['test:ci', false, 0, 0, 9],
['test', true, 3, 0, 9],
])(
'%s with permanent failure=%s reports %i failures and leaves %i tests unrun after %i attempts',
(script, permanent, failures, unrun, attempts) => {
const fixture = mkdtempSync(path.join(tmpdir(), 'qwen-cli-ci-'));
try {
const evidence = path.join(fixture, 'attempts.txt');
const jsonReport = path.join(fixture, 'results.json');
const junitReport = path.join(fixture, 'junit.xml');
const config = path.join(fixture, 'vitest.config.mjs');
writeFileSync(
config,
`export default ${JSON.stringify({
test: {
globals: true,
include: ['fixture.test.js'],
maxWorkers: 1,
minWorkers: 1,
reporters: ['json', 'junit'],
outputFile: { json: jsonReport, junit: junitReport },
},
})};`,
);
writeFileSync(
path.join(fixture, 'fixture.test.js'),
`import { appendFileSync } from 'node:fs';
for (const name of ['first settle', 'second settle', 'third settle']) {
let attempts = 0;
it(name, async () => {
attempts++;
appendFileSync(${JSON.stringify(evidence)}, name + '\\n');
await vi.waitFor(() => {
expect(${permanent} || attempts < 3, 'hasActivePrompt never cleared').toBe(false);
}, { timeout: 20, interval: 5 });
});
}`,
);
const [command, ...args] = scripts[script].split(/\s+/);
expect(command).toBe('vitest');
const result = spawnSync(
process.execPath,
[
path.join(root, 'node_modules/vitest/vitest.mjs'),
...args,
'--root',
fixture,
'--config',
config,
'--retry=2',
],
{
cwd: cli,
encoding: 'utf8',
timeout: 60_000,
},
);
expect(result.error).toBeUndefined();
expect(result.status, result.stdout + result.stderr).toBe(
failures ? 1 : 0,
);
const report = JSON.parse(readFileSync(jsonReport, 'utf8'));
expect(report.numFailedTests).toBe(failures);
expect(report.numPassedTests).toBe(3 - failures - unrun);
const assertions = report.testResults[0].assertionResults;
expect(assertions.map(({ status }) => status)).toEqual(
unrun
? ['failed', 'pending', 'pending']
: Array(3).fill(failures ? 'failed' : 'passed'),
);
const started = readFileSync(evidence, 'utf8').trim().split('\n');
expect(started).toHaveLength(attempts);
expect(started.filter((name) => name === 'first settle')).toHaveLength(
3,
);
const junit = readFileSync(junitReport, 'utf8');
expect(junit).toContain(`failures="${failures}"`);
if (failures) expect(junit).toContain('hasActivePrompt never cleared');
} finally {
rmSync(fixture, { recursive: true, force: true });
}
},
);
});