Skip to content

Commit d7c1e67

Browse files
hotlongclaude
andauthored
fix(devx): graduate metadata-fs and example-showcase out of TEST_DEBT (#8061)
Both packages' test layers were hidden from tsc by an include-shaped hole that #7353 taught TESTS_COVERED to see, and both were ledgered as measured TEST_DEBT rather than repaired. This repairs them via the #5286 route and deletes both entries in the same PR, which RECONCILED forces as a pair. Re-measured on the merged ref before repair; both matched their recorded numbers exactly (metadata-fs 6, example-showcase 4), so the entries are deleted against measurements rather than against hope. metadata-fs takes the sibling-config route. Its rootDir is `src` and `dev` emits (`tsc --watch`, outDir dist), so widening rootDir in the BUILD config would relocate dist/index.js and start emitting compiled tests; the sibling tsconfig.test.json carries the widened rootDir under noEmit instead and is named by the typecheck script. Its 6 errors were real: 5 dead imports (TS6133, from the root config's noUnusedLocals) and one TS2349 where an immediately-invoked drain loop made tsc narrow a captured `let` to `null`, so the optional call resolved to `never`. Binding the loop to a name before calling it restores the declared type; runtime behaviour is unchanged. example-showcase takes the widened-include route (#7312's shape for app-crm / app-todo) since its rootDir is already `.`. Its 4 TS2339 were `process.env` reads against the package's minimal ambient shim, fixed with the file-local `declare const process` idiom the package already uses in objectstack.config.ts and src/system/self-url.ts. The include glob is `e2e/**/*.spec.ts` and NOT `e2e/**/*`, holding the line the deleted entry's note drew: the wholesale glob would pull in e2e/global-setup.ts, a fixture rather than a test, and bill the layer 6 errors that are not its own. @objectstack/cli stays in the ledger — a programme, not a graduation. No other entry's recorded number is touched. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 214eb30 commit d7c1e67

9 files changed

Lines changed: 131 additions & 10 deletions

File tree

examples/app-showcase/e2e/bulk-capability-gate.spec.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ import { test, expect } from '@playwright/test';
2525
* filtered on it (objectui#3492) — this spec is what notices either side
2626
* regressing.
2727
*/
28+
29+
// Ambient `process` for the env read below — the showcase tsconfig doesn't pull
30+
// in `@types/node`, and the package-global shim in test/node-shim.d.ts declares
31+
// only `cwd()`. Same idiom (and same reason) as the declarations in
32+
// objectstack.config.ts and src/system/self-url.ts: keeps `pnpm typecheck` green
33+
// without widening the type surface. Playwright provides the real `process`.
34+
declare const process: { env: Record<string, string | undefined> };
35+
2836
const APP = process.env.SHOWCASE_APP || 'com.example.showcase';
2937

3038
test('selection bar hides capability-gated inline defs from a caller without the grants', async ({ page }) => {

examples/app-showcase/e2e/detail-shapes.spec.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ import { test, expect } from '@playwright/test';
2626
* symbol.
2727
*/
2828

29+
// Ambient `process` for the env reads below — the showcase tsconfig doesn't pull
30+
// in `@types/node`, and the package-global shim in test/node-shim.d.ts declares
31+
// only `cwd()`. Same idiom (and same reason) as the declarations in
32+
// objectstack.config.ts and src/system/self-url.ts: keeps `pnpm typecheck` green
33+
// without widening the type surface. Playwright provides the real `process`.
34+
declare const process: { env: Record<string, string | undefined> };
35+
2936
const APP = process.env.SHOWCASE_APP || 'com.example.showcase';
3037
const API = process.env.SMOKE_API_URL || 'http://localhost:3000';
3138
const recordUrl = (object: string, id: string) =>

examples/app-showcase/e2e/showcase-smoke.spec.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ import { test, expect } from '@playwright/test';
66
* / collapsed chart). Runs against the console the backend serves at /_console
77
* (baseURL set in playwright.config.ts). Non-blocking nightly + manual.
88
*/
9+
10+
// Ambient `process` for the env read below — the showcase tsconfig doesn't pull
11+
// in `@types/node`, and the package-global shim in test/node-shim.d.ts declares
12+
// only `cwd()`. Same idiom (and same reason) as the declarations in
13+
// objectstack.config.ts and src/system/self-url.ts: keeps `pnpm typecheck` green
14+
// without widening the type surface. Playwright provides the real `process`.
15+
declare const process: { env: Record<string, string | undefined> };
16+
917
const APP = process.env.SHOWCASE_APP || 'com.example.showcase';
1018
const base = (seg: string) => `/_console/apps/${APP}/${seg}`;
1119

examples/app-showcase/tsconfig.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,19 @@
1010
"outDir": "./dist",
1111
"rootDir": "."
1212
},
13-
"include": ["src/**/*", "objectstack.config.ts", "test/**/*"]
13+
// `e2e/**/*.spec.ts`, NOT `e2e/**/*` (#7923). This package took the widened-
14+
// `include` route rather than a sibling `tsconfig.test.json` because its
15+
// `rootDir` is already the package root, so nothing here needs neutralising —
16+
// the same repair #7312 applied to app-crm / app-todo.
17+
//
18+
// The glob is spec-scoped on purpose, and it is the trap the TEST_DEBT entry
19+
// that this change deletes was written to warn about: `e2e/**/*` would also
20+
// pull in `e2e/global-setup.ts`, a Playwright fixture rather than a test,
21+
// which carries 6 errors of its own (3 `process.env` reads plus 3 gaps in
22+
// test/node-shim.d.ts — no `mkdirSync`/`writeFileSync` on the `node:fs` shim
23+
// and no `node:path` module at all). Billing the test layer for a non-test
24+
// file is what the ledger note measured around, so the repair holds the same
25+
// line. `global-setup.ts` therefore stays outside this program; it is filed
26+
// separately rather than silently folded in here.
27+
"include": ["src/**/*", "objectstack.config.ts", "test/**/*", "e2e/**/*.spec.ts"]
1428
}

packages/metadata-fs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"clean": "rm -rf dist",
2525
"test": "vitest run",
2626
"test:watch": "vitest",
27-
"typecheck": "tsc --noEmit"
27+
"typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.test.json"
2828
},
2929
"keywords": [
3030
"objectstack",

packages/metadata-fs/test/contract.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import { describe, it, beforeEach, afterEach, expect } from 'vitest';
3+
// `describe`/`it`/`beforeEach`/`expect` are NOT imported here on purpose: this
4+
// file registers the shared contract suite, which declares every case of its
5+
// own, so the only hook this file itself uses is `afterEach` for cleanup.
6+
import { afterEach } from 'vitest';
47
import fs from 'node:fs/promises';
58
import path from 'node:path';
69
import os from 'node:os';
710
import { runRepositoryContractTests } from '@objectstack/metadata-core/testing';
8-
import type { MetaRef } from '@objectstack/metadata-core';
911
import { FileSystemRepository } from '../src/index.js';
1012

1113
/** Track repos + tmpdirs across the contract suite for cleanup. */

packages/metadata-fs/test/watch-write-registration.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,27 @@ describe('FileSystemRepository watcher — writes register their own path (#7282
128128
const iter = repo.watch({ org: 'system' }, 999)[Symbol.asyncIterator]();
129129
const events: MetadataEvent[] = [];
130130
let resolveNext: (() => void) | null = null;
131-
void (async () => {
131+
const nextEvent = () => new Promise<void>((res) => { resolveNext = res; });
132+
// The drain loop is bound to a name and then called, rather than written as
133+
// an immediately-invoked `void (async () => { … })()`. That is load-bearing
134+
// for tsc, not style: for an IIFE, control-flow analysis narrows a captured
135+
// `let` to its type AT THE POINT OF THE CALL — here `null`, from the
136+
// initialiser above — so `resolveNext?.()` narrowed to `never` and reported
137+
// TS2349 (one of the six errors this package's test layer was hiding until
138+
// #7923 put it in front of tsc). A function expression that is not
139+
// immediately invoked cannot be assumed to run at any particular point, so
140+
// the declared `(() => void) | null` survives and the optional call is
141+
// checked against what it can actually be. Runtime behaviour is identical:
142+
// the loop still starts here, unawaited.
143+
const drainEvents = async () => {
132144
for (;;) {
133145
const next = await iter.next();
134146
if (next.done) return;
135147
events.push(next.value as MetadataEvent);
136148
resolveNext?.();
137149
}
138-
})();
139-
const nextEvent = () => new Promise<void>((res) => { resolveNext = res; });
150+
};
151+
void drainEvents();
140152

141153
await Promise.race([scanned, sleep(EVENT_WAIT_MS)]);
142154
// The walk reached the type directory, so what follows is measuring the
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// The TEST-layer type-check program (#7923, via the mechanism #5286/PR #5478 set
2+
// for `packages/spec` and PR #5546 / #5476 carried to `packages/client` and
3+
// `packages/metadata-core`). `tsconfig.json` beside this one stays as it is: it
4+
// is the BUILD config, and `package.json`'s `typecheck` script NAMES this sibling
5+
// (`tsc --noEmit -p tsconfig.test.json`), because a config no script invokes is
6+
// exactly the phantom that mechanism exists to prevent.
7+
//
8+
// THE HOLE HERE IS THE INCLUDE-SHAPED ONE, structurally identical to
9+
// `packages/metadata-core`: nothing was ever excluded — `include` is
10+
// `["src/**/*"]` and all six test files live under a sibling `test/` tree,
11+
// outside that root. No `exclude` entry names them, so the pre-#7353
12+
// exclude-shaped detector never saw them either; #7353 taught TESTS_COVERED the
13+
// include-shaped form and this package surfaced with 6 hidden files.
14+
//
15+
// What differs from the build config, and what deliberately does NOT:
16+
// - `rootDir` widens from `src` to the package root. It steers emit layout
17+
// only, and this program emits nothing (`noEmit`), but inherited as `src` it
18+
// reports TS6059 ("not under rootDir") for all six `test/**` files — the
19+
// check being misconfigured, not the tests being wrong. Widening it in the
20+
// BUILD config instead is not an option: `tsc` there emits (`dev`:
21+
// `tsc --watch`, `outDir: dist`), so a package-root `rootDir` would relocate
22+
// `dist/index.js` to `dist/src/index.js` — breaking `main`/`exports` — and
23+
// start writing `dist/test/**/*.test.js`, which ci.yml gates against ("No
24+
// compiled test files in any dist"). Emit constraints belong to the build
25+
// config; this one has none.
26+
// - MODULE SEMANTICS ARE UNTOUCHED. `@objectstack/metadata-core` is `"type":
27+
// "module"` and so is this package, so the inherited NodeNext already reads
28+
// these files as ESM — and it is the stricter reading, since it holds the
29+
// `.js` import extensions this package must ship. Nothing to fix, so nothing
30+
// is changed.
31+
// - STRICTNESS IS UNTOUCHED. `strict`, `noUnusedLocals`, `noUnusedParameters`,
32+
// `noImplicitReturns` and the rest are inherited from the root config.
33+
// Nothing here may loosen a type rule; if a test does not compile, that is
34+
// the finding. The five TS6133 this package's TEST_DEBT entry recorded were
35+
// `noUnusedLocals` findings and were fixed in the tests, not silenced here.
36+
//
37+
// There is NO `test-typecheck-debt.json` beside this config, on purpose — same
38+
// reasoning as `packages/metadata-core`: the whole test layer compiles at ZERO
39+
// errors under it, so a per-file shrink-only ledger would hold nothing while
40+
// costing this package a `tsx` dependency and two more scripts. A bare
41+
// `tsc --noEmit -p tsconfig.test.json` is the strictly stronger gate at zero
42+
// residue: ANY error here is red immediately, with no ledger to be added to.
43+
{
44+
"extends": "./tsconfig.json",
45+
"compilerOptions": {
46+
"noEmit": true,
47+
"rootDir": "."
48+
},
49+
"include": ["src/**/*", "test/**/*"],
50+
"exclude": ["node_modules", "dist"]
51+
}

scripts/check-type-check-coverage.mjs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,13 +474,34 @@ const EXEMPT = {
474474
// re-ratcheted.
475475
//
476476
// THE THREE ENTRIES THAT ARRIVED WITH #7353 -- cli 188, metadata-fs 6,
477-
// example-showcase 4 -- are not new debt and did not slip past a ledger that is
477+
// example-showcase 4 -- were not new debt and did not slip past a ledger that is
478478
// closed to it. They are debt this gate had never been able to SEE: TESTS_COVERED
479479
// asked whether an `exclude` named the tests, so a package that had simply never
480480
// pointed `include` at its test tree answered "covered" while nothing compiled a
481481
// line of it. All three were in that state before this ledger existed. 198 raw
482482
// errors is what the blind spot was worth on the day it was measured, and the
483483
// only thing that changed to surface them is the question.
484+
//
485+
// TWO OF THE THREE HAVE SINCE GRADUATED (#7923), which is the point of a TEST_DEBT
486+
// entry: it is a holding position that makes a layer ratchet, not a destination.
487+
// Both were re-measured on the merged ref before repair and both matched their
488+
// recorded numbers exactly (metadata-fs 6, example-showcase 4), so the entries
489+
// were deleted against measurements rather than against hope.
490+
// - `@objectstack/metadata-fs` took the sibling-config route, because its
491+
// `rootDir` is `src` and its `dev` script emits (`tsc --watch`, `outDir:
492+
// dist`): a package-root `rootDir` in the BUILD config would relocate
493+
// `dist/index.js` and start emitting compiled tests. `tsconfig.test.json`
494+
// beside it is named by the `typecheck` script -- the #5286 mechanism, the
495+
// same one `packages/metadata-core` uses for the structurally identical hole.
496+
// - `@objectstack/example-showcase` took the widened-`include` route (#7312's
497+
// shape for app-crm / app-todo), because its `rootDir` is already `.` and
498+
// nothing needed neutralising. Its glob is `e2e/**/*.spec.ts` and NOT
499+
// `e2e/**/*`, holding the same line the deleted entry's note drew: the
500+
// wholesale glob would pull in `e2e/global-setup.ts`, a fixture rather than a
501+
// test, and bill the test layer 6 errors that are not its own. That file is
502+
// still read by no tsc program and is filed rather than folded in here.
503+
// `@objectstack/cli` (188 raw across 56 files) is deliberately NOT part of that
504+
// graduation -- it is a programme rather than a sitting, and its entry stands.
484505
const TEST_DEBT = {
485506
'@objectstack/plugin-approvals': {
486507
errors: 547,
@@ -628,7 +649,6 @@ const TEST_DEBT = {
628649
'@objectstack/formula': { errors: 17, note: 'TS2591 x6 (`process`), TS2345 x3, TS2352 x3, TS1470 x2, TS2339 x2. Re-measured 17 at 5ab08428, up from 12; the TS2591 half doubled, which is the missing `types:["node"]` again rather than five new defects.' },
629650
'@objectstack/trigger-record-change': { errors: 9, note: 'TS2353 x9 -- still the one unknown-property shape repeated, now in four files. Re-measured 9 at 5ab08428, up from 8.' },
630651
'@objectstack/verify': { errors: 8, note: 'TS2835 x4, TS7006 x4. Re-measured 8 at 5ab08428, up from 6; both classes are the NodeNext pair from the top-of-ledger note.' },
631-
'@objectstack/metadata-fs': { errors: 6, note: 'TS6133 x5 (declared, never read -- the root config sets `noUnusedLocals`), TS2349 x1. Include-shaped: `include: ["src/**/*"]`, no `exclude` naming tests, and all 6 test files in a sibling `test/` tree, so this was invisible to the exclude-shaped detector (#7353). 5 of the 6 are in test/contract.test.ts; the TS2349 is in test/watch-write-registration.test.ts. Measured at b9f930b, recorded exactly.' },
632652
'@objectstack/connector-mcp': { errors: 5, note: 'TS2339 x5. Re-measured 5 at 5ab08428, exact.' },
633653
'@objectstack/connector-openapi': { errors: 5, note: 'TS2339 x5. Re-measured 5 at 5ab08428, exact.' },
634654
'@objectstack/http-conformance': {
@@ -639,7 +659,6 @@ const TEST_DEBT = {
639659
+ 'this package\'s own code. Raw `tsc --noEmit` counts are what every number in these ledgers means, '
640660
+ 'so they are counted here rather than filtered out -- but they are not this package\'s debt to fix.',
641661
},
642-
'@objectstack/example-showcase': { errors: 4, note: 'TS2339 x4. The one entry here whose hidden files are Playwright specs rather than vitest tests: `include` names `src/**/*`, `objectstack.config.ts` and `test/**/*` -- so the vitest layer is compiled and the `e2e/` tree beside it is not. Three specs, 4 errors (detail-shapes.spec.ts x2, bulk-capability-gate.spec.ts x1, showcase-smoke.spec.ts x1). Measured by adding the three spec files to `include` one at a time, NOT `e2e/**/*`, which would also have pulled in e2e/global-setup.ts and billed this layer 6 errors from a file that is not a test. Sibling to #7312, which repaired app-crm and app-todo the same way and could not move this gate\'s count because neither app had ever counted toward it.' },
643662
'@objectstack/platform-objects': { errors: 3, note: 'TS2339 x2, TS7006 x1. Re-measured 3 at 5ab08428, exact.' },
644663
'@objectstack/plugin-sharing': { errors: 3, note: 'TS6133 x2, TS18048 x1. Re-measured 3 at 5ab08428, exact.' },
645664
'@objectstack/service-sms': { errors: 1, note: 'TS2493 x1, in transports.test.ts. Re-measured 1 at 5ab08428 and still 1 at e8db1a230, after two more hidden test files: #5773 added sms-manifest-providers.contract.test.ts and #2814 / PR #6042 added sms-daily-quota.test.ts. The file count moved twice while the error count did not -- both new files are type-clean with the exclusion lifted.' },

0 commit comments

Comments
 (0)