fixup tests / comments

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 {
-import { MatrixClient } from "matrix-js-sdk/src/client";
+  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 {
+import { prefetchSounds, useAudioContext } from "../useAudioContext";
-  prefetchSounds,
-  // We're using this from our mock, but it doesn't exist in the actual module.
-  //@ts-ignore
-  playSound,
-} 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(() => {
+vitest.mock("../useAudioContext");
-  soundEffectVolumeSetting.setValue(soundEffectVolumeSetting.defaultValue);
+
+afterEach(() => {
+  vitest.resetAllMocks();
 });
 
-vitest.mock("../useAudioContext", async () => {
+afterAll(() => {
-  const playSound = vitest.fn();
+  vitest.restoreAllMocks();
-  return {
+});
-    prefetchSounds: vitest.fn().mockReturnValueOnce({
+
-      sound: new ArrayBuffer(0),
+let playSound: Mock<
-    }),
+  NonNullable<ReturnType<typeof useAudioContext>>["playSound"]
+>;
+
+beforeEach(() => {
+  (prefetchSounds as MockedFunction<typeof prefetchSounds>).mockResolvedValue({
+    sound: new ArrayBuffer(0),
+  });
+  playSound = vitest.fn();
+  (useAudioContext as MockedFunction<typeof useAudioContext>).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", () => {
+test("plays one sound when entering a call", () => {
-  const members = new Map([alice, bob].map((p) => [p.userId, p]));
+  const liveKitRoom = new EmittableMockLivekitRoom({
-  const remoteParticipants = of([aliceParticipant]);
+    localParticipant,
-  const liveKitRoom = mockLivekitRoom(
+    remoteParticipants: new Map(),
-    { localParticipant },
+  });
-    { remoteParticipants },
+
-  );
   const room = new MockRoom(alice.userId);
   const vm = new CallViewModel(
-    room as any,
+    room.testGetAsMatrixRoom(),
-    liveKitRoom,
+    liveKitRoom.getAsLivekitRoom(),
     {
       kind: E2eeType.PER_PARTICIPANT,
     },
     of(ConnectionState.Connected),
   );
 
-  render(<TestComponent room={room} vm={vm} />);
+  // Joining a call usually means remote participants are added later.
-  expect(playSound).not.toBeCalled();
+  act(() => {
-});
+    liveKitRoom.addParticipant(bobParticipant);
-
+  });
-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),
-  );
 
   render(<TestComponent room={room} vm={vm} />);
-  // Play a sound when joining a call.
+  expect(playSound).toBeCalled();
-  expect(playSound).not.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,
+    room.testGetAsMatrixRoom(),
-    liveKitRoom as unknown as Room,
+    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<string, RemoteParticipant>([
     [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,
+    room.testGetAsMatrixRoom(),
-    liveKitRoom as unknown as Room,
+    liveKitRoom.getAsLivekitRoom(),
     {
       kind: E2eeType.PER_PARTICIPANT,
     },
     of(ConnectionState.Connected),
   );
   render(<TestComponent room={room} vm={vm} />);
-  // When the count drops
+  // When the count drops, play a leave sound.
   act(() => {
     liveKitRoom.removeParticipant(aliceParticipant);
   });
-  expect(playSound).not.toBeCalled();
+  expect(playSound).toBeCalledWith("left");
 });

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";

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 {
+import { prefetchSounds, useAudioContext } from "../useAudioContext";
-  prefetchSounds,
-  // We're using this from our mock, but it doesn't exist in the actual module.
-  //@ts-ignore
-  playSound,
-} 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 () => {
+vitest.mock("../useAudioContext");
-  const playSound = vitest.fn();
-  return {
-    prefetchSounds: vitest.fn().mockReturnValueOnce({
-      sound: new ArrayBuffer(0),
-    }),
-    playSound,
-    useAudioContext: () => ({
-      playSound,
-    }),
-  };
-});
 
 afterEach(() => {
-  vitest.clearAllMocks();
+  vitest.resetAllMocks();
   playReactionsSound.setValue(playReactionsSound.defaultValue);
   soundEffectVolumeSetting.setValue(soundEffectVolumeSetting.defaultValue);
 });
@@ -79,6 +71,20 @@ afterAll(() => {
   vitest.restoreAllMocks();
 });
 
+let playSound: Mock<
+  NonNullable<ReturnType<typeof useAudioContext>>["playSound"]
+>;
+
+beforeEach(() => {
+  (prefetchSounds as MockedFunction<typeof prefetchSounds>).mockResolvedValue({
+    sound: new ArrayBuffer(0),
+  });
+  playSound = vitest.fn();
+  (useAudioContext as MockedFunction<typeof useAudioContext>).mockReturnValue({
+    playSound,
+  });
+});
+
 test("preloads all audio elements", () => {
   playReactionsSound.setValue(true);
   const rtcSession = new MockRTCSession(

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 {

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;
   }

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";
 
-/**
+import { deviceStub, MediaDevicesContext } from "./livekit/MediaDevicesContext";
- * Test explanation.
+import { useAudioContext } from "./useAudioContext";
- * This test suite checks that the useReactions hook appropriately reacts
+import { soundEffectVolumeSetting } from "./settings/settings";
- * to new reactions, redactions and membership changesin the room. There is
- * a large amount of test structure used to construct a mock environment.
- */
 
 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 (
     <>
-      <button role="button" onClick={() => audioCtx.playSound("aSound")}>
+      <button onClick={() => audioCtx.playSound("aSound")}>Valid sound</button>
-        Valid sound
+      {/* eslint-disable-next-line @typescript-eslint/no-explicit-any*/}
-      </button>
+      <button onClick={() => audioCtx.playSound("not-valid" as any)}>
-      <button
-        role="button"
-        onClick={() => audioCtx.playSound("not-valid" as any)}
-      >
         Invalid sound
       </button>
     </>
@@ -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(
     <MediaDevicesContext.Provider

src/useAudioContext.tsx

@@ -1,5 +1,13 @@
+/*
+Copyright 2024 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only
+Please see LICENSE in the repository root for full details.
+*/
+
 import { logger } from "matrix-js-sdk/src/logger";
 import { useState, useEffect } from "react";
+
 import {
   soundEffectVolumeSetting as effectSoundVolumeSetting,
   useSetting,
@@ -9,20 +17,18 @@ import { useInitial } from "./useInitial";
 
 type SoundDefinition = { mp3?: string; ogg: string };
 
-async function fetchBuffer(filename: string) {
+/**
-  // Load an audio file
+ * Play a sound though a given AudioContext. Will take
-  const response = await fetch(filename);
+ * care of connecting the correct buffer and gating
-  if (!response.ok) {
+ * through gain.
-    throw Error("Could not load sound, resposne was not okay");
+ * @param volume The volume to play at.
-  }
+ * @param ctx The context to play through.
-  // Decode it
+ * @param buffer The buffer to play.
-  return await await response.arrayBuffer();
+ */
-}
-
 function playSound(
+  ctx: AudioContext,
+  buffer: AudioBuffer,
   volume: number,
-  ctx?: AudioContext,
-  buffer?: AudioBuffer,
 ): void {
   if (!ctx || !buffer) {
     return;
@@ -35,7 +41,13 @@ function playSound(
   src.start();
 }
 
-function getPreferredAudioFormat() {
+/**
+ * Determine the best format we can use to play our sounds
+ * through. We prefer ogg support if possible, but will fall
+ * back to MP3.
+ * @returns "ogg" if the browser is likely to support it, or "mp3" otherwise.
+ */
+function getPreferredAudioFormat(): "ogg" | "mp3" {
   const a = document.createElement("audio");
   if (a.canPlayType("audio/ogg") === "maybe") {
     return "ogg";
@@ -60,15 +72,23 @@ const PreferredFormat = getPreferredAudioFormat();
 export async function prefetchSounds<S extends string>(
   sounds: Record<S, SoundDefinition>,
 ): PrefetchedSounds<S> {
-  logger.debug(`Loading sounds`);
   const buffers: Record<string, ArrayBuffer> = {};
   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<S, ArrayBuffer>;
@@ -96,23 +116,22 @@ export function useAudioContext<S extends string>(
   const devices = useMediaDevices();
   const [audioContext, setAudioContext] = useState<AudioContext>();
   const [audioBuffers, setAudioBuffers] = useState<Record<S, AudioBuffer>>();
-  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<void> => {
       const buffers: Record<string, AudioBuffer> = {};
-      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<S extends string>(
     });
 
     setAudioContext(ctx);
-    return () => {
+    return (): void => {
-      controller.abort("Closing");
       void ctx.close().catch((ex) => {
         logger.debug("Failed to close audio engine", ex);
       });
@@ -135,22 +153,22 @@ export function useAudioContext<S extends string>(
   // 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: (name): void => {
-      playSound(effectSoundVolume, audioContext, audioBuffers[name]);
+      playSound(audioContext, audioBuffers[name], effectSoundVolume);
     },
   };
 }

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(

src/utils/testReactions.tsx

@@ -204,7 +204,11 @@ export class MockRoom extends EventEmitter {
     return evt.getId()!;
   }
 
-  public getMember() {
+  public getMember(): void {
-    return undefined;
+    return;
+  }
+
+  public testGetAsMatrixRoom(): Room {
+    return this as unknown as Room;
   }
 }