feat(tests): First test from torture to meet. #15298

damencho · 2024-11-14T04:07:34Z

No description provided.

pullrequest

🔍 This pull request has been sent to HackerOne's PullRequest review team because our automation detected one or more changes with potential security impact. Experts are now being assigned to this review based on relevant expertise and will validate or dismiss any security findings accordingly and post their feedback as comments within this pull request.

Check the status or cancel this secure code review here.

pullrequest

✅ Jacob reviewed all the included code changes and associated automation findings and determined that there were no immediately actionable security flaws. Note that they will continue to be notified of any new commits or comments and follow up as needed throughout the duration of this pull request's lifecycle.

saghul

Left some comments. YOU ARE THE MAN! Thanks for getting the ball rolling! ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️

tests/helpers/Participant.ts

saghul · 2024-11-14T08:58:52Z

tests/helpers/Participant.ts

+     * The driver it uses.
+     */
+    get driver() {
+        return browser.getInstance(this._name);


tests/helpers/Participant.ts

saghul · 2024-11-14T09:00:46Z

tests/helpers/Participant.ts

+            console.log(`lib-jitsi-meet version: ${JitsiMeetJS.version} sessionId: ${sessionId}`);
+        }, this._name, driver.sessionId));
+
+        // disable the blur effect in firefox as it has some performance issues


Why not disable it everywhere? It has no impact in tests, does it?

saghul · 2024-11-14T09:09:18Z

tests/helpers/participants.ts

