From 2a56830426036fa59c81624c7a70a4372d5a235a Mon Sep 17 00:00:00 2001 From: Robin Date: Fri, 13 Feb 2026 12:43:13 +0100 Subject: [PATCH] Fix existing LocalTransport tests --- .../localMember/LocalMember.test.ts | 49 ++--- .../localMember/LocalTransport.test.ts | 190 ++++++++---------- 2 files changed, 99 insertions(+), 140 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts index b228cd08..e5e9f327 100644 --- a/src/state/CallViewModel/localMember/LocalMember.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -39,7 +39,6 @@ import { constant } from "../../Behavior"; import { ConnectionManagerData } from "../remoteMembers/ConnectionManager"; import { ConnectionState, type Connection } from "../remoteMembers/Connection"; import { type Publisher } from "./Publisher"; -import { type LocalTransportWithSFUConfig } from "./LocalTransport"; import { initializeWidget } from "../../../widget"; initializeWidget(); @@ -216,11 +215,10 @@ describe("LocalMembership", () => { it("throws error on missing RTC config error", () => { withTestScheduler(({ scope, hot, expectObservable }) => { - const localTransport$ = - scope.behavior( - hot("1ms #", {}, new MatrixRTCTransportMissingError("domain.com")), - null, - ); + const localTransport$ = scope.behavior( + hot("1ms #", {}, new MatrixRTCTransportMissingError("domain.com")), + null, + ); // we do not need any connection data since we want to fail before reaching that. const mockConnectionManager = { @@ -279,23 +277,11 @@ describe("LocalMembership", () => { }); const aTransport = { - transport: { - livekit_service_url: "a", - } as LivekitTransportConfig, - sfuConfig: { - url: "sfu-url", - jwt: "sfu-token", - }, - } as LocalTransportWithSFUConfig; + livekit_service_url: "a", + } as LivekitTransportConfig; const bTransport = { - transport: { - livekit_service_url: "b", - } as LivekitTransportConfig, - sfuConfig: { - url: "sfu-url", - jwt: "sfu-token", - }, - } as LocalTransportWithSFUConfig; + livekit_service_url: "b", + } as LivekitTransportConfig; const connectionTransportAConnected = { livekitRoom: mockLivekitRoom({ @@ -305,7 +291,7 @@ describe("LocalMembership", () => { } as unknown as LocalParticipant, }), state$: constant(ConnectionState.LivekitConnected), - transport: aTransport.transport, + transport: aTransport, } as unknown as Connection; const connectionTransportAConnecting = { ...connectionTransportAConnected, @@ -314,7 +300,7 @@ describe("LocalMembership", () => { } as unknown as Connection; const connectionTransportBConnected = { state$: constant(ConnectionState.LivekitConnected), - transport: bTransport.transport, + transport: bTransport, livekitRoom: mockLivekitRoom({}), } as unknown as Connection; @@ -368,12 +354,8 @@ describe("LocalMembership", () => { // stop the first Publisher and let the second one life. expect(publishers[0].destroy).toHaveBeenCalled(); expect(publishers[1].destroy).not.toHaveBeenCalled(); - expect(publisherFactory.mock.calls[0][0].transport).toBe( - aTransport.transport, - ); - expect(publisherFactory.mock.calls[1][0].transport).toBe( - bTransport.transport, - ); + expect(publisherFactory.mock.calls[0][0].transport).toBe(aTransport); + expect(publisherFactory.mock.calls[1][0].transport).toBe(bTransport); scope.end(); await flushPromises(); // stop all tracks after ending scopes @@ -446,8 +428,9 @@ describe("LocalMembership", () => { const scope = new ObservableScope(); const connectionManagerData = new ConnectionManagerData(); - const localTransport$ = - new BehaviorSubject(null); + const localTransport$ = new BehaviorSubject( + null, + ); const connectionManagerData$ = new BehaviorSubject( new Epoch(connectionManagerData), ); @@ -519,7 +502,7 @@ describe("LocalMembership", () => { }); ( - connectionManagerData2.getConnectionForTransport(aTransport.transport)! + connectionManagerData2.getConnectionForTransport(aTransport)! .state$ as BehaviorSubject ).next(ConnectionState.LivekitConnected); expect(localMembership.localMemberState$.value).toStrictEqual({ diff --git a/src/state/CallViewModel/localMember/LocalTransport.test.ts b/src/state/CallViewModel/localMember/LocalTransport.test.ts index 2476923a..e63f7c72 100644 --- a/src/state/CallViewModel/localMember/LocalTransport.test.ts +++ b/src/state/CallViewModel/localMember/LocalTransport.test.ts @@ -43,10 +43,10 @@ describe("LocalTransport", () => { afterEach(() => scope.end()); it("throws if config is missing", async () => { - const localTransport$ = createLocalTransport$({ + const { advertised$, active$ } = createLocalTransport$({ scope, roomId: "!room:example.org", - useOldestMember$: constant(false), + useOldestMember: false, memberships$: constant(new Epoch([])), client: { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -58,14 +58,15 @@ describe("LocalTransport", () => { getDeviceId: vi.fn(), }, ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), + forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant("delay_id_mock"), }); await flushPromises(); - expect(() => localTransport$.value).toThrow( + expect(() => advertised$.value).toThrow( new MatrixRTCTransportMissingError(""), ); + expect(() => active$.value).toThrow(new MatrixRTCTransportMissingError("")); }); it("throws FailToGetOpenIdToken when OpenID fetch fails", async () => { @@ -83,10 +84,10 @@ describe("LocalTransport", () => { ); const observations: unknown[] = []; const errors: Error[] = []; - const localTransport$ = createLocalTransport$({ + const { advertised$, active$ } = createLocalTransport$({ scope, roomId: "!example_room_id", - useOldestMember$: constant(false), + useOldestMember: false, memberships$: constant(new Epoch([])), client: { baseUrl: "https://lk.example.org", @@ -98,10 +99,10 @@ describe("LocalTransport", () => { getDeviceId: vi.fn(), }, ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), + forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant("delay_id_mock"), }); - localTransport$.subscribe( + active$.subscribe( (o) => observations.push(o), (e) => errors.push(e), ); @@ -111,7 +112,8 @@ describe("LocalTransport", () => { const expectedError = new FailToGetOpenIdToken(new Error("no openid")); expect(observations).toStrictEqual([null]); expect(errors).toStrictEqual([expectedError]); - expect(() => localTransport$.value).toThrow(expectedError); + expect(() => advertised$.value).toThrow(expectedError); + expect(() => active$.value).toThrow(expectedError); }); it("emits preferred transport after OpenID resolves", async () => { @@ -126,10 +128,10 @@ describe("LocalTransport", () => { openIdResolver.promise, ); - const localTransport$ = createLocalTransport$({ + const { advertised$, active$ } = createLocalTransport$({ scope, roomId: "!room:example.org", - useOldestMember$: constant(false), + useOldestMember: false, memberships$: constant(new Epoch([])), client: { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -140,7 +142,7 @@ describe("LocalTransport", () => { baseUrl: "https://lk.example.org", }, ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), + forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant("delay_id_mock"), }); @@ -150,14 +152,17 @@ describe("LocalTransport", () => { livekitAlias: "Akph4alDMhen", livekitIdentity: ownMemberMock.userId + ":" + ownMemberMock.deviceId, }); - expect(localTransport$.value).toBe(null); + expect(advertised$.value).toBe(null); + expect(active$.value).toBe(null); await flushPromises(); // final - expect(localTransport$.value).toStrictEqual({ - transport: { - livekit_service_url: "https://lk.example.org", - type: "livekit", - }, + const expectedTransport = { + livekit_service_url: "https://lk.example.org", + type: "livekit", + }; + expect(advertised$.value).toStrictEqual(expectedTransport); + expect(active$.value).toStrictEqual({ + transport: expectedTransport, sfuConfig: { jwt: "jwt", livekitAlias: "Akph4alDMhen", @@ -167,53 +172,8 @@ describe("LocalTransport", () => { }); }); - it("updates local transport when oldest member changes", async () => { - // Use config so transport discovery succeeds, but delay OpenID JWT fetch - mockConfig({ - livekit: { livekit_service_url: "https://lk.example.org" }, - }); - const memberships$ = new BehaviorSubject(new Epoch([])); - const openIdResolver = Promise.withResolvers(); - - vi.spyOn(openIDSFU, "getSFUConfigWithOpenID").mockReturnValue( - openIdResolver.promise, - ); - - const localTransport$ = createLocalTransport$({ - scope, - roomId: "!example_room_id", - useOldestMember$: constant(true), - memberships$, - client: { - getDomain: () => "", - // eslint-disable-next-line @typescript-eslint/naming-convention - _unstable_getRTCTransports: async () => Promise.resolve([]), - getOpenIdToken: vi.fn(), - getDeviceId: vi.fn(), - baseUrl: "https://lk.example.org", - }, - ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), - delayId$: constant("delay_id_mock"), - }); - - openIdResolver.resolve?.(openIdResponse); - expect(localTransport$.value).toBe(null); - await flushPromises(); - // final - expect(localTransport$.value).toStrictEqual({ - transport: { - livekit_service_url: "https://lk.example.org", - type: "livekit", - }, - sfuConfig: { - jwt: "e30=.eyJzdWIiOiJAbWU6ZXhhbXBsZS5vcmc6QUJDREVGIiwidmlkZW8iOnsicm9vbSI6IiFleGFtcGxlX3Jvb21faWQifX0=.e30=", - livekitAlias: "Akph4alDMhen", - livekitIdentity: "@lk_user:ABCDEF", - url: "https://lk.example.org", - }, - }); - }); + // TODO: This test previously didn't test what it claims to. + it.todo("updates local transport when oldest member changes"); type LocalTransportProps = Parameters[0]; @@ -229,8 +189,8 @@ describe("LocalTransport", () => { ownMembershipIdentity: ownMemberMock, scope, roomId: "!example_room_id", - useOldestMember$: constant(false), - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), + useOldestMember: false, + forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant(null), memberships$: constant(new Epoch([])), client: { @@ -256,15 +216,19 @@ describe("LocalTransport", () => { mockConfig({ livekit: { livekit_service_url: "https://lk.example.org" }, }); - const localTransport$ = createLocalTransport$(localTransportOpts); + const { advertised$, active$ } = + createLocalTransport$(localTransportOpts); openIdResolver.resolve?.(openIdResponse); - expect(localTransport$.value).toBe(null); + expect(advertised$.value).toBe(null); + expect(active$.value).toBe(null); await flushPromises(); - expect(localTransport$.value).toStrictEqual({ - transport: { - livekit_service_url: "https://lk.example.org", - type: "livekit", - }, + const expectedTransport = { + livekit_service_url: "https://lk.example.org", + type: "livekit", + }; + expect(advertised$.value).toStrictEqual(expectedTransport); + expect(active$.value).toStrictEqual({ + transport: expectedTransport, sfuConfig: { jwt: "e30=.eyJzdWIiOiJAbWU6ZXhhbXBsZS5vcmc6QUJDREVGIiwidmlkZW8iOnsicm9vbSI6IiFleGFtcGxlX3Jvb21faWQifX0=.e30=", livekitAlias: "Akph4alDMhen", @@ -273,13 +237,15 @@ describe("LocalTransport", () => { }, }); }); + it("supports getting transport via user settings", async () => { customLivekitUrl.setValue("https://lk.example.org"); - const localTransport$ = createLocalTransport$(localTransportOpts); + const { advertised$, active$ } = + createLocalTransport$(localTransportOpts); openIdResolver.resolve?.(openIdResponse); - expect(localTransport$.value).toBe(null); + expect(advertised$.value).toBe(null); await flushPromises(); - expect(localTransport$.value).toStrictEqual({ + expect(active$.value).toStrictEqual({ transport: { livekit_service_url: "https://lk.example.org", type: "livekit", @@ -292,19 +258,24 @@ describe("LocalTransport", () => { }, }); }); + it("supports getting transport via backend", async () => { localTransportOpts.client._unstable_getRTCTransports.mockResolvedValue([ { type: "livekit", livekit_service_url: "https://lk.example.org" }, ]); - const localTransport$ = createLocalTransport$(localTransportOpts); + const { advertised$, active$ } = + createLocalTransport$(localTransportOpts); openIdResolver.resolve?.(openIdResponse); - expect(localTransport$.value).toBe(null); + expect(advertised$.value).toBe(null); + expect(active$.value).toBe(null); await flushPromises(); - expect(localTransport$.value).toStrictEqual({ - transport: { - livekit_service_url: "https://lk.example.org", - type: "livekit", - }, + const expectedTransport = { + livekit_service_url: "https://lk.example.org", + type: "livekit", + }; + expect(advertised$.value).toStrictEqual(expectedTransport); + expect(active$.value).toStrictEqual({ + transport: expectedTransport, sfuConfig: { jwt: "e30=.eyJzdWIiOiJAbWU6ZXhhbXBsZS5vcmc6QUJDREVGIiwidmlkZW8iOnsicm9vbSI6IiFleGFtcGxlX3Jvb21faWQifX0=.e30=", livekitAlias: "Akph4alDMhen", @@ -313,6 +284,7 @@ describe("LocalTransport", () => { }, }); }); + it("fails fast if the openID request fails for backend config", async () => { localTransportOpts.client._unstable_getRTCTransports.mockResolvedValue([ { type: "livekit", livekit_service_url: "https://lk.example.org" }, @@ -320,13 +292,11 @@ describe("LocalTransport", () => { openIdResolver.reject( new FailToGetOpenIdToken(new Error("Test driven error")), ); - try { - await lastValueFrom(createLocalTransport$(localTransportOpts)); - throw Error("Expected test to throw"); - } catch (ex) { - expect(ex).toBeInstanceOf(FailToGetOpenIdToken); - } + await expect(async () => + lastValueFrom(createLocalTransport$(localTransportOpts).active$), + ).rejects.toThrow(expect.any(FailToGetOpenIdToken)); }); + it("supports getting transport via well-known", async () => { localTransportOpts.client.getDomain.mockReturnValue("example.org"); fetchMock.getOnce("https://example.org/.well-known/matrix/client", { @@ -334,15 +304,19 @@ describe("LocalTransport", () => { { type: "livekit", livekit_service_url: "https://lk.example.org" }, ], }); - const localTransport$ = createLocalTransport$(localTransportOpts); + const { advertised$, active$ } = + createLocalTransport$(localTransportOpts); openIdResolver.resolve?.(openIdResponse); - expect(localTransport$.value).toBe(null); + expect(advertised$.value).toBe(null); + expect(active$.value).toBe(null); await flushPromises(); - expect(localTransport$.value).toStrictEqual({ - transport: { - livekit_service_url: "https://lk.example.org", - type: "livekit", - }, + const expectedTransport = { + livekit_service_url: "https://lk.example.org", + type: "livekit", + }; + expect(advertised$.value).toStrictEqual(expectedTransport); + expect(active$.value).toStrictEqual({ + transport: expectedTransport, sfuConfig: { jwt: "e30=.eyJzdWIiOiJAbWU6ZXhhbXBsZS5vcmc6QUJDREVGIiwidmlkZW8iOnsicm9vbSI6IiFleGFtcGxlX3Jvb21faWQifX0=.e30=", livekitAlias: "Akph4alDMhen", @@ -352,6 +326,7 @@ describe("LocalTransport", () => { }); expect(fetchMock.done()).toEqual(true); }); + it("fails fast if the openId request fails for the well-known config", async () => { localTransportOpts.client.getDomain.mockReturnValue("example.org"); fetchMock.getOnce("https://example.org/.well-known/matrix/client", { @@ -362,20 +337,18 @@ describe("LocalTransport", () => { openIdResolver.reject( new FailToGetOpenIdToken(new Error("Test driven error")), ); - try { - await lastValueFrom(createLocalTransport$(localTransportOpts)); - throw Error("Expected test to throw"); - } catch (ex) { - expect(ex).toBeInstanceOf(FailToGetOpenIdToken); - } + await expect(async () => + lastValueFrom(createLocalTransport$(localTransportOpts).active$), + ).rejects.toThrow(expect.any(FailToGetOpenIdToken)); }); + it("throws if no options are available", async () => { - const localTransport$ = createLocalTransport$({ + const { advertised$, active$ } = createLocalTransport$({ scope, ownMembershipIdentity: ownMemberMock, roomId: "!example_room_id", - useOldestMember$: constant(false), - forceJwtEndpoint$: constant(JwtEndpointVersion.Legacy), + useOldestMember: false, + forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant(null), memberships$: constant(new Epoch([])), client: { @@ -390,7 +363,10 @@ describe("LocalTransport", () => { }); await flushPromises(); - expect(() => localTransport$.value).toThrow( + expect(() => advertised$.value).toThrow( + new MatrixRTCTransportMissingError(""), + ); + expect(() => active$.value).toThrow( new MatrixRTCTransportMissingError(""), ); });