test: add e2e tests for tsc, slack and ambassadors page #4244
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. |
WalkthroughAdds three new Cypress page objects (SlackPage, TSCPage, AmbassadorsPage), extends HomePage with two navigation helpers, and adds three test suites for Slack invites, TSC newsletter/social checks, and Ambassadors page verifications. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Cypress Test
participant Home as HomePage
participant TSC as TSCPage
Test->>Home: goToTSCPage()
Home-->>Test: navigated to /community/tsc
Test->>TSC: hoverCommunityLink()
Test->>TSC: fillNewsletterForm(name, email)
Test->>TSC: submitNewsletter()
TSC-->>Test: getSuccessMessage() / getFailureMessage()
Note right of TSC: Optional: verifyMemberSocialLinks(name, links)
sequenceDiagram
participant Test as Cypress Test
participant Slack as SlackPage
Test->>Slack: visitSlack()
Slack-->>Test: waitForPageLoad()
alt invite active
Test->>Slack: verifyAllLoginMethods()
else invite inactive
Test->>Slack: verifyInactiveLinkMessage()
end
Test->>Slack: verifyAllFooterLinks()
sequenceDiagram
participant Test as Cypress Test
participant Home as HomePage
participant Amb as AmbassadorsPage
Test->>Home: goToAmbassadorsPage()
Home-->>Test: navigated to /community/ambassadors
Test->>Amb: verifyKeySectionsAndLinks()
Note right of Amb: iterate -> verifyAmbassadorSocialLinks(name, links)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 #4244 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 799 799
Branches 146 146
=========================================
Hits 799 799 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
cypress/pages/tscpage.js (1)
1-4: Clean up formatting and add proper class documentation.Consider adding a class comment describing the purpose of this page object and removing unnecessary blank lines.
- class TSCPage { - - + /** + * Page object for TSC (Technical Steering Committee) page interactions + */cypress/pages/slack.js (2)
3-5: Consider making the Slack URL configurable.The hardcoded URL could become outdated or may need to be different for different environments.
- visitSlack(){ - cy.visit('https://asyncapi.slack.com/join/shared_invite/zt-33bsaqqgz-ZL0a3ZUiuy4stSbXB~~E9A#/shared-invite/email'); - } + visitSlack(url = 'https://asyncapi.slack.com/join/shared_invite/zt-33bsaqqgz-ZL0a3ZUiuy4stSbXB~~E9A#/shared-invite/email'){ + cy.visit(url); + }
24-31: Improve consistency in assertion patterns.Consider using more specific assertions and consistent patterns across verification methods.
verifyPrivacyAndTerms() { cy.get('[href="/legal"]') - .should('have.attr', 'href', '/legal'); + .should('have.attr', 'href', '/legal') + .and('be.visible'); } verifyContactUs(){ cy.get('[href="/help/requests/new"]') - .should('have.attr', 'href', '/help/requests/new'); + .should('have.attr', 'href', '/help/requests/new') + .and('be.visible'); }cypress/tscpage.cy.js (1)
32-46: Improve formatting and structure of link verification test.The test has inconsistent spacing and formatting issues that affect readability.
- - it('verifies key links on the TSC page', () => { - - const linksToVerify = [ - {href: 'https://github.com/asyncapi/community/blob/master/TSC_MEMBERSHIP.md',label:'Link'}, - {href: 'https://github.com/asyncapi/community/blob/master/CHARTER.md',label:'Open Governance Model'}, - {href: 'https://www.asyncapi.com/blog/governance-motivation',label:'this'} - ]; - - linksToVerify.forEach(({ href,label }) => { - cy.get(`a[href="${href}"]`).contains(label); - - - }) - }); + it('verifies key links on the TSC page', () => { + const linksToVerify = [ + { href: 'https://github.com/asyncapi/community/blob/master/TSC_MEMBERSHIP.md', label: 'Link' }, + { href: 'https://github.com/asyncapi/community/blob/master/CHARTER.md', label: 'Open Governance Model' }, + { href: 'https://www.asyncapi.com/blog/governance-motivation', label: 'this' } + ]; + + linksToVerify.forEach(({ href, label }) => { + cy.get(`a[href="${href}"]`).contains(label).should('be.visible'); + }); + });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cypress/pages/homepage.js(2 hunks)cypress/pages/slack.js(1 hunks)cypress/pages/tscpage.js(1 hunks)cypress/slackworkspace.cy.js(1 hunks)cypress/tscpage.cy.js(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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
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: 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: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
cypress/slackworkspace.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/tscpage.cy.js (6)
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#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#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.
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#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.
cypress/pages/tscpage.js (2)
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.
⏰ 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: Lighthouse CI
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (4)
cypress/pages/tscpage.js (1)
5-29: LGTM! Well-structured page object methods.The page object methods are well-implemented with appropriate selectors and follow good Cypress patterns. The use of
data-testidattributes is excellent for test stability.cypress/slackworkspace.cy.js (1)
1-15: LGTM! Clean and well-structured test implementation.The test file follows good Cypress patterns with proper page object usage. The test case systematically verifies all the UI elements on the Slack workspace page.
cypress/pages/homepage.js (1)
1-2: LGTM! Proper imports added for page objects.The imports for ToolsPage and DocsPage are correctly added to support the new navigation methods.
cypress/tscpage.cy.js (1)
8-14: LGTM! Well-structured test setup.The beforeEach hook properly initializes page objects and navigates to the correct page for testing.
anshgoyalevil
added
the
gsoc
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cypress/slackworkspace.cy.js (1)
1-21: Consider adding error handling and test isolation.While the current implementation is solid, consider these enhancements for robustness:
import SlackPage from './pages/slack'; describe('Slack workspace tests', () => { const slackPage = new SlackPage(); beforeEach(() => { slackPage.visitSlack(); }); + afterEach(() => { + // Clean up any test state if needed + }); it('User can access all login methods', () => { slackPage.verifyGoogleLoginButton(); slackPage.verifyAppleLoginButton(); slackPage.verifyContinueWithEmail(); }); it('User can view links for Privacy, Contact Us, and Region Change', () => { slackPage.verifyPrivacyAndTerms(); slackPage.verifyContactUs(); slackPage.verifyChangeRegion(); }); });Additionally, consider adding test tags for better organization:
-describe('Slack workspace tests', () => { +describe('Slack workspace tests', { tags: ['@slack', '@e2e'] }, () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cypress/pages/tscpage.js(1 hunks)cypress/slackworkspace.cy.js(1 hunks)cypress/tscpage.cy.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cypress/pages/tscpage.js
- cypress/tscpage.cy.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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
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: 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: sagarkori143
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-06-20T14:47:22.389Z
Learning: The AsyncAPI website modularization PR uses Promise.allSettled for parallel execution of build tasks, which provides better performance and fault isolation compared to sequential execution.
cypress/slackworkspace.cy.js (3)
Learnt from: JeelRajodiya
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-03T08:14:02.138Z
Learning: The user (JeelRajodiya) has stated a preference to keep the test files in CommonJS rather than migrating them to TypeScript.
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: 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.
⏰ 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). (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (4)
cypress/slackworkspace.cy.js (4)
3-8: Good test suite structure with proper setup.The test suite follows Cypress best practices with a clear describe block and beforeEach hook for consistent test setup.
10-14: Well-structured test for login methods.The test case covers all login methods systematically. The test name clearly describes what is being verified.
16-20: Comprehensive test for privacy and contact links.The test case covers important user-facing elements like privacy terms, contact information, and region settings.
1-1: SlackPage methods verified – no issues foundAll required methods (visitSlack, verifyGoogleLoginButton, verifyAppleLoginButton, verifyContinueWithEmail, verifyPrivacyAndTerms, verifyContactUs, verifyChangeRegion) are present in cypress/pages/slack.js. No further changes needed.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4244--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
cypress/ambassadors.cy.js (1)
19-50: Hardcoded ambassador data may cause test brittleness.The test embeds specific ambassador names and social links that could become outdated if ambassadors are removed, renamed, or update their profiles. This creates a maintenance burden and potential for false negatives.
Consider these approaches to reduce brittleness:
- Test only that the social link elements exist and have valid href patterns, without verifying specific ambassadors
- Fetch ambassador data dynamically from the page and verify structure/format rather than specific values
- Document that this test data should be reviewed periodically
Example of a more resilient approach:
it('verifies social links structure for ambassadors', () => { // Verify that at least one ambassador has properly formatted social links cy.get('[data-testid="Ambassadors-members"]').first().within(() => { cy.get('a[href*="github.com"]').should('have.attr', 'href').and('match', /^https:\/\/www\.github\.com\//); cy.get('a[href*="twitter.com"]').should('have.attr', 'href').and('match', /^https:\/\/www\.twitter\.com\//); cy.get('a[href*="linkedin.com"]').should('have.attr', 'href').and('match', /^https:\/\/www\.linkedin\.com\//); }); });cypress/pages/ambassadors.js (2)
6-19: Brittle selectors using full GitHub URLs.The selectors use complete GitHub URLs (lines 7, 14, 16) which are fragile and will break if the repository structure changes (e.g., branch rename from
mastertomain, file reorganization, or URL updates).Consider more resilient selector strategies:
- Use partial URL matching with
[href*="..."]- Add
data-testidattributes to these links in the source code- Select by link text content instead of href
verifyKeySectionsAndLinks() { - cy.get('a[href="https://github.com/asyncapi/community/blob/master/docs/050-mentorship-program/ambassador-program/AMBASSADOR_PROGRAM.md"]') + cy.get('a[href*="/community/blob/"][href*="/AMBASSADOR_PROGRAM.md"]') .should('be.visible'); cy.get('[data-testid="Ambassadors-Iframe"]') .should('be.visible'); cy.get('[data-testid="Ambassadors-members-main"]') .should('be.visible'); - cy.get('a[href="https://github.com/asyncapi/community/blob/master/AMBASSADOR_ORGANIZATION.md#are-you-interested-in-becoming-an-official-asyncapi-ambassador"]') + cy.get('a[href*="/AMBASSADOR_ORGANIZATION.md#are-you-interested"]') .should('be.visible'); - cy.get('a[href="https://www.asyncapi.com/blog/asyncapi-ambassador-program"]') + cy.get('a[href*="/blog/asyncapi-ambassador-program"]') .should('be.visible'); }
21-30: Add error handling for ambassador not found.The method assumes the ambassador exists on the page. If the ambassador is removed or renamed, the test will fail with a cryptic timeout error rather than a clear message.
Add an explicit assertion after finding the ambassador:
verifyAmbassadorSocialLinks(name, links) { cy.contains('[data-testid="Ambassadors-members-details"]', name) + .should('exist') .closest('[data-testid="Ambassadors-members"]') .within(() => { if (links.twitter) cy.get(`a[href="${links.twitter}"]`).should('be.visible'); if (links.github) cy.get(`a[href="${links.github}"]`).should('be.visible'); if (links.linkedin) cy.get(`a[href="${links.linkedin}"]`).should('be.visible'); }); }cypress/tscpage.cy.js (2)
28-46: Brittle selectors using full GitHub/blog URLs.Similar to the ambassadors page, these selectors use complete URLs that will break if repository structure or blog URL paths change.
Use partial URL matching for more resilience:
const linksToVerify = [ { - href: 'https://github.com/asyncapi/community/blob/master/TSC_MEMBERSHIP.md', + href: '/community/blob/', + contains: 'TSC_MEMBERSHIP.md', label: 'Link', }, { - href: 'https://github.com/asyncapi/community/blob/master/CHARTER.md', + href: '/community/blob/', + contains: 'CHARTER.md', label: 'Open Governance Model', }, { - href: 'https://www.asyncapi.com/blog/governance-motivation', + href: '/blog/governance-motivation', label: 'this', }, ]; - linksToVerify.forEach(({ href, label }) => { - cy.get(`a[href="${href}"]`).contains(label).should('be.visible'); + linksToVerify.forEach(({ href, contains, label }) => { + cy.get(`a[href*="${href}"]${contains ? `[href*="${contains}"]` : ''}`).contains(label).should('be.visible'); });
48-80: Hardcoded TSC member data may cause test brittleness.Similar to the ambassadors test, this embeds specific member names and social links that could become outdated, creating maintenance overhead.
Consider the same alternatives mentioned for the ambassadors test:
- Test structural validity rather than specific member data
- Fetch member data dynamically and verify format
- Document the need for periodic review
Additionally, note the inconsistency in link property names: this test uses
GitHub,github,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cypress/ambassadors.cy.js(1 hunks)cypress/pages/ambassadors.js(1 hunks)cypress/pages/homepage.js(2 hunks)cypress/pages/slack.js(1 hunks)cypress/pages/tscpage.js(1 hunks)cypress/tscpage.cy.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/pages/tscpage.js
🧰 Additional context used
🧠 Learnings (2)
📚 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/tscpage.cy.js
📚 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/tscpage.cy.js
🧬 Code graph analysis (1)
cypress/ambassadors.cy.js (1)
cypress/tscpage.cy.js (1)
homePage(4-4)
🔇 Additional comments (2)
cypress/pages/homepage.js (1)
26-44: Inconsistent navigation patterns across methods.The navigation methods use two different approaches:
goToToolsPage()andgoToDocsPage()use.click()navigation, whilegoToTSCPage()andgoToAmbassadorsPage()use directcy.visit(). While this may be intentional (if TSC/Ambassadors aren't in the navbar), it creates an inconsistent API.Verify whether TSC and Ambassadors pages can be reached via navbar navigation. If they are accessible through the navbar, consider updating the methods to use
.click()for consistency:goToTSCPage(){ cy.get('[data-testid="Navbar-main"]').contains('Community').click(); cy.contains('TSC').click(); // or appropriate selector return new TSCPage(); }If direct navigation is required (e.g., these pages aren't in the navbar), consider adding JSDoc comments explaining why
cy.visit()is used for these methods.cypress/tscpage.cy.js (1)
14-25: Newsletter subscription tests may be flaky due to external dependencies.These tests rely on actual form submission to a backend service, making them vulnerable to:
- Network issues
- Backend downtime
- Rate limiting
- Email validation changes
Consider whether these tests should:
- Stub/mock the network request to control the response
- Be moved to a separate integration test suite with appropriate retries
- Use Cypress intercept to verify the request payload without actually submitting
it('should succeed in subscribing to the newsletter', () => { cy.intercept('POST', '**/newsletter**', { statusCode: 200 }).as('newsletter'); tscpage.fillNewsletterForm('anushka', '[email protected]'); tscpage.submitNewsletter(); cy.wait('@newsletter'); tscpage.getSuccessMessage().should('be.visible'); });
cypress/pages/slack.js
Outdated
| visitSlack(){ | ||
| cy.visit('https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email'); | ||
| } |
There was a problem hiding this comment.
Hardcoded invite URL will expire, causing test failures.
The Slack shared invite URL contains a time-limited token (zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw) that will eventually expire, breaking the test.
Consider one of these approaches:
- Store the invite URL in an environment variable or config file that can be easily updated
- Generate a fresh invite token before tests run (if Slack API supports this)
- Document the token expiration and update process for maintainers
visitSlack(){
- cy.visit('https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email');
+ cy.visit(Cypress.env('SLACK_INVITE_URL'));
}📝 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.
| visitSlack(){ | |
| cy.visit('https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email'); | |
| } | |
| visitSlack(){ | |
| cy.visit(Cypress.env('SLACK_INVITE_URL')); | |
| } |
🤖 Prompt for AI Agents
In cypress/pages/slack.js around lines 3 to 5 the Slack invite URL is hardcoded
with a time-limited token which will expire and break tests; replace the
hardcoded URL by reading it from configuration (CYPRESS_ENV, cypress.env.json or
a config file) or an environment variable, add fallback validation that the
variable exists and fail fast with a clear message, and update project docs to
show how to rotate/update the invite or add a script to generate a fresh invite
via Slack API if available.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cypress/pages/ambassadors.js(1 hunks)cypress/pages/homepage.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/pages/ambassadors.js
🧰 Additional context used
🪛 Biome (2.1.2)
cypress/pages/homepage.js
[error] 146-149: Duplicate class member name "goToToolsPage"
(lint/suspicious/noDuplicateClassMembers)
[error] 151-154: Duplicate class member name "goToDocsPage"
(lint/suspicious/noDuplicateClassMembers)
⏰ 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 - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (2)
cypress/pages/homepage.js (2)
1-2: LGTM!The imports for
AmbassadorsPageandTSCPageare correctly added and follow the existing import pattern.
156-164: LGTM!The new navigation methods
goToTSCPageandgoToAmbassadorsPagecorrectly follow the page object pattern by navigating to the respective pages and returning page object instances.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cypress/slackworkspace.cy.js (1)
29-42: Consider removing or enhancing this test.This test appears redundant with the first test. When the link is active, it only logs without any assertions, providing minimal value. Consider either:
- Removing this test if the first test adequately covers inactive link handling
- Adding meaningful assertions for the active state
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cypress/pages/slack.js(1 hunks)cypress/slackworkspace.cy.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/pages/slack.js
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: asyncapi-bot
Repo: asyncapi/website PR: 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
⏰ 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: cypress-run
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (1)
cypress/slackworkspace.cy.js (1)
1-8: LGTM: Test setup is well-structured.The page object instantiation and beforeEach hook follow standard Cypress patterns.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cypress/pages/slack.js (1)
2-6: Hardcoded Slack invite URL will cause test failures when token expires.The Slack invite URL contains a time-limited token (
zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw) that will eventually expire, breaking this test. This issue was previously flagged but remains unaddressed.Consider storing the invite URL in an environment variable:
visitSlack() { - cy.visit( - 'https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email', - ); + const inviteUrl = Cypress.env('SLACK_INVITE_URL'); + if (!inviteUrl) { + throw new Error('SLACK_INVITE_URL environment variable is not set'); + } + cy.visit(inviteUrl); }
🧹 Nitpick comments (2)
cypress/pages/homepage.js (1)
146-154: Navigation pattern differs from existing methods.The new methods
goToTSCPage()andgoToAmbassadorsPage()usecy.visit()for direct URL navigation, while most existing navigation methods (likegoToDocsPage()at lines 121-124) simulate user behavior by clicking navbar elements.Direct navigation is faster but doesn't verify the navigation UI. If these pages are accessible via the navbar or other UI elements, consider using the click-based pattern for consistency and broader test coverage.
If TSC and Ambassadors pages are in the Community dropdown:
goToTSCPage(){ - cy.visit('/community/tsc'); + cy.get('[data-testid="Navbar-main"]').contains('Community').click(); + cy.contains('a', 'TSC').click(); return new TSCPage(); } goToAmbassadorsPage() { - cy.visit('/community/ambassadors'); + cy.get('[data-testid="Navbar-main"]').contains('Community').click(); + cy.contains('a', 'Ambassadors').click(); return new AmbassadorsPage(); }cypress/tscpage.cy.js (1)
48-80: Hard-coded member data may become stale and require frequent updates.The test embeds TSC member names and social media links directly in the test code. This creates maintenance overhead when:
- TSC membership changes
- Members update their social media profiles
- Handles or URLs change
Consider externalizing this data to a fixture file:
Create
cypress/fixtures/tsc-members.json:{ "members": [ { "name": "Aishat Muibudeen", "links": { "GitHub": "https://www.github.com/Mayaleeeee", "Twitter": "https://www.twitter.com/maya_ux_ui", "Linkedin": "https://www.linkedin.com/in/aishatmuibudeen" } } ] }Then update the test:
it('verifies social links for selected TSC members', () => { - const members = [ - { name: 'Aishat Muibudeen', ... }, - ... - ]; + cy.fixture('tsc-members').then((data) => { + data.members.forEach(({ name, links }) => { + tscpage.verifyMemberSocialLinks(name, links); + }); + }); - members.forEach(({ name, links }) => { - tscpage.verifyMemberSocialLinks(name, links); - }); });This centralizes member data and makes updates easier across all tests.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cypress/pages/homepage.js(2 hunks)cypress/pages/slack.js(1 hunks)cypress/tscpage.cy.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/tscpage.cy.js
📚 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/tscpage.cy.js
📚 Learning: 2024-10-11T11:32:30.226Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the `BlogPostItem` component (`components/navigation/BlogPostItem.tsx`), nesting `<a>` tags inside the parent `Link` component leads to hydration issues; therefore, we should avoid nesting `<a>` tags inside `Link` components in this component.
Applied to files:
cypress/tscpage.cy.js
🧬 Code graph analysis (1)
cypress/tscpage.cy.js (1)
cypress/ambassadors.cy.js (1)
homePage(4-4)
⏰ 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)
- GitHub Check: cypress-run
- GitHub Check: Test NodeJS PR - windows-latest
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cypress/pages/slack.js (2)
2-6: Consider externalizing the time-limited Slack invite URL.The hardcoded invite URL contains a time-limited token (
zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw) that will eventually expire, causing test failures. Moving it to a configuration file or environment variable will make updates easier.Apply this diff to use an environment variable:
visitSlack() { - cy.visit( - 'https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email', - ); + const inviteUrl = Cypress.env('SLACK_INVITE_URL') || + 'https://asyncapi.slack.com/join/shared_invite/zt-3clk6rmc0-Cujl2fChHYnHDUwFKRlQCw#/shared-invite/email'; + cy.visit(inviteUrl); }Then add to
cypress.config.jsorcypress.env.json:{ "SLACK_INVITE_URL": "https://asyncapi.slack.com/join/shared_invite/YOUR_TOKEN#/shared-invite/email" }
78-82: Consider a more specific selector for the region change link.The selector
a[href="#"]is quite broad and might match unintended elements if other hash links are added to the page. While the text filter narrows it down, a more specific selector would be more robust.If the element has a data attribute or unique class, use that instead:
verifyChangeRegion() { - cy.get('a[href="#"]') + cy.get('[data-qa="change_region_link"]') // or a more specific selector .should('be.visible') .and('contain.text', 'Change region'); }Alternatively, scope it to a specific container if available:
cy.get('footer').find('a[href="#"]')
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cypress/pages/slack.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). (2)
- GitHub Check: cypress-run
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
cypress/pages/slack.js (2)
8-16: Well-implemented conditional wait.The method properly waits for actual page elements (either inactive message or login button) rather than using fixed timeouts, and handles both possible page states appropriately.
33-43: Good abstraction for grouped verifications.These wrapper methods improve test readability by grouping related verification steps.
3 participants
Description
Related issue(s)
Summary by CodeRabbit
New Features
Tests