+ * Generate a random room name.
+ */
+function generateRandomRoomName(): string {
+    return `jitsimeettorture-${Math.round(Math.random() * 1000000000)}`;


Suggested change

return `jitsimeettorture-${Math.round(Math.random() * 1000000000)}`;

return `jitsimeettorture-${crypto.randomUUID()}`;

saghul · 2024-11-14T09:11:10Z

tests/pageobjects/Toolbar.ts

+     * Clicks audio mute button.
+     */
+    async clickAudioMuteButton() {
+        await this.participant.driver.execute(() => console.log('Clicking on: Audio Mute Button'));


Perhaps the Participant class should offer a log method?

saghul · 2024-11-14T09:13:15Z

tests/specs/2way/activeSpeaker.spec.ts

+    logInfo(activeSpeaker.driver, 'Unmuting in testActiveSpeaker');
+
+    // Unmute
+    await new Toolbar(activeSpeaker).clickAudioUnmuteButton();


Rather than having to do new Toolbar(participant).foo(), how about: partifipant.getToolbar().foo() ?

saghul · 2024-11-14T09:14:13Z

tests/wdio.conf.ts

+}
+if (process.env.HEADLESS === 'true') {
+    chromeArgs.push('--headless');
+    chromeArgs.push('--window-size=1400,600');


Why the weird resolution? Why not 1080p?

🤷 I will change it, this is what torture runs today.

saghul · 2024-11-14T09:14:32Z

tests/wdio.conf.ts

+    ],
+    maxInstances: 1,
+
+    baseUrl: 'https://alpha.jitsi.net/torture',


Can you make this overridable via env var pl?

There is a very simple way to override stuff from configs. I wanted to leave it so it is a working one without any modifications, the way we have it for make dev.

https://webdriver.io/docs/organizingsuites#inherit-from-main-config-file
I will use the same to introduce the Firefox tests. And different environments will be overriding stuff and excluding tests if they need to ... and change the deployment URL.

We can take an env var and have a default for it, WDYT?

damencho · 2024-11-14T12:22:53Z

To efficiently use the browser instances we need to have this merged, released and used here: webdriverio/webdriverio#13878
Right now it will create 4 sessions per test, where it may use just one.

…trip.

saghul · 2024-11-14T16:13:07Z

tests/wdio.conf.ts

@@ -1,3 +1,5 @@
+/* global process, browser, Buffer */


since this runs with node you can import process and Buffer. Perhaps even browser too, but that's from wdio.

I think browser is special they inject ... you cannot import ... maybe

saghul

LGTM with a couple of comments.

saghul · 2024-11-14T22:52:27Z

tests/.eslintrc.js

+    'extends': [
+        '../.eslintrc.js',
+        '@jitsi/eslint-config/jsdoc',
+        '@jitsi/eslint-config/react'


Do we need this one?

Do you want to keep the jsdoc one? I was fixing the docs because of it. Keeps code consistent, but I'm not sure it is needed for tests. WDYT?

I meant the react one but yeah, we can remove both

saghul · 2024-11-14T22:53:37Z

tests/helpers/Participant.ts

+        testing: {
+            testMode: true
+        },
+        disableAEC: true,


Weird that AGC is also not disabled, is that intentional?

disableAP disables them all

…nfig * mobile-24.6: (367 commits) feat(prejoin): fix join meeting from external/calendar link while in another meeting (jitsi#15310) chore(apps, version): bump to 24.6.0 chore(sdk, version): bump to 10.3.0 fix(connection): Detects tenant hyphen and length problems and show notification. fix(connection): Shows notification instead of reload on conference request failed. feat(conference): revert fix for background app state (jitsi#15308) feat(tests): First test from torture to meet. (jitsi#15298) lang: update German translation chore(deps) lib-jitsi-meet@latest fix(local-recording) handle repeated values fix(ci) use macOS 15 to run iOS tests Update main-tr.json feat(reactions): Added heart reaction feat(ns) update Krisp to latest version feat(Video): Handle .play() errors. tr language updates fix(visitors): Fixes detection of turning of subtitles by visitor. fix(base/icons): error regarding default props fix(prejoin): Device indicator. chore(deps) lib-jitsi-meet@latest ... # Conflicts: # android/gradle.properties # android/sdk/src/main/java/org/jitsi/meet/sdk/JitsiMeetOngoingConferenceService.java # ios/app/broadcast-extension/Info.plist # ios/app/src/Info.plist # ios/app/watchos/app/Info.plist # ios/app/watchos/extension/Info.plist # ios/sdk/src/Info.plist # ios/sdk/src/Lite-Info.plist # react-native-sdk/package.json # react/features/keyboard-shortcuts/actions.web.ts

feat(tests): First test from torture to meet.

a79f84e

damencho requested a review from saghul November 14, 2024 04:07

pullrequest bot reviewed Nov 14, 2024

View reviewed changes

saghul reviewed Nov 14, 2024

View reviewed changes

damencho added 13 commits November 14, 2024 07:52

squash: Fixes logging as per comments.

578b7ac

squash: Fixes some eslint errors.

4658e57

squash: Drop no needed await and async declarations.

c5546ce

squash: Simplify syntax.

aa61b11

squash: Disable blur everywhere not only FF.

108f603

squash: Use allSettled.

6dec1ca

squash: Prettify intervals and timeouts.

4221a39

squash: Use uuids for torture rooms.

bf2f7d0

squash: Introduce helper methods in Participant for toolbar and films…

ebb3591

…trip.

squash: Changes headless resolution to a standard 720p.

85cc232

squash: Adds env BASE_URL.

174f6f0

squash: Fix some eslint errors.

35f3321

squash: Fix js error.

413a4c7

damencho marked this pull request as draft November 14, 2024 16:12

saghul reviewed Nov 14, 2024

View reviewed changes

damencho added 7 commits November 14, 2024 10:25

squash: Fix participant logs.

68e1f73

squash: Move bag to Promise.all.

e1bf04c

squash: More types thing.

08380f2

squash: Fix more ts errors.

bc3d7fe

squash: Bumps version to include webdriverio/webdriverio@6d146cd

1d78ddd

squash: More ts stuff.

54f0c46

squash: Fixes last ts errors.

8de3c6e

damencho marked this pull request as ready for review November 14, 2024 21:09

saghul previously approved these changes Nov 14, 2024

View reviewed changes

damencho added 2 commits November 14, 2024 17:14

squash: Drop eslint rule.

6474415

squash: Update default configs.

a3b2659

damencho dismissed saghul’s stale review via a3b2659 November 14, 2024 23:16

squash: Drop and docs eslint.

3c46356

saghul approved these changes Nov 15, 2024

View reviewed changes

damencho merged commit 5cd7b9b into master Nov 15, 2024
9 checks passed

damencho deleted the tests1 branch November 15, 2024 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tests): First test from torture to meet. #15298

feat(tests): First test from torture to meet. #15298

damencho commented Nov 14, 2024

pullrequest bot left a comment

pullrequest bot left a comment

saghul left a comment

saghul Nov 14, 2024

saghul Nov 14, 2024

saghul Nov 14, 2024

saghul Nov 14, 2024

saghul Nov 14, 2024

saghul Nov 14, 2024

damencho Nov 14, 2024

saghul Nov 14, 2024

damencho Nov 14, 2024

damencho Nov 14, 2024

saghul Nov 14, 2024

damencho commented Nov 14, 2024

saghul Nov 14, 2024

damencho Nov 14, 2024

damencho Nov 14, 2024

saghul left a comment

saghul Nov 14, 2024

damencho Nov 14, 2024

saghul Nov 14, 2024

saghul Nov 14, 2024

	return `jitsimeettorture-${Math.round(Math.random() * 1000000000)}`;
	return `jitsimeettorture-${crypto.randomUUID()}`;

feat(tests): First test from torture to meet. #15298

feat(tests): First test from torture to meet. #15298

Conversation

damencho commented Nov 14, 2024

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

saghul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damencho commented Nov 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saghul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment