Skip to content

Commit

Permalink
fix: issues suggested by coderabbitai
Browse files Browse the repository at this point in the history
  • Loading branch information
Aditya0733 committed Dec 16, 2024
1 parent 42fe248 commit e9efe75
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 31 deletions.
31 changes: 25 additions & 6 deletions scripts/markdown/check-markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -21,15 +23,15 @@ 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
if (parsedLimit <= 0) {
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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
}
Expand All @@ -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);
}
}
Expand Down
54 changes: 29 additions & 25 deletions tests/markdown/check-markdown.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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`),
Expand All @@ -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);
});
});

Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit e9efe75

Please sign in to comment.