test: add unit tests for scripts/compose.ts - #5685
Conversation
Signed-off-by: qwe123456789zxc <95130171+qwe123456789zxc@users.noreply.github.com>
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
📝 WalkthroughWalkthroughAdded Jest tests for the compose script. The tests mock dependencies, verify successful blog post generation, validate title slugification cases, and confirm file-write errors are logged. ChangesCompose script tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5685--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@tests/compose.test.ts`:
- Around line 45-52: Update every affected dynamic require in the test setup,
including the statements assigning promptMock, writeFileMock, loggerInfoMock,
and loggerErrorMock and the additional occurrences, so each ESLint directive
also suppresses global-require alongside `@typescript-eslint/no-var-requires`;
alternatively, replace them with a lint-approved mock accessor.
- Around line 101-102: Insert a blank line between the errorArg declaration and
the following expect assertion in the relevant test block to satisfy ESLint’s
required statement padding.
- Around line 11-16: Update the Day.js mock factory in jest.mock by adding a
blank line between the dayjsMock declaration and the return dayjsMock statement
to satisfy the padding lint rule.
- Line 25: Update the flushPromises helper’s Promise executor to use a block
body and invoke setImmediate without returning its handle, while preserving the
existing promise-resolution behavior.
- Line 1: Remove the unused logger import from tests/compose.test.ts; keep the
mocked logger access through the existing module lookup around the referenced
test lines unchanged.
- Around line 92-95: Update the writeFileMock setup in the “logs an error when
the file cannot be written” test to invoke its callback asynchronously, matching
fs.writeFile timing, and await/drain the resulting promise before assertions so
the callback error is observed without an unhandled throw.
🪄 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 Plus
Run ID: 3aaf1e2d-2f91-4fd4-8134-610fbd8fece9
📒 Files selected for processing (1)
tests/compose.test.ts
| @@ -0,0 +1,104 @@ | |||
| import { logger } from '../scripts/helpers/logger'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused logger import.
The test accesses the mocked logger through the module lookup on Lines 50-52. The top-level import has no consumer and triggers the reported unused-import rules.
Proposed fix
-import { logger } from '../scripts/helpers/logger';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { logger } from '../scripts/helpers/logger'; |
🧰 Tools
🪛 ESLint
[error] 1-1: 'logger' is defined but never used. Allowed unused vars must match /^_/u.
(no-unused-vars)
[error] 1-1: 'logger' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 1-1: 'logger' is defined but never used.
(unused-imports/no-unused-imports)
🤖 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 `@tests/compose.test.ts` at line 1, Remove the unused logger import from
tests/compose.test.ts; keep the mocked logger access through the existing module
lookup around the referenced test lines unchanged.
Source: Linters/SAST tools
| jest.mock('dayjs', () => { | ||
| const dayjsMock = jest.fn(() => ({ | ||
| format: jest.fn(() => '2021-05-01T10:00:00+02:00') | ||
| })); | ||
| return dayjsMock; | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the padding lint errors in the Day.js mock.
Add a blank line after the dayjsMock declaration and before return dayjsMock.
Proposed fix
const dayjsMock = jest.fn(() => ({
format: jest.fn(() => '2021-05-01T10:00:00+02:00')
}));
+
return dayjsMock;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mock('dayjs', () => { | |
| const dayjsMock = jest.fn(() => ({ | |
| format: jest.fn(() => '2021-05-01T10:00:00+02:00') | |
| })); | |
| return dayjsMock; | |
| }); | |
| jest.mock('dayjs', () => { | |
| const dayjsMock = jest.fn(() => ({ | |
| format: jest.fn(() => '2021-05-01T10:00:00+02:00') | |
| })); | |
| return dayjsMock; | |
| }); |
🧰 Tools
🪛 ESLint
[error] 12-14: Expected blank line after variable declarations.
(newline-after-var)
[error] 15-15: Expected blank line before this statement.
(padding-line-between-statements)
🤖 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 `@tests/compose.test.ts` around lines 11 - 16, Update the Day.js mock factory
in jest.mock by adding a blank line between the dayjsMock declaration and the
return dayjsMock statement to satisfy the padding lint rule.
Source: Linters/SAST tools
| } | ||
| })); | ||
|
|
||
| const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a block-bodied promise executor.
The expression-bodied executor returns the setImmediate handle. This triggers no-promise-executor-return.
Proposed fix
-const flushPromises = () => new Promise((resolve) => setImmediate(resolve));
+const flushPromises = () =>
+ new Promise<void>((resolve) => {
+ setImmediate(resolve);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); | |
| const flushPromises = () => | |
| new Promise<void>((resolve) => { | |
| setImmediate(resolve); | |
| }); |
🧰 Tools
🪛 ESLint
[error] 25-25: Return values from promise executor functions cannot be read.
(no-promise-executor-return)
🤖 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 `@tests/compose.test.ts` at line 25, Update the flushPromises helper’s Promise
executor to use a block body and invoke setImmediate without returning its
handle, while preserving the existing promise-resolution behavior.
Source: Linters/SAST tools
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| promptMock = require('inquirer').prompt; | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| writeFileMock = require('fs').writeFile; | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| loggerInfoMock = require('../scripts/helpers/logger').logger.info; | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| loggerErrorMock = require('../scripts/helpers/logger').logger.error; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the ESLint suppression for dynamic mock access.
The directives suppress only @typescript-eslint/no-var-requires. Static analysis reports global-require for these same statements. Add global-require to each directive, or replace the dynamic imports with a lint-approved mock accessor.
Proposed fix
- // eslint-disable-next-line `@typescript-eslint/no-var-requires`
+ // eslint-disable-next-line `@typescript-eslint/no-var-requires`, global-requireApply this change to each affected require statement.
Also applies to: 59-60, 84-85, 96-97
🧰 Tools
🪛 ESLint
[error] 46-46: Unexpected require().
(global-require)
[error] 48-48: Unexpected require().
(global-require)
[error] 50-50: Unexpected require().
(global-require)
[error] 52-52: Unexpected require().
(global-require)
🤖 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 `@tests/compose.test.ts` around lines 45 - 52, Update every affected dynamic
require in the test setup, including the statements assigning promptMock,
writeFileMock, loggerInfoMock, and loggerErrorMock and the additional
occurrences, so each ESLint directive also suppresses global-require alongside
`@typescript-eslint/no-var-requires`; alternatively, replace them with a
lint-approved mock accessor.
Source: Linters/SAST tools
| it('logs an error when the file cannot be written', async () => { | ||
| promptMock.mockResolvedValue(defaultAnswers); | ||
| writeFileMock.mockImplementation((_filePath, _content, _options, callback) => callback(new Error('EEXIST'))); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 'fs\.writeFile|throw err|logger\.error|\.catch' scripts/compose.ts
rg -n -C 6 'writeFileMock|callback\(new Error|flushPromises' tests/compose.test.tsRepository: asyncapi/website
Length of output: 4485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
function flushPromises() {
return new Promise((resolve) => setImmediate(resolve));
}
let caught = false;
const actualCallback = () => {
setTimeout(() => {
throw new Error('EEXIST');
}, 0);
};
Promise.resolve().then(() => {
actualCallback();
return flushPromises();
}).catch(() => {
caught = true;
});
flushPromises()
.then(() => {
const resolvedBeforeLaterCallback = !caught;
return flushPromises();
})
.then(() => {
console.log(JSON.stringify({ caughtWhenCallbackRunsLater: caught }));
});
JSRepository: asyncapi/website
Length of output: 384
Make the write-error mock match fs.writeFile callback timing.
scripts/compose.ts expects fs.writeFile callback errors to be observed inside the promise handler. The current test passes by synchronously calling callback(new Error('EEXIST')), while real fs.writeFile schedules callbacks later. Either invoke the test callback asynchronously, drain promises until the callback error is observed, or change production code to reject/recover from callback errors so the later async callback does not leave an unhandled throw.
🤖 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 `@tests/compose.test.ts` around lines 92 - 95, Update the writeFileMock setup
in the “logs an error when the file cannot be written” test to invoke its
callback asynchronously, matching fs.writeFile timing, and await/drain the
resulting promise before assertions so the callback error is observed without an
unhandled throw.
| const errorArg = loggerErrorMock.mock.calls.find((call) => call[0] instanceof Error)?.[0] as Error; | ||
| expect(errorArg.message).toBe('EEXIST'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the blank line required after errorArg.
ESLint reports missing padding before the following assertion.
Proposed fix
const errorArg = loggerErrorMock.mock.calls.find((call) => call[0] instanceof Error)?.[0] as Error;
+
expect(errorArg.message).toBe('EEXIST');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const errorArg = loggerErrorMock.mock.calls.find((call) => call[0] instanceof Error)?.[0] as Error; | |
| expect(errorArg.message).toBe('EEXIST'); | |
| const errorArg = loggerErrorMock.mock.calls.find((call) => call[0] instanceof Error)?.[0] as Error; | |
| expect(errorArg.message).toBe('EEXIST'); |
🧰 Tools
🪛 ESLint
[error] 101-101: Expected blank line after variable declarations.
(newline-after-var)
[error] 102-102: Expected blank line before this statement.
(padding-line-between-statements)
🤖 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 `@tests/compose.test.ts` around lines 101 - 102, Insert a blank line between
the errorArg declaration and the following expect assertion in the relevant test
block to satisfy ESLint’s required statement padding.
Source: Linters/SAST tools
|
@qwe123456789zxc Please have alook at coderabbit suggestion , also CI checks are failing because of linting and other errors so have a look at it. |
3 participants
What changed
Adds Jest unit tests for the blog post composition script (scripts/compose.ts), covering the interactive CLI flow and file generation behavior, as requested in #5096.
How it works
The script runs its inquirer prompt chain at module load, so the tests mock \inquirer, \ s, \dayjs, and the logger, then import the script and flush the promise chain. Deterministic date via a mocked \dayjs.
Testing
Ran locally with jest: 6 tests passing.
Certification
Summary by CodeRabbit