Changed koenig kg-* packages from mocha to vitest - #29216
Merged
Conversation
no ref - unifies the koenig test tooling with the rest of the monorepo, which standardised on vitest; mocha + c8 were the last holdout after the TryGhost/Koenig repo was merged into ghost/koenig - migrates the 10 remaining mocha packages (kg-card-factory, kg-clean-basic-html, kg-converters, kg-default-cards, kg-default-nodes, kg-default-transforms, kg-html-to-lexical, kg-lexical-html-renderer, kg-markdown-html-renderer, kg-utils); koenig-lexical and kg-unsplash-selector were already on vitest - runner-only swap: assertions stay on should/assert/sinon/chai for now, wired in via vitest setupFiles (the should/sinon globals the mocha suite installed through test/*/overrides.ts). Converting should->expect and sinon->vi is a deliberate, separately-reviewable follow-up - each package gains a vitest.config.ts (globals, node env, v8 coverage); test:unit now runs `vitest run --coverage`; mocha/@types/mocha/c8/tsx dropped for vitest + @vitest/coverage-v8; the redundant pretest build is removed since the nx test:unit target already dependsOn build - preserves the old `c8 --check-coverage` gates as vitest thresholds. kg-default-transforms keeps its 100% gate on statements/functions/lines but leaves branches ungated: vitest's v8 provider counts two defensive branches that c8 scored as covered, so a 100% branch gate would fail a suite c8 passed - fixes mocha-isms vitest rejects (not assertion changes): before->beforeAll, the `done` callback -> Promise (the editorTest helper + direct done tests in kg-default-nodes), and an empty describe -> describe.todo - adds the previously-missing @types/jsdom and @types/chai so the test type-checks pass; kg-default-nodes test-file type-checking stays src-only pending the should->expect follow-up that will clean up the should typings
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 11m 33s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 45s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run-many -t test:unit -p @tryghost/kg-card-f... |
✅ Succeeded | 6m 48s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 1m 40s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 3m 6s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 7s | View ↗ |
nx run-many -t lint -p @tryghost/kg-card-factor... |
✅ Succeeded | 3m 23s | View ↗ |
Additional runs (10) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-09 19:48:32 UTC
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis PR adds a shared Vitest configuration helper and updates multiple koenig packages to use it. Package test scripts and devDependencies were switched from Mocha/c8/tsx to Vitest-based type-checking and coverage runs, and test globals files now reference Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
no ref - the 10 per-package vitest.config.ts files were near-identical; only the setup files and coverage thresholds varied - adds koenig/vitest.shared.ts with a createKoenigVitestConfig factory, mirroring the createVitestConfig house pattern used by the React apps (admin-x-framework); the koenig libs are node/no-jsdom so they need their own base rather than reusing the React one - each package config collapses to a 3-6 line call passing only its setupFiles and thresholds; no behaviour change, all suites still green
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
koenig/kg-default-nodes/test/nodes/at-link.test.ts (1)
16-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the duplicated
editorTesthelper to a shared utility.The same
editorTestfunction is copy-pasted across all node test files. A shared factory intest/test-utils/that accepts theeditorinstance would eliminate this duplication:// test/test-utils/editor-test.ts import type {LexicalEditor} from 'lexical'; export const createEditorTest = (editor: LexicalEditor) => (testFn: () => void) => () => new Promise<void>((resolve, reject) => { editor.update(() => { try { testFn(); resolve(); } catch (e) { reject(e); } }); });Each test file would then just call
const editorTest = createEditorTest(editor);in itsbeforeAllhook. This is pre-existing duplication (not introduced by this PR), so defer if out of scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koenig/kg-default-nodes/test/nodes/at-link.test.ts` around lines 16 - 25, The duplicated editorTest helper should be centralized into a shared test utility instead of being copy-pasted in each node test file. Move the logic into a reusable factory such as createEditorTest under test/test-utils/ that accepts the LexicalEditor instance, then update at-link.test.ts and the other node tests to initialize editorTest from that shared helper in beforeAll. Keep the existing behavior of wrapping editor.update and resolving/rejecting the Promise unchanged.koenig/kg-default-nodes/test/nodes/call-to-action.test.ts (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the shared
editorTesthelper.This identical Promise-wrapper
editorTesthelper is duplicated verbatim across the entirekg-default-nodestest suite (call-to-action, callout, codeblock, email-cta, email, image, tk, and more). Now that all these files share the same runner (Vitest), consolidating this into a single shared test-utils module would reduce duplication and centralize any future changes to the async test-completion mechanism.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koenig/kg-default-nodes/test/nodes/call-to-action.test.ts` around lines 18 - 27, The Promise-wrapped editorTest helper is duplicated across multiple kg-default-nodes tests, so consolidate it into a shared test-utils module. Extract the common editor.update-based wrapper into one reusable helper and update the call-to-action test (and the other suite files using the same editorTest pattern) to import and use that shared helper. Keep the helper’s behavior unchanged so Vitest async completion still works consistently everywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@koenig/kg-clean-basic-html/package.json`:
- Line 42: Update the `@types/jsdom` dependency in package.json to use the shared
catalog pin instead of an inline version. Add `@types/jsdom` to
pnpm-workspace.yaml and change this dependency entry to catalog: so the version
is centrally managed across the workspace.
In `@koenig/kg-default-cards/package.json`:
- Around line 44-55: The dev dependency list in package.json still pins
`@types/jsdom` inline while the shared workspace dependencies use catalog:
entries. Add `@types/jsdom` to the pnpm catalog in pnpm-workspace.yaml and update
the package.json entry in kg-default-cards to use catalog: consistently with the
other shared dev deps.
In `@koenig/kg-default-cards/vitest.config.ts`:
- Around line 1-18: The Vitest setup in defineConfig currently reports coverage
but no longer enforces the package’s minimum coverage gate. Update the coverage
config in vitest.config.ts by adding a coverage.thresholds block alongside the
existing reporter/include/all settings, so CI fails when coverage drops below
the expected floor. Keep the change scoped to the test.coverage object in the
default export.
In `@koenig/kg-default-nodes/vitest.config.ts`:
- Around line 5-10: The Vitest setup in vitest.config.ts enables globals, but
the package is missing typings for those global test APIs, so TypeScript and
IDEs won’t recognize describe/it/beforeEach in the test files. Add a
vitest-globals.d.ts for kg-default-nodes, or otherwise import the Vitest types
so the globals are declared for the test suite. Make sure the new typings are
included for test files since test:types only applies to src/**/*.
---
Nitpick comments:
In `@koenig/kg-default-nodes/test/nodes/at-link.test.ts`:
- Around line 16-25: The duplicated editorTest helper should be centralized into
a shared test utility instead of being copy-pasted in each node test file. Move
the logic into a reusable factory such as createEditorTest under
test/test-utils/ that accepts the LexicalEditor instance, then update
at-link.test.ts and the other node tests to initialize editorTest from that
shared helper in beforeAll. Keep the existing behavior of wrapping editor.update
and resolving/rejecting the Promise unchanged.
In `@koenig/kg-default-nodes/test/nodes/call-to-action.test.ts`:
- Around line 18-27: The Promise-wrapped editorTest helper is duplicated across
multiple kg-default-nodes tests, so consolidate it into a shared test-utils
module. Extract the common editor.update-based wrapper into one reusable helper
and update the call-to-action test (and the other suite files using the same
editorTest pattern) to import and use that shared helper. Keep the helper’s
behavior unchanged so Vitest async completion still works consistently
everywhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c5c5842-5733-4d16-9350-964d5acca7c4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
koenig/kg-card-factory/package.jsonkoenig/kg-card-factory/test/vitest-globals.d.tskoenig/kg-card-factory/vitest.config.tskoenig/kg-clean-basic-html/package.jsonkoenig/kg-clean-basic-html/test/clean-basic-html.test.tskoenig/kg-clean-basic-html/test/vitest-globals.d.tskoenig/kg-clean-basic-html/vitest.config.tskoenig/kg-converters/package.jsonkoenig/kg-converters/test/vitest-globals.d.tskoenig/kg-converters/vitest.config.tskoenig/kg-default-cards/package.jsonkoenig/kg-default-cards/test/vitest-globals.d.tskoenig/kg-default-cards/vitest.config.tskoenig/kg-default-nodes/package.jsonkoenig/kg-default-nodes/test/generate-decorator-node.test.tskoenig/kg-default-nodes/test/nodes/aside.test.tskoenig/kg-default-nodes/test/nodes/at-link-search.test.tskoenig/kg-default-nodes/test/nodes/at-link.test.tskoenig/kg-default-nodes/test/nodes/audio.test.tskoenig/kg-default-nodes/test/nodes/bookmark.test.tskoenig/kg-default-nodes/test/nodes/button.test.tskoenig/kg-default-nodes/test/nodes/call-to-action.test.tskoenig/kg-default-nodes/test/nodes/callout.test.tskoenig/kg-default-nodes/test/nodes/codeblock.test.tskoenig/kg-default-nodes/test/nodes/email-cta.test.tskoenig/kg-default-nodes/test/nodes/email.test.tskoenig/kg-default-nodes/test/nodes/embed.test.tskoenig/kg-default-nodes/test/nodes/file.test.tskoenig/kg-default-nodes/test/nodes/gallery.test.tskoenig/kg-default-nodes/test/nodes/header.test.tskoenig/kg-default-nodes/test/nodes/horizontalrule.test.tskoenig/kg-default-nodes/test/nodes/html.test.tskoenig/kg-default-nodes/test/nodes/image.test.tskoenig/kg-default-nodes/test/nodes/markdown.test.tskoenig/kg-default-nodes/test/nodes/paywall.test.tskoenig/kg-default-nodes/test/nodes/product.test.tskoenig/kg-default-nodes/test/nodes/signup.test.tskoenig/kg-default-nodes/test/nodes/tk.test.tskoenig/kg-default-nodes/test/nodes/toggle.test.tskoenig/kg-default-nodes/test/nodes/transistor.test.tskoenig/kg-default-nodes/test/nodes/video.test.tskoenig/kg-default-nodes/test/nodes/zwnj.test.tskoenig/kg-default-nodes/test/serializers/linebreak.test.tskoenig/kg-default-nodes/test/serializers/paragraph.test.tskoenig/kg-default-nodes/test/utils/visibility.test.tskoenig/kg-default-nodes/vitest.config.tskoenig/kg-default-transforms/package.jsonkoenig/kg-default-transforms/vitest.config.tskoenig/kg-html-to-lexical/package.jsonkoenig/kg-html-to-lexical/vitest.config.tskoenig/kg-lexical-html-renderer/package.jsonkoenig/kg-lexical-html-renderer/vitest.config.tskoenig/kg-markdown-html-renderer/package.jsonkoenig/kg-markdown-html-renderer/test/vitest-globals.d.tskoenig/kg-markdown-html-renderer/vitest.config.tskoenig/kg-utils/package.jsonkoenig/kg-utils/test/vitest-globals.d.tskoenig/kg-utils/vitest.config.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29216 +/- ##
=======================================
Coverage 74.04% 74.04%
=======================================
Files 1570 1570
Lines 136624 136625 +1
Branches 16520 16523 +3
=======================================
+ Hits 101159 101163 +4
- Misses 34429 34456 +27
+ Partials 1036 1006 -30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…globals no ref - addresses CodeRabbit review on the vitest migration PR - @types/jsdom was pinned inline (28.0.3) in 5 koenig packages; the catalog already centralises the other shared @types (node/react/jest/...), so this moves it there too and points the koenig consumers at catalog: to kill the duplication. ghost/core keeps its existing inline pin (same version, out of this PR's scope) - re-adds koenig/kg-default-nodes/test/vitest-globals.d.ts so editors/tsc recognise the describe/it/beforeEach globals in its test files (lost when @types/mocha was removed). test:types stays src-only there: pulling the test files into the type-check resurfaces pre-existing @types/should gaps (.should.deepEqual etc.) that belong to the should->expect follow-up
9larsons
enabled auto-merge (squash)
July 9, 2026 19:44
9larsons
added a commit
that referenced
this pull request
Jul 9, 2026
no ref Follow-up to the mocha→vitest migration (#29216). After that landed, the koenig `kg-*` suites were running on vitest but still using `should`/`sinon`-style assertions, wired in through setup-file globals. This converts them to native vitest `expect` so the koenig tests match the rest of the monorepo's assertion style, and lets us delete the leftover scaffolding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The rest of the monorepo standardised on vitest; mocha + c8 were the last holdout in
koenig/after the TryGhost/Koenig repo was merged in. This unifies the test tooling across the remaining koenig packages.What
Migrates the 10 remaining mocha packages onto vitest:
kg-card-factory,kg-clean-basic-html,kg-converters,kg-default-cards,kg-default-nodes,kg-default-transforms,kg-html-to-lexical,kg-lexical-html-renderer,kg-markdown-html-renderer,kg-utils. (koenig-lexicalandkg-unsplash-selectorwere already on vitest;kg-simplemdehas no tests.)Per package:
vitest.config.ts(globals: true, node env, v8 coverage).test:unitnow runsvitest run --coverage;mocha/@types/mocha/c8/tsxremoved forvitest+@vitest/coverage-v8.pretestbuild step is dropped — the Nxtest:unittarget alreadydependsOn: build.Scope: runner only (assertions unchanged)
This is intentionally a runner-only swap. Assertions stay on
should/assert/sinon/chai, wired in via vitestsetupFiles(the sameshould/sinonglobals the mocha suite installed through each package'stest/*/overrides.ts). Convertingshould→expectandsinon→viis a deliberate, separately-reviewable follow-up so the diff here stays reviewable and any assertion-semantics regressions land on a stable base.Reviewer notes
thresholds(oldc8 --check-coverage= lines ≥ 90%;--100= 100%). One deliberate exception:kg-default-transformskeeps its 100% gate on statements/functions/lines but leaves branches ungated — vitest's v8 provider counts two defensive$isTextNodebranches inremove-at-link-nodes.tsthat c8 scored as covered, so a 100% branch gate would fail a suite c8 passed. Documented in that config.before→beforeAll; the mochadonecallback →Promise(the repeatededitorTesthelper + directdonetests inkg-default-nodes); one emptydescribe→describe.todo.@types/jsdomand@types/chai, sotest:typespasses.kg-default-nodestest-file type-checking is deliberately leftsrc-only for now — its test files carry pre-existing@types/shouldtyping gaps (.should.deepEqual, custom-assertion signatures) that theshould→expectfollow-up will clean up.Verification
All 10 packages green locally:
nx run-many -t test:unit(1,332 tests pass, 23 pre-existing pending),test:types, andlint(0 errors).Shared config factory
The 10 per-package
vitest.config.tsfiles were near-identical, so they now call acreateKoenigVitestConfigfactory inkoenig/vitest.shared.ts— mirroring thecreateVitestConfighouse pattern the React apps use (via admin-x-framework). The koenig libs are node/no-jsdom, so they get their own base rather than reusing the React one. Each package config is now a 3-6 line call passing only itssetupFilesand coveragethresholds; no behaviour change.