test(coordination): characterize file-backed claim_work with a parity fixture - #4422
Conversation
dc31bed to
3e1e7cb
Compare
… fixture tests/control_plane/test_coordination_executor.py proves the authority semantics against an in-memory provider and test_coordination_file_provider.py proves the storage verbs, but nothing drove the shipped provider through the real executor. This adds that seam as characterization only. - new tests/control_plane/test_coordination_provider_parity.py - one scenario matrix drives CoordinationAuthorityExecutor through FileCoordinationProvider: same-target competition, independent targets rebased internally, A -> B -> replay A across a reconstructed executor and a fresh provider handle, operation-identity reuse, and a stale provider generation - expectations are declared from the coordination contract (RFC section 10, checks 1-5) before anything runs; they are never read back from provider output and no authority rule is re-derived - the fixture is provider-neutral: scenarios speak only the storage protocol and receive providers from a handle factory, so another provider can be registered without touching the matrix - providers are synthetic and public-safe; default provider selection and public behavior are unchanged, and no production code is modified Fixes loopx-project#3700 Signed-off-by: Yue <108062406+Yue021130@users.noreply.github.com>
3e1e7cb to
c7a772f
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
动机
file-backed 的 claim 路径此前没有一份测试同时经过"权威执行层"和"真实存储 provider":tests/control_plane/test_coordination_executor.py(541 行)只用进程内 FakeProvider 验证规则,tests/control_plane/test_coordination_file_provider.py(491 行)只验证存储动词(identity、CAS、崩溃/fsync 行为)。两者之间的缝——真实文件 provider 上的重载、跨 handle 的 replay、CAS 期望值是否真的到达 shipped provider——此前没有任何用例覆盖,而线上走的就是这一对组合。本 PR 用一份 provider-neutral 的场景矩阵把这条缝补上。
改动思路
新增 tests/control_plane/test_coordination_provider_parity.py:一个 handle 工厂把每个场景放到独立的 tmp_path 目录上,用真实 FileCoordinationProvider + 真实 CoordinationAuthorityExecutor 跑 5 个场景(同目标竞争、独立目标 rebase、交错写后的 replay、operation identity 复用、stale generation 拒绝),期望值在运行前按 RFC 第 10 节写成字面量,再从观察结果里比对 result/reason/authority_revision/receipt_index/claimed_by/todo_revision 与显式 flag。RecordingProvider 只做三件事:代理存储动词、记录被要求使用的 CAS 期望值;它不携带任何权威语义。测试文件只做 characterization,不碰任何生产代码。
具体改动
tests/control_plane/test_coordination_provider_parity.py(+361,唯一改动文件):新增eligibility()/todo()/claim()/bootstrap()/executor_for()夹具、RecordingProvider、Observation/Expectation两个 dataclass、5 个场景函数与EXPECTED表,以及一个参数化的比对测试。
关键代码讲解
bootstrap(provider)(:100):head 的 store binding 取自provider.store_identity(),因此种子写必须通过 shipped provider 的 CAS 才成立——这一点正是 fake provider 测不到的。RecordingProvider(:132):只暴露store_identity/load/compare_and_put三个动词并记录cas_expectations,让stale_generation_does_not_duplicate能证明"陈旧的期望值确实被送进 CAS,且被拒绝后没有落盘"。EXPECTED+ 参数化测试(:300 起):每条期望都是协调域术语(result/reason/authority_revision/receipt id/claimed_by/todo_revision/flag),且是运行前写死的字面量,而不是从 provider 输出反推。
对主干的风险
结论:无阻塞项,可以进合并队列。我在 head c7a772f6a91141b89fb0f2a81a86f6eb642d3ac7 上做的验证:
pytest tests/control_plane/test_coordination_provider_parity.py -q→ 5 passed。- 反事实(是否只是"照着实现写期望"):把同一文件拷进 origin/main
a0b5bcbba的 worktree 运行 → 5 passed,说明它描述的是已经在 shipped 的行为,而不是它自己造的新契约。 - 变异敏感性:用包装器把执行层
apply的conflict改成applied、把rejected改成applied,对应场景分别失败——即这份 fixture 抓得住它声称要钉住的两种错误转换,而不是一张快照。 - 相邻协调套件
pytest tests/control_plane -q -k coordination→ 288 passed, 3 failed,3 个失败全是本机 Node 25.5.0 / SQLite 未资格化的[sqlite]canonical authority fixture 报错,在 base 上同样失败,属环境。
次要项(P3,非阻塞):① 文件 docstring 写"checks 1-5",但场景表实际覆盖 check 1/2/3/5 加一个 stale-generation 场景,check 4 缺失,建议把 docstring 改成显式列举或补上该场景;② 该 head 在远端没有任何 check run(check-runs total_count=0,commit status 为 pending),因此"没有红灯"并不等于跑过,本轮的结论完全建立在本地证据上,建议 push 后重跑 workflow 再走合并。
我的整体评价
APPROVE。这是一份合格的 characterization fixture:复用 shipped provider 与真实执行器、不新增第三个假 provider、期望值在运行前按契约声明、场景参数化后失败能直接指出是哪条契约破了。最关键的两点我都独立验证过——把文件拿到 main 上同样 5 passed(说明没有描述未落地的行为),以及注入两种错误转换后 fixture 会失败(说明它对契约违例敏感)。规模合适:361 行、单文件、低于 500 行 smoke 阈值,且比继续膨胀两个已接近 500 行的既有测试更可取。剩余两条都是 P3:docstring 的覆盖声明需要与场景表对齐,以及本地证据不能替代远端 CI。
English verdict: APPROVE — reviewed exact head c7a772f. The new matrix drives the shipped CoordinationAuthorityExecutor through the shipped FileCoordinationProvider, which no existing test did (the executor test uses an in-memory FakeProvider; the file-provider test never constructs an executor). Verification: 5 passed at the head; the same file copied into an origin/main worktree also passes 5, proving it characterizes shipped behaviour rather than a hoped-for contract; and mutation probes that turn a conflict (and a rejected operation-identity reuse) into "applied" each make the corresponding scenario fail, so the fixture is sensitive to the transitions it claims to pin. The adjacent coordination suite is 288 passed with 3 inherited local [sqlite]/Node-25 environment failures. Two P3s: the docstring claims RFC checks 1-5 while the matrix covers 1, 2, 3, 5 plus a stale-generation scenario, and this head has no remote check runs at all, so the local evidence above is the only executed evidence.
Summary / 概要
Fixes #3700.
test_coordination_executor.pyproves the authority semantics against an in-memory provider andtest_coordination_file_provider.pyproves the storage verbs, but nothing drove the shipped provider through the real executor. This adds exactly that seam — characterization only, no production code change.New file:
tests/control_plane/test_coordination_provider_parity.pyCoordinationAuthorityExecutorthroughFileCoordinationProvider, covering the five cases named in the issue:A -> B -> replay Areturns the exact original receipt, replayed through a reconstructed executor and a fresh provider handle;result/reason, finalauthority_revision, the receipt operations, per-todoclaimed_byandtodo_revision. They are never read back from provider output, and no authority rule is re-derived in the fixture — the executor remains the only decision maker under test.store_identity/load/compare_and_put) and receives providers from a handle factory. Registering another provider later needs no change to the matrix or the expectations.Validation / 验证
pytest tests/control_plane/test_coordination_provider_parity.py -q→ 5 passed.test_coordination_executor.py,test_coordination_file_provider.py,test_coordination_provider_parity.py,test_coordination_head.py,test_coordination_recoverable_execution.py→ 141 passed.file_providerto accept a staleexpected_provider_generation→stale_generation_does_not_duplicatefails;entry = None) →replay_after_interleaved_writeandoperation_identity_reusefail.Both mutations restored; the tree is clean apart from the new file.
ruff checkclean;python -m py_compileclean;git diff --checkclean.loopx canary premerge --from-git-diff: 4 direct checks passed,manual_holds: 0, public-boundary scan clean,self_merge_allowed: true. No catalog canaries were selected because the change adds a test file only (surfaces: python).Scope / 范围
Test-only. No production code change, no new provider, no NoKV / credential / service-startup / provider-promotion surface, no default-provider switch, and no change to scoring, scheduling, or public contracts.
Note / 说明
I also have PR #4421 open (issue #4413, likewise test-only). If the one-active-implementation-PR convention should apply here, I am happy to hold this one until #4421 is resolved — just say so and I will convert it to a draft.
Also happy to take the design-feedback route instead: if you would rather see the full matrix in an issue comment before any PR, I can close this and paste the scenario table into #3700.