fix(core): declare the dist-only test target in each package's jest config

[igorlukanin]

Issue

Thirteen packages test their compiled output, but only their unit script said so. The script's path argument — jest dist/test/unit — was the only thing expressing it; the jest config didn't.

jest.base.config.js declares no preset, no transform and no testMatch, so each of these packages inherited jest's default testMatch, which collects the TypeScript under test/ that nothing can transform. A bare jest in any of them picks up sources that die on import:

$ cd packages/cubejs-ksql-driver && npx jest
FAIL test/unit/params-escaping.test.ts
  ● Test suite failed to run
    Jest encountered an unexpected token

It collects dist/test/unit/params-escaping.test.js and test/unit/params-escaping.test.ts, and the second one cannot run.

The asymmetry in the base config is the tell: collectCoverageFrom is already ['dist/src/**/*.js', 'dist/src/**/*.ts']. It knows it operates on dist for coverage, then omits it for collection. The sibling jest.base-ts.config.js does declare its intent (testMatch: ['<rootDir>/test/**/*.test.ts']) right next to the preset and transform that make it work.

The consequence is that a test file can stop being collected and nobody notices, because the script's path filter papers over whatever the config says.

Fix

Each package declares its own testMatch pointing into dist/:

testMatch: ['<rootDir>/dist/test/**/*.{test,spec}.{ts,js}']

Nine packages already had a jest.config.js and needed only that line. Four had none — cubejs-athena-driver, cubejs-crate-driver, cubejs-jdbc-driver, cubejs-trino-driver — so they had nothing to inherit a fix into; they get a config, and the now-redundant jest: { testEnvironment: 'node' } key comes out of their package.json (the base config supplies testEnvironment).

.spec. is in the pattern because jest's default testMatch accepted it, and pinning this must not quietly drop a suite.

Why the pattern is the whole dist/test tree, not dist/test/unit

Pinning each config to the narrower path its unit script names looks tighter, and it is wrong: eight of these packages also have integration scripts pointing at dist/test/integration, and a testMatch of dist/test/unit makes those collect nothing.

A path argument is a filter applied within testMatch, not a replacement for it, so the broad pattern keeps both scripts working:

invocation	collects
jest dist/test/unit	the unit suites only
jest dist/test/integration	the integration suite only
jest	every compiled suite, and no untransformable source

testPathIgnorePatterns: ['/dist/test/integration/'] is the obvious alternative and does not work: it overrides the path argument, so jest dist/test/integration collects zero. cubejs-backend-native uses that key for its bridge tests, but only because those have a separate config file (jest-bridge.config.js) to run under.

The four packages in the repo that already pin a dist target use a single-star form (dist/test/*.{test,spec}.{ts,js}). That form is not transferable here — ten of these thirteen keep their suites in subdirectories, and applying it to cubejs-schema-compiler collects 0 of its 35 suites. Hence the double star.

Verification

Per package: built with yarn tsc, then compared what each script collects before and after, and ran the suite.

package	suites collected by unit, before → after	result
cubejs-athena-driver	1 → 1	10 tests pass
cubejs-backend-cloud	4 → 4	9 tests pass
cubejs-cli	2 → 2	4 tests pass
cubejs-clickhouse-driver	1 → 1	2 tests pass
cubejs-crate-driver	1 → 1	8 tests pass
cubejs-dremio-driver	1 → 1	8 tests pass
cubejs-jdbc-driver	1 → 1	3 tests pass
cubejs-ksql-driver	1 → 1	7 tests pass
cubejs-pinot-driver	1 → 1	8 tests pass
cubejs-prestodb-driver	2 → 2	9 tests pass
cubejs-schema-compiler	35 → 35	678 tests, 110 snapshots pass
cubejs-server-core	5 → 5	89 tests pass
cubejs-trino-driver	2 → 2	4 tests pass

Every count is unchanged, so no suite was dropped or gained. Additionally checked:

• All 15 integration* script variants still collect what they did (including integration:mssql / :mysql / :postgres / :clickhouse in cubejs-schema-compiler, and integration:athena / :crate / :dremio / :pinot / :presto / :trino). This is the regression the narrow pattern would have caused.
• Bare jest now collects zero non-dist files in all thirteen — the reported bug.
• CI is unaffected: yarn lerna run … unit still passes each script's own path argument, and every one of the thirteen carries one.
• .d.ts output is not collected — foo.test.d.ts has .d between .test. and the extension, so it does not match the pattern.
• Helper files under dist/test that aren't suites (global-setup.js, setup.js, snapshotResolver.js) are not collected.

Live-infrastructure suites keep their existing exclusions. cubejs-athena-driver, cubejs-dremio-driver, cubejs-pinot-driver and cubejs-crate-driver each hold test files outside the path their unit script names — AthenaDriver.test.ts, DremioQuery.test.ts, Pinot.test.ts, CrateDriver.test.ts and friends. Those look like the same defect but are not: every one drives testcontainers, Docker Compose or a real engine, and simply lives outside a test/integration/ folder. Pinning to the script's path preserves that.

What the four new config files also pick up

cubejs-athena-driver, cubejs-crate-driver and cubejs-jdbc-driver previously ran on package.json#jest: { testEnvironment: 'node' }, and cubejs-trino-driver on jest's bare defaults — none of the four loaded jest.base.config.js at all. Extending it now also brings in collectCoverage, coverageDirectory/coverageReporters, moduleNameMapper, setupFiles and snapshotFormat, so ...base is not inert in these four the way it is in the other nine.

Checked the ones that could bite: none of the four has a __snapshots__ directory, so snapshotFormat can't invalidate committed snapshots; **/coverage is already gitignored, so the new output stays untracked; and jest.setup.js is only a globalThis.crypto polyfill, itself a no-op on Node 18+. The one live effect is that yarn unit in these four now runs coverage instrumentation and prints a table where it didn't before. That's the intended uniformity, on suites of 3-10 tests each.

Not in this PR

cubejs-api-gateway has the identical gap; it is fixed in #11482, which touches the same line of the same file. Leaving it there avoids a conflict.

The convention this documents still isn't enforced, and six other packages (cubejs-bigquery-driver, cubejs-druid-driver, cubejs-duckdb-driver, cubejs-mysql-driver, cubejs-databricks-jdbc-driver, cubejs-backend-maven) have the same gap. Tracked separately in CORE-739.

[claude]

Claude finished @igorlukanin's task in 4m 9s —— View job

---

Approach is right and the diff is correct — 0 blocking, 3 non-blocking (1 medium, 2 low)

Full review — verification of the PR's claims, 3 inline comments

• Read the PR diff
• Check for stale review threads (none exist)
• Verify each package's test layout matches the new testMatch
• Check unit/integration scripts still resolve
• Post review

Verdict

The diagnosis is accurate and the fix is the right shape. The reasoning in the PR body about why not to hoist testMatch into jest.base.config.js, and why not to use testPathIgnorePatterns, both hold up under checking. No blocking issues.

What I verified independently

The narrower pattern doesn't silently drop anything. Replacing jest's default testMatch with dist/test/**/*.{test,spec}.{ts,js} gives up three things the default accepted, so I checked each against the actual sources in all thirteen packages:

the default also accepted	present in any of the 13?
**/__tests__/**/*.[jt]s?(x) — any file under a __tests__ dir, no .test. needed	none — no __tests__ directories
.tsx / .jsx extensions	none
plain-JS suites (test/foo.test.js) that tsc wouldn't copy to dist	none — every suite source is .ts

So the "111 additions, no suite lost" claim checks out, not just by count but by mechanism.

Every one of the thirteen unit scripts does carry a path argument (confirmed via package.json), so the CI-unaffected claim holds. The non-obvious one is cubejs-crate-driver, whose unit names a single file — jest --forceExit --verbose dist/test/CrateDriver.unit.test.js. That file lives at the top of dist/test, not in a subdirectory, and dist/test/**/*.test.js still matches it because ** matches zero path segments. Worth knowing that's load-bearing.

The anti-hoisting argument is real, not theoretical. cubejs-bigquery-driver, cubejs-druid-driver, cubejs-duckdb-driver and cubejs-mysql-driver all extend the base config and have only integration:* scripts pointing at dist/test. A hoisted testMatch would convert a bare jest in those from "fails loudly on import" into "silently attempts a live warehouse connection" — strictly worse than the bug being fixed. Good call.

The two comment variants are applied consistently. The unit / integration wording appears exactly on the eight packages that have an integration script; the "no transform" wording on the five that don't. That's a detail that's easy to get wrong across thirteen files.

Non-blocking feedback (3 inline comments)

1. jest.base.config.js (medium) — the new comment documents a convention nothing enforces, and the PR body names only cubejs-api-gateway as remaining. There are ~9 more packages extending this file with no dist-pointing testMatch. Also suggests exporting the pattern rather than writing it out 13 times.
2. packages/cubejs-athena-driver/jest.config.js (low) — the four packages gaining a config file inherit more than testMatch: coverage collection, moduleNameMapper, setupFiles, snapshotFormat. I checked the ones that could break (no committed snapshots, **/coverage already gitignored, jest.setup.js is a harmless polyfill) — it's safe, but it's an undocumented scope expansion.
3. packages/cubejs-dremio-driver/jest.config.js (low, pre-existing) — setupFiles overrides rather than spreads the base's, unlike moduleNameMapper two lines below.

Nothing to report on

Security, performance, error handling and input sanitization are all out of scope for a change that only touches jest configuration. No test coverage to add — the change is about which tests get collected, and the PR's before/after suite counts are the appropriate evidence. Docs need no update; the base config's new header comment is the right place for this explanation and is well written.

What I could not check

I reviewed statically and did not run yarn tsc + the thirteen suites myself — the sandbox blocked the commands needed to enumerate dist/ and exercise jest's resolver directly. The claims above are verified against sources, package.json scripts and glob semantics; the empirical suite counts in the PR body are unverified by me. CI on this PR covers that.

· branch `igor/core-736-dist-only-test-target-in-jest-config`

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.59%. Comparing base (f7ff67a) to head (b4d4083).

❗ There is a different number of reports uploaded between BASE (f7ff67a) and HEAD (b4d4083). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (f7ff67a)	HEAD (b4d4083)
cubesql	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #11484       +/-   ##
===========================================
- Coverage   79.46%   58.59%   -20.88%     
===========================================
  Files         480      233      -247     
  Lines       98810    18358    -80452     
  Branches     3636     3745      +109     
===========================================
- Hits        78515    10756    -67759     
+ Misses      19774     7071    -12703     
- Partials      521      531       +10     

Flag	Coverage Δ
cube-backend	58.59% <ø> (-0.52%)	⬇️
cubesql	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @igorlukanin's task in 1m 22s —— View job

---

New commit addresses the dremio nit correctly — no new findings; 2 prior threads still open (both non-blocking)

Incremental review of `8d9004b`

• Read the incremental diff since the last review
• Check existing review threads, resolve mine that no longer apply
• Verify the new dremio setupFiles change
• Post review

What changed since the last review

One commit, one line: 8d9004b — fix(dremio-driver): extend the base setupFiles instead of replacing them. It applies the suggestion from my third inline comment verbatim.

setupFiles: [
  ...base.setupFiles,
  './test/test-env.js'
],

Verification

• The spread resolves to the right file. base.setupFiles is ['../../jest.setup.js'] — a rootDir-relative path. packages/cubejs-dremio-driver/jest.config.js sets rootDir: '.', so it resolves to the repo-root jest.setup.js, which is the four-line globalThis.crypto polyfill. Same resolution the other nine packages already get.
• Order is right. The polyfill runs before ./test/test-env.js, and test-env.js only copies DRIVERS_TESTS_DREMIO_* env vars onto their unprefixed names — no dependency on crypto, so order is inert either way, but base-first matches the convention in moduleNameMapper two lines down.
• No other package has the same defect. Only three configs under packages/* touch setupFiles at all: cubejs-backend-native (its own standalone config, not extending the base), cubejs-dremio-driver (now fixed), and cubejs-server-core — which sets setupFilesAfterEnv, a different key, so the base's setupFiles survives its spread untouched. Nothing else to fix.
• The testMatch change from the first commit is unchanged, so everything in my previous review still stands — the diagnosis, the anti-hoisting argument, and the thirteen packages' before/after suite counts.

Thread status

thread	state
packages/cubejs-dremio-driver/jest.config.js:15 — setupFiles overrides instead of spreading	resolved — fixed by this commit
jest.base.config.js:15 — new comment documents an unenforced convention; ~9 more packages still lack a dist-pointing testMatch; pattern written out 13×, could be exported from the base	still open, still applies
packages/cubejs-athena-driver/jest.config.js:1 — the four new configs inherit coverage/moduleNameMapper/setupFiles/snapshotFormat, not just testMatch; worth a line in the PR body	still open, still applies

Both open threads are documentation/consistency observations, not defects. Neither blocks merge.

What I could not check

I reviewed statically again. The sandbox blocked npx jest --showConfig and the commands needed to enumerate dist/, so the setupFiles resolution above is reasoned from jest's rootDir-relative path semantics rather than observed. CI on this PR exercises it.

· branch `igor/core-736-dist-only-test-target-in-jest-config`