From ce0e986687ca21507709d4aca3675a2398a311ca Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 12:52:21 +0530 Subject: [PATCH 1/9] feat: Added concurrency limit in markdown files processing to improve performance --- package-lock.json | 176 ++++++++++++-- package.json | 5 +- scripts/markdown/check-markdown.js | 282 ++++++++++++++-------- tests/markdown/check-markdown.test.js | 331 +++++++++++++++----------- 4 files changed, 532 insertions(+), 262 deletions(-) diff --git a/package-lock.json b/package-lock.json index 726af180566c..cdafa181093b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,6 +54,7 @@ "next-mdx-remote": "^4.4.1", "node-fetch": "^3.3.2", "node-fetch-2": "npm:node-fetch@^2.7.0", + "p-limit": "^6.1.0", "postcss": "^8.4.35", "prettier": "^3.3.3", "react": "^18", @@ -4810,6 +4811,35 @@ } } }, + "node_modules/@netlify/plugin-nextjs/node_modules/p-limit": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", + "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "yocto-queue": "^0.1.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/@netlify/plugin-nextjs/node_modules/yocto-queue": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", + "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/@netlify/serverless-functions-api": { "version": "1.18.4", "resolved": "https://registry.npmjs.org/@netlify/serverless-functions-api/-/serverless-functions-api-1.18.4.tgz", @@ -9109,18 +9139,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/babel-loader/node_modules/yocto-queue": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-1.1.0.tgz", - "integrity": "sha512-cMojmlnwkAgIXqga+2sXshlgrrcI0QEPJ5n58pEvtuFo4PaekfomlCudArDD7hj8Hkswjl0/x4eu4q+Xa0WFgQ==", - "dev": true, - "engines": { - "node": ">=12.20" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/babel-plugin-istanbul": { "version": "6.1.1", "resolved": "https://registry.npmjs.org/babel-plugin-istanbul/-/babel-plugin-istanbul-6.1.1.tgz", @@ -16677,6 +16695,35 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/jest-changed-files/node_modules/p-limit": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", + "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "yocto-queue": "^0.1.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/jest-changed-files/node_modules/yocto-queue": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", + "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/jest-circus": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-circus/-/jest-circus-29.7.0.tgz", @@ -16720,6 +16767,22 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, + "node_modules/jest-circus/node_modules/p-limit": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", + "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "yocto-queue": "^0.1.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/jest-circus/node_modules/pretty-format": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/pretty-format/-/pretty-format-29.7.0.tgz", @@ -16740,6 +16803,19 @@ "integrity": "sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg==", "dev": true }, + "node_modules/jest-circus/node_modules/yocto-queue": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", + "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/jest-cli": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-cli/-/jest-cli-29.7.0.tgz", @@ -17300,6 +17376,35 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/jest-runner/node_modules/p-limit": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", + "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "yocto-queue": "^0.1.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/jest-runner/node_modules/yocto-queue": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", + "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/jest-runtime": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-runtime/-/jest-runtime-29.7.0.tgz", @@ -23086,14 +23191,15 @@ } }, "node_modules/p-limit": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", - "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-6.1.0.tgz", + "integrity": "sha512-H0jc0q1vOzlEk0TqAKXKZxdl7kX3OFUzCnNVUnq5Pc3DGo0kpeaMuPqxQn235HibwBEb0/pm9dgKTjXy66fBkg==", + "license": "MIT", "dependencies": { - "yocto-queue": "^0.1.0" + "yocto-queue": "^1.1.1" }, "engines": { - "node": ">=10" + "node": ">=18" }, "funding": { "url": "https://github.com/sponsors/sindresorhus" @@ -23113,6 +23219,33 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/p-locate/node_modules/p-limit": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", + "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", + "license": "MIT", + "dependencies": { + "yocto-queue": "^0.1.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/p-locate/node_modules/yocto-queue": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", + "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/p-queue": { "version": "6.6.2", "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-6.6.2.tgz", @@ -30455,11 +30588,12 @@ } }, "node_modules/yocto-queue": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", - "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-1.1.1.tgz", + "integrity": "sha512-b4JR1PFR10y1mKjhHY9LaGo6tmrgjit7hxVIeAmyMw3jegXR4dhYqLaQF5zMXZxY7tLpMyJeLjr1C4rLmkVe8g==", + "license": "MIT", "engines": { - "node": ">=10" + "node": ">=12.20" }, "funding": { "url": "https://github.com/sponsors/sindresorhus" diff --git a/package.json b/package.json index 84f538697d4d..dd4c75dc5669 100644 --- a/package.json +++ b/package.json @@ -90,6 +90,7 @@ "next-mdx-remote": "^4.4.1", "node-fetch": "^3.3.2", "node-fetch-2": "npm:node-fetch@^2.7.0", + "p-limit": "^6.1.0", "postcss": "^8.4.35", "prettier": "^3.3.3", "react": "^18", @@ -151,13 +152,13 @@ "eslint-plugin-storybook": "^0.8.0", "eslint-plugin-tailwindcss": "^3.14.2", "eslint-plugin-unused-imports": "^3.1.0", + "fast-xml-parser": "^4.5.0", "inquirer": "^9.2.14", "jest": "^29.7.0", "postcss-import": "^16.0.1", "remark-cli": "^12.0.1", "remark-lint": "^10.0.0", "remark-mdx": "^3.0.1", - "storybook": "^8.2.4", - "fast-xml-parser": "^4.5.0" + "storybook": "^8.2.4" } } diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index cd3bd7ddd1c5..aca0fbd985a7 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -1,6 +1,41 @@ const fs = require('fs').promises; const matter = require('gray-matter'); const path = require('path'); +const pLimit = require('p-limit'); + +/** + * Validates and retrieves the concurrency limit from environment variables. + * @returns {number} The validated concurrency limit. + */ +function getConcurrencyLimit() { + const envLimit = process.env.MARKDOWN_CONCURRENCY_LIMIT; + + // If no env var is set, return default + if (envLimit === undefined) { + return 10; + } + + // Attempt to parse the environment variable + const parsedLimit = parseInt(envLimit, 10); + + // Validate the parsed limit + if (Number.isNaN(parsedLimit)) { + console.warn( + `Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.`, + ); + return 10; + } + + // Check for non-positive integers + if (parsedLimit <= 0) { + console.warn( + `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer. Received: ${parsedLimit}. Falling back to default of 10.`, + ); + return 10; + } + + return parsedLimit; +} /** * Checks if a given string is a valid URL. @@ -8,12 +43,12 @@ const path = require('path'); * @returns {boolean} True if the string is a valid URL, false otherwise. */ function isValidURL(str) { - try { - new URL(str); - return true; - } catch (err) { - return false; - } + try { + new URL(str); + return true; + } catch (err) { + return false; + } } /** @@ -23,51 +58,60 @@ function isValidURL(str) { * @returns {string[]|null} An array of validation error messages, or null if no errors. */ function validateBlogs(frontmatter) { - const requiredAttributes = ['title', 'date', 'type', 'tags', 'cover', 'authors']; - const errors = []; - - // Check for required attributes - requiredAttributes.forEach(attr => { - if (!frontmatter.hasOwnProperty(attr)) { - errors.push(`${attr} is missing`); - } - }); - - // Validate date format - if (frontmatter.date && Number.isNaN(Date.parse(frontmatter.date))) { - errors.push(`Invalid date format: ${frontmatter.date}`); - } - - // Validate tags format (must be an array) - if (frontmatter.tags && !Array.isArray(frontmatter.tags)) { - errors.push(`Tags should be an array`); - } - - // Validate cover is a string - if (frontmatter.cover && typeof frontmatter.cover !== 'string') { - errors.push(`Cover must be a string`); + const requiredAttributes = [ + 'title', + 'date', + 'type', + 'tags', + 'cover', + 'authors', + ]; + const errors = []; + + // Check for required attributes + requiredAttributes.forEach((attr) => { + if (!frontmatter.hasOwnProperty(attr)) { + errors.push(`${attr} is missing`); } - - // Validate authors (must be an array with valid attributes) - if (frontmatter.authors) { - if (!Array.isArray(frontmatter.authors)) { - errors.push('Authors should be an array'); - } else { - frontmatter.authors.forEach((author, index) => { - if (!author.name) { - errors.push(`Author at index ${index} is missing a name`); - } - if (author.link && !isValidURL(author.link)) { - errors.push(`Invalid URL for author at index ${index}: ${author.link}`); - } - if (!author.photo) { - errors.push(`Author at index ${index} is missing a photo`); - } - }); + }); + + // Validate date format + if (frontmatter.date && Number.isNaN(Date.parse(frontmatter.date))) { + errors.push(`Invalid date format: ${frontmatter.date}`); + } + + // Validate tags format (must be an array) + if (frontmatter.tags && !Array.isArray(frontmatter.tags)) { + errors.push(`Tags should be an array`); + } + + // Validate cover is a string + if (frontmatter.cover && typeof frontmatter.cover !== 'string') { + errors.push(`Cover must be a string`); + } + + // Validate authors (must be an array with valid attributes) + if (frontmatter.authors) { + if (!Array.isArray(frontmatter.authors)) { + errors.push('Authors should be an array'); + } else { + frontmatter.authors.forEach((author, index) => { + if (!author.name) { + errors.push(`Author at index ${index} is missing a name`); + } + if (author.link && !isValidURL(author.link)) { + errors.push( + `Invalid URL for author at index ${index}: ${author.link}`, + ); } + if (!author.photo) { + errors.push(`Author at index ${index} is missing a photo`); + } + }); } + } - return errors.length ? errors : null; + return errors.length ? errors : null; } /** @@ -77,19 +121,22 @@ function validateBlogs(frontmatter) { * @returns {string[]|null} An array of validation error messages, or null if no errors. */ function validateDocs(frontmatter) { - const errors = []; - - // Check if title exists and is a string - if (!frontmatter.title || typeof frontmatter.title !== 'string') { - errors.push('Title is missing or not a string'); - } - - // Check if weight exists and is a number - if (frontmatter.weight === undefined || typeof frontmatter.weight !== 'number') { - errors.push('Weight is missing or not a number'); - } - - return errors.length ? errors : null; + const errors = []; + + // Check if title exists and is a string + if (!frontmatter.title || typeof frontmatter.title !== 'string') { + errors.push('Title is missing or not a string'); + } + + // Check if weight exists and is a number + if ( + frontmatter.weight === undefined || + typeof frontmatter.weight !== 'number' + ) { + errors.push('Weight is missing or not a number'); + } + + return errors.length ? errors : null; } /** @@ -97,62 +144,89 @@ function validateDocs(frontmatter) { * @param {string} folderPath - The path to the folder to check. * @param {Function} validateFunction - The function used to validate the frontmatter. * @param {string} [relativePath=''] - The relative path of the folder for logging purposes. + * @param {import('p-limit').default} limit - Concurrency limiter. */ -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); - - // Skip the folder 'docs/reference/specification' - if (relativeFilePath.includes('reference/specification')) { - 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); - - const errors = validateFunction(frontmatter); - if (errors) { - console.log(`Errors in file ${relativeFilePath}:`); - errors.forEach(error => console.log(` - ${error}`)); - process.exitCode = 1; - } - } +async function checkMarkdownFiles( + folderPath, + validateFunction, + relativePath = '', + limit, +) { + 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); + + // Skip the folder 'docs/reference/specification' + if (relativeFilePath.includes('reference/specification')) { + return; + } + + const stats = await fs.stat(filePath); + + // Recurse if directory, otherwise validate markdown file + if (stats.isDirectory()) { + await checkMarkdownFiles( + filePath, + validateFunction, + relativeFilePath, + limit, + ); + } else if (path.extname(file) === '.md') { + // Use the concurrency limiter for file processing + await limit(async () => { + 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; + } }); + } + }); - await Promise.all(filePromises); - } catch (err) { - console.error(`Error in directory ${folderPath}:`, err); - throw err; - } + await Promise.all(filePromises); + } catch (err) { + console.error(`Error in directory ${folderPath}:`, err); + throw err; + } } const docsFolderPath = path.resolve(__dirname, '../../markdown/docs'); const blogsFolderPath = path.resolve(__dirname, '../../markdown/blog'); async function main() { - try { - await Promise.all([ - checkMarkdownFiles(docsFolderPath, validateDocs), - checkMarkdownFiles(blogsFolderPath, validateBlogs) - ]); - } catch (error) { - console.error('Failed to validate markdown files:', error); - process.exit(1); - } + try { + // Get concurrency limit from environment or use default + const concurrencyLimit = getConcurrencyLimit(); + + // Create a concurrency limiter + const limit = pLimit(concurrencyLimit); + + await Promise.all([ + checkMarkdownFiles(docsFolderPath, validateDocs, '', limit), + checkMarkdownFiles(blogsFolderPath, validateBlogs, '', limit), + ]); + } catch (error) { + console.error('Failed to validate markdown files:', error); + process.exit(1); + } } /* istanbul ignore next */ if (require.main === module) { - main(); + main(); } -module.exports = { validateBlogs, validateDocs, checkMarkdownFiles, main, isValidURL }; +module.exports = { + validateBlogs, + validateDocs, + checkMarkdownFiles, + main, + isValidURL, + getConcurrencyLimit, +}; diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 85e06b70383f..df9a71e729c7 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -1,150 +1,211 @@ const fs = require('fs').promises; const path = require('path'); const os = require('os'); +const pLimit = require('p-limit'); const { - isValidURL, - main, - validateBlogs, - validateDocs, - checkMarkdownFiles + isValidURL, + main, + validateBlogs, + validateDocs, + checkMarkdownFiles, + getConcurrencyLimit, } = require('../../scripts/markdown/check-markdown'); describe('Frontmatter Validator', () => { - let tempDir; - let mockConsoleError; - let mockProcessExit; - - beforeEach(async () => { - mockConsoleError = jest.spyOn(console, 'error').mockImplementation(); - mockProcessExit = jest.spyOn(process, 'exit').mockImplementation(); - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'test-config')); + let tempDir; + let mockConsoleError; + let mockProcessExit; + let originalEnv; + + beforeEach(async () => { + // Store original environment variables + originalEnv = { ...process.env }; + + mockConsoleError = jest.spyOn(console, 'error').mockImplementation(); + mockProcessExit = jest.spyOn(process, 'exit').mockImplementation(); + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'test-config')); + }); + + afterEach(async () => { + // Restore original environment variables + process.env = originalEnv; + + mockConsoleError.mockRestore(); + mockProcessExit.mockRestore(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + describe('Concurrency Limit Validation', () => { + it('returns default concurrency limit when no env var is set', () => { + delete process.env.MARKDOWN_CONCURRENCY_LIMIT; + const limit = getConcurrencyLimit(); + expect(limit).toBe(10); }); - afterEach(async () => { - mockConsoleError.mockRestore(); - mockProcessExit.mockRestore(); - await fs.rm(tempDir, { recursive: true, force: true }); - }); - - it('validates authors array and returns specific errors', async () => { - const frontmatter = { - title: 'Test Blog', - date: '2024-01-01', - type: 'blog', - tags: ['test'], - cover: 'cover.jpg', - authors: [{ name: 'John' }, { photo: 'jane.jpg' }, { name: 'Bob', photo: 'bob.jpg', link: 'not-a-url' }] - }; - - const errors = validateBlogs(frontmatter); - expect(errors).toEqual(expect.arrayContaining([ - 'Author at index 0 is missing a photo', - 'Author at index 1 is missing a name', - 'Invalid URL for author at index 2: not-a-url' - ])); - }); - - it('validates docs frontmatter for required fields', async () => { - const frontmatter = { title: 123, weight: 'not-a-number' }; - const errors = validateDocs(frontmatter); - expect(errors).toEqual(expect.arrayContaining([ - 'Title is missing or not a string', - 'Weight is missing or not a number' - ])); - }); - - it('checks for errors in markdown files in a directory', async () => { - await fs.writeFile(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(); - }); - - 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).toEqual([ - 'Invalid date format: invalid-date', - 'Tags should be an array', - 'Cover must be a string', - 'Authors should be an array']); - }); - - 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'); - - expect(mockConsoleError.mock.calls[0][0]).toContain('Error in directory'); - }); - - 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---`); + it('returns default concurrency limit when env var is invalid', () => { + const mockWarn = jest.spyOn(console, 'warn').mockImplementation(); - const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(); + // Test various invalid inputs + const invalidInputs = ['abc', '-1', '0', ' ']; + invalidInputs.forEach((input) => { + process.env.MARKDOWN_CONCURRENCY_LIMIT = input; + const limit = getConcurrencyLimit(); + expect(limit).toBe(10); + }); - await checkMarkdownFiles(tempDir, validateDocs); - - expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors in file reference/specification/skipped.md')); - mockConsoleLog.mockRestore(); - }); - - it('logs and rejects when an exception occurs while processing a file', async () => { - const filePath = path.join(tempDir, 'invalid.md'); - await fs.writeFile(filePath, `---\ntitle: Valid Title\n---`); - - const mockReadFile = jest.spyOn(fs, 'readFile').mockRejectedValue(new Error('Test readFile error')); - - await expect(checkMarkdownFiles(tempDir, validateBlogs)).rejects.toThrow('Test readFile error'); - expect(mockConsoleError).toHaveBeenCalledWith( - expect.stringContaining(`Error in directory`), - expect.any(Error) - ); - - mockReadFile.mockRestore(); - }); - - it('should handle main function errors and exit with status 1', async () => { - jest.spyOn(fs, 'readdir').mockRejectedValue(new Error('Test error')); - - await main(); - - expect(mockProcessExit).toHaveBeenCalledWith(1); - - expect(mockConsoleError).toHaveBeenCalledWith( - 'Failed to validate markdown files:', - expect.any(Error) - ); - }); - - it('should handle successful main function execution', async () => { - - await main(); - - expect(mockConsoleError).not.toHaveBeenCalledWith(); + mockWarn.mockRestore(); }); - it('should return true or false for URLs', () => { - expect(isValidURL('http://example.com')).toBe(true); - expect(isValidURL('https://www.example.com')).toBe(true); - expect(isValidURL('ftp://ftp.example.com')).toBe(true); - expect(isValidURL('invalid-url')).toBe(false); - expect(isValidURL('/path/to/file')).toBe(false); - expect(isValidURL('www.example.com')).toBe(false); + it('returns custom concurrency limit when env var is a valid positive integer', () => { + process.env.MARKDOWN_CONCURRENCY_LIMIT = '20'; + const limit = getConcurrencyLimit(); + expect(limit).toBe(20); }); - + }); + + it('validates authors array and returns specific errors', async () => { + const frontmatter = { + title: 'Test Blog', + date: '2024-01-01', + type: 'blog', + tags: ['test'], + cover: 'cover.jpg', + authors: [ + { name: 'John' }, + { photo: 'jane.jpg' }, + { name: 'Bob', photo: 'bob.jpg', link: 'not-a-url' }, + ], + }; + + const errors = validateBlogs(frontmatter); + expect(errors).toEqual( + expect.arrayContaining([ + 'Author at index 0 is missing a photo', + 'Author at index 1 is missing a name', + 'Invalid URL for author at index 2: not-a-url', + ]), + ); + }); + + it('validates docs frontmatter for required fields', async () => { + const frontmatter = { title: 123, weight: 'not-a-number' }; + const errors = validateDocs(frontmatter); + expect(errors).toEqual( + expect.arrayContaining([ + 'Title is missing or not a string', + 'Weight is missing or not a number', + ]), + ); + }); + + it('checks for errors in markdown files in a directory', async () => { + await fs.writeFile( + path.join(tempDir, 'invalid.md'), + `---\ntitle: Invalid Blog\n---`, + ); + const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(); + + await checkMarkdownFiles(tempDir, validateBlogs, '', pLimit(10)); + + expect(mockConsoleLog).toHaveBeenCalledWith( + expect.stringContaining('Errors in file invalid.md:'), + ); + mockConsoleLog.mockRestore(); + }); + + 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).toEqual([ + 'Invalid date format: invalid-date', + 'Tags should be an array', + 'Cover must be a string', + 'Authors should be an array', + ]); + }); + + 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, '', pLimit(10)), + ).rejects.toThrow('ENOENT'); + + expect(mockConsoleError.mock.calls[0][0]).toContain('Error in directory'); + }); + + 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---`, + ); + + const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(); + + await checkMarkdownFiles(tempDir, validateDocs, '', pLimit(10)); + + expect(mockConsoleLog).not.toHaveBeenCalledWith( + expect.stringContaining( + 'Errors in file reference/specification/skipped.md', + ), + ); + mockConsoleLog.mockRestore(); + }); + + it('logs and rejects when an exception occurs while processing a file', async () => { + const filePath = path.join(tempDir, 'invalid.md'); + await fs.writeFile(filePath, `---\ntitle: Valid Title\n---`); + + const mockReadFile = jest + .spyOn(fs, 'readFile') + .mockRejectedValue(new Error('Test readFile error')); + + await expect( + checkMarkdownFiles(tempDir, validateBlogs, '', pLimit(10)), + ).rejects.toThrow('Test readFile error'); + expect(mockConsoleError).toHaveBeenCalledWith( + expect.stringContaining(`Error in directory`), + expect.any(Error), + ); + + mockReadFile.mockRestore(); + }); + + it('should handle main function errors and exit with status 1', async () => { + jest.spyOn(fs, 'readdir').mockRejectedValue(new Error('Test error')); + + await main(); + + expect(mockProcessExit).toHaveBeenCalledWith(1); + + expect(mockConsoleError).toHaveBeenCalledWith( + 'Failed to validate markdown files:', + expect.any(Error), + ); + }); + + it('should handle successful main function execution', async () => { + await main(); + + expect(mockConsoleError).not.toHaveBeenCalledWith(); + }); + + it('should return true or false for URLs', () => { + expect(isValidURL('http://example.com')).toBe(true); + expect(isValidURL('https://www.example.com')).toBe(true); + expect(isValidURL('ftp://ftp.example.com')).toBe(true); + expect(isValidURL('invalid-url')).toBe(false); + expect(isValidURL('/path/to/file')).toBe(false); + expect(isValidURL('www.example.com')).toBe(false); + }); }); From 129b9c9618da2096274a61ec2478ca4773d4dab7 Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 14:18:31 +0530 Subject: [PATCH 2/9] feat: Added concurrency limit in markdown files processing to improve performance --- scripts/markdown/check-markdown.js | 10 +++++----- tests/markdown/check-markdown.test.js | 15 +++++---------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index aca0fbd985a7..1499cb721662 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -21,7 +21,7 @@ function getConcurrencyLimit() { // Validate the parsed limit if (Number.isNaN(parsedLimit)) { console.warn( - `Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.`, + `Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.` ); return 10; } @@ -29,7 +29,7 @@ function getConcurrencyLimit() { // Check for non-positive integers if (parsedLimit <= 0) { console.warn( - `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer. Received: ${parsedLimit}. Falling back to default of 10.`, + `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer. Received: ${parsedLimit}. Falling back to default of 10.` ); return 10; } @@ -44,7 +44,7 @@ function getConcurrencyLimit() { */ function isValidURL(str) { try { - new URL(str); + URL(str); return true; } catch (err) { return false; @@ -209,7 +209,7 @@ async function main() { await Promise.all([ checkMarkdownFiles(docsFolderPath, validateDocs, '', limit), - checkMarkdownFiles(blogsFolderPath, validateBlogs, '', limit), + checkMarkdownFiles(blogsFolderPath, validateBlogs, '', limit) ]); } catch (error) { console.error('Failed to validate markdown files:', error); @@ -228,5 +228,5 @@ module.exports = { checkMarkdownFiles, main, isValidURL, - getConcurrencyLimit, + getConcurrencyLimit }; diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index df9a71e729c7..0a59788d15ce 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -8,7 +8,7 @@ const { validateBlogs, validateDocs, checkMarkdownFiles, - getConcurrencyLimit, + getConcurrencyLimit } = require('../../scripts/markdown/check-markdown'); describe('Frontmatter Validator', () => { @@ -37,7 +37,7 @@ describe('Frontmatter Validator', () => { describe('Concurrency Limit Validation', () => { it('returns default concurrency limit when no env var is set', () => { - delete process.env.MARKDOWN_CONCURRENCY_LIMIT; + process.env.MARKDOWN_CONCURRENCY_LIMIT = undefined; const limit = getConcurrencyLimit(); expect(limit).toBe(10); }); @@ -83,7 +83,7 @@ describe('Frontmatter Validator', () => { 'Author at index 0 is missing a photo', 'Author at index 1 is missing a name', 'Invalid URL for author at index 2: not-a-url', - ]), + ]) ); }); @@ -91,10 +91,7 @@ describe('Frontmatter Validator', () => { const frontmatter = { title: 123, weight: 'not-a-number' }; const errors = validateDocs(frontmatter); expect(errors).toEqual( - expect.arrayContaining([ - 'Title is missing or not a string', - 'Weight is missing or not a number', - ]), + expect.arrayContaining(['Title is missing or not a string', 'Weight is missing or not a number']) ); }); @@ -166,9 +163,7 @@ describe('Frontmatter Validator', () => { const filePath = path.join(tempDir, 'invalid.md'); await fs.writeFile(filePath, `---\ntitle: Valid Title\n---`); - const mockReadFile = jest - .spyOn(fs, 'readFile') - .mockRejectedValue(new Error('Test readFile error')); + const mockReadFile = jest.spyOn(fs, 'readFile').mockRejectedValue(new Error('Test readFile error')); await expect( checkMarkdownFiles(tempDir, validateBlogs, '', pLimit(10)), From 9bfeb3b63a7133f72ac847cb3a936dbdf90dd9f5 Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 14:47:27 +0530 Subject: [PATCH 3/9] feat: added concurrency limit in markdown files processing to improve performance --- scripts/markdown/check-markdown.js | 16 +++++------ tests/markdown/check-markdown.test.js | 39 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index 1499cb721662..a4becbdc19a7 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -20,9 +20,7 @@ function getConcurrencyLimit() { // Validate the parsed limit if (Number.isNaN(parsedLimit)) { - console.warn( - `Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.` - ); + console.warn(`Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.`); return 10; } @@ -100,9 +98,7 @@ function validateBlogs(frontmatter) { errors.push(`Author at index ${index} is missing a name`); } if (author.link && !isValidURL(author.link)) { - errors.push( - `Invalid URL for author at index ${index}: ${author.link}`, - ); + errors.push(`Invalid URL for author at index ${index}: ${author.link}`); } if (!author.photo) { errors.push(`Author at index ${index} is missing a photo`); @@ -149,8 +145,8 @@ function validateDocs(frontmatter) { async function checkMarkdownFiles( folderPath, validateFunction, - relativePath = '', limit, + relativePath = '' ) { try { const files = await fs.readdir(folderPath); @@ -170,8 +166,8 @@ async function checkMarkdownFiles( await checkMarkdownFiles( filePath, validateFunction, - relativeFilePath, limit, + relativeFilePath ); } else if (path.extname(file) === '.md') { // Use the concurrency limiter for file processing @@ -208,8 +204,8 @@ async function main() { const limit = pLimit(concurrencyLimit); await Promise.all([ - checkMarkdownFiles(docsFolderPath, validateDocs, '', limit), - checkMarkdownFiles(blogsFolderPath, validateBlogs, '', limit) + checkMarkdownFiles(docsFolderPath, validateDocs, limit, ''), + checkMarkdownFiles(blogsFolderPath, validateBlogs, limit, '') ]); } catch (error) { console.error('Failed to validate markdown files:', error); diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 0a59788d15ce..b1746cc80777 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -61,6 +61,43 @@ describe('Frontmatter Validator', () => { const limit = getConcurrencyLimit(); expect(limit).toBe(20); }); + + it('respects concurrency limit during file processing', async () => { + const processingTimes = []; + const mockValidateFunction = jest.fn().mockImplementation(() => { + const startTime = Date.now(); + processingTimes.push(startTime); + // Simulate some processing time + return new Promise(resolve => setTimeout(resolve, 50)); + }); + + // Create multiple test files + for (let i = 0; i < 20; i++) { + await fs.writeFile( + path.join(tempDir, `test${i}.md`), + '---\ntitle: Test\n---' + ); + } + + const limit = pLimit(5); // Set limit to 5 + await checkMarkdownFiles(tempDir, mockValidateFunction, '', limit); + + // Group processing times by 5 (our limit) and verify gaps between groups + const sortedTimes = processingTimes.sort(); + const groups = []; + for (let i = 0; i < sortedTimes.length; i += 5) { + groups.push(sortedTimes.slice(i, i + 5)); + } + + // Verify that each group of 5 started processing together + groups.forEach(group => { + const groupSpread = Math.max(...group) - Math.min(...group); + expect(groupSpread).toBeLessThan(50); // Should start within 50ms of each other + }); + + // Verify that the mock validate function was called for all files + expect(mockValidateFunction).toHaveBeenCalledTimes(20); + }); }); it('validates authors array and returns specific errors', async () => { @@ -203,4 +240,4 @@ describe('Frontmatter Validator', () => { expect(isValidURL('/path/to/file')).toBe(false); expect(isValidURL('www.example.com')).toBe(false); }); -}); +}); \ No newline at end of file From 75b8d029ee0bf6f2da62188efc9dee19219595fa Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 15:04:11 +0530 Subject: [PATCH 4/9] feat: added concurrency limit in markdown files processing to improve performance --- scripts/markdown/check-markdown.js | 34 ++++++++++++++++----------- tests/markdown/check-markdown.test.js | 11 +++++++-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index a4becbdc19a7..4f42fea4cea6 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -20,14 +20,14 @@ function getConcurrencyLimit() { // Validate the parsed limit if (Number.isNaN(parsedLimit)) { - console.warn(`Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Falling back to default of 10.`); + console.warn(`Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Value must be a number. Falling back to default of 10.`); return 10; } // Check for non-positive integers if (parsedLimit <= 0) { console.warn( - `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer. Received: ${parsedLimit}. Falling back to default of 10.` + `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer greater than 0. Received: ${parsedLimit}. Falling back to default of 10.` ); return 10; } @@ -68,7 +68,7 @@ function validateBlogs(frontmatter) { // Check for required attributes requiredAttributes.forEach((attr) => { - if (!frontmatter.hasOwnProperty(attr)) { + if (!Object.hasOwn(frontmatter, attr)) { errors.push(`${attr} is missing`); } }); @@ -170,18 +170,23 @@ async function checkMarkdownFiles( relativeFilePath ); } else if (path.extname(file) === '.md') { + try{ + await limit(async () => { + 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 (error) { + console.error(`Error processing file ${relativeFilePath}:`, error); + throw error; + } // Use the concurrency limiter for file processing - await limit(async () => { - 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; - } - }); } }); @@ -199,6 +204,7 @@ async function main() { try { // Get concurrency limit from environment or use default const concurrencyLimit = getConcurrencyLimit(); + console.log(`Configured markdown processing with concurrency limit: ${concurrencyLimit}`); // Create a concurrency limiter const limit = pLimit(concurrencyLimit); diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index b1746cc80777..3dd662b6a5c1 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -30,11 +30,18 @@ describe('Frontmatter Validator', () => { // Restore original environment variables process.env = originalEnv; + // Verify environment restoration + expect(process.env).toEqual(originalEnv); + mockConsoleError.mockRestore(); mockProcessExit.mockRestore(); await fs.rm(tempDir, { recursive: true, force: true }); }); +/** +* Test suite for concurrency limit validation. +* Verifies the behavior of concurrency limits in markdown processing. +*/ describe('Concurrency Limit Validation', () => { it('returns default concurrency limit when no env var is set', () => { process.env.MARKDOWN_CONCURRENCY_LIMIT = undefined; @@ -91,8 +98,8 @@ describe('Frontmatter Validator', () => { // Verify that each group of 5 started processing together groups.forEach(group => { - const groupSpread = Math.max(...group) - Math.min(...group); - expect(groupSpread).toBeLessThan(50); // Should start within 50ms of each other + const concurrentExecutions = mockValidateFunction.mock.calls.filter(call => Math.abs(call[0] - group[0]) < 10).length; + expect(concurrentExecutions).toBeLessThanOrEqual(5); }); // Verify that the mock validate function was called for all files From 42fe248f0bda4b0de51798423938a8596eb04624 Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 23:00:32 +0530 Subject: [PATCH 5/9] fix: issues suggested by coderabbitai --- scripts/markdown/check-markdown.js | 20 +++------ tests/markdown/check-markdown.test.js | 64 +++++++++++++-------------- 2 files changed, 35 insertions(+), 49 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index 4f42fea4cea6..150deb068774 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -42,7 +42,7 @@ function getConcurrencyLimit() { */ function isValidURL(str) { try { - URL(str); + new URL(str); return true; } catch (err) { return false; @@ -142,12 +142,7 @@ function validateDocs(frontmatter) { * @param {string} [relativePath=''] - The relative path of the folder for logging purposes. * @param {import('p-limit').default} limit - Concurrency limiter. */ -async function checkMarkdownFiles( - folderPath, - validateFunction, - limit, - relativePath = '' -) { +async function checkMarkdownFiles(folderPath, validateFunction, limit, relativePath = '') { try { const files = await fs.readdir(folderPath); const filePromises = files.map(async (file) => { @@ -163,14 +158,9 @@ async function checkMarkdownFiles( // Recurse if directory, otherwise validate markdown file if (stats.isDirectory()) { - await checkMarkdownFiles( - filePath, - validateFunction, - limit, - relativeFilePath - ); + await checkMarkdownFiles(filePath, validateFunction, limit, relativeFilePath); } else if (path.extname(file) === '.md') { - try{ + try { await limit(async () => { const fileContent = await fs.readFile(filePath, 'utf-8'); const { data: frontmatter } = matter(fileContent); @@ -182,7 +172,7 @@ async function checkMarkdownFiles( process.exitCode = 1; } }); - }catch (error) { + } catch (error) { console.error(`Error processing file ${relativeFilePath}:`, error); throw error; } diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 3dd662b6a5c1..50d468088ebb 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -70,38 +70,40 @@ describe('Frontmatter Validator', () => { }); it('respects concurrency limit during file processing', async () => { - const processingTimes = []; - const mockValidateFunction = jest.fn().mockImplementation(() => { - const startTime = Date.now(); - processingTimes.push(startTime); - // Simulate some processing time - return new Promise(resolve => setTimeout(resolve, 50)); + let activeCount = 0; + let maxActiveCount = 0; + const mockValidateFunction = jest.fn().mockImplementation(async () => { + activeCount++; + try { + // Track the maximum number of concurrent executions + if (activeCount > maxActiveCount) { + maxActiveCount = activeCount; + } + + // Simulate some processing time + await new Promise(resolve => setTimeout(resolve, 50)); + } finally { + activeCount--; + } }); - - // Create multiple test files + + const writePromises = []; for (let i = 0; i < 20; i++) { - await fs.writeFile( - path.join(tempDir, `test${i}.md`), - '---\ntitle: Test\n---' + writePromises.push( + fs.writeFile( + path.join(tempDir, `test${i}.md`), + '---\ntitle: Test\n---' + ) ); } - + await Promise.all(writePromises); + const limit = pLimit(5); // Set limit to 5 await checkMarkdownFiles(tempDir, mockValidateFunction, '', limit); - - // Group processing times by 5 (our limit) and verify gaps between groups - const sortedTimes = processingTimes.sort(); - const groups = []; - for (let i = 0; i < sortedTimes.length; i += 5) { - groups.push(sortedTimes.slice(i, i + 5)); - } - - // Verify that each group of 5 started processing together - groups.forEach(group => { - const concurrentExecutions = mockValidateFunction.mock.calls.filter(call => Math.abs(call[0] - group[0]) < 10).length; - expect(concurrentExecutions).toBeLessThanOrEqual(5); - }); - + + // Verify that the maximum number of concurrent executions never exceeds the limit + expect(maxActiveCount).toBeLessThanOrEqual(5); + // Verify that the mock validate function was called for all files expect(mockValidateFunction).toHaveBeenCalledTimes(20); }); @@ -114,11 +116,7 @@ describe('Frontmatter Validator', () => { type: 'blog', tags: ['test'], cover: 'cover.jpg', - authors: [ - { name: 'John' }, - { photo: 'jane.jpg' }, - { name: 'Bob', photo: 'bob.jpg', link: 'not-a-url' }, - ], + authors: [{ name: 'John' }, { photo: 'jane.jpg' }, { name: 'Bob', photo: 'bob.jpg', link: 'not-a-url' }] }; const errors = validateBlogs(frontmatter); @@ -148,9 +146,7 @@ describe('Frontmatter Validator', () => { await checkMarkdownFiles(tempDir, validateBlogs, '', pLimit(10)); - expect(mockConsoleLog).toHaveBeenCalledWith( - expect.stringContaining('Errors in file invalid.md:'), - ); + expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('Errors in file invalid.md:')); mockConsoleLog.mockRestore(); }); From e9efe751abbf803e6e62e09d19b33d9260ca151f Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 16 Dec 2024 23:21:54 +0530 Subject: [PATCH 6/9] fix: issues suggested by coderabbitai --- scripts/markdown/check-markdown.js | 31 ++++++++++++--- tests/markdown/check-markdown.test.js | 54 ++++++++++++++------------- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index 150deb068774..6acdf9154e6b 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -3,6 +3,8 @@ const matter = require('gray-matter'); const path = require('path'); const pLimit = require('p-limit'); +const DEFAULT_CONCURRENCY_LIMIT = 10; + /** * Validates and retrieves the concurrency limit from environment variables. * @returns {number} The validated concurrency limit. @@ -12,7 +14,7 @@ function getConcurrencyLimit() { // If no env var is set, return default if (envLimit === undefined) { - return 10; + return DEFAULT_CONCURRENCY_LIMIT; } // Attempt to parse the environment variable @@ -21,7 +23,7 @@ function getConcurrencyLimit() { // Validate the parsed limit if (Number.isNaN(parsedLimit)) { console.warn(`Invalid MARKDOWN_CONCURRENCY_LIMIT: '${envLimit}'. Value must be a number. Falling back to default of 10.`); - return 10; + return DEFAULT_CONCURRENCY_LIMIT; } // Check for non-positive integers @@ -29,7 +31,7 @@ function getConcurrencyLimit() { console.warn( `MARKDOWN_CONCURRENCY_LIMIT must be a positive integer greater than 0. Received: ${parsedLimit}. Falling back to default of 10.` ); - return 10; + return DEFAULT_CONCURRENCY_LIMIT; } return parsedLimit; @@ -141,6 +143,11 @@ function validateDocs(frontmatter) { * @param {Function} validateFunction - The function used to validate the frontmatter. * @param {string} [relativePath=''] - The relative path of the folder for logging purposes. * @param {import('p-limit').default} limit - Concurrency limiter. + * @throws {Error} When the concurrency limiter fails or when file operations fail + * @example + * // Process files with a concurrency limit of 5 + * const limit = pLimit(5); + * await checkMarkdownFiles(folderPath, validateFn, limit); */ async function checkMarkdownFiles(folderPath, validateFunction, limit, relativePath = '') { try { @@ -173,7 +180,11 @@ async function checkMarkdownFiles(folderPath, validateFunction, limit, relativeP } }); } catch (error) { - console.error(`Error processing file ${relativeFilePath}:`, error); + console.error(`Error processing markdown file ${relativeFilePath} (validation failed):`, { + error: error.message, + code: error.code, + stack: error.stack + }); throw error; } // Use the concurrency limiter for file processing @@ -182,7 +193,11 @@ async function checkMarkdownFiles(folderPath, validateFunction, limit, relativeP await Promise.all(filePromises); } catch (err) { - console.error(`Error in directory ${folderPath}:`, err); + console.error(`Failed to process markdown files in directory ${folderPath}:`, { + error: err.message, + code: err.code, + stack: err.stack + }); throw err; } } @@ -204,7 +219,11 @@ async function main() { checkMarkdownFiles(blogsFolderPath, validateBlogs, limit, '') ]); } catch (error) { - console.error('Failed to validate markdown files:', error); + console.error('Markdown validation process failed:', { + error: error.message, + code: error.code, + stack: error.stack + }); process.exit(1); } } diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 50d468088ebb..5966517f8397 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -69,7 +69,10 @@ describe('Frontmatter Validator', () => { expect(limit).toBe(20); }); - it('respects concurrency limit during file processing', async () => { + it('should process files concurrently while respecting the configured limit', async () => { + const CONCURRENCY_LIMIT = 5; + const TOTAL_FILES = 20; + const PROCESSING_TIME_MS = 50; let activeCount = 0; let maxActiveCount = 0; const mockValidateFunction = jest.fn().mockImplementation(async () => { @@ -81,14 +84,14 @@ describe('Frontmatter Validator', () => { } // Simulate some processing time - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise(resolve => setTimeout(resolve, PROCESSING_TIME_MS)); } finally { activeCount--; } }); const writePromises = []; - for (let i = 0; i < 20; i++) { + for (let i = 0; i < TOTAL_FILES; i++) { writePromises.push( fs.writeFile( path.join(tempDir, `test${i}.md`), @@ -98,14 +101,13 @@ describe('Frontmatter Validator', () => { } await Promise.all(writePromises); - const limit = pLimit(5); // Set limit to 5 + const limit = pLimit(CONCURRENCY_LIMIT); await checkMarkdownFiles(tempDir, mockValidateFunction, '', limit); - // Verify that the maximum number of concurrent executions never exceeds the limit - expect(maxActiveCount).toBeLessThanOrEqual(5); + // Verify concurrent execution bounds + expect(maxActiveCount).toBe(CONCURRENCY_LIMIT); - // Verify that the mock validate function was called for all files - expect(mockValidateFunction).toHaveBeenCalledTimes(20); + expect(mockValidateFunction).toHaveBeenCalledTimes(TOTAL_FILES); }); }); @@ -150,23 +152,25 @@ describe('Frontmatter Validator', () => { mockConsoleLog.mockRestore(); }); - 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).toEqual([ - 'Invalid date format: invalid-date', - 'Tags should be an array', - 'Cover must be a string', - 'Authors should be an array', - ]); + describe('Blog Frontmatter Validation', () => { + it('should return all validation errors when multiple fields are invalid', 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).toEqual([ + 'Invalid date format: invalid-date', + 'Tags should be an array', + 'Cover must be a string', + 'Authors should be an array', + ]); + }) }); it('logs error to console when an error occurs in checkMarkdownFiles', async () => { From c8e7e44e09c4cedff2f454f39a8d40012338d7a7 Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Tue, 17 Dec 2024 10:41:10 +0530 Subject: [PATCH 7/9] fix: issues suggested by coderabbitai --- scripts/markdown/check-markdown.js | 9 +-------- tests/markdown/check-markdown.test.js | 11 ++--------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index 6acdf9154e6b..9abce4c48dfc 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -58,14 +58,7 @@ function isValidURL(str) { * @returns {string[]|null} An array of validation error messages, or null if no errors. */ function validateBlogs(frontmatter) { - const requiredAttributes = [ - 'title', - 'date', - 'type', - 'tags', - 'cover', - 'authors', - ]; + const requiredAttributes = ['title', 'date', 'type', 'tags', 'cover', 'authors']; const errors = []; // Check for required attributes diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 5966517f8397..095195877f85 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -195,11 +195,7 @@ describe('Frontmatter Validator', () => { await checkMarkdownFiles(tempDir, validateDocs, '', pLimit(10)); - expect(mockConsoleLog).not.toHaveBeenCalledWith( - expect.stringContaining( - 'Errors in file reference/specification/skipped.md', - ), - ); + expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('Errors in file reference/specification/skipped.md')); mockConsoleLog.mockRestore(); }); @@ -227,10 +223,7 @@ describe('Frontmatter Validator', () => { expect(mockProcessExit).toHaveBeenCalledWith(1); - expect(mockConsoleError).toHaveBeenCalledWith( - 'Failed to validate markdown files:', - expect.any(Error), - ); + expect(mockConsoleError).toHaveBeenCalledWith('Failed to validate markdown files:',expect.any(Error)); }); it('should handle successful main function execution', async () => { From a457e103415170e75cbd577d32e14733c59192fa Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 30 Dec 2024 12:03:42 +0530 Subject: [PATCH 8/9] fix: p-limit version changed to resolve import issue --- package-lock.json | 164 +++++----------------------------------------- package.json | 2 +- 2 files changed, 18 insertions(+), 148 deletions(-) diff --git a/package-lock.json b/package-lock.json index cdafa181093b..5358fe1432fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,7 +54,7 @@ "next-mdx-remote": "^4.4.1", "node-fetch": "^3.3.2", "node-fetch-2": "npm:node-fetch@^2.7.0", - "p-limit": "^6.1.0", + "p-limit": "^3.1.0", "postcss": "^8.4.35", "prettier": "^3.3.3", "react": "^18", @@ -4811,35 +4811,6 @@ } } }, - "node_modules/@netlify/plugin-nextjs/node_modules/p-limit": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", - "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "yocto-queue": "^0.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/@netlify/plugin-nextjs/node_modules/yocto-queue": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", - "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/@netlify/serverless-functions-api": { "version": "1.18.4", "resolved": "https://registry.npmjs.org/@netlify/serverless-functions-api/-/serverless-functions-api-1.18.4.tgz", @@ -16695,35 +16666,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/jest-changed-files/node_modules/p-limit": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", - "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "yocto-queue": "^0.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/jest-changed-files/node_modules/yocto-queue": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", - "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/jest-circus": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-circus/-/jest-circus-29.7.0.tgz", @@ -16767,22 +16709,6 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, - "node_modules/jest-circus/node_modules/p-limit": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", - "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "yocto-queue": "^0.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/jest-circus/node_modules/pretty-format": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/pretty-format/-/pretty-format-29.7.0.tgz", @@ -16803,19 +16729,6 @@ "integrity": "sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg==", "dev": true }, - "node_modules/jest-circus/node_modules/yocto-queue": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", - "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/jest-cli": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-cli/-/jest-cli-29.7.0.tgz", @@ -17376,35 +17289,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/jest-runner/node_modules/p-limit": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", - "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "yocto-queue": "^0.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/jest-runner/node_modules/yocto-queue": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", - "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/jest-runtime": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-runtime/-/jest-runtime-29.7.0.tgz", @@ -23191,35 +23075,6 @@ } }, "node_modules/p-limit": { - "version": "6.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-6.1.0.tgz", - "integrity": "sha512-H0jc0q1vOzlEk0TqAKXKZxdl7kX3OFUzCnNVUnq5Pc3DGo0kpeaMuPqxQn235HibwBEb0/pm9dgKTjXy66fBkg==", - "license": "MIT", - "dependencies": { - "yocto-queue": "^1.1.1" - }, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/p-locate": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-5.0.0.tgz", - "integrity": "sha512-LaNjtRWUBY++zB5nE/NwcaoMylSPk+S+ZHNB1TzdbMJMny6dynpAGt7X/tl/QYq3TIeE6nxHppbo2LGymrG5Pw==", - "dependencies": { - "p-limit": "^3.0.2" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/p-locate/node_modules/p-limit": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", @@ -23234,7 +23089,7 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/p-locate/node_modules/yocto-queue": { + "node_modules/p-limit/node_modules/yocto-queue": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", @@ -23246,6 +23101,20 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/p-locate": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-5.0.0.tgz", + "integrity": "sha512-LaNjtRWUBY++zB5nE/NwcaoMylSPk+S+ZHNB1TzdbMJMny6dynpAGt7X/tl/QYq3TIeE6nxHppbo2LGymrG5Pw==", + "dependencies": { + "p-limit": "^3.0.2" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/p-queue": { "version": "6.6.2", "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-6.6.2.tgz", @@ -30591,6 +30460,7 @@ "version": "1.1.1", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-1.1.1.tgz", "integrity": "sha512-b4JR1PFR10y1mKjhHY9LaGo6tmrgjit7hxVIeAmyMw3jegXR4dhYqLaQF5zMXZxY7tLpMyJeLjr1C4rLmkVe8g==", + "dev": true, "license": "MIT", "engines": { "node": ">=12.20" diff --git a/package.json b/package.json index dd4c75dc5669..9450a8797375 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "next-mdx-remote": "^4.4.1", "node-fetch": "^3.3.2", "node-fetch-2": "npm:node-fetch@^2.7.0", - "p-limit": "^6.1.0", + "p-limit": "^3.1.0", "postcss": "^8.4.35", "prettier": "^3.3.3", "react": "^18", From 41c4a5dcea5a631c4593e74830c4aab417591f06 Mon Sep 17 00:00:00 2001 From: Aditya Patil - Parallel Minds Date: Mon, 30 Dec 2024 12:20:04 +0530 Subject: [PATCH 9/9] fix: fixed some test issues in check-markdown.test.js file --- scripts/markdown/check-markdown.js | 30 ++++++------- tests/markdown/check-markdown.test.js | 63 +++++++++++++-------------- 2 files changed, 44 insertions(+), 49 deletions(-) diff --git a/scripts/markdown/check-markdown.js b/scripts/markdown/check-markdown.js index 9abce4c48dfc..2052f9f7080e 100644 --- a/scripts/markdown/check-markdown.js +++ b/scripts/markdown/check-markdown.js @@ -149,19 +149,18 @@ async function checkMarkdownFiles(folderPath, validateFunction, limit, relativeP const filePath = path.join(folderPath, file); const relativeFilePath = path.join(relativePath, file); + const stats = await fs.stat(filePath); + // Skip the folder 'docs/reference/specification' if (relativeFilePath.includes('reference/specification')) { return; } - const stats = await fs.stat(filePath); - - // Recurse if directory, otherwise validate markdown file if (stats.isDirectory()) { await checkMarkdownFiles(filePath, validateFunction, limit, relativeFilePath); } else if (path.extname(file) === '.md') { - try { - await limit(async () => { + return limit(async () => { + try { const fileContent = await fs.readFile(filePath, 'utf-8'); const { data: frontmatter } = matter(fileContent); @@ -171,20 +170,19 @@ async function checkMarkdownFiles(folderPath, validateFunction, limit, relativeP errors.forEach((error) => console.log(` - ${error}`)); process.exitCode = 1; } - }); - } catch (error) { - console.error(`Error processing markdown file ${relativeFilePath} (validation failed):`, { - error: error.message, - code: error.code, - stack: error.stack - }); - throw error; - } - // Use the concurrency limiter for file processing + } catch (error) { + console.error(`Error processing markdown file ${relativeFilePath} (validation failed):`, { + error: error.message, + code: error.code, + stack: error.stack + }); + throw error; + } + }); } }); - await Promise.all(filePromises); + await Promise.all(filePromises.filter(Boolean)); } catch (err) { console.error(`Failed to process markdown files in directory ${folderPath}:`, { error: err.message, diff --git a/tests/markdown/check-markdown.test.js b/tests/markdown/check-markdown.test.js index 095195877f85..02e8a8165838 100644 --- a/tests/markdown/check-markdown.test.js +++ b/tests/markdown/check-markdown.test.js @@ -75,38 +75,28 @@ describe('Frontmatter Validator', () => { const PROCESSING_TIME_MS = 50; let activeCount = 0; let maxActiveCount = 0; - const mockValidateFunction = jest.fn().mockImplementation(async () => { + + const mockValidateFunction = jest.fn().mockImplementation(async (frontmatter) => { activeCount++; - try { - // Track the maximum number of concurrent executions - if (activeCount > maxActiveCount) { - maxActiveCount = activeCount; - } - - // Simulate some processing time - await new Promise(resolve => setTimeout(resolve, PROCESSING_TIME_MS)); - } finally { - activeCount--; - } + maxActiveCount = Math.max(maxActiveCount, activeCount); + await new Promise(resolve => setTimeout(resolve, PROCESSING_TIME_MS)); + activeCount--; + return null; // No validation errors }); - const writePromises = []; - for (let i = 0; i < TOTAL_FILES; i++) { - writePromises.push( - fs.writeFile( - path.join(tempDir, `test${i}.md`), - '---\ntitle: Test\n---' - ) - ); - } + // Create test files + const writePromises = Array.from({ length: TOTAL_FILES }).map((_, i) => + fs.writeFile( + path.join(tempDir, `test${i}.md`), + '---\ntitle: Test\n---' + ) + ); await Promise.all(writePromises); const limit = pLimit(CONCURRENCY_LIMIT); - await checkMarkdownFiles(tempDir, mockValidateFunction, '', limit); - - // Verify concurrent execution bounds - expect(maxActiveCount).toBe(CONCURRENCY_LIMIT); + await checkMarkdownFiles(tempDir, mockValidateFunction, limit, ''); + expect(maxActiveCount).toBeLessThanOrEqual(CONCURRENCY_LIMIT); expect(mockValidateFunction).toHaveBeenCalledTimes(TOTAL_FILES); }); }); @@ -175,12 +165,15 @@ describe('Frontmatter Validator', () => { 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, '', pLimit(10)), + checkMarkdownFiles(invalidFolderPath, validateBlogs, pLimit(10), '') ).rejects.toThrow('ENOENT'); - - expect(mockConsoleError.mock.calls[0][0]).toContain('Error in directory'); + + expect(mockConsoleError).toHaveBeenCalledWith( + expect.stringContaining('Failed to process markdown files in directory'), + expect.any(Object) + ); }); it('skips the "reference/specification" folder during validation', async () => { @@ -218,12 +211,16 @@ describe('Frontmatter Validator', () => { it('should handle main function errors and exit with status 1', async () => { jest.spyOn(fs, 'readdir').mockRejectedValue(new Error('Test error')); - + await main(); - + expect(mockProcessExit).toHaveBeenCalledWith(1); - - expect(mockConsoleError).toHaveBeenCalledWith('Failed to validate markdown files:',expect.any(Error)); + expect(mockConsoleError).toHaveBeenCalledWith( + 'Markdown validation process failed:', + expect.objectContaining({ + error: 'Test error' + }) + ); }); it('should handle successful main function execution', async () => {