-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
test: add e2e tests for pages under tools dropdown #4533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughIntroduces a centralized Cypress BasePage class, refactors many page objects to extend it, adds fixtures for AsyncAPI tools, and adds new page objects and E2E tests for multiple tools pages. Several helper methods and test descriptions were consolidated or updated. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 #4533 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4533--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cypress/pages/toolsGenerator.js (1)
6-8: Extract duplicated helper to reduce code duplication.The
verifyHeadingExistsmethod is duplicated identically intoolsModelina.js(lines 6-8) andtoolsMisc.js(lines 14-16). Consider extracting this to a base page class or a shared helper module.Example approach - create a base page class:
// cypress/pages/basePage.js class BasePage { verifyHeadingExists(text) { cy.contains('h1, h2, h3, h4, h5, h6', text).should('be.visible'); } } export default BasePage;Then extend it:
+import BasePage from './basePage'; + -class ToolsGeneratorPage { +class ToolsGeneratorPage extends BasePage { visit() { cy.visit('/tools/generator'); } - verifyHeadingExists(text) { - cy.contains('h1, h2, h3, h4, h5, h6', text).should('be.visible'); - } - verifyHeader() { this.verifyHeadingExists('Docs, Code, Anything!'); }cypress/pages/toolsModelina.js (1)
26-28: Consider using 'be.visible' for stronger assertion.The assertion uses
.should('exist')which only checks DOM presence. For e2e tests,.should('be.visible')provides stronger verification that users can actually see the install snippet.Apply this change (and similarly in
toolsMisc.jslines 38, 53):verifyInstallSnippet() { - cy.contains('code', 'npm install @asyncapi/modelina').should('exist'); + cy.contains('code', 'npm install @asyncapi/modelina').should('be.visible'); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cypress/pages/toolsGenerator.js(1 hunks)cypress/pages/toolsMisc.js(1 hunks)cypress/pages/toolsModelina.js(1 hunks)cypress/tools_generator.cy.js(1 hunks)cypress/tools_misc.cy.js(1 hunks)cypress/tools_modelina.cy.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). (3)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
🔇 Additional comments (6)
cypress/tools_generator.cy.js (2)
1-8: LGTM! Clean test setup.The test file follows Cypress best practices with proper page object instantiation and beforeEach hook for navigation.
10-24: LGTM! Well-structured test cases.The test cases are focused and follow the single-responsibility principle, making them easy to maintain and debug.
cypress/tools_modelina.cy.js (1)
1-25: LGTM! Consistent test structure.This test file follows the same pattern as the other tools tests, maintaining consistency across the test suite.
cypress/tools_misc.cy.js (1)
1-56: LGTM! Well-organized test suite.The use of nested describe blocks effectively organizes tests for multiple related pages, making the test suite easy to navigate and maintain.
cypress/pages/toolsMisc.js (1)
1-72: LGTM! Well-organized multi-page page object.The clear separation of methods with comments for each page (CLI, Parsers, GitHub Actions) makes this page object easy to navigate and maintain.
cypress/pages/toolsGenerator.js (1)
18-22: The review comment is based on incorrect assumptions and should be disregarded.The tests are actually correct in expecting
https://www.github.com/URLs because the source code inpages/tools/generator.tsx(line 25) and related tool pages explicitly use this format. The tests are properly validating what the page actually renders.While www.github.com redirects to github.com (the canonical form), both URLs are functional. If using canonical GitHub URLs is a goal, the fix should be in the source code files (
pages/tools/*.tsx), not in the test expectations (cypress/pages/*.js). The tests are doing their job correctly—they verify the rendered output matches the href attributes in the actual source components.Likely an incorrect or invalid review comment.
princerajpoot20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anushkaaaaaaaa
Please have a look at the review comments.
I stopped reviewing the PR in between, for the reason I mentioned in the review above: there are major changes that need to be done across all the open PRs you have so far (#4551, #4548, #4533, #4244, #4215).
Hence, I thought there was no point in continuing the review as there are major changes required. Ping me once you are done with all the changes in all those open PRs. I will then give another review.
Also, I am not sure if you have the same pattern in the previous PRs that you have already merged. But please review all your work, regardless of whether it is merged. We can still fix it easily. It will be difficult at a later stage. We should not have so much repetition in functions. This violates the DRY principle and will make the code very difficult to maintain in the long run. Adding a simple test will require a lot of repetitive work, and we don’t want to reach that state.
Thanks.
cypress/pages/toolsGenerator.js
Outdated
| @@ -0,0 +1,31 @@ | |||
| class ToolsGeneratorPage { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: consider naming class consistently with file name. Reivew these naming in all the files
cypress/pages/toolsGenerator.js
Outdated
| cy.visit('/tools/generator'); | ||
| } | ||
|
|
||
| verifyHeadingExists(text) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VerifyHeadingExists is duplicated across multiple pages in this PR and also in the all the other PRs you have raised.
Consider extracting it into a shared base helper to keep things DRY and easier to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@princerajpoot20 can you please elaborate this more
I didn't get it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sambhavgupta0705 This verifyHeadingExists function is duplicated in every file. It repeats the same logic again and again across multiple files and multiple PRs of @anushkaaaaaaaa. We need to remove this duplication and keep the code DRY.
cypress/pages/toolsGenerator.js
Outdated
| verifyMainImage() { | ||
| cy.get('img[alt="generator diagram"]').should('be.visible'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider asserting more strongly like src is non-empty. also please review the naming of function. What do you mean by mainImage?
cypress/pages/toolsMisc.js
Outdated
| } | ||
|
|
||
| verifyHeadingExists(text) { | ||
| cy.contains('h1, h2, h3, h4, h5, h6', text).should('be.visible'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as mentioned earlier. please follow DRY principle
cypress/pages/toolsMisc.js
Outdated
| cy.visit('/tools/cli'); | ||
| } | ||
|
|
||
| visitParsers() { | ||
| cy.visit('/tools/parsers'); | ||
| } | ||
|
|
||
| visitGithubActions() { | ||
| cy.visit('/tools/github-actions'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review these dedicated visit() methods that are everywhere. They are repeating the same stuff 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 via multiple PRs you have opened right now. If they can be extracted out into shared functions, it will make them reusable and keep the code DRY.
cypress/pages/toolsMisc.js
Outdated
| verifyCliHeader() { | ||
| this.verifyHeadingExists( | ||
| 'Interact with AsyncAPI from the comfort of your CLI', | ||
| ); | ||
| } | ||
|
|
||
| verifyCliGithubLink() { | ||
| cy.contains('a', 'View on Github') | ||
| .should('be.visible') | ||
| .and('have.attr', 'href', 'https://www.github.com/asyncapi/cli'); | ||
| } | ||
|
|
||
| verifyCliDocsLink() { | ||
| cy.contains('a', 'View Docs') | ||
| .should('be.visible') | ||
| .and('have.attr', 'href', '/docs/tools/cli'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anushkaaaaaaaa
As mentioned earlier,
all these can be removed. This is unnecessary repetition of code.
Make this a general function that takes parameters and move it to a shared folder so it can be used by everyone and you don’t have to write the same code again and again. Please refer to the DRY principle. This will make the code maintainable.
I have observed this across all your PRs that are open right now, so please review all your PRs for this.
cypress/pages/toolsMisc.js
Outdated
| verifyParsersGithubLink() { | ||
| cy.contains('a', 'View on Github') | ||
| .should('be.visible') | ||
| .and('have.attr', 'href', 'https://www.github.com/asyncapi/parser-js'); | ||
| } | ||
|
|
||
| verifyParsersInstallSnippet() { | ||
| cy.contains('code', 'npm install @asyncapi/parser').should('exist'); | ||
| } | ||
|
|
||
| // GitHub Actions page checks | ||
| verifyGhActionsHeader() { | ||
| this.verifyHeadingExists('Automate using GitHub Actions'); | ||
| } | ||
|
|
||
| verifyGhActionsGithubLink() { | ||
| cy.contains('a', 'View on Github') | ||
| .should('be.visible') | ||
| .and( | ||
| 'have.attr', | ||
| 'href', | ||
| 'https://www.github.com/asyncapi/github-action-for-generator', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above, you should avoid these and make a general function that takes parameters
cypress/pages/toolsMisc.js
Outdated
| } | ||
| } | ||
|
|
||
| export default ToolsMiscPage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review this naming
cypress/pages/toolsModelina.js
Outdated
| visit() { | ||
| cy.visit('/tools/modelina'); | ||
| } | ||
|
|
||
| verifyHeadingExists(text) { | ||
| cy.contains('h1, h2, h3, h4, h5, h6', text).should('be.visible'); | ||
| } | ||
|
|
||
| verifyHeader() { | ||
| this.verifyHeadingExists('Modelina'); | ||
| } | ||
|
|
||
| verifyGithubLink() { | ||
| cy.contains('a', 'View on Github') | ||
| .should('be.visible') | ||
| .and('have.attr', 'href', 'https://www.github.com/asyncapi/modelina'); | ||
| } | ||
|
|
||
| verifyTryItNowLink() { | ||
| cy.contains('a', 'Try it now') | ||
| .should('be.visible') | ||
| .and('have.attr', 'href', 'https://modelina.org/playground'); | ||
| } | ||
|
|
||
| verifyInstallSnippet() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope you are now able to understand the real benefit. If you had made general methods, you wouldn’t have to put repetitive effort into doing the same stuff. And most importantly, it makes the code maintainable.
cypress/pages/toolsGenerator.js
Outdated
| cy.visit('/tools/generator'); | ||
| } | ||
|
|
||
| verifyHeadingExists(text) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@princerajpoot20 can you please elaborate this more
I didn't get it
cypress/pages/BaseToolsPage.js
Outdated
| class BaseToolsPage { | ||
| visit(path) { | ||
| cy.visit(path); | ||
| return this; | ||
| } | ||
|
|
||
| verifyHeadingExists(text) { | ||
| return cy.contains('h1, h2, h3, h4, h5, h6', text).should('be.visible'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anushkaaaaaaaa can you please have a look on this
…olsPage extending it.
…, and Community pages.
princerajpoot20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anushkaaaaaaaa Please have a look at the review comments
…Text in BasePage and related tests
…CLI, Parsers, GitHub Actions, and Modelina.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cypress/tools_modelina.cy.js`:
- Around line 15-21: Update the two test case description strings to use
consistent capitalization and fix the typo: change the first it(...) title from
'verifying install snippet' to 'Verifying install snippet' and change the second
it(...) title from 'Verifiying the GitHub button link' to 'Verifying the GitHub
button link'; these are the human-readable names passed to the it(...) blocks
that wrap calls to page.verifyElementContainsText and page.verifyButtonLink,
respectively.
🧹 Nitpick comments (3)
cypress/pages/Footer.js (1)
10-12: Assert a single visible footer instead of silently picking the first.
Selecting.first()can mask regressions where multiple visible footers appear. Consider asserting exactly one visible footer (or use a more specific selector/data-testid).♻️ Suggested tweak
- cy.get(this.footerSelector).filter(':visible').first().within(() => { + cy.get(this.footerSelector) + .filter(':visible') + .should('have.length', 1) + .first() + .within(() => { this.verifyLink(href, text, attr); });cypress/tools_generator.cy.js (1)
1-5: Inconsistent class naming convention.The import and instantiation use camelCase (
toolsGenerator), while other similar test files in this PR use PascalCase (ToolsMisc,ToolsModelina). For consistency across the test suite and adherence to JavaScript class naming conventions, use PascalCase.♻️ Suggested fix
-import toolsGenerator from './pages/toolsGenerator'; +import ToolsGenerator from './pages/toolsGenerator'; import toolsData from './fixtures/toolsPages.json'; describe('Tools - Generator Page', () => { - const page = new toolsGenerator(); + const page = new ToolsGenerator();cypress/pages/BaseHeaderPage.js (1)
3-5: Note:visit()no longer returnsthisfor fluent chaining.The previous implementation likely had
return this;to allow method chaining. If any tests rely on chaining aftervisit(), this change would break them. For header-related tests that always start from the homepage, the hardcoded'/'path is likely intentional.♻️ Consider preserving fluent chaining if needed
visit() { - cy.visit('/') + cy.visit('/'); + return this; }
…mplementing a page object model with a new BasePage.
…into toolsPage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,12 +1,12 @@ | |||
| import BasePage from './BasePageTools'; | |||
| import BasePage from './BasePage'; | |||
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import was changed from BasePageTools to BasePage, but the new BasePage class doesn't provide the getLink method that ToolsPage relies on (used in line 23 of the file). This will cause a runtime error when verifyToolLink is called. Either add the getLink method to the new BasePage class, or keep ToolsPage extending BasePageTools.
| it('Verifying the main header and intro', () => { | ||
| page.verifyHeadingExists(toolsData.modelina.heading); | ||
| }); | ||
|
|
||
| it('verifying install snippet', () => { | ||
| page.verifyElementContainsText('code', toolsData.modelina.install); | ||
| }); | ||
|
|
||
| it('Verifiying the GitHub button link', () => { | ||
| page.verifyButtonLink(toolsData.modelina.github, 'View on Github'); | ||
| }); | ||
|
|
||
| it('Verifying the Try it now link', () => { |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test description capitalization is inconsistent within this file. Line 11 uses "Verifying", line 15 uses "verifying" (lowercase), line 19 uses "Verifiying" (misspelled), and line 23 uses "Verifying". The descriptions should follow a consistent capitalization pattern. Based on conventions in homepage.cy.js which uses "should" statements, consider either consistently using "Verifying" or adopting "should" style descriptions.
| it('Verifying the main header and intro', () => { | |
| page.verifyHeadingExists(toolsData.modelina.heading); | |
| }); | |
| it('verifying install snippet', () => { | |
| page.verifyElementContainsText('code', toolsData.modelina.install); | |
| }); | |
| it('Verifiying the GitHub button link', () => { | |
| page.verifyButtonLink(toolsData.modelina.github, 'View on Github'); | |
| }); | |
| it('Verifying the Try it now link', () => { | |
| it('should display the main header and intro', () => { | |
| page.verifyHeadingExists(toolsData.modelina.heading); | |
| }); | |
| it('should show the install snippet', () => { | |
| page.verifyElementContainsText('code', toolsData.modelina.install); | |
| }); | |
| it('should have the GitHub button link', () => { | |
| page.verifyButtonLink(toolsData.modelina.github, 'View on Github'); | |
| }); | |
| it('should have the Try it now link', () => { |
| .should('not.be.empty'); | ||
| } | ||
|
|
||
|
|
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line at the end of the class. This is inconsistent with the coding style seen in other page objects where there's only one blank line before the export statement.
| const page = new BasePage(); | ||
|
|
||
| beforeEach(() => { | ||
| cy.viewport(1280, 720); | ||
| page.visit(toolsData.cli.path); | ||
| }); | ||
|
|
||
| it('Verifying if the CLI header is visible', () => { | ||
| page.verifyHeadingExists(toolsData.cli.heading); | ||
| }); | ||
|
|
||
| it('Verifying if CLI GitHub and Docs links work', () => { | ||
| page.verifyButtonLink(toolsData.cli.github, 'View on Github'); | ||
| page.verifyButtonLink(toolsData.cli.docs, 'View Docs'); | ||
| }); | ||
|
|
||
| it('Verifying if CLI install snippet is visible', () => { | ||
| page.verifyElementContainsText('code', toolsData.cli.install); | ||
| }); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: This file uses 4 spaces for indentation in the beforeEach and test blocks, while other test files use 2 spaces (e.g., tools_generator.cy.js, tools_modelina.cy.js). Maintain consistent indentation throughout the codebase.
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.cli.path); | |
| }); | |
| it('Verifying if the CLI header is visible', () => { | |
| page.verifyHeadingExists(toolsData.cli.heading); | |
| }); | |
| it('Verifying if CLI GitHub and Docs links work', () => { | |
| page.verifyButtonLink(toolsData.cli.github, 'View on Github'); | |
| page.verifyButtonLink(toolsData.cli.docs, 'View Docs'); | |
| }); | |
| it('Verifying if CLI install snippet is visible', () => { | |
| page.verifyElementContainsText('code', toolsData.cli.install); | |
| }); | |
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.cli.path); | |
| }); | |
| it('Verifying if the CLI header is visible', () => { | |
| page.verifyHeadingExists(toolsData.cli.heading); | |
| }); | |
| it('Verifying if CLI GitHub and Docs links work', () => { | |
| page.verifyButtonLink(toolsData.cli.github, 'View on Github'); | |
| page.verifyButtonLink(toolsData.cli.docs, 'View Docs'); | |
| }); | |
| it('Verifying if CLI install snippet is visible', () => { | |
| page.verifyElementContainsText('code', toolsData.cli.install); | |
| }); |
| const page = new BasePage(); | ||
|
|
||
| beforeEach(() => { | ||
| cy.viewport(1280, 720); | ||
| page.visit(toolsData.parsers.path); | ||
| }); | ||
|
|
||
| it('Verifying if the Parsers header is visible', () => { | ||
| page.verifyHeadingExists(toolsData.parsers.heading); | ||
| }); | ||
|
|
||
| it('Verifying if Parsers GitHub link works', () => { | ||
| page.verifyButtonLink(toolsData.parsers.github, 'View on Github'); | ||
| }); | ||
|
|
||
| it('Verifying if Parsers install snippet is visible', () => { | ||
| page.verifyElementContainsText('code', toolsData.parsers.install); | ||
| }); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: This file uses 4 spaces for indentation in the beforeEach and test blocks, while other test files use 2 spaces (e.g., tools_generator.cy.js, tools_modelina.cy.js). Maintain consistent indentation throughout the codebase.
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.parsers.path); | |
| }); | |
| it('Verifying if the Parsers header is visible', () => { | |
| page.verifyHeadingExists(toolsData.parsers.heading); | |
| }); | |
| it('Verifying if Parsers GitHub link works', () => { | |
| page.verifyButtonLink(toolsData.parsers.github, 'View on Github'); | |
| }); | |
| it('Verifying if Parsers install snippet is visible', () => { | |
| page.verifyElementContainsText('code', toolsData.parsers.install); | |
| }); | |
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.parsers.path); | |
| }); | |
| it('Verifying if the Parsers header is visible', () => { | |
| page.verifyHeadingExists(toolsData.parsers.heading); | |
| }); | |
| it('Verifying if Parsers GitHub link works', () => { | |
| page.verifyButtonLink(toolsData.parsers.github, 'View on Github'); | |
| }); | |
| it('Verifying if Parsers install snippet is visible', () => { | |
| page.verifyElementContainsText('code', toolsData.parsers.install); | |
| }); |
| import toolsData from '../fixtures/toolsPages.json'; | ||
|
|
||
| class ToolsGenerator extends BasePage { | ||
|
|
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line should be removed. This blank line between the class definition and the method is unnecessary and inconsistent with other page objects in the codebase (see toolsModelina.js, BasePage.js, etc.).
| verifyTryItNowLink() { | ||
| return this.verifyLink(toolsData.modelina.tryIt.href, toolsData.modelina.tryIt.text, { findByText: true }); | ||
| } | ||
|
|
||
| verifyInstallSnippet() { | ||
| return this.verifyElementContainsText('code', toolsData.modelina.install); | ||
| } |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: This file uses 4 spaces for indentation, while other page objects in the codebase use 2 spaces (e.g., BasePage.js, toolsGenerator.js). The indentation should be consistent across all files in the project.
| verifyTryItNowLink() { | |
| return this.verifyLink(toolsData.modelina.tryIt.href, toolsData.modelina.tryIt.text, { findByText: true }); | |
| } | |
| verifyInstallSnippet() { | |
| return this.verifyElementContainsText('code', toolsData.modelina.install); | |
| } | |
| verifyTryItNowLink() { | |
| return this.verifyLink(toolsData.modelina.tryIt.href, toolsData.modelina.tryIt.text, { findByText: true }); | |
| } | |
| verifyInstallSnippet() { | |
| return this.verifyElementContainsText('code', toolsData.modelina.install); | |
| } |
| const page = new BasePage(); | ||
|
|
||
| beforeEach(() => { | ||
| cy.viewport(1280, 720); | ||
| page.visit(toolsData.githubActions.path); | ||
| }); | ||
|
|
||
| it('Verifying if the GitHub Actions header is visible', () => { | ||
| page.verifyHeadingExists(toolsData.githubActions.heading); | ||
| }); | ||
|
|
||
| it('Verifying if GitHub Actions GitHub link works', () => { | ||
| page.verifyButtonLink(toolsData.githubActions.github, 'View on Github'); | ||
| }); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: This file uses 4 spaces for indentation in the beforeEach and test blocks, while other test files use 2 spaces (e.g., tools_generator.cy.js, tools_modelina.cy.js). Maintain consistent indentation throughout the codebase.
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.githubActions.path); | |
| }); | |
| it('Verifying if the GitHub Actions header is visible', () => { | |
| page.verifyHeadingExists(toolsData.githubActions.heading); | |
| }); | |
| it('Verifying if GitHub Actions GitHub link works', () => { | |
| page.verifyButtonLink(toolsData.githubActions.github, 'View on Github'); | |
| }); | |
| const page = new BasePage(); | |
| beforeEach(() => { | |
| cy.viewport(1280, 720); | |
| page.visit(toolsData.githubActions.path); | |
| }); | |
| it('Verifying if the GitHub Actions header is visible', () => { | |
| page.verifyHeadingExists(toolsData.githubActions.heading); | |
| }); | |
| it('Verifying if GitHub Actions GitHub link works', () => { | |
| page.verifyButtonLink(toolsData.githubActions.github, 'View on Github'); | |
| }); |
| it('verifying install snippet', () => { | ||
| page.verifyElementContainsText('code', toolsData.modelina.install); | ||
| }); | ||
|
|
||
| it('Verifiying the GitHub button link', () => { | ||
| page.verifyButtonLink(toolsData.modelina.github, 'View on Github'); | ||
| }); | ||
|
|
||
| it('Verifying the Try it now link', () => { | ||
| page.verifyLink(toolsData.modelina.tryIt.href, toolsData.modelina.tryIt.text, { findByText: true }); | ||
| }); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ToolsModelina page object defines verifyTryItNowLink() and verifyInstallSnippet() methods, but the test directly calls page.verifyLink() and page.verifyElementContainsText() instead of using these page-specific methods. This breaks the page object pattern and makes the defined methods in toolsModelina.js unused. Consider using page.verifyTryItNowLink() and page.verifyInstallSnippet() instead to maintain proper abstraction.
| const page = new BasePage(); | ||
|
|
||
| beforeEach(() => { | ||
| cy.viewport(1280, 720); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The viewport setting cy.viewport(1280, 720) is only included in some tool page tests (CLI, GitHub Actions, Parsers) but not in others (Generator, Modelina). This inconsistency could lead to different test behaviors. Either all tool page tests should set the viewport explicitly, or none should (relying on Cypress defaults or global configuration).
| cy.viewport(1280, 720); |
Description
Related issue(s)
Summary by CodeRabbit
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.