test: add e2e tests for dashboard and events pages #4548
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. |
asyncapi-bot
requested review from
CBID2,
bandantonio,
devilkiller-ag and
vishvamsinh28
WalkthroughAdds four new Cypress files: two end-to-end test suites ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Cypress Test
participant P as Page Object
participant B as Browser/App
rect rgb(235,248,255)
Note right of T: beforeEach -> visit page
T->>P: visit()
P->>B: navigate to URL
end
rect rgb(245,255,235)
Note right of T: run verifications
T->>P: verifyHeader()/verifySections()/verifyLinks()
P->>B: query elements & read hrefs
P-->>T: assertions (visible/text/href)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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 #4548 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 780 780
Branches 144 144
=========================================
Hits 780 780 ☔ 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-4548--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
cypress/pages/events.js (3)
38-55: Remove unused method or add corresponding tests.This method is not called anywhere in the test suite (
cypress/events.cy.js). Consider removing it to reduce maintenance burden, or if it's intended for future use, add corresponding tests.
57-62: Remove unused method or add corresponding tests.This method is not referenced in the test suite. Consider removing it or adding tests that exercise this functionality.
64-113: Eliminate code duplication across event card verification methods.The methods
verifyAllEventCards,verifyUpcomingEventCards, andverifyRecordedEventCardshave identical implementations. This violates the DRY principle and increases maintenance burden.Consolidate into a single method:
- verifyAllEventCards(count) { + verifyEventCards(count) { cy.get('[data-testid="EventPostItem-main"]') .should('have.length.at.least', count) .each(($card, index) => { if (index < count) { cy.wrap($card) .find('a[data-testid="EventPostItem-link"]') .should('have.attr', 'href') .and('match', /github\.com\/asyncapi\/community\/issues\/\d+/); } }); } + + verifyAllEventCards(count) { + return this.verifyEventCards(count); + } + + verifyUpcomingEventCards(count) { + return this.verifyEventCards(count); + } + + verifyRecordedEventCards(count) { + return this.verifyEventCards(count); + } - - verifyUpcomingEventCards(count) { - cy.get('[data-testid="EventPostItem-main"]') - .should('have.length.at.least', count) - .each(($card, index) => { - if (index < count) { - cy.wrap($card) - .find('a[data-testid="EventPostItem-link"]') - .should('have.attr', 'href') - .and('match', /github\.com\/asyncapi\/community\/issues\/\d+/); - } - }); - } - - verifyRecordedEventCards(count) { - cy.get('[data-testid="EventPostItem-main"]') - .should('have.length.at.least', count) - .each(($card, index) => { - if (index < count) { - cy.wrap($card) - .find('a[data-testid="EventPostItem-link"]') - .should('have.attr', 'href') - .and('match', /github\.com\/asyncapi\/community\/issues\/\d+/); - } - }); - }cypress/pages/dashboard.js (2)
26-28: Remove unused method or add corresponding tests.The
verifyElementHasAttributemethod is not referenced in the test suite (cypress/dashboard.cy.js). Consider removing it to reduce code clutter.
6-8: Consider extracting common helper to a base class.The
verifyElementIsVisiblemethod is duplicated in bothDashboardPageandEventsPagewith identical implementations. For better maintainability, consider creating a baseBasePageclass that both page objects can extend.Example structure:
In
cypress/pages/base.js:class BasePage { verifyElementIsVisible(selector) { cy.get(selector).should('be.visible'); } } export default BasePage;Then update both page objects:
import BasePage from './base'; class DashboardPage extends BasePage { // existing methods }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cypress/dashboard.cy.js(1 hunks)cypress/events.cy.js(1 hunks)cypress/pages/dashboard.js(1 hunks)cypress/pages/events.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). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (5)
cypress/pages/events.js (2)
29-36: Verify ifforce: trueis necessary for the click action.Using
{ force: true }bypasses Cypress's actionability checks. This should only be used when the element is intentionally obscured or when there's a valid reason it can't be interacted with naturally.Consider removing the
force: trueoption and verifying if the click works without it:switchToFilter(label) { cy.get('[data-testid="EventFilters-main"]') .contains( '[data-testid="EventFilter-click"]', new RegExp(`^${label}$`, 'i'), ) - .click({ force: true }); + .click(); }If removing it causes test failures, investigate whether there's a UI issue (e.g., overlapping elements) that should be addressed in the application code rather than bypassed in tests.
1-161: Well-structured page object implementation.The EventsPage class follows the page object pattern effectively, with clear method names and good use of
data-testidattributes for stable selectors. The overall structure supports maintainable tests.cypress/events.cy.js (1)
1-53: Well-organized test suite with comprehensive coverage.The test suite is well-structured with clear, user-focused test descriptions. The use of
beforeEachfor page navigation and the separation of concerns across individual test cases makes the suite maintainable and easy to understand.cypress/pages/dashboard.js (1)
1-62: Clean page object implementation.The DashboardPage class is well-structured with clear, descriptive method names and appropriate use of helper methods. The page object pattern is correctly applied.
cypress/dashboard.cy.js (1)
1-25: Clean and focused test suite.The test suite is well-organized with clear test cases that each verify a specific aspect of the dashboard page. The structure promotes maintainability and readability.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cypress/pages/events.js (1)
115-127: Consider using descriptive selectors instead of hard-coded indices.Using
.eq(0)and.eq(1)makes tests fragile if button order changes. Consider adding more specific data-testid attributes or using text-based selection for better maintainability.Example refactor using more descriptive approach:
verifyEventButtonsLinks() { - cy.get('[data-testid="Events-Button"] a[data-testid="Button-link"]') - .eq(0) + cy.contains('[data-testid="Events-Button"] a[data-testid="Button-link"]', /add to google calendar/i) .should('be.visible') .and('have.attr', 'href') .and('include', 'calendar.google.com'); - cy.get('[data-testid="Events-Button"] a[data-testid="Button-link"]') - .eq(1) + cy.contains('[data-testid="Events-Button"] a[data-testid="Button-link"]', /download ics/i) .should('be.visible') .and('have.attr', 'href') .and('include', 'calendar.google.com/calendar/ical'); }cypress/pages/dashboard.js (2)
6-12: Consider extracting common helpers to a base page class.Both
DashboardPageandEventsPageimplement identicalverifyElementIsVisible()methods. Extracting common helpers to a base class would reduce duplication and improve maintainability.Example structure:
// cypress/pages/BasePage.js class BasePage { verifyElementIsVisible(selector) { cy.get(selector).should('be.visible'); } verifyTextVisible(text) { cy.contains(text).should('be.visible'); } } export default BasePage;Then update page objects:
// cypress/pages/dashboard.js import BasePage from './BasePage'; class DashboardPage extends BasePage { visit() { cy.visit('/community/dashboard'); } // ... rest of the methods }
26-28: I need to expand the search to check if this method might be referenced outside the cypress directory.Remove unused helper method
verifyElementHasAttribute.The method is defined in
cypress/pages/dashboard.js(lines 26–28) but never called within the codebase. Removing it aligns with Page Object Model best practices and reduces code maintenance overhead.- verifyElementHasAttribute(selector, attribute, value) { - cy.get(selector).should('have.attr', attribute, value); - } - verifyLinkWithText(selector, text, expectedHrefPart) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cypress/dashboard.cy.js(1 hunks)cypress/events.cy.js(1 hunks)cypress/pages/dashboard.js(1 hunks)cypress/pages/events.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). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (3)
cypress/dashboard.cy.js (1)
1-25: LGTM! Well-structured test suite.The test suite follows Cypress best practices with clear test separation, appropriate use of
beforeEach, and proper Page Object Model implementation.cypress/events.cy.js (1)
1-53: LGTM! Comprehensive test coverage.The test suite provides thorough coverage of the Events page functionality, including filtering, link validation, and section verification. Tests are well-organized and follow Cypress best practices.
cypress/pages/events.js (1)
29-36: Removeforce: truefrom the click action and verify tests pass.The
force: trueis the only instance of forced clicking in the entire cypress test suite, with no explanatory comments. The codebase consistently uses standard.click()without force and prioritizes visibility assertions via.should('be.visible'). Cypress best practices recommend avoidingforce: trueunless there's a legitimate reason (complex interaction patterns, element coverage, etc.), which is not evident here. Try removingforce: trueat line 35 and run the tests to confirm they pass—if they do, the force option was unnecessary and masks real actionability issues.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cypress/pages/events.js (1)
101-113: Consider extracting button link verification to reduce duplication.The two button link verifications follow the same pattern. While acceptable, extracting a helper would improve maintainability.
Apply this diff to reduce duplication:
+ verifyButtonLinkByIndex(index, expectedUrlPart) { + cy.get('[data-testid="Events-Button"] a[data-testid="Button-link"]') + .eq(index) + .should('be.visible') + .and('have.attr', 'href') + .and('include', expectedUrlPart); + } + verifyEventButtonsLinks() { - cy.get('[data-testid="Events-Button"] a[data-testid="Button-link"]') - .eq(0) - .should('be.visible') - .and('have.attr', 'href') - .and('include', 'calendar.google.com'); - - cy.get('[data-testid="Events-Button"] a[data-testid="Button-link"]') - .eq(1) - .should('be.visible') - .and('have.attr', 'href') - .and('include', 'calendar.google.com/calendar/ical'); + this.verifyButtonLinkByIndex(0, 'calendar.google.com'); + this.verifyButtonLinkByIndex(1, 'calendar.google.com/calendar/ical'); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/pages/events.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). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (7)
cypress/pages/events.js (7)
1-8: LGTM!The class structure and helper method are well-designed. The
verifyElementIsVisiblehelper promotes code reuse across visibility checks.
10-27: LGTM!The visibility verification methods consistently use the helper and follow good practices with descriptive test IDs.
64-99: LGTM! Past review comment successfully addressed.The duplication issue from the previous review has been resolved. The three verification methods (
verifyAllEventCards,verifyUpcomingEventCards,verifyRecordedEventCards) now properly delegate to the sharedverifyEventCardshelper, eliminating code duplication while maintaining clear, semantic method names for test readability.
38-62: LGTM!The event card link verification methods handle both title-based and index-based lookups correctly. The conditional logic in
verifyEventCardLinkByTitleAndHrefappropriately handles scenarios with and withoutmetaText.
115-126: LGTM!The FAQ and recordings link verification methods correctly validate both visibility and expected href patterns.
128-136: LGTM!The event recording links validation properly checks for YouTube URLs using an appropriate regex pattern.
147-147: LGTM!Standard ES6 default export.
…nd adjust Upcoming/Recorded tab checks
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cypress/pages/events.js (1)
146-151: Strengthen host link assertions.Reiterating earlier feedback: only checking that
hrefexists won’t catch empty or malformed links. Please assert that the value is present and well-formed so the test guards against regressions.verifyHostLinks() { cy.get('a[data-testid="TextLink-href"]') .should('have.length.greaterThan', 0) .each(($link) => { - cy.wrap($link).should('have.attr', 'href'); + cy.wrap($link) + .should('have.attr', 'href') + .and('not.be.empty') + .and('match', /^https?:\/\//); }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cypress/events.cy.js(1 hunks)cypress/pages/events.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). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
2 participants
Description
Related issue(s)
Summary by CodeRabbit