feat: add tests for check markdown script #3378
Conversation
vishvamsinh28
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
WalkthroughThe changes in this pull request involve updates to 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
Documentation and Community
|
✅ 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 77.22% 86.59% +9.37%
==========================================
Files 21 21
Lines 663 664 +1
==========================================
+ Hits 512 575 +63
+ Misses 151 89 -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)
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
@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: 0
🧹 Outside diff range and nitpick comments (4)
scripts/markdown/check-markdown.js (2)
141-151: Add input validation for folder paths.While the implementation is correct, consider adding validation for the folder paths before processing:
async function main() { try { + // Validate folder paths exist before processing + try { + await Promise.all([ + fs.access(docsFolderPath), + fs.access(blogsFolderPath) + ]); + } catch (error) { + throw new Error('Invalid folder paths provided'); + } await Promise.all([ checkMarkdownFiles(docsFolderPath, validateDocs), checkMarkdownFiles(blogsFolderPath, validateBlogs) ]); } catch (error) {🧰 Tools
🪛 eslint
[error] 142-142: Delete
··(prettier/prettier)
[error] 143-143: Delete
····(prettier/prettier)
[error] 144-144: Replace
············with······(prettier/prettier)
[error] 145-145: Replace
············with······(prettier/prettier)
[error] 146-146: Delete
····(prettier/prettier)
[error] 147-147: Delete
··(prettier/prettier)
[error] 148-148: Delete
····(prettier/prettier)
[error] 149-149: Replace
········with····(prettier/prettier)
[error] 150-150: Delete
··(prettier/prettier)
158-158: Add missing semicolon.Add a semicolon at the end of the module.exports statement.
-module.exports = { validateBlogs, validateDocs, checkMarkdownFiles, main, isValidURL } +module.exports = { validateBlogs, validateDocs, checkMarkdownFiles, main, isValidURL };🧰 Tools
🪛 eslint
[error] 158-158: Insert
;(prettier/prettier)
tests/markdown/check-markdown.test.js (2)
77-81: Use consistent string quotes.Use single quotes consistently throughout the test file.
expect(errors).toEqual([ - "Invalid date format: invalid-date", - "Tags should be an array", - "Cover must be a string", - "Authors should be an array"]); + 'Invalid date format: invalid-date', + 'Tags should be an array', + 'Cover must be a string', + 'Authors should be an array' + ]);🧰 Tools
🪛 eslint
[error] 77-77: Delete
····(prettier/prettier)
[error] 78-78: Replace
······"Invalid·date·format:·invalid-date"with'Invalid·date·format:·invalid-date'(prettier/prettier)
[error] 79-79: Replace
······"Tags·should·be·an·array"with'Tags·should·be·an·array'(prettier/prettier)
[error] 80-80: Replace
············"Cover·must·be·a·string"with······'Cover·must·be·a·string'(prettier/prettier)
[error] 81-81: Replace
······"Authors·should·be·an·array"with'Authors·should·be·an·array'⏎····(prettier/prettier)
138-138: Add missing semicolon.Add a semicolon after the expect statement.
-expect(mockConsoleError).not.toHaveBeenCalledWith() +expect(mockConsoleError).not.toHaveBeenCalledWith();🧰 Tools
🪛 eslint
[error] 138-138: Replace
········expect(mockConsoleError).not.toHaveBeenCalledWith()with····expect(mockConsoleError).not.toHaveBeenCalledWith();(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
scripts/markdown/check-markdown.js(4 hunks)tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
📓 Learnings (2)
scripts/markdown/check-markdown.js (3)
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:41:29.632Z
Learning: When updating workflows for the AsyncAPI website repository, use `.github/workflows/if-nodejs-pr-testing.yml` to include environment variables and secrets for Node.js PR testing.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-11-12T06:59:33.851Z
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.
tests/markdown/check-markdown.test.js (1)
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
🪛 eslint
scripts/markdown/check-markdown.js
[error] 102-102: Delete ··
(prettier/prettier)
[error] 103-103: Delete ····
(prettier/prettier)
[error] 104-104: Replace ········ with ····
(prettier/prettier)
[error] 113-113: Delete ··········
(prettier/prettier)
[error] 115-115: Delete ··········
(prettier/prettier)
[error] 116-116: Delete ··········
(prettier/prettier)
[error] 117-117: Delete ············
(prettier/prettier)
[error] 118-118: Delete ··········
(prettier/prettier)
[error] 119-119: Replace ···················· with ········
(prettier/prettier)
[error] 131-131: Replace ········ with ····
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 133-133: Replace ········ with ····
(prettier/prettier)
[error] 134-134: Delete ····
(prettier/prettier)
[error] 135-135: Delete ··
(prettier/prettier)
[error] 142-142: Delete ··
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 144-144: Replace ············ with ······
(prettier/prettier)
[error] 145-145: Replace ············ with ······
(prettier/prettier)
[error] 146-146: Delete ····
(prettier/prettier)
[error] 147-147: Delete ··
(prettier/prettier)
[error] 148-148: Delete ····
(prettier/prettier)
[error] 149-149: Replace ········ with ····
(prettier/prettier)
[error] 150-150: Delete ··
(prettier/prettier)
[error] 155-155: Delete ··
(prettier/prettier)
[error] 158-158: Insert ;
(prettier/prettier)
tests/markdown/check-markdown.test.js
[error] 5-5: Delete ··
(prettier/prettier)
[error] 6-6: Delete ··
(prettier/prettier)
[error] 7-7: Replace ···· with ··
(prettier/prettier)
[error] 8-8: Replace ···· with ··
(prettier/prettier)
[error] 9-9: Replace ···· with ··
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Delete ··
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 18-18: Delete ····
(prettier/prettier)
[error] 19-19: Delete ····
(prettier/prettier)
[error] 20-20: Delete ····
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 23-23: 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: 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] 33-33: Replace ············ with ······
(prettier/prettier)
[error] 34-34: Replace ············ with ······
(prettier/prettier)
[error] 35-35: Delete ······
(prettier/prettier)
[error] 36-36: Replace ············ with ······
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 39-39: Delete ····
(prettier/prettier)
[error] 40-40: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 41-41: Replace ············ with ········
(prettier/prettier)
[error] 42-42: Delete ····
(prettier/prettier)
[error] 43-43: Replace ············ with ········
(prettier/prettier)
[error] 44-44: Replace ··])); with ])
(prettier/prettier)
[error] 45-46: Replace });⏎ with );
(prettier/prettier)
[error] 47-47: Insert });⏎⏎
(prettier/prettier)
[error] 48-48: Replace ········ with ····
(prettier/prettier)
[error] 49-49: Delete ····
(prettier/prettier)
[error] 50-52: 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] 53-53: Replace ········]) with ····
(prettier/prettier)
[error] 54-54: Delete ··
(prettier/prettier)
[error] 56-56: Delete ··
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 58-58: Replace ········ with ····
(prettier/prettier)
[error] 60-60: Delete ····
(prettier/prettier)
[error] 62-62: Replace ········ with ····
(prettier/prettier)
[error] 63-63: Delete ····
(prettier/prettier)
[error] 64-64: Delete ··
(prettier/prettier)
[error] 66-66: Delete ··
(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] 71-71: Replace ············ with ······
(prettier/prettier)
[error] 72-72: Delete ······
(prettier/prettier)
[error] 73-73: Delete ······
(prettier/prettier)
[error] 74-74: Replace ········ with ····
(prettier/prettier)
[error] 75-75: Delete ····
(prettier/prettier)
[error] 77-77: Delete ····
(prettier/prettier)
[error] 78-78: Replace ······"Invalid·date·format:·invalid-date" with 'Invalid·date·format:·invalid-date'
(prettier/prettier)
[error] 79-79: Replace ······"Tags·should·be·an·array" with 'Tags·should·be·an·array'
(prettier/prettier)
[error] 80-80: Replace ············"Cover·must·be·a·string" with ······'Cover·must·be·a·string'
(prettier/prettier)
[error] 81-81: Replace ······"Authors·should·be·an·array" with 'Authors·should·be·an·array'⏎····
(prettier/prettier)
[error] 82-82: Delete ··
(prettier/prettier)
[error] 84-84: Delete ··
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 87-88: Replace ········await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))⏎············ with ····await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))
(prettier/prettier)
[error] 90-90: Delete ····
(prettier/prettier)
[error] 91-91: Delete ··
(prettier/prettier)
[error] 93-93: Delete ··
(prettier/prettier)
[error] 94-94: Replace ········ with ····
(prettier/prettier)
[error] 95-95: Replace ········ with ····
(prettier/prettier)
[error] 96-96: Delete ····
(prettier/prettier)
[error] 98-98: Replace ········ with ····
(prettier/prettier)
[error] 100-100: Delete ····
(prettier/prettier)
[error] 102-102: 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] 103-103: Insert );⏎
(prettier/prettier)
[error] 104-104: Delete ··
(prettier/prettier)
[error] 106-106: Replace ···· with ··
(prettier/prettier)
[error] 107-107: Delete ····
(prettier/prettier)
[error] 108-108: Replace ········ with ····
(prettier/prettier)
[error] 110-110: Delete ····
(prettier/prettier)
[error] 112-112: Replace ········ with ····
(prettier/prettier)
[error] 113-116: Replace ····expect(mockConsoleError).toHaveBeenCalledWith(⏎············expect.stringContaining(Error·in·directory),⏎············expect.any(Error)⏎········ with expect(mockConsoleError).toHaveBeenCalledWith(expect.stringContaining(Error·in·directory),·expect.any(Error)
(prettier/prettier)
[error] 118-118: Replace ········ with ····
(prettier/prettier)
[error] 119-119: Delete ··
(prettier/prettier)
[error] 121-121: Delete ··
(prettier/prettier)
[error] 122-122: Delete ····
(prettier/prettier)
[error] 124-124: Delete ····
(prettier/prettier)
[error] 126-126: Delete ····
(prettier/prettier)
[error] 128-131: Replace ····expect(mockConsoleError).toHaveBeenCalledWith(⏎············'Failed·to·validate·markdown·files:',⏎············expect.any(Error)⏎········ with expect(mockConsoleError).toHaveBeenCalledWith('Failed·to·validate·markdown·files:',·expect.any(Error)
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 134-134: Delete ··
(prettier/prettier)
[error] 135-136: Delete ⏎····
(prettier/prettier)
[error] 138-138: Replace ········expect(mockConsoleError).not.toHaveBeenCalledWith() with ····expect(mockConsoleError).not.toHaveBeenCalledWith();
(prettier/prettier)
[error] 139-139: Delete ··
(prettier/prettier)
[error] 141-141: Replace ···· with ··
(prettier/prettier)
[error] 142-142: Replace ········ with ····
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 144-144: Replace ········ with ····
(prettier/prettier)
[error] 145-145: Delete ····
(prettier/prettier)
[error] 146-146: Replace ········ with ····
(prettier/prettier)
[error] 147-147: Replace ········ with ····
(prettier/prettier)
[error] 148-149: Replace ··});⏎ with });
(prettier/prettier)
🔇 Additional comments (3)
scripts/markdown/check-markdown.js (1)
Line range hint 101-135: LGTM! The async implementation looks good.
The function correctly implements async/await pattern with proper error handling. While there's potential for optimization regarding parallel file processing, this can be addressed in a separate issue as per previous discussions.
🧰 Tools
🪛 eslint
[error] 110-110: Delete ········
(prettier/prettier)
[error] 111-111: Replace ············ with ······
(prettier/prettier)
[error] 113-113: Delete ··········
(prettier/prettier)
[error] 115-115: Delete ··········
(prettier/prettier)
[error] 116-116: Delete ··········
(prettier/prettier)
[error] 117-117: Delete ············
(prettier/prettier)
[error] 118-118: Delete ··········
(prettier/prettier)
[error] 119-119: Replace ···················· with ········
(prettier/prettier)
[error] 120-120: Delete ············
(prettier/prettier)
[error] 122-122: Delete ············
(prettier/prettier)
tests/markdown/check-markdown.test.js (2)
1-27: LGTM! Test setup is comprehensive.
The test setup follows best practices with proper mocking and cleanup.
🧰 Tools
🪛 eslint
[error] 5-5: Delete ··
(prettier/prettier)
[error] 6-6: Delete ··
(prettier/prettier)
[error] 7-7: Replace ···· with ··
(prettier/prettier)
[error] 8-8: Replace ···· with ··
(prettier/prettier)
[error] 9-9: Replace ···· with ··
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Delete ··
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 18-18: Delete ····
(prettier/prettier)
[error] 19-19: Delete ····
(prettier/prettier)
[error] 20-20: Delete ····
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 23-23: 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: Delete ··
(prettier/prettier)
84-119: LGTM! Error handling tests are thorough.
The tests cover various error scenarios and use modern Jest patterns for testing rejections.
🧰 Tools
🪛 eslint
[error] 84-84: Delete ··
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 87-88: Replace ········await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))⏎············ with ····await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))
(prettier/prettier)
[error] 90-90: Delete ····
(prettier/prettier)
[error] 91-91: Delete ··
(prettier/prettier)
[error] 93-93: Delete ··
(prettier/prettier)
[error] 94-94: Replace ········ with ····
(prettier/prettier)
[error] 95-95: Replace ········ with ····
(prettier/prettier)
[error] 96-96: Delete ····
(prettier/prettier)
[error] 98-98: Replace ········ with ····
(prettier/prettier)
[error] 100-100: Delete ····
(prettier/prettier)
[error] 102-102: 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] 103-103: Insert );⏎
(prettier/prettier)
[error] 104-104: Delete ··
(prettier/prettier)
[error] 106-106: Replace ···· with ··
(prettier/prettier)
[error] 107-107: Delete ····
(prettier/prettier)
[error] 108-108: Replace ········ with ····
(prettier/prettier)
[error] 110-110: Delete ····
(prettier/prettier)
[error] 112-112: Replace ········ with ····
(prettier/prettier)
[error] 113-116: Replace ····expect(mockConsoleError).toHaveBeenCalledWith(⏎············expect.stringContaining(Error·in·directory),⏎············expect.any(Error)⏎········ with expect(mockConsoleError).toHaveBeenCalledWith(expect.stringContaining(Error·in·directory),·expect.any(Error)
(prettier/prettier)
[error] 118-118: Replace ········ with ····
(prettier/prettier)
[error] 119-119: Delete ··
(prettier/prettier)
| } | ||
| } | ||
|
|
||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
It is added to exclude the if block beneath it from coverage reporting. This block ensures that the script runs its main() function only when executed directly (and not when imported as a module). Without this check, the main() function would execute automatically when the script is imported during testing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
scripts/markdown/check-markdown.js (2)
Line range hint
101-135: Add input validation and improve error handlingConsider adding input validation and making error messages more specific:
async function checkMarkdownFiles(folderPath, validateFunction, relativePath = '') { + if (!folderPath || typeof folderPath !== 'string') { + throw new Error('folderPath must be a non-empty string'); + } + if (typeof validateFunction !== 'function') { + throw new Error('validateFunction must be a function'); + } try { const files = await fs.readdir(folderPath); const filePromises = files.map(async (file) => { // ... rest of the code }); await Promise.all(filePromises); } catch (err) { - console.error(`Error in directory ${folderPath}:`, err); + const errorMessage = err.code === 'ENOENT' + ? `Directory not found: ${folderPath}` + : `Error processing directory ${folderPath}: ${err.message}`; + console.error(errorMessage); throw err; } }🧰 Tools
🪛 eslint
[error] 110-110: Delete
········(prettier/prettier)
[error] 111-111: Replace
············with······(prettier/prettier)
[error] 113-113: Delete
··········(prettier/prettier)
[error] 115-115: Delete
··········(prettier/prettier)
[error] 116-116: Delete
··········(prettier/prettier)
[error] 117-117: Delete
············(prettier/prettier)
[error] 118-118: Delete
··········(prettier/prettier)
[error] 119-119: Delete
············(prettier/prettier)
[error] 120-120: Replace
····················with········(prettier/prettier)
[error] 122-122: Delete
············(prettier/prettier)
141-151: Improve error handling and separate concernsThe main function mixes error handling with process termination. Consider separating these concerns:
async function main() { try { await Promise.all([ checkMarkdownFiles(docsFolderPath, validateDocs), checkMarkdownFiles(blogsFolderPath, validateBlogs) ]); + return true; } catch (error) { console.error('Failed to validate markdown files:', error); - process.exit(1); + return false; } } if (require.main === module) { - main(); + main().then(success => { + if (!success) process.exit(1); + }); }🧰 Tools
🪛 eslint
[error] 142-142: Delete
··(prettier/prettier)
[error] 143-143: Delete
····(prettier/prettier)
[error] 144-144: Delete
······(prettier/prettier)
[error] 145-145: Replace
············with······(prettier/prettier)
[error] 146-146: Replace
········with····(prettier/prettier)
[error] 147-147: Delete
··(prettier/prettier)
[error] 148-148: Delete
····(prettier/prettier)
[error] 149-149: Replace
········with····(prettier/prettier)
[error] 150-150: Delete
··(prettier/prettier)
tests/markdown/check-markdown.test.js (2)
29-82: Consider adding edge cases to validation testsThe validation tests are good but could be more comprehensive. Consider adding tests for:
- Empty frontmatter object
- Null/undefined values
- Empty arrays for tags and authors
- Special characters in titles
it('handles edge cases in blog frontmatter', async () => { const frontmatter = { title: '', date: null, type: 'blog', tags: [], cover: undefined, authors: [] }; const errors = validateBlogs(frontmatter); expect(errors).toContain('Empty title is not allowed'); expect(errors).toContain('Date is required'); expect(errors).toContain('At least one tag is required'); expect(errors).toContain('Cover image is required'); expect(errors).toContain('At least one author is required'); });🧰 Tools
🪛 eslint
[error] 29-29: Delete
··(prettier/prettier)
[error] 30-30: Delete
····(prettier/prettier)
[error] 31-31: Replace
············with······(prettier/prettier)
[error] 32-32: Delete
······(prettier/prettier)
[error] 33-33: Delete
······(prettier/prettier)
[error] 34-34: Replace
············with······(prettier/prettier)
[error] 35-35: Replace
············with······(prettier/prettier)
[error] 36-36: Delete
······(prettier/prettier)
[error] 37-37: Delete
····(prettier/prettier)
[error] 39-39: Delete
····(prettier/prettier)
[error] 40-40: Replace
········expect(errors).toEqual(with····expect(errors).toEqual(⏎······(prettier/prettier)
[error] 41-41: Delete
····(prettier/prettier)
[error] 42-42: Replace
············with········(prettier/prettier)
[error] 43-43: Delete
····(prettier/prettier)
[error] 44-44: Replace
········]));with······])(prettier/prettier)
[error] 45-45: Insert
);⏎··(prettier/prettier)
[error] 47-47: Replace
····with··(prettier/prettier)
[error] 48-48: Delete
····(prettier/prettier)
[error] 49-49: Delete
····(prettier/prettier)
[error] 50-52: 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] 53-53: Replace
········])with····(prettier/prettier)
[error] 54-54: Delete
··(prettier/prettier)
[error] 56-56: Delete
··(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 58-58: Delete
····(prettier/prettier)
[error] 60-60: Delete
····(prettier/prettier)
[error] 62-62: Delete
····(prettier/prettier)
[error] 63-63: Replace
········with····(prettier/prettier)
[error] 64-64: Delete
··(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] 71-71: Replace
············with······(prettier/prettier)
[error] 72-72: Replace
············with······(prettier/prettier)
[error] 73-73: Delete
······(prettier/prettier)
[error] 74-74: Replace
········with····(prettier/prettier)
[error] 75-75: Delete
····(prettier/prettier)
[error] 77-77: Replace
········with····(prettier/prettier)
[error] 78-78: Delete
······(prettier/prettier)
[error] 79-79: Delete
······(prettier/prettier)
[error] 80-80: Replace
············with······(prettier/prettier)
[error] 81-81: Replace
············'Authors·should·be·an·array'with······'Authors·should·be·an·array'⏎····(prettier/prettier)
[error] 82-82: Replace
····with··(prettier/prettier)
84-119: Make error assertions more specificThe error handling tests could be more specific about the types of errors they expect:
it('logs error to console when an error occurs in checkMarkdownFiles', async () => { const invalidFolderPath = path.join(tempDir, 'non-existent-folder'); await expect(checkMarkdownFiles(invalidFolderPath, validateBlogs)) - .rejects.toThrow('ENOENT'); + .rejects.toThrow(expect.objectContaining({ + code: 'ENOENT', + syscall: 'opendir' + })); expect(mockConsoleError.mock.calls[0][0]).toContain('Error in directory'); });🧰 Tools
🪛 eslint
[error] 84-84: Delete
··(prettier/prettier)
[error] 85-85: Delete
····(prettier/prettier)
[error] 87-88: Replace
········await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))⏎············with····await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))(prettier/prettier)
[error] 90-90: Replace
········with····(prettier/prettier)
[error] 91-91: Replace
····with··(prettier/prettier)
[error] 93-93: Delete
··(prettier/prettier)
[error] 94-94: Replace
········with····(prettier/prettier)
[error] 95-95: Delete
····(prettier/prettier)
[error] 96-96: Replace
········with····(prettier/prettier)
[error] 98-98: Delete
····(prettier/prettier)
[error] 100-100: Replace
········with····(prettier/prettier)
[error] 102-102: Replace
····expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors·in·file·reference/specification/skipped.md'));withexpect(mockConsoleLog).not.toHaveBeenCalledWith(⏎······expect.stringContaining('Errors·in·file·reference/specification/skipped.md')(prettier/prettier)
[error] 103-103: Insert
);⏎(prettier/prettier)
[error] 104-104: Delete
··(prettier/prettier)
[error] 106-106: Delete
··(prettier/prettier)
[error] 107-107: Replace
········with····(prettier/prettier)
[error] 108-108: Delete
····(prettier/prettier)
[error] 110-110: Replace
········with····(prettier/prettier)
[error] 112-112: Delete
····(prettier/prettier)
[error] 113-116: Replace
········expect(mockConsoleError).toHaveBeenCalledWith(⏎············expect.stringContaining(Error·in·directory),⏎············expect.any(Error)⏎········with····expect(mockConsoleError).toHaveBeenCalledWith(expect.stringContaining(Error·in·directory),·expect.any(Error)(prettier/prettier)
[error] 118-118: Delete
····(prettier/prettier)
[error] 119-119: Replace
····with··(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
scripts/markdown/check-markdown.js(4 hunks)tests/markdown/check-markdown.test.js(1 hunks)
🧰 Additional context used
📓 Learnings (2)
scripts/markdown/check-markdown.js (3)
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:41:29.632Z
Learning: When updating workflows for the AsyncAPI website repository, use `.github/workflows/if-nodejs-pr-testing.yml` to include environment variables and secrets for Node.js PR testing.
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-11-12T06:59:33.851Z
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.
tests/markdown/check-markdown.test.js (1)
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
🪛 eslint
scripts/markdown/check-markdown.js
[error] 102-102: Delete ··
(prettier/prettier)
[error] 103-103: Delete ····
(prettier/prettier)
[error] 104-104: Replace ········ with ····
(prettier/prettier)
[error] 113-113: Delete ··········
(prettier/prettier)
[error] 115-115: Delete ··········
(prettier/prettier)
[error] 116-116: Delete ··········
(prettier/prettier)
[error] 117-117: Delete ············
(prettier/prettier)
[error] 118-118: Delete ··········
(prettier/prettier)
[error] 119-119: Delete ············
(prettier/prettier)
[error] 131-131: Delete ····
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 133-133: Replace ········ with ····
(prettier/prettier)
[error] 134-134: Delete ····
(prettier/prettier)
[error] 135-135: Delete ··
(prettier/prettier)
[error] 142-142: Delete ··
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 144-144: Delete ······
(prettier/prettier)
[error] 145-145: Replace ············ with ······
(prettier/prettier)
[error] 146-146: Replace ········ with ····
(prettier/prettier)
[error] 147-147: Delete ··
(prettier/prettier)
[error] 148-148: Delete ····
(prettier/prettier)
[error] 149-149: Replace ········ with ····
(prettier/prettier)
[error] 150-150: Delete ··
(prettier/prettier)
[error] 155-155: Delete ··
(prettier/prettier)
tests/markdown/check-markdown.test.js
[error] 5-5: Delete ··
(prettier/prettier)
[error] 6-6: Delete ··
(prettier/prettier)
[error] 7-7: Replace ···· with ··
(prettier/prettier)
[error] 8-8: Replace ···· with ··
(prettier/prettier)
[error] 9-9: Replace ···· with ··
(prettier/prettier)
[error] 13-13: Delete ··
(prettier/prettier)
[error] 14-14: Delete ··
(prettier/prettier)
[error] 15-15: Delete ··
(prettier/prettier)
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 18-18: Delete ····
(prettier/prettier)
[error] 19-19: Replace ········ with ····
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 23-23: Replace ···· with ··
(prettier/prettier)
[error] 24-24: Delete ····
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ····
(prettier/prettier)
[error] 27-27: Delete ··
(prettier/prettier)
[error] 29-29: Delete ··
(prettier/prettier)
[error] 30-30: Delete ····
(prettier/prettier)
[error] 31-31: Replace ············ with ······
(prettier/prettier)
[error] 32-32: Delete ······
(prettier/prettier)
[error] 33-33: Delete ······
(prettier/prettier)
[error] 34-34: Replace ············ with ······
(prettier/prettier)
[error] 35-35: Replace ············ with ······
(prettier/prettier)
[error] 36-36: Delete ······
(prettier/prettier)
[error] 37-37: Delete ····
(prettier/prettier)
[error] 39-39: Delete ····
(prettier/prettier)
[error] 40-40: Replace ········expect(errors).toEqual( with ····expect(errors).toEqual(⏎······
(prettier/prettier)
[error] 41-41: Delete ····
(prettier/prettier)
[error] 42-42: Replace ············ with ········
(prettier/prettier)
[error] 43-43: Delete ····
(prettier/prettier)
[error] 44-44: Replace ········])); with ······])
(prettier/prettier)
[error] 45-45: Insert );⏎··
(prettier/prettier)
[error] 47-47: Replace ···· with ··
(prettier/prettier)
[error] 48-48: Delete ····
(prettier/prettier)
[error] 49-49: Delete ····
(prettier/prettier)
[error] 50-52: 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] 53-53: Replace ········]) with ····
(prettier/prettier)
[error] 54-54: Delete ··
(prettier/prettier)
[error] 56-56: Delete ··
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 58-58: Delete ····
(prettier/prettier)
[error] 60-60: Delete ····
(prettier/prettier)
[error] 62-62: Delete ····
(prettier/prettier)
[error] 63-63: Replace ········ with ····
(prettier/prettier)
[error] 64-64: Delete ··
(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] 71-71: Replace ············ with ······
(prettier/prettier)
[error] 72-72: Replace ············ with ······
(prettier/prettier)
[error] 73-73: Delete ······
(prettier/prettier)
[error] 74-74: Replace ········ with ····
(prettier/prettier)
[error] 75-75: Delete ····
(prettier/prettier)
[error] 77-77: Replace ········ with ····
(prettier/prettier)
[error] 78-78: Delete ······
(prettier/prettier)
[error] 79-79: Delete ······
(prettier/prettier)
[error] 80-80: Replace ············ with ······
(prettier/prettier)
[error] 81-81: Replace ············'Authors·should·be·an·array' with ······'Authors·should·be·an·array'⏎····
(prettier/prettier)
[error] 82-82: Replace ···· with ··
(prettier/prettier)
[error] 84-84: Delete ··
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 87-88: Replace ········await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))⏎············ with ····await·expect(checkMarkdownFiles(invalidFolderPath,·validateBlogs))
(prettier/prettier)
[error] 90-90: Replace ········ with ····
(prettier/prettier)
[error] 91-91: Replace ···· with ··
(prettier/prettier)
[error] 93-93: Delete ··
(prettier/prettier)
[error] 94-94: Replace ········ with ····
(prettier/prettier)
[error] 95-95: Delete ····
(prettier/prettier)
[error] 96-96: Replace ········ with ····
(prettier/prettier)
[error] 98-98: Delete ····
(prettier/prettier)
[error] 100-100: Replace ········ with ····
(prettier/prettier)
[error] 102-102: 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] 103-103: Insert );⏎
(prettier/prettier)
[error] 104-104: Delete ··
(prettier/prettier)
[error] 106-106: Delete ··
(prettier/prettier)
[error] 107-107: Replace ········ with ····
(prettier/prettier)
[error] 108-108: Delete ····
(prettier/prettier)
[error] 110-110: Replace ········ with ····
(prettier/prettier)
[error] 112-112: Delete ····
(prettier/prettier)
[error] 113-116: Replace ········expect(mockConsoleError).toHaveBeenCalledWith(⏎············expect.stringContaining(Error·in·directory),⏎············expect.any(Error)⏎········ with ····expect(mockConsoleError).toHaveBeenCalledWith(expect.stringContaining(Error·in·directory),·expect.any(Error)
(prettier/prettier)
[error] 118-118: Delete ····
(prettier/prettier)
[error] 119-119: Replace ···· with ··
(prettier/prettier)
[error] 121-121: Delete ··
(prettier/prettier)
[error] 122-122: Delete ····
(prettier/prettier)
[error] 124-124: Replace ········ with ····
(prettier/prettier)
[error] 126-126: Delete ····
(prettier/prettier)
[error] 128-131: Replace ········expect(mockConsoleError).toHaveBeenCalledWith(⏎············'Failed·to·validate·markdown·files:',⏎············expect.any(Error)⏎········ with ····expect(mockConsoleError).toHaveBeenCalledWith('Failed·to·validate·markdown·files:',·expect.any(Error)
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 134-134: Delete ··
(prettier/prettier)
[error] 135-136: Delete ⏎····
(prettier/prettier)
[error] 138-138: Replace ········ with ····
(prettier/prettier)
[error] 139-139: Delete ··
(prettier/prettier)
[error] 141-141: Replace ···· with ··
(prettier/prettier)
[error] 142-142: Replace ········ with ····
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 144-144: Replace ········ with ····
(prettier/prettier)
[error] 145-145: Delete ····
(prettier/prettier)
[error] 146-146: Replace ········ with ····
(prettier/prettier)
[error] 147-147: Replace ········ with ····
(prettier/prettier)
[error] 148-149: Replace ··});⏎ with });
(prettier/prettier)
🔇 Additional comments (3)
scripts/markdown/check-markdown.js (1)
158-158: LGTM!
The exports correctly include all the necessary functions.
tests/markdown/check-markdown.test.js (2)
17-27: LGTM!
The test setup and teardown are well-structured and handle cleanup properly.
🧰 Tools
🪛 eslint
[error] 17-17: Replace ···· with ··
(prettier/prettier)
[error] 18-18: Delete ····
(prettier/prettier)
[error] 19-19: Replace ········ with ····
(prettier/prettier)
[error] 20-20: Replace ········ with ····
(prettier/prettier)
[error] 21-21: Delete ··
(prettier/prettier)
[error] 23-23: Replace ···· with ··
(prettier/prettier)
[error] 24-24: Delete ····
(prettier/prettier)
[error] 25-25: Delete ····
(prettier/prettier)
[error] 26-26: Delete ····
(prettier/prettier)
[error] 27-27: Delete ··
(prettier/prettier)
121-139: LGTM!
The main function tests appropriately cover both error and success cases. As per previous learnings, the simple success test is sufficient since the underlying functions are tested elsewhere.
🧰 Tools
🪛 eslint
[error] 121-121: Delete ··
(prettier/prettier)
[error] 122-122: Delete ····
(prettier/prettier)
[error] 124-124: Replace ········ with ····
(prettier/prettier)
[error] 126-126: Delete ····
(prettier/prettier)
[error] 128-131: Replace ········expect(mockConsoleError).toHaveBeenCalledWith(⏎············'Failed·to·validate·markdown·files:',⏎············expect.any(Error)⏎········ with ····expect(mockConsoleError).toHaveBeenCalledWith('Failed·to·validate·markdown·files:',·expect.any(Error)
(prettier/prettier)
[error] 132-132: Delete ··
(prettier/prettier)
[error] 134-134: Delete ··
(prettier/prettier)
[error] 135-136: Delete ⏎····
(prettier/prettier)
[error] 138-138: Replace ········ with ····
(prettier/prettier)
[error] 139-139: Delete ··
(prettier/prettier)
|
@akshatnema I have applied the suggestions provided by Coderabbit. However, I have ignored a few suggestions because they caused the script/test to not work as expected. |
|
/rtm |
anshgoyalevil
added
the
gsoc
5 participants
This PR adds tests for check-markdown.js script
Summary by CodeRabbit
New Features
mainfunction to orchestrate markdown validation.Bug Fixes
Tests
mainfunction.