feat: add tests for check markdown scirpt #3378
Conversation
vishvamsinh28
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes in this pull request modify the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3378 +/- ##
==========================================
+ Coverage 67.77% 77.00% +9.23%
==========================================
Files 21 21
Lines 664 661 -3
==========================================
+ Hits 450 509 +59
+ Misses 214 152 -62 ☔ View full report in Codecov by Sentry. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3378--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: 2
🧹 Outside diff range and nitpick comments (6)
tests/markdown/check-markdown.test.js (3)
14-21: Consider improving error handling in beforeEach.While the setup works, the error handling could be more robust by using async/await pattern instead of the callback approach.
Consider refactoring to:
- beforeEach(done => { - mockConsoleError = jest.spyOn(console, 'error').mockImplementation(); - fs.mkdtemp(path.join(os.tmpdir(), 'test-config'), (err, directory) => { - if (err) throw err; - tempDir = directory; - done(); - }); - }); + beforeEach(async () => { + mockConsoleError = jest.spyOn(console, 'error').mockImplementation(); + tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'test-config')); + });
28-44: Add edge cases to author validation tests.The current test case doesn't cover all possible scenarios for author validation.
Consider adding tests for:
- Empty authors array
- Authors with empty strings
- Authors with malformed URLs (e.g., missing protocol)
- Maximum length validation for author fields
46-53: Enhance docs frontmatter validation test coverage.The current test only checks for invalid types but misses other important scenarios.
Consider adding test cases for:
- Empty frontmatter object
- Missing optional fields
- Edge cases for weight values (negative, zero, decimal)
- Special characters in title
scripts/markdown/check-markdown.js (3)
148-148: Fix the trailing comma in module.exports.The trailing comma in the exports statement is unnecessary and inconsistent with the rest of the codebase.
-module.exports = { validateBlogs, validateDocs, checkMarkdownFiles,} +module.exports = { validateBlogs, validateDocs, checkMarkdownFiles }
Line range hint
9-17: Consider using a more robust URL validation.The current URL validation might accept invalid URLs in some edge cases. Consider using a more comprehensive validation approach.
function isValidURL(str) { try { - new URL(str); - return true; + const url = new URL(str); + // Additional validation for http/https protocols + return ['http:', 'https:'].includes(url.protocol); } catch (err) { return false; } }
Line range hint
82-144: Consider refactoring to use promises for better testability.The
checkMarkdownFilesfunction uses nested callbacks, making it harder to test. Consider refactoring to use promises or async/await for better error handling and testability.Here's a suggested refactor:
async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') { try { const files = await fs.promises.readdir(folderPath); for (const file of files) { const filePath = path.join(folderPath, file); const relativeFilePath = path.join(relativePath, file); if (relativeFilePath.includes('reference/specification')) { continue; } const stats = await fs.promises.stat(filePath); if (stats.isDirectory()) { await checkMarkdownFiles(filePath, validateFunction, relativeFilePath); } else if (path.extname(file) === '.md') { const fileContent = await fs.promises.readFile(filePath, 'utf-8'); const { data: frontmatter } = matter(fileContent); const errors = validateFunction(frontmatter); if (errors) { console.log(`Errors in file ${relativeFilePath}:`); errors.forEach(error => console.log(` - ${error}`)); process.exitCode = 1; } } } } catch (err) { console.error('Error processing files:', err); process.exitCode = 1; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
scripts/markdown/check-markdown.js(1 hunks)tests/markdown/check-markdown.test.js(1 hunks)
🔇 Additional comments (2)
tests/markdown/check-markdown.test.js (1)
1-8: LGTM! Imports are well-organized.
The necessary Node.js built-in modules and validation functions are properly imported.
scripts/markdown/check-markdown.js (1)
147-148: Verify test coverage for exported functions.
The functions are now being exported for testing purposes. Let's verify that appropriate tests have been implemented.
✅ Verification successful
Let me gather more information about the test coverage by examining the actual test implementations.
Let me check the complete test file content to get a full picture of the test coverage.
Test coverage is comprehensive for all exported functions ✅
The test suite includes thorough coverage for all exported functions:
validateBlogs: Tests for author validation, multiple validation errors, and invalid frontmattervalidateDocs: Tests for required fields validationcheckMarkdownFiles: Tests for file processing, error handling for invalid files, and filesystem errors
The tests include proper assertions, error cases, and mock implementations for filesystem operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for the exported functions
# Test 1: Check if test file exists and contains tests for all exported functions
echo "Checking test coverage for exported functions..."
rg -A 5 "describe\(.*(?:validateBlogs|validateDocs|checkMarkdownFiles)" "tests/markdown/check-markdown.test.js"
# Test 2: Verify that the functions are only used in the main script and tests
echo "Verifying function usage across codebase..."
rg "(?:validateBlogs|validateDocs|checkMarkdownFiles)" --type js
Length of output: 1748
Script:
#!/bin/bash
# Check the actual test implementations
echo "Examining test implementations..."
rg -B 2 -A 10 "describe\('.*'" "tests/markdown/check-markdown.test.js"
# Check if there are any test assertions
echo "Checking test assertions..."
rg "expect|assert|should" "tests/markdown/check-markdown.test.js"
Length of output: 1192
Script:
#!/bin/bash
# Get the complete test file content
echo "Examining complete test file..."
cat "tests/markdown/check-markdown.test.js"
Length of output: 3860
| it('checks for errors in markdown files in a directory', done => { | ||
| fs.writeFileSync(path.join(tempDir, 'invalid.md'), `---\ntitle: Invalid Blog\n---`); | ||
| const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(); | ||
|
|
||
| checkMarkdownFiles(tempDir, validateBlogs); | ||
|
|
||
| setTimeout(() => { | ||
| expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file invalid.md:')); | ||
| mockConsoleLog.mockRestore(); | ||
| done(); | ||
| }, 100); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor async tests to use more reliable patterns.
The current implementation has several issues:
- Hardcoded timeout values (100ms) might cause flaky tests
- setTimeout with done callback is an anti-pattern in Jest
- Inconsistent async testing patterns across different test cases
Consider refactoring to use async/await pattern consistently:
- it('checks for errors in markdown files in a directory', done => {
- fs.writeFileSync(path.join(tempDir, 'invalid.md'), `---\ntitle: Invalid Blog\n---`);
- const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation();
-
- checkMarkdownFiles(tempDir, validateBlogs);
-
- setTimeout(() => {
- expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file invalid.md:'));
- mockConsoleLog.mockRestore();
- done();
- }, 100);
+ it('checks for errors in markdown files in a directory', async () => {
+ fs.writeFileSync(path.join(tempDir, 'invalid.md'), `---\ntitle: Invalid Blog\n---`);
+ const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation();
+
+ await checkMarkdownFiles(tempDir, validateBlogs);
+
+ expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file invalid.md:'));
+ mockConsoleLog.mockRestore();Also applies to: 83-94, 96-108
| authors: { name: 'John Doe' } | ||
| }; | ||
| const errors = validateBlogs(frontmatter); | ||
| console.log(errors) |
There was a problem hiding this comment.
Remove debug console.log statement.
Debug logging should not be committed to the test suite.
- console.log(errors)📝 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.
| console.log(errors) |
| setTimeout(() => { | ||
| expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file invalid.md:')); | ||
| mockConsoleLog.mockRestore(); | ||
| done(); | ||
| }, 100); | ||
| }); |
There was a problem hiding this comment.
Why are we adding timeout here?
There was a problem hiding this comment.
replaced it with async/await
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
tests/markdown/check-markdown.test.js (3)
61-73: Enhance test assertions for multiple validation errors.The current test only checks the number of errors but not their specific content. Consider adding more specific assertions to ensure the exact validation messages are present.
Consider updating the test like this:
const errors = validateBlogs(frontmatter); -expect(errors.length).toBeGreaterThan(3); +expect(errors).toEqual(expect.arrayContaining([ + 'Title must be a string', + 'Date must be a valid date string', + 'Tags must be an array', + 'Cover must be a string', + 'Authors must be an array' +]));🧰 Tools
🪛 eslint
[error] 61-61: Delete
··(prettier/prettier)
[error] 62-62: Replace
········with····(prettier/prettier)
[error] 63-63: Delete
······(prettier/prettier)
[error] 64-64: Replace
············with······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Delete
······(prettier/prettier)
[error] 67-67: Replace
············with······(prettier/prettier)
[error] 68-68: Replace
············with······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Delete
····(prettier/prettier)
[error] 72-72: Replace
········with····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
24-83: Add test cases for edge cases.The test suite would benefit from additional test cases covering:
- Empty frontmatter
- Malformed YAML
- Empty authors array
- Empty tags array
Would you like me to help generate these additional test cases?
🧰 Tools
🪛 eslint
[error] 24-24: Delete
··(prettier/prettier)
[error] 25-25: Delete
····(prettier/prettier)
[error] 26-26: Delete
······(prettier/prettier)
[error] 27-27: Replace
············with······(prettier/prettier)
[error] 28-28: Delete
······(prettier/prettier)
[error] 29-29: Replace
············with······(prettier/prettier)
[error] 30-30: Replace
············with······(prettier/prettier)
[error] 31-31: Delete
······(prettier/prettier)
[error] 32-32: Delete
····(prettier/prettier)
[error] 34-34: Delete
····(prettier/prettier)
[error] 35-35: Replace
········expect(errors).toEqual(with····expect(errors).toEqual(⏎······(prettier/prettier)
[error] 36-36: Replace
············with········(prettier/prettier)
[error] 37-37: Delete
····(prettier/prettier)
[error] 38-38: Replace
············with········(prettier/prettier)
[error] 39-39: Replace
··])with])⏎····(prettier/prettier)
[error] 40-40: Delete
··(prettier/prettier)
[error] 42-42: Delete
··(prettier/prettier)
[error] 43-43: Delete
····(prettier/prettier)
[error] 44-44: Replace
········with····(prettier/prettier)
[error] 45-47: Replace
····expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number'withexpect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])(prettier/prettier)
[error] 48-48: Replace
········])with····(prettier/prettier)
[error] 49-49: Delete
··(prettier/prettier)
[error] 51-51: Delete
··(prettier/prettier)
[error] 52-52: Delete
····(prettier/prettier)
[error] 53-53: Replace
········with····(prettier/prettier)
[error] 55-55: Delete
····(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 58-58: Replace
········with····(prettier/prettier)
[error] 59-59: Delete
··(prettier/prettier)
[error] 61-61: Delete
··(prettier/prettier)
[error] 62-62: Replace
········with····(prettier/prettier)
[error] 63-63: Delete
······(prettier/prettier)
[error] 64-64: Replace
············with······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Delete
······(prettier/prettier)
[error] 67-67: Replace
············with······(prettier/prettier)
[error] 68-68: Replace
············with······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Delete
····(prettier/prettier)
[error] 72-72: Replace
········with····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
[error] 75-75: Delete
··(prettier/prettier)
[error] 76-76: Replace
········with····(prettier/prettier)
[error] 77-77: Delete
····(prettier/prettier)
[error] 77-77: 'mockConsoleError' is already declared in the upper scope on line 12 column 9.
(no-shadow)
[error] 78-78: Delete
····(prettier/prettier)
[error] 79-79: Delete
····(prettier/prettier)
[error] 80-80: Delete
····(prettier/prettier)
[error] 81-81: Replace
········with····(prettier/prettier)
[error] 82-82: Delete
····(prettier/prettier)
1-85: Fix formatting issues throughout the file.The file has numerous formatting inconsistencies. Consider running Prettier to automatically fix these issues.
Run the following command to fix formatting:
npx prettier --write tests/markdown/check-markdown.test.js🧰 Tools
🪛 eslint
[error] 4-8: Replace
⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎with·validateBlogs,·validateDocs,·checkMarkdownFiles·(prettier/prettier)
[error] 11-11: Delete
··(prettier/prettier)
[error] 12-12: Delete
··(prettier/prettier)
[error] 14-14: Replace
····with··(prettier/prettier)
[error] 15-15: Delete
····(prettier/prettier)
[error] 16-16: Delete
····(prettier/prettier)
[error] 17-17: Replace
····with··(prettier/prettier)
[error] 19-19: Delete
··(prettier/prettier)
[error] 20-20: Replace
········with····(prettier/prettier)
[error] 21-21: Replace
········with····(prettier/prettier)
[error] 22-22: Delete
··(prettier/prettier)
[error] 24-24: Delete
··(prettier/prettier)
[error] 25-25: Delete
····(prettier/prettier)
[error] 26-26: Delete
······(prettier/prettier)
[error] 27-27: Replace
············with······(prettier/prettier)
[error] 28-28: Delete
······(prettier/prettier)
[error] 29-29: Replace
············with······(prettier/prettier)
[error] 30-30: Replace
············with······(prettier/prettier)
[error] 31-31: Delete
······(prettier/prettier)
[error] 32-32: Delete
····(prettier/prettier)
[error] 34-34: Delete
····(prettier/prettier)
[error] 35-35: Replace
········expect(errors).toEqual(with····expect(errors).toEqual(⏎······(prettier/prettier)
[error] 36-36: Replace
············with········(prettier/prettier)
[error] 37-37: Delete
····(prettier/prettier)
[error] 38-38: Replace
············with········(prettier/prettier)
[error] 39-39: Replace
··])with])⏎····(prettier/prettier)
[error] 40-40: Delete
··(prettier/prettier)
[error] 42-42: Delete
··(prettier/prettier)
[error] 43-43: Delete
····(prettier/prettier)
[error] 44-44: Replace
········with····(prettier/prettier)
[error] 45-47: Replace
····expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number'withexpect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])(prettier/prettier)
[error] 48-48: Replace
········])with····(prettier/prettier)
[error] 49-49: Delete
··(prettier/prettier)
[error] 51-51: Delete
··(prettier/prettier)
[error] 52-52: Delete
····(prettier/prettier)
[error] 53-53: Replace
········with····(prettier/prettier)
[error] 55-55: Delete
····(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 58-58: Replace
········with····(prettier/prettier)
[error] 59-59: Delete
··(prettier/prettier)
[error] 61-61: Delete
··(prettier/prettier)
[error] 62-62: Replace
········with····(prettier/prettier)
[error] 63-63: Delete
······(prettier/prettier)
[error] 64-64: Replace
············with······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Delete
······(prettier/prettier)
[error] 67-67: Replace
············with······(prettier/prettier)
[error] 68-68: Replace
············with······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Delete
····(prettier/prettier)
[error] 72-72: Replace
········with····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
[error] 75-75: Delete
··(prettier/prettier)
[error] 76-76: Replace
········with····(prettier/prettier)
[error] 77-77: Delete
····(prettier/prettier)
[error] 77-77: 'mockConsoleError' is already declared in the upper scope on line 12 column 9.
(no-shadow)
[error] 78-78: Delete
····(prettier/prettier)
[error] 79-79: Delete
····(prettier/prettier)
[error] 80-80: Delete
····(prettier/prettier)
[error] 81-81: Replace
········with····(prettier/prettier)
[error] 82-82: Delete
····(prettier/prettier)
[error] 83-84: Replace
··});·⏎·······with});(prettier/prettier)
scripts/markdown/check-markdown.js (1)
134-141: Improve script execution and error handling.The script execution could be enhanced to properly handle errors and provide better feedback.
Consider wrapping the execution in an async function:
const docsFolderPath = path.resolve(__dirname, '../../markdown/docs'); const blogsFolderPath = path.resolve(__dirname, '../../markdown/blog'); -checkMarkdownFiles(docsFolderPath, validateDocs); -checkMarkdownFiles(blogsFolderPath, validateBlogs); +async function main() { + try { + await Promise.all([ + checkMarkdownFiles(docsFolderPath, validateDocs), + checkMarkdownFiles(blogsFolderPath, validateBlogs) + ]); + } catch (err) { + console.error('Failed to check markdown files:', err); + process.exit(1); + } +} + +if (require.main === module) { + main(); +} module.exports = { validateBlogs, validateDocs, checkMarkdownFiles };🧰 Tools
🪛 eslint
[error] 141-141: Insert
;(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
scripts/markdown/check-markdown.js(2 hunks)tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
📓 Learnings (1)
scripts/markdown/check-markdown.js (1)
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-10-18T17:28:41.718Z
Learning: In `scripts/markdown/check-markdown.js`, when processing large markdown files with recursion and loops, synchronous file reads (`fs.readFileSync`) are preferred over asynchronous reads.
🪛 GitHub Check: codecov/patch
scripts/markdown/check-markdown.js
[warning] 110-110: scripts/markdown/check-markdown.js#L110
Added line #L110 was not covered by tests
🪛 eslint
scripts/markdown/check-markdown.js
[error] 102-102: Delete ··
(prettier/prettier)
[error] 103-103: Replace ········ with ····
(prettier/prettier)
[error] 104-104: Delete ····
(prettier/prettier)
[error] 104-129: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.
(no-restricted-syntax)
[error] 105-105: Replace ············ with ······
(prettier/prettier)
[error] 106-106: Delete ······
(prettier/prettier)
[error] 108-108: Replace ············ with ······
(prettier/prettier)
[error] 109-109: Delete ······
(prettier/prettier)
[error] 110-110: Replace ················ with ········
(prettier/prettier)
[error] 110-110: Unexpected use of continue statement.
(no-continue)
[error] 111-111: Delete ······
(prettier/prettier)
[error] 113-113: Replace ············ with ······
(prettier/prettier)
[error] 113-113: Unexpected await inside a loop.
(no-await-in-loop)
[error] 115-115: Delete ······
(prettier/prettier)
[error] 116-116: Replace ············ with ······
(prettier/prettier)
[error] 117-117: Delete ········
(prettier/prettier)
[error] 117-117: Unexpected await inside a loop.
(no-await-in-loop)
[error] 118-118: Replace ············ with ······
(prettier/prettier)
[error] 119-119: Replace ················ with ········
(prettier/prettier)
[error] 119-119: Unexpected await inside a loop.
(no-await-in-loop)
[error] 120-120: Delete ········
(prettier/prettier)
[error] 122-122: Replace ················ with ········
(prettier/prettier)
[error] 123-123: Replace ················ with ········
(prettier/prettier)
[error] 124-124: Delete ··········
(prettier/prettier)
[error] 125-125: Replace ····················errors.forEach(error with ··········errors.forEach((error)
(prettier/prettier)
[error] 126-126: Replace ···················· with ··········
(prettier/prettier)
[error] 127-127: Replace ················ with ········
(prettier/prettier)
[error] 128-128: Replace ············ with ······
(prettier/prettier)
[error] 129-129: Replace ········ with ····
(prettier/prettier)
[error] 130-130: Delete ··
(prettier/prettier)
[error] 131-131: Delete ····
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 141-141: Insert ;
(prettier/prettier)
tests/markdown/check-markdown.test.js
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Replace ···· with ··
(prettier/prettier)
[error] 15-15: Delete ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Replace ············ with ······
(prettier/prettier)
[error] 28-28: Delete ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Delete ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Replace ············ with ········
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 38-38: Replace ············ with ········
(prettier/prettier)
[error] 39-39: Replace ··]) with ])⏎····
(prettier/prettier)
[error] 40-40: Delete ··
(prettier/prettier)
[error] 42-42: Delete ··
(prettier/prettier)
[error] 43-43: Delete ····
(prettier/prettier)
[error] 44-44: Replace ········ with ····
(prettier/prettier)
[error] 45-47: Replace ····expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Replace ········]) with ····
(prettier/prettier)
[error] 49-49: Delete ··
(prettier/prettier)
[error] 51-51: Delete ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Replace ········ with ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 58-58: Replace ········ with ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Replace ········ with ····
(prettier/prettier)
[error] 63-63: Delete ······
(prettier/prettier)
[error] 64-64: Replace ············ with ······
(prettier/prettier)
[error] 65-65: Replace ············ with ······
(prettier/prettier)
[error] 66-66: Delete ······
(prettier/prettier)
[error] 67-67: Replace ············ with ······
(prettier/prettier)
[error] 68-68: Replace ············ with ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Delete ····
(prettier/prettier)
[error] 72-72: Replace ········ with ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Replace ········ with ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 77-77: 'mockConsoleError' is already declared in the upper scope on line 12 column 9.
(no-shadow)
[error] 78-78: Delete ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Replace ········ with ····
(prettier/prettier)
[error] 82-82: Delete ····
(prettier/prettier)
[error] 83-84: Replace ··});·⏎······· with });
(prettier/prettier)
🔇 Additional comments (1)
scripts/markdown/check-markdown.js (1)
1-1: 🛠️ Refactor suggestion
Consider using synchronous file operations.
A previous implementation (PR #3301) found that synchronous operations (fs.readFileSync) are preferred when processing large markdown files with recursion and loops. The current change to fs.promises might impact performance.
Let's verify the performance impact:
Consider reverting to synchronous operations:
-const fs = require('fs').promises;
+const fs = require('fs');| let tempDir; | ||
| let mockConsoleError; | ||
|
|
There was a problem hiding this comment.
Fix variable shadowing of mockConsoleError.
The mockConsoleError variable is already declared in the outer scope (line 12) but is redeclared in the test case (line 77). This could lead to confusion and potential issues.
Apply this fix:
it('logs error to console when an error occurs in checkMarkdownFiles', async () => {
const invalidFolderPath = path.join(tempDir, 'non-existent-folder');
- const mockConsoleError = jest.spyOn(console, 'error').mockImplementation();
await checkMarkdownFiles(invalidFolderPath, validateBlogs);
expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:');
- mockConsoleError.mockRestore();
});Also applies to: 75-82
🧰 Tools
🪛 eslint
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
| async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') { | ||
| try { | ||
| const files = await fs.readdir(folderPath); | ||
| for (const file of files) { | ||
| const filePath = path.join(folderPath, file); | ||
| const relativeFilePath = path.join(relativePath, file); | ||
|
|
||
| // Skip the folder 'docs/reference/specification' | ||
| if (relativeFilePath.includes('reference/specification')) { | ||
| return; | ||
| continue; | ||
| } | ||
|
|
||
| fs.stat(filePath, (err, stats) => { | ||
| if (err) { | ||
| console.error('Error reading file stats:', err); | ||
| return; | ||
| } | ||
| const stats = await fs.stat(filePath); | ||
|
|
||
| // Recurse if directory, otherwise validate markdown file | ||
| if (stats.isDirectory()) { | ||
| await checkMarkdownFiles(filePath, validateFunction, relativeFilePath); | ||
| } else if (path.extname(file) === '.md') { | ||
| const fileContent = await fs.readFile(filePath, 'utf-8'); | ||
| const { data: frontmatter } = matter(fileContent); | ||
|
|
||
| // Recurse if directory, otherwise validate markdown file | ||
| if (stats.isDirectory()) { | ||
| checkMarkdownFiles(filePath, validateFunction, relativeFilePath); | ||
| } else if (path.extname(file) === '.md') { | ||
| const fileContent = fs.readFileSync(filePath, 'utf-8'); | ||
| const { data: frontmatter } = matter(fileContent); | ||
|
|
||
| const errors = validateFunction(frontmatter); | ||
| if (errors) { | ||
| console.log(`Errors in file ${relativeFilePath}:`); | ||
| errors.forEach(error => console.log(` - ${error}`)); | ||
| process.exitCode = 1; | ||
| } | ||
| const errors = validateFunction(frontmatter); | ||
| if (errors) { | ||
| console.log(`Errors in file ${relativeFilePath}:`); | ||
| errors.forEach(error => console.log(` - ${error}`)); | ||
| process.exitCode = 1; | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error('Error processing files:', err); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and optimize async operations.
Several improvements could be made to this function:
- The error handling is too generic
- Multiple
awaitoperations inside the loop could impact performance - The 'reference/specification' skip condition needs test coverage
Consider these improvements:
async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') {
try {
const files = await fs.readdir(folderPath);
+ const filePromises = files.map(async file => {
const filePath = path.join(folderPath, file);
const relativeFilePath = path.join(relativePath, file);
if (relativeFilePath.includes('reference/specification')) {
return;
}
- const stats = await fs.stat(filePath);
+ try {
+ const stats = await fs.stat(filePath);
- if (stats.isDirectory()) {
- await checkMarkdownFiles(filePath, validateFunction, relativeFilePath);
- } else if (path.extname(file) === '.md') {
- const fileContent = await fs.readFile(filePath, 'utf-8');
- const { data: frontmatter } = matter(fileContent);
+ if (stats.isDirectory()) {
+ await checkMarkdownFiles(filePath, validateFunction, relativeFilePath);
+ } else if (path.extname(file) === '.md') {
+ const fileContent = await fs.readFile(filePath, 'utf-8');
+ const { data: frontmatter } = matter(fileContent);
- const errors = validateFunction(frontmatter);
- if (errors) {
- console.log(`Errors in file ${relativeFilePath}:`);
- errors.forEach(error => console.log(` - ${error}`));
- process.exitCode = 1;
+ const errors = validateFunction(frontmatter);
+ if (errors) {
+ console.log(`Errors in file ${relativeFilePath}:`);
+ errors.forEach(error => console.log(` - ${error}`));
+ process.exitCode = 1;
+ }
}
+ } catch (err) {
+ console.error(`Error processing file ${relativeFilePath}:`, err);
+ throw err;
}
- }
+ });
+ await Promise.all(filePromises);
} catch (err) {
- console.error('Error processing files:', err);
+ console.error(`Error in directory ${folderPath}:`, err);
+ throw err;
}
}📝 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.
| async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') { | |
| try { | |
| const files = await fs.readdir(folderPath); | |
| for (const file of files) { | |
| const filePath = path.join(folderPath, file); | |
| const relativeFilePath = path.join(relativePath, file); | |
| // Skip the folder 'docs/reference/specification' | |
| if (relativeFilePath.includes('reference/specification')) { | |
| return; | |
| continue; | |
| } | |
| fs.stat(filePath, (err, stats) => { | |
| if (err) { | |
| console.error('Error reading file stats:', err); | |
| return; | |
| } | |
| const stats = await fs.stat(filePath); | |
| // Recurse if directory, otherwise validate markdown file | |
| if (stats.isDirectory()) { | |
| await checkMarkdownFiles(filePath, validateFunction, relativeFilePath); | |
| } else if (path.extname(file) === '.md') { | |
| const fileContent = await fs.readFile(filePath, 'utf-8'); | |
| const { data: frontmatter } = matter(fileContent); | |
| // Recurse if directory, otherwise validate markdown file | |
| if (stats.isDirectory()) { | |
| checkMarkdownFiles(filePath, validateFunction, relativeFilePath); | |
| } else if (path.extname(file) === '.md') { | |
| const fileContent = fs.readFileSync(filePath, 'utf-8'); | |
| const { data: frontmatter } = matter(fileContent); | |
| const errors = validateFunction(frontmatter); | |
| if (errors) { | |
| console.log(`Errors in file ${relativeFilePath}:`); | |
| errors.forEach(error => console.log(` - ${error}`)); | |
| process.exitCode = 1; | |
| } | |
| const errors = validateFunction(frontmatter); | |
| if (errors) { | |
| console.log(`Errors in file ${relativeFilePath}:`); | |
| errors.forEach(error => console.log(` - ${error}`)); | |
| process.exitCode = 1; | |
| } | |
| }); | |
| }); | |
| }); | |
| } | |
| } | |
| } catch (err) { | |
| console.error('Error processing files:', err); | |
| } | |
| async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') { | |
| try { | |
| const files = await fs.readdir(folderPath); | |
| const filePromises = files.map(async file => { | |
| const filePath = path.join(folderPath, file); | |
| const relativeFilePath = path.join(relativePath, file); | |
| if (relativeFilePath.includes('reference/specification')) { | |
| return; | |
| } | |
| try { | |
| const stats = await fs.stat(filePath); | |
| if (stats.isDirectory()) { | |
| await checkMarkdownFiles(filePath, validateFunction, relativeFilePath); | |
| } else if (path.extname(file) === '.md') { | |
| const fileContent = await fs.readFile(filePath, 'utf-8'); | |
| const { data: frontmatter } = matter(fileContent); | |
| const errors = validateFunction(frontmatter); | |
| if (errors) { | |
| console.log(`Errors in file ${relativeFilePath}:`); | |
| errors.forEach(error => console.log(` - ${error}`)); | |
| process.exitCode = 1; | |
| } | |
| } | |
| } catch (err) { | |
| console.error(`Error processing file ${relativeFilePath}:`, err); | |
| throw err; | |
| } | |
| }); | |
| await Promise.all(filePromises); | |
| } catch (err) { | |
| console.error(`Error in directory ${folderPath}:`, err); | |
| throw err; | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 110-110: scripts/markdown/check-markdown.js#L110
Added line #L110 was not covered by tests
🪛 eslint
[error] 102-102: Delete ··
(prettier/prettier)
[error] 103-103: Replace ········ with ····
(prettier/prettier)
[error] 104-104: Delete ····
(prettier/prettier)
[error] 104-129: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.
(no-restricted-syntax)
[error] 105-105: Replace ············ with ······
(prettier/prettier)
[error] 106-106: Delete ······
(prettier/prettier)
[error] 108-108: Replace ············ with ······
(prettier/prettier)
[error] 109-109: Delete ······
(prettier/prettier)
[error] 110-110: Replace ················ with ········
(prettier/prettier)
[error] 110-110: Unexpected use of continue statement.
(no-continue)
[error] 111-111: Delete ······
(prettier/prettier)
[error] 113-113: Replace ············ with ······
(prettier/prettier)
[error] 113-113: Unexpected await inside a loop.
(no-await-in-loop)
[error] 115-115: Delete ······
(prettier/prettier)
[error] 116-116: Replace ············ with ······
(prettier/prettier)
[error] 117-117: Delete ········
(prettier/prettier)
[error] 117-117: Unexpected await inside a loop.
(no-await-in-loop)
[error] 118-118: Replace ············ with ······
(prettier/prettier)
[error] 119-119: Replace ················ with ········
(prettier/prettier)
[error] 119-119: Unexpected await inside a loop.
(no-await-in-loop)
[error] 120-120: Delete ········
(prettier/prettier)
[error] 122-122: Replace ················ with ········
(prettier/prettier)
[error] 123-123: Replace ················ with ········
(prettier/prettier)
[error] 124-124: Delete ··········
(prettier/prettier)
[error] 125-125: Replace ····················errors.forEach(error with ··········errors.forEach((error)
(prettier/prettier)
[error] 126-126: Replace ···················· with ··········
(prettier/prettier)
[error] 127-127: Replace ················ with ········
(prettier/prettier)
[error] 128-128: Replace ············ with ······
(prettier/prettier)
[error] 129-129: Replace ········ with ····
(prettier/prettier)
[error] 130-130: Delete ··
(prettier/prettier)
[error] 131-131: Delete ····
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
|
@akshatnema Please review this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tests/markdown/check-markdown.test.js (2)
42-49: Enhance docs frontmatter validation test coverage.Consider adding more edge cases to thoroughly test the validation:
- Empty string for title
- Negative or decimal values for weight
- Missing fields
it('validates docs frontmatter for required fields', async () => { - const frontmatter = { title: 123, weight: 'not-a-number' }; + const testCases = [ + { frontmatter: { title: 123, weight: 'not-a-number' }, expectedError: 'Title is missing or not a string' }, + { frontmatter: { title: '', weight: 1 }, expectedError: 'Title cannot be empty' }, + { frontmatter: { title: 'Valid', weight: -1 }, expectedError: 'Weight must be a positive number' }, + { frontmatter: {}, expectedError: 'Title is required' } + ]; + + for (const { frontmatter, expectedError } of testCases) { + const errors = validateDocs(frontmatter); + expect(errors).toContain(expectedError); + } });🧰 Tools
🪛 eslint
[error] 42-42: Delete
··(prettier/prettier)
[error] 43-43: Replace
········with····(prettier/prettier)
[error] 44-44: Replace
········with····(prettier/prettier)
[error] 45-47: Replace
····expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number'withexpect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])(prettier/prettier)
[error] 48-48: Delete
····])(prettier/prettier)
[error] 49-49: Replace
····with··(prettier/prettier)
61-73: Make validation error expectations more specific.The current test only checks the number of errors. Consider asserting specific error messages to ensure the validation logic is working as expected.
it('returns multiple validation errors for invalid blog frontmatter', async () => { const frontmatter = { title: 123, date: 'invalid-date', type: 'blog', tags: 'not-an-array', cover: ['not-a-string'], authors: { name: 'John Doe' } }; const errors = validateBlogs(frontmatter); - expect(errors.length).toBeGreaterThan(3); + expect(errors).toEqual(expect.arrayContaining([ + 'Title must be a string', + 'Date must be a valid date string', + 'Tags must be an array', + 'Cover must be a string', + 'Authors must be an array' + ])); });🧰 Tools
🪛 eslint
[error] 61-61: Replace
····with··(prettier/prettier)
[error] 62-62: Delete
····(prettier/prettier)
[error] 63-63: Replace
············with······(prettier/prettier)
[error] 64-64: Delete
······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Replace
············with······(prettier/prettier)
[error] 67-67: Delete
······(prettier/prettier)
[error] 68-68: Replace
············with······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Replace
········with····(prettier/prettier)
[error] 72-72: Replace
········with····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
🪛 eslint
tests/markdown/check-markdown.test.js
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Replace ···· with ··
(prettier/prettier)
[error] 15-15: Delete ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Replace ············ with ······
(prettier/prettier)
[error] 28-28: Delete ······
(prettier/prettier)
[error] 29-29: Delete ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Delete ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Delete ····
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 38-38: Replace ············ with ········
(prettier/prettier)
[error] 39-39: Replace ··]) with ])⏎····
(prettier/prettier)
[error] 40-40: Delete ··
(prettier/prettier)
[error] 42-42: Delete ··
(prettier/prettier)
[error] 43-43: Replace ········ with ····
(prettier/prettier)
[error] 44-44: Replace ········ with ····
(prettier/prettier)
[error] 45-47: Replace ····expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Delete ····])
(prettier/prettier)
[error] 49-49: Replace ···· with ··
(prettier/prettier)
[error] 51-51: Delete ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Delete ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Replace ········ with ····
(prettier/prettier)
[error] 58-58: Delete ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Replace ···· with ··
(prettier/prettier)
[error] 62-62: Delete ····
(prettier/prettier)
[error] 63-63: Replace ············ with ······
(prettier/prettier)
[error] 64-64: Delete ······
(prettier/prettier)
[error] 65-65: Replace ············ with ······
(prettier/prettier)
[error] 66-66: Replace ············ with ······
(prettier/prettier)
[error] 67-67: Delete ······
(prettier/prettier)
[error] 68-68: Replace ············ with ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Replace ········ with ····
(prettier/prettier)
[error] 72-72: Replace ········ with ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 77-77: 'mockConsoleError' is already declared in the upper scope on line 12 column 9.
(no-shadow)
[error] 78-78: Delete ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Delete ····
(prettier/prettier)
[error] 82-82: Delete ····
(prettier/prettier)
[error] 83-83: Replace ··});····· with });
(prettier/prettier)
🔇 Additional comments (2)
tests/markdown/check-markdown.test.js (2)
14-22: LGTM! Well-structured test lifecycle hooks.
The setup and teardown logic is well implemented with proper async handling and cleanup.
🧰 Tools
🪛 eslint
[error] 14-14: Replace ···· with ··
(prettier/prettier)
[error] 15-15: Delete ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
11-13:
Fix variable shadowing of mockConsoleError.
The mockConsoleError variable is already declared in the outer scope and is shadowed in a test case below (line 77). This could lead to confusion and potential issues.
🧰 Tools
🪛 eslint
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/markdown/check-markdown.test.js (2)
19-22: Consider adding error handling for cleanup operationsThe cleanup in afterEach could fail silently. Consider wrapping it in a try-catch to log any cleanup failures.
afterEach(async () => { mockConsoleError.mockRestore(); - await fs.rm(tempDir, { recursive: true, force: true }); + try { + await fs.rm(tempDir, { recursive: true, force: true }); + } catch (error) { + console.warn(`Failed to clean up temp directory: ${error.message}`); + } });🧰 Tools
🪛 eslint
[error] 19-19: Delete
··(prettier/prettier)
[error] 20-20: Replace
········with····(prettier/prettier)
[error] 21-21: Replace
········with····(prettier/prettier)
[error] 22-22: Delete
··(prettier/prettier)
61-73: Enhance test specificity for validation errorsThe test for multiple validation errors could be more specific about which errors it expects.
const errors = validateBlogs(frontmatter); -expect(errors.length).toBeGreaterThan(3); +expect(errors).toEqual(expect.arrayContaining([ + 'Title must be a string', + 'Date must be a valid date string', + 'Tags must be an array', + 'Cover must be a string', + 'Authors must be an array' +]));🧰 Tools
🪛 eslint
[error] 61-61: Delete
··(prettier/prettier)
[error] 62-62: Replace
········with····(prettier/prettier)
[error] 63-63: Delete
······(prettier/prettier)
[error] 64-64: Replace
············with······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Delete
······(prettier/prettier)
[error] 67-67: Replace
············with······(prettier/prettier)
[error] 68-68: Replace
············with······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Replace
········with····(prettier/prettier)
[error] 72-72: Delete
····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
🪛 eslint
tests/markdown/check-markdown.test.js
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Replace ···· with ··
(prettier/prettier)
[error] 15-15: Delete ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Replace ········ with ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Replace ········ with ····
(prettier/prettier)
[error] 26-26: Replace ············ with ······
(prettier/prettier)
[error] 27-27: Replace ············ with ······
(prettier/prettier)
[error] 28-28: Replace ············ with ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Delete ····
(prettier/prettier)
[error] 34-34: Replace ········ with ····
(prettier/prettier)
[error] 35-35: Replace ····expect(errors).toEqual( with expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Delete ····
(prettier/prettier)
[error] 37-37: Replace ············ with ········
(prettier/prettier)
[error] 38-38: Delete ····
(prettier/prettier)
[error] 39-39: Replace ··])); with ])
(prettier/prettier)
[error] 40-40: Delete }
(prettier/prettier)
[error] 41-42: Replace ⏎·· with ··});⏎⏎
(prettier/prettier)
[error] 43-43: Replace ········ with ····
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 45-47: Replace ········expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with ····expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Delete ····])
(prettier/prettier)
[error] 49-49: Replace ···· with ··
(prettier/prettier)
[error] 51-51: Replace ···· with ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Delete ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 58-58: Replace ········ with ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Replace ········ with ····
(prettier/prettier)
[error] 63-63: Delete ······
(prettier/prettier)
[error] 64-64: Replace ············ with ······
(prettier/prettier)
[error] 65-65: Replace ············ with ······
(prettier/prettier)
[error] 66-66: Delete ······
(prettier/prettier)
[error] 67-67: Replace ············ with ······
(prettier/prettier)
[error] 68-68: Replace ············ with ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Replace ········ with ····
(prettier/prettier)
[error] 72-72: Delete ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Replace ········ with ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Replace ··});····· with });
(prettier/prettier)
🔇 Additional comments (1)
tests/markdown/check-markdown.test.js (1)
1-8: LGTM! Clean setup with proper imports
The test setup follows best practices with async fs operations and clean imports.
🧰 Tools
🪛 eslint
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
| it('logs error to console when an error occurs in checkMarkdownFiles', async () => { | ||
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | ||
|
|
||
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | ||
|
|
||
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix variable shadowing and add more error scenarios
- The test still has the variable shadowing issue mentioned in past reviews
- Consider adding more error scenarios for better coverage
-it('logs error to console when an error occurs in checkMarkdownFiles', async () => {
+describe('error handling', () => {
+ it('logs error for non-existent folder', async () => {
const invalidFolderPath = path.join(tempDir, 'non-existent-folder');
-
await checkMarkdownFiles(invalidFolderPath, validateBlogs);
-
expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:');
- });
+ });
+
+ it('handles invalid file content', async () => {
+ await fs.writeFile(path.join(tempDir, 'invalid.md'), 'Invalid YAML');
+ await checkMarkdownFiles(tempDir, validateBlogs);
+ expect(mockConsoleError).toHaveBeenCalled();
+ });
+});📝 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.
| it('logs error to console when an error occurs in checkMarkdownFiles', async () => { | |
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | |
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | |
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | |
| }); | |
| describe('error handling', () => { | |
| it('logs error for non-existent folder', async () => { | |
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | |
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | |
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | |
| }); | |
| it('handles invalid file content', async () => { | |
| await fs.writeFile(path.join(tempDir, 'invalid.md'), 'Invalid YAML'); | |
| await checkMarkdownFiles(tempDir, validateBlogs); | |
| expect(mockConsoleError).toHaveBeenCalled(); | |
| }); | |
| }); |
🧰 Tools
🪛 eslint
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Replace ········ with ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Replace ··});····· with });
(prettier/prettier)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/markdown/check-markdown.test.js (2)
61-73: Enhance assertions for multiple validation errorsWhile the test verifies that multiple errors are present, it would be more robust to verify the specific error messages.
const errors = validateBlogs(frontmatter); -expect(errors.length).toBeGreaterThan(3); +expect(errors).toEqual(expect.arrayContaining([ + 'Title must be a string', + 'Date must be a valid date string', + 'Tags must be an array', + 'Cover must be a string', + 'Authors must be an array' +]));🧰 Tools
🪛 eslint
[error] 61-61: Delete
··(prettier/prettier)
[error] 62-62: Delete
····(prettier/prettier)
[error] 63-63: Replace
············with······(prettier/prettier)
[error] 64-64: Delete
······(prettier/prettier)
[error] 65-65: Replace
············with······(prettier/prettier)
[error] 66-66: Delete
······(prettier/prettier)
[error] 67-67: Replace
············with······(prettier/prettier)
[error] 68-68: Delete
······(prettier/prettier)
[error] 69-69: Delete
····(prettier/prettier)
[error] 70-70: Delete
····(prettier/prettier)
[error] 72-72: Replace
········with····(prettier/prettier)
[error] 73-73: Delete
··(prettier/prettier)
83-94: Enhance skip folder test with additional assertionsThe test could be more comprehensive by verifying that other files outside the skipped folder are still validated.
it('skips the "reference/specification" folder during validation', async () => { const referenceSpecDir = path.join(tempDir, 'reference', 'specification'); await fs.mkdir(referenceSpecDir, { recursive: true }); await fs.writeFile(path.join(referenceSpecDir, 'skipped.md'), `---\ntitle: Skipped File\n---`); + await fs.writeFile(path.join(tempDir, 'validated.md'), `---\ntitle: Test File\n---`); const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(); await checkMarkdownFiles(tempDir, validateDocs); expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors in file reference/specification/skipped.md')); + expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file validated.md')); mockConsoleLog.mockRestore(); });🧰 Tools
🪛 eslint
[error] 83-83: Delete
··(prettier/prettier)
[error] 84-84: Replace
········with····(prettier/prettier)
[error] 85-85: Delete
····(prettier/prettier)
[error] 86-86: Delete
····(prettier/prettier)
[error] 87-87: Delete
····(prettier/prettier)
[error] 88-88: Delete
····(prettier/prettier)
[error] 89-89: Delete
····(prettier/prettier)
[error] 90-90: Delete
····(prettier/prettier)
[error] 91-91: Delete
····(prettier/prettier)
[error] 92-92: Replace
········expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors·in·file·reference/specification/skipped.md'));with····expect(mockConsoleLog).not.toHaveBeenCalledWith(⏎······expect.stringContaining('Errors·in·file·reference/specification/skipped.md')(prettier/prettier)
[error] 93-93: Insert
);⏎(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
🪛 eslint
tests/markdown/check-markdown.test.js
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Replace ········ with ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Delete ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 24-24: Replace ···· with ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Delete ······
(prettier/prettier)
[error] 28-28: Replace ············ with ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Replace ········ with ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ····expect(errors).toEqual( with expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Delete ····
(prettier/prettier)
[error] 37-37: Replace ············ with ········
(prettier/prettier)
[error] 38-38: Delete ····
(prettier/prettier)
[error] 39-39: Replace ···]) with ·])⏎····
(prettier/prettier)
[error] 40-40: Replace ···· with ··
(prettier/prettier)
[error] 42-42: Delete ··
(prettier/prettier)
[error] 43-43: Replace ········ with ····
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 45-47: Replace ········expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with ····expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Delete ····])
(prettier/prettier)
[error] 49-49: Delete ··
(prettier/prettier)
[error] 51-51: Delete ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Delete ····
(prettier/prettier)
[error] 55-55: Replace ········ with ····
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 58-58: Delete ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Delete ····
(prettier/prettier)
[error] 63-63: Replace ············ with ······
(prettier/prettier)
[error] 64-64: Delete ······
(prettier/prettier)
[error] 65-65: Replace ············ with ······
(prettier/prettier)
[error] 66-66: Delete ······
(prettier/prettier)
[error] 67-67: Replace ············ with ······
(prettier/prettier)
[error] 68-68: Delete ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Delete ····
(prettier/prettier)
[error] 72-72: Replace ········ with ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Delete ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Delete ··
(prettier/prettier)
[error] 83-83: Delete ··
(prettier/prettier)
[error] 84-84: Replace ········ with ····
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 86-86: Delete ····
(prettier/prettier)
[error] 87-87: Delete ····
(prettier/prettier)
[error] 88-88: Delete ····
(prettier/prettier)
[error] 89-89: Delete ····
(prettier/prettier)
[error] 90-90: Delete ····
(prettier/prettier)
[error] 91-91: Delete ····
(prettier/prettier)
[error] 92-92: Replace ········expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors·in·file·reference/specification/skipped.md')); with ····expect(mockConsoleLog).not.toHaveBeenCalledWith(⏎······expect.stringContaining('Errors·in·file·reference/specification/skipped.md')
(prettier/prettier)
[error] 93-93: Insert );⏎
(prettier/prettier)
[error] 94-95: Replace ··});⏎···· with });
(prettier/prettier)
🔇 Additional comments (3)
tests/markdown/check-markdown.test.js (3)
1-10: LGTM! Clean setup with proper imports
The test setup follows best practices by using fs.promises for async file operations and properly importing all required dependencies.
🧰 Tools
🪛 eslint
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
14-22: LGTM! Well-structured test lifecycle management
The test setup and teardown are properly implemented with:
- Async/await pattern for file operations
- Proper cleanup of temporary files and mocks
- Good test isolation practices
🧰 Tools
🪛 eslint
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Replace ········ with ····
(prettier/prettier)
[error] 16-16: Delete ····
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Delete ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
24-40: LGTM! Comprehensive validation test for authors array
The test case effectively covers multiple validation scenarios for the authors array, including missing fields and invalid URLs.
🧰 Tools
🪛 eslint
[error] 24-24: Replace ···· with ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ······
(prettier/prettier)
[error] 27-27: Delete ······
(prettier/prettier)
[error] 28-28: Replace ············ with ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Replace ········ with ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ····expect(errors).toEqual( with expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Delete ····
(prettier/prettier)
[error] 37-37: Replace ············ with ········
(prettier/prettier)
[error] 38-38: Delete ····
(prettier/prettier)
[error] 39-39: Replace ···]) with ·])⏎····
(prettier/prettier)
[error] 40-40: Replace ···· with ··
(prettier/prettier)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
🪛 eslint
tests/markdown/check-markdown.test.js
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Replace ········ with ····
(prettier/prettier)
[error] 16-16: Replace ········ with ····
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Delete ····
(prettier/prettier)
[error] 21-21: Delete ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Replace ············ with ······
(prettier/prettier)
[error] 27-27: Replace ············ with ······
(prettier/prettier)
[error] 28-28: Replace ············ with ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Delete ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Replace ············ with ········
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 38-38: Delete ····
(prettier/prettier)
[error] 39-39: Replace ··]) with ])⏎····
(prettier/prettier)
[error] 40-40: Delete ··
(prettier/prettier)
[error] 42-42: Delete ··
(prettier/prettier)
[error] 43-43: Delete ····
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 45-47: Replace ········expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with ····expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Replace ········]) with ····
(prettier/prettier)
[error] 49-49: Replace ···· with ··
(prettier/prettier)
[error] 51-51: Delete ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Replace ········ with ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Replace ········ with ····
(prettier/prettier)
[error] 58-58: Replace ········ with ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Delete ····
(prettier/prettier)
[error] 63-63: Delete ······
(prettier/prettier)
[error] 64-64: Replace ············ with ······
(prettier/prettier)
[error] 65-65: Delete ······
(prettier/prettier)
[error] 66-66: Replace ············ with ······
(prettier/prettier)
[error] 67-67: Replace ············ with ······
(prettier/prettier)
[error] 68-68: Replace ············ with ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Replace ········ with ····
(prettier/prettier)
[error] 72-72: Delete ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Replace ········ with ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Replace ········ with ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Delete ··
(prettier/prettier)
[error] 83-83: Delete ··
(prettier/prettier)
[error] 84-84: Delete ····
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 86-86: Delete ····
(prettier/prettier)
[error] 87-87: Delete ····
(prettier/prettier)
[error] 88-88: Replace ········ with ····
(prettier/prettier)
[error] 89-89: Delete ····
(prettier/prettier)
[error] 90-90: Delete ····
(prettier/prettier)
[error] 91-91: Delete ····
(prettier/prettier)
[error] 92-92: Replace ········expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors·in·file·reference/specification/skipped.md')); with ····expect(mockConsoleLog).not.toHaveBeenCalledWith(⏎······expect.stringContaining('Errors·in·file·reference/specification/skipped.md')
(prettier/prettier)
[error] 93-93: Insert );⏎
(prettier/prettier)
[error] 94-94: Delete ··
(prettier/prettier)
🔇 Additional comments (3)
tests/markdown/check-markdown.test.js (3)
1-23: LGTM! Well-structured test setup.
The test setup follows best practices with:
- Proper test isolation using temporary directories
- Appropriate console mocking
- Clean resource teardown
🧰 Tools
🪛 eslint
[error] 4-8: Replace ⏎····validateBlogs,⏎····validateDocs,⏎····checkMarkdownFiles⏎ with ·validateBlogs,·validateDocs,·checkMarkdownFiles·
(prettier/prettier)
[error] 11-11: Delete ··
(prettier/prettier)
[error] 12-12: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Replace ········ with ····
(prettier/prettier)
[error] 16-16: Replace ········ with ····
(prettier/prettier)
[error] 17-17: Delete ··
(prettier/prettier)
[error] 19-19: Delete ··
(prettier/prettier)
[error] 20-20: Delete ····
(prettier/prettier)
[error] 21-21: Delete ····
(prettier/prettier)
[error] 22-22: Delete ··
(prettier/prettier)
24-73: LGTM! Comprehensive test coverage.
The test cases thoroughly cover:
- Author validation with multiple scenarios
- Type checking for docs frontmatter
- File processing with proper async/await
- Multiple validation errors
🧰 Tools
🪛 eslint
[error] 24-24: Delete ··
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Replace ············ with ······
(prettier/prettier)
[error] 27-27: Replace ············ with ······
(prettier/prettier)
[error] 28-28: Replace ············ with ······
(prettier/prettier)
[error] 29-29: Replace ············ with ······
(prettier/prettier)
[error] 30-30: Replace ············ with ······
(prettier/prettier)
[error] 31-31: Delete ······
(prettier/prettier)
[error] 32-32: Delete ····
(prettier/prettier)
[error] 34-34: Delete ····
(prettier/prettier)
[error] 35-35: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 36-36: Replace ············ with ········
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 38-38: Delete ····
(prettier/prettier)
[error] 39-39: Replace ··]) with ])⏎····
(prettier/prettier)
[error] 40-40: Delete ··
(prettier/prettier)
[error] 42-42: Delete ··
(prettier/prettier)
[error] 43-43: Delete ····
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 45-47: Replace ········expect(errors).toEqual(expect.arrayContaining([⏎············'Title·is·missing·or·not·a·string',⏎············'Weight·is·missing·or·not·a·number' with ····expect(errors).toEqual(⏎······expect.arrayContaining(['Title·is·missing·or·not·a·string',·'Weight·is·missing·or·not·a·number'])
(prettier/prettier)
[error] 48-48: Replace ········]) with ····
(prettier/prettier)
[error] 49-49: Replace ···· with ··
(prettier/prettier)
[error] 51-51: Delete ··
(prettier/prettier)
[error] 52-52: Delete ····
(prettier/prettier)
[error] 53-53: Replace ········ with ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Replace ········ with ····
(prettier/prettier)
[error] 58-58: Replace ········ with ····
(prettier/prettier)
[error] 59-59: Delete ··
(prettier/prettier)
[error] 61-61: Delete ··
(prettier/prettier)
[error] 62-62: Delete ····
(prettier/prettier)
[error] 63-63: Delete ······
(prettier/prettier)
[error] 64-64: Replace ············ with ······
(prettier/prettier)
[error] 65-65: Delete ······
(prettier/prettier)
[error] 66-66: Replace ············ with ······
(prettier/prettier)
[error] 67-67: Replace ············ with ······
(prettier/prettier)
[error] 68-68: Replace ············ with ······
(prettier/prettier)
[error] 69-69: Delete ····
(prettier/prettier)
[error] 70-70: Replace ········ with ····
(prettier/prettier)
[error] 72-72: Delete ····
(prettier/prettier)
[error] 73-73: Delete ··
(prettier/prettier)
83-94: LGTM! Well-implemented folder exclusion test.
The test properly verifies that the specified folder is skipped during validation.
🧰 Tools
🪛 eslint
[error] 83-83: Delete ··
(prettier/prettier)
[error] 84-84: Delete ····
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 86-86: Delete ····
(prettier/prettier)
[error] 87-87: Delete ····
(prettier/prettier)
[error] 88-88: Replace ········ with ····
(prettier/prettier)
[error] 89-89: Delete ····
(prettier/prettier)
[error] 90-90: Delete ····
(prettier/prettier)
[error] 91-91: Delete ····
(prettier/prettier)
[error] 92-92: Replace ········expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors·in·file·reference/specification/skipped.md')); with ····expect(mockConsoleLog).not.toHaveBeenCalledWith(⏎······expect.stringContaining('Errors·in·file·reference/specification/skipped.md')
(prettier/prettier)
[error] 93-93: Insert );⏎
(prettier/prettier)
[error] 94-94: Delete ··
(prettier/prettier)
| it('logs error to console when an error occurs in checkMarkdownFiles', async () => { | ||
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | ||
|
|
||
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | ||
|
|
||
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor error handling test to follow test suite patterns.
The test case should be restructured to use the existing mockConsoleError from the outer scope instead of creating a new one.
Apply this fix:
it('logs error to console when an error occurs in checkMarkdownFiles', async () => {
const invalidFolderPath = path.join(tempDir, 'non-existent-folder');
-
await checkMarkdownFiles(invalidFolderPath, validateBlogs);
-
expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:');
});📝 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.
| it('logs error to console when an error occurs in checkMarkdownFiles', async () => { | |
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | |
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | |
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | |
| }); | |
| it('logs error to console when an error occurs in checkMarkdownFiles', async () => { | |
| const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); | |
| await checkMarkdownFiles(invalidFolderPath, validateBlogs); | |
| expect(mockConsoleError.mock.calls[0][0]).toContain('Error processing files:'); | |
| }); |
🧰 Tools
🪛 eslint
[error] 75-75: Delete ··
(prettier/prettier)
[error] 76-76: Replace ········ with ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Replace ········ with ····
(prettier/prettier)
[error] 79-79: Delete ····
(prettier/prettier)
[error] 80-80: Delete ····
(prettier/prettier)
[error] 81-81: Delete ··
(prettier/prettier)
This PR adds tests for check-markdown.js script
Summary by CodeRabbit
New Features
Tests
Chores