Skip to content

Commit 6bc9d2f

Browse files
committed
test(cli): make the doctor ledger e2e block fail legibly when cloud-connection is unbuilt (#5612)
`doctor-ledger-read-failure.test.ts` 的 e2e 整块通过 doctor 的动态 `import('@objectstack/cloud-connection')` 观察 ledger 报告面,而那个 catch 是**故意静默**的(`os doctor` 必须能在从未装过该可选包的 checkout 里跑完)。 后果:worktree 里 `packages/cloud-connection/dist` 缺失时,doctor 照常跑完每 一项、照打 `✓ Unique scope`,7 条用例以 7 份断言 diff 变红,读起来与 「#5412/#5413 的报告面被回退了」一模一样 —— #5612 正是据此立单的。 本提交不改产品码、不放宽任何断言,只加一层前置断言:两个依赖真实模块的 describe 在 `beforeAll` 里确认该包可加载、且 `list()` 返回 #5413 引入的 `{ entries, skipped }` 形状(陈旧 dist 会解析成 `skipped === undefined`, 在 doctor 里拐进第三种错误报告 —— AGENTS.md §9 的陈旧产物陷阱)。 第三个 describe 更需要这层保护:它模拟「包加载不了」并断言 doctor 保持静默, 在包真的加载不了的 worktree 里会**因为同一个原因**而变绿 —— PR #5046 记下的 空判定假绿。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh
1 parent 2614aef commit 6bc9d2f

1 file changed

Lines changed: 90 additions & 1 deletion

File tree

packages/cli/src/commands/doctor-ledger-read-failure.test.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
* and some files in it would not parse (#5413).
4747
*/
4848

49-
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
49+
import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest';
5050
import fs from 'node:fs';
5151
import os from 'node:os';
5252
import path from 'node:path';
@@ -75,6 +75,81 @@ const LEDGER_HEADLINE = 'Could not read the installed-package ledger';
7575
/** The head of its ENTRY-level sibling (#5413). Deliberately distinct text. */
7676
const SKIPPED_HEADLINE = 'installed-package ledger entr';
7777

78+
/**
79+
* ── Why this file needs a preflight (#5612) ──────────────────────────────
80+
*
81+
* Every end-to-end case below observes doctor's ledger half, and doctor reaches
82+
* that half through a dynamic `import('@objectstack/cloud-connection')` whose
83+
* `catch` is DELIBERATELY silent (`readInstalledPackageEntries()` in
84+
* `doctor.ts`): `os doctor` must run to completion in a checkout that never had
85+
* the optional package, so an unresolvable specifier means "nothing installed"
86+
* and prints nothing. That contract is correct and is itself pinned by the last
87+
* describe in this file.
88+
*
89+
* It also means this file cannot tell "the report face regressed" from "the
90+
* optional package is simply not built in this worktree" — both arrive as the
91+
* same total silence. In a worktree where `packages/cloud-connection/dist` is
92+
* missing, doctor runs every other check, prints `✓ Unique scope`, and the
93+
* seven cases that expect a ledger row fail with seven assertion diffs that
94+
* read exactly like #5412/#5413 having been reverted. #5612 was filed on
95+
* precisely that reading, after three unrelated causes had been eliminated:
96+
* the only variable was the unbuilt package.
97+
*
98+
* The sister file `test/platform-page-i18n-parity.test.ts` imports the same
99+
* package STATICALLY and therefore fails the honest way — one error that names
100+
* the package — which is the failure mode this preflight gives back to a file
101+
* that cannot use a static import (doctor's own load must stay dynamic, and
102+
* this file's last describe must be able to make it throw).
103+
*
104+
* This is a precondition, not a tolerance: nothing below is relaxed, no
105+
* assertion is weakened, and in a correctly built worktree the guard is a
106+
* no-op. It only replaces a misleading red with an accurate one.
107+
*/
108+
const PREFLIGHT_HINT = [
109+
'Preflight failed: the end-to-end ledger cases below cannot observe anything.',
110+
'',
111+
"`@objectstack/cloud-connection` is the package doctor reads the ledger through, and it is",
112+
'either not built or built from a source older than #5413 in this worktree. Doctor swallows',
113+
'that load failure on purpose, so without this guard the cases below would fail as seven',
114+
'assertion diffs that look like the #5412/#5413 report face regressed (#5612).',
115+
'',
116+
'Build the dependency graph first:',
117+
" pnpm --workspace-concurrency=2 --filter '@objectstack/cli^...' build",
118+
].join('\n');
119+
120+
/**
121+
* Assert that the real ledger reader is loadable AND speaks the post-#5413
122+
* contract doctor destructures without a fallback (`{ entries, skipped }`).
123+
*
124+
* The shape probe is not redundant with the load probe: a `dist/` built before
125+
* #5413 resolves fine and returns a bare array, which reaches doctor as
126+
* `skipped === undefined` and derails into the DIRECTORY-level failure row —
127+
* a third distinct wrong report, and the AGENTS.md §9 stale-artefact trap in
128+
* the exact place this file is least able to recognise it.
129+
*/
130+
async function assertLedgerReaderIsBuilt(): Promise<void> {
131+
let mod: Record<string, any>;
132+
try {
133+
mod = await import('@objectstack/cloud-connection');
134+
} catch (err) {
135+
throw new Error(`${PREFLIGHT_HINT}\n\ncause: ${err instanceof Error ? err.message : String(err)}`);
136+
}
137+
138+
if (typeof mod.LocalManifestSource !== 'function') {
139+
throw new Error(`${PREFLIGHT_HINT}\n\ncause: the module loaded but exports no LocalManifestSource.`);
140+
}
141+
142+
// `list()` short-circuits on a non-existent directory, so this probes the
143+
// return SHAPE without touching the filesystem.
144+
const listing = new mod.LocalManifestSource(path.join(os.tmpdir(), 'os-5612-preflight-absent')).list();
145+
if (!Array.isArray(listing?.entries) || !Array.isArray(listing?.skipped)) {
146+
throw new Error(
147+
`${PREFLIGHT_HINT}\n\ncause: LocalManifestSource.list() returned ${JSON.stringify(listing)}, ` +
148+
'not the { entries, skipped } listing #5413 introduced — the built artefact predates it.',
149+
);
150+
}
151+
}
152+
78153
describe('installedPackageLedgerFailureCheck — the finding the shared catch used to eat', () => {
79154
it('quotes what was thrown, in the row AND in the verbose detail', () => {
80155
const err = Object.assign(new Error("ENOTDIR: not a directory, scandir '/p/.objectstack'"), {
@@ -145,6 +220,10 @@ describe('os doctor, end to end, against an unreadable installed-package ledger'
145220
let cwdSpy: ReturnType<typeof vi.spyOn>;
146221
const savedPosture = process.env.OS_TENANCY_POSTURE;
147222

223+
// #5612 — one accurate failure instead of seven misleading ones. See
224+
// `assertLedgerReaderIsBuilt()`.
225+
beforeAll(assertLedgerReaderIsBuilt);
226+
148227
beforeEach(() => {
149228
tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-5412-e2e-'));
150229
fs.mkdirSync(path.join(tmp, 'node_modules'));
@@ -425,6 +504,16 @@ describe('the optional package being absent stays completely silent (#5412 does
425504
* module registry is what keeps this scoped to this one test — every case
426505
* above needs the real module.
427506
*/
507+
/**
508+
* #5612 — this case needs the guard MORE than the e2e block does, not less.
509+
* It simulates the package being unloadable and asserts doctor stays silent;
510+
* in a worktree where the package really is unloadable it passes for that
511+
* reason instead of for the mock's, i.e. green because nothing was proven
512+
* (the empty-verdict trap PR #5046 wrote down). The preflight is what keeps
513+
* the simulation distinguishable from the accident it simulates.
514+
*/
515+
beforeAll(assertLedgerReaderIsBuilt);
516+
428517
afterEach(() => {
429518
vi.doUnmock('@objectstack/cloud-connection');
430519
vi.resetModules();

0 commit comments

Comments
 (0)