From 8e80ee097a6ced22f873331bc150b23189f64c1f Mon Sep 17 00:00:00 2001 From: hai007 Date: Wed, 27 Apr 2022 01:19:15 -0700 Subject: [PATCH] Unit test for WIFI Hotspot in Connection Medium layer Fix 230126796: go/tsan error (data_race) found while running //third_party/nearby/internal/platform:public_test PiperOrigin-RevId: 444788086 --- .../mediums/wifi_hotspot_test.cc | 182 +++++++++++++++--- internal/platform/medium_environment.cc | 4 - internal/platform/medium_environment.h | 3 - internal/platform/wifi_hotspot_test.cc | 83 +++++++- 4 files changed, 230 insertions(+), 42 deletions(-) diff --git a/connections/implementation/mediums/wifi_hotspot_test.cc b/connections/implementation/mediums/wifi_hotspot_test.cc index 646c1b1c..2193fa02 100644 --- a/connections/implementation/mediums/wifi_hotspot_test.cc +++ b/connections/implementation/mediums/wifi_hotspot_test.cc @@ -20,6 +20,8 @@ #include "gmock/gmock.h" #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "absl/time/clock.h" +#include "internal/platform/medium_environment.h" #include "internal/platform/system_clock.h" #include "internal/platform/wifi_hotspot.h" #include "internal/platform/wifi_hotspot_credential.h" @@ -29,44 +31,170 @@ namespace nearby { namespace connections { namespace { -constexpr absl::string_view kGateway{"0.0.0.0"}; -constexpr absl::Duration kWait = absl::Milliseconds(300); +using FeatureFlags = FeatureFlags::Flags; + +constexpr FeatureFlags kTestCases[] = { + FeatureFlags{ + .enable_cancellation_flag = true, + }, + FeatureFlags{ + .enable_cancellation_flag = false, + }, +}; + +constexpr absl::string_view kServiceID{"com.google.location.nearby.apps.test"}; constexpr absl::string_view kSsid{"Direct_Nearby"}; constexpr absl::string_view kPassword{"12345678"}; +constexpr absl::string_view kIp = "123.234.23.2"; +constexpr const size_t kPort = 20; +constexpr absl::Duration kWaitDuration = absl::Milliseconds(100); -TEST(WifiHotspotTest, ConStartHotspot) { - WifiHotspot wifi_hotspot_a; - - if (wifi_hotspot_a.IsAvailable()) { - wifi_hotspot_a.StartWifiHotspot(); - SystemClock::Sleep(kWait); - wifi_hotspot_a.StopWifiHotspot(); +class WifiHotspotTest : public testing::TestWithParam { + protected: + WifiHotspotTest() { + env_.Stop(); + env_.Start(); } - // TODO(b/227482970): Add test coverage for wifi_hotspot.cc + ~WifiHotspotTest() override{ + env_.Stop(); + } + + MediumEnvironment& env_{MediumEnvironment::Instance()}; +}; + +INSTANTIATE_TEST_SUITE_P(ParametrisedWifiHotspotMediumTest, WifiHotspotTest, + testing::ValuesIn(kTestCases)); + +TEST_F(WifiHotspotTest, ConstructorDestructorWorks) { + auto wifi_hotspot_a = std::make_unique(); + auto wifi_hotspot_b = std::make_unique(); + + EXPECT_TRUE(wifi_hotspot_a->IsAvailable()); + EXPECT_TRUE(wifi_hotspot_b->IsAvailable()); + + wifi_hotspot_a.reset(); + wifi_hotspot_b.reset(); } -TEST(WifiHotspotTest, CanConnectToHotspot) { - WifiHotspot wifi_hotspot_a; +TEST_F(WifiHotspotTest, CanStartStopHotspot) { + std::string service_id(kServiceID); + auto wifi_hotspot_a = std::make_unique(); - if (wifi_hotspot_a.IsAvailable()) { - std::string ssid(kSsid); - std::string password(kPassword); - wifi_hotspot_a.ConnectWifiHotspot(ssid, password); - SystemClock::Sleep(kWait); - wifi_hotspot_a.DisconnectWifiHotspot(); - } - // TODO(b/227482970): Add test coverage for wifi_hotspot.cc + EXPECT_TRUE(wifi_hotspot_a->StartWifiHotspot()); + EXPECT_TRUE(wifi_hotspot_a->StartAcceptingConnections(service_id, {})); + absl::SleepFor(kWaitDuration); + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + wifi_hotspot_a.reset(); } -TEST(WifiHotspotTest, CanGetCredentials) { - WifiHotspot wifi_hotspot_a; +TEST_F(WifiHotspotTest, CanConnectDisconnectHotspot) { + auto wifi_hotspot_a = std::make_unique(); + std::string ssid(kSsid); + std::string password(kPassword); - if (wifi_hotspot_a.IsAvailable()) { - HotspotCredentials* hotspot_crendential = - wifi_hotspot_a.GetCredentials("TEST_SERVICE_ID"); - EXPECT_EQ(kGateway, hotspot_crendential->GetGateway()); + EXPECT_FALSE(wifi_hotspot_a->ConnectWifiHotspot(ssid, password)); + EXPECT_TRUE(wifi_hotspot_a->DisconnectWifiHotspot()); + wifi_hotspot_a.reset(); +} + +TEST_P(WifiHotspotTest, CanStartHotspotThatOtherConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + std::string service_id(kServiceID); + std::string ip(kIp); + auto wifi_hotspot_a = std::make_unique(); + auto wifi_hotspot_b = std::make_unique(); + + EXPECT_TRUE(wifi_hotspot_a->StartWifiHotspot()); + absl::SleepFor(kWaitDuration); + if (!wifi_hotspot_a->IsAcceptingConnections(service_id)) { + EXPECT_TRUE(wifi_hotspot_a->StartAcceptingConnections(service_id, {})); } - // TODO(b/227482970): Add test coverage for wifi_hotspot.cc + + HotspotCredentials* hotspot_credentials = + wifi_hotspot_a->GetCredentials(service_id); + + EXPECT_TRUE(wifi_hotspot_b->ConnectWifiHotspot( + hotspot_credentials->GetSSID(), hotspot_credentials->GetPassword())); + + WifiHotspotSocket socket_client; + EXPECT_FALSE(socket_client.IsValid()); + + CancellationFlag flag; + socket_client = wifi_hotspot_b->Connect(service_id, ip, kPort, &flag); + EXPECT_FALSE(socket_client.IsValid()); + + socket_client = + wifi_hotspot_b->Connect(service_id, hotspot_credentials->GetGateway(), + hotspot_credentials->GetPort(), &flag); + EXPECT_TRUE(socket_client.IsValid()); + + EXPECT_TRUE(wifi_hotspot_b->DisconnectWifiHotspot()); + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + wifi_hotspot_a.reset(); + wifi_hotspot_b.reset(); +} + +TEST_P(WifiHotspotTest, CanStartHotspotThatOtherCanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + + std::string service_id(kServiceID); + std::string ip(kIp); + auto wifi_hotspot_a = std::make_unique(); + auto wifi_hotspot_b = std::make_unique(); + + EXPECT_TRUE(wifi_hotspot_a->StartWifiHotspot()); + absl::SleepFor(kWaitDuration); + if (!wifi_hotspot_a->IsAcceptingConnections(service_id)) { + EXPECT_TRUE(wifi_hotspot_a->StartAcceptingConnections(service_id, {})); + } + + HotspotCredentials* hotspot_credentials = + wifi_hotspot_a->GetCredentials(service_id); + + EXPECT_TRUE(wifi_hotspot_b->ConnectWifiHotspot( + hotspot_credentials->GetSSID(), hotspot_credentials->GetPassword())); + + WifiHotspotSocket socket_client; + EXPECT_FALSE(socket_client.IsValid()); + + CancellationFlag flag(true); + socket_client = + wifi_hotspot_b->Connect(service_id, hotspot_credentials->GetGateway(), + hotspot_credentials->GetPort(), &flag); + + // If FeatureFlag is disabled, Cancelled is false as no-op. + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(socket_client.IsValid()); + EXPECT_TRUE(wifi_hotspot_b->DisconnectWifiHotspot()); + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + } else { + EXPECT_FALSE(socket_client.IsValid()); + EXPECT_TRUE(wifi_hotspot_b->DisconnectWifiHotspot()); + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + } + wifi_hotspot_a.reset(); + wifi_hotspot_b.reset(); +} + + +TEST_F(WifiHotspotTest, CanStartHotspotTheOtherFailConnect) { + auto wifi_hotspot_a = std::make_unique(); + auto wifi_hotspot_b = std::make_unique(); + + EXPECT_TRUE(wifi_hotspot_a->StartWifiHotspot()); + + std::string ssid(kSsid); + std::string password(kPassword); + + EXPECT_FALSE(wifi_hotspot_b->ConnectWifiHotspot(ssid, password)); + EXPECT_TRUE(wifi_hotspot_b->DisconnectWifiHotspot()); + + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + wifi_hotspot_a.reset(); + wifi_hotspot_b.reset(); } } // namespace diff --git a/internal/platform/medium_environment.cc b/internal/platform/medium_environment.cc index 505f4c67..22895e76 100644 --- a/internal/platform/medium_environment.cc +++ b/internal/platform/medium_environment.cc @@ -856,10 +856,6 @@ void MediumEnvironment::UpdateWifiHotspotMediumForStartOrConnect( }); } -bool MediumEnvironment::IsWifiHotspotMediumsEmpty() { - return wifi_hotspot_mediums_.empty(); -} - void MediumEnvironment::UnregisterWifiHotspotMedium( api::WifiHotspotMedium& medium) { if (!enabled_) return; diff --git a/internal/platform/medium_environment.h b/internal/platform/medium_environment.h index e47def58..e1565938 100644 --- a/internal/platform/medium_environment.h +++ b/internal/platform/medium_environment.h @@ -285,9 +285,6 @@ class MediumEnvironment { api::WifiHotspotMedium& medium, HotspotCredentials* hotspot_credentials, bool is_ap, bool enabled); - // Check if wifi_hotspot_mediums_ map is empty or not. - bool IsWifiHotspotMediumsEmpty(); - // Removes medium-related info. This should correspond to device stopped or // disconnected. void UnregisterWifiHotspotMedium(api::WifiHotspotMedium& medium); diff --git a/internal/platform/wifi_hotspot_test.cc b/internal/platform/wifi_hotspot_test.cc index 5db6f56d..1b241999 100644 --- a/internal/platform/wifi_hotspot_test.cc +++ b/internal/platform/wifi_hotspot_test.cc @@ -64,25 +64,24 @@ TEST(HotspotCredentialsTest, SetGetPassword) { EXPECT_EQ(hotspot_credentials.GetPassword(), kPassword); } -class WifiHotspotMediumTest : public ::testing::TestWithParam { +class WifiHotspotMediumTest : public testing::TestWithParam { protected: WifiHotspotMediumTest() { env_.Stop(); env_.Start(); } ~WifiHotspotMediumTest() override{ - absl::SleepFor(kWaitDuration); - EXPECT_TRUE(env_.IsWifiHotspotMediumsEmpty()); env_.Stop(); } MediumEnvironment& env_{MediumEnvironment::Instance()}; }; -INSTANTIATE_TEST_SUITE_P(ParametrisedWifiLanMediumTest, WifiHotspotMediumTest, - ::testing::ValuesIn(kTestCases)); +INSTANTIATE_TEST_SUITE_P(ParametrisedWifiHotspotMediumTest, + WifiHotspotMediumTest, + testing::ValuesIn(kTestCases)); -TEST_P(WifiHotspotMediumTest, ConstructorDestructorWorks) { +TEST_F(WifiHotspotMediumTest, ConstructorDestructorWorks) { auto wifi_hotspot_a = std::make_unique(); auto wifi_hotspot_b = std::make_unique(); @@ -123,7 +122,9 @@ TEST_F(WifiHotspotMediumTest, CanConnectDisconnectHotspot) { wifi_hotspot_a.reset(); } -TEST_F(WifiHotspotMediumTest, CanStartHotspotThatOtherConnect) { +TEST_P(WifiHotspotMediumTest, CanStartHotspotThatOtherConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); auto wifi_hotspot_a = std::make_unique(); auto wifi_hotspot_b = std::make_unique(); @@ -139,7 +140,7 @@ TEST_F(WifiHotspotMediumTest, CanStartHotspotThatOtherConnect) { wifi_hotspot_a->GetCredential()->SetIPAddress(server_socket.GetIPAddress()); WifiHotspotSocket socket_a; - WifiHotspotSocket socket_b; + WifiHotspotSocket socket_b; EXPECT_FALSE(socket_a.IsValid()); EXPECT_FALSE(socket_b.IsValid()); @@ -192,6 +193,72 @@ TEST_F(WifiHotspotMediumTest, CanStartHotspotThatOtherConnect) { wifi_hotspot_b.reset(); } +TEST_P(WifiHotspotMediumTest, CanStartHotspotThatOtherCanCancelConnect) { + FeatureFlags feature_flags = GetParam(); + env_.SetFeatureFlags(feature_flags); + auto wifi_hotspot_a = std::make_unique(); + auto wifi_hotspot_b = std::make_unique(); + + EXPECT_TRUE(wifi_hotspot_a->StartWifiHotspot()); + HotspotCredentials* hotspot_credentials = wifi_hotspot_a->GetCredential(); + absl::SleepFor(kWaitDuration); + EXPECT_TRUE(wifi_hotspot_b->ConnectWifiHotspot( + hotspot_credentials->GetSSID(), hotspot_credentials->GetPassword())); + absl::SleepFor(kWaitDuration); + + WifiHotspotServerSocket server_socket = wifi_hotspot_a->ListenForService(); + EXPECT_TRUE(server_socket.IsValid()); + wifi_hotspot_a->GetCredential()->SetIPAddress(server_socket.GetIPAddress()); + + WifiHotspotSocket socket_a; + WifiHotspotSocket socket_b; + EXPECT_FALSE(socket_a.IsValid()); + EXPECT_FALSE(socket_b.IsValid()); + + { + CancellationFlag flag(true); + SingleThreadExecutor server_executor; + SingleThreadExecutor client_executor; + client_executor.Execute( + [&wifi_hotspot_b, &socket_b, &server_socket, &flag]() { + socket_b = wifi_hotspot_b->ConnectToService(kIp, kPort, &flag); + EXPECT_FALSE(socket_b.IsValid()); + socket_b = wifi_hotspot_b->ConnectToService( + server_socket.GetIPAddress(), server_socket.GetPort(), &flag); + if (!socket_b.IsValid()) { + server_socket.Close(); + } + }); + server_executor.Execute([&socket_a, &server_socket]() { + socket_a = server_socket.Accept(); + if (!socket_a.IsValid()) { + server_socket.Close(); + } + }); + } + absl::SleepFor(kWaitDuration); + + if (!feature_flags.enable_cancellation_flag) { + EXPECT_TRUE(socket_a.IsValid()); + EXPECT_TRUE(socket_b.IsValid()); + } else { + EXPECT_FALSE(socket_a.IsValid()); + EXPECT_FALSE(socket_b.IsValid()); + } + + // absl::SleepFor(kWaitDuration); + // socket_a.Close(); + // socket_b.Close(); + + server_socket.Close(); + absl::SleepFor(kWaitDuration); + EXPECT_TRUE(wifi_hotspot_b->DisconnectWifiHotspot()); + EXPECT_TRUE(wifi_hotspot_a->StopWifiHotspot()); + absl::SleepFor(kWaitDuration); + wifi_hotspot_a.reset(); + wifi_hotspot_b.reset(); +} + TEST_F(WifiHotspotMediumTest, CanStartHotspotTheOtherFailConnect) { auto wifi_hotspot_a = std::make_unique(); auto wifi_hotspot_b = std::make_unique();