Skip to content

Commit 39136cc

Browse files
yinlianghuiclaude
andauthored
fix(test): runner 的包级 test 入口指回仓根配置,不再落到 app 的 vite.config (#3746) (#3869)
`pnpm --filter @object-ui/runner test` 在干净 main 上 4 红(`window is not defined`),而同一批 13 个用例从仓根跑全绿。机制:`packages/runner` 声明了 `"test": "vitest run"` 却没有 vitest 配置,pnpm 把 cwd 定在包目录,vitest 于是 把该目录里那份 **Vite 应用**配置 `vite.config.ts` 当成自己的配置读进来 —— 它只有 `resolve.alias` / `build`,没有 `test` 块,于是没有 DOM 环境、没有 setup(那次运行自己的汇总就写着 `setup 0ms ... environment 0ms`), `App.navigation.test.tsx` 的 4 个用例在 node 环境里撞死在 `window` 上。 修法是把这个入口指回**仓根配置**,而不是给 runner 补一份包级配置: - 包级配置会是第 18 份「#3240 裁决 A(2026-08-06,全删)」要删的东西,且它解析 `@object-ui/*` 走 runner 自己那张 15 条的 alias 表(其余落 dist),而仓根配置把 ~40 个包名指向 `src` —— 正是 #3240 要消灭的「同一文件两套结论」; - 它还会把这次运行从 `scripts/vitest-invocation-guard.mjs` 底下抬出去(guard 只在 仓根配置里被调用),#3288 的 `--` 转发陷阱对本包就不再被拒绝。 所以脚本改成 AGENTS.md(§怎么跑测试)与 guard 报错正文都指定的那条写法: `vitest run --root ../.. packages/runner/`。两个入口从此是同一份配置,用例数一致 (3 文件 / 13 用例),`vitest list` 显示 runner 的 3 个文件各自只在一个 project 注册([unit] x2、[dom] x1),仓根 projects 机制不受影响。 钉子 `scripts/__tests__/runner-package-test-entry-3746.test.ts`:钉住该脚本把 vitest root 指回仓根、被 guard 接受、过滤面正好覆盖 runner 自己的全部测试文件; 另有一条全仓棘轮,记录另外 7 个同形状(有 vite.config、无 vitest 配置、bare `vitest run`)的包为**上限**,只许缩不许长 —— 它们的整改属于 #3240 的单独批次, 本单不碰。 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt Co-authored-by: Claude <noreply@anthropic.com>
1 parent d11996e commit 39136cc

2 files changed

Lines changed: 254 additions & 1 deletion

File tree

‎packages/runner/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"type-check": "tsc --noEmit && tsc -p tsconfig.test.json",
2020
"lint": "eslint .",
2121
"preview": "vite preview",
22-
"test": "vitest run"
22+
"test": "vitest run --root ../.. packages/runner/"
2323
},
2424
"dependencies": {
2525
"@object-ui/components": "workspace:*",
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
import { describe, expect, it } from 'vitest';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
4+
import { fileURLToPath } from 'node:url';
5+
6+
// Plain-JS CI helper. Its types are INFERRED from the .mjs source by
7+
// `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here — adding one
8+
// would itself be an error (TS2578). See objectui#3494.
9+
import { evaluateVitestInvocation, parseVitestArgv } from '../vitest-invocation-guard.mjs';
10+
11+
/**
12+
* objectui#3746 — `pnpm --filter @object-ui/runner test` was 4 red on a clean
13+
* `main` while the very same 13 tests were green from the repo root.
14+
*
15+
* Mechanism, measured at 6402e253f: `packages/runner` declared
16+
* `"test": "vitest run"` and has NO vitest config of its own, so vitest —
17+
* launched by pnpm with the package as its cwd, hence as its root — adopted the
18+
* `vite.config.ts` sitting in that directory. That file is runner's config as a
19+
* Vite APPLICATION: `resolve.alias` plus `build`, with no `test` block, so no DOM
20+
* environment and no setup files. The four `App.navigation.test.tsx` cases then
21+
* ran in the node environment and died on `ReferenceError: window is not
22+
* defined` — the run's own summary said `setup 0ms ... environment 0ms`.
23+
*
24+
* The fix routes that entry back to the ROOT config rather than giving runner a
25+
* local one, and that choice is the part worth pinning:
26+
*
27+
* - a package-local vitest config would be an 18th of exactly the kind
28+
* objectui#3240's maintainer ruling (option A, 2026-08-06) is to DELETE, and
29+
* it would resolve `@object-ui/*` through runner's own 15-entry alias table
30+
* (Node resolution, i.e. `dist`, for everything else) while the root config
31+
* maps ~40 of them to `src` — the "one file, two verdicts" divergence #3240
32+
* exists to remove;
33+
* - it would also lift the run out from under
34+
* `scripts/vitest-invocation-guard.mjs`, which is reachable only through the
35+
* root config: the objectui#3288 `--`-forwarding trap would silently stop
36+
* being refused for this package.
37+
*
38+
* So the script now spells the invocation AGENTS.md (§怎么跑测试) and the guard's
39+
* own error message both prescribe for a package directory:
40+
* `vitest run --root ../.. packages/runner/`. Both entries are then literally the
41+
* same config, and the counts agree (13 tests in 3 files, either way).
42+
*
43+
* Reverse verification: restore `"test": "vitest run"` and this file goes red
44+
* naming runner, while the repo-root entry stays green — the package entry is the
45+
* only thing the change moves.
46+
*
47+
* Scope: this pins RUNNER. Seven sibling packages carry the same hijackable shape
48+
* and are deliberately left alone — see BARE_ENTRY_BASELINE.
49+
*/
50+
51+
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
52+
53+
const RUNNER_DIR = 'packages/runner';
54+
55+
/** Workspace groups from the root `package.json` that can hold a `test` script. */
56+
const WORKSPACE_GROUPS = ['packages', 'apps', 'examples'];
57+
58+
const VITE_CONFIG = /^vite\.config\.[cm]?[jt]s$/;
59+
const VITEST_CONFIG = /^vitest\.config\.[cm]?[jt]s$/;
60+
const TEST_FILE = /\.test\.[cm]?[jt]sx?$/;
61+
62+
/**
63+
* Packages whose `test` script still hands vitest a package-level root while the
64+
* directory holds a `vite.config.*` and no vitest config, so vitest adopts the
65+
* app-facing Vite config. Whether each is actually RED depends on whether its
66+
* tests touch the DOM (runner's did) — the shape is what is measured here, which
67+
* is all a static check can honestly claim.
68+
*
69+
* Recorded as a CEILING, not an inventory: the list may shrink and must not grow.
70+
* The repo-wide redefinition of the package-level `test` scripts belongs to
71+
* objectui#3240 (maintainer ruling A), which is to run as its own batch; this
72+
* file must not become a reason to touch these packages out of band.
73+
*/
74+
const BARE_ENTRY_BASELINE = [
75+
'packages/plugin-ai',
76+
'packages/plugin-chatbot',
77+
'packages/plugin-designer',
78+
'packages/plugin-editor',
79+
'packages/plugin-markdown',
80+
'packages/plugin-report',
81+
'packages/plugin-tree',
82+
];
83+
84+
type PackageEntry = {
85+
/** Repo-relative directory, e.g. `packages/runner`. */
86+
dir: string;
87+
testScript: string;
88+
hasViteConfig: boolean;
89+
hasVitestConfig: boolean;
90+
};
91+
92+
function readEntriesWithTestScript(): PackageEntry[] {
93+
const entries: PackageEntry[] = [];
94+
for (const group of WORKSPACE_GROUPS) {
95+
const groupDir = path.join(repoRoot, group);
96+
if (!fs.existsSync(groupDir)) continue;
97+
for (const name of fs.readdirSync(groupDir)) {
98+
const dir = path.join(groupDir, name);
99+
const manifest = path.join(dir, 'package.json');
100+
if (!fs.existsSync(manifest)) continue;
101+
const pkg = JSON.parse(fs.readFileSync(manifest, 'utf8')) as {
102+
scripts?: Record<string, string>;
103+
};
104+
const testScript = pkg.scripts?.test;
105+
if (!testScript) continue;
106+
const files = fs.readdirSync(dir);
107+
entries.push({
108+
dir: `${group}/${name}`,
109+
testScript,
110+
hasViteConfig: files.some((f) => VITE_CONFIG.test(f)),
111+
hasVitestConfig: files.some((f) => VITEST_CONFIG.test(f)),
112+
});
113+
}
114+
}
115+
return entries;
116+
}
117+
118+
/** `process.argv` as pnpm would hand it to a package `test` script. */
119+
function argvForScript(script: string): string[] {
120+
const tokens = script.trim().split(/\s+/);
121+
// tokens[0] is the binary (`vitest`); in process.argv that slot is argv[1].
122+
return ['/usr/bin/node', '/bin/vitest', ...tokens.slice(1)];
123+
}
124+
125+
/**
126+
* The directory this script makes vitest's ROOT — the cwd it is launched from,
127+
* unless it points `--root` somewhere else. That root is what decides which
128+
* config file vitest loads and what the projects' `include` globs resolve
129+
* against (objectui#3378).
130+
*/
131+
function vitestRootOf(entry: PackageEntry): string {
132+
const { flags } = parseVitestArgv(argvForScript(entry.testScript));
133+
const raw = flags['--root'] ?? flags['-r'];
134+
const cwd = path.join(repoRoot, entry.dir);
135+
return typeof raw === 'string' ? path.resolve(cwd, raw) : cwd;
136+
}
137+
138+
function collectTestFiles(dir: string): string[] {
139+
const out: string[] = [];
140+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
141+
if (entry.name === 'node_modules' || entry.name === 'dist') continue;
142+
const abs = path.join(dir, entry.name);
143+
if (entry.isDirectory()) out.push(...collectTestFiles(abs));
144+
else if (TEST_FILE.test(entry.name)) out.push(abs);
145+
}
146+
return out;
147+
}
148+
149+
const entries = readEntriesWithTestScript();
150+
151+
function requireRunner(): PackageEntry {
152+
const found = entries.find((e) => e.dir === RUNNER_DIR);
153+
if (!found) {
154+
throw new Error(
155+
`${RUNNER_DIR} declares no "test" script. objectui#3746 pinned the SHAPE of that ` +
156+
'entry; if the entry itself is being removed, retire this pin with it.'
157+
);
158+
}
159+
return found;
160+
}
161+
162+
const runner = requireRunner();
163+
164+
describe('objectui#3746 — the runner package-level test entry', () => {
165+
it('still has the hijackable shape: a vite.config with no `test` block, and no vitest config', () => {
166+
// If either of these flips, the assertions below stop protecting what they
167+
// were written for and this pin needs re-reading rather than re-spelling.
168+
expect(runner.hasViteConfig).toBe(true);
169+
expect(runner.hasVitestConfig).toBe(false);
170+
171+
const viteConfig = fs.readFileSync(path.join(repoRoot, RUNNER_DIR, 'vite.config.ts'), 'utf8');
172+
expect(viteConfig).not.toMatch(/^\s*test\s*:/m);
173+
});
174+
175+
it('routes vitest root back to the repo root instead of running under the app vite config', () => {
176+
expect(runner.testScript).not.toBe('vitest run');
177+
expect(vitestRootOf(runner)).toBe(repoRoot);
178+
});
179+
180+
it('is accepted by the invocation guard that the root config wires up', () => {
181+
// Root == repo root, and no `--` forwarding: the two refusals of
182+
// objectui#3378 / #3288 both stay live for this entry, and it passes them.
183+
const verdict = evaluateVitestInvocation({
184+
argv: argvForScript(runner.testScript),
185+
cwd: path.join(repoRoot, RUNNER_DIR),
186+
repoRoot,
187+
});
188+
189+
expect(verdict).toBeNull();
190+
});
191+
192+
it("filters onto runner's own test files, all of them", () => {
193+
const { positionals } = parseVitestArgv(argvForScript(runner.testScript));
194+
195+
// No filter would mean the package entry runs the WHOLE repo suite; a filter
196+
// pointing outside runner would mean it runs somebody else's tests and calls
197+
// them green — objectui#3378's original shape.
198+
expect(positionals.length).toBeGreaterThan(0);
199+
for (const positional of positionals) {
200+
const abs = path.resolve(repoRoot, positional);
201+
expect(fs.existsSync(abs), `${positional} does not exist relative to the repo root`).toBe(
202+
true
203+
);
204+
expect(abs.startsWith(path.join(repoRoot, RUNNER_DIR))).toBe(true);
205+
}
206+
207+
// Vitest matches a positional as a substring of each test file's path, so
208+
// every one of runner's test files has to sit under one of them; otherwise the
209+
// package entry would quietly run a subset of the package.
210+
const testFiles = collectTestFiles(path.join(repoRoot, RUNNER_DIR));
211+
expect(testFiles.length).toBeGreaterThan(0);
212+
for (const file of testFiles) {
213+
const rel = path.relative(repoRoot, file);
214+
expect(
215+
positionals.some((positional) => rel.startsWith(positional)),
216+
`${rel} is not covered by the test script's filters (${positionals.join(' ')})`
217+
).toBe(true);
218+
}
219+
});
220+
221+
it('a bare `vitest run` from the package directory is the invocation this repo refuses', () => {
222+
// Note what this does and does not claim. For runner the bare form never
223+
// REACHED the guard: with no vitest config, vitest loaded `vite.config.ts` and
224+
// the root config — the only place the guard is called from — was never
225+
// evaluated, so the run proceeded in the node environment and produced four
226+
// `window is not defined` failures instead of a refusal. This is the verdict
227+
// the same invocation gets once the root config is in play, i.e. the reason
228+
// the fix belongs in the script and not in a new package-local config.
229+
const verdict = evaluateVitestInvocation({
230+
argv: argvForScript('vitest run'),
231+
cwd: path.join(repoRoot, RUNNER_DIR),
232+
repoRoot,
233+
});
234+
235+
expect(verdict?.code).toBe('package-cwd');
236+
});
237+
});
238+
239+
describe('objectui#3746 — the hijackable-entry population must not grow', () => {
240+
const bareEntries = entries
241+
.filter((e) => e.hasViteConfig && !e.hasVitestConfig && vitestRootOf(e) !== repoRoot)
242+
.map((e) => e.dir)
243+
.sort();
244+
245+
it('no longer counts runner', () => {
246+
expect(bareEntries).not.toContain(RUNNER_DIR);
247+
});
248+
249+
it('admits no package beyond the recorded baseline', () => {
250+
// Subset, not equality: objectui#3240 fixing these must keep this green.
251+
expect(bareEntries.filter((dir) => !BARE_ENTRY_BASELINE.includes(dir))).toEqual([]);
252+
});
253+
});

0 commit comments

Comments
 (0)