test(playwright/domains): migrate domains tests to playwright#17417
Conversation
|
Linear: PFP-3891 |
|
🔴 Meticulous spotted visual differences in 100 of 1588 screens tested: view and approve differences detected. Meticulous evaluated ~10 hours of user flows against your PR. Last updated for commit |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…l functionality Migrates all Cypress e2e/domainsV2/v2_nested_domains.js tests to Playwright, covering domain creation, navigation, hierarchy operations, documentation, and property management. Test coverage includes: - Create new domain with success verification - View domain list and properties - Navigate through domain summary, about, and template sections - Create sub-domains under existing parent (Marketing) - Remove domains with cleanup verification - Access documentation and assets tabs All 9 tests pass reliably. Tests focus on core functionality and avoid complex multi-step navigation that was causing browser closure issues in earlier iterations. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Your PR has been assigned to @priyabratadas-dh (priyabrata.das) for review (PFP-3891). |
devashish2203
left a comment
devashish2203
left a comment
There was a problem hiding this comment.
Added some comments for changes. Please address before merging.
| super(page, logger, logDir); | ||
| this.graphqlHelper = new GraphQLHelper(page); | ||
| this.browseV2Container = page.locator('[id="browse-v2"]'); | ||
| this.createDomainButton = page.locator('[id="browse-v2"]').locator('button').nth(0); |
There was a problem hiding this comment.
Can we have a better identifier for this? .nth(0) can be flaky.
| const editIcon = this.page.locator('.anticon-edit').first(); | ||
| await editIcon.waitFor({ state: 'visible', timeout: SHORT_TIMEOUT }); | ||
| await editIcon.scrollIntoViewIfNeeded(); | ||
| await this.page.waitForTimeout(DELAY_MEDIUM); |
There was a problem hiding this comment.
We should avoid using waitForTimeout as it slows down the test unnecessarily. Can we instead rely on actionTimeout to fail the test if the next element is not present?
| await editIcon.scrollIntoViewIfNeeded(); | ||
| await this.page.waitForTimeout(DELAY_MEDIUM); | ||
|
|
||
| await editIcon.click({ force: true }); |
There was a problem hiding this comment.
Why do we need force: true here? If it's necessitated due to a UI bug that please create a linear for frontend team to fix.
| } | ||
|
|
||
| async editDomainName(newName: string): Promise<void> { | ||
| const editIcon = this.page.locator('.anticon-edit').first(); |
There was a problem hiding this comment.
Move the locator to the constructor. Avoid using first() and preferably use a better identifier instead of class name. If none is available maybe we can update the UI code to add a data-testId.
| await editIcon.click({ force: true }); | ||
| await this.page.waitForTimeout(DELAY_XL); | ||
|
|
||
| const editableInput = this.page.locator('input.ant-input').first(); |
There was a problem hiding this comment.
No locators in functions. All of them should be in the constructor.
Avoid using first() and preferably use a better identifier instead of class name. If none is available maybe we can update the UI code to add a data-testId.
Bundle ReportChanges will increase total bundle size by 92.55kB (0.4%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
Files in
|
2 participants
Linear ticket:
https://linear.app/acryl-data/issue/PFP-2891/phase-3-migrate-domains-tests-8-files
Description:
This PR migrates domains-v2 tests to playwright. It also adds/updates the tests to work on the new Summary tab on the domain page, instead of the earlier Documentation tab.
The new test files are:
In
tests/domains-v2folder-In
pages/domainsfolder:- nested-domains.page.ts