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 test for build docs script #3137

Merged
merged 29 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f851a03
build docs test
vishvamsinh28 Aug 9, 2024
2b5c902
build docs test completed
vishvamsinh28 Aug 9, 2024
2f5223f
test updated
vishvamsinh28 Aug 9, 2024
4d90c2e
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 11, 2024
e9a1066
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 16, 2024
4fb8dac
split tests into 3 different files
vishvamsinh28 Sep 16, 2024
c17425b
ewgewge
vishvamsinh28 Sep 16, 2024
e43adfe
fewg
vishvamsinh28 Sep 16, 2024
a78cef8
old error replaced by new one
vishvamsinh28 Sep 16, 2024
c2091cb
build tree
vishvamsinh28 Sep 16, 2024
4b474df
fewf
vishvamsinh28 Sep 16, 2024
b7ad0ae
buildnavtreee complete
vishvamsinh28 Sep 17, 2024
c0b3a42
refactored test
vishvamsinh28 Sep 17, 2024
0f47f0d
updated test for converDocPost
vishvamsinh28 Sep 17, 2024
e87699d
build-docs test updated
vishvamsinh28 Sep 17, 2024
097ce70
random push
vishvamsinh28 Sep 17, 2024
aab0a3a
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 19, 2024
c9a164a
fegeg
vishvamsinh28 Sep 19, 2024
5368a44
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 23, 2024
bb90a79
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 26, 2024
f22ffd7
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 28, 2024
dab658b
Merge branch 'master' into buildDocsTest
vishvamsinh28 Sep 30, 2024
c1cea61
Merge branch 'master' into buildDocsTest
anshgoyalevil Oct 3, 2024
855ee4a
added error variable
vishvamsinh28 Oct 3, 2024
7f484ea
wffqw
vishvamsinh28 Oct 3, 2024
af99af2
test updated
vishvamsinh28 Oct 3, 2024
0a94c17
fwf
vishvamsinh28 Oct 3, 2024
a562e78
test update complete
vishvamsinh28 Oct 4, 2024
5ce78ca
Merge branch 'master' into buildDocsTest
vishvamsinh28 Oct 7, 2024
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
208 changes: 113 additions & 95 deletions scripts/build-docs.js
Copy link
Member

Choose a reason for hiding this comment

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

Kindly write a similar doc/gist explaining the architecture of build-docs scripts, like said in build-tools PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anshgoyalevil Created a doc for it

Original file line number Diff line number Diff line change
@@ -1,164 +1,182 @@
const sortBy = require('lodash/sortBy')
function buildNavTree(navItems) {
try {
const tree = {
'welcome': {
item: { title: 'Welcome', weight: 0, isRootSection: true, isSection: true, rootSectionId: 'welcome', sectionWeight: 0, slug: '/docs' },
children: {}
}
}

//first we make sure that list of items lists main section items and then sub sections, documents last
const sortedItems = sortBy(navItems, ['isRootSection', 'weight', 'isSection']);
const sortedItems = sortBy(navItems, ['isRootSection', 'weight', 'isSection']);

sortedItems.forEach(item => {
//identify main sections
if (item.isRootSection) {
tree[item.rootSectionId] = { item, children: {} }
}

//identify subsections
if (item.parent) {
tree[item.parent].children[item.sectionId] = { item, children: [] }
if (!tree[item.parent]) {
throw new Error(`Parent section ${item.parent} not found for item ${item.title}`);
}
tree[item.parent].children[item.sectionId] = { item, children: [] };
}

if (!item.isSection) {
if (item.sectionId) {
let section = tree[item.rootSectionId].children[item.sectionId];
let section = tree[item.rootSectionId]?.children[item.sectionId];
if (!section) {
tree[item.rootSectionId].children[item.sectionId] = { item, children: [] }
tree[item.rootSectionId].children[item.sectionId] = { item, children: [] };
}
tree[item.rootSectionId].children[item.sectionId].children.push(item)
tree[item.rootSectionId].children[item.sectionId].children.push(item);
} else {
tree[item.rootSectionId].children[item.title] = { item };
}
}
})
});

for (const [rootKey, rootValue] of Object.entries(tree)) {
const allChildren = rootValue.children;
const allChildrenKeys = Object.keys(allChildren);

rootValue.children = allChildrenKeys
.sort((prev, next) => {
return allChildren[prev].item.weight - allChildren[next].item.weight;
}).reduce(
(obj, key) => {
obj[key] = allChildren[key];
return obj;
},
{}
);

})
.reduce((obj, key) => {
obj[key] = allChildren[key];
return obj;
}, {});

//handling subsections
if (allChildrenKeys.length > 1) {
for (const key of allChildrenKeys) {
allChildren[key].children?.sort((prev, next) => {
return prev.weight - next.weight;
});

if (allChildren[key].children) {
allChildren[key].children.sort((prev, next) => {
return prev.weight - next.weight;
});
}

// point in slug for specification subgroup to the latest specification version
if (rootKey === 'reference' && key === 'specification') {
allChildren[key].item.href = allChildren[key].children.find(c => c.isPrerelease === undefined).slug;
}
}
}
}

return tree;

} catch (err) {
throw new Error(`Failed to build navigation tree: ${err.message}`);
}
}

// A recursion function, works on the logic of Depth First Search to traverse all the root and child posts of the
// DocTree to get sequential order of the Doc Posts
// A recursion function, works on the logic of Depth First Search to traverse all the root and child posts of the
// DocTree to get sequential order of the Doc Posts
const convertDocPosts = (docObject) => {
try {
let docsArray = []
// certain entries in the DocPosts are either a parent to many posts or itself a post.
docsArray.push(docObject?.item || docObject)
if(docObject.children){
docsArray.push(docObject?.item || docObject)
if (docObject.children) {
let children = docObject.children
Object.keys(children).forEach((child) => {
let docChildArray = convertDocPosts(children[child])
docsArray = [...docsArray, ...docChildArray]
})
}
return docsArray
}
catch (err) {
throw new Error('Error in convertDocPosts:', err);
}
Comment on lines +95 to +96
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Include Original Error Message in 'convertDocPosts'

The error handling in the catch block does not correctly include the original error message. The Error constructor accepts a single message string, so the err parameter is ignored. To include the original error message, interpolate err.message into the new error message.

Apply this diff to fix the error handling:

   } catch (err) {
-    throw new Error('Error in convertDocPosts:', err);
+    throw new Error(`Error in convertDocPosts: ${err.message}`);
   }
📝 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
throw new Error('Error in convertDocPosts:', err);
}
throw new Error(`Error in convertDocPosts: ${err.message}`);
}

}


function addDocButtons(docPosts, treePosts){

function addDocButtons(docPosts, treePosts) {
let structuredPosts = [];
let rootSections = [];

// Traversing the whole DocTree and storing each post inside them in sequential order
Object.keys(treePosts).forEach((rootElement) => {
structuredPosts.push(treePosts[rootElement].item)
if(treePosts[rootElement].children){
let children = treePosts[rootElement].children
Object.keys(children).forEach((child) => {
let docChildArray = convertDocPosts(children[child])
structuredPosts = [...structuredPosts, ...docChildArray]
})
}
})
// Appending the content of welcome page pf Docs from the posts.json
structuredPosts[0] = docPosts.filter(p => p.slug === '/docs')[0]

// Traversing the strucutredPosts in order to add `nextPage` and `prevPage` details for each page
let countDocPages = structuredPosts.length
structuredPosts = structuredPosts.map((post, index) => {
// post item specifying the root Section or sub-section in the docs are excluded as
// they doesn't comprise any Doc Page or content to be shown in website.
if(post?.isRootSection || post?.isSection || index==0){
if(post?.isRootSection || index==0)
rootSections.push(post.title)
return post
}
try {
// Traversing the whole DocTree and storing each post inside them in sequential order
Object.keys(treePosts).forEach((rootElement) => {
structuredPosts.push(treePosts[rootElement].item);
if (treePosts[rootElement].children) {
let children = treePosts[rootElement].children;
Object.keys(children).forEach((child) => {
let docChildArray = convertDocPosts(children[child]);
structuredPosts = [...structuredPosts, ...docChildArray];
});
}
});

let nextPage = {}, prevPage = {}
let docPost = post;

// checks whether the next page for the current docPost item exists or not
if(index+1<countDocPages){
// checks whether the next item inside structuredPosts is a rootElement or a sectionElement
// if yes, it goes again to a next to next item in structuredPosts to link the nextPage
if(!structuredPosts[index+1].isRootElement && !structuredPosts[index+1].isSection){
nextPage = {
title: structuredPosts[index+1].title,
href: structuredPosts[index+1].slug
}
} else {
nextPage = {
title: `${structuredPosts[index+1].title} - ${structuredPosts[index+2].title}`,
href: structuredPosts[index+2].slug
}
// Appending the content of welcome page of Docs from the posts.json
structuredPosts[0] = docPosts.filter(p => p.slug === '/docs')[0];

// Traversing the structuredPosts in order to add `nextPage` and `prevPage` details for each page
let countDocPages = structuredPosts.length;
structuredPosts = structuredPosts.map((post, index) => {
// post item specifying the root Section or sub-section in the docs are excluded as
// they doesn't comprise any Doc Page or content to be shown in website.
if (post?.isRootSection || post?.isSection || index == 0) {
if (post?.isRootSection || index == 0)
rootSections.push(post.title)
return post
}
docPost = {...docPost, nextPage}
}

// checks whether the previous page for the current docPost item exists or not
if(index>0){
// checks whether the previous item inside structuredPosts is a rootElement or a sectionElement
// if yes, it goes again to a next previous item in structuredPosts to link the prevPage
if(!structuredPosts[index-1]?.isRootElement && !structuredPosts[index-1]?.isSection){
prevPage = {
title: structuredPosts[index-1].title,
href: structuredPosts[index-1].slug
let nextPage = {}, prevPage = {}
let docPost = post;

// checks whether the next page for the current docPost item exists or not
if (index + 1 < countDocPages) {
// checks whether the next item inside structuredPosts is a rootElement or a sectionElement
// if yes, it goes again to a next to next item in structuredPosts to link the nextPage
if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) {
nextPage = {
title: structuredPosts[index + 1].title,
href: structuredPosts[index + 1].slug
}
} else {
nextPage = {
title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`,
href: structuredPosts[index + 2].slug
}
Comment on lines +138 to +147
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent Potential Index Out-of-Bounds Errors in Next Page Logic

When accessing structuredPosts[index + 2], there's a risk of an index out-of-bounds error if index + 2 exceeds the array length. Ensure that index + 2 < countDocPages before accessing to prevent runtime errors.

Apply this diff to add the necessary boundary check:

       if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) {
         nextPage = {
           title: structuredPosts[index + 1].title,
           href: structuredPosts[index + 1].slug
         }
-      } else {
+      } else if (index + 2 < countDocPages) {
         nextPage = {
           title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`,
           href: structuredPosts[index + 2].slug
         }
+      } else {
+        // No next page available
+        nextPage = null;
       }
📝 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 (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) {
nextPage = {
title: structuredPosts[index + 1].title,
href: structuredPosts[index + 1].slug
}
} else {
nextPage = {
title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`,
href: structuredPosts[index + 2].slug
}
if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) {
nextPage = {
title: structuredPosts[index + 1].title,
href: structuredPosts[index + 1].slug
}
} else if (index + 2 < countDocPages) {
nextPage = {
title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`,
href: structuredPosts[index + 2].slug
}
} else {
// No next page available
nextPage = null;
}

}
docPost = {...docPost, prevPage}
}else{
// additonal check for the first page of Docs so that it doesn't give any Segementation fault
if(index-2>=0){
docPost = { ...docPost, nextPage }
}

// checks whether the previous page for the current docPost item exists or not
if (index > 0) {
// checks whether the previous item inside structuredPosts is a rootElement or a sectionElement
// if yes, it goes again to a next previous item in structuredPosts to link the prevPage
if (!structuredPosts[index - 1]?.isRootElement && !structuredPosts[index - 1]?.isSection) {
prevPage = {
title: `${structuredPosts[index-1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index-2].title}`,
href: structuredPosts[index-2].slug
title: structuredPosts[index - 1].title,
href: structuredPosts[index - 1].slug
}
docPost = { ...docPost, prevPage }
} else {
// additonal check for the first page of Docs so that it doesn't give any Segementation fault
if (index - 2 >= 0) {
prevPage = {
title: `${structuredPosts[index - 1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index - 2].title}`,
href: structuredPosts[index - 2].slug
};
docPost = { ...docPost, prevPage };
Comment on lines +164 to +169
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent Potential Index Out-of-Bounds Errors in Previous Page Logic

Similarly, when accessing structuredPosts[index - 2], ensure that index - 2 >= 0 to avoid index out-of-bounds errors.

Apply this diff to adjust the condition:

       if (index - 2 >= 0) {
         prevPage = {
           title: `${structuredPosts[index - 1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index - 2].title}`,
           href: structuredPosts[index - 2].slug
         };
         docPost = { ...docPost, prevPage };
+      } else {
+        // No previous page available
+        prevPage = null;
       }
📝 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 (index - 2 >= 0) {
prevPage = {
title: `${structuredPosts[index - 1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index - 2].title}`,
href: structuredPosts[index - 2].slug
};
docPost = { ...docPost, prevPage };
if (index - 2 >= 0) {
prevPage = {
title: `${structuredPosts[index - 1]?.isRootSection ? rootSections[rootSections.length - 2] : rootSections[rootSections.length - 1]} - ${structuredPosts[index - 2].title}`,
href: structuredPosts[index - 2].slug
};
docPost = { ...docPost, prevPage };
} else {
// No previous page available
prevPage = null;
}

}
docPost = {...docPost, prevPage}
}
}
}
return docPost
})
return structuredPosts
}
return docPost;
});

} catch (err) {
throw new Error("An error occurred while adding doc buttons:", err);
}
Comment on lines +177 to +178
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Include Original Error Message in 'addDocButtons'

Similarly, the error handling in the addDocButtons function does not include the original error message. The additional err parameter is ignored by the Error constructor.

Apply this diff to correct the error handling:

 } catch (err) {
-    throw new Error("An error occurred while adding doc buttons:", err);
+    throw new Error(`An error occurred while adding doc buttons: ${err.message}`);
 }
📝 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
throw new Error("An error occurred while adding doc buttons:", err);
}
throw new Error(`An error occurred while adding doc buttons: ${err.message}`);
}

return structuredPosts;
}

