From 13894aaf3aff75156310409ac906365cc40d71d5 Mon Sep 17 00:00:00 2001 From: Robin Date: Fri, 17 Oct 2025 12:34:06 -0400 Subject: [PATCH] Simplify some test helpers that no longer need continuations --- src/state/MediaViewModel.test.ts | 186 +++++++++++++++---------------- src/tile/GridTile.test.tsx | 69 ++++++------ src/tile/SpotlightTile.test.tsx | 97 ++++++++-------- src/utils/test.ts | 18 +-- 4 files changed, 175 insertions(+), 195 deletions(-) diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 8b186658..61fa2d8c 100644 --- a/src/state/MediaViewModel.test.ts +++ b/src/state/MediaViewModel.test.ts @@ -17,8 +17,8 @@ import { mockLocalParticipant, mockMediaDevices, mockRtcMembership, - withLocalMedia, - withRemoteMedia, + createLocalMedia, + createRemoteMedia, withTestScheduler, } from "../utils/test"; import { getValue } from "../utils/observable"; @@ -42,92 +42,89 @@ vi.mock("../Platform", () => ({ const rtcMembership = mockRtcMembership("@alice:example.org", "AAAA"); -test("control a participant's volume", async () => { +test("control a participant's volume", () => { const setVolumeSpy = vi.fn(); - await withRemoteMedia(rtcMembership, {}, { setVolume: setVolumeSpy }, (vm) => - withTestScheduler(({ expectObservable, schedule }) => { - schedule("-ab---c---d|", { - a() { - // Try muting by toggling - vm.toggleLocallyMuted(); - expect(setVolumeSpy).toHaveBeenLastCalledWith(0); - }, - b() { - // Try unmuting by dragging the slider back up - vm.setLocalVolume(0.6); - vm.setLocalVolume(0.8); - vm.commitLocalVolume(); - expect(setVolumeSpy).toHaveBeenCalledWith(0.6); - expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8); - }, - c() { - // Try muting by dragging the slider back down - vm.setLocalVolume(0.2); - vm.setLocalVolume(0); - vm.commitLocalVolume(); - expect(setVolumeSpy).toHaveBeenCalledWith(0.2); - expect(setVolumeSpy).toHaveBeenLastCalledWith(0); - }, - d() { - // Try unmuting by toggling - vm.toggleLocallyMuted(); - // The volume should return to the last non-zero committed volume - expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8); - }, - }); - expectObservable(vm.localVolume$).toBe("ab(cd)(ef)g", { - a: 1, - b: 0, - c: 0.6, - d: 0.8, - e: 0.2, - f: 0, - g: 0.8, - }); - }), - ); + const vm = createRemoteMedia(rtcMembership, {}, { setVolume: setVolumeSpy }); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-ab---c---d|", { + a() { + // Try muting by toggling + vm.toggleLocallyMuted(); + expect(setVolumeSpy).toHaveBeenLastCalledWith(0); + }, + b() { + // Try unmuting by dragging the slider back up + vm.setLocalVolume(0.6); + vm.setLocalVolume(0.8); + vm.commitLocalVolume(); + expect(setVolumeSpy).toHaveBeenCalledWith(0.6); + expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8); + }, + c() { + // Try muting by dragging the slider back down + vm.setLocalVolume(0.2); + vm.setLocalVolume(0); + vm.commitLocalVolume(); + expect(setVolumeSpy).toHaveBeenCalledWith(0.2); + expect(setVolumeSpy).toHaveBeenLastCalledWith(0); + }, + d() { + // Try unmuting by toggling + vm.toggleLocallyMuted(); + // The volume should return to the last non-zero committed volume + expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8); + }, + }); + expectObservable(vm.localVolume$).toBe("ab(cd)(ef)g", { + a: 1, + b: 0, + c: 0.6, + d: 0.8, + e: 0.2, + f: 0, + g: 0.8, + }); + }); }); -test("toggle fit/contain for a participant's video", async () => { - await withRemoteMedia(rtcMembership, {}, {}, (vm) => - withTestScheduler(({ expectObservable, schedule }) => { - schedule("-ab|", { - a: () => vm.toggleFitContain(), - b: () => vm.toggleFitContain(), - }); - expectObservable(vm.cropVideo$).toBe("abc", { - a: true, - b: false, - c: true, - }); - }), - ); +test("toggle fit/contain for a participant's video", () => { + const vm = createRemoteMedia(rtcMembership, {}, {}); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-ab|", { + a: () => vm.toggleFitContain(), + b: () => vm.toggleFitContain(), + }); + expectObservable(vm.cropVideo$).toBe("abc", { + a: true, + b: false, + c: true, + }); + }); }); -test("local media remembers whether it should always be shown", async () => { - await withLocalMedia( +test("local media remembers whether it should always be shown", () => { + const vm1 = createLocalMedia( rtcMembership, {}, mockLocalParticipant({}), mockMediaDevices({}), - (vm) => - withTestScheduler(({ expectObservable, schedule }) => { - schedule("-a|", { a: () => vm.setAlwaysShow(false) }); - expectObservable(vm.alwaysShow$).toBe("ab", { a: true, b: false }); - }), ); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-a|", { a: () => vm1.setAlwaysShow(false) }); + expectObservable(vm1.alwaysShow$).toBe("ab", { a: true, b: false }); + }); + // Next local media should start out *not* always shown - await withLocalMedia( + const vm2 = createLocalMedia( rtcMembership, {}, mockLocalParticipant({}), mockMediaDevices({}), - (vm) => - withTestScheduler(({ expectObservable, schedule }) => { - schedule("-a|", { a: () => vm.setAlwaysShow(true) }); - expectObservable(vm.alwaysShow$).toBe("ab", { a: false, b: true }); - }), ); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-a|", { a: () => vm2.setAlwaysShow(true) }); + expectObservable(vm2.alwaysShow$).toBe("ab", { a: false, b: true }); + }); }); test("switch cameras", async () => { @@ -164,7 +161,7 @@ test("switch cameras", async () => { const selectVideoInput = vi.fn(); - await withLocalMedia( + const vm = createLocalMedia( rtcMembership, {}, mockLocalParticipant({ @@ -179,27 +176,26 @@ test("switch cameras", async () => { select: selectVideoInput, }, }), - async (vm) => { - // Switch to back camera - getValue(vm.switchCamera$)!(); - expect(restartTrack).toHaveBeenCalledExactlyOnceWith({ - facingMode: "environment", - }); - await waitFor(() => { - expect(selectVideoInput).toHaveBeenCalledTimes(1); - expect(selectVideoInput).toHaveBeenCalledWith("back camera"); - }); - expect(deviceId).toBe("back camera"); - - // Switch to front camera - getValue(vm.switchCamera$)!(); - expect(restartTrack).toHaveBeenCalledTimes(2); - expect(restartTrack).toHaveBeenLastCalledWith({ facingMode: "user" }); - await waitFor(() => { - expect(selectVideoInput).toHaveBeenCalledTimes(2); - expect(selectVideoInput).toHaveBeenLastCalledWith("front camera"); - }); - expect(deviceId).toBe("front camera"); - }, ); + + // Switch to back camera + getValue(vm.switchCamera$)!(); + expect(restartTrack).toHaveBeenCalledExactlyOnceWith({ + facingMode: "environment", + }); + await waitFor(() => { + expect(selectVideoInput).toHaveBeenCalledTimes(1); + expect(selectVideoInput).toHaveBeenCalledWith("back camera"); + }); + expect(deviceId).toBe("back camera"); + + // Switch to front camera + getValue(vm.switchCamera$)!(); + expect(restartTrack).toHaveBeenCalledTimes(2); + expect(restartTrack).toHaveBeenLastCalledWith({ facingMode: "user" }); + await waitFor(() => { + expect(selectVideoInput).toHaveBeenCalledTimes(2); + expect(selectVideoInput).toHaveBeenLastCalledWith("front camera"); + }); + expect(deviceId).toBe("front camera"); }); diff --git a/src/tile/GridTile.test.tsx b/src/tile/GridTile.test.tsx index 783dcc37..dd0bc9d6 100644 --- a/src/tile/GridTile.test.tsx +++ b/src/tile/GridTile.test.tsx @@ -12,7 +12,7 @@ import { axe } from "vitest-axe"; import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; import { GridTile } from "./GridTile"; -import { mockRtcMembership, withRemoteMedia } from "../utils/test"; +import { mockRtcMembership, createRemoteMedia } from "../utils/test"; import { GridTileViewModel } from "../state/TileViewModel"; import { ReactionsSenderProvider } from "../reactions/useReactionsSender"; import type { CallViewModel } from "../state/CallViewModel"; @@ -25,7 +25,7 @@ global.IntersectionObserver = class MockIntersectionObserver { } as unknown as typeof IntersectionObserver; test("GridTile is accessible", async () => { - await withRemoteMedia( + const vm = createRemoteMedia( mockRtcMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", @@ -36,41 +36,40 @@ test("GridTile is accessible", async () => { getTrackPublication: () => ({}) as Partial as RemoteTrackPublication, }, - async (vm) => { - const fakeRtcSession = { + ); + + const fakeRtcSession = { + on: () => {}, + off: () => {}, + room: { + on: () => {}, + off: () => {}, + client: { + getUserId: () => null, + getDeviceId: () => null, on: () => {}, off: () => {}, - room: { - on: () => {}, - off: () => {}, - client: { - getUserId: () => null, - getDeviceId: () => null, - on: () => {}, - off: () => {}, - }, - }, - memberships: [], - } as unknown as MatrixRTCSession; - const cVm = { - reactions$: constant({}), - handsRaised$: constant({}), - } as Partial as CallViewModel; - const { container } = render( - - {}} - targetWidth={300} - targetHeight={200} - showSpeakingIndicators - focusable={true} - /> - , - ); - expect(await axe(container)).toHaveNoViolations(); - // Name should be visible - screen.getByText("Alice"); + }, }, + memberships: [], + } as unknown as MatrixRTCSession; + const cVm = { + reactions$: constant({}), + handsRaised$: constant({}), + } as Partial as CallViewModel; + const { container } = render( + + {}} + targetWidth={300} + targetHeight={200} + showSpeakingIndicators + focusable={true} + /> + , ); + expect(await axe(container)).toHaveNoViolations(); + // Name should be visible + screen.getByText("Alice"); }); diff --git a/src/tile/SpotlightTile.test.tsx b/src/tile/SpotlightTile.test.tsx index 4cb0f6c2..fb7008b8 100644 --- a/src/tile/SpotlightTile.test.tsx +++ b/src/tile/SpotlightTile.test.tsx @@ -15,8 +15,8 @@ import { mockLocalParticipant, mockMediaDevices, mockRtcMembership, - withLocalMedia, - withRemoteMedia, + createLocalMedia, + createRemoteMedia, } from "../utils/test"; import { SpotlightTileViewModel } from "../state/TileViewModel"; import { constant } from "../state/Behavior"; @@ -27,62 +27,53 @@ global.IntersectionObserver = class MockIntersectionObserver { } as unknown as typeof IntersectionObserver; test("SpotlightTile is accessible", async () => { - await withRemoteMedia( + const vm1 = createRemoteMedia( mockRtcMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", }, {}, - async (vm1) => { - await withLocalMedia( - mockRtcMembership("@bob:example.org", "BBBB"), - { - rawDisplayName: "Bob", - getMxcAvatarUrl: () => "mxc://dlskf", - }, - mockLocalParticipant({}), - mockMediaDevices({}), - async (vm2) => { - const user = userEvent.setup(); - const toggleExpanded = vi.fn(); - const { container } = render( - , - ); - - expect(await axe(container)).toHaveNoViolations(); - // Alice should be in the spotlight, with her name and avatar on the - // first page - screen.getByText("Alice"); - const aliceAvatar = screen.getByRole("img"); - expect(screen.queryByRole("button", { name: "common.back" })).toBe( - null, - ); - // Bob should be out of the spotlight, and therefore invisible - expect(isInaccessible(screen.getByText("Bob"))).toBe(true); - // Now navigate to Bob - await user.click(screen.getByRole("button", { name: "Next" })); - screen.getByText("Bob"); - expect(screen.getByRole("img")).not.toBe(aliceAvatar); - expect(isInaccessible(screen.getByText("Alice"))).toBe(true); - // Can toggle whether the tile is expanded - await user.click(screen.getByRole("button", { name: "Expand" })); - expect(toggleExpanded).toHaveBeenCalled(); - }, - ); - }, ); + + const vm2 = createLocalMedia( + mockRtcMembership("@bob:example.org", "BBBB"), + { + rawDisplayName: "Bob", + getMxcAvatarUrl: () => "mxc://dlskf", + }, + mockLocalParticipant({}), + mockMediaDevices({}), + ); + + const user = userEvent.setup(); + const toggleExpanded = vi.fn(); + const { container } = render( + , + ); + + expect(await axe(container)).toHaveNoViolations(); + // Alice should be in the spotlight, with her name and avatar on the + // first page + screen.getByText("Alice"); + const aliceAvatar = screen.getByRole("img"); + expect(screen.queryByRole("button", { name: "common.back" })).toBe(null); + // Bob should be out of the spotlight, and therefore invisible + expect(isInaccessible(screen.getByText("Bob"))).toBe(true); + // Now navigate to Bob + await user.click(screen.getByRole("button", { name: "Next" })); + screen.getByText("Bob"); + expect(screen.getByRole("img")).not.toBe(aliceAvatar); + expect(isInaccessible(screen.getByText("Alice"))).toBe(true); + // Can toggle whether the tile is expanded + await user.click(screen.getByRole("button", { name: "Expand" })); + expect(toggleExpanded).toHaveBeenCalled(); }); diff --git a/src/utils/test.ts b/src/utils/test.ts index 49a9800a..baef14b7 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -268,14 +268,13 @@ export function mockLocalParticipant( } as Partial as LocalParticipant; } -export async function withLocalMedia( +export function createLocalMedia( localRtcMember: CallMembership, roomMember: Partial, localParticipant: LocalParticipant, mediaDevices: MediaDevices, - continuation: (vm: LocalUserMediaViewModel) => void | Promise, -): Promise { - const vm = new LocalUserMediaViewModel( +): LocalUserMediaViewModel { + return new LocalUserMediaViewModel( testScope(), "local", mockMatrixRoomMember(localRtcMember, roomMember), @@ -290,8 +289,6 @@ export async function withLocalMedia( constant(null), constant(null), ); - // TODO: Simplify to just return the view model - await continuation(vm); } export function mockRemoteParticipant( @@ -307,14 +304,13 @@ export function mockRemoteParticipant( } as RemoteParticipant; } -export async function withRemoteMedia( +export function createRemoteMedia( localRtcMember: CallMembership, roomMember: Partial, participant: Partial, - continuation: (vm: RemoteUserMediaViewModel) => void | Promise, -): Promise { +): RemoteUserMediaViewModel { const remoteParticipant = mockRemoteParticipant(participant); - const vm = new RemoteUserMediaViewModel( + return new RemoteUserMediaViewModel( testScope(), "remote", mockMatrixRoomMember(localRtcMember, roomMember), @@ -329,8 +325,6 @@ export async function withRemoteMedia( constant(null), constant(null), ); - // TODO: Simplify to just return the view model - await continuation(vm); } export function mockConfig(config: Partial = {}): void {