Skip to content

Commit 7d090d1

Browse files
authored
ci: ratchet type errors in apps/sim test files (#8959)
* ci: ratchet type errors in apps/sim unit tests per file apps/sim's tsconfig excludes *.test.ts(x), so ~3.4k test files are never type-checked and any new test can add errors. check:test-types runs the native tsc on apps/sim/tsconfig.tests.json (the app config with tests left in), counts errors per file, and compares with a shrink-only baseline, matching check:explicit-any's semantics: a rise or a new file fails, a drop fails until --update shrinks it, --update never raises, --update --init only creates a missing baseline. Output it cannot place (global diagnostics, a crash) fails outright instead of reading as fewer errors. It type-checks the whole app, so it runs in its own test-types job gated like test (wired into ci-gate.sh), not in check:audits. On failure the job uploads counts.json and the compiler output; --counts <file> lets the baseline be bootstrapped or shrunk from that artifact without a local compile. * ci: commit the test type-error baseline from CI and drop the one-time --init path Baseline from run 38342772080's test-types counts: 3209 errors in 874 files. --update stays shrink-only; a missing baseline is restored from git. * ci: ratchet test type errors per file and TS code, with command-level tests A per-file count let one new error replace a fixed one with the total flat. The baseline is now { file: { TSnnnn: count } }: any (file, code) count that rises fails, including a new code in a baselined file, and --update stays shrink-only per pair. The key stops at the code: line and column move with edits, and messages print types that change with unrelated source or compiler versions. --diagnostics <file> replaces --counts: it reads a saved compiler output (the CI artifact's diagnostics.txt) through the same parser and undercount checks, so a fix can shrink the baseline without a local compile. The baseline was migrated from run 38342772080's diagnostics.txt with the script's parser: 3209 errors in 874 files across 52 codes, every per-file total equal to the previous baseline's. Command-level tests run the real script on fixture diagnostics: unchanged, a rising count, a new code replacing a fixed error, a new file, a dropped count, --update refusing a raise, --update shrinking, and unparseable or inconsistent output. The checks.yml comment now describes the artifact as input for shrinking the existing baseline. * ci: verify saved test-type diagnostics against the compile's exit status --diagnostics read an empty or truncated diagnostics.txt as fewer errors, so a failed compile's artifact plus --update could erase the baseline. --report now writes status.json (exit code and printed error count) once the compile's output and exit code agree, and --diagnostics requires it: exit 0 must mean no errors, the parse must match the recorded count, and a missing or malformed status fails. Command-level tests cover each case and that --update never writes the baseline from such input. Also passes a route context to the socket-token route test's POST call, the one new test type error (TS2554) the ratchet caught after #8958. * ci: read the saved compiler status through toRecord
1 parent 18b1750 commit 7d090d1

12 files changed

Lines changed: 3822 additions & 10 deletions

File tree

‎.github/scripts/ci-changes.sh‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
# diff sets all of them.
77
#
88
# full integration, e2e, e2e-build, e2e-mobile: any file not allowlisted below
9-
# test the Vitest shards: content, docs app, and desktop changes, plus `full`
9+
# test the Vitest shards and test-types: content, docs app, and desktop changes,
10+
# plus `full`
1011
# build the app's `next build`: content changes, plus `full`
1112
# desktop_live the live Electron suite: any file outside its own skip list
1213
#

‎.github/scripts/ci-gate.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
# A skip passes only for a gated job whose `changes` flag is exactly `false`:
77
#
88
# integration, e2e, e2e-build, e2e-mobile full
9-
# test test
9+
# test, test-types test
1010
# build build
1111
# desktop-live desktop_live
1212
#
@@ -25,7 +25,7 @@ fi
2525

