Skip to content

vitest runner#210

Closed
sroussey wants to merge 6 commits into
mainfrom
chore/vitest-runner
Closed

vitest runner#210
sroussey wants to merge 6 commits into
mainfrom
chore/vitest-runner

Conversation

@sroussey

Copy link
Copy Markdown
Contributor

No description provided.

sroussey and others added 6 commits July 22, 2026 15:42
A full bootstrap flooded with STORE_ERRORs that lost whole CIKs' facts
because the company-facts constraints were tighter than real EDGAR data:

- fp: EDGAR emits CY / H1 / H2 (calendar-year, half-year) beyond FY/Q1-Q4,
  and an empty string "" for period-agnostic facts. Widen the FP enum and
  add normalizeFp() to map "" / unknown codes to null at parse time (the
  storage boundary already coalesces null to the "" PK sentinel), so one
  odd fact can't fail a CIK's entire batch.
- val_unit: composite units run to ~20 chars; bump 12 -> 32.
- grouping: ifrs-full is 9 chars; bump 8/10 -> 20.
  (Existing Postgres company_facts columns widened separately via ALTER.)

Also fix `update facts --retry-failed`, which crashed with "Query criteria
must not be empty" — the all-CIK read used query({}, ...); switch to
getAll({ orderBy }). This path is only reached by `update facts`, not
`bootstrap`, so it stayed hidden until a retry run.

Adds regression tests for the widened fp/val_unit/grouping schemas,
normalizeFp, and the getAll retry-lane read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019ZKz3adGzMpEdy9jJ3HNXu
- xbrl_fact.context_ref overflowed varchar(128) on real filings (iXBRL
  context ids are long, esp. dimensional contexts). Widen schema to 512.
- Add widenNarrowColumns() migration, run at the end of setupAllDatabases:
  CREATE TABLE IF NOT EXISTS never alters an existing table, so a Postgres
  DB set up before the company-facts (val_unit/grouping) and XBRL
  (context_ref) widenings kept the narrow columns and silently re-hit the
  original overflow. The migration brings existing columns up to the schema
  width. Postgres-only (SQLite TEXT is unbounded); guarded to skip columns
  already wide enough, so it is a cheap idempotent no-op on a clean DB.
- Add context_ref width regression test (parity with the facts tests).

Addresses the review's MAJOR finding: the maxLength bump alone left the
fix incomplete on the very backend (Postgres) that exhibited the bug.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019ZKz3adGzMpEdy9jJ3HNXu
The test registers SEC_DRY_RUN=true in the global service registry to
exercise the dry-run retry lane, but resetDependencyInjectionsForTesting()
only re-registers repos — it never clears SEC_DRY_RUN. Under `bun test`
all files share one process, so the leaked flag poisoned later files: on
bun 1.3.14's file ordering it ran before SecFetchFileOutputCache.test.ts,
whose saveOutput early-returns under dry-run, failing all 5 of its
save-path assertions. Reset the flag to false in afterEach (matching
dryRunRouting/runCommand). Verified: full suite 1938 pass / 0 fail on
bun 1.3.14 in a Linux container (previously 5 fail).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019ZKz3adGzMpEdy9jJ3HNXu
Adds vitest.config.ts (env loading, 30s timeout, per-file isolation),
converts the 8 bun:test files to vitest, and replaces the Bun-only
import.meta.dir with a portable fileURLToPath form across 10 files.

Vitest isolates each test file, removing the cross-file global-state
leakage that `bun test` (one shared process) allowed. 256/265 files pass;
remaining CLI-subprocess / Bun.spawn / sqlite-repo files are in progress.
- Route the 5 CLI integration tests' subprocess spawning through a shared
  node:child_process helper (runCliProcess) instead of Bun.spawn; clean
  stdout/stderr separation on both runtimes (4/6 previously-failing CLI
  files now pass under vitest).
- Rewrite forceWarning's streaming spawnUntilWarning on node:child_process.
- Replace bun-only jest-extended matchers (.toBeFunction/.toBeString) with
  vitest's .toBeTypeOf in query + EntityObserver tests.

Remaining under vitest: version.test.ts (vitest-worker env leaks into the
spawned CLI and pollutes its stdout), BootstrapDownloadTask (app-level
Bun.spawn), EntityTemporalRepo, one Form_424 test.
- package.json test script bun test -> vitest run; CI runs `bun run test`.
- Curate the spawned CLI subprocess env (cliEnv) instead of spreading the
  whole process.env, so vitest-injected worker vars can't leak onto the
  child's stdout and corrupt --format json output.
- Replace bun-only `mock`/`spyOn` (imported from "vitest", which doesn't
  export them) with vitest's `vi.fn`/`vi.spyOn` across 6 files.

Full suite: 265/265 files, 1938/1938 tests pass under vitest on bun 1.3.14
(CI's runtime) in a Linux container. No production code changed — the app's
Bun.spawn/Bun.file/Bun.which run fine under vitest-on-bun.

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #211.

#211 replays this branch's net effect as its first commit and additionally adds the ESM extension-fixup plugin, pool: "forks", the expanded Bun-skip gates (BootstrapDownloadTask, SecFetchJob, index-fetch), TestingDI env-derived-token cleanup, and the PARSE_ERROR dead-letter containment in ProcessAccessionDocFormTask — all of which are needed for the suite to actually reach a green run under vitest. This branch is also 8 commits behind main (missing 0.0.13–0.0.16 releases plus the address-normalization and CanonicalCompanyRepo work).

No content in #210 is missing from #211.


Generated by Claude Code

@sroussey sroussey closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant