From 4432f727a463a690bc05880e4b29bfcf1e177179 Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty Date: Wed, 29 Oct 2025 15:06:11 -0400 Subject: [PATCH] fix(tests) Check for continguous thunbnails in the filmstrip --- react/features/base/config/configType.ts | 1 + tests/pageobjects/Filmstrip.ts | 107 +++++++++++++++++++++++ tests/specs/media/activeSpeaker.spec.ts | 11 +++ tests/specs/media/desktopSharing.spec.ts | 44 +++++++--- tests/specs/moderation/lobby.spec.ts | 7 +- 5 files changed, 158 insertions(+), 12 deletions(-) diff --git a/react/features/base/config/configType.ts b/react/features/base/config/configType.ts index 5eb8ab1977..31df87f5b9 100644 --- a/react/features/base/config/configType.ts +++ b/react/features/base/config/configType.ts @@ -393,6 +393,7 @@ export interface IConfig { disabled?: boolean; initialWidth?: number; minParticipantCountForTopPanel?: number; + stageFilmstripParticipants?: number; }; flags?: { ssrcRewritingEnabled: boolean; diff --git a/tests/pageobjects/Filmstrip.ts b/tests/pageobjects/Filmstrip.ts index d6a6538956..1760ed5a9f 100644 --- a/tests/pageobjects/Filmstrip.ts +++ b/tests/pageobjects/Filmstrip.ts @@ -275,4 +275,111 @@ export default class Filmstrip extends BasePageObject { reverse: !isDisplayed, }); } + + /** + * Checks for visible gaps in the filmstrip thumbnails. + * This detects if there are any missing thumbnails or excessive spacing between consecutive visible thumbnails. + * + * @returns Returns true if gaps are detected, false otherwise. + */ + async hasGapsInFilmstrip(): Promise { + return await this.participant.execute(() => { + // Get all visible thumbnail containers in the filmstrip + const thumbnails = Array.from( + document.querySelectorAll('#remoteVideos span.videocontainer') + ).filter((thumb: any) => { + const style = window.getComputedStyle(thumb); + const rect = thumb.getBoundingClientRect(); + + // Check if element is visible and has dimensions + return style.display !== 'none' + && style.visibility !== 'hidden' + && rect.width > 0 + && rect.height > 0; + }); + + if (thumbnails.length < 2) { + // Can't have gaps with less than 2 thumbnails + return false; + } + + // Get positions and calculated margins of all visible thumbnails + const positions = thumbnails.map((thumb: any) => { + const rect = thumb.getBoundingClientRect(); + const style = window.getComputedStyle(thumb); + + return { + left: rect.left, + right: rect.right, + top: rect.top, + bottom: rect.bottom, + width: rect.width, + height: rect.height, + marginTop: parseFloat(style.marginTop) || 0, + marginBottom: parseFloat(style.marginBottom) || 0, + marginLeft: parseFloat(style.marginLeft) || 0, + marginRight: parseFloat(style.marginRight) || 0 + }; + }); + + // Calculate expected spacing between thumbnails based on first two + const firstGap = positions.length >= 2 + ? Math.abs(positions[1].top - positions[0].top) !== 0 + ? positions[1].top - positions[0].bottom // vertical + : positions[1].left - positions[0].right // horizontal + : 0; + + // Check if filmstrip is vertical or horizontal + const isVertical = Math.abs(positions[1].top - positions[0].top) > Math.abs(positions[1].left - positions[0].left); + + if (isVertical) { + // For vertical filmstrip, check vertical spacing consistency + for (let i = 0; i < positions.length - 1; i++) { + const current = positions[i]; + const next = positions[i + 1]; + const gap = next.top - current.bottom; + + // Compare against the first gap with some tolerance + // Flag if gap is more than 2x the expected spacing + if (gap > Math.max(firstGap * 2, current.height * 0.3)) { + return true; + } + } + } else { + // For horizontal filmstrip, check horizontal spacing consistency + for (let i = 0; i < positions.length - 1; i++) { + const current = positions[i]; + const next = positions[i + 1]; + const gap = next.left - current.right; + + // Compare against the first gap with some tolerance + // Flag if gap is more than 2x the expected spacing + if (gap > Math.max(firstGap * 2, current.width * 0.3)) { + return true; + } + } + } + + return false; + }); + } + + /** + * Asserts that there are no gaps in the filmstrip. + * This is useful for detecting layout issues where thumbnails might be missing or mispositioned. + * + * @param reverse - If true, asserts that gaps should exist. Default false. + */ + async assertNoGapsInFilmstrip(reverse = false): Promise { + const hasGaps = await this.hasGapsInFilmstrip(); + const expectedResult = reverse ? true : false; + + if (hasGaps !== expectedResult) { + throw new Error( + `Expected filmstrip to ${reverse ? 'have' : 'not have'} gaps, but ${ + hasGaps ? 'gaps were detected' : 'no gaps were found' + }` + ); + } + } } diff --git a/tests/specs/media/activeSpeaker.spec.ts b/tests/specs/media/activeSpeaker.spec.ts index 309373e564..43686a5ffb 100644 --- a/tests/specs/media/activeSpeaker.spec.ts +++ b/tests/specs/media/activeSpeaker.spec.ts @@ -76,6 +76,10 @@ describe('Active speaker', () => { timeoutMsg: 'P3 tile should remain visible in filmstrip during screenshare' }); + // Check that there are no gaps in the filmstrip during local screenshare + await p2.getFilmstrip().assertNoGapsInFilmstrip(); + await p3.getFilmstrip().assertNoGapsInFilmstrip(); + await p1.getToolbar().clickAudioMuteButton(); await p1.getToolbar().clickStopDesktopSharingButton(); }); @@ -216,6 +220,10 @@ describe('Active speaker', () => { timeout: 3_000, timeoutMsg: 'P3 tile should remain visible in filmstrip during P2 screenshare' }); + + // Check that there are no gaps in the filmstrip when remote starts screensharing + await p1.getFilmstrip().assertNoGapsInFilmstrip(); + await p3.getFilmstrip().assertNoGapsInFilmstrip(); }); it('testFilmstripTilesWithMultipleScreenshares', async () => { @@ -256,6 +264,9 @@ describe('Active speaker', () => { // Verify screenshare tiles are present await checkForScreensharingTile(p1, p3); await checkForScreensharingTile(p2, p3); + + // Check that there are no gaps in the filmstrip with multiple screenshares + await p3.getFilmstrip().assertNoGapsInFilmstrip(); }); }); diff --git a/tests/specs/media/desktopSharing.spec.ts b/tests/specs/media/desktopSharing.spec.ts index abb7fedce7..be6da1b11c 100644 --- a/tests/specs/media/desktopSharing.spec.ts +++ b/tests/specs/media/desktopSharing.spec.ts @@ -83,7 +83,9 @@ describe('Desktop sharing', () => { timeoutMsg: 'P3 camera tile should be visible on P1 during P2 screenshare' }); - expect(await p3.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + await p1.getFilmstrip().assertNoGapsInFilmstrip(); + await p3.getFilmstrip().assertNoGapsInFilmstrip(); + await p3.waitForParticipantOnLargeVideo(`${p2EndpointId}-v1`); }); /** @@ -103,7 +105,7 @@ describe('Desktop sharing', () => { await checkForScreensharingTile(p2, p2); // The video should be playing. - expect(await p1.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + await p1.waitForParticipantOnLargeVideo(`${await p2.getEndpointId()}-v1`); // Start desktop share on p1. await p1.getToolbar().clickDesktopSharingButton(); @@ -123,9 +125,6 @@ describe('Desktop sharing', () => { await checkForScreensharingTile(p1, p3); await checkForScreensharingTile(p2, p3); - - // The large video should be playing on p3. - expect(await p3.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); }); /** @@ -166,8 +165,22 @@ describe('Desktop sharing', () => { await checkForScreensharingTile(p1, p3); await checkForScreensharingTile(p2, p3); - // The large video should be playing on p3. - expect(await p3.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + // Add another particpant to verify multiple screenshares are visible without gaps in filmstrip. + await ensureFourParticipants({ + configOverwrite: { + filmstrip: { + stageFilmstripParticipants: 2 + }, + startWithAudioMuted: true + } + }); + + const { p4 } = ctx; + + await checkForScreensharingTile(p1, p4); + await checkForScreensharingTile(p2, p4); + await p3.getFilmstrip().assertNoGapsInFilmstrip(); + await p4.getFilmstrip().assertNoGapsInFilmstrip(); }); /** @@ -205,6 +218,10 @@ describe('Desktop sharing', () => { } }); const { p2, p3 } = ctx; + const p1EndpointId = await p1.getEndpointId(); + const p1ScreenShareTileId = `${p1EndpointId}-v1`; + const p2EndpointId = await p2.getEndpointId(); + const p2ScreenShareTileId = `${p2EndpointId}-v1`; // p1 starts share again when call switches to jvb. await p1.getToolbar().clickDesktopSharingButton(); @@ -213,8 +230,8 @@ describe('Desktop sharing', () => { await checkForScreensharingTile(p1, p2); await checkForScreensharingTile(p1, p3); - expect(await p2.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); - expect(await p3.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + await p2.waitForParticipantOnLargeVideo(p1ScreenShareTileId); + await p3.waitForParticipantOnLargeVideo(p1ScreenShareTileId); // p3 leaves the call. await p3.hangup(); @@ -225,14 +242,14 @@ describe('Desktop sharing', () => { // Make sure p2 see's p1's share after the call switches back to p2p. await checkForScreensharingTile(p1, p2); - expect(await p2.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + await p2.waitForParticipantOnLargeVideo(p1ScreenShareTileId); // p2 starts share when in p2p. await p2.getToolbar().clickDesktopSharingButton(); // Makes sure p2's share is visible on p1. await checkForScreensharingTile(p2, p1); - expect(await p1.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + await p1.waitForParticipantOnLargeVideo(p2ScreenShareTileId); }); /** @@ -357,6 +374,11 @@ describe('Desktop sharing', () => { // And the video should be playing expect(await p4.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true); + // Check that there are no gaps in the filmstrip when participant joins during screensharing + await p1.getFilmstrip().assertNoGapsInFilmstrip(); + await p2.getFilmstrip().assertNoGapsInFilmstrip(); + await p4.getFilmstrip().assertNoGapsInFilmstrip(); + const p1EndpointId = await p1.getEndpointId(); const p2EndpointId = await p2.getEndpointId(); diff --git a/tests/specs/moderation/lobby.spec.ts b/tests/specs/moderation/lobby.spec.ts index f3bec3bb7e..5b0b19b3bd 100644 --- a/tests/specs/moderation/lobby.spec.ts +++ b/tests/specs/moderation/lobby.spec.ts @@ -323,7 +323,7 @@ describe('Lobby', () => { await ensureTwoParticipants(); await enableLobby(); - const { p1 } = ctx; + const { p1, p2 } = ctx; const knockingParticipant = await enterLobby(p1, true, true); @@ -368,6 +368,11 @@ describe('Lobby', () => { await p3.waitForRemoteStreams(2); expect(await p3.getFilmstrip().countVisibleThumbnails()).toBe(3); + + // Check that there are no gaps in the filmstrip after joining from lobby + await p1.getFilmstrip().assertNoGapsInFilmstrip(); + await p2.getFilmstrip().assertNoGapsInFilmstrip(); + await p3.getFilmstrip().assertNoGapsInFilmstrip(); }); });