2626
failed="$(jq -r '
2727
{"integration": "full", "e2e": "full", "e2e-build": "full", "e2e-mobile": "full", "test": "test",
28-
"build": "build", "desktop-live": "desktop_live"} as $gate
28+
"test-types": "test", "build": "build", "desktop-live": "desktop_live"} as $gate
2929
| (.changes.outputs // {}) as $flags
3030
| to_entries[]
3131
| select(.value.result != "success")

‎.github/workflows/checks.yml‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,9 +653,9 @@ jobs:
653653
# Every workspace, not just realtime. packages/emcn, packages/utils,
654654
# apps/desktop and apps/docs had no type check in CI at all; apps/sim's
655655
# source was covered only as a side effect of `next build` in the separate
656-
# `build` job. Note this does NOT cover apps/sim's tests — its tsconfig
657-
# excludes *.test.ts(x), and including them today surfaces ~2.2k errors,
658-
# so that is its own cleanup rather than a gate to switch on here.
656+
# `build` job. This does NOT cover apps/sim's tests — its tsconfig
657+
# excludes *.test.ts(x), which carry ~2.2k errors; the `test-types` job
658+
# ratchets those per file instead.
659659
- name: Type-check all workspaces
660660
run: bunx turbo run type-check
661661

@@ -672,6 +672,41 @@ jobs:
672672
fi
673673
echo "✅ Schema and migrations are in sync"
674674
675+
# Type errors in apps/sim's unit tests, ratcheted per file and TS code against a committed
676+
# baseline (scripts/check-test-types.ts). It type-checks the whole app with its ~3.4k test files:
677+
# lint's uncached apps/sim type-check alone takes ~35 s on this runner, so it gets its own job
678+
# beside lint rather than a place in check:audits, which also runs on laptops. Gated like `test`.
679+
# On failure the compiler output and its exit status are uploaded, so a fix can shrink the
680+
# existing baseline from them (`--diagnostics <file> --update`) without a local compile; nothing
681+
# ever raises the baseline.
682+
test-types:
683+
name: test-types
684+
needs: changes
685+
if: needs.changes.outputs.test == 'true'
686+
runs-on: *runner-4vcpu
687+
timeout-minutes: 10
688+
689+
steps:
690+
- *checkout-mirror
691+
- *checkout-plain
692+
693+
- name: Setup workspace
694+
uses: ./.github/actions/setup
695+
with:
696+
provider: ${{ vars.CI_PROVIDER }}
697+
698+
- name: Ratchet type errors in apps/sim tests
699+
run: bun run scripts/check-test-types.ts --report "$RUNNER_TEMP/test-types"
700+
701+
- name: Upload compiler output and counts
702+
if: failure()
703+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
704+
with:
705+
name: test-types
706+
path: ${{ runner.temp }}/test-types/
707+
if-no-files-found: ignore
708+
retention-days: 14
709+
675710
# The root scripts and every workspace's Vitest suite (`bun run test`), split in two. apps/sim is
676711
# about 98% of the time, so only its files are sharded; shard 1 also runs the root scripts and
677712
# the other workspaces. Each shard keeps its own Turbo cache: pass-through args are part of the
@@ -876,7 +911,8 @@ jobs:
876911
# on a cancelled run would report a cancelled head commit as passing.
877912
ci:
878913
name: ci
879-
needs: [changes, integration, e2e, e2e-mobile, desktop-live, lint, test, build, e2e-build]
914+
needs:
915+
[changes, integration, e2e, e2e-mobile, desktop-live, lint, test-types, test, build, e2e-build]
880916
if: ${{ always() }}
881917
runs-on: *runner-2vcpu
882918
timeout-minutes: 5

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ This file (also `AGENTS.md`) holds the repo-wide rules. Area detail lives in `.c
2222
- `compareStrings(left, right)` from `@sim/utils/string` — code-unit ordering for hashes, fingerprints, and cross-process comparisons; never `localeCompare` there
2323
- `backoffWithJitter(attempt, retryAfterMs, options?)` / `parseRetryAfter(header)` from `@sim/utils/retry` — never reimplement exponential backoff inline
2424
- **Deployment flags in the browser**: client code inside a workspace, organization, or standalone settings surface reads `hosted`, `billingEnabled`, `chatEnabled`, and the enterprise feature set through `useDeploymentShape()` (components) or `getDeploymentShape()` (block conditions, stores, helpers) from `@/lib/core/config/deployment-shape`, never `isHosted`/`isBillingEnabled`/... from `env-flags`. Those constants freeze at module init from the root layout's `NEXT_PUBLIC_*` transport, which Next's bare 404 shell and `global-error` never emit, so a recovered tab would render Sim Cloud as self-hosted; the reader is seeded from the server-resolved workspace host context, organization layout, or standalone settings layout instead. Server code keeps reading `env-flags`.
25-
- **Type-checking**: `bun run type-check` at the root checks every workspace (`bun run type-check` inside a workspace checks just that one). `apps/sim`'s tsconfig excludes `*.test.ts(x)`, so its tests are not type-checked. Never remove the `@typescript/native` alias from the root `devDependencies`: nothing imports it, but without it a bare `tsc` resolves to the ~10x slower TypeScript 6 compiler. `bun run check:native-typecheck` enforces this; its header explains the bin-resolution trap.
25+
- **Type-checking**: `bun run type-check` at the root checks every workspace (`bun run type-check` inside a workspace checks just that one). `apps/sim`'s tsconfig excludes `*.test.ts(x)`, so `type-check` skips its tests; CI's `test-types` job ratchets their errors per file and TS code instead (`check:test-types`, whose header says how to fix a finding): a test file may not gain one. Never remove the `@typescript/native` alias from the root `devDependencies`: nothing imports it, but without it a bare `tsc` resolves to the ~10x slower TypeScript 6 compiler. `bun run check:native-typecheck` enforces this; its header explains the bin-resolution trap.
2626

2727
## Architecture
2828

‎apps/sim/app/api/auth/socket-token/route.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ const IP_BACKSTOP = 300
4141
async function mintAs(userId: string | null, ip: string) {
4242
requestUtilsMockFns.mockGetClientIp.mockReturnValue(ip)
4343
authMockFns.mockGetSession.mockResolvedValue(userId ? { user: { id: userId } } : null)
44-
return POST(createMockRequest('POST'))
44+
return POST(createMockRequest('POST'), { params: Promise.resolve({}) })
4545
}
4646

4747
describe('POST /api/auth/socket-token', () => {

‎apps/sim/tsconfig.tests.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"extends": "./tsconfig.json",
3+
"exclude": ["node_modules", ".next", "vitest.config.ts", "vitest.setup.ts"]
4+
}

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
"check:unused-exports": "bun run scripts/check-unused-exports.ts",
8787
"check:explicit-any": "bun run scripts/check-explicit-any.ts",
8888
"check:file-names": "bun run scripts/check-file-names.ts",
89+
"check:test-types": "bun run scripts/check-test-types.ts",
8990
"desktop-bridge-contract:update": "bun run scripts/check-desktop-bridge-contract.ts --update",
9091
"mship-contracts:generate": "bun run scripts/sync-mothership-stream-contract.ts",
9192
"mship-contracts:check": "bun run scripts/sync-mothership-stream-contract.ts --check",

0 commit comments

Comments
 (0)