Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add tests for build post list script #3284

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions scripts/build-post-list.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { readdirSync, statSync, existsSync, readFileSync, writeFileSync } = require('fs')
const { resolve, basename } = require('path')
const { basename } = require('path')
const frontMatter = require('gray-matter')
const toc = require('markdown-toc')
const { slugify } = require('markdown-toc/lib/utils')
Expand All @@ -15,36 +15,36 @@ const result = {
docsTree: {}
}
const releaseNotes = []
const basePath = 'pages'
const postDirectories = [
// order of these directories is important, as the blog should come before docs, to create a list of available release notes, which will later be used to release-note-link for spec docs
[`${basePath}/blog`, '/blog'],
[`${basePath}/docs`, '/docs'],
[`${basePath}/about`, '/about']
];

const addItem = (details) => {
if(details.slug.startsWith('/docs'))
if (details.slug.startsWith('/docs'))
result["docs"].push(details)
else if(details.slug.startsWith('/blog'))
else if (details.slug.startsWith('/blog'))
result["blog"].push(details)
else if(details.slug.startsWith('/about'))
else if (details.slug.startsWith('/about'))
result["about"].push(details)
else {}
else { }
Comment on lines +20 to +26
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unnecessary empty else block in addItem function

The empty else { } block at line 26 serves no purpose and can be removed to clean up the code.

Apply this diff to remove the empty block:

  else if (details.slug.startsWith('/about'))
    result["about"].push(details)
- else { }
📝 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.

Suggested change
if (details.slug.startsWith('/docs'))
result["docs"].push(details)
else if(details.slug.startsWith('/blog'))
else if (details.slug.startsWith('/blog'))
result["blog"].push(details)
else if(details.slug.startsWith('/about'))
else if (details.slug.startsWith('/about'))
result["about"].push(details)
else {}
else { }
if (details.slug.startsWith('/docs'))
result["docs"].push(details)
else if (details.slug.startsWith('/blog'))
result["blog"].push(details)
else if (details.slug.startsWith('/about'))
result["about"].push(details)

}

module.exports = async function buildPostList() {
walkDirectories(postDirectories, result)
const treePosts = buildNavTree(result["docs"].filter((p) => p.slug.startsWith('/docs/')))
result["docsTree"] = treePosts
result["docs"] = addDocButtons(result["docs"], treePosts)
if (process.env.NODE_ENV === 'production') {
// console.log(inspect(result, { depth: null, colors: true }))
async function buildPostList(postDirectories, basePath, writeFilePath) {
try {
if (postDirectories.length === 0) {
throw new Error('Error while building post list: No post directories provided');
}
walkDirectories(postDirectories, result, basePath)
const treePosts = buildNavTree(result["docs"].filter((p) => p.slug.startsWith('/docs/')))
result["docsTree"] = treePosts
result["docs"] = addDocButtons(result["docs"], treePosts)
if (process.env.NODE_ENV === 'production') {
// console.log(inspect(result, { depth: null, colors: true }))
}
writeFileSync(writeFilePath, JSON.stringify(result, null, ' '))
} catch (error) {
throw new Error(`Error while building post list: ${error.message}`);
Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Preserve original error stack trace when rethrowing errors

When rethrowing an error in the catch block, consider using throw error to preserve the original stack trace, or use the cause property to retain the error's context.

Apply this diff to preserve the original error:

  } catch (error) {
-    throw new Error(`Error while building post list: ${error.message}`);
+    throw error;
  }

Alternatively, to add context while preserving the original error:

  } catch (error) {
+    throw new Error('Error while building post list', { cause: error });
  }

Note: The cause option is available in Node.js v16.9.0 and newer.

📝 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.

Suggested change
} catch (error) {
throw new Error(`Error while building post list: ${error.message}`);
} catch (error) {
throw error;
}
```
Option 2: Add context while preserving the original error
```suggestion
} catch (error) {
throw new Error('Error while building post list', { cause: error });
}

}
writeFileSync(resolve(__dirname, '..', 'config', 'posts.json'), JSON.stringify(result, null, ' '))
}

