test: add e2e tests for pages under tools dropdown#5061
test: add e2e tests for pages under tools dropdown#5061anushkaaaaaaaa wants to merge 2 commits intoasyncapi:masterfrom
Conversation
Signed-off-by: Anushka Sharan <anushkasharan05@gmail.com>
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis pull request refactors the Cypress test infrastructure by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 #5061 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5061--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cypress/docspage.cy.js (1)
32-47:⚠️ Potential issue | 🔴 CriticalCritical: Nested
it()blocks are invalid in Mocha/Cypress.The
it('verifying Card Links present on the Docs Page')test at line 32 is nested inside theit('verifying navigation to SideBar')test starting at line 15. Mocha does not support nestedit()blocks—this inner test will be silently skipped and never executed.The inner test should be moved outside to be a sibling test case.
🐛 Proposed fix to extract the nested test
docsPage.goToMigrationsSection(); docsPage.goToCommunitySection(); + }); - it('verifying Card Links present on the Docs Page', () => { - const cards = [ - { href: '/docs/concepts' }, - { href: '/docs/tutorials' }, - { href: '/docs/guides' }, - { href: '/docs/tools' }, - { href: '/docs/reference' }, - { href: '/docs/migration' }, - { href: '/docs/community' }, - { href: '/docs/reference/specification/v3.0.0-explorer' }, - ]; + it('verifying Card Links present on the Docs Page', () => { + const cards = [ + { href: '/docs/concepts' }, + { href: '/docs/tutorials' }, + { href: '/docs/guides' }, + { href: '/docs/tools' }, + { href: '/docs/reference' }, + { href: '/docs/migration' }, + { href: '/docs/community' }, + { href: '/docs/reference/specification/v3.0.0-explorer' }, + ]; - cards.forEach((card) => { - docsPage.verifyCardLinks(card.href); - }); + cards.forEach((card) => { + docsPage.verifyCardLinks(card.href); }); - }); + }); });cypress/pages/BaseFooterPage.js (1)
1-14:⚠️ Potential issue | 🟠 MajorNaming conflict: This file exports
BasePagebut a newBasePageclass exists inBasePage.js.This file (
BaseFooterPage.js) exports a class namedBasePage, but there's now a separatecypress/pages/BasePage.jsthat also exportsBasePage. This creates ambiguity:
- Imports like
import BasePage from './BasePage'will resolve to the new file- This file's class name doesn't match its filename (
BaseFooterPage.jsvsclass BasePage)Consider renaming this class to
BaseFooterPageto match the filename and avoid confusion with the newBasePage.Proposed fix
-class BasePage { +class BaseFooterPage { visit() { cy.visit('/'); } // ... } -export default BasePage; +export default BaseFooterPage;
🤖 Fix all issues with AI agents
In `@cypress/fixtures/toolsPages.json`:
- Line 6: Update the GitHub URLs in the fixtures so they use the canonical
hostname without the www prefix: replace occurrences like the "github" value
"https://www.github.com/asyncapi/generator" with
"https://github.com/asyncapi/generator" and apply the same replacement to the
other "github" entries in this JSON fixture (the other values that currently use
"https://www.github.com/..."). Ensure all "github" keys in the JSON use URLs
starting with "https://github.com/".
🧹 Nitpick comments (2)
cypress/tools_generator.cy.js (1)
1-5: Inconsistent class naming convention.The class
toolsGeneratoruses camelCase, which is inconsistent with JavaScript class naming conventions and withToolsModelinaused intools_modelina.cy.js. Classes should use PascalCase.♻️ Proposed fix for naming consistency
-import toolsGenerator from './pages/toolsGenerator'; +import ToolsGenerator from './pages/toolsGenerator'; import toolsData from './fixtures/toolsPages.json'; describe('Tools - Generator Page', () => { - const page = new toolsGenerator(); + const page = new ToolsGenerator();Note: This also requires renaming the export in
cypress/pages/toolsGenerator.jstoToolsGenerator.cypress/pages/homepage.js (1)
66-68: Minor inconsistency: consider using inherited helper for consistency.Other methods in this class use
this.verifyElementIsVisible()(lines 54, 63), but this method usescy.get().should()directly. For consistency with the refactor pattern:♻️ Optional: Use inherited helper
verifyReadTheDocsButton(link = readDocsLink) { - return cy.get(`[data-testid="Button-link"][href="${link}"]`).should('be.visible'); + return this.verifyElementIsVisible(`[data-testid="Button-link"][href="${link}"]`); }
| "path": "/tools/generator", | ||
| "heading": "Docs, Code, Anything!", | ||
| "image": "generator diagram", | ||
| "github": "https://www.github.com/asyncapi/generator", |
There was a problem hiding this comment.
Consider using canonical GitHub URLs without www.
The GitHub URLs use www.github.com (e.g., https://www.github.com/asyncapi/generator), but the canonical GitHub URL format is https://github.com/... without the www prefix. If the actual links on the pages use the canonical format, these assertions may fail.
Proposed fix
- "github": "https://www.github.com/asyncapi/generator",
+ "github": "https://github.com/asyncapi/generator",Apply similar changes to lines 13, 20, 25, and 35.
📝 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.
| "github": "https://www.github.com/asyncapi/generator", | |
| "github": "https://github.com/asyncapi/generator", |
🤖 Prompt for AI Agents
In `@cypress/fixtures/toolsPages.json` at line 6, Update the GitHub URLs in the
fixtures so they use the canonical hostname without the www prefix: replace
occurrences like the "github" value "https://www.github.com/asyncapi/generator"
with "https://github.com/asyncapi/generator" and apply the same replacement to
the other "github" entries in this JSON fixture (the other values that currently
use "https://www.github.com/..."). Ensure all "github" keys in the JSON use URLs
starting with "https://github.com/".
Description
Related issue(s)
Summary by CodeRabbit