-
Notifications
You must be signed in to change notification settings - Fork 295
test(e2e): implement account settings regression tests [WPB-19935] #19540
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: dev
Are you sure you want to change the base?
test(e2e): implement account settings regression tests [WPB-19935] #19540
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #19540 +/- ##
==========================================
- Coverage 43.66% 43.65% -0.01%
==========================================
Files 1294 1294
Lines 32318 32318
Branches 7177 7177
==========================================
- Hits 14111 14110 -1
Misses 16515 16515
- Partials 1692 1693 +1 🚀 New features to boost your workflow:
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
Failed Tests:❌ Calls in channels with device switch and screenshare (tags: TC-8754, crit-flow-web)Location: specs/CriticalFlow/channelsCall-TC-8755.spec.ts:38 Errors:
❌ Channels Management (tags: TC-8752, crit-flow-web)Location: specs/CriticalFlow/channelsManagement-TC-8752.spec.ts:36 Errors:
❌ Planning group call with sending various messages during call (tags: TC-8632, crit-flow-web)Location: specs/CriticalFlow/groupCalls-TC-8632.spec.ts:37 Errors:
❌ Group Video call (tags: TC-8637, crit-flow-web)Location: specs/CriticalFlow/groupVideoCall-TC-8637.spec.ts:39 Errors:
❌ Messages in Channels (tags: TC-8753, crit-flow-web)Location: specs/CriticalFlow/messagesInChannels-TC-8753.spec.ts:44 Errors:
Flaky Tests: |
1ba75cd
to
7ef30fa
Compare
for the code duplication. i wanna refactor all modals but the tests are failing so i cannot test it in this case. |
one note the vars: |
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.
Well done 🚀
Added few comments, you can decide it you want to cater them in next PR or not cater at all.
constructor(page: Page, modalLocator: string) { | ||
this.page = page; | ||
this.modal = page.locator(modalLocator); | ||
this.modalTitle = this.modal.locator("[data-uie-name='status-modal-title']"); |
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.
Maybe we can also replace these with getByTestId.
…egression-tests-in-playwright
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.
Added a few comments
this.allConverationsButton = page.locator(`${selectByDataAttribute('go-recent-view')}`); | ||
this.connectButton = page.locator(`button${selectByDataAttribute('go-people')}`); | ||
this.archiveButton = page.locator(selectByDataAttribute('go-archive')); | ||
this.manageTeamButton = page.locator(selectByDataAttribute('go-team-management')); |
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 doesn't make sense to have both getByTestId
and selectByDataAttribute
because they both locate elements by data-uie-name
.
test/e2e_tests/pageManager/webapp/components/conversationSidebar.component.ts
Show resolved
Hide resolved
|
||
export class ErrorModal extends BaseModal { | ||
constructor(page: Page) { | ||
super(page, "[data-uie-name='primary-modals-container']"); |
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 guess it doesn't make much sense since a lot of modals we have utilise the same [data-uie-name='primary-modals-container']
. We either need to distinguish modals with some other means or add unique identifiers. @e-maad what do you think?
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 agree with @iskvortsov, we should add unique identifiers.
…egression-tests-in-playwright
…egression-tests-in-playwright
…egression-tests-in-playwright
|
Description
Checklist