-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
test: add e2e tests for roadmap and case studies page #4215
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds new Cypress E2E test suites (Roadmap, Case Studies), a fixture and helpers, multiple page-object methods, and renames several base page classes with corresponding consumer updates across cypress/pages for standardized navigation and verification. Changes
Sequence Diagram(s)(No sequence diagrams provided.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4215 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 798 798
Branches 146 146
=========================================
Hits 798 798 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CaseStudiesPage.js.-.asyncapi.-.Visual.Studio.Code.2025-07-01.15-00-24.mp4casestudies and roadmap page tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (5)
cypress/pages/homepage.js (1)
25-33: Consider more specific selectors for better test reliability.The navigation methods follow good page object patterns, but using text-based selectors could be fragile if the link text changes.
Consider using more specific selectors if available:
goToCaseStudiesPage() { - cy.contains('a', 'Case Studies').click(); + cy.get('[data-testid="nav-case-studies"]').click(); // if data-testid exists return new CaseStudiesPage(); } goToRoadmapPage() { - cy.contains('a', 'Roadmap').click(); + cy.get('[data-testid="nav-roadmap"]').click(); // if data-testid exists return new RoadmapPage(); }If data-testids are not available, the current approach is acceptable for this use case.
cypress/RoadmapPage.cy.js (1)
22-32: Add missing semicolons for consistency.Missing semicolons in method calls should be added for code consistency.
it('User verifies Outcome tooltip', () => { - roadmapPage.verifyTooltip(0) + roadmapPage.verifyTooltip(0); }); it('User verifies Solution tooltip', () => { - roadmapPage.verifyTooltip(1) + roadmapPage.verifyTooltip(1); }); it('User verifies Implementation tooltip', () => { - roadmapPage.verifyTooltip(2) + roadmapPage.verifyTooltip(2); });cypress/pages/roadmap.js (1)
15-21: Consider adding explicit wait for tooltip visibility.The mouseover trigger might need a small wait to ensure the tooltip is properly displayed before verification.
verifyTooltip(index){ cy.get('[data-testid="InlineHelp-icon"]') - .eq(index) - .trigger('mouseover'); + .eq(index) + .trigger('mouseover') + .wait(100); // Small wait for tooltip animation cy.get('[data-testid="InlineHelp"]') - .should('be.visible') + .should('be.visible'); }Alternatively, you could use
.should('be.visible')with a timeout:cy.get('[data-testid="InlineHelp"]') - .should('be.visible') + .should('be.visible', { timeout: 1000 });cypress/pages/CaseStudiesPage.js (2)
18-22: Fix inconsistent formatting and improve method reliability.The method has inconsistent indentation and could be more robust.
- verifyScrollDown(){ - cy.contains('h1', 'Adopters') - .scrollIntoView() - .should('be.visible'); + verifyScrollDown() { + cy.contains('h1', 'Adopters') + .scrollIntoView() + .should('be.visible'); }
49-56: Fix inconsistent formatting.The method logic is correct but has formatting inconsistencies.
- verifySubmitPullRequestLink(){ - cy.contains('a','submit a pull request') - .should('be.visible') - .should('have.attr', - 'href', - 'https://github.com/asyncapi/website/blob/master/config/adopters.yml', - ); + verifySubmitPullRequestLink() { + cy.contains('a', 'submit a pull request') + .should('be.visible') + .should('have.attr', 'href', 'https://github.com/asyncapi/website/blob/master/config/adopters.yml'); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cypress/RoadmapPage.cy.js(1 hunks)cypress/casestudies.cy.js(1 hunks)cypress/fixtures/caseStudiesLinks.json(1 hunks)cypress/homepage.cy.js(1 hunks)cypress/pages/CaseStudiesPage.js(1 hunks)cypress/pages/homepage.js(2 hunks)cypress/pages/roadmap.js(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
cypress/homepage.cy.js (4)
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In JavaScript test files within the AsyncAPI website project, TypeScript file imports must include the .ts extension to avoid lint errors, even though the files being imported are JavaScript files.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/build-rss.test.js:25-27
Timestamp: 2024-11-01T09:55:20.531Z
Learning: In `tests/build-rss.test.js`, replacing `jest.resetModules()` with `jest.resetAllMocks()` in the `afterEach()` block causes errors. It is necessary to use `jest.resetModules()` to reset the module registry between tests in this file.
cypress/fixtures/caseStudiesLinks.json (4)
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/fixtures/rssData.js:1-57
Timestamp: 2024-11-01T13:32:15.472Z
Learning: In the `tests/fixtures/rssData.js` file of the `@asyncapi/website` project, tests for edge cases such as empty strings for title or excerpt, very long text content, international characters (UTF-8), or malformed URLs in `slug` or `cover` are not necessary because these cases will not occur.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
cypress/casestudies.cy.js (3)
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/fixtures/rssData.js:1-57
Timestamp: 2024-11-01T13:32:15.472Z
Learning: In the `tests/fixtures/rssData.js` file of the `@asyncapi/website` project, tests for edge cases such as empty strings for title or excerpt, very long text content, international characters (UTF-8), or malformed URLs in `slug` or `cover` are not necessary because these cases will not occur.
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
cypress/pages/roadmap.js (1)
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
cypress/RoadmapPage.cy.js (1)
Learnt from: vishvamsinh28
PR: asyncapi/website#3378
File: tests/markdown/check-markdown.test.js:133-138
Timestamp: 2024-11-29T17:36:09.783Z
Learning: When testing the 'main' function in 'check-markdown.test.js', it's acceptable to simply ensure it doesn't throw any errors, as the functions it calls are already tested elsewhere.
cypress/pages/CaseStudiesPage.js (1)
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
🧬 Code Graph Analysis (1)
cypress/pages/roadmap.js (1)
pages/roadmap.tsx (1)
RoadmapPage(485-696)
🔇 Additional comments (10)
cypress/homepage.cy.js (1)
1-1: Good consistency improvement.The import statement update aligns with proper PascalCase naming conventions for page object classes.
cypress/pages/homepage.js (1)
1-2: LGTM: Clean import additions.The new page object imports are correctly placed and follow proper naming conventions.
cypress/pages/roadmap.js (1)
1-13: LGTM: Well-structured page object methods.The class structure and first three methods (
visit,verifyHeader,verifyLink) follow Cypress best practices with appropriate selectors and assertions.cypress/casestudies.cy.js (3)
6-10: LGTM! Well-structured test setup.The beforeEach hook properly initializes the page objects and navigates to the target page before each test, following good testing practices.
39-43: LGTM! Proper fixture loading pattern.The fixture loading is correctly implemented using the
beforehook, ensuring the data is available for all tests in the describe block.
45-49: Fixture data structure verified
Thecypress/fixtures/caseStudiesLinks.jsonfile contains an array of objects with bothhrefandlabelkeys as expected. No changes are required.cypress/pages/CaseStudiesPage.js (4)
2-4: LGTM! Standard page object visit method.The visit method correctly navigates to the case studies page.
6-8: LGTM! Reusable element visibility verification.Good utility method for verifying element visibility that can be reused across different elements.
39-47: LGTM! Well-implemented FAQ link verification.The method properly verifies both visibility and the correct href attribute for the FAQ link.
58-61: LGTM! Effective use of method composition.Good approach to verify multiple card links by calling the reusable
verifyLinkExistsmethod.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4215--asyncapi-website.netlify.app/ |
…mepage navigation and comments
princerajpoot20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have a look at this: #4533 (review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cypress/pages/BaseDocsPage.js (1)
10-16: Minor inconsistent indentation in callback.The indentation within the
verifyLinkByHrefcallback is inconsistent (lines 14 and 16 use different spacing)..then($a => { const wrap = cy.wrap($a); return $a.prop('target') === '_self' ? wrap.invoke('removeAttr', 'target').click() - : wrap.click(); - }); + : wrap.click(); + });cypress/pages/ToolsPage.js (1)
17-21: Minor: "Github" should be "GitHub".The official brand name uses capital "H":
GitHub.const linkTexts = { website: 'Visit Website', - github: 'View Github', + github: 'View GitHub', docs: 'Visit Docs', };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cypress/pages/BaseDocsPage.js(2 hunks)cypress/pages/BaseFooterPage.js(2 hunks)cypress/pages/BaseHeaderPage.js(2 hunks)cypress/pages/BaseToolsPage.js(2 hunks)cypress/pages/DocsPage.js(2 hunks)cypress/pages/Footer.js(1 hunks)cypress/pages/ToolsPage.js(1 hunks)cypress/pages/header.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: anshgoyalevil
Repo: asyncapi/website PR: 3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
🔇 Additional comments (9)
cypress/pages/BaseDocsPage.js (1)
1-27: Class rename looks good.The rename from
BasePagetoBaseDocsPagecorrectly specializes the base class for documentation-related page objects, aligning with the broader refactor pattern across the suite.cypress/pages/BaseHeaderPage.js (1)
1-18: LGTM!The class rename to
BaseHeaderPageis consistent with the specialized base class pattern. The fluentreturn thisinvisit()enables method chaining, which is a good practice for page objects.cypress/pages/header.js (1)
1-3: LGTM!The
Headerclass correctly imports and extendsBaseHeaderPage, aligning with the base class refactor.cypress/pages/DocsPage.js (2)
1-3: LGTM!The
DocsPageclass correctly imports and extendsBaseDocsPage, consistent with the refactor pattern.
48-50: Verify label/method naming consistency.The method is named
goToMigrationsSection(plural) but uses the label'Migration'(singular). Confirm this matches the actual UI label text. If the UI uses "Migrations", update the label accordingly.cypress/pages/BaseFooterPage.js (1)
1-15: LGTM!The class rename to
BaseFooterPagecorrectly specializes the base class for footer-related page objects, consistent with the suite-wide refactor.cypress/pages/ToolsPage.js (1)
1-3: LGTM!The
ToolsPageclass correctly imports and extendsBaseToolsPage, aligning with the base class refactor.cypress/pages/BaseToolsPage.js (1)
1-17: LGTM – Class rename is consistent and complete.The class name and export have been correctly updated from
BasePagetoBaseToolsPage. All importing files reference the new name correctly—ToolsPage.jsimports and extendsBaseToolsPageas expected. No staleBasePagereferences remain in the codebase. The internal methods remain unchanged, making this a clean rename as part of the base class specialization refactoring.cypress/pages/Footer.js (1)
1-3: Correct import refactoring to specialized base class.The import and class extension correctly use
BaseFooterPageinstead of a generic base class. This specialization improves code organization and maintains consistency with the framework's page hierarchy (BaseDocsPage, BaseHeaderPage, BaseToolsPage).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cypress/casestudies.cy.js (1)
3-7: Use camelCase for variable naming.The variable
casestudiespageshould follow JavaScript camelCase convention:caseStudiesPage.Apply this diff:
-let casestudiespage; +let caseStudiesPage; beforeEach(() => { - casestudiespage = new CaseStudiesPage(); - casestudiespage.visit(); + caseStudiesPage = new CaseStudiesPage(); + caseStudiesPage.visit(); });Update all references throughout the file (lines 12-15, 25, 40).
🧹 Nitpick comments (2)
cypress/casestudies.cy.js (1)
38-42: Consider using the verifyLinks helper for consistency.The forEach loop here could leverage the new
verifyLinkshelper fromcypress/support/helpers.jsfor consistency with the pattern used infooter.cy.jsandtoolspage.cy.js.Example refactor:
+import { verifyLinks } from '../support/helpers'; + describe('Links under Resources Section', () => { let links; before(() => { cy.fixture('caseStudiesLinks').then((data) => { links = data; }); }); it('Verifies all Links under Resources work', () => { - links.forEach(({ href, label }) => { - caseStudiesPage.verifyLinksWork(href, label); - }); + verifyLinks(links, (href, label) => caseStudiesPage.verifyLinksWork(href, label), 'href', 'label'); }); });cypress/support/helpers.js (1)
1-14: LGTM! Well-documented helper function.The
verifyLinkshelper provides a clean abstraction for link verification with flexible key naming via default parameters. The JSDoc is clear and comprehensive.Note: This helper could be applied in
footer.cy.js(lines 17-22) andtoolspage.cy.js(lines 22-25) to further reduce repetitiveforEachusage and improve consistency across test files.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cypress/RoadmapPage.cy.js(1 hunks)cypress/casestudies.cy.js(1 hunks)cypress/docspage.cy.js(1 hunks)cypress/footer.cy.js(1 hunks)cypress/support/e2e.js(1 hunks)cypress/support/helpers.js(1 hunks)cypress/toolspage.cy.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cypress/docspage.cy.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-19T04:51:41.255Z
Learnt from: anshgoyalevil
Repo: asyncapi/website PR: 3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Applied to files:
cypress/toolspage.cy.jscypress/footer.cy.js
📚 Learning: 2024-11-01T13:32:15.472Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3101
File: tests/fixtures/rssData.js:1-57
Timestamp: 2024-11-01T13:32:15.472Z
Learning: In the `tests/fixtures/rssData.js` file of the `asyncapi/website` project, tests for edge cases such as empty strings for title or excerpt, very long text content, international characters (UTF-8), or malformed URLs in `slug` or `cover` are not necessary because these cases will not occur.
Applied to files:
cypress/toolspage.cy.js
🧬 Code graph analysis (2)
cypress/footer.cy.js (1)
components/footer/FooterData.ts (3)
initiativeLinks(19-40)socialMediaData(42-67)newsLinks(69-74)
cypress/RoadmapPage.cy.js (1)
pages/roadmap.tsx (1)
RoadmapPage(485-696)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cypress-run
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (6)
cypress/casestudies.cy.js (1)
10-21: LGTM! Clean data-driven test structure.The use of a
verificationsarray withforEachto generate tests is an excellent pattern that improves maintainability and reduces code duplication.cypress/RoadmapPage.cy.js (2)
1-17: LGTM! Clean test structure.The test setup and basic verification tests are well-structured. Navigation is properly handled in
beforeEach, and the verification methods are appropriately invoked.
19-29: LGTM! Excellent data-driven tooltip testing.The tooltip verification tests effectively use a data-driven approach to test all three legend tooltips (Outcome, Solution, Implementation). The indices align correctly with the roadmap legend structure.
cypress/support/e2e.js (1)
18-18: LGTM! Appropriate global helper import.Adding the helpers import ensures the
verifyLinksutility is available across all test files, which supports the data-driven testing pattern used throughout this PR.cypress/footer.cy.js (1)
11-23: LGTM! Excellent refactoring to data-driven tests.The consolidation of separate link verification tests into a single data-driven structure significantly improves maintainability and reduces code duplication. The pattern is consistent with other refactored test files in this PR.
cypress/toolspage.cy.js (1)
16-26: LGTM! Well-structured data-driven refactor.The consolidation into a
linkTypesarray with dynamic test generation is excellent. The defensive null handling with(links || [])on line 24 ensures robustness when link data is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cypress/support/helpers.js (1)
1-5: Nice reusable helper! Consider adding input validation.The function is clean and follows a good data-driven testing pattern. However, adding basic input validation would make test failures easier to debug when the helper is called with unexpected data.
Apply this diff to add input validation:
export function verifyLinks(linksArray, verifyFn, urlKey = 'url', labelKey = 'label') { + if (!Array.isArray(linksArray)) { + throw new Error('verifyLinks: linksArray must be an array'); + } + if (typeof verifyFn !== 'function') { + throw new Error('verifyLinks: verifyFn must be a function'); + } linksArray.forEach((link) => { + if (!link || typeof link !== 'object') { + throw new Error(`verifyLinks: invalid link object at index ${linksArray.indexOf(link)}`); + } + if (!(urlKey in link) || !(labelKey in link)) { + throw new Error(`verifyLinks: link object missing required keys '${urlKey}' or '${labelKey}'`); + } verifyFn(link[urlKey], link[labelKey]); }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/support/helpers.js(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: cypress-run
… for the Case Studies page.
Description
I have added E2E tests using Cypress for Roadmap Page and Case Studies Page to ensure functionality and correct rendering.
Related issue(s)
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.