function walkDirectories(directories, result, sectionWeight = 0, sectionTitle, sectionId, rootSectionId) {
function walkDirectories(directories, result, basePath, sectionWeight = 0, sectionTitle, sectionId, rootSectionId) {
for (let dir of directories) {
let directory = dir[0]
let sectionSlug = dir[1] || ''
Expand All @@ -68,8 +68,8 @@ function walkDirectories(directories, result, sectionWeight = 0, sectionTitle, s
}
details.isSection = true
if (slugElements.length > 3) {
details.parent = slugElements[slugElements.length - 2]
details.sectionId = slugElements[slugElements.length - 1]
details.parent = slugElements[slugElements.length - 2]
details.sectionId = slugElements[slugElements.length - 1]
}
if (!details.parent) {
details.isRootSection = true
Expand All @@ -79,7 +79,7 @@ function walkDirectories(directories, result, sectionWeight = 0, sectionTitle, s
details.slug = slug
addItem(details)
const rootId = details.parent || details.rootSectionId
walkDirectories([[fileName, slug]], result, details.weight, details.title, details.sectionId, rootId)
walkDirectories([[fileName, slug]], result, basePath, details.weight, details.title, details.sectionId, rootId)
} else if (file.endsWith('.mdx') && !fileName.endsWith('/_section.mdx')) {
const fileContent = readFileSync(fileName, 'utf-8')
// Passing a second argument to frontMatter disables cache. See https://github.com/asyncapi/website/issues/1057
Expand All @@ -96,18 +96,18 @@ function walkDirectories(directories, result, sectionWeight = 0, sectionTitle, s
details.id = fileName
details.isIndex = fileName.endsWith('/index.mdx')
details.slug = details.isIndex ? sectionSlug : slug.replace(/\.mdx$/, '')
if(details.slug.includes('/reference/specification/') && !details.title) {
if (details.slug.includes('/reference/specification/') && !details.title) {
const fileBaseName = basename(data.slug) // ex. v2.0.0 | v2.1.0-next-spec.1
const fileName = fileBaseName.split('-')[0] // v2.0.0 | v2.1.0
details.weight = specWeight--

if (fileName.startsWith('v')) {
details.title = capitalize(fileName.slice(1))
details.title = capitalize(fileName.slice(1))
Comment on lines +99 to +105
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid reusing variable names to prevent confusion

The variable fileName is redefined within this block, which may cause confusion with the outer fileName. Consider renaming the inner fileName variable to avoid shadowing.

Apply this diff to rename the inner fileName to versionName:

  const fileBaseName = basename(data.slug)  // ex. v2.0.0 | v2.1.0-next-spec.1
- const fileName = fileBaseName.split('-')[0] // v2.0.0 | v2.1.0
+ const versionName = fileBaseName.split('-')[0] // v2.0.0 | v2.1.0
  details.weight = specWeight--

- if (fileName.startsWith('v')) {
-   details.title = capitalize(fileName.slice(1))
+ if (versionName.startsWith('v')) {
+   details.title = capitalize(versionName.slice(1))
  } else {
-   details.title = capitalize(fileName)
+   details.title = capitalize(versionName)
  }
📝 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.

Suggested change
if (details.slug.includes('/reference/specification/') && !details.title) {
const fileBaseName = basename(data.slug) // ex. v2.0.0 | v2.1.0-next-spec.1
const fileName = fileBaseName.split('-')[0] // v2.0.0 | v2.1.0
details.weight = specWeight--
if (fileName.startsWith('v')) {
details.title = capitalize(fileName.slice(1))
details.title = capitalize(fileName.slice(1))
if (details.slug.includes('/reference/specification/') && !details.title) {
const fileBaseName = basename(data.slug) // ex. v2.0.0 | v2.1.0-next-spec.1
const versionName = fileBaseName.split('-')[0] // v2.0.0 | v2.1.0
details.weight = specWeight--
if (versionName.startsWith('v')) {
details.title = capitalize(versionName.slice(1))
} else {
details.title = capitalize(versionName)
}

} else {
details.title = capitalize(fileName)
}

if(releaseNotes.includes(details.title)){
if (releaseNotes.includes(details.title)) {
details.releaseNoteLink = `/blog/release-notes-${details.title}`
}

Expand All @@ -122,10 +122,10 @@ function walkDirectories(directories, result, sectionWeight = 0, sectionTitle, s
}

// To create a list of available ReleaseNotes list, which will be used to add details.releaseNoteLink attribute.
if(file.startsWith("release-notes") && dir[1] === "/blog"){
const fileName_without_extension = file.slice(0,-4)
if (file.startsWith("release-notes") && dir[1] === "/blog") {
const fileName_without_extension = file.slice(0, -4)
// removes the file extension. For example, release-notes-2.1.0.md -> release-notes-2.1.0
const version = fileName_without_extension.slice(fileName_without_extension.lastIndexOf("-")+1)
const version = fileName_without_extension.slice(fileName_without_extension.lastIndexOf("-") + 1)
Comment on lines +126 to +128
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use path module functions to handle file extensions safely

Instead of manually slicing the file name to remove the extension, use path.basename and path.extname for safer and more reliable handling of file names and extensions.

Apply this diff:

- const fileName_without_extension = file.slice(0, -4)
+ const fileNameWithoutExtension = basename(file, extname(file))

Don't forget to import the required functions at the top of the file:

  const { resolve, basename } = require('path')
+ const { extname } = require('path')

Update subsequent code to use fileNameWithoutExtension.

Committable suggestion was skipped due to low confidence.


// gets the version from the name of the releaseNote .md file (from /blog). For example, version = 2.1.0 if fileName_without_extension = release-notes-2.1.0
releaseNotes.push(version)
Expand Down Expand Up @@ -159,3 +159,5 @@ function isDirectory(dir) {
function capitalize(text) {
return text.split(/[\s\-]/g).map(word => `${word[0].toUpperCase()}${word.substr(1)}`).join(' ')
}

module.exports = { slugifyToC, buildPostList }
14 changes: 12 additions & 2 deletions scripts/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
const { resolve } = require('path');
const fs = require('fs');
const rssFeed = require('./build-rss');
const buildPostList = require('./build-post-list');
const { buildPostList } = require('./build-post-list');
const buildCaseStudiesList = require('./casestudies');
const buildAdoptersList = require('./adopters');
const buildFinanceInfoList = require('./finance');

async function start() {
await buildPostList();

const postDirectories = [
['pages/blog', '/blog'],
['pages/docs', '/docs'],
['pages/about', '/about']
];
const basePath = 'pages';
const writeFilePath = resolve(__dirname, '../config', 'posts.json');

await buildPostList(postDirectories, basePath, writeFilePath);

rssFeed(
'blog',
'AsyncAPI Initiative Blog RSS Feed',
Expand Down
192 changes: 192 additions & 0 deletions tests/build-post-list.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
const { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync } = require('fs');
const { resolve, join } = require('path');
const { buildPostList, slugifyToC } = require('../scripts/build-post-list');

describe('buildPostList', () => {
let tempDir;
let writeFilePath;
let postDirectories;

beforeEach(() => {
tempDir = resolve(__dirname, `test-config`);
writeFilePath = resolve(tempDir, 'posts.json');
postDirectories = [
[join(tempDir, 'blog'), '/blog'],
[join(tempDir, 'docs'), '/docs'],
[join(tempDir, 'about'), '/about'],
];

mkdirSync(tempDir, { recursive: true });

mkdirSync(join(tempDir, 'blog'), { recursive: true });
writeFileSync(join(tempDir, 'blog', 'release-notes-2.1.0.mdx'), '---\ntitle: Release Notes 2.1.0\n---\nThis is a release note.');

mkdirSync(join(tempDir, 'docs'), { recursive: true });
writeFileSync(join(tempDir, 'docs', 'index.mdx'), '---\ntitle: Docs Home\n---\nThis is the documentation homepage.');

mkdirSync(join(tempDir, 'about'), { recursive: true });
writeFileSync(join(tempDir, 'about', 'index.mdx'), '---\ntitle: About Us\n---\nThis is the about page.');

mkdirSync(join(tempDir, 'docs', 'reference', 'specification'), { recursive: true });
});

afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});

it('builds a post list and writes the result to a file', async () => {

await buildPostList(postDirectories, tempDir, writeFilePath);

const outputExists = existsSync(writeFilePath);
expect(outputExists).toBe(true);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));

expect(output).toHaveProperty('docs');
expect(output).toHaveProperty('blog');
expect(output).toHaveProperty('about');
expect(output).toHaveProperty('docsTree');

const blogEntry = output.blog.find(item => item.slug === '/blog/release-notes-2.1.0');
expect(blogEntry).toBeDefined();
expect(blogEntry.title).toBe('Release Notes 2.1.0');
});

it('handles a directory with only section files', async () => {
mkdirSync(join(tempDir, 'docs', 'section1'), { recursive: true });
writeFileSync(join(tempDir, 'docs', 'section1', '_section.mdx'), '---\ntitle: Section 1\n---\nThis is section 1.');

await buildPostList(postDirectories, tempDir, writeFilePath);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));

expect(output.docs.length).toBeGreaterThan(0);
expect(output.docs.find(item => item.title === 'Section 1')).toBeDefined();
});

it('handles multiple release notes correctly', async () => {
writeFileSync(join(tempDir, 'blog', 'release-notes-2.1.1.mdx'), '---\ntitle: Release Notes 2.1.1\n---\nThis is a release note.');

await buildPostList(postDirectories, tempDir, writeFilePath);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));

const firstReleaseNote = output.blog.find(item => item.slug === '/blog/release-notes-2.1.0');
const secondReleaseNote = output.blog.find(item => item.slug === '/blog/release-notes-2.1.1');

expect(firstReleaseNote).toBeDefined();
expect(firstReleaseNote.title).toBe('Release Notes 2.1.0');

expect(secondReleaseNote).toBeDefined();
expect(secondReleaseNote.title).toBe('Release Notes 2.1.1');
});

it('handles errors gracefully', async () => {
const invalidDir = [join(tempDir, 'non-existent-dir'), '/invalid'];
await expect(buildPostList([invalidDir], tempDir, writeFilePath)).rejects.toThrow();
});

it('handles heading ids like {# myHeadingId}', () => {
const input = '## My Heading {#custom-id}';
expect(slugifyToC(input)).toBe('custom-id');
});

it('handles heading ids like {<a name="myHeadingId"/>}', () => {
const input = '## My Heading {<a name="custom-anchor-id"/>}';
expect(slugifyToC(input)).toBe('custom-anchor-id');
});

it('handles empty strings', () => {
expect(slugifyToC('')).toBe('');
});

it('does not process specification files without a title', async () => {
const specDir = join(tempDir, 'docs', 'reference', 'specification');
writeFileSync(
join(specDir, 'v2.1.0-no-title.mdx'),
'---\n---\nContent of specification without a title.'
);

await buildPostList(postDirectories, tempDir, writeFilePath);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));
const noTitleEntry = output.docs.find(item => item.slug.includes('/reference/specification/v2.1.0-no-title'));

expect(noTitleEntry).toBeUndefined();
});

it('does not process specification files with "next-spec" in the filename', async () => {
const specDir = join(tempDir, 'docs', 'reference', 'specification');
writeFileSync(
join(specDir, 'v2.1.0-next-spec.1.mdx'),
'---\n---\nContent of pre-release specification v2.1.0-next-spec.1.'
);

await buildPostList(postDirectories, tempDir, writeFilePath);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));
const nextSpecEntry = output.docs.find(item => item.slug.includes('/reference/specification/v2.1.0-next-spec.1'));

expect(nextSpecEntry).toBeUndefined();
});

it('does not process specification files with "explorer" in the filename', async () => {
const specDir = join(tempDir, 'docs', 'reference', 'specification');
writeFileSync(
join(specDir, 'explorer.mdx'),
'---\n---\nContent of explorer specification.'
);

await buildPostList(postDirectories, tempDir, writeFilePath);

const output = JSON.parse(readFileSync(writeFilePath, 'utf-8'));
const explorerEntry = output.docs.find(item => item.slug.includes('/reference/specification/explorer'));

expect(explorerEntry).toBeUndefined();
});

it('throws an error if the directory cannot be read', async () => {
const invalidDir = [join(tempDir, 'non-existent-dir'), '/invalid'];

let error;
try {
await buildPostList([invalidDir], tempDir, writeFilePath);
} catch (err) {
error = err;
}

expect(error).toBeDefined();
expect(error.message).toMatch(/Error while building post list/);
});
Comment on lines +149 to +161
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Duplicate test cases for invalid directory error handling

The test case at lines 175-187 duplicates the earlier test at lines 108-111, both checking the handling of invalid directories. Consider consolidating these tests to avoid redundancy.

Apply this diff to remove the duplicate test:

-      it('throws an error if the directory cannot be read', async () => {
-        const invalidDir = [join(tempDir, 'non-existent-dir'), '/invalid'];
-
-        let error;
-        try {
-          await buildPostList([invalidDir], tempDir, writeFilePath);
-        } catch (err) {
-          error = err;
-        }
-
-        expect(error).toBeDefined();
-        expect(error.message).toMatch(/Error while building post list/);
-      });
📝 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.

Suggested change
it('throws an error if the directory cannot be read', async () => {
const invalidDir = [join(tempDir, 'non-existent-dir'), '/invalid'];
let error;
try {
await buildPostList([invalidDir], tempDir, writeFilePath);
} catch (err) {
error = err;
}
expect(error).toBeDefined();
expect(error.message).toMatch(/Error while building post list/);
});



it('throws an error if the front matter cannot be parsed', async () => {
writeFileSync(join(tempDir, 'docs', 'invalid.mdx'), '---\ninvalid front matter\n---\nContent');

let error;
try {
await buildPostList(postDirectories, tempDir, writeFilePath);
} catch (err) {
error = err;
}

expect(error).toBeDefined();
expect(error.message).toMatch(/Error while building post list/);
});

it('throws an error if no post directories are provided', async () => {

let error;

try {
await buildPostList([], tempDir, writeFilePath);
} catch (err) {
error = err;
}

expect(error).toBeDefined();
expect(error.message).toMatch(/Error while building post list/);
});

});
2 changes: 1 addition & 1 deletion tests/index.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const rssFeed = require('../scripts/build-rss');
const buildPostList = require('../scripts/build-post-list');
const { buildPostList } = require('../scripts/build-post-list');
const buildCaseStudiesList = require('../scripts/casestudies');
const buildAdoptersList = require('../scripts/adopters');
const buildFinanceInfoList = require('../scripts/finance');
Expand Down
Loading