Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hot marbles for speaker tests #2815

Merged
merged 6 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/state/CallViewModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
ConnectionState,
LocalParticipant,
Participant,
ParticipantEvent,
RemoteParticipant,
} from "livekit-client";
import * as ComponentsCore from "@livekit/components-core";
Expand Down Expand Up @@ -188,11 +189,15 @@ function withCallViewModel(
);
const eventsSpy = vi
.spyOn(ComponentsCore, "observeParticipantEvents")
.mockImplementation((p) =>
(speaking.get(p) ?? of(false)).pipe(
map((s) => ({ ...p, isSpeaking: s }) as Participant),
),
);
.mockImplementation((p, ...eventTypes) => {
if (eventTypes.includes(ParticipantEvent.IsSpeakingChanged)) {
return (speaking.get(p) ?? of(false)).pipe(
map((s) => ({ ...p, isSpeaking: s }) as Participant),
);
} else {
return of(p);
}
});
Comment on lines +192 to +200
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that's what was happening. Nice catch


const roomEventSelectorSpy = vi
.spyOn(ComponentsCore, "roomEventSelector")
Expand Down Expand Up @@ -407,7 +412,7 @@ test("participants stay in the same order unless to appear/disappear", () => {
});

test("spotlight speakers swap places", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ hot, schedule, expectObservable }) => {
// Go immediately into spotlight mode for the test
const modeInputMarbles = " s";
// First Bob speaks, then Dave, then Alice
Expand All @@ -424,9 +429,9 @@ test("spotlight speakers swap places", () => {
of([aliceParticipant, bobParticipant, daveParticipant]),
of(ConnectionState.Connected),
new Map([
[aliceParticipant, cold(aSpeakingInputMarbles, { y: true, n: false })],
[bobParticipant, cold(bSpeakingInputMarbles, { y: true, n: false })],
[daveParticipant, cold(dSpeakingInputMarbles, { y: true, n: false })],
[aliceParticipant, hot(aSpeakingInputMarbles, { y: true, n: false })],
[bobParticipant, hot(bSpeakingInputMarbles, { y: true, n: false })],
[daveParticipant, hot(dSpeakingInputMarbles, { y: true, n: false })],
]),
(vm) => {
schedule(modeInputMarbles, { s: () => vm.setGridMode("spotlight") });
Expand Down
19 changes: 2 additions & 17 deletions src/state/CallViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
EMPTY,
Observable,
Subject,
audit,
combineLatest,
concat,
distinctUntilChanged,
Expand Down Expand Up @@ -76,6 +75,7 @@ import { spotlightExpandedLayout } from "./SpotlightExpandedLayout";
import { oneOnOneLayout } from "./OneOnOneLayout";
import { pipLayout } from "./PipLayout";
import { EncryptionSystem } from "../e2ee/sharedKeyManagement";
import { observeSpeaker } from "./observeSpeaker";

// How long we wait after a focus switch before showing the real participant
// list again
Expand Down Expand Up @@ -248,22 +248,7 @@ class UserMedia {
livekitRoom,
);

this.speaker = this.vm.speaking.pipe(
// Require 1 s of continuous speaking to become a speaker, and 60 s of
// continuous silence to stop being considered a speaker
audit((s) =>
merge(
timer(s ? 1000 : 60000),
// If the speaking flag resets to its original value during this time,
// end the silencing window to stick with that original value
this.vm.speaking.pipe(filter((s1) => s1 !== s)),
),
),
startWith(false),
// Make this Observable hot so that the timers don't reset when you
// resubscribe
this.scope.state(),
);
this.speaker = observeSpeaker(this.vm.speaking).pipe(this.scope.state());

this.presenter = observeParticipantEvents(
participant,
Expand Down
119 changes: 119 additions & 0 deletions src/state/observeSpeaker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
*/

import { describe, test } from "vitest";

import { withTestScheduler } from "../utils/test";
import { observeSpeaker } from "./observeSpeaker";

const yesNo = {
y: true,
n: false,
};

describe("observeSpeaker", () => {
describe("does not activate", () => {
const expectedOutputMarbles = "n";
test("starts correctly", () => {
// should default to false when no input is given
const speakingInputMarbles = "";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("after no speaking", () => {
const speakingInputMarbles = "n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("with speaking for 1ms", () => {
const speakingInputMarbles = "y n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("with speaking for 999ms", () => {
const speakingInputMarbles = "y 999ms n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("with speaking intermittently", () => {
const speakingInputMarbles =
"y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("with consecutive speaking then stops speaking", () => {
const speakingInputMarbles = "y y y y y y y y y y n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});
});

describe("activates", () => {
test("after 1s", () => {
// this will active after 1s as no `n` follows it:
const speakingInputMarbles = " y";
const expectedOutputMarbles = "n 999ms y";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("speaking for 1001ms activates for 60s", () => {
const speakingInputMarbles = " y 1s n ";
const expectedOutputMarbles = "n 999ms y 60s n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});

test("speaking for 5s activates for 64s", () => {
const speakingInputMarbles = " y 5s n ";
const expectedOutputMarbles = "n 999ms y 64s n";
withTestScheduler(({ hot, expectObservable }) => {
expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
expectedOutputMarbles,
yesNo,
);
});
});
});
});
36 changes: 36 additions & 0 deletions src/state/observeSpeaker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
*/
import {
Observable,
audit,
merge,
timer,
filter,
startWith,
distinctUntilChanged,
} from "rxjs";

/**
* Require 1 second of continuous speaking to become a speaker, and 60 second of
* continuous silence to stop being considered a speaker
*/
export function observeSpeaker(
isSpeakingObservable: Observable<boolean>,
): Observable<boolean> {
const distinct = isSpeakingObservable.pipe(distinctUntilChanged());

return distinct.pipe(
// Either change to the new value after the timer or re-emit the same value if it toggles back
// (audit will return the latest (toggled back) value) before the timeout.
audit((s) =>
merge(timer(s ? 1000 : 60000), distinct.pipe(filter((s1) => s1 !== s))),
),
// Filter the re-emissions (marked as: | ) that happen if we toggle quickly (<1s) from false->true->false|->..
startWith(false),
distinctUntilChanged(),
);
}