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 combine tools script #3136

Merged
merged 82 commits into from
Dec 9, 2024

Conversation

vishvamsinh28
Copy link
Contributor

@vishvamsinh28 vishvamsinh28 commented Aug 9, 2024

This PR adds test for combine tools script.

Summary by CodeRabbit

  • New Features

    • Enhanced error handling in the tool combining process, providing descriptive error messages for exceptions.
    • Introduced new tools and categories in JSON files, including "Tool A" and "Tool B" with specific metadata.
  • Bug Fixes

    • Improved validation and logging for tools with missing or invalid data.
  • Tests

    • Added a comprehensive suite of unit tests for the tool combining functionality, covering various scenarios and ensuring robustness.

Copy link

netlify bot commented Aug 9, 2024

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 98f3238
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/67569b9e07c1db0008919172
😎 Deploy Preview https://deploy-preview-3136--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Aug 9, 2024

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 47
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-3136--asyncapi-website.netlify.app/

@@ -106,7 +105,7 @@ const getFinalTool = async (toolObject) => {

// Combine the automated tools and manual tools list into single JSON object file, and
// lists down all the language and technology tags in one JSON file.
const combineTools = async (automatedTools, manualTools) => {
const combineTools = async (automatedTools, manualTools, toolsPath, tagsPath) => {
Copy link
Member

Choose a reason for hiding this comment

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

what does tagspath signify??

JSON.stringify({ languages: languageList, technologies: technologyList }),
)
fs.writeFileSync(toolsPath,JSON.stringify(finalTools));
fs.writeFileSync(tagsPath,JSON.stringify({ languages: languageList, technologies: technologyList }),)
}

module.exports = { combineTools }
Copy link
Member

Choose a reason for hiding this comment

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

fix this linter error

const tagsData = readJSON(tagsPath);
expect(tagsData.languages).toContainEqual(expect.objectContaining({ name: 'NewLanguage' }));
});
});
Copy link
Member

Choose a reason for hiding this comment

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

fix this one also

@anshgoyalevil anshgoyalevil added the gsoc This label should be used for issues or discussions related to ideas for Google Summer of Code label Aug 18, 2024
@@ -14,7 +14,7 @@ const buildTools = async () => {
resolve(__dirname, '../config', 'tools-automated.json'),
JSON.stringify(automatedTools, null, ' ')
);
await combineTools(automatedTools, manualTools);
await combineTools(automatedTools, manualTools, resolve(__dirname, '../../config', 'tools.json'), resolve(__dirname, '../../config', 'all-tags.json'));
Copy link
Member

Choose a reason for hiding this comment

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

Can we do it in a more better way, instead of passing filePath here? Try to refactor tools scripts instead of manually passing filePaths for tags file.

tests/tools/combine-tools.test.js Show resolved Hide resolved
});


