Skip to content

Test and Cypress scripts can silently run against stale sibling dist/ #1675

Description

@dqnykamp

Every cross-package @doenet/* import in this repo resolves to the built dist/ of the sibling package: each one exports "." → "./dist/index.js", and of the 46 tracked vite*.config.* / vitest.config.* / cypress.config.* files exactly one maps a @doenet/* specifier to source — packages/doenetml/cypress.config.ts, which aliases @doenet/codemirror (only that package) for its component-test dev server, for an unrelated reason it documents inline. The three @doenet/* tsconfig path mappings (tsconfig.build.json, tsconfig.test.json, packages/vscode-extension/tsconfig.json) all point at dist. So any npm script that reaches a sibling package and is not a wireit script with that package's build declared can run against stale output — and it does not fail, it just quietly uses old code.

#1672 fixed this for the three generator scripts in @doenet/static-assets (build:schema, build:assets, check:docs-coverage), where the symptom was a regenerated schema losing entries a branch had just added (hit for real on #1669's Klingon catalog). The same hazard is still live in the test scripts.

Where it still bites

  • Every test script whose specs reach a @doenet/* import. Most consequentially doenetml-worker-javascript's test / test:group1-4, plus doenetml, parser, utils, lsp, lsp-tools, doenetml-worker, v06-to-v07, static-assets. packages/utils/test/styleDescriptions.test.ts already carries a comment admitting the hazard and telling the reader to run npm run build -w @doenet/i18n by hand; a stale @doenet/utils is enough to make styleDescriptionLocale.test.ts fail on an otherwise-unmodified checkout.
  • The Cypress runners in test-cypress, codemirror, doenetml, doenetml-iframe, and the root test:e2e-group* / test:*-cypress wrappers. This is the hazard TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md's "rebuild before Cypress runs" rule exists to paper over.
  • packages/static-assets' own test, which reaches the worker source through scripts/get-schema.ts — the same import path Let the schema scripts build the packages they read #1672 was about.
  • packages/doenetml-to-pretext / doenetml-prototype test are partially guarded: their build:test1 is wireit and covers doenetml-worker + parser, but their src/ also imports @doenet/utils, @doenet/codemirror and @doenet/ui-components, which are not covered.

packages/memory-benchmark's benchmark is the exemplar of the right pattern already: a wireit script with ../standalone:build and ../doenetml-iframe:build declared.

Suggested fix shape: split watch and single-run entry points

The obvious move — make test a wireit script — breaks watch mode. Wireit spawns the command with piped stdio (wireit/lib/script-child-process.js passes no stdio option, so the default pipe applies), so the child sees no TTY and no stdin. These test scripts are a bare vitest, i.e. watch mode, whose interactive keypress UI would stop working — for the one command developers spend all day in. Wireit has no option to inherit stdio.

So instead:

  • leave test a plain vitest (watch, interactive, no wireit in the way);
  • add a wireit test:run per package: command: "vitest run", dependencies = the same sibling builds that package's own build declares, and no files/output so it always re-runs. Where a package's specs reach the worker's source (@doenet/static-assets' own test does), the dependency to declare is ../doenetml-worker-javascript:build:deps — the no-command aggregator Let the schema scripts build the packages they read #1672 added for exactly that, which avoids a vite pass over the worker's 7 MB bundle that nothing loads;
  • point CI and the root aggregate scripts at test:run. The root test and test:all-no-worker-js already append -- run, so they would just call test:run instead.

Notes for whoever picks this up:

  • Argument forwarding is not a blocker. npm run test:run -- <file> reaches the underlying command (wireit README, Extra arguments), and extra args are part of the fingerprint. On Node 24 passing args does make wireit emit a DEP0190 deprecation warning, because it spawns with shell: true and an args array — harmless, but it would show up on every targeted run.
  • CI cost is ~nil. Both test jobs already build against the downloaded .wireit cache before testing (test-main runs build:all-no-docs; test-worker-js runs build -w packages/doenetml-worker, which reaches the same packages), so the declared dependencies would be cache hits. WIREIT_CACHE has to be set explicitly in CI, though — wireit defaults it to "none" whenever CI=true.
  • Wireit's quiet logger passes the root script's output through, so test output is not swallowed.
  • The Cypress runners are a bigger change than adding dependencies: packages/test-cypress/vite.config.ts hard-codes ../standalone/dist/*, so the wiring has to grow ../standalone:build and friends, and TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md would want rewriting around it. Worth doing as a second step.

Not on this list

packages/i18n's check:instances (root check:i18n-instances) looks like a member of this family, since it reads packages/*/dist directly. It is not: skipping a package nobody built is deliberate and documented in scripts/check-bundle-instances.ts, the number of packages scanned is printed, and a scan that finds nothing at all is an explicit failure ("No built scripts found under packages/*/dist/ … a scan of nothing passes trivially"). Left alone here. (An earlier version of this issue listed it as a separate bug; that was wrong.)

Follow-up to #1672.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    Priority

    None yet

    Effort

    None yet

    Projects

    Status
    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions