-
Notifications
You must be signed in to change notification settings - Fork 27
FfiClient state guarding, additional testing coverage #125
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
base: main
Are you sure you want to change the base?
Changes from all commits
090b1a4
8fd5c11
4068f09
a0362ab
85646a8
102300d
67c220e
d0b703f
af908da
2850627
62bee73
1827d7a
7414f2e
60845b4
0728ddd
a68e984
fba56fb
a56bec1
2d8817f
7a830bd
af15cb8
8273678
9159f3e
92159e9
18a6b12
59276e3
a0b7602
39012ef
893871a
8fc5988
1e9a7dc
79c7841
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,78 +19,8 @@ | |
|
|
||
| namespace livekit::test { | ||
|
|
||
| class RoomTest : public ::testing::Test { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI for review: these aren't deleted -- moved to a same-named unit test (didn't depend on server). I am starting a convention of the same files/test names across test modalities, i.e.:
Since they are entirely different built binaries, no conflicts, and consistency for what's under test from the output.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FMU why did we remove these?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to preempt this question but maybe wasn't visible in your review diff: #125 (comment) |
||
| protected: | ||
| void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } | ||
|
|
||
| void TearDown() override { livekit::shutdown(); } | ||
| }; | ||
|
|
||
| TEST_F(RoomTest, CreateRoom) { | ||
| Room room; | ||
| // Room should be created without issues | ||
| EXPECT_EQ(room.localParticipant(), nullptr) << "Local participant should be null before connect"; | ||
| } | ||
|
|
||
| TEST_F(RoomTest, RoomOptionsDefaults) { | ||
| RoomOptions options; | ||
|
|
||
| EXPECT_TRUE(options.auto_subscribe) << "auto_subscribe should default to true"; | ||
| EXPECT_FALSE(options.dynacast) << "dynacast should default to false"; | ||
| EXPECT_FALSE(options.rtc_config.has_value()) << "rtc_config should not have a value by default"; | ||
| EXPECT_FALSE(options.encryption.has_value()) << "encryption should not have a value by default"; | ||
| } | ||
|
|
||
| TEST_F(RoomTest, RtcConfigDefaults) { | ||
| RtcConfig config; | ||
|
|
||
| EXPECT_EQ(config.ice_transport_type, 0); | ||
| EXPECT_EQ(config.continual_gathering_policy, 0); | ||
| EXPECT_TRUE(config.ice_servers.empty()); | ||
| } | ||
|
|
||
| TEST_F(RoomTest, IceServerConfiguration) { | ||
| IceServer server; | ||
| server.url = "stun:stun.l.google.com:19302"; | ||
| server.username = "user"; | ||
| server.credential = "pass"; | ||
|
|
||
| EXPECT_EQ(server.url, "stun:stun.l.google.com:19302"); | ||
| EXPECT_EQ(server.username, "user"); | ||
| EXPECT_EQ(server.credential, "pass"); | ||
| } | ||
|
|
||
| TEST_F(RoomTest, RoomWithCustomRtcConfig) { | ||
| RoomOptions options; | ||
| options.auto_subscribe = false; | ||
| options.dynacast = true; | ||
|
|
||
| RtcConfig rtc_config; | ||
| rtc_config.ice_servers.push_back({"stun:stun.l.google.com:19302", "", ""}); | ||
| rtc_config.ice_servers.push_back({"turn:turn.example.com:3478", "user", "pass"}); | ||
|
|
||
| options.rtc_config = rtc_config; | ||
|
|
||
| EXPECT_FALSE(options.auto_subscribe); | ||
| EXPECT_TRUE(options.dynacast); | ||
| EXPECT_TRUE(options.rtc_config.has_value()); | ||
| EXPECT_EQ(options.rtc_config->ice_servers.size(), 2); | ||
| } | ||
|
|
||
| TEST_F(RoomTest, RemoteParticipantsEmptyBeforeConnect) { | ||
| Room room; | ||
| auto participants = room.remoteParticipants(); | ||
| EXPECT_TRUE(participants.empty()) << "Remote participants should be empty before connect"; | ||
| } | ||
|
|
||
| TEST_F(RoomTest, RemoteParticipantLookupBeforeConnect) { | ||
| Room room; | ||
| auto participant = room.remoteParticipant("nonexistent"); | ||
| EXPECT_EQ(participant, nullptr) << "Looking up participant before connect should return nullptr"; | ||
| } | ||
|
|
||
| // Server-dependent tests - require LIVEKIT_URL and LIVEKIT_TOKEN_A env vars | ||
| class RoomServerTest : public ::testing::Test { | ||
| class RoomTest : public ::testing::Test { | ||
| protected: | ||
| void SetUp() override { | ||
| livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); | ||
|
|
@@ -112,12 +42,7 @@ class RoomServerTest : public ::testing::Test { | |
| std::string token_; | ||
| }; | ||
|
|
||
| TEST_F(RoomServerTest, ConnectToServer) { | ||
| if (!server_available_) { | ||
| GTEST_SKIP() << "LIVEKIT_URL and LIVEKIT_TOKEN_A not set, skipping server " | ||
| "connection test"; | ||
| } | ||
|
|
||
| TEST_F(RoomTest, ConnectToServer) { | ||
| Room room; | ||
| RoomOptions options; | ||
|
|
||
|
|
@@ -129,19 +54,15 @@ TEST_F(RoomServerTest, ConnectToServer) { | |
| } | ||
| } | ||
|
|
||
| TEST_F(RoomServerTest, ConnectWithInvalidToken) { | ||
| if (!server_available_) { | ||
| GTEST_SKIP() << "LIVEKIT_URL not set, skipping invalid token test"; | ||
| } | ||
|
|
||
| TEST_F(RoomTest, ConnectWithInvalidToken) { | ||
| Room room; | ||
| RoomOptions options; | ||
|
|
||
| bool connected = room.Connect(server_url_, "invalid_token", options); | ||
| EXPECT_FALSE(connected) << "Should fail to connect with invalid token"; | ||
| } | ||
|
|
||
| TEST_F(RoomServerTest, ConnectWithInvalidUrl) { | ||
| TEST_F(RoomTest, ConnectWithInvalidUrl) { | ||
| Room room; | ||
| RoomOptions options; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * Copyright 2026 LiveKit | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include <gtest/gtest.h> | ||
| #include <livekit/audio_source.h> | ||
| #include <livekit/livekit.h> | ||
|
|
||
| namespace livekit::test { | ||
|
|
||
| class AudioSourceTest : public ::testing::Test { | ||
| protected: | ||
| void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } | ||
| void TearDown() override { livekit::shutdown(); } | ||
| }; | ||
|
|
||
| TEST_F(AudioSourceTest, ConstructAndQueryProperties) { | ||
| AudioSource source(48000, 1); | ||
| EXPECT_EQ(source.sample_rate(), 48000); | ||
| EXPECT_EQ(source.num_channels(), 1); | ||
| EXPECT_NE(source.ffi_handle_id(), 0u); | ||
| EXPECT_DOUBLE_EQ(source.queuedDuration(), 0.0); | ||
| } | ||
|
|
||
| TEST_F(AudioSourceTest, ClearQueueIsSafeOnFreshSource) { | ||
| AudioSource source(48000, 2, /*queue_size_ms=*/0); | ||
| source.clearQueue(); | ||
| EXPECT_DOUBLE_EQ(source.queuedDuration(), 0.0); | ||
| } | ||
|
|
||
| } // namespace livekit::test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /* | ||
| * Copyright 2026 LiveKit | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include <gtest/gtest.h> | ||
| #include <livekit/data_stream.h> | ||
|
|
||
| #include <type_traits> | ||
|
|
||
| namespace livekit::test { | ||
|
|
||
| TEST(DataStreamTest, TextStreamReaderConstructAndInfo) { | ||
| TextStreamInfo info; | ||
| info.stream_id = "stream-1"; | ||
| info.mime_type = "text/plain"; | ||
| info.topic = "chat"; | ||
| info.timestamp = 42; | ||
| info.attachments = {"a.txt"}; | ||
|
|
||
| TextStreamReader reader(info); | ||
| EXPECT_EQ(reader.info().stream_id, "stream-1"); | ||
| EXPECT_EQ(reader.info().mime_type, "text/plain"); | ||
| EXPECT_EQ(reader.info().topic, "chat"); | ||
| EXPECT_EQ(reader.info().timestamp, 42); | ||
| ASSERT_EQ(reader.info().attachments.size(), 1u); | ||
| EXPECT_EQ(reader.info().attachments.front(), "a.txt"); | ||
| } | ||
|
|
||
| TEST(DataStreamTest, ByteStreamReaderConstructAndInfo) { | ||
| ByteStreamInfo info; | ||
| info.stream_id = "stream-2"; | ||
| info.mime_type = "application/octet-stream"; | ||
| info.topic = "files"; | ||
| info.name = "data.bin"; | ||
|
|
||
| ByteStreamReader reader(info); | ||
| EXPECT_EQ(reader.info().stream_id, "stream-2"); | ||
| EXPECT_EQ(reader.info().name, "data.bin"); | ||
| } | ||
|
|
||
| TEST(DataStreamTest, WriterTypesAreDerivedFromBase) { | ||
| static_assert(std::is_base_of_v<BaseStreamWriter, TextStreamWriter>); | ||
| static_assert(std::is_base_of_v<BaseStreamWriter, ByteStreamWriter>); | ||
| EXPECT_GT(kStreamChunkSize, 0u); | ||
| } | ||
|
|
||
| } // namespace livekit::test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Copyright 2026 LiveKit | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include <gtest/gtest.h> | ||
| #include <livekit/data_track_error.h> | ||
|
|
||
| #include "data_track.pb.h" | ||
|
|
||
| namespace livekit::test { | ||
|
|
||
| TEST(DataTrackErrorTest, PublishErrorFromEmptyProto) { | ||
| proto::PublishDataTrackError proto_err; | ||
| PublishDataTrackError err = PublishDataTrackError::fromProto(proto_err); | ||
| EXPECT_EQ(err.code, PublishDataTrackErrorCode::UNKNOWN); | ||
| } | ||
|
|
||
| TEST(DataTrackErrorTest, TryPushErrorFromEmptyProto) { | ||
| proto::LocalDataTrackTryPushError proto_err; | ||
| LocalDataTrackTryPushError err = LocalDataTrackTryPushError::fromProto(proto_err); | ||
| EXPECT_EQ(err.code, LocalDataTrackTryPushErrorCode::UNKNOWN); | ||
| } | ||
|
|
||
| TEST(DataTrackErrorTest, SubscribeErrorFromEmptyProto) { | ||
| proto::SubscribeDataTrackError proto_err; | ||
| SubscribeDataTrackError err = SubscribeDataTrackError::fromProto(proto_err); | ||
| EXPECT_EQ(err.code, SubscribeDataTrackErrorCode::UNKNOWN); | ||
| } | ||
|
|
||
| } // namespace livekit::test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this here was critical so that different include-ers of
FfiClient.hdon't get different instances of the singleton. This came up in the unit test (lib had one, unit test binary had a different one)