Repository navigation
test(cli): render, codemod and doctor fixtures live in the OS temp directory - #9899
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. This is a draft from a fork, so CI runs once it is marked ready for review and a maintainer adds the ci:run label.
Moving the CLI test fixtures into the OS temp directory looks right, and the approval covers both commits, including the #9892 change. The moved tests pass with unchanged assertions, check:cli-structure passes, and none of these tests leaves a fixture folder in packages/cli any more.
- Non-blocking: the description says the structure walk never enters doctor's fixture folders, but
allMjsFiles(CLI)inscripts/check-cli-structure.mjsdoes descend intoclients/cli/commands; those folders just exist too briefly to race, so that sentence could be reworded.
[Automated review]
8565fd9 to
23236bc
Compare
There was a problem hiding this comment.
Changes requested: one fix before this can merge.
- Miss: the
packages/cli/.gitignoreedit makes the requiredlintjob's Changeset coverage step (scripts/release/changeset-coverage.mjs pr) fail, because it counts that file as shipping in@astryxdesign/cliand no Changeset names the package; drop the.gitignorehunk (the stale entry is harmless) or add a Changeset naming@astryxdesign/cli.
Otherwise the move looks right: the four test files make no fixture inside packages/cli, the changed tests, the helper test and the structure test pass with unchanged assertions, and pnpm check:cli-structure passes. CI has not run on this head yet; it runs once the pull request is marked ready for review and a maintainer adds the ci:run label.
[Automated review]
|
|
||
| # Transient fixture dir written by authoring-migration.tsc.test.mjs (runs the | ||
| # real tsc over migrated output); created + removed per test run. | ||
| .codemod-tsc-*/ |
There was a problem hiding this comment.
This hunk makes the required lint job's Changeset coverage step fail: scripts/release/changeset-coverage.mjs counts packages/cli/.gitignore as shipping in @astryxdesign/cli, and no Changeset names that package. Keeping the stale entry (it is harmless) keeps this pull request test-only; otherwise add a Changeset naming @astryxdesign/cli.
There was a problem hiding this comment.
Dropped the hunk in 6246d85: the stale entry stays, so this pull request stays test-only. The coverage step now passes: "nothing here ships to consumers".
…rectory Four more test files made fixture projects inside packages/cli: the render API and the render time limit (so React and the page's imports resolved), the v0.3.0 authoring codemod's tsc check (so @astryxdesign/cli resolved as itself), and doctor's config checks. That is the tree check-cli-structure and other scanners walk, and a fixture can vanish in the middle of a walk. The render and codemod fixtures use makeLinkedFixture, which now links the CLI's own package by name. The codemod's tsconfig extends the repository's by its absolute path. Doctor's config files import nothing, so they go to a plain temporary directory: Vitest now imports a file from there.
23236bc to
6246d85
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. This is a draft from a fork, so CI runs once it is marked ready for review and a maintainer adds the ci:run label.
Dropping the .gitignore hunk clears the earlier Changeset coverage failure, and the four test files now make no fixture inside packages/cli; the changed tests, the helper test and the structure test pass with unchanged assertions, and pnpm check:cli-structure passes.
- Non-blocking: the description still says the structure walk never enters doctor's fixture folders, but
allMjsFiles(CLI)inscripts/check-cli-structure.mjsdoes descend intoclients/cli/commands; those folders just exist too briefly to race, so that sentence could be reworded.
[Automated review]
cixzhang
added
the
ci:run
Follows #9892, which added
test-utils/linked-fixture.mjs.Maintainer impact
Four more test files make fixture projects inside
packages/cli, the treescripts/check-cli-structure.mjswalks: the render API (api/render), the render time limit (foundation/render), the v0.3.0 authoring codemod'stsccheck, and doctor's config checks. When a walk lists such a fixture and reaches it after the test has removed it, the check fails with ENOENT. With this change, no test makes a fixture insidepackages/cli.Intended invariant
The tests' assertions are unchanged, and each fixture's imports resolve as before. No product code changes.
Change
makeLinkedFixture, which now links the CLI's own package by name, so@astryxdesign/cliresolves as itself. The codemod's tsconfig extends the repository's by its absolute path, and its fixture links thenodeand jest-dom types the root tsconfig names.Evidence
e3bd880f26(test(cli): theme build fixtures live in the OS temp directory, not inside packages/cli #9892's head before its merge) andeb6516e115(this change). As in test(cli): theme build fixtures live in the OS temp directory, not inside packages/cli #9892, a vitest setup file announces each temporary directory a test makes insidepackages/cli. A preload holds the structure walk until that fixture holds a file, then lists, stats or reads it only after the test has removed it. Order-balanced runs (before, this, this, before), the same in both rounds:packages/clicheck:cli-structurebeforeThe structure walk enters doctor's fixture folders too, but they exist too briefly for the evidence run to catch one, so no race shows there; they move all the same. Each test file passes in every run, on both arms.
Product/runtime behavior verified unchanged: test-only.
Scope
Testing
Local, at 6246d85 on
main6255d30 (Linux): the four files,test-utils/linked-fixture.test.mjsandscripts/check-cli-structure.test.mjspass, 148 tests; prettier on the helper,pnpm linton the changed files, and the Changeset coverage check (nothing ships). The four test files were not prettier-clean before, so their edits stay minimal.