admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix P2PTransportChannel unit tests to not rely on connections ordering

Browse Source 
Tests currently rely on the sorted order of connections held within the ICE controller, which sorts the connections by usability. The internal ordering is not part of the ICE controller contract.

Tests use the ordering as a proxy for certain expectations, so changed the tests to explicitly test the expectations.

Bug: webrtc:14367, webrtc:1413
Change-Id: Iaf33c61f6eb968c2c93a0265b6c48ad6218e23a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275304
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Sameer Vijaykar <samvi@google.com>
Cr-Commit-Position: refs/heads/main@{#38088}
This commit is contained in:
Sameer Vijaykar 2022-09-13 16:19:22 +02:00 committed by WebRTC LUCI CQ
parent a01e7275c2
commit 282de03603
1 changed files with 52 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -241,6 +241,16 @@ class ResolverFactoryFixture : public webrtc::MockAsyncDnsResolverFactory {
std::function<void()> saved_callback_;
};
bool HasLocalAddress(const cricket::CandidatePairInterface* pair,
const SocketAddress& address) {
return pair->local_candidate().address().EqualIPs(address);
}
bool HasRemoteAddress(const cricket::CandidatePairInterface* pair,
const SocketAddress& address) {
return pair->remote_candidate().address().EqualIPs(address);
}
} // namespace
namespace cricket {
@ -1696,8 +1706,7 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveRemoteCandidateIsSanitized) {
auto updated_pair_ep1 = ep1_ch1()->GetSelectedCandidatePair();
ASSERT_TRUE(updated_pair_ep1.has_value());
EXPECT_EQ(LOCAL_PORT_TYPE, updated_pair_ep1->remote_candidate().type());
EXPECT_TRUE(
updated_pair_ep1->remote_candidate().address().EqualIPs(kPublicAddrs[1]));
EXPECT_TRUE(HasRemoteAddress(&updated_pair_ep1.value(), kPublicAddrs[1]));
ep1_ch1()->GetStats(&ice_transport_stats);
// Check the candidate pair stats.
@ -2507,7 +2516,7 @@ class P2PTransportChannelMultihomedTest : public P2PTransportChannelTestBase {
P2PTransportChannel* channel,
const SocketAddress& address) {
for (Connection* conn : channel->connections()) {
if (conn->remote_candidate().address().EqualIPs(address)) {
if (HasRemoteAddress(conn, address)) {
return conn;
}
}
@ -2517,7 +2526,7 @@ class P2PTransportChannelMultihomedTest : public P2PTransportChannelTestBase {
Connection* GetConnectionWithLocalAddress(P2PTransportChannel* channel,
const SocketAddress& address) {
for (Connection* conn : channel->connections()) {
if (conn->local_candidate().address().EqualIPs(address)) {
if (HasLocalAddress(conn, address)) {
return conn;
}
}
@ -2528,14 +2537,33 @@ class P2PTransportChannelMultihomedTest : public P2PTransportChannelTestBase {
const SocketAddress& local,
const SocketAddress& remote) {
for (Connection* conn : channel->connections()) {
if (conn->local_candidate().address().EqualIPs(local) &&
conn->remote_candidate().address().EqualIPs(remote)) {
if (HasLocalAddress(conn, local) && HasRemoteAddress(conn, remote)) {
return conn;
}
}
return nullptr;
}
Connection* GetBestConnection(P2PTransportChannel* channel) {
rtc::ArrayView<Connection*> connections = channel->connections();
auto it = absl::c_find(connections, channel->selected_connection());
if (it == connections.end()) {
return nullptr;
}
return *it;
}
Connection* GetBackupConnection(P2PTransportChannel* channel) {
rtc::ArrayView<Connection*> connections = channel->connections();
auto it = absl::c_find_if_not(connections, [channel](Connection* conn) {
return conn == channel->selected_connection();
});
if (it == connections.end()) {
return nullptr;
}
return *it;
}
void DestroyAllButBestConnection(P2PTransportChannel* channel) {
const Connection* selected_connection = channel->selected_connection();
// Copy the list of connections since the original will be modified.
@ -3011,7 +3039,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPingBackupConnectionRate) {
1000);
auto connections = ep2_ch1()->connections();
ASSERT_EQ(2U, connections.size());
Connection* backup_conn = connections[1];
Connection* backup_conn = GetBackupConnection(ep2_ch1());
EXPECT_TRUE_WAIT(backup_conn->writable(), kMediumTimeout);
int64_t last_ping_response_ms = backup_conn->last_ping_response_received();
EXPECT_TRUE_WAIT(
@ -3052,7 +3080,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestStableWritableRate) {
1000);
auto connections = ep2_ch1()->connections();
ASSERT_EQ(2U, connections.size());
Connection* conn = connections[0];
Connection* conn = GetBestConnection(ep2_ch1());
EXPECT_TRUE_WAIT(conn->writable(), kMediumTimeout);
int64_t last_ping_response_ms;
@ -3153,10 +3181,10 @@ TEST_F(P2PTransportChannelMultihomedTest,
AddAddress(1, wifi[1], "test_wifi1", rtc::ADAPTER_TYPE_WIFI);
const Connection* conn;
EXPECT_TRUE_WAIT((conn = ep1_ch1()->selected_connection()) != nullptr &&
conn->remote_candidate().address().EqualIPs(wifi[1]),
HasRemoteAddress(conn, wifi[1]),
kDefaultTimeout);
EXPECT_TRUE_WAIT((conn = ep2_ch1()->selected_connection()) != nullptr &&
conn->local_candidate().address().EqualIPs(wifi[1]),
HasLocalAddress(conn, wifi[1]),
kDefaultTimeout);
// Add a new cellular interface on end point 1, we should expect a new
@ -3164,15 +3192,23 @@ TEST_F(P2PTransportChannelMultihomedTest,
AddAddress(0, cellular[0], "test_cellular0", rtc::ADAPTER_TYPE_CELLULAR);
EXPECT_TRUE_WAIT(
ep1_ch1()->GetState() == IceTransportState::STATE_COMPLETED &&
(conn = GetConnectionWithLocalAddress(ep1_ch1(), cellular[0])) !=
nullptr &&
conn != ep1_ch1()->selected_connection() && conn->writable(),
absl::c_any_of(ep1_ch1()->connections(),
[channel = ep1_ch1(),
address = cellular[0]](const Connection* conn) {
return HasLocalAddress(conn, address) &&
conn != channel->selected_connection() &&
conn->writable();
}),
kDefaultTimeout);
EXPECT_TRUE_WAIT(
ep2_ch1()->GetState() == IceTransportState::STATE_COMPLETED &&
(conn = GetConnectionWithRemoteAddress(ep2_ch1(), cellular[0])) !=
nullptr &&
conn != ep2_ch1()->selected_connection() && conn->receiving(),
absl::c_any_of(ep2_ch1()->connections(),
[channel = ep2_ch1(),
address = cellular[0]](const Connection* conn) {
return HasRemoteAddress(conn, address) &&
conn != channel->selected_connection() &&
conn->receiving();
}),
kDefaultTimeout);
DestroyChannels();