Skip to content

Commit 593c16d

Browse files
markbrockhoffmarkbrockhoff
authored
test: remove util isMessageVisible() in favor of web first assertions (#19749)
This is a performance optimization for our tests. I noticed that sending a message takes a lot of time. Turns out the util always waits 5s after sending the message to ensure it was sent. I replaced this check with the existing `isMessageVisible` check since it only takes into account messages where the optimistic update already finished. This should shave of at least 4.5s of every message sent inside our tests.
1 parent 2aa79d8 commit 593c16d

12 files changed

+89
-107
lines changed

‎test/e2e_tests/pageManager/webapp/pages/conversation.page.ts‎

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,6 @@ export class ConversationPage {
152152
async sendMessage(message: string) {
153153
await this.messageInput.fill(message);
154154
await this.sendMessageButton.click();
155-
// Wait for the specific message to appear in the conversation
156-
const messageLocator = this.messages.filter({hasText: message}).last();
157-
await messageLocator.waitFor({state: 'visible', timeout: 20_000});
158155
}
159156

160157
async typeMessage(message: string) {
@@ -184,28 +181,6 @@ export class ConversationPage {
184181
await this.messageInput.press('Enter');
185182
}
186183

187-
async isMessageVisible(messageText: string, waitForVisibility = true) {
188-
if (waitForVisibility) {
189-
// Wait for the last message to be visible
190-
await this.messages.last().waitFor({state: 'visible', timeout: 20_000});
191-
}
192-
193-
// Then get all matching elements
194-
const messages = await this.messages.all();
195-
196-
for (const message of messages) {
197-
const messageTextContent = await message.locator(selectByClass('text')).textContent();
198-
if (messageTextContent?.trim() === messageText) {
199-
return true;
200-
}
201-
}
202-
return false;
203-
}
204-
205-
getMessageByText(messageText: string): Locator {
206-
return this.messageItems.filter({hasText: messageText});
207-
}
208-
209184
async isImageFromUserVisible(user: User) {
210185
// Trying multiple times for the image to appear
211186
const locator = this.getImageLocator(user);
@@ -227,17 +202,26 @@ export class ConversationPage {
227202
}
228203

229204
/**
230-
* Util to get a message in the conversation sent by a given user
231-
* @param messageContent Optional parameter to specify content the message should contain. If undefined the last message sent by the user will be returned.
205+
* Util to get a message in the conversation
206+
* @param options.content Only match messages containing this text
207+
* @param options.sender Only match messages send by this user
208+
* @returns a Locator to the matching message(s)
232209
*/
233-
getMessageFromUser(user: User, messageContent?: string) {
234-
const messagesFromUser = this.messageItems.filter({
235-
has: this.page.getByTestId('sender-name').getByText(user.fullName),
236-
});
237-
if (messageContent !== undefined) {
238-
return messagesFromUser.filter({hasText: messageContent});
210+
getMessage(options?: {content?: string | RegExp; sender?: User}): Locator {
211+
let message = this.messageItems;
212+
213+
if (options?.content) {
214+
message = message.filter({hasText: options.content});
239215
}
240-
return messagesFromUser.last();
216+
217+
if (options?.sender?.fullName) {
218+
message = message.filter({
219+
// Using getByLabel doesn't work here as the aria label is just placed on a div with no input inside which could be located
220+
has: this.page.locator(`.content-message-wrapper[aria-label*="${options.sender.fullName}"]`),
221+
});
222+
}
223+
224+
return message;
241225
}
242226

243227
/**

‎test/e2e_tests/specs/AccountSettingsSpecs/accountSettings.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ test.describe('account settings', () => {
241241
// check that the chat is open
242242
expect(await pages.conversationList().isConversationItemVisible(groupName)).toBeTruthy();
243243
await pages.conversation().sendMessage('test');
244-
const message = await pages.conversation().messageItems.nth(1); // skip the system messages
244+
const message = pages.conversation().getMessage({content: 'test', sender: memberA});
245245

246246
await expect(message.getByTestId('sender-name')).toHaveText(memberA.fullName);
247247

‎test/e2e_tests/specs/CriticalFlow/addMembersToChat-TC-8631.spec.ts‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -122,80 +122,76 @@ test(
122122
// Now all members can send and receive encrypted messages
123123
// Team owner sends a message
124124
await pages.conversation().sendMessage(`Hello from ${owner.firstName}!`);
125-
expect(await pages.conversation().isMessageVisible(`Hello from ${owner.firstName}!`)).toBeTruthy();
125+
await expect(pages.conversation().getMessage({content: `Hello from ${owner.firstName}!`})).toBeVisible();
126126

127127
// Member1 sends a message
128128
await member1PageManager.webapp.pages.conversation().sendMessage(`Hello from ${member1.firstName}!`);
129-
expect(
130-
await member1PageManager.webapp.pages.conversation().isMessageVisible(`Hello from ${member1.firstName}!`),
131-
).toBeTruthy();
129+
await expect(
130+
member1PageManager.webapp.pages.conversation().getMessage({content: `Hello from ${member1.firstName}!`}),
131+
).toBeVisible();
132132

133133
// Member2 sends a message
134134
await member2PageManager.webapp.pages.conversation().sendMessage(`Hello from ${member2.firstName}!`);
135-
expect(
136-
await member2PageManager.webapp.pages.conversation().isMessageVisible(`Hello from ${member2.firstName}!`),
137-
).toBeTruthy();
135+
await expect(
136+
member2PageManager.webapp.pages.conversation().getMessage({content: `Hello from ${member2.firstName}!`}),
137+
).toBeVisible();
138138

139139
// Owner verifies all messages are visible
140140
await pages.conversationList().openConversation(conversationName);
141-
expect(await pages.conversation().isMessageVisible(`Hello from ${member1.firstName}!`)).toBeTruthy();
142-
expect(await pages.conversation().isMessageVisible(`Hello from ${member2.firstName}!`)).toBeTruthy();
141+
await expect(pages.conversation().getMessage({content: `Hello from ${member1.firstName}!`})).toBeVisible();
142+
await expect(pages.conversation().getMessage({content: `Hello from ${member2.firstName}!`})).toBeVisible();
143143
});
144144

145145
await test.step('Team owner and group members react on received messages with reactions', async () => {
146146
// Owner reacts to member1's message with +1 (thumbs up)
147147
await pages.conversationList().openConversation(conversationName);
148-
const member1MessageForOwner = pages.conversation().getMessageByText(`Hello from ${member1.firstName}!`).first();
148+
const member1MessageForOwner = pages.conversation().getMessage({content: `Hello from ${member1.firstName}!`});
149149
await member1MessageForOwner.waitFor({state: 'visible'}); // Wait for message to be ready
150150
await pages.conversation().reactOnMessage(member1MessageForOwner, 'plus-one');
151151

152152
// Owner reacts to member2's message with +1 (thumbs up)
153-
const member2MessageForOwner = pages.conversation().getMessageByText(`Hello from ${member2.firstName}!`).first();
153+
const member2MessageForOwner = pages.conversation().getMessage({content: `Hello from ${member2.firstName}!`});
154154
await pages.conversation().reactOnMessage(member2MessageForOwner, 'plus-one');
155155

156156
// Member1 reacts to owner's message with heart (❤️)
157157
await member1PageManager.webapp.pages.conversationList().openConversation(conversationName);
158158
const ownerMessageForMember1 = member1PageManager.webapp.pages
159159
.conversation()
160-
.getMessageByText(`Hello from ${owner.firstName}!`)
161-
.first();
160+
.getMessage({content: `Hello from ${owner.firstName}!`});
162161
await member1PageManager.webapp.pages.conversation().reactOnMessage(ownerMessageForMember1, 'heart');
163162

164163
// Member1 reacts to member2's message with heart (❤️)
165164
const member2MessageForMember1 = member1PageManager.webapp.pages
166165
.conversation()
167-
.getMessageByText(`Hello from ${member2.firstName}!`)
168-
.first();
166+
.getMessage({content: `Hello from ${member2.firstName}!`});
169167
await member1PageManager.webapp.pages.conversation().reactOnMessage(member2MessageForMember1, 'heart');
170168

171169
// Member2 reacts to owner's message with joy (😂)
172170
await member2PageManager.webapp.pages.conversationList().openConversation(conversationName);
173171
const ownerMessageForMember2 = member2PageManager.webapp.pages
174172
.conversation()
175-
.getMessageByText(`Hello from ${owner.firstName}!`)
176-
.first();
173+
.getMessage({content: `Hello from ${owner.firstName}!`});
177174
await member2PageManager.webapp.pages.conversation().reactOnMessage(ownerMessageForMember2, 'joy');
178175

179176
// Member2 reacts to member1's message with joy (😂)
180177
const member1MessageForMember2 = member2PageManager.webapp.pages
181178
.conversation()
182-
.getMessageByText(`Hello from ${member1.firstName}!`)
183-
.first();
179+
.getMessage({content: `Hello from ${member1.firstName}!`});
184180
await member2PageManager.webapp.pages.conversation().reactOnMessage(member1MessageForMember2, 'joy');
185181
});
186182

187183
await test.step('All group participants make sure they see reactions from other group participants', async () => {
188184
// Owner verifies they can see heart (❤️) and joy (😂) reactions on their message from member1 and member2
189185
await pages.conversationList().openConversation(conversationName);
190-
const ownerMessage = pages.conversation().getMessageByText(`Hello from ${owner.firstName}!`);
186+
const ownerMessage = pages.conversation().getMessage({content: `Hello from ${owner.firstName}!`});
191187
await expect(pages.conversation().getReactionOnMessage(ownerMessage, 'heart')).toBeVisible();
192188
await expect(pages.conversation().getReactionOnMessage(ownerMessage, 'joy')).toBeVisible();
193189

194190
// Member1 verifies they can see thumbs up (+1) and joy (😂) reactions on their message from owner and member2
195191
await member1PageManager.webapp.pages.conversationList().openConversation(conversationName);
196192
const member1Message = member1PageManager.webapp.pages
197193
.conversation()
198-
.getMessageByText(`Hello from ${member1.firstName}!`);
194+
.getMessage({content: `Hello from ${member1.firstName}!`});
199195
await expect(
200196
member1PageManager.webapp.pages.conversation().getReactionOnMessage(member1Message, 'plus-one'),
201197
).toBeVisible();
@@ -207,7 +203,7 @@ test(
207203
await member2PageManager.webapp.pages.conversationList().openConversation(conversationName);
208204
const member2Message = member2PageManager.webapp.pages
209205
.conversation()
210-
.getMessageByText(`Hello from ${member2.firstName}!`);
206+
.getMessage({content: `Hello from ${member2.firstName}!`});
211207
await expect(
212208
member2PageManager.webapp.pages.conversation().getReactionOnMessage(member2Message, 'plus-one'),
213209
).toBeVisible();

‎test/e2e_tests/specs/CriticalFlow/backupRestoration-TC-8634.spec.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ test('Setting up new device with a backup', {tag: ['@TC-8634', '@crit-flow-web']
7070
await test.step('User generates data', async () => {
7171
await pages.conversationList().openConversation(userB.fullName);
7272
await pages.conversation().sendMessage(personalMessage);
73-
74-
expect(pages.conversation().isMessageVisible(personalMessage)).toBeTruthy();
73+
await expect(pages.conversation().getMessage({content: personalMessage})).toBeVisible();
7574

7675
await pages.conversationList().clickCreateGroup();
7776
await pages.groupCreation().setGroupName(groupName);
@@ -80,7 +79,7 @@ test('Setting up new device with a backup', {tag: ['@TC-8634', '@crit-flow-web']
8079
await pages.conversationList().openConversation(groupName);
8180
await pages.conversation().sendMessage(groupMessage);
8281

83-
expect(pages.conversation().isMessageVisible(groupMessage)).toBeTruthy();
82+
await expect(pages.conversation().getMessage({content: groupMessage})).toBeVisible();
8483
});
8584

8685
await test.step('User creates and saves a backup', async () => {
@@ -103,10 +102,10 @@ test('Setting up new device with a backup', {tag: ['@TC-8634', '@crit-flow-web']
103102

104103
await test.step('User doesnt see previous data (messages)', async () => {
105104
await pages.conversationList().openConversation(userB.fullName);
106-
expect(await pages.conversation().isMessageVisible(personalMessage, false)).toBeFalsy();
105+
await expect(pages.conversation().getMessage({content: personalMessage})).not.toBeVisible();
107106

108107
await pages.conversationList().openConversation(groupName);
109-
expect(await pages.conversation().isMessageVisible(groupMessage, false)).toBeFalsy();
108+
await expect(pages.conversation().getMessage({content: groupMessage})).not.toBeVisible();
110109
});
111110

112111
await test.step('User restores the previously created backup', async () => {
@@ -128,10 +127,10 @@ test('Setting up new device with a backup', {tag: ['@TC-8634', '@crit-flow-web']
128127
await test.step('All data (chat history, contacts) are restored', async () => {
129128
await components.conversationSidebar().clickAllConversationsButton();
130129
await pages.conversationList().openConversation(groupName);
131-
expect(pages.conversation().isMessageVisible(groupMessage)).toBeTruthy();
130+
await expect(pages.conversation().getMessage({content: groupMessage})).toBeVisible();
132131

133132
await pages.conversationList().openConversation(userB.fullName);
134-
expect(pages.conversation().isMessageVisible(personalMessage)).toBeTruthy();
133+
await expect(pages.conversation().getMessage({content: personalMessage})).toBeVisible();
135134
});
136135
});
137136

‎test/e2e_tests/specs/CriticalFlow/channelsManagement-TC-8752.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,9 @@ test('Channels Management', {tag: ['@TC-8752', '@crit-flow-web']}, async ({pageM
151151
});
152152

153153
await test.step('Team member sees the message', async () => {
154-
expect(await memberPages.webapp.pages.conversation().isMessageVisible('Hello team! Admin here.')).toBeTruthy();
154+
await expect(
155+
memberPages.webapp.pages.conversation().getMessage({content: 'Hello team! Admin here.'}),
156+
).toBeVisible();
155157
});
156158
});
157159

‎test/e2e_tests/specs/CriticalFlow/conversationManagement-TC-8636.spec.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,24 @@ test('Conversation Management', {tag: ['@TC-8636', '@crit-flow-web']}, async ({p
7272

7373
await test.step('Team owner signed in to the application and verify messages', async () => {
7474
await pages.conversationList().openConversation(conversationName);
75-
for (const member of members) {
76-
expect(await pages.conversation().isMessageVisible(`Hello team! ${member.firstName} here.`)).toBeTruthy();
77-
}
75+
await Promise.all(
76+
members.map(async member => {
77+
const message = pages.conversation().getMessage({content: `Hello team! ${member.firstName} here.`});
78+
await expect(message).toBeVisible();
79+
}),
80+
);
7881
});
7982

8083
await test.step('Team owner send self-destructing messages', async () => {
8184
const textMessage = 'This message will self-destruct in 10 seconds.';
8285
await components.inputBarControls().setEphemeralTimerTo('10 seconds');
8386
await pages.conversation().sendMessage(textMessage);
8487

85-
expect(await pages.conversation().isMessageVisible(textMessage)).toBeTruthy();
88+
await expect(pages.conversation().getMessage({content: textMessage})).toBeVisible();
8689
// Wait for more than 10 seconds to ensure the message is deleted
8790
await pages.conversation().page.waitForTimeout(11000);
8891

89-
expect(await pages.conversation().isMessageVisible(textMessage, false)).toBeFalsy();
92+
await expect(pages.conversation().getMessage({content: textMessage})).not.toBeVisible();
9093
await components.inputBarControls().setEphemeralTimerTo('Off');
9194
});
9295

‎test/e2e_tests/specs/CriticalFlow/messagesIn1On1-TC-8750.spec.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,22 @@ test('Messages in 1:1', {tag: ['@TC-8750', '@crit-flow-web']}, async ({pageManag
176176
await test.step('User A sends a quick (10 sec) self deleting message', async () => {
177177
await components.inputBarControls().setEphemeralTimerTo('10 seconds');
178178
await pages.conversation().sendMessage(selfDestructMessageText);
179-
expect(await pages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
179+
await expect(pages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
180180
});
181+
181182
await test.step('User B sees the message', async () => {
182-
expect(await memberBPM.webapp.pages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
183+
await expect(memberBPM.webapp.pages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
183184
});
184185

185186
// Step 7: Message removal
186187
await test.step('Wait 11 seconds', async () => {
187188
await memberBPM.webapp.pages.conversation().page.waitForTimeout(11_000);
188189
});
189190
await test.step('Both users see the message as removed', async () => {
190-
expect(await memberBPM.webapp.pages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
191-
expect(await pages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
191+
await expect(
192+
memberBPM.webapp.pages.conversation().getMessage({content: selfDestructMessageText}),
193+
).not.toBeVisible();
194+
await expect(pages.conversation().getMessage({content: selfDestructMessageText})).not.toBeVisible();
192195

193196
// Reset ephemeral timer to 'Off'
194197
await components.inputBarControls().setEphemeralTimerTo('Off');

‎test/e2e_tests/specs/CriticalFlow/messagesInChannels-TC-8753.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ test(
101101
await userBPageManager.refreshPage({waitUntil: 'load'});
102102

103103
await userBPages.conversationList().openConversation(channelName);
104-
expect(await userBPages.conversation().isMessageVisible(`@${userB.fullName} ${messageText}`)).toBeTruthy();
104+
await expect(userBPages.conversation().getMessage({content: `@${userB.fullName} ${messageText}`})).toBeVisible();
105105
});
106106

107107
await test.step('User A sends image', async () => {
@@ -165,20 +165,20 @@ test(
165165
await test.step('User A sends a quick (10 sec) self deleting message', async () => {
166166
await userAComponents.inputBarControls().setEphemeralTimerTo('10 seconds');
167167
await userAPages.conversation().sendMessage(selfDestructMessageText);
168-
expect(await userAPages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
168+
await expect(userAPages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
169169
});
170170

171171
await test.step('User B sees the message', async () => {
172-
expect(await userBPages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
172+
await expect(userBPages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
173173
});
174174

175175
await test.step('User B waits 10 seconds', async () => {
176176
await userBPages.conversation().page.waitForTimeout(11_000);
177177
});
178178

179179
await test.step('Both users see the message as removed', async () => {
180-
expect(await userBPages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
181-
expect(await userAPages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
180+
await expect(userBPages.conversation().getMessage({content: selfDestructMessageText})).not.toBeVisible();
181+
await expect(userAPages.conversation().getMessage({content: selfDestructMessageText})).not.toBeVisible();
182182

183183
// Reset ephemeral timer to 'Off'
184184
await userAComponents.inputBarControls().setEphemeralTimerTo('Off');

‎test/e2e_tests/specs/CriticalFlow/messagesInGroups-TC-8751.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ test(
9090
await userBPageManager.refreshPage({waitUntil: 'load'});
9191

9292
await userBPages.conversationList().openConversation(conversationName);
93-
expect(await userBPages.conversation().isMessageVisible(`@${userB.fullName} ${messageText}`)).toBeTruthy();
93+
await expect(userBPages.conversation().getMessage({content: `@${userB.fullName} ${messageText}`})).toBeVisible();
9494
});
9595

9696
await test.step('User A sends image', async () => {
@@ -154,20 +154,20 @@ test(
154154
await test.step('User A sends a quick (10 sec) self deleting message', async () => {
155155
await userAComponents.inputBarControls().setEphemeralTimerTo('10 seconds');
156156
await userAPages.conversation().sendMessage(selfDestructMessageText);
157-
expect(await userAPages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
157+
await expect(userAPages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
158158
});
159159

160160
await test.step('User B sees the message', async () => {
161-
expect(await userBPages.conversation().isMessageVisible(selfDestructMessageText)).toBeTruthy();
161+
await expect(userBPages.conversation().getMessage({content: selfDestructMessageText})).toBeVisible();
162162
});
163163

164164
await test.step('User B waits 10 seconds', async () => {
165165
await userBPages.conversation().page.waitForTimeout(11_000);
166166
});
167167

168168
await test.step('Both users see the message as removed', async () => {
169-
expect(await userBPages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
170-
expect(await userAPages.conversation().isMessageVisible(selfDestructMessageText, false)).toBeFalsy();
169+
await expect(userBPages.conversation().getMessage({content: selfDestructMessageText})).not.toBeVisible();
170+
await expect(userAPages.conversation().getMessage({content: selfDestructMessageText})).not.toBeVisible();
171171

172172
// Reset ephemeral timer to 'Off'
173173
await userAComponents.inputBarControls().setEphemeralTimerTo('Off');

0 commit comments

Comments
 (0)