Converted Koenig test utils to TypeScript#28426
Conversation
no ref This test-only change should have no user impact. Koenig's test utilites now use TypeScript.
WalkthroughThis PR converts test utilities in the koenig module from CommonJS to ES modules with TypeScript types. Configuration files are updated to support TypeScript test files and include the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ghost/core/test/unit/server/services/koenig/test-utils/assert-prettified-includes.test.ts (1)
44-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAssert the throw path explicitly to prevent false-positive pass.
In Line 44-Line 52, this test succeeds if
assertPrettifiedIncludesdoes not throw, because no assertion runs outsidecatch.Suggested fix
- try { - assertPrettifiedIncludes(actual, expected); - } catch (error) { - assert(error instanceof Error); - assert.ok(error.message.includes('Received:')); - assert.ok(error.message.includes('Expected:')); - assert.ok(error.message.includes(actual)); - assert.ok(error.message.includes(expected)); - } + assert.throws(() => { + assertPrettifiedIncludes(actual, expected); + }, (error: unknown) => { + assert(error instanceof Error); + assert.ok(error.message.includes('Received:')); + assert.ok(error.message.includes('Expected:')); + assert.ok(error.message.includes(actual)); + assert.ok(error.message.includes(expected)); + return true; + });🤖 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 `@ghost/core/test/unit/server/services/koenig/test-utils/assert-prettified-includes.test.ts` around lines 44 - 52, The test currently masks a false-positive because if assertPrettifiedIncludes(actual, expected) does not throw the catch block never runs; change the test to explicitly assert the throw (either use assert.throws(() => assertPrettifiedIncludes(actual, expected), Error, /Received:/) and additional regex checks for 'Expected:' and the actual/expected strings, or call assert.fail() immediately after assertPrettifiedIncludes to ensure the test fails when no error is thrown). Update the test around the assertPrettifiedIncludes invocation so the throw path is explicitly asserted while keeping the existing message-content assertions on the caught error.
🤖 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.
Outside diff comments:
In
`@ghost/core/test/unit/server/services/koenig/test-utils/assert-prettified-includes.test.ts`:
- Around line 44-52: The test currently masks a false-positive because if
assertPrettifiedIncludes(actual, expected) does not throw the catch block never
runs; change the test to explicitly assert the throw (either use
assert.throws(() => assertPrettifiedIncludes(actual, expected), Error,
/Received:/) and additional regex checks for 'Expected:' and the actual/expected
strings, or call assert.fail() immediately after assertPrettifiedIncludes to
ensure the test fails when no error is thrown). Update the test around the
assertPrettifiedIncludes invocation so the throw path is explicitly asserted
while keeping the existing message-content assertions on the caught error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a5a3973-10b0-4ec3-b6b6-00785922b690
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
ghost/core/jsconfig.jsonghost/core/package.jsonghost/core/test/unit/server/services/koenig/test-utils/assert-prettified-includes.test.tsghost/core/test/unit/server/services/koenig/test-utils/assert-prettified-includes.tsghost/core/test/unit/server/services/koenig/test-utils/assert-prettifies-to.tsghost/core/test/unit/server/services/koenig/test-utils/build-call-renderer.tsghost/core/test/unit/server/services/koenig/test-utils/html.tsghost/core/test/unit/server/services/koenig/test-utils/index.jsghost/core/test/unit/server/services/koenig/test-utils/index.tsghost/core/test/unit/server/services/koenig/test-utils/prettify-html.tsghost/core/test/unit/server/services/koenig/test-utils/visibility.jsghost/core/test/unit/server/services/koenig/test-utils/visibility.tspnpm-workspace.yaml
💤 Files with no reviewable changes (2)
- ghost/core/test/unit/server/services/koenig/test-utils/index.js
- ghost/core/test/unit/server/services/koenig/test-utils/visibility.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #28426 +/- ##
========================================
Coverage 73.76% 73.76%
========================================
Files 1539 1540 +1
Lines 131190 131982 +792
Branches 15719 15780 +61
========================================
+ Hits 96768 97354 +586
- Misses 33456 33661 +205
- Partials 966 967 +1
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:
|
|
Fixed @CodeRabbit's comment in #28427. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check PR `#28427` details and CodeRabbit comments
gh pr view 28427 --repo TryGhost/Ghost --json title,body,url,state
# List comments on PR `#28426` by coderabbitai to find what was flagged
gh api repos/TryGhost/Ghost/pulls/28426/comments --jq '.[] | select(.user.login == "coderabbitai") | {id: .id, path: .path, body: .body[:300], url: .html_url}'Length of output: 814
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
ref #28426 (review) This is a test-only change. `assertPrettifiedIncludes()` had a test that ran some assertions if a function threw, but not if it didn't. This fixes that by using `assert.throws()`.
ref #28426 (review) This is a test-only change. `assertPrettifiedIncludes()` had a test that ran some assertions if a function threw, but not if it didn't. This fixes that by using `assert.throws()`.
1 participant
no ref
This test-only change should have no user impact. Koenig's test utilites now use TypeScript.