test: add e2e tests for roadmap and case studies page#4215
test: add e2e tests for roadmap and case studies page#4215anushkaaaaaaaa wants to merge 58 commits intoasyncapi:masterfrom
Conversation
anushkaaaaaaaa
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces new E2E tests for RoadMap and Case Studies pages, refactors base page classes with semantic naming (BasePage → specialized classes), extends page object verification methods, migrates fixture data from cypress/fixtures to cypress/data directory, and removes global Cypress command setup from support files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 #4215 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 830 830
Branches 159 159
=========================================
Hits 830 830 ☔ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
anshgoyalevil
added
the
gsoc
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4215--asyncapi-website.netlify.app/ |
…mepage navigation and comments
config/tools.json
Outdated
| @@ -1556,7 +1556,7 @@ | |||
| "borderColor": "border-[#CA1A33]" | |||
| }, | |||
| { | |||
| "name": "Liquid", | |||
| "name": "AsyncAPI CLI", | |||
There was a problem hiding this comment.
@anushkaaaaaaaa what is the reason behind these changes?
Signed-off-by: Anushka Sharan <anushkasharan05@gmail.com>
anushkaaaaaaaa
force-pushed
the
roadcase
branch
from
0f99b44 to
b72779c
Compare
princerajpoot20
left a comment
princerajpoot20
left a comment
There was a problem hiding this comment.
@anushkaaaaaaaa good work. left some review comments, can you please have a look
cypress/pages/DocsPage.js
Outdated
| class DocsPage extends BasePage { | ||
| class DocsPage extends BaseDocsPage { | ||
| visitDocsPage() { | ||
| cy.visit('/docs'); |
There was a problem hiding this comment.
consider using 'super' here
| @@ -0,0 +1,82 @@ | |||
| [ | |||
There was a problem hiding this comment.
can we rename the folder name 'fixtures' . currently it does not look good for this file we are storing
| verifyResourceLink(href) { | ||
| cy.get(`a[href="${href}"]`) | ||
| .first() | ||
| .should('exist') | ||
| .should('have.attr', 'href', href); | ||
| } | ||
|
|
There was a problem hiding this comment.
if i am not wrong, we have a generic method to verify any kind of links. can we use that
cypress/pages/CaseStudiesPage.js
Outdated
| } | ||
|
|
||
| verifyFaqLink() { | ||
| this.verifyButtonLink( |
There was a problem hiding this comment.
this verifyButtonLink are the base class methods, hence use super for clarity
cypress/pages/CaseStudiesPage.js
Outdated
| } | ||
|
|
||
| verifySubmitPullRequestLink() { | ||
| this.verifyButtonLink( |
cypress/pages/RoadmapPage.js
Outdated
| } | ||
|
|
||
| verifyCommunityLink() { | ||
| this.verifyElementHasAttribute( |
There was a problem hiding this comment.
please verify all the method calls you are using. if it is a base class method then it should be called using 'super'
cypress/support/e2e.js
Outdated
| @@ -15,3 +15,4 @@ | |||
|
|
|||
| // Import commands.js using ES2015 syntax: | |||
cypress/support/e2e.js
Outdated
| @@ -15,3 +15,4 @@ | |||
|
|
|||
| // Import commands.js using ES2015 syntax: | |||
| import './commands' | |||
There was a problem hiding this comment.
i did not get this..why we need this import commands?
cypress/support/helpers.js
Outdated
| export function verifyLinks(linksArray, verifyFn, urlKey = 'url', labelKey = 'label') { | ||
| linksArray.forEach((link) => { | ||
| verifyFn(link[urlKey], link[labelKey]); | ||
| }); | ||
| } |
There was a problem hiding this comment.
this is just an array traversal. we are calling a method multiple times. hence, i would say we don't have to extract it as a helper function, as this is not something we are going to use again and again in our case. hence, we can avoid this complexity.
cypress/casestudies.cy.js
Outdated
| }); | ||
|
|
||
| it('Verifies all Links under Resources work', () => { | ||
| verifyLinks(links, (href, label) => casestudiespage.verifyResourceLink(href), 'href', 'label'); |
There was a problem hiding this comment.
in verifyResourceLink function, we have .first() in the function. so it will get us only a single dom link, then this array traversal does not make sense as it is always going to have a single entry. or am i missing something here.
|
also, e2e tests are failing. please have a look at that as well |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cypress/pages/RoadmapPage.js (1)
20-23:⚠️ Potential issue | 🟡 MinorScope tooltip visibility assertion to the hovered index.
Line 22 uses
.first(), soverifyTooltip(index)can assert the wrong tooltip whenindexis not0.Suggested fix
verifyTooltip(index) { cy.get('[data-testid="InlineHelp-icon"]').eq(index).trigger('mouseover'); - cy.get('[data-testid="InlineHelp"]').first().should('be.visible'); + cy.get('[data-testid="InlineHelp"]').eq(index).should('be.visible'); }#!/bin/bash # Verify tooltip assertion target in RoadmapPage rg -n -A3 -B1 'verifyTooltip\(index\)|InlineHelp' cypress/pages/RoadmapPage.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/pages/RoadmapPage.js` around lines 20 - 23, The tooltip assertion in verifyTooltip(index) is using .first() and can check the wrong element for non-zero indexes; update the second selector so it targets the tooltip at the same index as the hovered icon (e.g., replace .first() with .eq(index)) in the verifyTooltip function to ensure the hovered InlineHelp-icon and the asserted [data-testid="InlineHelp"] use the same index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cypress/pages/RoadmapPage.js`:
- Around line 20-23: The tooltip assertion in verifyTooltip(index) is using
.first() and can check the wrong element for non-zero indexes; update the second
selector so it targets the tooltip at the same index as the hovered icon (e.g.,
replace .first() with .eq(index)) in the verifyTooltip function to ensure the
hovered InlineHelp-icon and the asserted [data-testid="InlineHelp"] use the same
index.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
cypress.config.jscypress/casestudies.cy.jscypress/data/caseStudiesLinks.jsoncypress/data/docsSections.jsoncypress/data/example.jsoncypress/data/footerPageData.jsoncypress/data/homePageData.jsoncypress/data/toolsData.jsoncypress/data/toolsPages.jsoncypress/docspage.cy.jscypress/homepage.cy.jscypress/pages/CaseStudiesPage.jscypress/pages/DocsPage.jscypress/pages/RoadmapPage.jscypress/pages/toolsGenerator.jscypress/pages/toolsModelina.jscypress/support/e2e.jscypress/tools_cli.cy.jscypress/tools_generator.cy.jscypress/tools_github_actions.cy.jscypress/tools_modelina.cy.jscypress/tools_parsers.cy.js
💤 Files with no reviewable changes (1)
- cypress/support/e2e.js
✅ Files skipped from review due to trivial changes (1)
- cypress/data/caseStudiesLinks.json
🚧 Files skipped from review as they are similar to previous changes (2)
- cypress/casestudies.cy.js
- cypress/pages/CaseStudiesPage.js
|
@princerajpoot20 i have made the required changes and also fixed the e2e tests please review once, thankyou |
anshgoyalevil
left a comment
anshgoyalevil
left a comment
There was a problem hiding this comment.
Approving this PR. Please handle any required changes (if any) in follow up PRs.
|
@anushkaaaaaaaa can you please resolve the merge conflicts |
anushkaaaaaaaa
force-pushed
the
roadcase
branch
from
40e4b3c to
558ade6
Compare
|
@princerajpoot20 done, please review |
|
@anushkaaaaaaaa I can only see a very minor changes here: https://github.com/asyncapi/website/pull/4215/changes Please have a look again. I don't have much context of this PR but it seems that a major chunk of the code was removed when you force-pushed this branch/PR. If you have the local changes on your machine, you can re-add them via a new commit. |
|
@anushkaaaaaaaa please have a look at the PR. It is currently a blank PR (no line changes). Thanks @animeshk923 for your help with the review. |
anushkaaaaaaaa
force-pushed
the
roadcase
branch
from
558ade6 to
40e4b3c
Compare
|
Happy to help! :) |
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
Release Notes