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

Conversation

hughns
Copy link
Member

@hughns hughns commented Nov 21, 2024

No description provided.

@robintown the tests pass, but some of the values were off by 1ms from what I was expecting. Please can you sanity check them?
@hughns hughns changed the title Refactor the speaker detection logic into observeSpeaker and add tests Use hot marbles for speaker tests Nov 21, 2024
@hughns hughns marked this pull request as ready for review November 22, 2024 12:43
@hughns hughns requested a review from a team as a code owner November 22, 2024 12:43
Comment on lines +192 to +200
.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);
}
});
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

Base automatically changed from hughns/observe-speaker to livekit November 23, 2024 08:59
@hughns hughns merged commit fc8da6e into livekit Nov 23, 2024
7 checks passed
@hughns hughns deleted the hughns/hot-speaking-marbles branch November 23, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants