Test CallViewModel

This adds tests for a couple of the less trivial bits of code in CallViewModel. Testing them helped me uncover why focus switches still weren't being smooth! (It was because I was using RxJS's sample operator when I really wanted withLatestFrom.)

src/state/CallViewModel.test.ts

@@ -0,0 +1,276 @@
+/*
+Copyright 2024 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only
+Please see LICENSE in the repository root for full details.
+*/
+
+import { test, vi, onTestFinished } from "vitest";
+import { map, Observable } from "rxjs";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+import {
+  ConnectionState,
+  LocalParticipant,
+  RemoteParticipant,
+} from "livekit-client";
+import * as ComponentsCore from "@livekit/components-core";
+
+import { CallViewModel, Layout } from "./CallViewModel";
+import {
+  mockLivekitRoom,
+  mockLocalParticipant,
+  mockMatrixRoom,
+  mockMember,
+  mockRemoteParticipant,
+  OurRunHelpers,
+  withTestScheduler,
+} from "../utils/test";
+import {
+  ECAddonConnectionState,
+  ECConnectionState,
+} from "../livekit/useECConnectionState";
+
+vi.mock("@livekit/components-core");
+
+const aliceId = "@alice:example.org:AAAA";
+const bobId = "@bob:example.org:BBBB";
+
+const alice = mockMember({ userId: "@alice:example.org" });
+const bob = mockMember({ userId: "@bob:example.org" });
+const carol = mockMember({ userId: "@carol:example.org" });
+
+const localParticipant = mockLocalParticipant({ identity: "" });
+const aliceParticipant = mockRemoteParticipant({ identity: aliceId });
+const aliceSharingScreen = mockRemoteParticipant({
+  identity: aliceId,
+  isScreenShareEnabled: true,
+});
+const bobParticipant = mockRemoteParticipant({ identity: bobId });
+const bobSharingScreen = mockRemoteParticipant({
+  identity: bobId,
+  isScreenShareEnabled: true,
+});
+
+const members = new Map([
+  [alice.userId, alice],
+  [bob.userId, bob],
+  [carol.userId, carol],
+]);
+
+export interface GridLayoutSummary {
+  type: "grid";
+  spotlight?: string[];
+  grid: string[];
+}
+
+export interface SpotlightLandscapeLayoutSummary {
+  type: "spotlight-landscape";
+  spotlight: string[];
+  grid: string[];
+}
+
+export interface SpotlightPortraitLayoutSummary {
+  type: "spotlight-portrait";
+  spotlight: string[];
+  grid: string[];
+}
+
+export interface SpotlightExpandedLayoutSummary {
+  type: "spotlight-expanded";
+  spotlight: string[];
+  pip?: string;
+}
+
+export interface OneOnOneLayoutSummary {
+  type: "one-on-one";
+  local: string;
+  remote: string;
+}
+
+export interface PipLayoutSummary {
+  type: "pip";
+  spotlight: string[];
+}
+
+export type LayoutSummary =
+  | GridLayoutSummary
+  | SpotlightLandscapeLayoutSummary
+  | SpotlightPortraitLayoutSummary
+  | SpotlightExpandedLayoutSummary
+  | OneOnOneLayoutSummary
+  | PipLayoutSummary;
+
+function summarizeLayout(l: Layout): LayoutSummary {
+  switch (l.type) {
+    case "grid":
+      return {
+        type: l.type,
+        spotlight: l.spotlight?.map((vm) => vm.id),
+        grid: l.grid.map((vm) => vm.id),
+      };
+    case "spotlight-landscape":
+    case "spotlight-portrait":
+      return {
+        type: l.type,
+        spotlight: l.spotlight.map((vm) => vm.id),
+        grid: l.grid.map((vm) => vm.id),
+      };
+    case "spotlight-expanded":
+      return {
+        type: l.type,
+        spotlight: l.spotlight.map((vm) => vm.id),
+        pip: l.pip?.id,
+      };
+    case "one-on-one":
+      return { type: l.type, local: l.local.id, remote: l.remote.id };
+    case "pip":
+      return { type: l.type, spotlight: l.spotlight.map((vm) => vm.id) };
+  }
+}
+
+function withCallViewModel(
+  { cold }: OurRunHelpers,
+  remoteParticipants: Observable<RemoteParticipant[]>,
+  connectionState: Observable<ECConnectionState>,
+  continuation: (vm: CallViewModel) => void,
+): void {
+  const participantsSpy = vi
+    .spyOn(ComponentsCore, "connectedParticipantsObserver")
+    .mockReturnValue(remoteParticipants);
+  const mediaSpy = vi
+    .spyOn(ComponentsCore, "observeParticipantMedia")
+    .mockImplementation((p) =>
+      cold("a", {
+        a: { participant: p } as Partial<
+          ComponentsCore.ParticipantMedia<LocalParticipant>
+        > as ComponentsCore.ParticipantMedia<LocalParticipant>,
+      }),
+    );
+  const eventsSpy = vi
+    .spyOn(ComponentsCore, "observeParticipantEvents")
+    .mockImplementation((p) => cold("a", { a: p }));
+
+  const vm = new CallViewModel(
+    mockMatrixRoom({
+      client: {
+        getUserId: () => "@carol:example.org",
+      } as Partial<MatrixClient> as MatrixClient,
+      getMember: (userId) => members.get(userId) ?? null,
+    }),
+    mockLivekitRoom({ localParticipant }),
+    true,
+    connectionState,
+  );
+
+  onTestFinished(() => {
+    vm!.destroy();
+    participantsSpy!.mockRestore();
+    mediaSpy!.mockRestore();
+    eventsSpy!.mockRestore();
+  });
+
+  continuation(vm);
+}
+
+test("participants are retained during a focus switch", () => {
+  withTestScheduler((helpers) => {
+    const { hot, expectObservable } = helpers;
+    // Participants disappear on frame 2 and come back on frame 3
+    const partMarbles = "a-ba";
+    // Start switching focus on frame 1 and reconnect on frame 3
+    const connMarbles = "ab-a";
+    // The visible participants should remain the same throughout the switch
+    const laytMarbles = "aaaa 2997ms a 56998ms a";
+
+    withCallViewModel(
+      helpers,
+      hot(partMarbles, {
+        a: [aliceParticipant, bobParticipant],
+        b: [],
+      }),
+      hot(connMarbles, {
+        a: ConnectionState.Connected,
+        b: ECAddonConnectionState.ECSwitchingFocus,
+      }),
+      (vm) => {
+        expectObservable(vm.layout.pipe(map(summarizeLayout))).toBe(
+          laytMarbles,
+          {
+            a: {
+              type: "grid",
+              spotlight: undefined,
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+          },
+        );
+      },
+    );
+  });
+});
+
+test("screen sharing activates spotlight layout", () => {
+  withTestScheduler((helpers) => {
+    const { hot, schedule, expectObservable } = helpers;
+    // Start with no screen shares, then have Alice and Bob share their screens,
+    // then return to no screen shares, then have just Alice share for a bit
+    const partMarbles = "abc---d---a-b---a";
+    // While there are no screen shares, switch to spotlight manually, and then
+    // switch back to grid at the end
+    const modeMarbles = "-----------a--------b";
+    // We should automatically enter spotlight for the first round of screen
+    // sharing, then return to grid, then manually go into spotlight, and
+    // remain in spotlight until we manually go back to grid
+    const laytMarbles = "ab(cc)(dd)ae(bb)(ee)a 59979ms a";
+
+    withCallViewModel(
+      helpers,
+      hot(partMarbles, {
+        a: [aliceParticipant, bobParticipant],
+        b: [aliceSharingScreen, bobParticipant],
+        c: [aliceSharingScreen, bobSharingScreen],
+        d: [aliceParticipant, bobSharingScreen],
+      }),
+      hot("a", { a: ConnectionState.Connected }),
+      (vm) => {
+        schedule(modeMarbles, {
+          a: () => vm.setGridMode("spotlight"),
+          b: () => vm.setGridMode("grid"),
+        });
+
+        expectObservable(vm.layout.pipe(map(summarizeLayout))).toBe(
+          laytMarbles,
+          {
+            a: {
+              type: "grid",
+              spotlight: undefined,
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+            b: {
+              type: "spotlight-landscape",
+              spotlight: [`${aliceId}:0:screen-share`],
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+            c: {
+              type: "spotlight-landscape",
+              spotlight: [
+                `${aliceId}:0:screen-share`,
+                `${bobId}:0:screen-share`,
+              ],
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+            d: {
+              type: "spotlight-landscape",
+              spotlight: [`${bobId}:0:screen-share`],
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+            e: {
+              type: "spotlight-landscape",
+              spotlight: [`${aliceId}:0`],
+              grid: [":0", `${aliceId}:0`, `${bobId}:0`],
+            },
+          },
+        );
+      },
+    );
+  });
+});

src/state/CallViewModel.ts

@@ -26,13 +26,13 @@ import {
   concat,
   distinctUntilChanged,
   filter,
+  forkJoin,
   fromEvent,
   map,
   merge,
-  mergeAll,
+  mergeMap,
   of,
   race,
-  sample,
   scan,
   skip,
   startWith,
@@ -42,7 +42,7 @@ import {
   take,
   throttleTime,
   timer,
-  zip,
+  withLatestFrom,
 } from "rxjs";
 import { logger } from "matrix-js-sdk/src/logger";
 
@@ -165,10 +165,19 @@ class UserMedia {
     participant: LocalParticipant | RemoteParticipant,
     callEncrypted: boolean,
   ) {
-    this.vm =
+    this.vm = participant.isLocal
-      participant instanceof LocalParticipant
+      ? new LocalUserMediaViewModel(
-        ? new LocalUserMediaViewModel(id, member, participant, callEncrypted)
+          id,
-        : new RemoteUserMediaViewModel(id, member, participant, callEncrypted);
+          member,
+          participant as LocalParticipant,
+          callEncrypted,
+        )
+      : new RemoteUserMediaViewModel(
+          id,
+          member,
+          participant as RemoteParticipant,
+          callEncrypted,
+        );
 
     this.speaker = this.vm.speaking.pipe(
       // Require 1 s of continuous speaking to become a speaker, and 60 s of
@@ -182,6 +191,7 @@ class UserMedia {
         ),
       ),
       startWith(false),
+      distinctUntilChanged(),
       // Make this Observable hot so that the timers don't reset when you
       // resubscribe
       this.scope.state(),
@@ -252,10 +262,9 @@ export class CallViewModel extends ViewModel {
   // Lists of participants to "hold" on display, even if LiveKit claims that
   // they've left
   private readonly remoteParticipantHolds: Observable<RemoteParticipant[][]> =
-    zip(
+    this.connectionState.pipe(
-      this.connectionState,
+      withLatestFrom(this.rawRemoteParticipants),
-      this.rawRemoteParticipants.pipe(sample(this.connectionState)),
+      mergeMap(([s, ps]) => {
-      (s, ps) => {
         // Whenever we switch focuses, we should retain all the previous
         // participants for at least POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS ms to
         // give their clients time to switch over and avoid jarring layout shifts
@@ -264,29 +273,19 @@ export class CallViewModel extends ViewModel {
             // Hold these participants
             of({ hold: ps }),
             // Wait for time to pass and the connection state to have changed
-            Promise.all([
+            forkJoin([
-              new Promise<void>((resolve) =>
+              timer(POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS),
-                setTimeout(resolve, POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS),
+              this.connectionState.pipe(
+                filter((s) => s !== ECAddonConnectionState.ECSwitchingFocus),
+                take(1),
               ),
-              new Promise<void>((resolve) => {
-                const subscription = this.connectionState
-                  .pipe(this.scope.bind())
-                  .subscribe((s) => {
-                    if (s !== ECAddonConnectionState.ECSwitchingFocus) {
-                      resolve();
-                      subscription.unsubscribe();
-                    }
-                  });
-              }),
               // Then unhold them
-            ]).then(() => ({ unhold: ps })),
+            ]).pipe(map(() => ({ unhold: ps }))),
           );
         } else {
           return EMPTY;
         }
-      },
+      }),
-    ).pipe(
-      mergeAll(),
       // Accumulate the hold instructions into a single list showing which
       // participants are being held
       accumulate([] as RemoteParticipant[][], (holds, instruction) =>

src/utils/test.ts

@@ -7,12 +7,13 @@ Please see LICENSE in the repository root for full details.
 import { map } from "rxjs";
 import { RunHelpers, TestScheduler } from "rxjs/testing";
 import { expect, vi } from "vitest";
-import { RoomMember } from "matrix-js-sdk/src/matrix";
+import { RoomMember, Room as MatrixRoom } from "matrix-js-sdk/src/matrix";
 import {
   LocalParticipant,
   LocalTrackPublication,
   RemoteParticipant,
   RemoteTrackPublication,
+  Room as LivekitRoom,
 } from "livekit-client";
 
 import {
@@ -62,7 +63,7 @@ export function withTestScheduler(
   );
 }
 
-function mockMember(member: Partial<RoomMember>): RoomMember {
+export function mockMember(member: Partial<RoomMember>): RoomMember {
   return {
     on() {
       return this;
@@ -80,14 +81,47 @@ function mockMember(member: Partial<RoomMember>): RoomMember {
   } as RoomMember;
 }
 
-export async function withLocalMedia(
+export function mockMatrixRoom(room: Partial<MatrixRoom>): MatrixRoom {
-  member: Partial<RoomMember>,
+  return {
-  continuation: (vm: LocalUserMediaViewModel) => void | Promise<void>,
+    on() {
-): Promise<void> {
+      return this as MatrixRoom;
-  const vm = new LocalUserMediaViewModel(
+    },
-    "local",
+    off() {
-    mockMember(member),
+      return this as MatrixRoom;
-    {
+    },
+    addEventListener() {
+      return this as MatrixRoom;
+    },
+    removeEventListener() {
+      return this as MatrixRoom;
+    },
+    ...room,
+  } as Partial<MatrixRoom> as MatrixRoom;
+}
+
+export function mockLivekitRoom(room: Partial<LivekitRoom>): LivekitRoom {
+  return {
+    on() {
+      return this as LivekitRoom;
+    },
+    off() {
+      return this as LivekitRoom;
+    },
+    addEventListener() {
+      return this as LivekitRoom;
+    },
+    removeEventListener() {
+      return this as LivekitRoom;
+    },
+    ...room,
+  } as Partial<LivekitRoom> as LivekitRoom;
+}
+
+export function mockLocalParticipant(
+  participant: Partial<LocalParticipant>,
+): LocalParticipant {
+  return {
+    isLocal: true,
     getTrackPublication: () =>
       ({}) as Partial<LocalTrackPublication> as LocalTrackPublication,
     on() {
@@ -102,7 +136,18 @@ export async function withLocalMedia(
     removeListener() {
       return this as LocalParticipant;
     },
-    } as Partial<LocalParticipant> as LocalParticipant,
+    ...participant,
+  } as Partial<LocalParticipant> as LocalParticipant;
+}
+
+export async function withLocalMedia(
+  member: Partial<RoomMember>,
+  continuation: (vm: LocalUserMediaViewModel) => void | Promise<void>,
+): Promise<void> {
+  const vm = new LocalUserMediaViewModel(
+    "local",
+    mockMember(member),
+    mockLocalParticipant({}),
     true,
   );
   try {
@@ -112,15 +157,11 @@ export async function withLocalMedia(
   }
 }
 
-export async function withRemoteMedia(
+export function mockRemoteParticipant(
-  member: Partial<RoomMember>,
   participant: Partial<RemoteParticipant>,
-  continuation: (vm: RemoteUserMediaViewModel) => void | Promise<void>,
+): RemoteParticipant {
-): Promise<void> {
+  return {
-  const vm = new RemoteUserMediaViewModel(
+    isLocal: false,
-    "remote",
-    mockMember(member),
-    {
     setVolume() {},
     getTrackPublication: () =>
       ({}) as Partial<RemoteTrackPublication> as RemoteTrackPublication,
@@ -137,7 +178,18 @@ export async function withRemoteMedia(
       return this;
     },
     ...participant,
-    } as RemoteParticipant,
+  } as RemoteParticipant;
+}
+
+export async function withRemoteMedia(
+  member: Partial<RoomMember>,
+  participant: Partial<RemoteParticipant>,
+  continuation: (vm: RemoteUserMediaViewModel) => void | Promise<void>,
+): Promise<void> {
+  const vm = new RemoteUserMediaViewModel(
+    "remote",
+    mockMember(member),
+    mockRemoteParticipant(participant),
     true,
   );
   try {