feat(tests): Adds kick, lastN and lobby tests. #15517

damencho · 2025-01-22T23:37:49Z

No description provided.

saghul · 2025-01-23T09:38:28Z

tests/helpers/Participant.ts

@@ -135,7 +151,7 @@ export class Participant {
        if (!options.skipDisplayName) {
            // @ts-ignore
            config.userInfo = {
-                displayName: options.displayName || this._name
+                displayName: this._displayName = options.displayName || this._name


Did you intend this to be an assignment? Since there is a getter, shouldn't this be this.displayName ?

Yep, this is where we set it from the options... But I can drop part of the later changes where I use a constant instead of call the dispalyName getter.

No strong opinion, you pick!

saghul · 2025-01-23T09:39:27Z

tests/helpers/Participant.ts

@@ -291,7 +309,7 @@ export class Participant {
     */
    async waitToJoinMUC(): Promise<void> {
        return this.driver.waitUntil(
-            () => this.isInMuc(),
+            async () => await this.isInMuc(),


Is this necessary? It will make the function return a promise, and if this.isInMuc() already does that, this change shouldn't be necessary.

saghul · 2025-01-23T09:40:06Z

tests/helpers/Participant.ts

+            timeout = 15_000, msg = `expected to send data in 15s for ${this.name}`): Promise<void> {
+        const driver = this.driver;
+
+        return driver.waitUntil(async () =>


The async / await here should be unnecessary for the same reason as above.

tests/helpers/Participant.ts

squash: fix lint.

feat(dialog): Adds a print when opening and hiding dialogs.

tests/pageobjects/IframeAPI.ts

tests/pageobjects/Filmstrip.ts

tests/pageobjects/LobbyScreen.ts

tests/pageobjects/PreJoinScreen.ts

tests/pageobjects/LobbyScreen.ts

tests/pageobjects/PreJoinScreen.ts

tests/pageobjects/Notifications.ts

hristoterezov · 2025-01-23T22:49:42Z

tests/pageobjects/Notifications.ts

+            timeoutMsg: 'Knocking participant notification not displayed'
+        });
+
+        return await knockingParticipantNotification.getText();


Do we really want await here?

Generally in most cases return await would not make any sense, it would do exactly the same as what return would do.

Interesting read: https://nurkiewicz.com/2024/08/return-await-in-javascript.html I'd keep the await.

Hah, interesting.

Interesting indeed. Do we care about the difference on the stack trace for those cases?

To me the version without await looks way more readable. I would still remove it unless we are sure we are going to need the extra line on the stack trace.

Here are some examples:

[0-0] RUNNING in MultiRemote - file:///tests/specs/alone/invite.spec.ts [0-0] Error in "Invite.url displayed" Error: Can't call click on element with selector "#modal-header-close-button1" because element wasn't found at async Context.<anonymous> (/Users/damencho/dev/jitsi-meet/tests/specs/alone/invite.spec.ts:18:9)

This ^ is from the following function:

clickCloseButton(): Promise<void> { return this.participant.driver.$(`#${CLOSE_BUTTON}`).click(); }

Next one:

[0-0] Error in "Invite.dial-in displayed" Error: Can't call click on element with selector "#modal-header-close-button1" because element wasn't found at async Context.<anonymous> (/Users/damencho/dev/jitsi-meet/tests/specs/alone/invite.spec.ts:24:9)

This is from:

async clickCloseButton2(): Promise<void> { return this.participant.driver.$(`#${CLOSE_BUTTON}`).click(); }

Last one:

[0-0] Error in "Invite.view more numbers" Error: Can't call click on element with selector "#modal-header-close-button1" because element wasn't found at async InviteDialog.clickCloseButton3 (/Users/damencho/dev/jitsi-meet/tests/pageobjects/BaseDialog.ts:25:9) at async Context.<anonymous> (/Users/damencho/dev/jitsi-meet/tests/specs/alone/invite.spec.ts:41:9) [0-0] FAILED in MultiRemote - file:///tests/specs/alone/invite.spec.ts

This is from:

async clickCloseButton3(): Promise<void> { await this.participant.driver.$(`#${CLOSE_BUTTON}`).click(); }

For this example it seems ok to miss the line at async InviteDialog.clickCloseButton3 (/Users/damencho/dev/jitsi-meet/tests/pageobjects/BaseDialog.ts:25:9) ... but I guess if there more calls on the chain we may need that ... Will do another example with one more step in the chain of calls.

hristoterezov · 2025-01-23T22:49:56Z

tests/pageobjects/Notifications.ts

+     * The notification that someone's access was approved.
+     */
+    async getLobbyParticipantAccessGranted() {
+        return await this.getNotificationText(LOBBY_PARTICIPANT_ACCESS_GRANTED_TEST_ID);


hristoterezov · 2025-01-23T22:50:13Z

tests/pageobjects/Notifications.ts

+
+        await notificationElement.waitForExist();
+
+        return await notificationElement.getText();


hristoterezov · 2025-01-23T22:50:24Z

tests/pageobjects/Notifications.ts

+     * The notification test that someone's access was denied.
+     */
+    async getLobbyParticipantAccessDenied() {
+        return await this.getNotificationText(LOBBY_PARTICIPANT_ACCESS_DENIED_TEST_ID);


hristoterezov

I see that menu of the underlying wdio operations are async and we use await.

I'm wondering do we expect them to fail from time to time? Should we generally add any error handing, try-catch, etc. ? Have we tought what to do if some of those fail? Maybe exit and fail everything?

hristoterezov · 2025-01-23T23:00:07Z

tests/pageobjects/PreMeetingScreen.ts

+     * Checks internally whether lobby room is joined.
+     */
+    async isLobbyRoomJoined() {
+        return await this.participant.driver.execute(


Maybe remove the async/await here too

hristoterezov · 2025-01-23T23:02:24Z

tests/pageobjects/SecurityDialog.ts

+    /**
+     * Returns is the lobby enabled.
+     */
+    async isLobbyEnabled() {


I think this is redundant async, isn't it?

hristoterezov · 2025-01-23T23:02:43Z

tests/pageobjects/SecurityDialog.ts

+     * Waits for the lobby to be enabled or disabled.
+     * @param reverse
+     */
+    async waitForLobbyEnabled(reverse = false) {


I think this is redundant async, isn't it?

hristoterezov · 2025-01-23T23:02:55Z

tests/pageobjects/SecurityDialog.ts

+     * Checks whether lobby section is present in the UI.
+     */
+    async isLobbySectionPresent() {
+        return await this.getLobbySwitch().isExisting();


Maybe we don't need async/await?

hristoterezov · 2025-01-23T23:03:16Z

tests/pageobjects/SecurityDialog.ts

+     * @return {@code true} if the conference is displayed as locked locally in
+     * the security dialog, {@code false} otherwise.
+     */
+    private async isLockedLocally() {


I think this is redundant async, isn't it?

hristoterezov · 2025-01-23T23:03:38Z

tests/pageobjects/SecurityDialog.ts

+     * in the security dialog, {@code false} otherwise.
+     */
+    private async isLockedRemotely() {
+        return await this.participant.driver.$(`.${REMOTE_LOCK}`).isExisting();


Maybe we don't need async/await?

hristoterezov

LGTM!

Added some comments about redundant async/awaits and a question!

damencho · 2025-01-24T00:51:20Z

I see that menu of the underlying wdio operations are async and we use await.

I'm wondering do we expect them to fail from time to time? Should we generally add any error handing, try-catch, etc. ? Have we tought what to do if some of those fail? Maybe exit and fail everything?

If those failed the test is marked as failed and this is what we want.

damencho · 2025-01-24T01:34:29Z

@hristoterezov I think I have addressed all of those, I went and through the other page objects and fix more like that.
AVmoderation and lobby tests need more love though ... may give it a shot tomorrow.

damencho added 2 commits January 22, 2025 17:40

feat(tests): Adds kick test.

8b5a47a

feat(tests): Adds lastN test.

3b94fd6

damencho force-pushed the more-tests8 branch from 1e1490d to 6466ec0 Compare January 22, 2025 23:40

damencho changed the title ~~feat(tests): Adds follow-me, invite dialog, lastN and lobby tests.~~ feat(tests): Adds kick, lastN and lobby tests. Jan 22, 2025

damencho force-pushed the more-tests8 branch 2 times, most recently from 807ce67 to a8de771 Compare January 23, 2025 00:32

saghul approved these changes Jan 23, 2025

View reviewed changes

damencho added 3 commits January 23, 2025 07:46

fix(tests): Fixes element not interceptable in AV moderation tests.

56c23ed

fix(tests): Fixes breakout room wait for room update.

290fa30

fix(tests): Simplify suite names.

18d0a48

squash: fix lint.

damencho force-pushed the more-tests8 branch from a8de771 to e79d33a Compare January 23, 2025 14:14

damencho added 2 commits January 23, 2025 12:52

feat(tests): Adds lobby test.

868ff81

feat(dialog): Adds a print when opening and hiding dialogs.

fix(pre-join): Remove from dom when lobby is shown.

6e7e31e

damencho force-pushed the more-tests8 branch from e79d33a to 186fedc Compare January 23, 2025 18:53