it('should log validation errors to console.error', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Errors shouldn't be validated via console messages. You should formally return a error using Promise.reject.

Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 Have you worked on this?

@@ -6,15 +6,19 @@ const manualTools = require('../config/tools-manual.json')
const fs = require('fs');
const { resolve } = require('path');

let toolsPath = resolve(__dirname, '../../config', 'tools.json')
Copy link
Member

Choose a reason for hiding this comment

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

is using let a good option here?

@vishvamsinh28 vishvamsinh28 marked this pull request as draft September 21, 2024 09:47
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (11)
tests/fixtures/combineToolsData.js (6)

28-34: Consider adding more error test cases.

The current error test cases cover missing data, invalid URLs, and invalid categories. Consider adding these additional scenarios:

  • Empty strings for required fields
  • Malformed filter objects
  • Special characters in URLs
  • Cross-site scripting (XSS) attempts in titles

Example additions:

const additionalErrorCases = {
    emptyFields: {
        title: '',
        filters: { language: '' },
        links: { repoUrl: '' }
    },
    malformedFilters: {
        title: 'Malformed Filters',
        filters: null,
        links: { repoUrl: 'https://github.com/example/tool' }
    },
    xssAttempt: {
        title: '<script>alert("xss")</script>',
        filters: { language: 'JavaScript' },
        links: { repoUrl: 'https://github.com/example/tool' }
    }
};

Also applies to: 138-177

🧰 Tools
🪛 eslint

[error] 29-29: Delete ··

(prettier/prettier)


[error] 30-30: Replace ········ with ····

(prettier/prettier)


[error] 31-31: Delete ····

(prettier/prettier)


[error] 32-32: Replace ········ with ····

(prettier/prettier)


[error] 33-33: Delete ··

(prettier/prettier)


36-52: Enhance sorting test cases.

The current sorting test case is basic. Consider adding these scenarios:

  • Tools with same titles but different languages
  • Special characters in titles
  • Case sensitivity handling
  • Multiple categories with varying tool counts

Example enhancement:

const manualToolsToSort = {
    category1: {
        description: 'Sample Category',
        toolsList: [
            {
                title: 'Tool A',
                filters: { language: 'JavaScript' },
                links: { repoUrl: 'https://github.com/asyncapi/tool-a-js' }
            },
            {
                title: 'Tool A',  // Same title, different language
                filters: { language: 'Python' },
                links: { repoUrl: 'https://github.com/asyncapi/tool-a-py' }
            },
            {
                title: '@Special Tool',  // Special character
                filters: { language: 'JavaScript' },
                links: { repoUrl: 'https://github.com/asyncapi/special-tool' }
            }
        ]
    },
    category2: {  // Additional category
        description: 'Another Category',
        toolsList: [/* ... */]
    }
};
🧰 Tools
🪛 eslint

[error] 37-37: Delete ··

(prettier/prettier)


[error] 38-38: Replace ········ with ····

(prettier/prettier)


[error] 39-39: Delete ····

(prettier/prettier)


[error] 40-40: Replace ············ with ······

(prettier/prettier)


[error] 41-41: Replace ················ with ········

(prettier/prettier)


[error] 42-42: Delete ········

(prettier/prettier)


[error] 43-43: Replace ················ with ········

(prettier/prettier)


[error] 44-44: Delete ······

(prettier/prettier)


[error] 45-45: Replace ············ with ······

(prettier/prettier)


[error] 46-46: Replace ················ with ········

(prettier/prettier)


[error] 47-47: Delete ········

(prettier/prettier)


[error] 48-48: Replace ················ with ········

(prettier/prettier)


[error] 49-49: Delete ······

(prettier/prettier)


[error] 50-50: Delete ····

(prettier/prettier)


[error] 51-51: Delete ··

(prettier/prettier)


54-114: Add edge cases for language/technology combinations.

Consider adding these scenarios:

  • Empty arrays for languages/technologies
  • Duplicate entries in arrays
  • Case sensitivity in language/technology names
  • Very long arrays of languages/technologies

Example additions:

const edgeCaseTools = {
    emptyArrays: {
        title: 'Empty Arrays Tool',
        filters: {
            language: [],
            technology: []
        },
        links: { repoUrl: 'https://github.com/example/empty-arrays-tool' }
    },
    duplicateEntries: {
        title: 'Duplicate Entries Tool',
        filters: {
            language: ['JavaScript', 'javascript', 'JAVASCRIPT'],
            technology: ['Node.js', 'node.js']
        },
        links: { repoUrl: 'https://github.com/example/duplicate-entries-tool' }
    }
};
🧰 Tools
🪛 eslint

[error] 55-55: Delete ··

(prettier/prettier)


[error] 56-56: Delete ··

(prettier/prettier)


[error] 57-57: Replace ········ with ····

(prettier/prettier)


[error] 58-58: Delete ····

(prettier/prettier)


[error] 59-59: Delete ··

(prettier/prettier)


[error] 60-60: Delete ··

(prettier/prettier)


[error] 64-64: Replace ··'category1' with category1

(prettier/prettier)


[error] 65-65: Replace ········ with ····

(prettier/prettier)


[error] 66-66: Replace ········ with ····

(prettier/prettier)


[error] 67-67: Delete ··

(prettier/prettier)


[error] 73-73: Replace ··'category1' with category1

(prettier/prettier)


[error] 74-74: Delete ····

(prettier/prettier)


[error] 75-75: Delete ····

(prettier/prettier)


[error] 76-76: Delete ··

(prettier/prettier)


[error] 79-79: Replace ··'category1' with category1

(prettier/prettier)


[error] 80-80: Replace ········ with ····

(prettier/prettier)


[error] 81-81: Delete ··

(prettier/prettier)


[error] 85-85: Replace ···· with ··

(prettier/prettier)


[error] 86-86: Delete ··

(prettier/prettier)


[error] 87-87: Replace ········ with ····

(prettier/prettier)


[error] 88-88: Replace ········ with ····

(prettier/prettier)


[error] 89-89: Delete ··

(prettier/prettier)


[error] 90-90: Delete ··

(prettier/prettier)


[error] 94-94: Replace ····'category1' with ··category1

(prettier/prettier)


[error] 95-95: Delete ····

(prettier/prettier)


[error] 96-96: Replace ········ with ····

(prettier/prettier)


[error] 97-97: Delete ··

(prettier/prettier)


[error] 101-101: Delete ··

(prettier/prettier)


[error] 102-102: Delete ··

(prettier/prettier)


[error] 103-103: Delete ····

(prettier/prettier)


[error] 104-104: Delete ····

(prettier/prettier)


[error] 105-105: Delete ··

(prettier/prettier)


[error] 106-106: Delete ··

(prettier/prettier)


[error] 110-110: Replace ··'category1' with category1

(prettier/prettier)


[error] 111-111: Delete ····

(prettier/prettier)


[error] 112-112: Delete ····

(prettier/prettier)


[error] 113-113: Delete ··

(prettier/prettier)


116-136: Expand valid tool test cases.

The current valid tool test cases are basic. Consider adding these scenarios:

  • Tools with all optional fields populated
  • Tools with minimum required fields
  • Tools with various repository hosting services (not just GitHub)

Example additions:

const validToolsExtended = {
    fullTool: {
        title: 'Full Tool',
        description: 'Detailed description',
        filters: {
            language: 'JavaScript',
            technology: ['Node.js'],
            categories: ['API', 'Testing']
        },
        links: {
            repoUrl: 'https://github.com/example/full-tool',
            docsUrl: 'https://docs.example.com',
            websiteUrl: 'https://example.com'
        }
    },
    minimalTool: {
        title: 'Minimal Tool',
        filters: {
            language: 'JavaScript'
        },
        links: { repoUrl: 'https://gitlab.com/example/minimal-tool' }
    }
};
🧰 Tools
🪛 eslint

[error] 117-117: Delete ··

(prettier/prettier)


[error] 118-118: Replace ···· with ··

(prettier/prettier)


[error] 119-119: Replace ········ with ····

(prettier/prettier)


[error] 120-120: Replace ········ with ····

(prettier/prettier)


[error] 121-121: Delete ··

(prettier/prettier)


[error] 122-122: Replace ···· with ··

(prettier/prettier)


[error] 126-126: Delete ··

(prettier/prettier)


[error] 127-127: Delete ····

(prettier/prettier)


[error] 128-128: Replace ········ with ····

(prettier/prettier)


[error] 129-129: Replace ···· with ··

(prettier/prettier)


[error] 133-133: Delete ··

(prettier/prettier)


[error] 134-134: Replace ········ with ····

(prettier/prettier)


[error] 135-135: Replace ···· with ··

(prettier/prettier)


179-193: Enhance circular reference test cases.

Consider adding more complex circular reference scenarios:

  • Nested circular references
  • Multiple tools with circular dependencies
  • Self-referential tools

Example enhancement:

const circularTools = {
    toolA: {
        title: 'Tool A',
        filters: { language: 'JavaScript' },
        links: { repoUrl: 'https://github.com/example/tool-a' },
        dependencies: ['toolB']
    },
    toolB: {
        title: 'Tool B',
        filters: { language: 'JavaScript' },
        links: { repoUrl: 'https://github.com/example/tool-b' },
        dependencies: ['toolC']
    },
    toolC: {
        title: 'Tool C',
        filters: { language: 'JavaScript' },
        links: { repoUrl: 'https://github.com/example/tool-c' },
        dependencies: ['toolA']  // Creates a circular dependency
    }
};
🧰 Tools
🪛 eslint

[error] 180-180: Replace ···· with ··

(prettier/prettier)


[error] 181-181: Delete ··

(prettier/prettier)


[error] 182-182: Replace ········ with ····

(prettier/prettier)


[error] 183-183: Replace ········ with ····

(prettier/prettier)


[error] 184-184: Replace ···· with ··

(prettier/prettier)


[error] 185-185: Delete ··

(prettier/prettier)


[error] 189-189: Delete ··

(prettier/prettier)


[error] 190-190: Replace ········ with ····

(prettier/prettier)


[error] 191-191: Replace ········ with ····

(prettier/prettier)


[error] 192-192: Replace ···· with ··

(prettier/prettier)


195-212: Consider organizing exports by test scenario.

Group related exports together using object literals for better organization and maintainability.

Example reorganization:

module.exports = {
    // Basic data
    expectedDataT1,
    
    // Error cases
    errorCases: {
        missingData: manualToolsWithMissingData,
        invalidUrl: manualToolsWithInvalidURLT11,
        invalidCategory: invalidAutomatedToolsT10
    },
    
    // Sorting cases
    sortingCases: {
        manualToolsToSort
    },
    
    // Language/Technology cases
    languageCases: {
        multipleLanguages: toolWithMultipleLanguages,
        newLanguage: toolWithNewLanguageT7,
        newTags: toolWithNewTagsT6
    },
    
    // Valid tool cases
    validCases: {
        automated: automatedToolsT8,
        manual: manualToolsT8
    },
    
    // Circular reference cases
    circularCases: {
        tool: circularTool,
        automated: automatedToolsT12
    }
};
🧰 Tools
🪛 eslint

[error] 196-196: Replace ···· with ··

(prettier/prettier)


[error] 197-197: Delete ··

(prettier/prettier)


[error] 198-198: Replace ···· with ··

(prettier/prettier)


[error] 199-199: Replace ···· with ··

(prettier/prettier)


[error] 200-200: Replace ···· with ··

(prettier/prettier)


[error] 201-201: Delete ··

(prettier/prettier)


[error] 202-202: Replace ···· with ··

(prettier/prettier)


[error] 203-203: Replace ···· with ··

(prettier/prettier)


[error] 204-204: Replace ···· with ··

(prettier/prettier)


[error] 205-205: Delete ··

(prettier/prettier)


[error] 206-206: Replace ···· with ··

(prettier/prettier)


[error] 207-207: Delete ··

(prettier/prettier)


[error] 208-208: Replace ···· with ··

(prettier/prettier)


[error] 209-209: Delete ··

(prettier/prettier)


[error] 210-210: Replace ···· with ··

(prettier/prettier)


[error] 211-211: Delete ··

(prettier/prettier)


[error] 212-212: Insert ;

(prettier/prettier)

scripts/tools/combine-tools.js (3)

109-142: Consider enhancing error handling with operation context

The current error handling could be more informative by including the specific operation that failed.

 try {
   // ... existing code ...
 } catch (err) {
-  throw new Error(`Error combining tools: ${err}`);
+  const operation = err.message.includes('validation') ? 'tool validation' :
+    err.message.includes('URL') ? 'URL parsing' :
+    err.message.includes('ENOENT') ? 'file operation' : 'tools combination';
+  throw new Error(`Failed during ${operation}: ${err.message}`);
 }
🧰 Tools
🪛 eslint

[error] 109-109: Delete ··

(prettier/prettier)


[error] 110-110: Delete ····

(prettier/prettier)


[error] 110-138: for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.

(no-restricted-syntax)


[error] 110-138: The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype.

(guard-for-in)


[error] 111-111: Replace ············ with ······

(prettier/prettier)


[error] 111-111: 'finalToolsList' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 112-112: Delete ······

(prettier/prettier)


[error] 113-113: Replace ················ with ········

(prettier/prettier)


[error] 113-115: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 114-114: Replace ··········finalToolsList.push(await·getFinalTool(tool)) with finalToolsList.push(await·getFinalTool(tool));

(prettier/prettier)


[error] 114-114: Unexpected await inside a loop.

(no-await-in-loop)


[error] 115-115: Replace ················ with ········

(prettier/prettier)


[error] 116-116: Replace ············ with ······

(prettier/prettier)


[error] 117-117: Delete ······

(prettier/prettier)


[error] 118-118: Delete ········

(prettier/prettier)


[error] 118-134: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 119-119: Delete ··········

(prettier/prettier)


[error] 120-120: Replace ····················const·isValid·=·await·validate(tool) with ··········const·isValid·=·await·validate(tool);

(prettier/prettier)


[error] 120-120: Unexpected await inside a loop.

(no-await-in-loop)


[error] 121-121: Replace ···················· with ··········

(prettier/prettier)


[error] 122-122: Replace ························ with ············

(prettier/prettier)


[error] 123-123: Replace ····························const·url·=·new·URL(tool.links.repoUrl) with ··············const·url·=·new·URL(tool.links.repoUrl);

(prettier/prettier)


[error] 124-124: Replace ····························isAsyncAPIrepo·=·url.href.startsWith("https://github.com/asyncapi/") with ··············isAsyncAPIrepo·=·url.href.startsWith('https://github.com/asyncapi/');

(prettier/prettier)


[error] 125-125: Replace ························}·else·isAsyncAPIrepo·=·false with ············}·else·isAsyncAPIrepo·=·false;

(prettier/prettier)


[error] 126-126: Replace ························let·toolObject·=·await·createToolObject(tool,·"",·"",·isAsyncAPIrepo) with ············let·toolObject·=·await·createToolObject(tool,·'',·'',·isAsyncAPIrepo);

(prettier/prettier)


[error] 126-126: 'toolObject' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 126-126: Unexpected await inside a loop.

(no-await-in-loop)


[error] 127-127: Replace ························finalToolsList.push(await·getFinalTool(toolObject)) with ············finalToolsList.push(await·getFinalTool(toolObject));

(prettier/prettier)


[error] 127-127: Unexpected await inside a loop.

(no-await-in-loop)


[error] 128-128: Delete ··········

(prettier/prettier)


[error] 129-129: Replace ························ with ············

(prettier/prettier)


[error] 130-130: Delete ············

(prettier/prettier)


[error] 131-131: Replace ························ with ············

(prettier/prettier)


[error] 132-132: Delete ············

(prettier/prettier)


[error] 133-133: Replace ···················· with ··········

(prettier/prettier)


[error] 134-134: Replace ················ with ········

(prettier/prettier)


[error] 135-135: Replace ············ with ······

(prettier/prettier)


[error] 136-136: Delete ······

(prettier/prettier)


[error] 137-137: Replace ············finalTools[key].toolsList·=·finalToolsList with ······finalTools[key].toolsList·=·finalToolsList;

(prettier/prettier)


[error] 138-138: Delete ····

(prettier/prettier)


