mozilla / mozilla/pilo

core and server never typecheck their own test files

Open
#692 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
TypeScript
Stars
100
Forks
14
Avg merge
15d 7h
Merged PRs (30d)
13

Description

Summary

pilo-core and pilo-server never typecheck their own test files. Nothing is broken today — both are clean at baseline — but the tests are outside every typecheck gate, so type errors in them can't be caught by CI.

Split out of #688, where this surfaced while measuring flag costs.

Where it comes from

packages/core/tsconfig.json:

"include": ["src/**/*"],
"exclude": ["node_modules", "dist", "**/*.test.ts"]

packages/server/tsconfig.json has the same shape, and packages/server/tsconfig.check.json only adds noEmit: true — so it inherits the src-only include and the typecheck-specific config doesn't widen coverage.

Coverage today:

Package Test files Typechecked? How
core 42 (993 tests) no tsconfig.json excludes **/*.test.ts
server 2 (103 tests) no same exclusion; tsconfig.check.json doesn't widen include
cli 7 yes tsconfig.check.json sets include: ["src/**/*", "test/**/*"]
extension 12 + e2e yes WXT's generated config includes ../**/*

cli already demonstrates the fix: a tsconfig.check.json that widens include for typecheck-only, leaving the build config's rootDir/declaration settings alone.

Why it's worth fixing

The tests run under vitest, which transpiles without typechecking — so a test can reference a renamed export, pass wrong argument types, or assert against a stale shape, and nothing flags it as long as the assertions happen to pass at runtime. That's the same class of silent gap as the typo'd script name in #689: green CI that isn't checking what you assume.

Both packages are clean at baseline right now, so adopting this is cheap today and gets more expensive the longer it waits.

Caveat, and the link back to #688

Enabling this interacts with #688. Core's test files carry 145 noUncheckedIndexedAccess findings — measured — versus 14 in core/src. So:

  • Typechecking core's tests at current strictness: free (0 errors).
  • Typechecking them after adopting noUncheckedIndexedAccess: +145 sites.

Whichever of the two lands second pays that bill, which is an argument for deciding #688 with this in view rather than discovering it afterward.

Suggested shape

Add a tsconfig.check.json to core mirroring cli's, widen server's, and wire both into their typecheck scripts. Note core needs rootDir widened (or declaration disabled) in the check config — inheriting rootDir: "./src" while including test/ produces one TS6059 per test file, which is a config error rather than a type error and masks everything else. I hit exactly that while measuring.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Compare the existing packages/cli/tsconfig.check.json with packages/core/tsconfig.json and packages/server/tsconfig.json, then inspect the typecheck scripts in both packages. Run the package typechecks after widening test coverage, ensuring core avoids the rootDir/declaration issue; done means core and server test files are included and both checks pass with no baseline errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
build-system, testing-qa
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.