diff --git a/src/room/CallEventAudioRenderer.test.tsx b/src/room/CallEventAudioRenderer.test.tsx index bb609323..46f517a8 100644 --- a/src/room/CallEventAudioRenderer.test.tsx +++ b/src/room/CallEventAudioRenderer.test.tsx @@ -6,18 +6,23 @@ Please see LICENSE in the repository root for full details. */ import { render } from "@testing-library/react"; -import { beforeEach, expect, test, vitest } from "vitest"; -import { MatrixClient } from "matrix-js-sdk/src/client"; +import { + afterAll, + afterEach, + beforeEach, + expect, + Mock, + MockedFunction, + test, + vitest, +} from "vitest"; import { ConnectionState, RemoteParticipant, Room } from "livekit-client"; import { of } from "rxjs"; import { act, ReactNode } from "react"; -import { soundEffectVolumeSetting } from "../settings/settings"; import { EmittableMockLivekitRoom, - mockLivekitRoom, mockLocalParticipant, - mockMatrixRoom, mockMatrixRoomMember, mockRemoteParticipant, } from "../utils/test"; @@ -27,12 +32,7 @@ import { CallEventAudioRenderer, MAX_PARTICIPANT_COUNT_FOR_SOUND, } from "./CallEventAudioRenderer"; -import { - prefetchSounds, - // We're using this from our mock, but it doesn't exist in the actual module. - //@ts-ignore - playSound, -} from "../useAudioContext"; +import { prefetchSounds, useAudioContext } from "../useAudioContext"; import { MockRoom, MockRTCSession, @@ -46,23 +46,29 @@ const bobId = `${bob.userId}:BBBB`; const localParticipant = mockLocalParticipant({ identity: "" }); const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const bobParticipant = mockRemoteParticipant({ identity: bobId }); -const leaveSound = "http://localhost:3000/src/sound/left_call.ogg"; -beforeEach(() => { - soundEffectVolumeSetting.setValue(soundEffectVolumeSetting.defaultValue); +vitest.mock("../useAudioContext"); + +afterEach(() => { + vitest.resetAllMocks(); }); -vitest.mock("../useAudioContext", async () => { - const playSound = vitest.fn(); - return { - prefetchSounds: vitest.fn().mockReturnValueOnce({ - sound: new ArrayBuffer(0), - }), +afterAll(() => { + vitest.restoreAllMocks(); +}); + +let playSound: Mock< + NonNullable>["playSound"] +>; + +beforeEach(() => { + (prefetchSounds as MockedFunction).mockResolvedValue({ + sound: new ArrayBuffer(0), + }); + playSound = vitest.fn(); + (useAudioContext as MockedFunction).mockReturnValue({ playSound, - useAudioContext: () => ({ - playSound, - }), - }; + }); }); function TestComponent({ @@ -86,49 +92,29 @@ function TestComponent({ * participants join from our perspective. We don't want to make * a noise every time. */ -test("does NOT play a sound when entering a call", () => { - const members = new Map([alice, bob].map((p) => [p.userId, p])); - const remoteParticipants = of([aliceParticipant]); - const liveKitRoom = mockLivekitRoom( - { localParticipant }, - { remoteParticipants }, - ); +test("plays one sound when entering a call", () => { + const liveKitRoom = new EmittableMockLivekitRoom({ + localParticipant, + remoteParticipants: new Map(), + }); + const room = new MockRoom(alice.userId); const vm = new CallViewModel( - room as any, - liveKitRoom, + room.testGetAsMatrixRoom(), + liveKitRoom.getAsLivekitRoom(), { kind: E2eeType.PER_PARTICIPANT, }, of(ConnectionState.Connected), ); - render(); - expect(playSound).not.toBeCalled(); -}); - -test("plays no sound when muted", () => { - soundEffectVolumeSetting.setValue(0); - const members = new Map([alice, bob].map((p) => [p.userId, p])); - const remoteParticipants = of([aliceParticipant, bobParticipant]); - const liveKitRoom = mockLivekitRoom( - { localParticipant }, - { remoteParticipants }, - ); - - const room = new MockRoom(alice.userId); - const vm = new CallViewModel( - room as any, - liveKitRoom, - { - kind: E2eeType.PER_PARTICIPANT, - }, - of(ConnectionState.Connected), - ); + // Joining a call usually means remote participants are added later. + act(() => { + liveKitRoom.addParticipant(bobParticipant); + }); render(); - // Play a sound when joining a call. - expect(playSound).not.toBeCalled(); + expect(playSound).toBeCalled(); }); test("plays a sound when a user joins", () => { @@ -142,7 +128,7 @@ test("plays a sound when a user joins", () => { const room = new MockRoom(alice.userId); const vm = new CallViewModel( - room as any, + room.testGetAsMatrixRoom(), liveKitRoom as unknown as Room, { kind: E2eeType.PER_PARTICIPANT, @@ -169,8 +155,8 @@ test("plays a sound when a user leaves", () => { const room = new MockRoom(alice.userId); const vm = new CallViewModel( - room as any, - liveKitRoom as unknown as Room, + room.testGetAsMatrixRoom(), + liveKitRoom.getAsLivekitRoom(), { kind: E2eeType.PER_PARTICIPANT, }, @@ -181,36 +167,44 @@ test("plays a sound when a user leaves", () => { act(() => { liveKitRoom.removeParticipant(aliceParticipant); }); - expect(playSound).toBeCalledWith("leave"); + expect(playSound).toBeCalledWith("left"); }); test("plays no sound when the participant list is more than the maximum size", () => { + expect(playSound).not.toBeCalled(); const remoteParticipants = new Map([ [aliceParticipant.identity, aliceParticipant], + // You + other participants to hit the max. ...Array.from({ length: MAX_PARTICIPANT_COUNT_FOR_SOUND - 1 }).map< [string, RemoteParticipant] >((_, index) => { - const p = mockRemoteParticipant({ identity: `user${index}` }); + const p = mockRemoteParticipant({ + identity: `@user${index}:example.com:DEV${index}`, + }); return [p.identity, p]; }), ]); + + // Preload the call with the maximum members, assume that + // we're already in the call by this point rather than + // joining. const liveKitRoom = new EmittableMockLivekitRoom({ localParticipant, remoteParticipants, }); const room = new MockRoom(alice.userId); const vm = new CallViewModel( - room as any, - liveKitRoom as unknown as Room, + room.testGetAsMatrixRoom(), + liveKitRoom.getAsLivekitRoom(), { kind: E2eeType.PER_PARTICIPANT, }, of(ConnectionState.Connected), ); render(); - // When the count drops + // When the count drops, play a leave sound. act(() => { liveKitRoom.removeParticipant(aliceParticipant); }); - expect(playSound).not.toBeCalled(); + expect(playSound).toBeCalledWith("left"); }); diff --git a/src/room/CallEventAudioRenderer.tsx b/src/room/CallEventAudioRenderer.tsx index 8f4576f2..0e410ffb 100644 --- a/src/room/CallEventAudioRenderer.tsx +++ b/src/room/CallEventAudioRenderer.tsx @@ -6,7 +6,8 @@ Please see LICENSE in the repository root for full details. */ import { ReactNode, useDeferredValue, useEffect, useMemo } from "react"; -import { debounce, filter, interval, tap, throttle } from "rxjs"; +import { filter, interval, throttle } from "rxjs"; + import { CallViewModel } from "../state/CallViewModel"; import joinCallSoundMp3 from "../sound/join_call.mp3"; import joinCallSoundOgg from "../sound/join_call.ogg"; diff --git a/src/room/ReactionAudioRenderer.test.tsx b/src/room/ReactionAudioRenderer.test.tsx index 9d631d18..a6471114 100644 --- a/src/room/ReactionAudioRenderer.test.tsx +++ b/src/room/ReactionAudioRenderer.test.tsx @@ -6,9 +6,18 @@ Please see LICENSE in the repository root for full details. */ import { render } from "@testing-library/react"; -import { afterAll, expect, test, vitest } from "vitest"; +import { + afterAll, + beforeEach, + expect, + test, + vitest, + MockedFunction, + Mock, +} from "vitest"; import { TooltipProvider } from "@vector-im/compound-web"; import { act, ReactNode } from "react"; +import { afterEach } from "node:test"; import { MockRoom, @@ -20,14 +29,8 @@ import { playReactionsSound, soundEffectVolumeSetting, } from "../settings/settings"; -import { - prefetchSounds, - // We're using this from our mock, but it doesn't exist in the actual module. - //@ts-ignore - playSound, -} from "../useAudioContext"; +import { prefetchSounds, useAudioContext } from "../useAudioContext"; import { GenericReaction, ReactionSet } from "../reactions"; -import { afterEach } from "node:test"; const memberUserIdAlice = "@alice:example.org"; const memberUserIdBob = "@bob:example.org"; @@ -56,21 +59,10 @@ function TestComponent({ ); } -vitest.mock("../useAudioContext", async () => { - const playSound = vitest.fn(); - return { - prefetchSounds: vitest.fn().mockReturnValueOnce({ - sound: new ArrayBuffer(0), - }), - playSound, - useAudioContext: () => ({ - playSound, - }), - }; -}); +vitest.mock("../useAudioContext"); afterEach(() => { - vitest.clearAllMocks(); + vitest.resetAllMocks(); playReactionsSound.setValue(playReactionsSound.defaultValue); soundEffectVolumeSetting.setValue(soundEffectVolumeSetting.defaultValue); }); @@ -79,6 +71,20 @@ afterAll(() => { vitest.restoreAllMocks(); }); +let playSound: Mock< + NonNullable>["playSound"] +>; + +beforeEach(() => { + (prefetchSounds as MockedFunction).mockResolvedValue({ + sound: new ArrayBuffer(0), + }); + playSound = vitest.fn(); + (useAudioContext as MockedFunction).mockReturnValue({ + playSound, + }); +}); + test("preloads all audio elements", () => { playReactionsSound.setValue(true); const rtcSession = new MockRTCSession( diff --git a/src/room/ReactionAudioRenderer.tsx b/src/room/ReactionAudioRenderer.tsx index 487157f7..22ca2fcd 100644 --- a/src/room/ReactionAudioRenderer.tsx +++ b/src/room/ReactionAudioRenderer.tsx @@ -46,7 +46,6 @@ export function ReactionsAudioRenderer(): ReactNode { // Don't replay old reactions return; } - console.log("playing sound", reactionName); if (SoundMap[reactionName]) { audioEngineRef.current.playSound(reactionName); } else { diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 8d8aaa43..73320329 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -302,7 +302,7 @@ function findMatrixRoomMember( // must be at least 3 parts because we know the first part is a userId which must necessarily contain a colon if (parts.length < 3) { logger.warn( - "Livekit participants ID doesn't look like a userId:deviceId combination", + `Livekit participants ID (${id}) doesn't look like a userId:deviceId combination`, ); return undefined; } diff --git a/src/useAudioContext.test.tsx b/src/useAudioContext.test.tsx index 99a5f725..81cf7d69 100644 --- a/src/useAudioContext.test.tsx +++ b/src/useAudioContext.test.tsx @@ -1,22 +1,23 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + import { expect, test, vitest } from "vitest"; -import { useAudioContext } from "./useAudioContext"; import { FC } from "react"; import { render } from "@testing-library/react"; -import { deviceStub, MediaDevicesContext } from "./livekit/MediaDevicesContext"; import { afterEach } from "node:test"; -import { soundEffectVolumeSetting } from "./settings/settings"; -/** - * Test explanation. - * This test suite checks that the useReactions hook appropriately reacts - * to new reactions, redactions and membership changesin the room. There is - * a large amount of test structure used to construct a mock environment. - */ +import { deviceStub, MediaDevicesContext } from "./livekit/MediaDevicesContext"; +import { useAudioContext } from "./useAudioContext"; +import { soundEffectVolumeSetting } from "./settings/settings"; const TestComponent: FC = () => { const audioCtx = useAudioContext({ sounds: Promise.resolve({ - aSound: new ArrayBuffer(32), + aSound: new ArrayBuffer(0), }), latencyHint: "balanced", }); @@ -25,13 +26,9 @@ const TestComponent: FC = () => { } return ( <> - - + {/* eslint-disable-next-line @typescript-eslint/no-explicit-any*/} + @@ -39,9 +36,9 @@ const TestComponent: FC = () => { }; class MockAudioContext { - static testContext: MockAudioContext; + public static testContext: MockAudioContext; - constructor() { + public constructor() { MockAudioContext.testContext = this; } @@ -88,7 +85,7 @@ test("will ignore sounds that are not registered", async () => { ).not.toHaveBeenCalled(); }); -test("will use the correct device", async () => { +test("will use the correct device", () => { vitest.stubGlobal("AudioContext", MockAudioContext); render( ( sounds: Record, ): PrefetchedSounds { - logger.debug(`Loading sounds`); const buffers: Record = {}; await Promise.all( Object.entries(sounds).map(async ([name, file]) => { const { mp3, ogg } = file as SoundDefinition; // Use preferred format, fallback to ogg if no mp3 is provided. - buffers[name] = await fetchBuffer( + // Load an audio file + const response = await fetch( PreferredFormat === "ogg" ? ogg : (mp3 ?? ogg), ); + if (!response.ok) { + // If the sound doesn't load, it's not the end of the world. We won't play + // the sound when requested, but it's better than failing the whole application. + logger.warn(`Could not load sound ${name}, resposne was not okay`); + return; + } + // Decode it + buffers[name] = await response.arrayBuffer(); }), ); return buffers as Record; @@ -96,23 +116,22 @@ export function useAudioContext( const devices = useMediaDevices(); const [audioContext, setAudioContext] = useState(); const [audioBuffers, setAudioBuffers] = useState>(); - const soundCache = useInitial(() => props.sounds); + const soundCache = useInitial(async () => props.sounds); useEffect(() => { const ctx = new AudioContext({ // We want low latency for these effects. latencyHint: props.latencyHint, - // XXX: Types don't include this yet. - ...{ sinkId: devices.audioOutput.selectedId }, }); - const controller = new AbortController(); - (async () => { + + // We want to clone the content of our preloaded + // sound buffers into this context. The context may + // close during this process, so it's okay if it throws. + (async (): Promise => { const buffers: Record = {}; - controller.signal.throwIfAborted(); for (const [name, buffer] of Object.entries(await soundCache)) { - controller.signal.throwIfAborted(); - // Type quirk, this is *definitely* a ArrayBuffer. const audioBuffer = await ctx.decodeAudioData( + // Type quirk, this is *definitely* a ArrayBuffer. (buffer as ArrayBuffer).slice(0), ); buffers[name] = audioBuffer; @@ -123,8 +142,7 @@ export function useAudioContext( }); setAudioContext(ctx); - return () => { - controller.abort("Closing"); + return (): void => { void ctx.close().catch((ex) => { logger.debug("Failed to close audio engine", ex); }); @@ -135,22 +153,22 @@ export function useAudioContext( // Update the sink ID whenever we change devices. useEffect(() => { if (audioContext && "setSinkId" in audioContext) { - // setSinkId doesn't exist in types but does exist for some browsers. // https://developer.mozilla.org/en-US/docs/Web/API/AudioContext/setSinkId - // @ts-ignore + // @ts-expect-error - setSinkId doesn't exist yet in types, maybe because it's not supported everywhere. audioContext.setSinkId(devices.audioOutput.selectedId).catch((ex) => { logger.warn("Unable to change sink for audio context", ex); }); } }, [audioContext, devices]); + // Don't return a function until we're ready. if (!audioContext || !audioBuffers) { logger.debug("Audio not ready yet"); return null; } return { - playSound: (name) => { - playSound(effectSoundVolume, audioContext, audioBuffers[name]); + playSound: (name): void => { + playSound(audioContext, audioBuffers[name], effectSoundVolume); }, }; } diff --git a/src/utils/test.ts b/src/utils/test.ts index 607e1b12..08fff4fb 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -136,6 +136,10 @@ export class EmittableMockLivekitRoom extends EventEmitter { this.remoteParticipants.delete(remoteParticipant.identity); this.emit(RoomEvent.ParticipantDisconnected, remoteParticipant); } + + public getAsLivekitRoom(): LivekitRoom { + return this as unknown as LivekitRoom; + } } export function mockLivekitRoom( diff --git a/src/utils/testReactions.tsx b/src/utils/testReactions.tsx index 4e4aff5e..9a19a5e6 100644 --- a/src/utils/testReactions.tsx +++ b/src/utils/testReactions.tsx @@ -204,7 +204,11 @@ export class MockRoom extends EventEmitter { return evt.getId()!; } - public getMember() { - return undefined; + public getMember(): void { + return; + } + + public testGetAsMatrixRoom(): Room { + return this as unknown as Room; } }