test: implement regression tests for in conversation search [WPB-19955]#20492
test: implement regression tests for in conversation search [WPB-19955]#20492JacquelineLehner wants to merge 3 commits intodevfrom
Conversation
…ia collection This commit introduces a new test suite to verify the in-conversation search and media collection functionality. Implemented scenarios include: - Verifying the media overview for images, audio, and files (@TC-352, @TC-356, @TC-357, @TC-359). - Searching for specific messages, links, and special characters, as well as testing search string trimming (@TC-385, @TC-391, @TC-398, @TC-403, @TC-405). - Verifying that clicking a search result scrolls the relevant message into view (@TC-408). - Ensuring deleted messages and media do not appear in search results or the media collection (@TC-360, @TC-392). Note: Tests @TC-358 (overview of all links) and @TC-388 (text mixed with link preview) are currently skipped due to the known bug [WPB-22484]. Refs: WPB-19955
JacquelineLehner
requested review from
V-Gira,
arjita-mitra,
e-maad,
mathias-niboulies,
nkaramarko,
otto-the-bot,
screendriver,
thisisamir98 and
zskhan
as code owners
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #20492 +/- ##
==========================================
- Coverage 45.38% 45.38% -0.01%
==========================================
Files 1637 1637
Lines 40370 40370
Branches 8337 8337
==========================================
- Hits 18323 18322 -1
Misses 20112 20112
- Partials 1935 1936 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
specs/Accessibility/Accessibility.spec.ts (❌ 0 failed,
|
apps/webapp/test/e2e_tests/specs/InConversationSearch/inConversationSearch.spec.ts
Show resolved
Hide resolved
apps/webapp/test/e2e_tests/specs/InConversationSearch/inConversationSearch.spec.ts
Show resolved
Hide resolved
apps/webapp/test/e2e_tests/specs/InConversationSearch/inConversationSearch.spec.ts
Show resolved
Hide resolved
| await userAPages.collection().page.getByRole('presentation').click(); | ||
| await expect(userAPages.collection().page.getByRole('presentation')).toBeVisible(); |
There was a problem hiding this comment.
This locator looks for an item with the role "presentation" on the whole page, this will fail due to strict mode as there are multiple such items on the page. Please limit the area you're searching and don't use the whole page. E.g.: userAPages.collection().imagesSection.getByRole('presentation').first()
You're also just clicking the image and checking that it's visible. I don't think this check is sufficient to verify the image was opened in the full screen detail view.
apps/webapp/test/e2e_tests/specs/InConversationSearch/inConversationSearch.spec.ts
Show resolved
Hide resolved
| for (let i = 0; i < 20; i++) { | ||
| await userAPages.conversation().sendMessage(`Message from User A: ${i}`); | ||
| } |
There was a problem hiding this comment.
Tipp: Instead of sending multiple messages you could also just send one very long message containing line breaks ;)
| for (let i = 0; i < 20; i++) { | |
| await userAPages.conversation().sendMessage(`Message from User A: ${i}`); | |
| } | |
| await pages.conversation().sendMessage('Empty\n'.repeat(20)); |
| await userAPages.conversation().sendMessage(`Message from User A: ${i}`); | ||
| } | ||
|
|
||
| await expect(userAPages.conversation().getMessage({content: 'Message from User A: 19'})).toBeVisible(); |
There was a problem hiding this comment.
Instead of asserting the last message is visible I'd suggest to rather check the first message is not visible as that's the state you're trying to reach here.
| get imagesSection() { | ||
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Images'})}); | ||
| } | ||
|
|
||
| get audioSection() { | ||
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Audio'})}); | ||
| } | ||
|
|
||
| get filesSection() { | ||
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Files'})}); | ||
| } | ||
|
|
||
| get overviewImagesButton() { | ||
| return this.imagesSection.getByRole('button', {name: /Show all/i}); | ||
| } | ||
|
|
||
| get overviewFilesButton() { | ||
| return this.filesSection.getByRole('button', {name: /Show all/i}); | ||
| } |
There was a problem hiding this comment.
What happens if there's a new type of section, e.g. the one for the links? -> You'd have to add two new functions, one to get the section and one for the show all button. This is just code duplication and not very scaleable.
Here's a trick how you can combine all of this into one small package, by leveraging Object.assign:
| get imagesSection() { | |
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Images'})}); | |
| } | |
| get audioSection() { | |
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Audio'})}); | |
| } | |
| get filesSection() { | |
| return this.component.locator('section').filter({has: this.page.locator('header', {hasText: 'Files'})}); | |
| } | |
| get overviewImagesButton() { | |
| return this.imagesSection.getByRole('button', {name: /Show all/i}); | |
| } | |
| get overviewFilesButton() { | |
| return this.filesSection.getByRole('button', {name: /Show all/i}); | |
| } | |
| getSection(type: 'Images' | 'Audio' | 'Files') { | |
| const section = this.component.locator('section').filter({has: this.page.locator('header', {hasText: type})}); | |
| return Object.assign(section, { | |
| showAllButton: section.getByRole('button', {name: 'Show all'}), | |
| }); | |
| } |
Instead of returning only the locator for the wanted section, we add a new property showAllButton to it and then return it. This way the returned locator can still be used like any other e.g. inside an expect or to chain off other locators from it. But we can also access the showAllButton using getSection('Images').showAllButton on it. A neat trick for when you want to add util to something returned by an other util but it's not worth creating a whole new POM for it ;)
| return this.filesSection.getByRole('button', {name: /Show all/i}); | ||
| } | ||
|
|
||
| get fullSearchBar() { |
There was a problem hiding this comment.
❓ Why "fullSearchBar" and not just "searchBar"?
| * Returns a locator for the highlighted (marked) text within the search results. | ||
| * @param searchTerm The text you expect to be highlighted. | ||
| */ | ||
| getMarkedSearchResult(searchTerm: string): Locator { |
There was a problem hiding this comment.
I think it makes a util useless if you need to pass in what you're searching for. Maybe a locator for all searchResults in general could be more useful. Then we could assert on the number of results, or filter them or find the marked texts inside. :)
This would also replace the
resultTexts()util below as we could just assert on the text in the results in general instead of having a util for it.
3 participants
Pull Request
Summary
Security Checklist (required)
Accessibility (required)
Standards Acknowledgement (required)
Screenshots or demo (if the user interface changed)
Notes for reviewers