[error] 139-139: Delete ····

(prettier/prettier)


[error] 140-140: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)


[error] 141-141: Delete ··

(prettier/prettier)


[error] 142-142: Replace ········ with ····

(prettier/prettier)


129-133: Improve validation error logging structure

The current error logging could be more structured and consistent.

-console.error('Script is not failing, it is just dropping errors for further investigation');
-console.error(`Invalid ${tool.title} .asyncapi-tool file.`);
-console.error(`Located in manual-tools.json file`);
-console.error('Validation errors:', JSON.stringify(validate.errors, null, 2));
+console.error({
+  message: 'Tool validation failed',
+  tool: tool.title,
+  source: 'manual-tools.json',
+  errors: validate.errors,
+  note: 'Script continues execution, error logged for investigation'
+});
🧰 Tools
🪛 eslint

[error] 129-129: Replace ························ with ············

(prettier/prettier)


[error] 130-130: Delete ············

(prettier/prettier)


[error] 131-131: Replace ························ with ············

(prettier/prettier)


[error] 132-132: Delete ············

(prettier/prettier)


[error] 133-133: Replace ···················· with ··········

(prettier/prettier)


111-111: Use const for variables that aren't reassigned

The variables finalToolsList and toolObject are never reassigned.

-let finalToolsList = [];
+const finalToolsList = [];

-let toolObject = await createToolObject(tool, "", "", isAsyncAPIrepo)
+const toolObject = await createToolObject(tool, '', '', isAsyncAPIrepo);

Also applies to: 126-126

🧰 Tools
🪛 eslint

[error] 111-111: Replace ············ with ······

(prettier/prettier)


[error] 111-111: 'finalToolsList' is never reassigned. Use 'const' instead.

(prefer-const)

tests/tools/combine-tools.test.js (2)

23-27: Enhance AJV mock implementation for better test coverage.

The current AJV mock is overly simplified, only validating the title field. Consider implementing a more comprehensive mock that validates all required fields according to your schema.

 jest.mock('ajv', () => {
   return jest.fn().mockImplementation(() => ({
-    compile: jest.fn().mockImplementation(() => (data) => data.title !== 'Invalid Tool'),
+    compile: jest.fn().mockImplementation(() => (data) => {
+      const errors = [];
+      if (!data.title) errors.push({ message: 'title is required' });
+      if (!data.description) errors.push({ message: 'description is required' });
+      if (!data.filters) errors.push({ message: 'filters is required' });
+      return errors.length === 0;
+    }),
   }));
 });
🧰 Tools
🪛 eslint

[error] 23-23: 'jest' is not defined.

(no-undef)


[error] 24-24: 'jest' is not defined.

(no-undef)


[error] 25-25: 'jest' is not defined.

(no-undef)


[error] 25-25: Delete ,

(prettier/prettier)


84-94: Improve test descriptions for better clarity.

The test descriptions could be more specific about what they're testing. Consider updating them to clearly state the expected behavior and conditions being tested.

