ReviewOS

also looking at this

stacks/bun-query-builder

test: run every test file in the package, not just the test directory

#1188
Merged glennmichael123 wants to merge test/ci-runs-every-test-file into main
2 files +95 -1
package.jsonmodified+1-1
Changes to package.json
@@ -62,7 +62,7 @@
6262 "zip:darwin-x64": "zip -j bin/qbx-darwin-x64.zip bin/qbx-darwin-x64",
6363 "zip:darwin-arm64": "zip -j bin/qbx-darwin-arm64.zip bin/qbx-darwin-arm64",
6464 "fresh": "bun x rimraf node_modules/ bun.lock && bun i",
65 "test": "bun run build && bun test --max-concurrency=1 packages/bun-query-builder/test",
65 "test": "bun run build && bun test --max-concurrency=1 packages/bun-query-builder",
6666 "lint": "bun x --bun pickier .",
6767 "lint:fix": "bun x --bun pickier . --fix",
6868 "format": "bun x --bun pickier . --format",
packages/bun-query-builder/test/suite-coverage.test.tsadded+94-0
Changes to packages/bun-query-builder/test/suite-coverage.test.ts
@@ -0,0 +1,94 @@
1/**
2 * Every test file in this package is reachable from the command CI runs.
3 *
4 * The `test` script hands bun an explicit path. That path was
5 * `packages/bun-query-builder/test`, so `src/__tests__/type-narrowing.test.ts`
6 * — 20 tests, 50 assertions — was never executed by CI. It passes, and it had
7 * passed all along for anyone running bare `bun test` from the repository root,
8 * which discovers the whole tree. CI's narrower path did not, and nothing said
9 * so: the run reported 216 files and a green tick, and 216 looks no different
10 * from 217 unless you already suspect the number.
11 *
12 * Only the runtime half was missing. tsconfig.json INCLUDES every source file
13 * under a package's `src`, so that file's types are checked by the typecheck
14 * job — and, inverted, it EXCLUDES everything under a package's `test`, so the
15 * directory holding every other test is not typechecked at all. The two halves
16 * cover opposite sides of the package, which is how a file could sit in the gap.
17 *
18 * So this asserts the invariant rather than the fix: a test file added anywhere
19 * under the package must be somewhere the `test` script will find it. Moving
20 * one into a new directory, or narrowing the script's path again, fails here
21 * instead of quietly dropping it from the only run that gates a merge.
22 */
23
24import { describe, expect, it } from 'bun:test'
25import { readdirSync, readFileSync } from 'node:fs'
26import { join, relative, resolve, sep } from 'node:path'
27
28const REPO = resolve(import.meta.dir, '../../..')
29const PACKAGE = resolve(import.meta.dir, '..')
30
31/**
32 * The paths the `test` script passes to bun, relative to the repository root.
33 *
34 * Read from package.json rather than hardcoded, so this tracks the script it
35 * is about: narrowing the path is the regression, and a copy of the path here
36 * would be narrowed along with it.
37 */
38function configuredTestPaths(): string[] {
39 const script = JSON.parse(readFileSync(join(REPO, 'package.json'), 'utf8')).scripts?.test as string | undefined
40 expect(script).toBeTypeOf('string')
41
42 // `bun run build && bun test --max-concurrency=1 <path>...` — take the
43 // segment that invokes the runner, then its non-flag arguments. A flag that
44 // takes a separate value would need handling here; the script uses `=`.
45 const invocation = script!.split('&&').map(part => part.trim()).find(part => /^bun\s+test\b/.test(part))
46 expect(invocation).toBeTypeOf('string')
47
48 return invocation!
49 .replace(/^bun\s+test\b/, '')
50 .split(/\s+/)
51 .filter(argument => argument.length > 0 && !argument.startsWith('-'))
52}
53
54function testFilesUnder(directory: string, found: string[] = []): string[] {
55 for (const entry of readdirSync(directory, { withFileTypes: true })) {
56 if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name.startsWith('.'))
57 continue
58 const path = join(directory, entry.name)
59 if (entry.isDirectory())
60 testFilesUnder(path, found)
61 else if (/\.test\.ts$/.test(entry.name))
62 found.push(path)
63 }
64 return found
65}
66
67describe('the test command CI runs', () => {
68 it('reaches every test file in the package', () => {
69 const paths = configuredTestPaths()
70
71 // No path at all means bun discovers the whole tree, which is the thing
72 // being asked for — nothing to check.
73 if (paths.length === 0)
74 return
75
76 const roots = paths.map(path => resolve(REPO, path))
77 const orphans = testFilesUnder(PACKAGE)
78 .filter(file => !roots.some(root => file === root || file.startsWith(root + sep)))
79 .map(file => relative(REPO, file))
80
81 // Named, not counted: the fix is to move the file or widen the script, and
82 // either way the person reading this needs to know which file.
83 expect(orphans).toEqual([])
84 })
85
86 it('finds the file that was missing, so this guard is wired to something real', () => {
87 // A guard whose discovery is broken reports zero orphans forever. This
88 // pins that the walk actually sees the file the incident was about.
89 const found = testFilesUnder(PACKAGE).map(file => relative(REPO, file))
90
91 expect(found).toContain('packages/bun-query-builder/src/__tests__/type-narrowing.test.ts')
92 expect(found).toContain('packages/bun-query-builder/test/suite-coverage.test.ts')
93 })
94})