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,
ashmitjsg,
bandantonio 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 798 798
Branches 146 146
=========================================
Hits 798 798 ☔ 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.
| verifyHostLinks() { | ||
| cy.get('a[data-testid="TextLink-href"]') | ||
| .should('have.length.greaterThan', 0) | ||
| .each(($link) => { | ||
| cy.wrap($link).should('have.attr', 'href'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Strengthen host link validation.
The method only verifies that the href attribute exists but doesn't validate its content. This weak assertion could allow empty or malformed URLs to pass.
Apply this diff to add meaningful validation:
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?:\/\/.+/);
});
}This ensures hrefs are non-empty and follow a valid URL pattern.
📝 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.
| verifyHostLinks() { | |
| cy.get('a[data-testid="TextLink-href"]') | |
| .should('have.length.greaterThan', 0) | |
| .each(($link) => { | |
| cy.wrap($link).should('have.attr', 'href'); | |
| }); | |
| } | |
| verifyHostLinks() { | |
| cy.get('a[data-testid="TextLink-href"]') | |
| .should('have.length.greaterThan', 0) | |
| .each(($link) => { | |
| cy.wrap($link) | |
| .should('have.attr', 'href') | |
| .and('not.be.empty') | |
| .and('match', /^https?:\/\/.+/); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In cypress/pages/events.js around lines 138 to 144, the verifyHostLinks() helper
only checks that an href attribute exists but not its content; update it to
assert href is non-empty and matches a URL pattern by fetching each link's href
(cy.wrap($link).invoke('attr', 'href')) and then asserting it is truthy/length>0
and matches a reasonable regex (e.g., starts with http:// or https:// or a
protocol-relative or relative path pattern depending on app requirements);
ensure assertions use Cypress should with match or include to fail on empty or
malformed values.
There was a problem hiding this comment.
@anushkaaaaaaaa What is the reason behind making it a resolve? For me, it makes sense to have a stronger assertion.
There was a problem hiding this comment.
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!
…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
| switchToFilter(label) { | ||
| cy.get('[data-testid="EventFilters-main"]') | ||
| .contains( | ||
| '[data-testid="EventFilter-click"]', | ||
| new RegExp(`^${label}$`, 'i'), | ||
| ) | ||
| .click(); | ||
| cy.wait(500); | ||
| } |
There was a problem hiding this comment.
Replace arbitrary wait with deterministic synchronization.
Using cy.wait(500) hides timing bugs and flakes whenever the DOM takes longer than half a second (or faster, if debounce finishes earlier). Please wait on a real condition—e.g., chain a .should(...) that asserts the clicked filter’s active state or wait on the network request powering the tab—so Cypress can retry automatically and the test remains stable.
switchToFilter(label) {
cy.get('[data-testid="EventFilters-main"]')
.contains(
'[data-testid="EventFilter-click"]',
new RegExp(`^${label}$`, 'i'),
)
- .click();
- cy.wait(500);
+ .click()
+ .should('have.attr', 'aria-pressed', 'true'); // adjust to the actual active-state indicator
}📝 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.
| switchToFilter(label) { | |
| cy.get('[data-testid="EventFilters-main"]') | |
| .contains( | |
| '[data-testid="EventFilter-click"]', | |
| new RegExp(`^${label}$`, 'i'), | |
| ) | |
| .click(); | |
| cy.wait(500); | |
| } | |
| switchToFilter(label) { | |
| cy.get('[data-testid="EventFilters-main"]') | |
| .contains( | |
| '[data-testid="EventFilter-click"]', | |
| new RegExp(`^${label}$`, 'i'), | |
| ) | |
| .click() | |
| .should('have.attr', 'aria-pressed', 'true'); // adjust to the actual active-state indicator | |
| } |
🤖 Prompt for AI Agents
In cypress/pages/events.js around lines 29 to 37, replace the hard-coded
cy.wait(500) with deterministic synchronization: after clicking the filter,
chain a .should(...) that asserts the clicked filter entered its active state
(for example check the active CSS class or an aria attribute on the filter
element) OR wait on the network request that loads the filtered data by adding a
cy.intercept(...) and cy.wait('@alias') before proceeding; ensure the assertion
or network wait is chained to the existing get/contains/click flow so Cypress
can retry until the condition is met.
There was a problem hiding this comment.
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!
princerajpoot20
left a comment
princerajpoot20
left a comment
There was a problem hiding this comment.
@anushkaaaaaaaa Please have a look at the review comments and CodeRabbit suggestions
| dashboard.verifyHotTopicsSection(); | ||
| }); | ||
|
|
||
| it('User verifies Good First Issues section', () => { |
There was a problem hiding this comment.
Can you please improve the naming of all these tests? You can refer to the test naming conventions. For example, test names should not start with phrases like user verifies.
The naming should follow a pattern like: should display the Good First Issues section
| events.verifyFaqLink(); | ||
| }); | ||
|
|
||
| it('User verifies all host profile links', () => { |
There was a problem hiding this comment.
As mentioned above, please review the naming of all these tests
| verifyHostLinks() { | ||
| cy.get('a[data-testid="TextLink-href"]') | ||
| .should('have.length.greaterThan', 0) | ||
| .each(($link) => { | ||
| cy.wrap($link).should('have.attr', 'href'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
@anushkaaaaaaaa What is the reason behind making it a resolve? For me, it makes sense to have a stronger assertion.
| switchToFilter(label) { | ||
| cy.get('[data-testid="EventFilters-main"]') | ||
| .contains( | ||
| '[data-testid="EventFilter-click"]', | ||
| new RegExp(`^${label}$`, 'i'), | ||
| ) | ||
| .click(); | ||
| cy.wait(500); | ||
| } |
| @@ -0,0 +1,25 @@ | |||
| import DashboardPage from './pages/dashboard'; | |||
|
|
|||
| describe('Dashboard Page Tests', () => { | |||
There was a problem hiding this comment.
Please pay attention to the naming. It is really important in order to have maintainable code.
It is a test file. Do you think we should have another tests appended in the naming? It should be simply like: Dashboard Page
| @@ -0,0 +1,53 @@ | |||
| import EventsPage from './pages/events'; | |||
|
|
|||
| describe('Events Page Tests', () => { | |||
There was a problem hiding this comment.
As mentioned above, please review all these namings
| @@ -0,0 +1,62 @@ | |||
| class DashboardPage { | |||
| visit() { | |||
| cy.visit('/community/dashboard'); | |||
There was a problem hiding this comment.
These visit() methods are everywhere. They are repeating everywhere. Consider using a single helper like visit(path). This reduces repetition and makes your code DRY.
Please review all the methods in all the tests you have added vis multiple PRs. If they can be extract out to shared functions which makes it re-usable and keeps the code DRY
|
FYI: #4533 (review) |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive end-to-end test coverage for the Dashboard and Events pages using Cypress, implementing the page object model pattern for better test maintainability and reusability.
Key changes:
- Created page object classes encapsulating reusable interactions for Dashboard and Events pages
- Added 14 test cases covering UI elements, navigation, links, and filtering functionality
- Followed existing Cypress test patterns in the codebase with data-testid selectors
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| cypress/pages/events.js | Page object implementing methods to interact with and verify the Events page, including filters, event cards, recordings, and various links |
| cypress/pages/dashboard.js | Page object implementing methods to interact with and verify the Dashboard page header, sections, and navigation links |
| cypress/events.cy.js | Test suite with 9 test cases covering Events page functionality including filters, recordings, event types, and link validation |
| cypress/dashboard.cy.js | Test suite with 4 test cases verifying Dashboard page header, sections, and link functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| verifyElementHasAttribute(selector, attribute, value) { | ||
| cy.get(selector).should('have.attr', attribute, value); | ||
| } | ||
|
|
There was a problem hiding this comment.
The method verifyElementHasAttribute is defined but never used in the test file or within the page object itself. This creates unnecessary code that adds no value. Consider removing it to keep the codebase clean and maintainable.
| verifyElementHasAttribute(selector, attribute, value) { | |
| cy.get(selector).should('have.attr', attribute, value); | |
| } |
| this.verifyLinkWithText( | ||
| '[data-testid="Button-link"]', | ||
| 'Contribution Guide', | ||
| 'github.com/asyncapi', | ||
| ); | ||
|
|
||
| cy.contains('[data-testid="Button-link"]', 'Contribution Guide') | ||
| .should('have.attr', 'href') |
There was a problem hiding this comment.
The verification of the 'Contribution Guide' link is duplicated. Lines 38-42 already verify the href contains 'github.com/asyncapi', then lines 44-46 unnecessarily retrieve the same link again just to check for 'type=source'. These assertions should be combined into a single chain or the link should be aliased to avoid redundant DOM queries.
| this.verifyLinkWithText( | |
| '[data-testid="Button-link"]', | |
| 'Contribution Guide', | |
| 'github.com/asyncapi', | |
| ); | |
| cy.contains('[data-testid="Button-link"]', 'Contribution Guide') | |
| .should('have.attr', 'href') | |
| cy.contains('[data-testid="Button-link"]', 'Contribution Guide') | |
| .should('be.visible') | |
| .and('have.attr', 'href') | |
| .and('include', 'github.com/asyncapi') |
| new RegExp(`^${label}$`, 'i'), | ||
| ) | ||
| .click(); | ||
| cy.wait(500); |
There was a problem hiding this comment.
Hardcoded waits like cy.wait(500) are a Cypress anti-pattern and can lead to flaky tests. Instead, use explicit assertions to wait for the expected state after clicking the filter. For example, wait for the filter to become active or for the content to update by asserting on a class change or content change.
| cy.wait(500); | |
| cy.get('[data-testid="EventPostItem-main"]').should('be.visible'); |
| verifyAllEventCards() { | ||
| this.verifyEventCards(); | ||
| } | ||
|
|
||
| switchToAll() { | ||
| this.switchToFilter('All'); | ||
| } | ||
|
|
||
| switchToUpcoming() { | ||
| this.switchToFilter('Upcoming'); | ||
| } | ||
|
|
||
| switchToRecorded() { | ||
| this.switchToFilter('Recorded'); | ||
| } | ||
|
|
||
| verifyUpcomingEventCards() { | ||
| this.verifyEventCards(); | ||
| } | ||
|
|
||
| verifyRecordedEventCards() { | ||
| this.verifyEventCards(); | ||
| } |
There was a problem hiding this comment.
The methods verifyAllEventCards(), verifyUpcomingEventCards(), and verifyRecordedEventCards() all delegate to the same verifyEventCards() method without any distinction. This creates unnecessary duplication and doesn't provide tab-specific validation. Consider either removing these wrapper methods and calling verifyEventCards() directly from tests, or add tab-specific assertions to validate that the correct subset of events is displayed.
| verifyEventCardLinkByTitleAndHref(title, href, metaText) { | ||
| if (metaText) { | ||
| cy.contains('[data-testid="Event-span"]', metaText) | ||
| .parents('[data-testid="EventPostItem-main"]') | ||
| .within(() => { | ||
| cy.contains('h3', title).should('exist'); | ||
| cy.get('a[data-testid="EventPostItem-link"]').should( | ||
| 'have.attr', | ||
| 'href', | ||
| href, | ||
| ); | ||
| }); | ||
| } else { | ||
| cy.contains('[data-testid="EventPostItem-main"]', title) | ||
| .find('a[data-testid="EventPostItem-link"]') | ||
| .should('have.attr', 'href', href); | ||
| } | ||
| } |
There was a problem hiding this comment.
The method verifyEventCardLinkByTitleAndHref is defined but never used in the test file. This suggests either incomplete test coverage or unnecessary code. If this method is intended for future use, consider removing it until needed to keep the codebase clean. If specific event validation is needed, add corresponding tests.
| verifyEventCardHrefByIndex(index, expectedHref) { | ||
| cy.get('[data-testid="EventPostItem-main"]') | ||
| .eq(index) | ||
| .find('a[data-testid="EventPostItem-link"]') | ||
| .should('have.attr', 'href', expectedHref); | ||
| } | ||
|
|
There was a problem hiding this comment.
The method verifyEventCardHrefByIndex is defined but never used in the test file. This suggests either incomplete test coverage or unnecessary code. Consider removing it until needed to keep the codebase clean.
| verifyEventCardHrefByIndex(index, expectedHref) { | |
| cy.get('[data-testid="EventPostItem-main"]') | |
| .eq(index) | |
| .find('a[data-testid="EventPostItem-link"]') | |
| .should('have.attr', 'href', expectedHref); | |
| } |
3 participants
Description
Related issue(s)
Summary by CodeRabbit