-  it('should combine tools and create correct JSON files', async () => {
+  it('should combine automated and manual tools into JSON files with correct structure', async () => {

-  it('should handle tools with missing language or technology', async () => {
+  it('should successfully process tools when language or technology fields are missing', async () => {

-  it('should sort tools alphabetically by title', async () => {
+  it('should sort tools within each category alphabetically by title', async () => {

-  it('should log validation errors to console.error', async () => {
+  it('should log detailed validation errors to console.error without failing the script', async () => {

Also applies to: 96-102, 104-111, 113-126

🧰 Tools
🪛 eslint

[error] 84-84: 'it' is not defined.

(no-undef)


[error] 93-93: Insert ;

(prettier/prettier)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 496de09 and ca72a7f.

📒 Files selected for processing (3)
  • scripts/tools/combine-tools.js (1 hunks)
  • tests/fixtures/combineToolsData.js (1 hunks)
  • tests/tools/combine-tools.test.js (1 hunks)
🧰 Additional context used
📓 Learnings (1)
scripts/tools/combine-tools.js (1)
Learnt from: akshatnema
PR: asyncapi/website#3136
File: scripts/tools/combine-tools.js:122-125
Timestamp: 2024-11-01T12:49:32.806Z
Learning: In `scripts/tools/combine-tools.js`, the existing URL parsing logic for `repoUrl` without additional error handling is acceptable and does not require changes.
🪛 eslint
scripts/tools/combine-tools.js

[error] 109-109: Delete ··

(prettier/prettier)


[error] 110-110: Delete ····

(prettier/prettier)


[error] 110-138: for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.

(no-restricted-syntax)


[error] 110-138: The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype.

(guard-for-in)


[error] 111-111: Replace ············ with ······

(prettier/prettier)


[error] 111-111: 'finalToolsList' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 112-112: Delete ······

(prettier/prettier)


[error] 113-113: Replace ················ with ········

(prettier/prettier)


[error] 113-115: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 114-114: Replace ··········finalToolsList.push(await·getFinalTool(tool)) with finalToolsList.push(await·getFinalTool(tool));

(prettier/prettier)


[error] 114-114: Unexpected await inside a loop.

(no-await-in-loop)


[error] 115-115: Replace ················ with ········

(prettier/prettier)


[error] 116-116: Replace ············ with ······

(prettier/prettier)


[error] 117-117: Delete ······

(prettier/prettier)


[error] 118-118: Delete ········

(prettier/prettier)


[error] 118-134: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 119-119: Delete ··········

(prettier/prettier)


[error] 120-120: Replace ····················const·isValid·=·await·validate(tool) with ··········const·isValid·=·await·validate(tool);

(prettier/prettier)


[error] 120-120: Unexpected await inside a loop.

(no-await-in-loop)


[error] 121-121: Replace ···················· with ··········

(prettier/prettier)


[error] 122-122: Replace ························ with ············

(prettier/prettier)


[error] 123-123: Replace ····························const·url·=·new·URL(tool.links.repoUrl) with ··············const·url·=·new·URL(tool.links.repoUrl);

(prettier/prettier)


[error] 124-124: Replace ····························isAsyncAPIrepo·=·url.href.startsWith("https://github.com/asyncapi/") with ··············isAsyncAPIrepo·=·url.href.startsWith('https://github.com/asyncapi/');

(prettier/prettier)


[error] 125-125: Replace ························}·else·isAsyncAPIrepo·=·false with ············}·else·isAsyncAPIrepo·=·false;

(prettier/prettier)


[error] 126-126: Replace ························let·toolObject·=·await·createToolObject(tool,·"",·"",·isAsyncAPIrepo) with ············let·toolObject·=·await·createToolObject(tool,·'',·'',·isAsyncAPIrepo);

(prettier/prettier)


[error] 126-126: 'toolObject' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 126-126: Unexpected await inside a loop.

(no-await-in-loop)


[error] 127-127: Replace ························finalToolsList.push(await·getFinalTool(toolObject)) with ············finalToolsList.push(await·getFinalTool(toolObject));

(prettier/prettier)


[error] 127-127: Unexpected await inside a loop.

(no-await-in-loop)


[error] 128-128: Delete ··········

(prettier/prettier)


[error] 129-129: Replace ························ with ············

(prettier/prettier)


[error] 130-130: Delete ············

(prettier/prettier)


[error] 131-131: Replace ························ with ············

(prettier/prettier)


[error] 132-132: Delete ············

(prettier/prettier)


[error] 133-133: Replace ···················· with ··········

(prettier/prettier)


[error] 134-134: Replace ················ with ········

(prettier/prettier)


[error] 135-135: Replace ············ with ······

(prettier/prettier)


[error] 136-136: Delete ······

(prettier/prettier)


[error] 137-137: Replace ············finalTools[key].toolsList·=·finalToolsList with ······finalTools[key].toolsList·=·finalToolsList;

(prettier/prettier)


[error] 138-138: Delete ····

(prettier/prettier)


[error] 139-139: Delete ····

(prettier/prettier)


[error] 140-140: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)


[error] 141-141: Delete ··

(prettier/prettier)


[error] 142-142: Replace ········ with ····

(prettier/prettier)


[error] 143-143: Delete ··

(prettier/prettier)


[error] 144-144: Insert ;

(prettier/prettier)


[error] 146-146: Insert ;

(prettier/prettier)

tests/fixtures/combineToolsData.js

[error] 2-2: Delete ··

(prettier/prettier)


[error] 3-3: Delete ····

(prettier/prettier)


[error] 4-4: Replace ············ with ······

(prettier/prettier)


[error] 5-5: Replace ············ with ······

(prettier/prettier)


[error] 6-6: Delete ······

(prettier/prettier)


[error] 7-7: Replace ········ with ····

(prettier/prettier)


[error] 8-8: Replace ········ with ····

(prettier/prettier)


[error] 9-9: Replace ············ with ······

(prettier/prettier)


[error] 10-10: Replace ············ with ······

(prettier/prettier)


[error] 11-11: Delete ······

(prettier/prettier)


[error] 12-12: Delete ····

(prettier/prettier)


[error] 13-13: Delete ··

(prettier/prettier)


[error] 14-14: Delete ··

(prettier/prettier)


[error] 15-15: Delete ····

(prettier/prettier)


[error] 16-16: Replace ············ with ······

(prettier/prettier)


[error] 17-17: Delete ······

(prettier/prettier)


[error] 18-18: Replace ············ with ······

(prettier/prettier)


[error] 19-19: Replace ········ with ····

(prettier/prettier)


[error] 20-20: Replace ········ with ····

(prettier/prettier)


[error] 21-21: Replace ············ with ······

(prettier/prettier)


[error] 22-22: Replace ············ with ······

(prettier/prettier)


[error] 23-23: Delete ······

(prettier/prettier)


[error] 24-24: Replace ········ with ····

(prettier/prettier)


[error] 25-25: Delete ··

(prettier/prettier)


[error] 29-29: Delete ··

(prettier/prettier)


[error] 30-30: Replace ········ with ····

(prettier/prettier)


[error] 31-31: Delete ····

(prettier/prettier)


[error] 32-32: Replace ········ with ····

(prettier/prettier)


[error] 33-33: Delete ··

(prettier/prettier)


[error] 37-37: Delete ··

(prettier/prettier)


[error] 38-38: Replace ········ with ····

(prettier/prettier)


[error] 39-39: Delete ····

(prettier/prettier)


[error] 40-40: Replace ············ with ······

(prettier/prettier)


[error] 41-41: Replace ················ with ········

(prettier/prettier)


[error] 42-42: Delete ········

(prettier/prettier)


[error] 43-43: Replace ················ with ········

(prettier/prettier)


[error] 44-44: Delete ······

(prettier/prettier)


[error] 45-45: Replace ············ with ······

(prettier/prettier)


[error] 46-46: Replace ················ with ········

(prettier/prettier)


[error] 47-47: Delete ········

(prettier/prettier)


[error] 48-48: Replace ················ with ········

(prettier/prettier)


[error] 49-49: Delete ······

(prettier/prettier)


[error] 50-50: Delete ····

(prettier/prettier)


[error] 51-51: Delete ··

(prettier/prettier)


[error] 55-55: Delete ··

(prettier/prettier)


[error] 56-56: Delete ··

(prettier/prettier)


[error] 57-57: Replace ········ with ····

(prettier/prettier)


[error] 58-58: Delete ····

(prettier/prettier)


[error] 59-59: Delete ··

(prettier/prettier)


[error] 60-60: Delete ··

(prettier/prettier)


[error] 64-64: Replace ··'category1' with category1

(prettier/prettier)


[error] 65-65: Replace ········ with ····

(prettier/prettier)


[error] 66-66: Replace ········ with ····

(prettier/prettier)


[error] 67-67: Delete ··

(prettier/prettier)


[error] 73-73: Replace ··'category1' with category1

(prettier/prettier)


[error] 74-74: Delete ····

(prettier/prettier)


[error] 75-75: Delete ····

(prettier/prettier)


[error] 76-76: Delete ··

(prettier/prettier)


[error] 79-79: Replace ··'category1' with category1

(prettier/prettier)


[error] 80-80: Replace ········ with ····

(prettier/prettier)


[error] 81-81: Delete ··

(prettier/prettier)


[error] 85-85: Replace ···· with ··

(prettier/prettier)


[error] 86-86: Delete ··

(prettier/prettier)


[error] 87-87: Replace ········ with ····

(prettier/prettier)


[error] 88-88: Replace ········ with ····

(prettier/prettier)


[error] 89-89: Delete ··

(prettier/prettier)


[error] 90-90: Delete ··

(prettier/prettier)


[error] 94-94: Replace ····'category1' with ··category1

(prettier/prettier)


[error] 95-95: Delete ····

(prettier/prettier)


[error] 96-96: Replace ········ with ····

(prettier/prettier)


[error] 97-97: Delete ··

(prettier/prettier)


[error] 101-101: Delete ··

(prettier/prettier)


[error] 102-102: Delete ··

(prettier/prettier)


[error] 103-103: Delete ····

(prettier/prettier)


[error] 104-104: Delete ····

(prettier/prettier)


[error] 105-105: Delete ··

(prettier/prettier)


[error] 106-106: Delete ··

(prettier/prettier)


[error] 110-110: Replace ··'category1' with category1

(prettier/prettier)


[error] 111-111: Delete ····

(prettier/prettier)


[error] 112-112: Delete ····

(prettier/prettier)


[error] 113-113: Delete ··

(prettier/prettier)


[error] 117-117: Delete ··

(prettier/prettier)


[error] 118-118: Replace ···· with ··

(prettier/prettier)


[error] 119-119: Replace ········ with ····

(prettier/prettier)


[error] 120-120: Replace ········ with ····

(prettier/prettier)


[error] 121-121: Delete ··

(prettier/prettier)


[error] 122-122: Replace ···· with ··

(prettier/prettier)


[error] 126-126: Delete ··

(prettier/prettier)


[error] 127-127: Delete ····

(prettier/prettier)


[error] 128-128: Replace ········ with ····

(prettier/prettier)


[error] 129-129: Replace ···· with ··

(prettier/prettier)


[error] 133-133: Delete ··

(prettier/prettier)


[error] 134-134: Replace ········ with ····

(prettier/prettier)


[error] 135-135: Replace ···· with ··

(prettier/prettier)


[error] 139-139: Delete ··

(prettier/prettier)


[error] 140-140: Replace ···· with ··

(prettier/prettier)


[error] 141-141: Delete ····

(prettier/prettier)


[error] 142-142: Replace ········ with ····

(prettier/prettier)


[error] 143-143: Delete ··

(prettier/prettier)


[error] 144-144: Replace ···· with ··

(prettier/prettier)


[error] 148-148: Delete ··

(prettier/prettier)


[error] 149-149: Replace ········ with ····

(prettier/prettier)


[error] 150-150: Delete ····

(prettier/prettier)


[error] 151-151: Replace ···· with ··

(prettier/prettier)


[error] 155-155: Delete ··

(prettier/prettier)


[error] 156-156: Replace ········ with ····

(prettier/prettier)


[error] 157-157: Delete ··

(prettier/prettier)


[error] 161-161: Delete ··

(prettier/prettier)


[error] 162-162: Delete ····

(prettier/prettier)


[error] 163-163: Replace ········ with ····

(prettier/prettier)


[error] 164-164: Delete ··

(prettier/prettier)


[error] 168-168: Delete ··

(prettier/prettier)


[error] 169-169: Replace ········ with ····

(prettier/prettier)


[error] 170-170: Delete ······

(prettier/prettier)


[error] 171-171: Delete ········

(prettier/prettier)


[error] 172-172: Delete ········

(prettier/prettier)


[error] 173-173: Delete ········

(prettier/prettier)


[error] 174-174: Delete ······

(prettier/prettier)


[error] 175-175: Replace ········ with ····

(prettier/prettier)


[error] 176-176: Delete ··

(prettier/prettier)


[error] 180-180: Replace ···· with ··

(prettier/prettier)


[error] 181-181: Delete ··

(prettier/prettier)


[error] 182-182: Replace ········ with ····

(prettier/prettier)


[error] 183-183: Replace ········ with ····

(prettier/prettier)


[error] 184-184: Replace ···· with ··

(prettier/prettier)


[error] 185-185: Delete ··

(prettier/prettier)


[error] 189-189: Delete ··

(prettier/prettier)


[error] 190-190: Replace ········ with ····

(prettier/prettier)


[error] 191-191: Replace ········ with ····

(prettier/prettier)


[error] 192-192: Replace ···· with ··

(prettier/prettier)


[error] 196-196: Replace ···· with ··

(prettier/prettier)


[error] 197-197: Delete ··

(prettier/prettier)


[error] 198-198: Replace ···· with ··

(prettier/prettier)


[error] 199-199: Replace ···· with ··

(prettier/prettier)


[error] 200-200: Replace ···· with ··

(prettier/prettier)


[error] 201-201: Delete ··

(prettier/prettier)


[error] 202-202: Replace ···· with ··

(prettier/prettier)


[error] 203-203: Replace ···· with ··

(prettier/prettier)


[error] 204-204: Replace ···· with ··

(prettier/prettier)


[error] 205-205: Delete ··

(prettier/prettier)


[error] 206-206: Replace ···· with ··

(prettier/prettier)


[error] 207-207: Delete ··

(prettier/prettier)


[error] 208-208: Replace ···· with ··

(prettier/prettier)


[error] 209-209: Delete ··

(prettier/prettier)


[error] 210-210: Replace ···· with ··

(prettier/prettier)


[error] 211-211: Delete ··

(prettier/prettier)


[error] 212-212: Insert ;

(prettier/prettier)

tests/tools/combine-tools.test.js

[error] 23-23: 'jest' is not defined.

(no-undef)


[error] 24-24: 'jest' is not defined.

(no-undef)


[error] 25-25: 'jest' is not defined.

(no-undef)


[error] 25-25: Delete ,

(prettier/prettier)


[error] 27-28: Delete

(prettier/prettier)


[error] 30-30: 'jest' is not defined.

(no-undef)


[error] 31-31: 'jest' is not defined.

(no-undef)


[error] 34-34: 'jest' is not defined.

(no-undef)


[error] 45-45: 'jest' is not defined.

(no-undef)


[error] 54-54: 'describe' is not defined.

(no-undef)


[error] 64-64: 'beforeAll' is not defined.

(no-undef)


[error] 69-69: 'afterAll' is not defined.

(no-undef)


[error] 76-76: 'beforeEach' is not defined.

(no-undef)


[error] 77-77: 'jest' is not defined.

(no-undef)


[error] 77-77: Delete ·

(prettier/prettier)


[error] 80-80: 'afterEach' is not defined.

(no-undef)


[error] 81-81: 'jest' is not defined.

(no-undef)


[error] 84-84: 'it' is not defined.

(no-undef)


[error] 93-93: Insert ;

(prettier/prettier)


[error] 96-96: 'it' is not defined.

(no-undef)


[error] 96-97: Delete

(prettier/prettier)


[error] 104-104: 'it' is not defined.

(no-undef)


[error] 105-106: Delete

(prettier/prettier)


[error] 109-109: Replace tool with (tool)

(prettier/prettier)


[error] 113-113: 'it' is not defined.

(no-undef)


[error] 113-114: Delete

(prettier/prettier)


[error] 128-128: 'it' is not defined.

(no-undef)


[error] 128-129: Delete

(prettier/prettier)


[error] 144-144: 'it' is not defined.

(no-undef)


[error] 145-146: Delete

(prettier/prettier)


[error] 170-170: 'it' is not defined.

(no-undef)


[error] 171-172: Delete

(prettier/prettier)


[error] 188-188: 'it' is not defined.

(no-undef)


[error] 188-189: Delete

(prettier/prettier)


[error] 192-193: Delete

(prettier/prettier)


[error] 206-206: 'it' is not defined.

(no-undef)


[error] 206-207: Delete

(prettier/prettier)


[error] 216-216: 'it' is not defined.

(no-undef)


[error] 218-218: Replace "this/is/not/valid" with 'this/is/not/valid';

(prettier/prettier)


[error] 229-229: 'it' is not defined.

(no-undef)


[error] 241-241: 'it' is not defined.

(no-undef)


[error] 253-253: 'it' is not defined.

(no-undef)

🔇 Additional comments (2)
scripts/tools/combine-tools.js (1)

139-140: LGTM: File operations are appropriate for this use case

The synchronous file operations are suitable for this script's requirements.

🧰 Tools
🪛 eslint

[error] 139-139: Delete ····

(prettier/prettier)


[error] 140-140: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)

tests/tools/combine-tools.test.js (1)

216-227: 🛠️ Refactor suggestion

Refactor error handling tests using Jest's async utilities.

These tests use verbose try-catch blocks. Consider using Jest's .rejects.toThrow() for cleaner and more idiomatic error handling tests.

Also applies to: 229-239, 241-251, 253-264

🧰 Tools
🪛 eslint

[error] 216-216: 'it' is not defined.

(no-undef)


[error] 218-218: Replace "this/is/not/valid" with 'this/is/not/valid';

(prettier/prettier)

Comment on lines 1 to 26
const expectedDataT1 = {
languages: [
{
name: 'JavaScript',
color: 'bg-[#57f281]',
borderColor: 'border-[#37f069]'
},
{
name: 'Python',
color: 'bg-[#3572A5]',
borderColor: 'border-[#3572A5]'
}
],
technologies: [
{
name: 'Node.js',
color: 'bg-[#61d0f2]',
borderColor: 'border-[#40ccf7]'
},
{
name: 'Flask',
color: 'bg-[#000000]',
borderColor: 'border-[#FFFFFF]'
}
]
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider reusing tagsData instead of duplicating data.

The color and border color data for languages and technologies appears to be duplicated from tagsData. To maintain consistency and reduce maintenance overhead, consider importing and reusing the existing data.

Example approach:

const { tagsData } = require('../path/to/tagsData');

const expectedDataT1 = {
    languages: tagsData.languages.map(({ name, color, borderColor }) => ({
        name,
        color,
        borderColor
    })).filter(lang => ['JavaScript', 'Python'].includes(lang.name)),
    technologies: tagsData.technologies.map(({ name, color, borderColor }) => ({
        name,
        color,
        borderColor
    })).filter(tech => ['Node.js', 'Flask'].includes(tech.name))
};
🧰 Tools
🪛 eslint

[error] 2-2: Delete ··

(prettier/prettier)


[error] 3-3: Delete ····

(prettier/prettier)


[error] 4-4: Replace ············ with ······

(prettier/prettier)


[error] 5-5: Replace ············ with ······

(prettier/prettier)


[error] 6-6: Delete ······

(prettier/prettier)


[error] 7-7: Replace ········ with ····

(prettier/prettier)


[error] 8-8: Replace ········ with ····

(prettier/prettier)


[error] 9-9: Replace ············ with ······

(prettier/prettier)


[error] 10-10: Replace ············ with ······

(prettier/prettier)


[error] 11-11: Delete ······

(prettier/prettier)


[error] 12-12: Delete ····

(prettier/prettier)


[error] 13-13: Delete ··

(prettier/prettier)


[error] 14-14: Delete ··

(prettier/prettier)


[error] 15-15: Delete ····

(prettier/prettier)


[error] 16-16: Replace ············ with ······

(prettier/prettier)


[error] 17-17: Delete ······

(prettier/prettier)


[error] 18-18: Replace ············ with ······

(prettier/prettier)


[error] 19-19: Replace ········ with ····

(prettier/prettier)


[error] 20-20: Replace ········ with ····

(prettier/prettier)


[error] 21-21: Replace ············ with ······

(prettier/prettier)


[error] 22-22: Replace ············ with ······

(prettier/prettier)


[error] 23-23: Delete ······

(prettier/prettier)


[error] 24-24: Replace ········ with ····

(prettier/prettier)


[error] 25-25: Delete ··

(prettier/prettier)

Comment on lines +1 to +82
const fs = require('fs');
const path = require('path');
const { combineTools } = require('../../scripts/tools/combine-tools');
const {
expectedDataT1,
manualToolsWithMissingData,
manualToolsToSort,
automatedToolsT5,
automatedToolsT4,
manualToolsT4,
automatedToolsT6,
automatedToolsT7,
automatedToolsT8,
manualToolsT8,
automatedToolsT9,
manualToolsT9,
automatedToolsT12,
invalidAutomatedToolsT10,
manualToolsWithInvalidURLT11,
circularTool
} = require('../fixtures/combineToolsData');

jest.mock('ajv', () => {
return jest.fn().mockImplementation(() => ({
compile: jest.fn().mockImplementation(() => (data) => data.title !== 'Invalid Tool'),
}));
});


jest.mock('ajv-formats', () => {
return jest.fn();
});

jest.mock('../../scripts/tools/tags-color', () => ({
languagesColor: [
{ name: 'JavaScript', color: 'bg-[#57f281]', borderColor: 'border-[#37f069]' },
{ name: 'Python', color: 'bg-[#3572A5]', borderColor: 'border-[#3572A5]' }
],
technologiesColor: [
{ name: 'Node.js', color: 'bg-[#61d0f2]', borderColor: 'border-[#40ccf7]' },
{ name: 'Flask', color: 'bg-[#000000]', borderColor: 'border-[#FFFFFF]' }
]
}));

jest.mock('../../scripts/tools/categorylist', () => ({
categoryList: [
{ name: 'category1', description: 'Sample Category 1' },
{ name: 'category2', description: 'Sample Category 2' }
]
}));

const readJSON = (filePath) => JSON.parse(fs.readFileSync(filePath, 'utf-8'));

describe('combineTools function', () => {
const toolsPath = path.join(__dirname, '../', 'fixtures', 'tools', 'tools.json');
const tagsPath = path.join(__dirname, '../', 'fixtures', 'tools', 'tags.json');
const manualToolsPath = path.join(__dirname, '../', 'fixtures', 'tools', 'manual-tools.json');
const automatedToolsPath = path.join(__dirname, '../', 'fixtures', 'tools', 'automated-tools.json');

let manualTools;
let automatedTools;
let consoleErrorMock;

beforeAll(() => {
manualTools = readJSON(manualToolsPath);
automatedTools = readJSON(automatedToolsPath);
});

afterAll(() => {
if (fs.existsSync(toolsPath)) fs.unlinkSync(toolsPath);
if (fs.existsSync(tagsPath)) fs.unlinkSync(tagsPath);

consoleErrorMock.mockRestore();
});

beforeEach(() => {
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => { });
});

afterEach(() => {
jest.clearAllMocks();
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add ESLint configuration for Jest environment.

The static analysis shows multiple 'undefined' errors for Jest globals. This can be fixed by adding the Jest environment to your ESLint configuration.

Add this to your .eslintrc.js or equivalent:

module.exports = {
  env: {
    jest: true
  }
}
🧰 Tools
🪛 eslint

[error] 23-23: 'jest' is not defined.

(no-undef)


[error] 24-24: 'jest' is not defined.

(no-undef)


[error] 25-25: 'jest' is not defined.

(no-undef)


[error] 25-25: Delete ,

(prettier/prettier)


[error] 27-28: Delete

(prettier/prettier)


[error] 30-30: 'jest' is not defined.

(no-undef)


[error] 31-31: 'jest' is not defined.

(no-undef)


[error] 34-34: 'jest' is not defined.

(no-undef)


[error] 45-45: 'jest' is not defined.

(no-undef)


[error] 54-54: 'describe' is not defined.

(no-undef)


[error] 64-64: 'beforeAll' is not defined.

(no-undef)


[error] 69-69: 'afterAll' is not defined.

(no-undef)


[error] 76-76: 'beforeEach' is not defined.

(no-undef)


[error] 77-77: 'jest' is not defined.

(no-undef)


[error] 77-77: Delete ·

(prettier/prettier)


[error] 80-80: 'afterEach' is not defined.

(no-undef)


[error] 81-81: 'jest' is not defined.

(no-undef)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
tests/tools/combine-tools.test.js (2)

69-74: Improve test file cleanup

Consider adding file cleanup in beforeAll to ensure a clean state for each test run, and combine the file existence checks for better readability.

 afterAll(() => {
-  if (fs.existsSync(toolsPath)) fs.unlinkSync(toolsPath);
-  if (fs.existsSync(tagsPath)) fs.unlinkSync(tagsPath);
+  [toolsPath, tagsPath].forEach(file => {
+    if (fs.existsSync(file)) fs.unlinkSync(file);
+  });

   consoleErrorMock.mockRestore();
 });

84-215: Improve test consistency and assertions

The test cases are well-structured, but consider these improvements:

  1. Remove unnecessary blank lines between test blocks
  2. Use more specific assertions where possible
  3. Add test cases for edge cases (empty arrays, null values)

Example of more specific assertions:

-    expect(combinedTools).toHaveProperty('category1');
+    expect(combinedTools.category1).toBeDefined();
+    expect(combinedTools.category1.toolsList).toBeInstanceOf(Array);
🧰 Tools
🪛 eslint

[error] 93-93: Insert ;

(prettier/prettier)


[error] 96-97: Delete

(prettier/prettier)


[error] 104-105: Delete

(prettier/prettier)


[error] 109-109: Replace tool with (tool)

(prettier/prettier)


[error] 113-114: Delete

(prettier/prettier)


[error] 129-130: Delete

(prettier/prettier)


[error] 144-145: Delete

(prettier/prettier)


[error] 170-171: Delete

(prettier/prettier)


[error] 189-191: Replace ⏎····await·combineTools(automatedToolsT8,·manualToolsT8,·toolsPath,·tagsPath);⏎ with ····await·combineTools(automatedToolsT8,·manualToolsT8,·toolsPath,·tagsPath);

(prettier/prettier)


[error] 207-208: Delete

(prettier/prettier)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ca72a7f and 1698ce2.

📒 Files selected for processing (1)
  • tests/tools/combine-tools.test.js (1 hunks)
🧰 Additional context used
🪛 eslint
tests/tools/combine-tools.test.js

[error] 25-25: Delete ,

(prettier/prettier)


[error] 27-28: Delete

(prettier/prettier)


[error] 77-77: Delete ·

(prettier/prettier)


[error] 93-93: Insert ;

(prettier/prettier)


[error] 96-97: Delete

(prettier/prettier)


[error] 104-105: Delete

(prettier/prettier)


[error] 109-109: Replace tool with (tool)

(prettier/prettier)


[error] 113-114: Delete

(prettier/prettier)


[error] 129-130: Delete

(prettier/prettier)


[error] 144-145: Delete

(prettier/prettier)


[error] 170-171: Delete

(prettier/prettier)


[error] 189-191: Replace ⏎····await·combineTools(automatedToolsT8,·manualToolsT8,·toolsPath,·tagsPath);⏎ with ····await·combineTools(automatedToolsT8,·manualToolsT8,·toolsPath,·tagsPath);

(prettier/prettier)


[error] 207-208: Delete

(prettier/prettier)


[error] 218-218: Replace "this/is/not/valid" with 'this/is/not/valid';

(prettier/prettier)


[error] 264-265: Delete ⏎··

(prettier/prettier)

Comment on lines 216 to 264
it('should throw an error when fs.writeFileSync fails', async () => {
let error;
const invalidPath = "this/is/not/valid"

try {
await combineTools(automatedTools, manualTools, invalidPath, invalidPath);
} catch (err) {
error = err;
expect(err.message).toMatch(/ENOENT|EACCES/);
}
expect(error).toBeDefined();
});

it('should throw an error when there is an invalid category', async () => {
let error;

try {
await combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Error combining tools');
}
expect(error).toBeDefined();
});

it('should throw an error when URL parsing fails', async () => {
let error;

try {
await combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Invalid URL');
}
expect(error).toBeDefined();
});

it('should handle errors when processing tools with circular references', async () => {
let error;
circularTool.circular = circularTool;

try {
await combineTools(automatedToolsT12, {}, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Converting circular structure to JSON');
}
expect(error).toBeDefined();
});
Copy link

@coderabbitai coderabbitai bot Nov 13, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify error handling tests using Jest's .rejects.toThrow()

The current error handling tests use verbose try-catch blocks. Use Jest's built-in .rejects.toThrow() for cleaner and more idiomatic tests.

-  it('should throw an error when fs.writeFileSync fails', async () => {
-    let error;
-    const invalidPath = "this/is/not/valid"
-
-    try {
-      await combineTools(automatedTools, manualTools, invalidPath, invalidPath);
-    } catch (err) {
-      error = err;
-      expect(err.message).toMatch(/ENOENT|EACCES/);
-    }
-    expect(error).toBeDefined();
-  });
+  it('should throw an error when fs.writeFileSync fails', async () => {
+    const invalidPath = 'this/is/not/valid';
+    await expect(combineTools(automatedTools, manualTools, invalidPath, invalidPath))
+      .rejects.toThrow(/ENOENT|EACCES/);
+  });

-  it('should throw an error when there is an invalid category', async () => {
-    let error;
-
-    try {
-      await combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath);
-    } catch (err) {
-      error = err;
-      expect(err.message).toContain('Error combining tools');
-    }
-    expect(error).toBeDefined();
-  });
+  it('should throw an error when there is an invalid category', async () => {
+    await expect(combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath))
+      .rejects.toThrow('Error combining tools');
+  });

-  it('should throw an error when URL parsing fails', async () => {
-    let error;
-
-    try {
-      await combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath);
-    } catch (err) {
-      error = err;
-      expect(err.message).toContain('Invalid URL');
-    }
-    expect(error).toBeDefined();
-  });
+  it('should throw an error when URL parsing fails', async () => {
+    await expect(combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath))
+      .rejects.toThrow('Invalid URL');
+  });

-  it('should handle errors when processing tools with circular references', async () => {
-    let error;
-    circularTool.circular = circularTool;
-
-    try {
-      await combineTools(automatedToolsT12, {}, toolsPath, tagsPath);
-    } catch (err) {
-      error = err;
-      expect(err.message).toContain('Converting circular structure to JSON');
-    }
-    expect(error).toBeDefined();
-  });
+  it('should handle errors when processing tools with circular references', async () => {
+    circularTool.circular = circularTool;
+    await expect(combineTools(automatedToolsT12, {}, toolsPath, tagsPath))
+      .rejects.toThrow('Converting circular structure to JSON');
+  });
📝 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('should throw an error when fs.writeFileSync fails', async () => {
let error;
const invalidPath = "this/is/not/valid"
try {
await combineTools(automatedTools, manualTools, invalidPath, invalidPath);
} catch (err) {
error = err;
expect(err.message).toMatch(/ENOENT|EACCES/);
}
expect(error).toBeDefined();
});
it('should throw an error when there is an invalid category', async () => {
let error;
try {
await combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Error combining tools');
}
expect(error).toBeDefined();
});
it('should throw an error when URL parsing fails', async () => {
let error;
try {
await combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Invalid URL');
}
expect(error).toBeDefined();
});
it('should handle errors when processing tools with circular references', async () => {
let error;
circularTool.circular = circularTool;
try {
await combineTools(automatedToolsT12, {}, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Converting circular structure to JSON');
}
expect(error).toBeDefined();
});
it('should throw an error when fs.writeFileSync fails', async () => {
const invalidPath = 'this/is/not/valid';
await expect(combineTools(automatedTools, manualTools, invalidPath, invalidPath))
.rejects.toThrow(/ENOENT|EACCES/);
});
it('should throw an error when there is an invalid category', async () => {
await expect(combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath))
.rejects.toThrow('Error combining tools');
});
it('should throw an error when URL parsing fails', async () => {
await expect(combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath))
.rejects.toThrow('Invalid URL');
});
it('should handle errors when processing tools with circular references', async () => {
circularTool.circular = circularTool;
await expect(combineTools(automatedToolsT12, {}, toolsPath, tagsPath))
.rejects.toThrow('Converting circular structure to JSON');
});
🧰 Tools
🪛 eslint

[error] 218-218: Replace "this/is/not/valid" with 'this/is/not/valid';

(prettier/prettier)

Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 Apply this suggestion

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 any updates on this suggestion?

tests/tools/combine-tools.test.js Show resolved Hide resolved
});


it('should log validation errors to console.error', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 Have you worked on this?

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

Added few more comments.

@@ -0,0 +1,212 @@
const expectedDataT1 = {
Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 This file has indentation errors. Kindly fix it.

Comment on lines 129 to 133
console.error('Script is not failing, it is just dropping errors for further investigation');
console.error(`Invalid ${tool.title} .asyncapi-tool file.`);
console.error(`Located in manual-tools.json file`);
console.error('Validation errors:', JSON.stringify(validate.errors, null, 2));
}
Copy link
Member

Choose a reason for hiding this comment

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

@JeelRajodiya Kindly take a note of this. We have to improve error handling for this function.

});

it('should handle tools with multiple languages, including new ones', async () => {

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra line from here in every test. Is it due to eslint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I added that myself. I'll remove it

Comment on lines 216 to 264
it('should throw an error when fs.writeFileSync fails', async () => {
let error;
const invalidPath = "this/is/not/valid"

try {
await combineTools(automatedTools, manualTools, invalidPath, invalidPath);
} catch (err) {
error = err;
expect(err.message).toMatch(/ENOENT|EACCES/);
}
expect(error).toBeDefined();
});

it('should throw an error when there is an invalid category', async () => {
let error;

try {
await combineTools(invalidAutomatedToolsT10, manualTools, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Error combining tools');
}
expect(error).toBeDefined();
});

it('should throw an error when URL parsing fails', async () => {
let error;

try {
await combineTools(automatedTools, manualToolsWithInvalidURLT11, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Invalid URL');
}
expect(error).toBeDefined();
});

it('should handle errors when processing tools with circular references', async () => {
let error;
circularTool.circular = circularTool;

try {
await combineTools(automatedToolsT12, {}, toolsPath, tagsPath);
} catch (err) {
error = err;
expect(err.message).toContain('Converting circular structure to JSON');
}
expect(error).toBeDefined();
});
Copy link
Member

Choose a reason for hiding this comment

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

@vishvamsinh28 any updates on this suggestion?

@vishvamsinh28
Copy link
Contributor Author

@akshatnema applied suggested changes

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (5)
tests/fixtures/combineToolsData.js (2)

28-52: Enhance test data documentation.

Add JSDoc comments to describe the purpose of each test data constant and what scenario it's testing. This will make the test suite more maintainable and self-documenting.

Example:

+/**
+ * Test data for tools with missing required fields.
+ * Used to verify the behavior when processing incomplete tool data.
+ */
 const manualToolsWithMissingData = [
   {
     title: 'Tool C',
     filters: {},
     links: { repoUrl: 'https://github.com/asyncapi/tool-c' },
   },
 ];
🧰 Tools
🪛 eslint

[error] 32-32: Delete ,

(prettier/prettier)


[error] 33-33: Delete ,

(prettier/prettier)


[error] 43-43: Delete ,

(prettier/prettier)


[error] 48-48: Delete ,

(prettier/prettier)


[error] 49-49: Delete ,

(prettier/prettier)


[error] 50-50: Delete ,

(prettier/prettier)


[error] 51-51: Delete ,

(prettier/prettier)


70-82: Use meaningful test data names.

The test data constants use numeric suffixes (T4, T5, etc.) which don't convey their purpose. Consider using more descriptive names that indicate the test scenario.

Example:

-const invalidToolT4 = { title: 'Invalid Tool' };
+const invalidToolMissingFields = { title: 'Invalid Tool' };

-const automatedToolsT4 = {
+const automatedToolsEmpty = {
🧰 Tools
🪛 eslint

[error] 75-75: Delete ,

(prettier/prettier)


[error] 76-76: Delete ,

(prettier/prettier)


[error] 80-80: Delete ,

(prettier/prettier)


[error] 81-81: Delete ,

(prettier/prettier)

scripts/tools/combine-tools.js (2)

109-146: Improve error handling structure.

The error handling could be more specific and informative. Consider:

  1. Using a custom error class for different error types.
  2. Including more context in error messages.
+class ToolsCombineError extends Error {
+  constructor(operation, details) {
+    super(`Failed to ${operation}: ${details}`);
+    this.name = 'ToolsCombineError';
+  }
+}

 try {
   // ... existing code ...
 } catch (err) {
-  throw new Error(`Error combining tools: ${err}`);
+  throw new ToolsCombineError('combine tools', err.message);
 }
🧰 Tools
🪛 eslint

[error] 109-109: Delete ··

(prettier/prettier)


[error] 110-110: Delete ····

(prettier/prettier)


[error] 110-141: for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.

(no-restricted-syntax)


[error] 110-141: The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype.

(guard-for-in)


[error] 111-111: Delete ······

(prettier/prettier)


[error] 111-111: 'finalToolsList' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 112-112: Replace ············ with ······

(prettier/prettier)


[error] 113-113: Delete ········

(prettier/prettier)


[error] 113-115: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 114-114: Replace ····················finalToolsList.push(await·getFinalTool(tool)) with ··········finalToolsList.push(await·getFinalTool(tool));

(prettier/prettier)


[error] 114-114: Unexpected await inside a loop.

(no-await-in-loop)


[error] 115-115: Replace ················ with ········

(prettier/prettier)


[error] 116-116: Replace ············ with ······

(prettier/prettier)


[error] 117-117: Replace ············ with ······

(prettier/prettier)


[error] 118-118: Delete ········

(prettier/prettier)


[error] 118-137: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 119-119: Replace ···················· with ··········

(prettier/prettier)


[error] 120-120: Replace ····················const·isValid·=·await·validate(tool) with ··········const·isValid·=·await·validate(tool);

(prettier/prettier)


[error] 120-120: Unexpected await inside a loop.

(no-await-in-loop)


[error] 121-121: Delete ··········

(prettier/prettier)


[error] 122-122: Replace ························ with ············

(prettier/prettier)


[error] 123-123: Replace ····························const·url·=·new·URL(tool.links.repoUrl) with ··············const·url·=·new·URL(tool.links.repoUrl);

(prettier/prettier)


[error] 124-124: Replace ····························isAsyncAPIrepo·=·url.href.startsWith("https://github.com/asyncapi/") with ··············isAsyncAPIrepo·=·url.href.startsWith('https://github.com/asyncapi/');

(prettier/prettier)


[error] 125-125: Replace ············}·else·isAsyncAPIrepo·=·false with }·else·isAsyncAPIrepo·=·false;

(prettier/prettier)


[error] 126-126: Replace ························let·toolObject·=·await·createToolObject(tool,·"",·"",·isAsyncAPIrepo) with ············let·toolObject·=·await·createToolObject(tool,·'',·'',·isAsyncAPIrepo);

(prettier/prettier)


[error] 126-126: 'toolObject' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 126-126: Unexpected await inside a loop.

(no-await-in-loop)


[error] 127-127: Replace ························finalToolsList.push(await·getFinalTool(toolObject)) with ············finalToolsList.push(await·getFinalTool(toolObject));

(prettier/prettier)


[error] 127-127: Unexpected await inside a loop.

(no-await-in-loop)


[error] 128-128: Delete ··········

(prettier/prettier)


[error] 129-129: Delete ············

(prettier/prettier)


[error] 130-130: Replace ···························· with ··············

(prettier/prettier)


[error] 131-131: Delete ··············

(prettier/prettier)


[error] 132-132: Replace ···························· with ··············

(prettier/prettier)


[error] 133-133: Replace ···························· with ··············

(prettier/prettier)


[error] 134-134: Replace ···························· with ··············

(prettier/prettier)


[error] 135-135: Delete ············

(prettier/prettier)


[error] 136-136: Replace ···················· with ··········

(prettier/prettier)


[error] 137-137: Delete ········

(prettier/prettier)


[error] 138-138: Delete ······

(prettier/prettier)


[error] 139-139: Delete ······

(prettier/prettier)


[error] 140-140: Replace ············finalTools[key].toolsList·=·finalToolsList with ······finalTools[key].toolsList·=·finalToolsList;

(prettier/prettier)


[error] 141-141: Replace ········ with ····

(prettier/prettier)


[error] 142-142: Delete ····

(prettier/prettier)


[error] 143-143: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)


[error] 144-144: Delete ··

(prettier/prettier)


[error] 145-145: Replace ········ with ····

(prettier/prettier)


[error] 146-146: Delete ··

(prettier/prettier)


142-143: Consider using atomic file operations.

The current file writing operations could be made more robust by using atomic writes to prevent partial updates.

-fs.writeFileSync(toolsPath, JSON.stringify(finalTools));
-fs.writeFileSync(tagsPath, JSON.stringify({ languages: languageList, technologies: technologyList }),);
+const writeFileAtomic = require('write-file-atomic');
+
+writeFileAtomic.sync(toolsPath, JSON.stringify(finalTools, null, 2));
+writeFileAtomic.sync(tagsPath, JSON.stringify({
+  languages: languageList,
+  technologies: technologyList
+}, null, 2));
🧰 Tools
🪛 eslint

[error] 142-142: Delete ····

(prettier/prettier)


[error] 143-143: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)

tests/tools/combine-tools.test.js (1)

84-227: Improve test organization and readability.

The test suite could benefit from better organization:

  1. Group related tests using describe blocks.
  2. Add more descriptive test names.

Example:

+describe('successful operations', () => {
   it('should combine tools and create correct JSON files', async () => {
     // ... existing test ...
   });
 });

+describe('error handling', () => {
   it('should throw an error when fs.writeFileSync fails', async () => {
     // ... existing test ...
   });
 });
🧰 Tools
🪛 eslint

[error] 93-93: Insert ;

(prettier/prettier)


[error] 107-107: Replace tool with (tool)

(prettier/prettier)


[error] 209-210: Delete ⏎······

(prettier/prettier)


[error] 214-215: Replace ⏎······.rejects.toThrow('Error·combining·tools' with .rejects.toThrow(⏎······'Error·combining·tools'⏎····

(prettier/prettier)


[error] 219-220: Replace ⏎······.rejects.toThrow('Invalid·URL' with .rejects.toThrow(⏎······'Invalid·URL'⏎····

(prettier/prettier)


[error] 225-226: Replace ⏎······.rejects.toThrow('Converting·circular·structure·to·JSON' with .rejects.toThrow(⏎······'Converting·circular·structure·to·JSON'⏎····

(prettier/prettier)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1698ce2 and 98f3238.

📒 Files selected for processing (3)
  • scripts/tools/combine-tools.js (1 hunks)
  • tests/fixtures/combineToolsData.js (1 hunks)
  • tests/tools/combine-tools.test.js (1 hunks)
🧰 Additional context used
📓 Learnings (1)
scripts/tools/combine-tools.js (1)
Learnt from: akshatnema
PR: asyncapi/website#3136
File: scripts/tools/combine-tools.js:122-125
Timestamp: 2024-11-12T06:59:33.852Z
Learning: In `scripts/tools/combine-tools.js`, the existing URL parsing logic for `repoUrl` without additional error handling is acceptable and does not require changes.
🪛 eslint
tests/fixtures/combineToolsData.js

[error] 6-6: Delete ,

(prettier/prettier)


[error] 11-11: Delete ,

(prettier/prettier)


[error] 12-12: Delete ,

(prettier/prettier)


[error] 18-18: Delete ,

(prettier/prettier)


[error] 23-23: Delete ,

(prettier/prettier)


[error] 24-24: Delete ,

(prettier/prettier)


[error] 25-25: Delete ,

(prettier/prettier)


[error] 32-32: Delete ,

(prettier/prettier)


[error] 33-33: Delete ,

(prettier/prettier)


[error] 43-43: Delete ,

(prettier/prettier)


[error] 48-48: Delete ,

(prettier/prettier)


[error] 49-49: Delete ,

(prettier/prettier)


[error] 50-50: Delete ,

(prettier/prettier)


[error] 51-51: Delete ,

(prettier/prettier)


[error] 58-58: Delete ,

(prettier/prettier)


[error] 60-60: Delete ,

(prettier/prettier)


[error] 66-66: Delete ,

(prettier/prettier)


[error] 67-67: Delete ,

(prettier/prettier)


[error] 75-75: Delete ,

(prettier/prettier)


[error] 76-76: Delete ,

(prettier/prettier)


[error] 80-80: Delete ,

(prettier/prettier)


[error] 81-81: Delete ,

(prettier/prettier)


[error] 88-88: Delete ,

(prettier/prettier)


[error] 90-90: Delete ,

(prettier/prettier)


[error] 96-96: Delete ,

(prettier/prettier)


[error] 97-97: Delete ,

(prettier/prettier)


[error] 104-104: Delete ,

(prettier/prettier)


[error] 106-106: Delete ,

(prettier/prettier)


[error] 112-112: Delete ,

(prettier/prettier)


[error] 113-113: Delete ,

(prettier/prettier)


[error] 120-120: Delete ,

(prettier/prettier)


[error] 122-122: Delete ,

(prettier/prettier)


[error] 128-128: Delete ,

(prettier/prettier)


[error] 129-129: Delete ,

(prettier/prettier)


[error] 134-134: Delete ,

(prettier/prettier)


[error] 135-135: Delete ,

(prettier/prettier)


[error] 142-142: Delete ,

(prettier/prettier)


[error] 144-144: Delete ,

(prettier/prettier)


[error] 150-150: Delete ,

(prettier/prettier)


[error] 151-151: Delete ,

(prettier/prettier)


[error] 156-156: Delete ,

(prettier/prettier)


[error] 157-157: Delete ,

(prettier/prettier)


[error] 163-163: Delete ,

(prettier/prettier)


[error] 164-164: Delete ,

(prettier/prettier)


[error] 173-173: Delete ,

(prettier/prettier)


[error] 174-174: Delete ,

(prettier/prettier)


[error] 175-175: Delete ,

(prettier/prettier)


[error] 176-176: Delete ,

(prettier/prettier)


[error] 183-183: Delete ,

(prettier/prettier)


[error] 185-185: Delete ,

(prettier/prettier)


[error] 191-191: Delete ,

(prettier/prettier)


[error] 192-192: Delete ,

(prettier/prettier)


[error] 211-211: Delete ,

(prettier/prettier)

scripts/tools/combine-tools.js

[error] 109-109: Delete ··

(prettier/prettier)


[error] 110-110: Delete ····

(prettier/prettier)


[error] 110-141: for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.

(no-restricted-syntax)


[error] 110-141: The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype.

(guard-for-in)


[error] 111-111: Delete ······

(prettier/prettier)


[error] 111-111: 'finalToolsList' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 112-112: Replace ············ with ······

(prettier/prettier)


[error] 113-113: Delete ········

(prettier/prettier)


[error] 113-115: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 114-114: Replace ····················finalToolsList.push(await·getFinalTool(tool)) with ··········finalToolsList.push(await·getFinalTool(tool));

(prettier/prettier)


[error] 114-114: Unexpected await inside a loop.

(no-await-in-loop)


[error] 115-115: Replace ················ with ········

(prettier/prettier)


[error] 116-116: Replace ············ with ······

(prettier/prettier)


[error] 117-117: Replace ············ with ······

(prettier/prettier)


[error] 118-118: Delete ········

(prettier/prettier)


[error] 118-137: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.

(no-restricted-syntax)


[error] 119-119: Replace ···················· with ··········

(prettier/prettier)


[error] 120-120: Replace ····················const·isValid·=·await·validate(tool) with ··········const·isValid·=·await·validate(tool);

(prettier/prettier)


[error] 120-120: Unexpected await inside a loop.

(no-await-in-loop)


[error] 121-121: Delete ··········

(prettier/prettier)


[error] 122-122: Replace ························ with ············

(prettier/prettier)


[error] 123-123: Replace ····························const·url·=·new·URL(tool.links.repoUrl) with ··············const·url·=·new·URL(tool.links.repoUrl);

(prettier/prettier)


[error] 124-124: Replace ····························isAsyncAPIrepo·=·url.href.startsWith("https://github.com/asyncapi/") with ··············isAsyncAPIrepo·=·url.href.startsWith('https://github.com/asyncapi/');

(prettier/prettier)


[error] 125-125: Replace ············}·else·isAsyncAPIrepo·=·false with }·else·isAsyncAPIrepo·=·false;

(prettier/prettier)


[error] 126-126: Replace ························let·toolObject·=·await·createToolObject(tool,·"",·"",·isAsyncAPIrepo) with ············let·toolObject·=·await·createToolObject(tool,·'',·'',·isAsyncAPIrepo);

(prettier/prettier)


[error] 126-126: 'toolObject' is never reassigned. Use 'const' instead.

(prefer-const)


[error] 126-126: Unexpected await inside a loop.

(no-await-in-loop)


[error] 127-127: Replace ························finalToolsList.push(await·getFinalTool(toolObject)) with ············finalToolsList.push(await·getFinalTool(toolObject));

(prettier/prettier)


[error] 127-127: Unexpected await inside a loop.

(no-await-in-loop)


[error] 128-128: Delete ··········

(prettier/prettier)


[error] 129-129: Delete ············

(prettier/prettier)


[error] 130-130: Replace ···························· with ··············

(prettier/prettier)


[error] 131-131: Delete ··············

(prettier/prettier)


[error] 132-132: Replace ···························· with ··············

(prettier/prettier)


[error] 133-133: Replace ···························· with ··············

(prettier/prettier)


[error] 134-134: Replace ···························· with ··············

(prettier/prettier)


[error] 135-135: Delete ············

(prettier/prettier)


[error] 136-136: Replace ···················· with ··········

(prettier/prettier)


[error] 137-137: Delete ········

(prettier/prettier)


[error] 138-138: Delete ······

(prettier/prettier)


[error] 139-139: Delete ······

(prettier/prettier)


[error] 140-140: Replace ············finalTools[key].toolsList·=·finalToolsList with ······finalTools[key].toolsList·=·finalToolsList;

(prettier/prettier)


[error] 141-141: Replace ········ with ····

(prettier/prettier)


[error] 142-142: Delete ····

(prettier/prettier)


[error] 143-143: Replace ········fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}),) with ····fs.writeFileSync(tagsPath,·JSON.stringify({·languages:·languageList,·technologies:·technologyList·}));

(prettier/prettier)


[error] 144-144: Delete ··

(prettier/prettier)


[error] 145-145: Replace ········ with ····

(prettier/prettier)


[error] 146-146: Delete ··

(prettier/prettier)


[error] 147-147: Insert ;

(prettier/prettier)


[error] 149-149: Insert ;

(prettier/prettier)

tests/tools/combine-tools.test.js

[error] 25-25: Delete ,

(prettier/prettier)


[error] 27-28: Delete

(prettier/prettier)


[error] 77-77: Delete ·

(prettier/prettier)


[error] 93-93: Insert ;

(prettier/prettier)


[error] 107-107: Replace tool with (tool)

(prettier/prettier)


[error] 209-210: Delete ⏎······

(prettier/prettier)


[error] 214-215: Replace ⏎······.rejects.toThrow('Error·combining·tools' with .rejects.toThrow(⏎······'Error·combining·tools'⏎····

(prettier/prettier)


[error] 219-220: Replace ⏎······.rejects.toThrow('Invalid·URL' with .rejects.toThrow(⏎······'Invalid·URL'⏎····

(prettier/prettier)


[error] 225-226: Replace ⏎······.rejects.toThrow('Converting·circular·structure·to·JSON' with .rejects.toThrow(⏎······'Converting·circular·structure·to·JSON'⏎····

(prettier/prettier)


[error] 227-228: Delete

(prettier/prettier)

🔇 Additional comments (1)
tests/fixtures/combineToolsData.js (1)

1-26: Consider reusing tagsData instead of duplicating data.

The color and border color data for languages and technologies appears to be duplicated from tagsData. To maintain consistency and reduce maintenance overhead, consider importing and reusing the existing data.

🧰 Tools
🪛 eslint

[error] 6-6: Delete ,

(prettier/prettier)


[error] 11-11: Delete ,

(prettier/prettier)


[error] 12-12: Delete ,

(prettier/prettier)


[error] 18-18: Delete ,

(prettier/prettier)


[error] 23-23: Delete ,

(prettier/prettier)


[error] 24-24: Delete ,

(prettier/prettier)


[error] 25-25: Delete ,

(prettier/prettier)

tests/tools/combine-tools.test.js Show resolved Hide resolved
@akshatnema
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 22fcbcf into asyncapi:master Dec 9, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc This label should be used for issues or discussions related to ideas for Google Summer of Code ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants