diff --git a/packages/test/src/test/util/PublishedEntryExportParity.test.ts b/packages/test/src/test/util/PublishedEntryExportParity.test.ts new file mode 100644 index 000000000..830880714 --- /dev/null +++ b/packages/test/src/test/util/PublishedEntryExportParity.test.ts @@ -0,0 +1,184 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +/** + * The workspace groups, derived from the root manifest's `workspaces` field. + * + * Duplicated derivation code rather than a duplicated list, for the reason + * `PublishedEntryImports.test.ts` spells out: `packages/test` is a `composite` + * project rooted at `./src`, so importing `scripts/lib/workspaceGroups.ts` + * would pull those files into its program and break `build-types`. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); + +/** + * The conditions a plain `import` from Node activates, and only those. Node's + * own algorithm: walk the keys IN ORDER and take the first active one, so a + * `types`-first block resolves to whatever follows it rather than to nothing. + */ +const NODE_CONDITIONS: ReadonlySet = new Set(["node", "import", "default"]); + +/** Source extensions a built entry can have come from, in resolution order. */ +const SOURCE_EXTENSIONS = [".ts", ".tsx"] as const; + +/** + * Entries no CI job can import, each with the reason — the same map, for the + * same entry and the same reason, as `PublishedEntryImports.test.ts`. Kept + * deliberately small so a NEW package defaults to being checked. Every key is + * proved below to still name a published entry. + */ +const UNCHECKABLE: Readonly> = { + "@workglow/cli": + "an example app, and `packages/test` does not depend on it — under bunfig's " + + "isolated linker the specifier does not resolve from here at all, so importing " + + "it would test the linker rather than the bundle", +}; + +interface EntryPair { + /** The specifier a consumer writes, e.g. `@workglow/util/schema`. */ + readonly specifier: string; + /** Absolute path of the source module the built entry was built from. */ + readonly sourcePath: string; +} + +/** The target a conditional `exports` value resolves to under Node. */ +function resolveUnderNode(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (typeof value !== "object" || value === null || Array.isArray(value)) return undefined; + for (const [condition, child] of Object.entries(value as Record)) { + if (!NODE_CONDITIONS.has(condition)) continue; + const resolved = resolveUnderNode(child); + if (resolved !== undefined) return resolved; + } + return undefined; +} + +/** + * `/dist/.js` → `/src/.{ts,tsx}` — the inverse of what + * the build emits, and the same mapping `use-source` writes its stubs from. + * + * Re-derived here rather than imported from `scripts/lib/workspaceSource.ts` + * for the composite-project reason above. Deliberately returns `undefined` + * rather than guessing for a target with no counterpart (generated or copied + * build output), so such an entry is reported as skipped instead of failing. + */ +function sourceCounterpart(packageDir: string, target: string): string | undefined { + const match = /^\.\/dist\/(?.+)\.(?:js|mjs|cjs)$/.exec(target); + if (!match?.groups) return undefined; + for (const extension of SOURCE_EXTENSIONS) { + const candidate = join(packageDir, "src", `${match.groups.entry}${extension}`); + if (existsSync(candidate)) return candidate; + } + return undefined; +} + +/** Every published Node entry that has a source counterpart to compare against. */ +function collectEntryPairs(): { pairs: EntryPair[]; unmapped: string[] } { + const pairs: EntryPair[] = []; + const unmapped: string[] = []; + for (const group of WORKSPACE_GROUPS) { + let directories: string[]; + try { + directories = readdirSync(join(repoRoot, group)); + } catch { + continue; + } + for (const directory of directories) { + const packageDir = join(repoRoot, group, directory); + let manifest: { name?: unknown; exports?: unknown }; + try { + manifest = JSON.parse(readFileSync(join(packageDir, "package.json"), "utf8")); + } catch { + continue; // not a package directory + } + const { name, exports } = manifest; + if (typeof name !== "string") continue; + if (typeof exports !== "object" || exports === null || Array.isArray(exports)) continue; + for (const [subpath, value] of Object.entries(exports as Record)) { + if (!subpath.startsWith(".")) continue; // a bare condition map, not a subpath + const target = resolveUnderNode(value); + if (target === undefined) continue; // browser-only entry; nothing for Node to load + const specifier = name + subpath.slice(1); + const sourcePath = sourceCounterpart(packageDir, target); + if (sourcePath === undefined) { + unmapped.push(`${specifier} -> ${target}`); + continue; + } + pairs.push({ specifier, sourcePath }); + } + } + } + pairs.sort((a, b) => a.specifier.localeCompare(b.specifier)); + return { pairs, unmapped }; +} + +const { pairs, unmapped } = collectEntryPairs(); +const checkable = pairs.filter((pair) => !(pair.specifier in UNCHECKABLE)); + +/** + * Every published entry, imported twice — once by the specifier a consumer + * writes, once by the source file it was built from — with the export NAME sets + * compared. + * + * Why names, and why both sides: a bundle that lost a re-export still resolves + * and still evaluates cleanly, so `PublishedEntryImports.test.ts`'s "loads and + * exports something" check passes over it unchanged. That test's `> 0` bound is + * satisfied by a bundle carrying one symbol out of ninety. The export list is + * the only observable that says the entry point is intact, and the source file + * is the only available statement of what it should be. + * + * Under the default `source` target both sides resolve to the same module, so + * this passes trivially — a green source run is not a bundle check. The run + * that means something is `test-vitest-dist` (`WORKGLOW_TEST_TARGET=dist`), + * where the left side is the real bundle; hence a unit-tier file, since that is + * the tier the dist job runs. + */ +describe("published entry export parity", () => { + it("enumerates every workspace manifest, so an empty sweep cannot pass", () => { + // Anti-vacuity: a typo in the walk yields a short list rather than an + // error, and a short list passes every assertion below. + expect(pairs.length).toBeGreaterThan(60); + }); + + it("maps every published entry back to a source file", () => { + // A published entry whose target is not `./dist/.js`, or whose + // source twin is missing, is silently dropped from the sweep above — the + // same hole in a different shape. Listed here so it fails loudly instead. + expect(unmapped).toEqual([]); + }); + + it("keeps every exemption pinned to an entry that still exists", () => { + // An exemption that outlives its package silently exempts nothing, and + // reads as if a real hole were still open. + const published = new Set(pairs.map((pair) => pair.specifier)); + expect(Object.keys(UNCHECKABLE).filter((specifier) => !published.has(specifier))).toEqual([]); + for (const reason of Object.values(UNCHECKABLE)) { + expect(reason.length).toBeGreaterThan(20); + } + }); + + it.each(checkable.map((pair) => [pair.specifier, pair.sourcePath] as const))( + "%s exports the same names as its source", + async (specifier, sourcePath) => { + const [published, source] = await Promise.all([ + import(/* @vite-ignore */ specifier) as Promise>, + import(/* @vite-ignore */ sourcePath) as Promise>, + ]); + // Sorted, so the diff on failure names the missing symbols rather than + // reporting two shuffled lists as unequal. + expect(Object.keys(published).sort()).toEqual(Object.keys(source).sort()); + } + ); +}); diff --git a/packages/test/src/test/util/PublishedEntryIdentity.test.ts b/packages/test/src/test/util/PublishedEntryIdentity.test.ts new file mode 100644 index 000000000..f0875dcf0 --- /dev/null +++ b/packages/test/src/test/util/PublishedEntryIdentity.test.ts @@ -0,0 +1,268 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AiProvider, getAiProviderRegistry } from "@workglow/ai"; +import { AiProvider as WorkerAiProvider } from "@workglow/ai/worker"; +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { beforeAll, describe, expect, it } from "vitest"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +/** + * The workspace groups, derived from the root manifest's `workspaces` field. + * + * Duplicated derivation code rather than a duplicated list, and duplicated for + * the same reason `PublishedEntryImports.test.ts` gives: `packages/test` is a + * `composite` project rooted at `./src`, so importing `scripts/lib/workspaceGroups.ts` + * would pull those files into its program and break `build-types`. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); + +/** + * Providers whose `ai-runtime` entry cannot be registered inside a plain CI + * job, each with the reason it cannot. + * + * EMPTY, and measured to be: the local providers this was expected to need — + * `node-llama-cpp`, `huggingface-transformers`, `cactus`, + * `stable-diffusion-server` — all register cleanly with no native runtime + * present, because `register*Inline` only CONSTRUCTS the provider and hands its + * run-fn table to the registry. Every SDK, native binding and server probe is + * behind a run-fn and is reached only by an actual generation call, which this + * file never makes. Exempting them would have been a false statement that cost + * five of the sixteen candidates their coverage. + * + * The map stays as the seam for a provider that genuinely cannot register. It + * is stated per entry and must be kept small, so a NEW provider defaults to + * being checked — a permissive default would reopen exactly the hole this file + * closes. Every key is proved below to still name a candidate package, so a + * renamed or deleted provider takes its exemption with it. + */ +const NEEDS_NATIVE_RUNTIME: Readonly> = {}; + +/** + * The base classes a provider is allowed to have been built from — the ones + * `@workglow/ai` itself PUBLISHES. + * + * There are two, and that is deliberate rather than an oversight this test + * papers over. `@workglow/ai` builds `.` and `./worker` as separate `bun build` + * invocations, so each bundle carries its own copy of `AiProvider`; the worker + * entry exists precisely so a worker bundle does not drag in the full node + * entry, and the five providers with a worker runtime (`chrome-ai`, + * `tf-mediapipe`, `cactus`, `huggingface-transformers`, `node-llama-cpp`) + * extend the worker copy on purpose. Under the `source` target both specifiers + * resolve to one file and this set collapses to a single class. + * + * That published split is a real cross-entry identity seam — a consumer holding + * `AiProvider` from `@workglow/ai` and testing a Chrome AI provider with + * `instanceof` gets `false` today — but it is pre-existing, intentional, and + * belongs to the `@workglow/ai` build rather than to this check. What this file + * catches is the THIRD copy: one inlined into a provider's own `ai-runtime` + * bundle, which matches neither published class. + */ +const PUBLISHED_BASE_CLASSES = [AiProvider, WorkerAiProvider] as const; + +interface RuntimeCandidate { + /** The package that publishes both `./ai` and `./ai-runtime`. */ + readonly packageName: string; + /** The `ai-runtime` specifier a consumer writes. */ + readonly specifier: string; +} + +/** Every workspace package publishing BOTH an `./ai` and an `./ai-runtime` entry. */ +function collectRuntimeCandidates(): RuntimeCandidate[] { + const candidates: RuntimeCandidate[] = []; + for (const group of WORKSPACE_GROUPS) { + let directories: string[]; + try { + directories = readdirSync(join(repoRoot, group)); + } catch { + continue; + } + for (const directory of directories) { + let manifest: { name?: unknown; exports?: unknown }; + try { + manifest = JSON.parse( + readFileSync(join(repoRoot, group, directory, "package.json"), "utf8") + ); + } catch { + continue; // not a package directory + } + const { name, exports } = manifest; + if (typeof name !== "string") continue; + if (typeof exports !== "object" || exports === null || Array.isArray(exports)) continue; + const subpaths = Object.keys(exports as Record); + // Both halves matter: `./ai` holds the class hierarchy every consumer + // holds, `./ai-runtime` holds the registration that constructs into it. + // A package publishing only one of them cannot exhibit the split. + if (!subpaths.includes("./ai") || !subpaths.includes("./ai-runtime")) continue; + candidates.push({ packageName: name, specifier: `${name}/ai-runtime` }); + } + } + return candidates.sort((a, b) => a.packageName.localeCompare(b.packageName)); +} + +const candidates = collectRuntimeCandidates(); +const checkable = candidates.filter((c) => !(c.packageName in NEEDS_NATIVE_RUNTIME)); + +/** What one package's `register*Inline` actually put into the registry. */ +interface Registration { + readonly packageName: string; + /** The `register*Inline` export that was called. */ + readonly registrarName: string; + /** Registry keys that appeared as a result of calling it. */ + readonly providerNames: readonly string[]; +} + +const registrations: Registration[] = []; +/** + * Packages whose `ai-runtime` exports no `register*Inline` at all — skipped + * rather than failed, and reported in the anti-vacuity message below so a + * silently shrinking sweep is visible. Today that is `@workglow/mlx`, whose + * `registerMlx` deliberately returns without touching the registry until an + * mlx-lm runtime is bundled. + */ +const withoutInlineRegistrar: string[] = []; + +/** + * What this file is actually checking, and why the obvious version of it is + * vacuous. + * + * The failure mode is CLASS identity across bundle boundaries, exactly as + * `packages/task-graph/src/test-entry.ts` documents it. `registerAnthropicInline` + * constructs `new AnthropicQueuedProvider(...)` from a RELATIVE import inside the + * `ai-runtime` module graph, while that class extends a base built from + * `AiProvider` imported BY SPECIFIER. Inline `@workglow/ai` into + * `ai-runtime.js` — a bundler flag, a dropped `external`, a re-export rewritten + * from `export *` to `export { … } from` — and the constructed instance stops + * being `instanceof` the `AiProvider` every consumer holds, while every + * existing check stays green. + * + * Asserting on the SERVICE REGISTRY instead would prove nothing: the global DI + * container is stashed on `Symbol.for("@workglow/util/di/globalContainer")` + * (`packages/util/src/di/Container.ts`) precisely so duplicated bundle copies + * share one instance, and `createServiceToken` returns a plain string id. A + * duplicated `@workglow/ai` therefore resolves the SAME registry, and a `===` + * assertion on it is green by construction. + * + * Under the default `source` target this file passes trivially — every + * specifier resolves to `src`, so there is only ever one copy of every class. A + * green source run is therefore NOT a bundle check. The run that means + * something is `test-vitest-dist` (`WORKGLOW_TEST_TARGET=dist`), which is why + * this is a unit-tier file: that is the tier the dist job runs. + * + * It is affordable there because `register*Inline` needs NO API key. It + * constructs the provider and calls `registerProviderInline`, which calls + * `provider.register(...)` — registry bookkeeping and a strategy resolver, no + * network. Putting this on an integration tier instead would have cost real + * money for no extra signal, and would have been skipped entirely on fork PRs, + * where the secrets those suites gate on are unavailable. + */ +describe("published entry identity", () => { + beforeAll(async () => { + const registry = getAiProviderRegistry(); + for (const candidate of checkable) { + const loaded: Record = await import(/* @vite-ignore */ candidate.specifier); + const registrarName = Object.keys(loaded).find((key) => /^register\w+Inline$/.test(key)); + if (registrarName === undefined) { + withoutInlineRegistrar.push(candidate.packageName); + continue; + } + const before = new Set(registry.getProviders().keys()); + await (loaded[registrarName] as () => Promise)(); + registrations.push({ + packageName: candidate.packageName, + registrarName, + providerNames: [...registry.getProviders().keys()].filter((name) => !before.has(name)), + }); + } + }); + + it("enumerates the providers that publish both an ai and an ai-runtime entry", () => { + // Anti-vacuity. A typo in the walk (wrong group key, wrong subpath name) + // yields a SHORT list rather than an error, and every assertion below + // passes over a short list — including over an empty one. + expect(candidates.length).toBeGreaterThan(4); + expect(checkable.length).toBeGreaterThan(4); + }); + + it("keeps every exemption pinned to a package that still exists", () => { + // An exemption that outlives its package silently exempts nothing while + // reading as if a real hole were still open. + const known = new Set(candidates.map((c) => c.packageName)); + expect(Object.keys(NEEDS_NATIVE_RUNTIME).filter((name) => !known.has(name))).toEqual([]); + for (const reason of Object.values(NEEDS_NATIVE_RUNTIME)) { + expect(reason.length).toBeGreaterThan(20); + } + }); + + it("registers a provider from more than a handful of runtime entries", () => { + // The anti-vacuity guard that matters: the per-provider assertions below + // iterate what registration actually produced, so a run in which every + // registration silently no-opped would satisfy all of them. + const providers = getAiProviderRegistry().getProviders(); + expect( + providers.size, + `only ${providers.size} provider(s) registered from ${checkable.length} runtime entries. ` + + `Entries exporting no register*Inline: ${withoutInlineRegistrar.join(", ") || "(none)"}` + ).toBeGreaterThan(4); + }); + + it("publishes no more base classes than @workglow/ai has entry points", () => { + // Guards the allowance above from growing quietly. One class under the + // `source` target (both specifiers are one file), two under `dist` (`.` and + // `./worker` are separate bundles). A third would mean a new split nobody + // decided on. + expect(new Set(PUBLISHED_BASE_CLASSES).size).toBeLessThanOrEqual(2); + }); + + it("constructs every provider from an AiProvider class @workglow/ai publishes", () => { + // THE check. A provider built against a copy of `AiProvider` inlined into + // its own runtime bundle is a perfectly functional object that fails this + // and nothing else — no other assertion anywhere distinguishes it. + const registry = getAiProviderRegistry(); + const offenders: string[] = []; + for (const { packageName, registrarName, providerNames } of registrations) { + for (const providerName of providerNames) { + const provider = registry.getProvider(providerName); + expect( + provider, + `${packageName}: ${registrarName}() registered "${providerName}" but the registry has no such provider` + ).toBeDefined(); + if (!PUBLISHED_BASE_CLASSES.some((base) => provider instanceof base)) { + offenders.push(`${packageName} -> ${providerName} (via ${registrarName})`); + } + } + } + // Collected rather than asserted in the loop, so one bad bundle reports + // itself instead of hiding every provider sorted after it. + expect( + offenders, + `these providers are not an instanceof any AiProvider that @workglow/ai publishes, which is ` + + `the signature of @workglow/ai being INLINED into the provider's own ai-runtime bundle ` + + `instead of left external: the runtime graph built its provider on a private copy of the ` + + `base class, so every consumer's instanceof check now returns false` + ).toEqual([]); + }); + + it("registers at least one run function per registered provider", () => { + // `instanceof` alone would still pass for a bundle that lost its run-fn + // module: the provider object is intact and serves nothing. + const registry = getAiProviderRegistry(); + const empty: string[] = []; + for (const { packageName, providerNames } of registrations) { + for (const providerName of providerNames) { + if (registry.getRunFnRegistrations(providerName).length === 0) { + empty.push(`${packageName} -> ${providerName}`); + } + } + } + expect(empty).toEqual([]); + }); +}); diff --git a/packages/test/src/test/util/PublishedEntryImports.test.ts b/packages/test/src/test/util/PublishedEntryImports.test.ts index 7e4c085d4..a33952a7d 100644 --- a/packages/test/src/test/util/PublishedEntryImports.test.ts +++ b/packages/test/src/test/util/PublishedEntryImports.test.ts @@ -11,8 +11,21 @@ import { describe, expect, it } from "vitest"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); -/** The root `workspaces` globs, in the order a manifest scan walks them. */ -const WORKSPACE_GROUPS = ["packages", "providers", "examples"] as const; +/** + * The workspace groups, derived from the root manifest's `workspaces` field: + * `"./packages/*"` → `"packages"`. + * + * This is duplicated DERIVATION CODE, not a duplicated list. + * `scripts/lib/workspaceGroups.ts` owns the same reduction and every other + * caller imports it from there; this file cannot, for the reason spelled out on + * the `describe` below — `packages/test` is a `composite` project rooted at + * `./src`, so importing from `scripts/` would put those files in its program + * and break `build-types`. Re-deriving still beats copying the list: a group + * added to `package.json` is picked up here instead of being silently skipped. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); /** * The conditions a plain `import` from Node activates, and only those. diff --git a/scripts/lib/testDiscovery.ts b/scripts/lib/testDiscovery.ts index 3ff9d5b1b..efd185aaf 100644 --- a/scripts/lib/testDiscovery.ts +++ b/scripts/lib/testDiscovery.ts @@ -13,14 +13,25 @@ import { readdirSync, readFileSync, statSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +// Extension required: this module is in `vitest.config.ts`'s graph, which Vite's +// native config loader resolves without extension inference. +import { workspaceGroups } from "./workspaceGroups.ts"; // `import.meta.dir` and `Bun.Glob` are Bun-only; this module is also imported by // a vitest (Node) test, so everything here stays on portable node: APIs. export const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "../.."); /** The legacy monolithic test package: sections come from its subdirectories. */ export const TEST_BASE = join(ROOT, "packages/test/src/test"); -/** Workspace groups that may hold in-package tests (sectioned by package name). */ -export const PACKAGE_GROUPS = ["packages", "providers", "examples"] as const; +/** + * Workspace groups that may hold in-package tests (sectioned by package name). + * + * Derived from the root manifest rather than listed: this is the same concept + * as the resolver's and the coverage denominator's — "a top-level directory + * holding workspace packages" — and a group declared in `package.json` but + * missing from a hand-written copy here does not error, it just makes every + * test under it invisible to the runner. + */ +export const PACKAGE_GROUPS: readonly string[] = workspaceGroups(ROOT); /** Non-package directories that also hold tests (e.g. tests for the tooling itself). */ export const EXTRA_TEST_DIRS = ["scripts"] as const; @@ -237,7 +248,7 @@ export function projectDirOf(filePath: string): string | undefined { const rel = filePath.startsWith(ROOT + "/") ? filePath.slice(ROOT.length + 1) : filePath; const parts = rel.split("/"); if ((EXTRA_TEST_DIRS as readonly string[]).includes(parts[0])) return parts[0]; - if ((PACKAGE_GROUPS as readonly string[]).includes(parts[0]) && parts.length > 1) { + if (PACKAGE_GROUPS.includes(parts[0]) && parts.length > 1) { return `${parts[0]}/${parts[1]}`; } return undefined; diff --git a/scripts/lib/workspaceGroups.ts b/scripts/lib/workspaceGroups.ts new file mode 100644 index 000000000..a0201d485 --- /dev/null +++ b/scripts/lib/workspaceGroups.ts @@ -0,0 +1,88 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + * + * The top-level directories that hold workspace packages, derived from the root + * manifest's `workspaces` field. + * + * The same three-element list — `packages`, `providers`, `examples` — was + * written out by hand in four places (the source-resolving plugin, the test + * discovery walk, the published-entry sweep, and the coverage denominator). + * Adding a fourth group to `package.json` therefore silently no-oped in all of + * them: nothing errors, the walks simply never look in the new directory, and + * the only symptom is a shorter file list in a report nobody diffs. + * + * This module owns the derivation and nothing else, for the same reason + * `testDiscovery.ts` gives for owning its own: several modules must agree on + * it, so none of them should own it. + * + * Node-portable on purpose. `vitest.config.ts` imports this and Vite loads that + * config under Node, so `Bun.Glob` (which `scripts/lib/util.ts` uses for its + * own, Bun-only, workspace scan) is not available here. + */ + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** + * The workspace groups declared by the root manifest, in declaration order and + * de-duplicated. + * + * A pattern is reduced to the single directory a scan can walk: + * `"./packages/*"` → `"packages"`. Anything that does not reduce to exactly one + * scannable directory THROWS rather than being guessed at — a recursive pattern + * names no single directory to read, and picking its prefix would quietly walk + * the wrong tree, which is the same silent no-op this module exists to remove. + */ +export function workspaceGroups(root: string): readonly string[] { + const manifestPath = join(root, "package.json"); + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { workspaces?: unknown }; + const patterns = manifest.workspaces; + if (!Array.isArray(patterns)) { + throw new Error( + `${manifestPath} declares no "workspaces" array, so there are no workspace groups to derive.` + ); + } + + const groups: string[] = []; + for (const raw of patterns) { + if (typeof raw !== "string") { + throw new Error(`${manifestPath}: workspace pattern ${JSON.stringify(raw)} is not a string.`); + } + const pattern = raw.startsWith("./") ? raw.slice(2) : raw; + const star = pattern.indexOf("*"); + const head = star === -1 ? pattern : pattern.slice(0, star); + const tail = star === -1 ? "" : pattern.slice(star); + // `packages/*` is the only glob shape that names one directory. `**/x`, + // `*/x` and a bare `**` all match at more than one depth. + if (tail !== "" && tail !== "*") { + throw new Error( + `${manifestPath}: workspace pattern "${raw}" matches more than one directory depth. ` + + `Only a plain directory or "/*" reduces to a single scannable group.` + ); + } + const group = head.endsWith("/") ? head.slice(0, -1) : head; + if (group === "") { + throw new Error( + `${manifestPath}: workspace pattern "${raw}" names no directory to scan. ` + + `Expected something like "./packages/*".` + ); + } + if (!groups.includes(group)) groups.push(group); + } + return groups; +} + +/** + * The coverage `include` globs for a set of groups. + * + * The denominator is every workspace package's own `src`, so it has to be + * derived from the same groups everything else walks: a group present in the + * scan but absent here has its source rewritten to `src`, executed by its own + * tests, and then left out of the report entirely — invisible, because a + * coverage report shows a shorter file list rather than an error. + */ +export function coverageIncludeGlobs(groups: readonly string[]): string[] { + return groups.map((group) => `${group}/*/src/**/*.{ts,tsx}`); +} diff --git a/scripts/lib/workspaceSource.ts b/scripts/lib/workspaceSource.ts index 6810f6ca2..b1bbb68bb 100644 --- a/scripts/lib/workspaceSource.ts +++ b/scripts/lib/workspaceSource.ts @@ -32,9 +32,9 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; import type { Plugin } from "vite"; - -/** Workspace groups from the root `workspaces` globs. */ -export const WORKSPACE_GROUPS = ["packages", "providers", "examples"] as const; +// Extension required: this module is in `vitest.config.ts`'s graph, which Vite's +// native config loader resolves without extension inference. +import { workspaceGroups } from "./workspaceGroups.ts"; /** Source extensions a dist entry can have come from, in resolution order. */ const SOURCE_EXTENSIONS = [".ts", ".tsx"] as const; @@ -88,6 +88,16 @@ export interface WorkspacePackage { readonly exports: unknown; /** Every name this package lists in any dependency block. */ readonly dependencies: ReadonlySet; + /** + * Whether this workspace ships anything to a registry — `false` only for a + * manifest that opts out with `publishConfig.access: "none"`. + * + * Deliberately NOT `private`. `packages/test`, `providers/aws` and + * `providers/cloudflare` are all `private: true` and all carry real tests + * whose source belongs in the coverage denominator; `access: "none"` is the + * narrower statement that a workspace publishes no API at all. + */ + readonly publishes: boolean; } /** Dependency blocks that make a package resolvable from another one. */ @@ -118,7 +128,7 @@ function declaredDependencies(manifest: Record): ReadonlySet), + publishes: access !== "none", + }); } } return found.sort((a, b) => a.name.localeCompare(b.name)); @@ -427,10 +450,19 @@ export const WORKSPACE_SOURCE_PLUGIN_NAME = "workglow:workspace-source"; * * A one-line adapter over {@link resolveWorkspaceSourceId}: the package list is * read once, and every decision lives in that function. + * + * `packages` is accepted so a caller building MANY projects can scan once and + * share the result. `vitest.config.ts` builds one project per workspace that + * holds tests and every one needs the plugin attached (projects are standalone + * Vite configs, so a root-level `plugins` entry never reaches them); scanning + * per project re-read every workspace manifest once per project at config load, + * for an answer that cannot differ between them. + * The plugin holds no per-project state, so one instance serves them all. */ -export function workspaceSourcePlugin(root: string): Plugin { - const packages = listWorkspacePackages(root); - +export function workspaceSourcePlugin( + root: string, + packages: readonly WorkspacePackage[] = listWorkspacePackages(root) +): Plugin { return { name: WORKSPACE_SOURCE_PLUGIN_NAME, enforce: "pre", diff --git a/scripts/workspaceSource.test.ts b/scripts/workspaceSource.test.ts index bf176dd44..06d68aa2e 100644 --- a/scripts/workspaceSource.test.ts +++ b/scripts/workspaceSource.test.ts @@ -4,11 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { stubSpecsFor, type PackageManifest } from "./lib/sourceStubs"; import { ROOT } from "./lib/testDiscovery"; +import { coverageIncludeGlobs, workspaceGroups } from "./lib/workspaceGroups"; import type { UnresolvedWorkspaceContext, WorkspacePackage, @@ -24,7 +26,6 @@ import { resolveWorkspaceSourceId, TEST_TARGETS, unresolvedWorkspaceMessage, - WORKSPACE_GROUPS, WORKSPACE_SOURCE_PLUGIN_NAME, workspaceSourcePlugin, } from "./lib/workspaceSource"; @@ -81,26 +82,156 @@ describe("workspace source resolution", () => { }); /** - * The invariant that actually broke: the resolver covers all three workspace - * groups, but the coverage denominator listed only two, so `examples/*` - * source was rewritten to `src`, executed by its own tests, and then left out - * of the denominator entirely. All three example packages are published and - * none is `private`, so there is no "not really shipped" argument for the - * omission — and a missing group is invisible in a coverage report, which - * shows a smaller file list rather than an error. + * The workspace groups, and what the coverage denominator is built from. * - * Reads the ACTUAL config rather than re-deriving it, so the two cannot drift - * back apart. + * The invariant that actually broke: the resolver covered all three groups + * while the denominator listed two, so `examples/*` source was rewritten to + * `src`, executed by its own tests, and then left out of the report entirely + * — invisible, because a coverage report shows a shorter file list rather + * than an error. The same list was written out by hand in four places. + * + * The guard that used to sit here ("every group in `WORKSPACE_GROUPS` appears + * in `coverage.include`") is tautological now that both sides derive from the + * root manifest, and its stated goal — fail when a group is added to + * `package.json` and nowhere else — is no longer satisfiable in the literal + * sense: with derivation, nowhere else NEEDS the change. What is still worth + * guarding is that the derivation is real and reaches something: that nobody + * hardcodes the list back, that every declared group actually holds packages, + * and that the denominator is built from those same groups and not from a + * hand-edited glob. */ - it("counts every workspace group in the coverage denominator", async () => { - const mod = (await import("../vitest.config.ts")) as { - default: { test?: { coverage?: { include?: string[] } } }; - }; - const include = mod.default.test?.coverage?.include ?? []; - const missing = WORKSPACE_GROUPS.filter( - (group) => !include.some((glob) => glob.startsWith(`${group}/`)) - ); - expect(missing).toEqual([]); + describe("workspace groups", () => { + it("derives the groups from the root workspaces field, not from a list", () => { + // Parsed independently HERE, so the test does not merely re-run the code + // it is checking. Hardcoding the array back into `workspaceGroups` — the + // exact regression this file exists to prevent — fails the moment a + // fourth group is declared, and reads as a deliberate contradiction now. + const manifest = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")) as { + workspaces: string[]; + }; + const expected = manifest.workspaces.map((pattern) => + pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "") + ); + expect(workspaceGroups(ROOT)).toEqual(expected); + }); + + it("finds at least one package in every declared workspace group", () => { + // Catches the other half: a pattern the reduction mishandles, or a + // directory renamed in the tree but not in the manifest, yields a group + // name that scans to nothing. Every walk downstream then silently covers + // one group fewer. + const empty = workspaceGroups(ROOT).filter( + (group) => !packages.some((pkg) => pkg.dir.startsWith(join(ROOT, group) + "/")) + ); + expect(empty).toEqual([]); + }); + + it("builds the coverage denominator from those same groups", async () => { + // Compared EXACTLY, not by prefix: a hand-edited or hand-appended glob is + // precisely how the two drifted apart the first time, and a `startsWith` + // check passes over a narrowed one. + const mod = (await import("../vitest.config.ts")) as { + default: { test?: { coverage?: { include?: string[] } } }; + }; + expect(mod.default.test?.coverage?.include).toEqual( + coverageIncludeGlobs(workspaceGroups(ROOT)) + ); + }); + + /** + * The reduction's own contract, on a fixture rather than on this repo — the + * real manifest exercises exactly one pattern shape, so nothing here would + * otherwise pin what happens to the others. + */ + it("reduces each declared pattern to one scannable directory", () => { + const root = mkdtempSync(join(tmpdir(), "workglow-groups-")); + try { + writeFileSync( + join(root, "package.json"), + JSON.stringify({ workspaces: ["./packages/*", "./integrations/*"] }) + ); + expect(workspaceGroups(root)).toEqual(["packages", "integrations"]); + + // A recursive pattern names no single directory to read. Guessing its + // prefix would walk the wrong tree and report nothing missing, which is + // the same silent no-op the derivation exists to remove. + writeFileSync(join(root, "package.json"), JSON.stringify({ workspaces: ["./a/**/c"] })); + expect(() => workspaceGroups(root)).toThrow(/more than one directory depth/); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it("distinguishes a malformed manifest from a non-package directory", () => { + // `listWorkspacePackages` swallowed every read failure alike. A directory + // with no `package.json` is the ordinary case and must stay silent; a + // manifest that fails to parse is a real fault, and dropping the package + // silently makes the source rewrite no-op for it — whose only symptom is + // that one package's coverage collapses back onto `dist/*`. + const root = mkdtempSync(join(tmpdir(), "workglow-manifest-")); + try { + writeFileSync(join(root, "package.json"), JSON.stringify({ workspaces: ["./packages/*"] })); + + // Declared group that does not exist on disk: no packages, no error. + expect(listWorkspacePackages(root)).toEqual([]); + + const brokenDir = join(root, "packages", "broken"); + mkdirSync(brokenDir, { recursive: true }); + writeFileSync(join(brokenDir, "package.json"), "{ not json"); + expect(() => listWorkspacePackages(root)).toThrow(join(brokenDir, "package.json")); + + // And a sibling with no manifest at all is still just "not a package". + rmSync(join(brokenDir, "package.json")); + expect(listWorkspacePackages(root)).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + }); + + /** + * Which PACKAGES the denominator counts, as opposed to which groups it walks. + * + * `examples/web` declares `publishConfig.access: "none"`, `exports: {}`, and + * neither `main` nor `bin`: it is a Vite app, and none of its ~34 non-test + * modules is reachable as published API. Counting them moves the repo's + * headline number without saying anything about the libraries. (The comment + * that used to sit here claimed all three example packages were published, + * which was simply false.) + */ + describe("coverage denominator membership", () => { + async function coverageExclude(): Promise { + const mod = (await import("../vitest.config.ts")) as { + default: { test?: { coverage?: { exclude?: string[] } } }; + }; + return mod.default.test?.coverage?.exclude ?? []; + } + + it("keeps packages that publish nothing out of the denominator", async () => { + const exclude = await coverageExclude(); + expect(exclude).toContain("examples/web/src/**"); + + // Pinned to the PROPERTY that justifies the exclusion, re-read from the + // manifest, so the exception dies with its reason: publish `@workglow/web` + // for real and this test demands the exclusion be removed. + const manifest = JSON.parse( + readFileSync(join(ROOT, "examples/web/package.json"), "utf8") + ) as { publishConfig?: { access?: string } }; + expect(manifest.publishConfig?.access).toBe("none"); + }); + + it("counts every package that does publish", async () => { + // The gate is `access: "none"`, NOT `private`. `packages/test`, + // `providers/aws` and `providers/cloudflare` are `private: true`, and the + // last two carry real suites whose source has to stay counted — gating on + // `private` would silently drop them. + const exclude = await coverageExclude(); + const wronglyExcluded = packages + .filter((pkg) => pkg.publishes) + .map((pkg) => `${pkg.dir.slice(ROOT.length + 1)}/src/**`) + .filter((glob) => exclude.includes(glob)); + expect(wronglyExcluded).toEqual([]); + }); }); /** @@ -277,6 +408,21 @@ describe("workspace source resolution", () => { expect(projects.filter(carriesPlugin)).toEqual([]); }); + it("attaches one plugin instance to every project", async () => { + // The only thing stopping the hoist being undone. Constructing the plugin + // inside the project `.map()` re-scanned every workspace manifest once + // per project — 12 projects x 41 manifests today, ~500 file reads at + // config load, for an answer that cannot differ between projects — and + // nothing about the result changes, so nothing else would catch it. + // + // Reference equality, not "same name": a per-project instance passes any + // name-based check. + const projects = await projectsForTarget("source"); + const instances = new Set(projects.map((project) => project.plugins?.[0])); + expect(instances.size).toBe(1); + expect([...instances][0]).toBeDefined(); + }); + it("names the plugin the same thing the diagnostic does", () => { expect(workspaceSourcePlugin(ROOT).name).toBe(WORKSPACE_SOURCE_PLUGIN_NAME); expect( diff --git a/vitest.config.ts b/vitest.config.ts index 30106e7f6..24e8f172b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,10 +1,15 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; -import { configDefaults, defineConfig } from "vitest/config"; +import { configDefaults, coverageConfigDefaults, defineConfig } from "vitest/config"; // Extension is required: Vite's native config loader cannot resolve an // extensionless relative import here. import { discoverTestFiles, listTestProjects } from "./scripts/lib/testDiscovery.ts"; -import { resolveTestTarget, workspaceSourcePlugin } from "./scripts/lib/workspaceSource.ts"; +import { coverageIncludeGlobs, workspaceGroups } from "./scripts/lib/workspaceGroups.ts"; +import { + listWorkspacePackages, + resolveTestTarget, + workspaceSourcePlugin, +} from "./scripts/lib/workspaceSource.ts"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const abs = (p: string): string => path.join(__dirname, p); @@ -73,6 +78,22 @@ const testsRunAgainstSource = resolveTestTarget(process.env.WORKGLOW_TEST_TARGET const discovered = discoverTestFiles(); +/** + * The workspace scan and the plugin are BOTH hoisted out of the project map + * below, and shared by every project. + * + * The plugin is stateless — every decision lives in `resolveWorkspaceSourceId`, + * over a package list that cannot differ between projects — so one instance + * serves all of them. Constructing it inside the map re-read every workspace + * manifest once per project: 12 projects x 41 manifests today, so ~500 file + * reads before a single test ran, for an answer identical every time. + * + * The package list is also what the coverage `exclude` below subtracts + * non-publishing workspaces from, so it is read exactly once for both uses. + */ +const workspacePackages = listWorkspacePackages(__dirname); +const sourcePlugin = workspaceSourcePlugin(__dirname, workspacePackages); + /** * One project per workspace that actually holds tests, derived from the same * discovery the runner and the reachability guard use. Deriving rather than @@ -95,7 +116,7 @@ const projects = listTestProjects(discovered).map((p) => { return { // Projects are standalone Vite configs, so a root-level `plugins` entry // would never reach them — the resolver has to be attached per project. - plugins: testsRunAgainstSource ? [workspaceSourcePlugin(__dirname)] : [], + plugins: testsRunAgainstSource ? [sourcePlugin] : [], test: { ...shared, name: p.name, root, exclude: [...shared.exclude, ...bunOnly] }, }; }); @@ -123,18 +144,26 @@ export default defineConfig({ * report exists to surface — and its file list changes with whichever * section CI happened to run. * - * All THREE workspace groups, matching `WORKSPACE_GROUPS`: `examples/*` - * holds published, non-private packages with tests of their own, so - * omitting it drops real source from the denominator while still counting - * the tests that cover it. + * EVERY workspace group, derived from the root manifest's `workspaces` + * field — the same derivation the source-resolving plugin and the test + * discovery walk use. A group the resolver rewrites to `src` but the + * denominator omits has its source executed by its own tests and then + * left out of the report, which shows up as a shorter file list rather + * than an error. Individual packages are subtracted below, by name, so + * every exception carries its own reason instead of being hidden in a + * shortened glob. */ - include: [ - "packages/*/src/**/*.{ts,tsx}", - "providers/*/src/**/*.{ts,tsx}", - "examples/*/src/**/*.{ts,tsx}", - ], + include: coverageIncludeGlobs(workspaceGroups(__dirname)), exclude: [ - ...configDefaults.exclude, + // `coverageConfigDefaults`, not `configDefaults`: the latter is + // vitest's TEST-FILE exclude list, spliced in here for a question it + // does not answer. It is empty in vitest 4 — which is precisely why the + // two must not be confused, since `configDefaults.exclude` is NOT, and + // the difference between them was silently supplying the two entries + // restated on the next line. + ...coverageConfigDefaults.exclude, + "**/node_modules/**", + "**/.git/**", // Built output is never the unit of measure. Nothing should resolve // here now that specifiers land on `src`, but a `use-source` stub or a // stale bundle left in a working tree would otherwise be reported as a @@ -146,12 +175,38 @@ export default defineConfig({ // odd non-`.test.` helper (`chromeAvailability.ts`) that the filename // rules below cannot catch. "examples/*/src/test/**", - // Tests, fixtures and testing-only helpers: counting them inflates - // every package by the coverage of code that exists to be run. + /** + * Workspaces that publish nothing (`publishConfig.access: "none"`). + * Today that is only `examples/web`, a Vite app whose ~34 non-test + * modules are UI wiring behind no published entry point: it exports + * nothing, has no `main` and no `bin`, so none of that source is API + * anyone can consume, and counting it moves the repo's headline number + * without saying anything about the libraries. + * + * The gate is `access: "none"`, deliberately NOT `private`. + * `packages/test`, `providers/aws` and `providers/cloudflare` are all + * `private: true` and aws/cloudflare carry real suites whose source has + * to stay counted. + * + * Subtracted by package path rather than by shortening the `include` + * globs above, so the group list stays derived and this exception + * carries its own reason. + */ + ...workspacePackages + .filter((pkg) => !pkg.publishes) + .map((pkg) => `${path.relative(__dirname, pkg.dir)}/src/**`), + // Tests, fixtures and typing-only files: counting them inflates every + // package by the coverage of code that exists to be run. + // + // `**/testing/**` is deliberately NOT here. Those directories are + // published API — `@workglow/task-graph/test` and `@workglow/util/test` + // ship the repository contracts, the shared fake tasks and the testing + // logger that other packages' suites import by specifier — so excluding + // them dropped 11 real source files from the denominator. The one test + // file among them is removed by `**/__tests__/**` below. "**/__tests__/**", "**/*.test.*", "**/*.test-d.ts", - "**/testing/**", "**/*.d.ts", "**/bench/**", ],