test(ci): qualify lease inspection on public Node - #4857
huangruiteng merged 1 commit into
Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
|
Exact-head status for |
|
Independent review of exact head Verdict: APPROVE. Root-cause fix done right: the skip condition reuses the production qualification probe ( Direct effect verified at this head: node-minimum-compatibility passes with Reviewer-chain note (transparency): the first reviewer pass at this head timed out at 900s and emitted only a bare verdict line; it was discarded and a full re-run completed — PASS with minor editorial suggestions (line-number off-by-one in the review prose, immaterial to the verdict). |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这个测试修复解决的是一条真实的 CI 信号问题:公开最低 Node 运行时没有满足 SQLite authority 的 WAL-reset 资格时,生产路径会 fail closed,但原测试仍强行执行 SQLite provider,于是把“不受支持”误报成产品回归。目标应当是只跳过不具备资格的 SQLite 行,而不是弱化整个 mixed-provider 测试。
改动思路
实现复用生产侧已有的 sqliteRuntimeIdentity().sqlite_authority_qualified,没有再维护 Node/SQLite 版本表,也没有复制第二份资格规则。skip 条件同时绑定 provider === "sqlite",所以 legacy/file 行继续运行;合格运行时上的 SQLite 行也继续运行。该模式与 goal_acceptance_authority.test.ts 的既有做法一致。
具体改动
关键内容讲解
- 从既有 runtime qualification owner 导入
sqliteRuntimeIdentity,在测试注册前计算一次sqliteQualified。 - 将原先只处理 PostgreSQL 环境缺失的 skip 条件扩展为两个独立条件:无 PostgreSQL URL 时跳过 PostgreSQL;SQLite 未资格化时只跳过 SQLite。
- 不触碰
inspectTaskLease、authority store 或生产 admission 逻辑;4 additions / 1 deletion 全在单个测试文件内。
对主干的风险
最强反例是“为了让最低版本变绿而把合格运行时或其它 provider 的覆盖一起跳掉”。本地 Node 24.21.0 精确 head 验证实际执行了 SQLite:10 项通过,仅 2 个无连接的 PostgreSQL 行跳过;typecheck:control-plane 和 git diff --check 通过。把当前 origin/main 以 no-commit 方式合入后,相同测试与 typecheck 也继续通过。远端 node-minimum-compatibility、node-forward-compatibility 和 real-server PostgreSQL job 均通过,说明资格化/非资格化与真实 PostgreSQL 边界都有覆盖。
当前 blocker 仍是最终 CI:精确 head 1acf90d895f42e50a1d352a4dda9fa5e9bb589b0 落后主干,test-shard (2)、pytest 与 merge-gate 失败。失败日志显示 shard 2 是 project-lifecycle fixture 的 SimpleNamespace.progress 缺失,不由本 PR 的测试文件触发;但 capability 要求 final CI,不能用“与本 diff 无关”代替当前 head 的绿色收据。最小修复是 rebase 当前 main 后重跑全部 required checks。
我的整体评价
这是一份正确、窄且可回滚的 test-only 修复:它复用现有资格事实,保留其它 provider 与合格 SQLite 的真实覆盖,没有引入生产语义或重复 smoke。代码层面没有额外 blocker;由于当前 exact head 没有成功的最终 CI,本轮结论仍为请求修改,待 rebase 后按新 head 复审。
English verdict: REQUEST_CHANGES - head 1acf90d implements the correct provider-specific qualification and passes focused, forward-runtime, PostgreSQL, and current-main integration checks, but it is behind main and lacks the required successful final CI; rebase and rerun all required checks.
Summary
Root cause
task_lease_inspection.test.tswas added to the public-minimum suite with File, SQLite, and PostgreSQL variants. Node 22.18.0 embeds SQLite 3.50.2, which the authority boundary correctly rejects because it lacks the required WAL-reset fix. The test did not apply the runtime-qualified SQLite skip already used by adjacent mixed-provider suites, sonode-minimum-compatibilityfailed independently of the PR under test. This was observed on #4855.Validation
npm run typecheck:control-planepassed on Node 22.18.0.loopx canary premerge --from-git-diff: 4/4 passed, 0 manual holds.git diff --checkpassed.This changes test qualification only; production authority admission remains fail-closed.