module.exports = {buildNavTree, addDocButtons, convertDocPosts}
module.exports = { buildNavTree, addDocButtons, convertDocPosts }
79 changes: 79 additions & 0 deletions tests/build-docs/addDocButtons.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
const { addDocButtons } = require("../../scripts/build-docs");
const { docPosts, treePosts, mockDocPosts, mockTreePosts, invalidTreePosts } = require("../fixtures/addDocButtonsData");

describe('addDocButtons', () => {
it('should add next and previous page information', () => {
const expectedFirstItem = {
title: 'Welcome',
slug: '/docs',
content: 'Welcome content'
};

const expectedSecondItem = {
isRootSection: true,
title: 'Section 1'
};

const expectedThirdItem = {
title: 'Page 1',
slug: '/docs/section1/page1',
nextPage: {
title: 'Page 2',
href: '/docs/section1/page2'
},
prevPage: {
title: 'Section 1',
href: undefined
}
};

const expectedFourthItem = {
title: 'Page 2',
slug: '/docs/section1/page2',
prevPage: {
title: 'Page 1',
href: '/docs/section1/page1'
}
};

const result = addDocButtons(docPosts, treePosts);

expect(result).toHaveLength(4);
expect(result[0]).toEqual(expectedFirstItem);
expect(result[1]).toEqual(expectedSecondItem);
expect(result[2]).toEqual(expectedThirdItem);
expect(result[3]).toEqual(expectedFourthItem);
});

it('should set nextPage correctly when next item is a root element', () => {
const result = addDocButtons(mockDocPosts, mockTreePosts);

expect(result[1].nextPage).toBeDefined();
expect(result[1].nextPage.title).toBe('Root 2 - Child 2');
expect(result[1].nextPage.href).toBe('/docs/root2/child2');
});

it('should throw an error if treePosts is missing', () => {
try {
addDocButtons(docPosts, undefined);
} catch (err) {
expect(err.message).toMatch(/An error occurred while adding doc buttons:/);
}
});

it('should throw an error if docPosts is missing', () => {
try {
addDocButtons(undefined, treePosts);
} catch (err) {
expect(err.message).toMatch(/An error occurred while adding doc buttons:/);
}
});

it('should handle invalid data structure in treePosts', () => {
try {
addDocButtons(docPosts, invalidTreePosts);
} catch (err) {
expect(err.message).toMatch(/An error occurred while adding doc buttons:/);
Copy link
Member

Choose a reason for hiding this comment

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

This expect statement is not being executed here. Try modiying the line /An error occurred while adding doc buttons:/ and you would observe it is always passing the test.

}
});
});
Loading
Loading