Improve playwright ci test #205
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis update introduces a structured approach to Playwright testing by defining page objects and fixtures for various elements across web pages, enhancing test maintainability and readability. It also sets a global expectation timeout, refactors tests into more manageable parts, and introduces new tests for specific site pages. Changes
Related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
…ght-ci-test-draft
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (26)
- playwright/ci-test/playwright.config.ts (1 hunks)
- playwright/ci-test/tests/01-home-page.spec.ts (1 hunks)
- playwright/ci-test/tests/02-download-page.spec.ts (1 hunks)
- playwright/ci-test/tests/03-product-page.spec.ts (1 hunks)
- playwright/ci-test/tests/04-community-page.spec.ts (1 hunks)
- playwright/ci-test/tests/05-funding-page.spec.ts (1 hunks)
- playwright/ci-test/tests/06-resources-page.spec.ts (1 hunks)
- playwright/ci-test/tests/07-support-pages.spec.ts (1 hunks)
- playwright/ci-test/tests/fixtures/books-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/bug-reporting-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/community-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/download-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/faq-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/footer.ts (1 hunks)
- playwright/ci-test/tests/fixtures/funding-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/header.ts (1 hunks)
- playwright/ci-test/tests/fixtures/home-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/installation-guide-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/interfaces.ts (1 hunks)
- playwright/ci-test/tests/fixtures/product-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/qgis-resources-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/releases-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/report-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/roadmap-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/sidebar.ts (1 hunks)
- playwright/ci-test/tests/fixtures/support-page.ts (1 hunks)
Files not summarized due to errors (1)
- playwright/ci-test/tests/06-resources-page.spec.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- playwright/ci-test/tests/fixtures/interfaces.ts
Additional comments: 39
playwright/ci-test/tests/fixtures/releases-page.ts (1)
- 3-24: - The
ReleasesPageclass is well-structured, encapsulating locators and expected text strings for the releases page. This approach enhances test readability and maintainability.
- Using
getByRolefor locators is a good practice for accessibility and robustness against UI changes.- The
textListarray contains hardcoded strings expected to be found on the page. While this is useful for assertions, consider the maintainability of this approach if the website content changes frequently.playwright/ci-test/tests/fixtures/books-page.ts (1)
- 3-31: - The
BooksPageclass follows a consistent pattern with other fixture classes, which is good for codebase consistency.
- The use of
getByRolefor thecontactLinklocator is commendable for the same reasons mentioned in theReleasesPagereview.- The
textListarray, as before, could pose maintainability challenges. Consider fetching these strings from a centralized or external source if they change often.playwright/ci-test/tests/fixtures/report-page.ts (1)
- 3-33: - The
ReportsPageclass is consistent with the structure and practices observed in other fixture classes, which is good for maintainability.
- The use of specific locators like
certificationImageandogcCertificationLinkis well thought out, focusing on key elements of the reports page.- As with other pages, the
textListarray's hardcoding might require frequent updates. Consider strategies for easier maintenance of these expected strings.playwright/ci-test/tests/fixtures/download-page.ts (1)
- 3-45: - The
DownloadPageclass effectively encapsulates elements related to the download process, enhancing test readability.
- The use of
getByTextfor buttons liketenEurosButtonis appropriate given the context, but consider the impact of localization or currency format changes on these selectors.- The
urlproperty is set to "/", which might be a placeholder. Ensure this aligns with the actual path of the download page in the application.playwright/ci-test/tests/02-download-page.spec.ts (1)
- 1-54: - The refactoring to use fixture classes (
Sidebar,DownloadPage,HomePage) in02-download-page.spec.tsenhances the test's structure and readability.
- The use of
expectassertions withtoBeVisibleis appropriate for verifying the visibility of elements on the page.- Consider adding comments or documentation to describe the purpose of each test block or significant steps within the test for better readability and maintainability.
playwright/ci-test/playwright.config.ts (1)
- 17-20: - Setting the expect timeout to 15 seconds in
playwright.config.tsis a thoughtful adjustment to accommodate tests that may require more time due to network latency or heavy processing.
- Ensure that this timeout value is balanced to avoid excessively long test runs while still allowing for necessary wait times.
playwright/ci-test/tests/03-product-page.spec.ts (1)
- 1-71: - The restructuring of
03-product-page.spec.tsto use fixture classes (HomePage,Sidebar,ProductPage) is a significant improvement, making the test more organized and readable.
- The loop through
productPage.productTextListto assert text presence is a good use of dynamic testing based on the content of the fixture class.- As with the previous test file, consider adding comments or documentation for clarity on the test's purpose and key steps.
playwright/ci-test/tests/fixtures/support-page.ts (2)
- 3-36: - The
SupportPageclass effectively encapsulates elements and expected text strings for the support page, following the established pattern of fixture classes.
- The
urlproperty is correctly set to the support page's path, which is a good practice for page-specific fixtures.
- 39-89: - The
OtherSupportPageclass introduces additional locators and actions for navigating to different sections of the support page, enhancing test flexibility.
- The methods like
navigateToCommunicationChannelsandnavigateToCommercialSupportare well-named, clearly indicating their purpose.- Consider the potential need for updating the
textListarray and locators if the website's structure or content changes.playwright/ci-test/tests/fixtures/roadmap-page.ts (1)
- 3-84: - The
RoadmapPageclass is well-structured, encapsulating key elements of the project's roadmap page, which will facilitate targeted assertions in tests.
- The use of
getByRolefor locators likeeventandlatestis consistent with accessibility best practices and robust test design.- As with other fixture classes, consider the maintainability of the
textListarray if the roadmap content changes frequently.playwright/ci-test/tests/fixtures/faq-page.ts (1)
- 3-84: - The
FaqPageclass effectively encapsulates elements and expected text strings for the FAQ page, following the pattern established by other fixture classes.
- The inclusion of locators for specific links like
faqLinkandqgisIssueTrackerLinkis useful for navigating within the FAQ page during tests.- The
textListarray contains a comprehensive list of expected FAQ content. As with other pages, consider how to manage this list for maintainability.playwright/ci-test/tests/fixtures/qgis-resources-page.ts (1)
- 3-88: - The class
QgisResourcesPageis well-structured and provides a clear encapsulation of the elements on the QGIS Resources page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor URL and locators ensures that these properties are not modified after initialization, which is a good practice for test stability.- The
gotomethod (lines 85-87) simplifies navigation to the resources page, which is likely to be a common step in multiple tests. This method enhances test readability and reusability.playwright/ci-test/tests/05-funding-page.spec.ts (2)
- 10-39: - The fixture setup (lines 10-39) is well-organized and provides a clear structure for initializing page objects required for the funding page tests. This setup enhances test readability and maintainability by abstracting page details into fixture classes.
- Using
extendto create a custom test fixture with all necessary page objects is a good practice. It ensures that each test has access to the required page objects without needing to initialize them in every test case.
- 41-103: - The test cases are well-structured and focus on validating the visibility of elements and the presence of specific texts on the funding page. This approach ensures that the page's critical components are rendered correctly and contain the expected content.
- The use of
expectassertions to check for element visibility and text content is appropriate for these test scenarios. These assertions provide clear feedback when a test fails, making it easier to diagnose issues.- The dynamic retrieval of text lists based on the page being tested (e.g., lines 53-61 for the "Donate" page and lines 68-76 for the "Membership" page) is a clever way to organize test data. This approach keeps the tests clean and focused while allowing for easy updates to the test data.
playwright/ci-test/tests/fixtures/header.ts (1)
- 3-100: - The class
Headeris well-structured and provides a clear encapsulation of the elements in the header section of the web page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor locators ensures that these properties are not modified after initialization, which is a good practice for test stability.- The constructor (lines 32-100) efficiently initializes locators for various links in the header section. Using
getByRoleandgetByTextmethods for locating elements is a good practice as it makes the tests more accessible and aligns with web standards.playwright/ci-test/tests/fixtures/product-page.ts (1)
- 3-109: - The class
ProductPageis well-structured and provides a clear encapsulation of the elements on the Product page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor locators and text lists ensures that these properties are not modified after initialization, which is a good practice for test stability.- The organization of text lists into separate arrays for different sections of the page (e.g.,
productTextList,mapsTextList,screenshotsTextList,caseStudiesTextList,visualChangelogsTextList) is a good practice. It allows for easy maintenance and updates to the test data.playwright/ci-test/tests/fixtures/sidebar.ts (1)
- 3-126: - The class
Sidebaris well-structured and provides a clear encapsulation of the elements in the sidebar section of the web page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor locators ensures that these properties are not modified after initialization, which is a good practice for test stability.- The constructor (lines 31-126) efficiently initializes locators for various links in the sidebar section. Using
getByRolewith specific names for locating elements is a good practice as it makes the tests more accessible and aligns with web standards.playwright/ci-test/tests/07-support-pages.spec.ts (2)
- 7-36: - The fixture setup (lines 7-36) is well-organized and provides a clear structure for initializing page objects required for the support pages tests. This setup enhances test readability and maintainability by abstracting page details into fixture classes.
- Using
extendto create a custom test fixture with all necessary page objects is a good practice. It ensures that each test has access to the required page objects without needing to initialize them in every test case.
- 38-108: - The test cases are well-structured and focus on validating the visibility of elements and the presence of specific texts on the support pages. This approach ensures that the page's critical components are rendered correctly and contain the expected content.
- The use of
expectassertions to check for element visibility and text content is appropriate for these test scenarios. These assertions provide clear feedback when a test fails, making it easier to diagnose issues.- The dynamic retrieval of text lists based on the page being tested (e.g., lines 47-49 for the "Support" page and lines 65-67 for the "FAQ" page) is a clever way to organize test data. This approach keeps the tests clean and focused while allowing for easy updates to the test data.
playwright/ci-test/tests/fixtures/bug-reporting-page.ts (1)
- 3-114: - The class
BugReportingPageis well-structured and provides a clear encapsulation of the elements on the Bug Reporting page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor locators and text lists ensures that these properties are not modified after initialization, which is a good practice for test stability.- The organization of the
textListarray (lines 20-62) to include expected texts for the Bug Reporting page is a good practice. It allows for easy maintenance and updates to the test data.playwright/ci-test/tests/fixtures/funding-page.ts (1)
- 4-125: - The classes
FundingPage,MembersPage, andPastMembersPageare well-structured and provide clear encapsulations of the elements on their respective pages. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor URLs, locators, and text lists ensures that these properties are not modified after initialization, which is a good practice for test stability.- The organization of text lists into separate arrays for different sections of the funding page (e.g.,
textListinFundingPage) is a good practice. It allows for easy maintenance and updates to the test data.- The
gotomethod inFundingPage(lines 93-95) simplifies navigation to the funding page, which is likely to be a common step in multiple tests. This method enhances test readability and reusability.playwright/ci-test/tests/fixtures/footer.ts (1)
- 3-128: - The class
Footeris well-structured and provides a clear encapsulation of the elements in the footer section of the web page. This approach enhances test readability and maintainability by abstracting page details into fixture classes.
- The use of
readonlyfor locators ensures that these properties are not modified after initialization, which is a good practice for test stability.- The constructor (lines 46-128) efficiently initializes locators for various links in the footer section. Using
getByRolewith specific names for locating elements is a good practice as it makes the tests more accessible and aligns with web standards.playwright/ci-test/tests/01-home-page.spec.ts (6)
- 1-4: The import statements are correctly updated to use aliases and import the necessary fixture classes. This improves readability and maintainability by clearly indicating the purpose of each import.
- 6-10: The definition of
HomePageFixturestype is a good practice as it explicitly declares the fixtures available for tests in this file, enhancing code readability and maintainability.- 12-25: The extension of the base test with
HomePageFixturesis well-implemented. Using async functions to initialize each fixture ensures that they are properly set up before use. This pattern promotes code reuse and simplifies test case structure.- 27-63: The "Header" test section is well-organized and makes effective use of the header fixture. The assertions are descriptive and cover a wide range of elements, ensuring comprehensive testing of the header functionality. Consider adding a comment explaining the rationale behind the specific elements being tested, to provide context for future maintainers.
- 65-94: The "Content" test section is clearly structured and leverages the homePage fixture effectively. The assertions are descriptive, ensuring that key elements of the home page content are visible and as expected. This approach enhances test readability and maintainability.
- 97-137: The "Footer" test section is comprehensive, with assertions covering all key elements of the footer. This thorough testing ensures that the footer's functionality and visibility are as expected. The use of the footer fixture streamlines the test code and improves readability.
playwright/ci-test/tests/fixtures/community-page.ts (3)
- 1-3: The import statements are correctly organized, importing only the necessary types and interfaces from Playwright and other fixtures. This keeps the code clean and focused on its purpose.
- 4-223: The
CommunityPageclass is well-structured, with a clear separation of properties for different page elements. The use of Playwright'sLocatorand role-based selectors enhances test reliability by targeting elements in a more semantic and robust manner. Consider adding brief comments above each group of related locators (e.g., donation links, meeting links) to improve readability.- 38-133: The
textListarray is a creative approach to organize texts expected to be found on different sections of the community page. This structure facilitates easier maintenance and updates to the test content. However, ensure that the texts are kept up-to-date with the actual content on the website to prevent false negatives in tests.playwright/ci-test/tests/04-community-page.spec.ts (3)
- 1-5: The import statements correctly import the necessary fixtures and base test from Playwright. This setup ensures that the test file has access to the required functionalities and fixtures for the community page tests.
- 12-25: The extension of the base test with
CommunityPageFixturesis correctly implemented, using async functions to initialize each fixture. This setup promotes code reuse and simplifies the structure of test cases.- 27-237: The test cases are well-organized into sections corresponding to different parts of the community page. This structure, along with the use of fixtures, enhances the readability and maintainability of the tests. However, consider adding comments to explain the purpose of each test section for better clarity.
playwright/ci-test/tests/06-resources-page.spec.ts (4)
- 1-8: The import statements are correctly organized, importing the necessary fixtures and base test from Playwright. This setup ensures that the test file has access to the required functionalities and fixtures for the resources page tests.
- 10-18: The definition of
ResourcesPageFixturesis well-structured, explicitly declaring the fixtures available for tests in this file. This enhances code readability and maintainability by clearly indicating the purpose of each fixture.- 20-49: The extension of the base test with
ResourcesPageFixturesis correctly implemented, using async functions to initialize each fixture. This setup promotes code reuse and simplifies the structure of test cases, ensuring that fixtures are properly set up before use.- 51-267: The test cases are well-organized into sections corresponding to different resources pages. This structure, along with the use of fixtures, enhances the readability and maintainability of the tests. However, consider adding comments to explain the purpose of each test section for better clarity, especially for future maintainers or contributors who might be less familiar with the structure of the resources page.
playwright/ci-test/tests/fixtures/installation-guide-page.ts (1)
- 1-383: The class
InstallationGuidePageis well-structured and follows good practices for a page object model, encapsulating the locators and interactions with the Installation Guide Page. However, there are a few areas where improvements can be made:
Consistency in Locator Initialization: Most locators are initialized directly within the constructor, which is fine for a class of this size. However, for larger page objects or if this class were to grow, consider initializing locators in separate methods to keep the constructor concise and improve readability.
Error Handling: While not directly related to the changes, it's important to consider error handling for scenarios where elements might not be found or when interactions fail. This can be part of the test setup or teardown processes, ensuring tests fail gracefully and provide clear error messages.
Documentation: Adding comments to describe the purpose of each locator or group of locators can greatly improve maintainability. This is especially useful for complex pages or when locators have specific roles that aren't immediately obvious from their names.
Use of
filterandgetByRole: The use offilterandgetByRoleis appropriate and leverages Playwright's capabilities to precisely locate elements. Ensure that the selectors and roles used are stable and unlikely to change frequently to avoid flaky tests.Overall, the implementation aligns well with the objectives of enhancing the Playwright CI testing setup by introducing structured and readable tests through the use of fixture classes.
Comment on lines
+3
to
+156
| "Explore a diverse ecosystem", | ||
| "QGIS provides an equal", | ||
| "Industry-leading format", | ||
| "Conquer data integration", | ||
| "Standards and interoperability", | ||
| "Amplify your impact by", | ||
| "Publish your work", | ||
| "Extend QGIS to the cloud and", | ||
| "Our community development", | ||
| "Join our annual international", | ||
| "Find local user groups and", | ||
| "Learn how people around the", | ||
| "QGIS is a public project", | ||
| "For that reason QGIS is Free", | ||
| "Quick-start tutorials", | ||
| "Live demos", | ||
| "Up-to-date documentation", | ||
| ]; | ||
|
|
||
| constructor(public readonly page: Page) { | ||
| this.pageBody = this.page.locator("body"); | ||
| this.freeOpenSourceSpatialDiv = this.page | ||
| .locator("div") | ||
| .filter({ hasText: "Free and open source Spatial" }) | ||
| .first(); | ||
| this.downloadLink = this.page | ||
| .locator("section") | ||
| .filter({ hasText: "Free and open source Spatial" }) | ||
| .getByRole("link"); | ||
| this.spatialWithoutCompromiseHeading = this.page.getByRole("heading", { | ||
| name: "Spatial without compromise", | ||
| }); | ||
| this.createMapsHeading = this.page.getByRole("heading", { | ||
| name: "Create maps", | ||
| }); | ||
| this.editLayersHeading = this.page.getByRole("heading", { | ||
| name: "Edit layers", | ||
| }); | ||
| this.processAndAnalyzeHeading = this.page.getByRole("heading", { | ||
| name: "Process and analyze", | ||
| }); | ||
| this.shareMapsHeading = this.page.getByRole("heading", { | ||
| name: "Share maps", | ||
| }); | ||
| this.exploreQGISLink = this.page.getByRole("link", { | ||
| name: "Explore QGIS", | ||
| }); | ||
| this.powerOfOpenSourceHeading = this.page.getByRole("heading", { | ||
| name: "👋 The power of open source", | ||
| }); | ||
| this.getInvolvedLink = this.page | ||
| .getByRole("link", { name: "Get involved" }) | ||
| .first(); | ||
| this.freeAndOpenSourceHeading = this.page.getByRole("heading", { | ||
| name: "Free and open source", | ||
| }); | ||
| this.freeDownloadLink = this.page.getByRole("link", { | ||
| name: "Free download", | ||
| }); | ||
| this.youTubeVideoThumbnailOverlayImage = this.page | ||
| .frameLocator('iframe[title="YouTube video player"]') | ||
| .locator(".ytp-cued-thumbnail-overlay-image"); | ||
| this.startUsingQGISHeading = this.page.getByRole("heading", { | ||
| name: "Start using QGIS 🚀", | ||
| }); | ||
| this.goToMaterialsLink = this.page.getByRole("link", { | ||
| name: "Go to materials", | ||
| }); | ||
| this.qgisSupportersHeading = this.page.getByRole("heading", { | ||
| name: "QGIS supporters", | ||
| }); | ||
| this.addYourLogoHereText = this.page | ||
| .locator("div") | ||
| .filter({ hasText: "Add your logo here?" }) | ||
| .nth(2); | ||
| this.silverPartnerText = this.page | ||
| .locator("div") | ||
| .filter({ hasText: "Silver partner" }) | ||
| .nth(2); | ||
| this.supportersGridDiv = this.page | ||
| .locator(".supporters-grid > div:nth-child(3)") | ||
| .first(); | ||
| this.createMapsImg = this.page.locator(".deco-block-1 > img"); | ||
| this.editLayersImg = this.page.locator(".deco-block-2 > img"); | ||
| this.processImg = this.page.locator(".deco-block-3 > img"); | ||
| this.shareMapsImg = this.page.locator(".deco-block-4 > img"); | ||
| this.communityImg = this.page.locator( | ||
| ".explore > .columns > div > figure > img", | ||
| ); | ||
| this.changeLogVideo = this.page | ||
| .frameLocator('iframe[title="YouTube video player"]') | ||
| .locator(".ytp-cued-thumbnail-overlay-image"); | ||
| this.otherSupporters = this.page | ||
| .locator(".container > div:nth-child(3)") | ||
| .first(); | ||
| } | ||
|
|
||
| async goto() { | ||
| await this.page.goto(this.url); | ||
| } |
There was a problem hiding this comment.
The HomePage class is well-structured and provides a comprehensive set of locators for interacting with the home page elements. However, there are several areas where improvements can be made for better maintainability and readability:
-
Locator Strategy: The use of
.first()and.nth(2)for selecting elements might lead to fragile tests if the page structure changes. Consider using more specific selectors that are less likely to change, such as IDs or data attributes. -
Repeated Code: The
frameLocatorfor YouTube video player is repeated foryouTubeVideoThumbnailOverlayImageandchangeLogVideo. If the frame's locator or its purpose changes, you'll need to update it in multiple places. Consider abstracting this into a method or a property to DRY up the code. -
Text List Array: The
textListarray contains strings that seem to be used for assertions or checks within tests. While having them centralized is good, the array is quite large and might be better managed in a separate file or as a more structured object to improve readability and maintainability. -
Method Documentation: The
gotomethod is straightforward but lacks documentation. Adding a brief comment explaining its purpose, especially in a file that might be used by multiple team members, can improve code readability and maintainability. -
Error Handling: There's no visible error handling for the
gotomethod. While Playwright's methods typically throw errors on failure, handling these errors explicitly could provide more descriptive error messages or perform additional cleanup if needed.
Consider refining the locator strategy and abstracting repeated code. Additionally, evaluate the management of the textList array for better maintainability, and add method documentation and error handling where appropriate.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- playwright/ci-test/tests/04-community-page.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- playwright/ci-test/tests/04-community-page.spec.ts
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- playwright/ci-test/tests/02-download-page.spec.ts (1 hunks)
- playwright/ci-test/tests/04-community-page.spec.ts (1 hunks)
- playwright/ci-test/tests/05-funding-page.spec.ts (1 hunks)
- playwright/ci-test/tests/fixtures/community-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/donate-window.ts (1 hunks)
- playwright/ci-test/tests/fixtures/download-page.ts (1 hunks)
- playwright/ci-test/tests/fixtures/funding-page.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- playwright/ci-test/tests/02-download-page.spec.ts
- playwright/ci-test/tests/05-funding-page.spec.ts
- playwright/ci-test/tests/fixtures/community-page.ts
- playwright/ci-test/tests/fixtures/download-page.ts
- playwright/ci-test/tests/fixtures/funding-page.ts
Additional comments: 2
playwright/ci-test/tests/fixtures/donate-window.ts (1)
- 4-25: The
FundingPageclass is well-structured and follows good practices for Playwright page object models. The use ofpageBodyas a locator andtextListfor organizing text assertions by page sections enhances readability and maintainability.playwright/ci-test/tests/04-community-page.spec.ts (1)
- 1-240: The test script for the community page is well-refactored, utilizing fixtures for page elements, organizing tests by sections, and interacting with page elements through fixture methods. This enhances code readability and maintainability. The tests are structured and clear, following best practices for Playwright testing.
dqunbp
approved these changes
Mar 11, 2024
|
Much better now! Small refactoring advice: the pages can be inherited. There should be a base page with methods and selectors that are used everywhere, so that other pages could inherit from it. For example, the "goto" function was separately implemented on each page. But it's OK for the current scope |
sleeping-h
approved these changes
Mar 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a fix for #210
Summary by CodeRabbit
CommunityPage,FundingPage, andDownloadPageto interact with specific page elements and enhance test coverage.